crabidy/plan/summary.md

415 lines
23 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Implementation summaries
## captures follow-up: W on queues and bookmarks (2026-07-21)
Small fix on top of the captures feature: `/queues` and `/bookmarks`
nodes are now `W`-capturable. `fsdy::Client` gained
`with_downloadable_nodes()` (instance-wide `is_downloadable`, applied to
the queues and bookmarks mounts; `/fs` and `/captures` stay off), and
the download sink softens all-or-nothing for exactly one case: a track
whose source cannot be captured — stream resolution fails, or resolves
to a non-http(s) target like a local file playable — is skipped with a
warning instead of aborting, since queue/bookmark captures mix
providers. Real download failures (bad status, transport, timeout) stay
fatal. `architecture/captures.md` D3/D4 reconciled; new tests
`downloadable_instances_flag_every_node` (fsdy) and
`download_capture_skips_uncapturable_tracks` (capture store).
## youtube-provider (2026-07-21)
Built per `plan/youtube-provider.md`: a new workspace crate **`ytdy`**
mounts YouTube at `/youtube`, backed by a `yt-dlp` subprocess (declared
in `devenv.nix`). All extraction goes through one `Engine` seam:
argv-only invocations with `--no-warnings`, an optional `--cookies`
flag, a per-call timeout (`kill_on_drop`), a 32 MiB stdout cap, and
typed `EngineError`s — tests drive the whole provider through a fake
shell-script binary, no network.
Search needs no login and mirrors tidal's search exactly: `%` on
`/youtube/search` creates an in-memory term (deduplicated, implicitly
recreated on stale paths, rename re-searches, delete idempotent), whose
node lists the top N (`ytsearchN:`, default 20) results as queueable,
downloadable tracks. With a readable cookies file configured in
`ytdy.toml` ("logged in"), a `playlists` subtree appears
(`feed/playlists` flat listing → playlist nodes with tracks); an
unreadable cookies file degrades to logged-out with a warning, never a
failed init. Streams resolve via `-f bestaudio/best -g`; captures work
end to end (`extension_for` gained `audio/webm → webm`). The
orchestrator wires `/youtube` non-fatally: a failed `--version` probe
disables the provider, nothing else.
Deviations: `Entry` keeps separate `uploader`/`channel` fields with an
`artist()` preference — the planned serde alias rejects real yt-dlp
output as a duplicate field (found by the live probe). The live probe
validated search, stream resolution, and a real download capture
(252 KB webm) through the capture store; the **playlists feed
invocation is live-unvalidated** (no cookies on this machine) — flagged
in `architecture/youtube-provider.md` as the standing risk. 150
workspace tests green (8 new in `ytdy`); every gate in
`quality/youtube-provider.md` checked.
## captures (2026-07-21)
Built per `plan/captures.md`: `W` (shift) on a downloadable library
selection captures the subtree like a bookmark, but into
`<config>/crabidy/captures/<name>/` with every track's audio
**downloaded** next to its order-prefixed toml — the toml's playable is
the audio file's *relative* name (`TrackFile::from_track_with_file`), so
a capture plays with no provider round trip and the folder stays
relocatable. `/captures` is a fourth `fsdy` instance (editable top
level, nothing reserved): browse, queue, rename (`e`), delete (`d`),
and re-capture to refresh; the audio files are invisible to listings
(only dirs and `*.cbd-track.toml` count).
The bookmark walk was extracted into `capture.rs`
(`capture_into`/`write_tree`, `Caps`, one `CaptureError` for both
stores) parameterized by a per-track `Sink``Link` is byte-identical
bookmark behavior, `Download` fetches the first `get_urls_for_track`
URL through one shared reqwest client (30 s connect timeout, 600 s
per-track deadline, no retries), streams the body to disk against a
capture-wide byte budget, picks the extension from `Content-Type` (URL
path, then `bin`, as fallbacks), and writes the toml only after the
audio succeeded. Download caps: 1 000 dirs, 500 tracks, 4 GiB. Still
all-or-nothing with temp cleanup; downloads are sequential inside the
one spawned capture task. Download error messages carry the track's
library path, never the stream URL (`reqwest::Error::without_url`).
Nodes opt in via new additive proto flags
(`LibraryNode.is_downloadable = 8`, `LibraryNodeChild = 7`). Tidal sets
them centrally at the end of `get_lib_node`: downloadable = queueable
**or lists tracks** (so search-term track results are downloadable even
though the term node isn't queueable); children mirror `is_queable`;
tracks inherit their node's flag in the TUI. The server re-enforces at
the capture root (`Unsupported` → `failed_precondition`); the rpc
gained `CaptureLibraryNodeRequest.download = 3` (additive; old clients
keep bookmarking).
Deviations from the plan/architecture: the naming helper ended up
`audio_file_name` (the path variant was clippy-dead); the tidal flag
rule grew the "or lists tracks" clause (architecture D4 reconciled);
the live probe downloaded a single real track (8.6 MB m4a,
Content-Type-derived extension, replayed through a `/captures`
instance) instead of a whole album — the multi-track walk is
unit-covered and a full album download is needlessly heavy for a smoke
test. 142 workspace tests green (1 new in `fsdy`, 8 in
`capture`/`capture_store`, 3 TUI + 1 extended); every gate in
`quality/captures.md` checked.
## bookmarks (2026-07-21)
Built per `plan/bookmarks.md`: `w` on a queueable library selection now
captures the whole subtree as a **bookmark** — a structure-preserving
snapshot under `<config>/crabidy/bookmarks/<name>/`, mounted read-only at
`/bookmarks` by a third `fsdy` instance. The capture runs on the
orchestrator (a spawned task walking `get_lib_node` iteratively across
any provider): every child node becomes an order-prefixed folder
(`fsdy::dir_name`, sharing the queue entries' sanitizer), every track an
order-prefixed link file, so the case-insensitive listing reproduces the
source order and replaying is plain fs-provider behavior. Caps (1 000
dirs / 20 000 tracks) abort cleanly with the temp folder removed; writes
are tmp-and-swap; re-capturing a name overwrites it. The wire gained one
additive rpc, `CaptureLibraryNode(path, name)` (invalid name/source →
`invalid_argument`, over-cap/disabled → `failed_precondition`). The TUI
opens the existing input overlay prefilled with the selection's title
(`bookmark`), gated on a queueable bare selection.
On top, `fsdy::Client` gained `with_editable_top_level(reserved)`:
editable instances mark their root's child folders
`is_editable`/`is_deletable` and implement rename (no-merge, validated
titles, returns the renamed node) and delete (idempotent, returns the
refreshed root). Applied to `/bookmarks` (nothing reserved) **and
`/queues`** (reserved: `current`) — saved queues are now renamable and
deletable through the existing `e`/`d` flows with zero TUI changes.
`/fs` stays immutable. All 130 workspace tests green (6 new in `fsdy`, 7
in `bookmark_store`, 2 TUI); every gate in `quality/bookmarks.md`
checked. A temporary live probe (removed after passing) captured a
19-track album from the live Tidal API, browsed it with editable flags,
renamed it, resolved it in order, and fetched a stream URL for a
captured link.
The whole feature ran autonomously per standing instruction; decisions
are recorded in `architecture/bookmarks.md` (options + rationale).
### Deviations from plan / architecture (bookmarks)
- **Capture is all-or-nothing**: any provider or write failure mid-walk
aborts the whole capture (temp folder removed) instead of skipping the
failing subtree with a warning — a bookmark that *looks* complete must
*be* complete. The architecture only specified the unreadable-*root*
case; this extends it to every node.
- **Rename to the current name is a no-op success** (returns the node),
not a collision error — the target "exists" only because it is the
source.
- **Rename targets don't pass `disk_path`**: the new folder name is
validated by `validate_folder_name` (no separators, NUL, or leading
dots), which makes it a plain sibling name by construction; the
traversal gate still covers every client-supplied *path*.
- **`track_file_name` was refactored onto a shared `ordered_name`**
helper rather than duplicated for `dir_name` (planned as "shared
sanitizer", realized as one function).
- **Environment note**: builds/tests again ran with a session-local
`CARGO_TARGET_DIR`; no repo change.
## queue-persistence (2026-07-21)
Built per `plan/queue-persistence.md`: queues now survive server restarts,
realized entirely on top of the fs provider. `fsdy::Client` became
instance-mountable (`Client::new(provider_root, disk_root)`); the
orchestrator mounts a second, read-only instance at `/queues` over
`<config>/crabidy/queues/`, so saved queues are ordinary browsable,
queueable library folders. Every queue is a folder of order-prefixed
(`0001 <title>.cbd-track.toml`) **link** files — metadata copied from the
queue entry, `playable.link = Track.path` — written only by the new
`crabidy_server::queue_store::QueueStore` (tmp-and-swap, hidden
`.queue-state.toml` sidecar for position/repeat/shuffle). The playback
loop feeds every queue-state change into a latest-wins `watch` channel; a
persister task debounces, skips unchanged snapshots, and rewrites
`queues/current/`. On startup the server restores tracks, position, and
modifiers from `current/` without ever starting playback. `w` on the TUI
queue pane opens the existing input overlay (`save queue`) and drives the
previously stubbed `SaveQueue` rpc (invalid name → `invalid_argument`,
empty queue/disabled persistence → `failed_precondition`). Reloading a
saved queue is just queueing `/queues/<name>` — the listing rewrites each
link back to its target, so zero new resolve mechanisms. All 116
workspace tests green (10 new in `fsdy`, 9 in `queue_store`, 5 playback,
3 TUI); every gate in `quality/queue-persistence.md` checked. A temporary
live probe (removed after passing) round-tripped a mixed queue — a track
fetched from the live Tidal API plus an fs url track — through persist,
reload, `/queues` listing, and the resolve walk, and the reloaded Tidal
path still yielded a stream URL.
The whole feature ran autonomously per standing instruction; decisions
are recorded in `architecture/queue-persistence.md` (options + rationale).
### Deviations from plan / architecture (queue-persistence)
- **The "no links into `/fs`" rule was dropped** (fs-provider D3): queue
entries persist as links to whatever path the queue held, including
`/fs/...` tracks. Replaced by the one-hop argument —
`get_urls_for_track` never follows a link, so chains die at play time
and cycles cannot recurse. `architecture/fs-provider.md` reconciled.
- **`SaveQueueError` gained `Disabled` and `State` variants** beyond the
stub: `Disabled` (no usable queues directory) maps to
`failed_precondition` instead of masquerading as I/O; `State` covers
sidecar serialization.
- **The orchestrator mounts `/queues` independently of `QueueStore`**:
both derive the directory from `queue_store::queues_dir()`, so a
mount over a not-yet-created folder simply lists as missing until the
store (created in `main`) writes it. No plumbing between the two.
- **Shuffle order is not persisted** (documented in D3/D4 but worth
repeating): restoring `shuffle = true` reshuffles around the restored
current track.
- **The live probe needed no bespoke server run**: provider-layer clients
plus `QueueStore` cover the full D2/D5 story; the gRPC and TUI layers
above are unit-tested.
- **Environment note**: builds/tests again ran with a session-local
`CARGO_TARGET_DIR`; no repo change.
## fs-provider (2026-07-21)
Built per `plan/fs-provider.md`: a second media provider (crate `fsdy`,
`/fs`) that walks one configured root directory and treats
`*.cbd-track.toml` files as serialized track nodes — metadata plus exactly
one playable reference: a local audio file (absolute or relative to the
track file), an http(s) URL, or a crabidy-internal link. The wire types
are unchanged (architecture D1): the only new datastructure is the
on-disk TOML schema. Link tracks rewrite `Track.path` to the target at
listing time (D2), so playback routes to the owning provider through the
orchestrator's existing prefix routing with zero new mechanisms; links
into `/fs` are rejected at parse time, making chains impossible.
Directories list sorted and queue via the default chunked resolve walk;
client paths are decoded and validated in a single helper so they cannot
escape the root; symlinks, hidden entries, and broken files are skipped
with warnings. `ProviderOrchestrator` gained an optional fs client
(non-fatal init from `fsdy.toml`, default root `dirs::audio_dir()`) and
`/fs` routing arms in every trait method. No player or TUI changes were
needed. All 91 workspace tests green (15 new in `fsdy`); every gate in
`quality/fs-provider.md` checked. A temporary live probe (removed after
passing) built a real tree whose link track pointed at a track fetched
from the live Tidal API: listing order held, the link path was
rewritten, and the target resolved a stream URL — the full D2 story
end-to-end.
The whole feature ran autonomously per standing instruction; decisions
are recorded in `architecture/fs-provider.md` (options + rationale).
### Deviations from plan / architecture (fs-provider)
- **Extension renamed to `.cbd-track.toml`** (user request, follow-up
commit): the original `.track.toml` was too generic; the `cbd-` prefix
makes the files unmistakably crabidy's.
- **`TrackFileError::UrlScheme` carries only the scheme**, not the URL:
the parse error ends up in skip-warnings, and a private stream URL may
embed a token (quality gate "no file contents in logs"). The
architecture's schema and behavior are otherwise as designed.
- **The live probe ran at the provider layer**, not against a running
server (no interactive terminal/audio device here, same as previous
features): `fsdy` and `tidaldy` clients driven directly, mimicking the
orchestrator's routing exactly. It also had to *fetch* its link target
first — the well-known id from the progressive-queueing probe is an
album path, and a link must point at a track.
- **`get_lib_node` on a track path is `MalformedPath`** — implicit in
the design, made explicit so the default resolve walk can never
mistake a track file for a directory.
- **Environment note**: builds/tests again ran with a session-local
`CARGO_TARGET_DIR` (owner-built artifacts in `target/`); no repo
change.
## progressive-queueing (2026-07-21)
Built per `plan/progressive-queueing.md`: queueing a large nested collection
now fills the queue progressively instead of freezing until the full
resolve. `ProviderClient` gained `resolve_tracks_into` (chunk-streaming over
a bounded channel; sender-drop = done, receiver-drop = cancel) with a
default pre-order walk; tidaldy overrides it so playlists and albums emit
one chunk per fetched 50-track page. The playback loop registers a pending
op per queue command, spawns a forwarder, applies chunks on the loop
(single-writer preserved), broadcasts after every chunk, and starts playback
with the first chunk that makes a track current. `Replace`/`Clear` cancel
in-flight resolves down to the HTTP fetch. The wire gained
`Queue.resolving = 4` (additive); the TUI renders an animated one-to-three
dots pseudo-item after the last queue row while it is set. All 76 workspace
tests green; every gate in `quality/progressive-queueing.md` checked.
Verified against the live Tidal API with a temporary ignored probe (removed
after passing): a 71-album artist streamed its first 19-track chunk (first
album, listing order) while the walk was still running, and dropping the
receiver mid-stream ended the resolve cleanly in 1.8 s instead of draining
the discography.
The whole feature ran autonomously per standing instruction; decisions are
recorded in `architecture/progressive-queueing.md` (options + rationale).
### Deviations from plan / architecture (progressive-queueing)
- **`make_paginated_request_into` became `stream_track_pages_into`**: the
planned generic `AsyncFnMut` page sink dies on a rustc
"implementation of `Send` is not general enough" limitation inside
`async_trait` methods. The concrete method (fixed `Track` item type,
proto mapping and channel send inlined) sidesteps it with the same
page-loop and cancellation semantics.
- **Zero-track warning lives in the forwarder, not `finish_resolve`**: the
forwarder sees each path and its chunk count, so the existing per-path
"resolved to no playable tracks" message survives verbatim; the planned
op-level warning would have had to smuggle paths into `PendingResolve`.
- **Fixed alongside (user-reported)**: Enter on a non-queueable library
item used to blank the queue while audio kept playing. Two causes, both
fixed: `Library::get_selected` now gates the bare selection on
`is_queable` (marks were already gated), and a replace that resolves to
zero tracks no longer touches the queue at all — structurally, since the
queue is only mutated by arriving chunks. Regression test
`queue_ops_ignore_non_queueable_selections`.
- **`Queue` (play-next) captures the current position when the command
arrives**, not per chunk: chunks of one op stay contiguous after the
track the user was on when they pressed the key, even if playback
advances mid-resolve.
- **Live probe scope**: the first full-discography probe was cut short
(hundreds of album fetches for no extra signal) and replaced by a
receive-two-chunks-then-cancel probe — which also exercises mid-stream
cancellation against the live API, which the drain-everything version
could not.
- **Environment note**: builds/tests again ran with a session-local
`CARGO_TARGET_DIR` (owner-built artifacts in `target/`); no repo change.
## node-editing (2026-07-20)
Built per `plan/node-editing.md`: search-term nodes (created via `%`) are now
modifiable — `e` opens the input overlay prefilled with the current title and
renames (re-running the search; merge on title collision), `d` deletes
without confirmation (documented decision, architecture/node-editing.md D4).
Capabilities travel as `LibraryNodeChild.is_editable`/`is_deletable` (fields
5/6, child-only — no consumer for node-level copies), surfaced as a `[ed]`
marker; two new rpcs `RenameLibraryNode` (returns the renamed node, TUI
navigates into it) and `DeleteLibraryNode` (returns the refreshed parent).
All 59 workspace tests green; every gate in `quality/node-editing.md`
checked. Verified against the live Tidal API with a temporary ignored probe
(removed after passing): create `beatles` → rename to `rolling stones`
(in-place, 20 tracks / 40 children) → a track queued under the old term
still resolved a stream URL → delete emptied the listing.
The whole feature ran autonomously per standing instruction; decisions are
recorded in `architecture/node-editing.md` (options + rationale per topic).
### Deviations from plan / architecture (node-editing)
- **Self-rename bug caught by the gate tests**: the first
`rename_search_term` implementation deleted a term renamed to itself (the
merge branch removed the "old" slot). Fixed with an explicit `old != new`
guard; the architecture text ("merge on collision") now implicitly means
*distinct* titles.
- **`pane_bindings_only_match_their_own_pane` (help-modal suite) updated**:
it asserted plain `d` is unbound in the library — now it is
`LibraryDeleteNode` by design; the test's queue-only example key moved to
`c`.
- **`delete_library_node` also maps `InvalidInput``invalid_argument`**
although no provider raises it for delete today — keeps the error contract
uniform across the three node-mutation rpcs.
- **End-to-end check ran at the provider layer** (as with search): no
interactive terminal/audio device in this environment; the gRPC handler
and TUI layers above it are covered by unit tests and review.
- **Environment note**: builds/tests again ran with a session-local
`CARGO_TARGET_DIR` (owner-built artifacts in `target/`); no repo change.
## search (2026-07-20)
Built per `plan/search.md`: `%` inside `/tidal/search` opens a one-line input;
the term becomes a persistent (per-process) tree node holding Tidal search
results — 20 tracks queueable in place, plus artist/album results as canonical
`/tidal/artists/...` children. Creatable nodes carry an `is_creatable` flag
end-to-end (proto → provider → TUI marker `[%]` + pane hint). All 48 workspace
tests green; every gate in `quality/search.md` checked. Verified against the
live Tidal API: payload shapes match the existing models (probe kept as the
ignored `probe_search_shapes` test), and a full create→list→resolve-URL round
trip succeeded (`beatles` → 20 tracks / 40 children, idempotent, playable
stream URL from a search-track path).
The whole feature ran autonomously on user instruction; decisions were taken
without mid-stage confirmation and recorded in `architecture/search.md`
(options + decision per topic).
### Deviations from plan / architecture (search)
- **`Library::update` now concatenates tracks and children** (tracks first).
The old code showed tracks *instead of* children, which would have hidden
the artist/album results on term nodes — architecture assumed both would
render. Existing nodes are unaffected (they only ever carry one kind).
- **Search categories degrade independently**: a failing category logs and
contributes nothing; only all three failing is a `FetchError`. The plan
did not specify partial-failure behavior.
- **`get_lib_node` no longer requires a user id up front** — the gate moved
into the favorites arms (planned), which also means `create_lib_node`
validation works fully offline (used by the new unit tests).
- **End-to-end check ran at the provider layer** (temporary ignored test,
removed after passing) rather than driving the full TUI + server — no
interactive terminal/audio device in this environment. The gRPC handler and
TUI layers above it are covered by unit tests and review.
- **Environment note**: `target/` contains owner-built artifacts not writable
by this agent's user; builds/tests ran with a session-local
`CARGO_TARGET_DIR`. No repo change involved.
## help-modal (2026-07-20)
Built per `plan/help-modal.md`: `app/bindings.rs` (declarative
`BINDINGS` table + `lookup` + `key_label`), `app/help.rs` (overlay), the
`App::dispatch`/`DispatchResult` seam, and the rewired event loop in
`main.rs`. All 20 tests pass; every gate in `quality/help-modal.md` checked.
### Deviations from plan / architecture (help-modal)
- **Two-column modal layout.** The architecture assumed a single-column list;
the full table is ~50 rows and would not fit even a 100×40 frame. The modal
renders Global in the left column and Library + Queue stacked in the right
column, with the close keys as a footer line (`Close help: ?, Esc, q`)
derived from the `Scope::Help` bindings instead of a fourth listed group.
The open question "scroll vs truncate" stays resolved as truncate — but
after the column split the content fits ~34×94, so truncation only kicks in
on genuinely small terminals.
- **`Scope` derives `Hash`** (not in the stub) so the chord-uniqueness test
can use a `HashSet`.
- **`QueueInsertHere` description reworded** to "Insert library selection
after this track": `crabidy-server`'s `insert_tracks` splices at
`position + 1`. Same check confirmed the planned "Queue selection after
current track" wording for `LibraryQueueNext`.
- **`main.rs`** passes `tx` to `App::new` without the now-unneeded clone; the
`KeyCode`/`KeyModifiers`/`UiFocus`/`StatefulList` imports moved out with the
old match.