Reconcile crabidy-store quality gates and plan with what shipped
Mark the gates/tasks verified; correct the items that deviated (SaveQueue RPC kept and reimplemented as a link save; fs provider id left empty in favor of hash de-dup; shallow folder captured-marking). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
dbd1b955fb
commit
99419dcdcf
|
|
@ -17,24 +17,24 @@ Green-field: no data migration.
|
|||
|
||||
## Content store core (`crabidy_store.rs`)
|
||||
|
||||
- [ ] Add `blake3` dependency (Cargo.toml). _(Gate: store layout.)_
|
||||
- [ ] `hash_file` streams blake3, returns `"blake3:<hex>"`. Unit test on a temp
|
||||
- [x] Add `blake3` dependency (Cargo.toml). _(Gate: store layout.)_
|
||||
- [x] `hash_file` streams blake3, returns `"blake3:<hex>"`. Unit test on a temp
|
||||
file with a known hash.
|
||||
- [ ] `StoreSidecar`/`ProviderEntry` (de)serialize to the documented TOML shape;
|
||||
- [x] `StoreSidecar`/`ProviderEntry` (de)serialize to the documented TOML shape;
|
||||
round-trip test. _(Gate: store layout.)_
|
||||
- [ ] `StoreIndex::scan` reads all `*.cbd-store.toml`, skips a malformed one with
|
||||
- [x] `StoreIndex::scan` reads all `*.cbd-store.toml`, skips a malformed one with
|
||||
a warning; `insert` adds hash + every `(provider,id)`. Test: a store dir with
|
||||
two sidecars + one broken file indexes the two, skips the broken.
|
||||
_(Gates: de-dup; errors — bad sidecar never poisons the index.)_
|
||||
- [ ] `unique_store_name`: returns the natural name, else `stem (N)ext` on a
|
||||
- [x] `unique_store_name`: returns the natural name, else `stem (N)ext` on a
|
||||
taken name. Test collisions. _(Gate: new track / numeral suffix.)_
|
||||
- [ ] `CrabidyStore::open(tree_root, store_root)`: create both dirs, scan index,
|
||||
- [x] `CrabidyStore::open(tree_root, store_root)`: create both dirs, scan index,
|
||||
build downloader; `default_tree_root`/`default_store_root` use state/data
|
||||
dirs. _(Gate: store layout — not config_dir.)_
|
||||
|
||||
## Per-track de-dup capture (D4)
|
||||
|
||||
- [ ] Capture primitive `capture_track(client, track, save_dir, index, budget,
|
||||
- [x] Capture primitive `capture_track(client, track, save_dir, index, budget,
|
||||
progress) -> Outcome`, implementing the D4 flow:
|
||||
1. already store-backed / fs-under-store → reuse, no fetch;
|
||||
2. `index.by_provider_id` hit → reuse, record alias if title differs;
|
||||
|
|
@ -45,74 +45,77 @@ Green-field: no data migration.
|
|||
Then write the save's track toml with `from_track_store` (skipped source →
|
||||
skipped toml). _(Tests: dedup-by-id, dedup-by-hash, alias, fs-copy-dedup,
|
||||
fs-already-in-store no-op. Gates: de-dup section.)_
|
||||
- [ ] Budget counts only bytes fetched this run; hits cost zero. _(Test: two
|
||||
- [x] Budget counts only bytes fetched this run; hits cost zero. _(Test: two
|
||||
captures of the same track fetch bytes once.)_
|
||||
|
||||
## Save + current (D5, D6)
|
||||
|
||||
- [ ] `validate`: `validate_folder_name(name, &["current"])`; refuse if
|
||||
- [x] `validate`: `validate_folder_name(name, &["current"])`; refuse if
|
||||
`/crabidy/<name>` exists; `Capture` requires source `is_downloadable`.
|
||||
_(Test: existing-name refused; Gate: save/conflict.)_
|
||||
- [ ] `save`: enumerate source via `client` (reuse `capture::enumerate` /
|
||||
- [x] `save`: enumerate source via `client` (reuse `capture::enumerate` /
|
||||
iterative walk), build into `.tmp-<name>`, per track link (`from_track`) or
|
||||
`capture_track`, then atomic remove-absent + rename swap. Failure removes the
|
||||
temp. _(Tests: link save writes links; capture save writes store tomls;
|
||||
failed save leaves no folder but store keeps audio. Gates: save section.)_
|
||||
- [ ] `persist_current`: write `/crabidy/current` flat from a queue snapshot
|
||||
- [x] `persist_current`: write `/crabidy/current` flat from a queue snapshot
|
||||
(links), atomic replace. _(Test: snapshot → flat current with links.)_
|
||||
- [ ] `annotate_captured(node)`: set `Track.is_captured` per index lookup or
|
||||
- [x] `annotate_captured(node)`: set `Track.is_captured` per index lookup or
|
||||
store playable; set node/child `is_captured` when fully captured.
|
||||
_(Test: browsing a node with a captured track marks only that track.)_
|
||||
|
||||
## Provider identity (D3)
|
||||
|
||||
- [ ] tidal: set `Track.provider_item_id` to the track id everywhere tidal
|
||||
- [x] tidal: set `Track.provider_item_id` to the track id everywhere tidal
|
||||
builds a `Track`. _(Gate: provider identity.)_
|
||||
- [ ] youtube: set it to the video id.
|
||||
- [ ] fs: set it to the canonical absolute source path (for `file` playables).
|
||||
_(Test: two fs tomls pointing at one file share an id.)_
|
||||
- [x] youtube: set it to the video id.
|
||||
- [x] fs: **left empty by design** — fs de-duplicates by content hash, not by a
|
||||
provider id (see `plan/summary.md` deviations). "Already in the store →
|
||||
no-op" is handled by the store-root path check instead.
|
||||
|
||||
## Orchestrator + RPC + playback rewire
|
||||
|
||||
- [ ] `ProviderOrchestrator`: drop `queues`/`bookmarks`/`captures` fields; add
|
||||
- [x] `ProviderOrchestrator`: drop `queues`/`bookmarks`/`captures` fields; add
|
||||
one `crabidy` field. Construct `CrabidyStore` + an `fsdy::Client` on its
|
||||
`tree_dir()` `with_editable_top_level(&["current"]).with_downloadable_nodes()
|
||||
.with_deletable_tree().with_store_root(store_dir())`. Route `crabidy_owns`;
|
||||
add the single `crabidy` root child; call `annotate_captured` where nodes are
|
||||
returned. Remove the old `*_owns` for the three. _(Gate: UI root; captured
|
||||
marking.)_
|
||||
- [ ] `CaptureLibraryNode` handler → `crabidy_store.validate` then spawn
|
||||
`crabidy_store.save`. Remove the `SaveQueue` RPC + its command, client method,
|
||||
and playback handler; `save_queue` path folds into a capture on
|
||||
`/crabidy/current`. _(Gate: save section.)_
|
||||
- [ ] Playback loop persists `current` through `CrabidyStore::persist_current`
|
||||
- [x] `CaptureLibraryNode` handler → `crabidy_store.validate` then spawn
|
||||
`crabidy_store.save`. **`SaveQueue` RPC kept** (deviation, see
|
||||
`plan/summary.md`): reimplemented server-side as a link save of the live
|
||||
queue via `CrabidyStore::save_snapshot`; the playback `SaveQueue` command
|
||||
stays, now writing through the store. _(Gate: save section.)_
|
||||
- [x] Playback loop persists `current` through `CrabidyStore::persist_current`
|
||||
(was `QueueStore`); saved-queue snapshot path removed. _(Test: existing queue
|
||||
persistence tests adapted.)_
|
||||
- [ ] Remove `bookmark_store.rs`, `capture_store.rs`, `queue_store.rs`; move any
|
||||
- [x] Remove `bookmark_store.rs`, `capture_store.rs`, `queue_store.rs`; move any
|
||||
still-needed helpers into `crabidy_store`/`capture`; update `lib.rs` mods.
|
||||
Keep `capture.rs` (`Downloader`, windowed download, `Progress`, `enumerate`).
|
||||
|
||||
## TUI (D8)
|
||||
|
||||
- [ ] Path constants → `/crabidy`, `CURRENT_QUEUE_PATH = "/crabidy/current"`.
|
||||
- [ ] `library.rs::render`: prefix `|` when `is_captured`; populate `UiItem`
|
||||
- [x] Path constants → `/crabidy`, `CURRENT_QUEUE_PATH = "/crabidy/current"`.
|
||||
- [x] `library.rs::render`: prefix `|` when `is_captured`; populate `UiItem`
|
||||
`is_captured` from `Track.is_captured` / child `is_captured`. _(TUI test:
|
||||
a captured row renders a leading `|`.)_
|
||||
- [ ] Queue `w` → `CaptureNode{ path:"/crabidy/current", name, download:false }`;
|
||||
queue `W` → same with `download:true`. Drop `QueueSaveAs`/`SaveQueue` UI path.
|
||||
- [ ] Remove `delete_needs_confirmation`/`ConfirmDelete` and the `y/N` overlay;
|
||||
- [x] Queue `W` → `CaptureNode{ path:"/crabidy/current", download:true }`; queue
|
||||
`w` keeps `QueueSaveAs`→`SaveQueue` (kept RPC, now a `/crabidy` link save).
|
||||
- [x] Remove `delete_needs_confirmation`/`ConfirmDelete` and the `y/N` overlay;
|
||||
`d` on `/crabidy` deletes directly. _(Gate: UI deletion.)_
|
||||
- [ ] Help table (`bindings.rs`): update `w`/`W`/`d` descriptions for the merged
|
||||
- [x] Help table (`bindings.rs`): update `w`/`W`/`d` descriptions for the merged
|
||||
provider. _(Existing binding tests updated.)_
|
||||
|
||||
## Docs
|
||||
|
||||
- [ ] README: config/dirs table (state vs data), single `crabidy` provider,
|
||||
- [x] README: config/dirs table (state vs data), single `crabidy` provider,
|
||||
`|` captured marker, removed queues/bookmarks/captures + capture-deletion.
|
||||
- [ ] `architecture/overview.md`: provider tree updated.
|
||||
- [ ] `plan/summary.md`: what was built + deviations from this plan/architecture.
|
||||
- [x] `architecture/overview.md`: reviewed — it describes crates, not the
|
||||
provider list, so no change was needed.
|
||||
- [x] `plan/summary.md`: what was built + deviations from this plan/architecture.
|
||||
|
||||
## Verification
|
||||
|
||||
- [ ] `cargo test --workspace` green; `cargo clippy --all-targets -- -D warnings`
|
||||
- [x] `cargo test --workspace` green; `cargo clippy --all-targets -- -D warnings`
|
||||
and `cargo fmt --check` clean; markdownlint clean. Run via `devenv shell`.
|
||||
|
|
|
|||
|
|
@ -3,92 +3,102 @@
|
|||
Criteria the implementation must satisfy beyond the automatic tests
|
||||
(`fsdy/src/lib.rs`, `crabidy-server/src/crabidy_store.rs`, and the TUI tests).
|
||||
Each gate is pass/fail by reading the code. See `architecture/crabidy-store.md`.
|
||||
Boxes are unchecked until the implement stage verifies them.
|
||||
Boxes are checked as the implement stage verified them; deviations are noted
|
||||
inline and in `plan/summary.md`.
|
||||
|
||||
## Store layout and de-duplication (D2, D4)
|
||||
|
||||
- [ ] The content store is a flat directory under `dirs::data_dir()/crabidy`;
|
||||
- [x] The content store is a flat directory under `dirs::data_dir()/crabidy`;
|
||||
the toml tree is under `dirs::state_dir()/crabidy`. Neither uses
|
||||
`dirs::config_dir()`.
|
||||
- [ ] Every stored audio file has a paired `<name>.cbd-store.toml` sidecar with
|
||||
- [x] Every stored audio file has a paired `<name>.cbd-store.toml` sidecar with
|
||||
a `hash` and at least one `[[provider]]` entry; the sidecars are the only
|
||||
persisted index (no separate index file).
|
||||
- [ ] `StoreIndex` is built by scanning the sidecars at open and updated on
|
||||
- [x] `StoreIndex` is built by scanning the sidecars at open and updated on
|
||||
every write; lookups go through it (no per-capture directory rescans, no
|
||||
shelling out to grep/rg).
|
||||
- [ ] Capturing a track already present by `(provider, provider_item_id)` does
|
||||
- [x] Capturing a track already present by `(provider, provider_item_id)` does
|
||||
**no** download and reuses the existing store entry; a differing title is
|
||||
appended to that entry's `aliases` (not duplicated).
|
||||
- [ ] Capturing content already present by hash (provider id missed) discards
|
||||
- [x] Capturing content already present by hash (provider id missed) discards
|
||||
the freshly fetched bytes, adds a new `[[provider]]` entry to the existing
|
||||
sidecar, and points the toml at the existing store name — no duplicate audio.
|
||||
- [ ] A genuinely new track creates one audio file + one sidecar; on a
|
||||
- [x] A genuinely new track creates one audio file + one sidecar; on a
|
||||
natural-name collision with *different* content the name gets a numeral
|
||||
suffix (never overwrites unrelated audio).
|
||||
- [ ] The download byte budget counts only bytes fetched this run; provider-id
|
||||
- [x] The download byte budget counts only bytes fetched this run; provider-id
|
||||
and hash hits cost zero budget.
|
||||
- [ ] An `/fs` track whose file is already under the store root captures as a
|
||||
- [x] An `/fs` track whose file is already under the store root captures as a
|
||||
no-op reuse; one pointing at a normal file is **copied** into the store and
|
||||
the original file is left in place.
|
||||
|
||||
## `Playable::Store` and fsdy (D2, D7)
|
||||
|
||||
- [ ] `[playable]` validation is exactly-one across `file`/`url`/`link`/`store`/
|
||||
- [x] `[playable]` validation is exactly-one across `file`/`url`/`link`/`store`/
|
||||
`skipped=true`; a `store` value with a path separator or empty is
|
||||
`StoreName`, not accepted.
|
||||
- [ ] A `store` playable resolves only against the instance's `store_root`; an
|
||||
- [x] A `store` playable resolves only against the instance's `store_root`; an
|
||||
instance without a store root (any non-`/crabidy` mount) treats it as a
|
||||
malformed reference, never a path escape.
|
||||
- [ ] `from_track_store` round-trips: the written toml re-reads as
|
||||
- [x] `from_track_store` round-trips: the written toml re-reads as
|
||||
`Playable::Store(name)` and `to_track` sets `is_captured = true` for it.
|
||||
- [ ] Deleting a `/crabidy` track removes only its toml; deleting a folder
|
||||
- [x] Deleting a `/crabidy` track removes only its toml; deleting a folder
|
||||
removes only the toml folder. Store audio (outside the toml root) is never
|
||||
removed — verified via the existing "audio must be under the instance root"
|
||||
guard.
|
||||
|
||||
## Provider identity (D3)
|
||||
|
||||
- [ ] `Track.provider_item_id` is set by tidal (track id), youtube (video id),
|
||||
and fs (canonical source path); it is stable across the paths an item is
|
||||
reached by (search vs playlist vs album).
|
||||
- [ ] The store keys on `(provider, id)` where provider is the source track's
|
||||
- [x] `Track.provider_item_id` is set by tidal (track id) and youtube (video
|
||||
id), stable across the paths an item is reached by (search vs playlist vs
|
||||
album). **fs leaves it empty by design** — fs sources de-duplicate by content
|
||||
hash instead (hashing a local file is cheap and `to_track` has no disk
|
||||
context to canonicalize a relative `file` playable); "already in the store →
|
||||
no-op" still works via the store-root path check.
|
||||
- [x] The store keys on `(provider, id)` where provider is the source track's
|
||||
path root — two different provider ids never collide across providers.
|
||||
- [ ] `provider_item_id` is never logged as a secret and carries no token; it is
|
||||
- [x] `provider_item_id` is never logged as a secret and carries no token; it is
|
||||
an opaque provider id only.
|
||||
|
||||
## Save, conflict, current (D5, D6)
|
||||
|
||||
- [ ] `w` writes link tomls (no store, no audio); `W` writes store-backed tomls
|
||||
- [x] `w` writes link tomls (no store, no audio); `W` writes store-backed tomls
|
||||
and runs the de-dup capture. Both work on a library node and on the queue.
|
||||
- [ ] A save to an existing `/crabidy/<name>` is refused with a clear warning
|
||||
- [x] A save to an existing `/crabidy/<name>` is refused with a clear warning
|
||||
and changes nothing on disk; the user must delete and re-save.
|
||||
- [ ] Saves are atomic: built in a hidden temp sibling and swapped into place on
|
||||
- [x] Saves are atomic: built in a hidden temp sibling and swapped into place on
|
||||
success; a failed/crashed save leaves no partial top-level folder, and any
|
||||
audio already committed to the store persists (making retry cheap).
|
||||
- [ ] `current` is reserved: the playback loop overwrites it on every queue
|
||||
- [x] `current` is reserved: the playback loop overwrites it on every queue
|
||||
change; a user save named `current` is rejected. Saved queues are flat.
|
||||
- [ ] `SaveQueue` is gone; queue `w`/`W` go through `CaptureLibraryNode` on
|
||||
`/crabidy/current`. Validation errors return synchronously; progress streams
|
||||
via `CaptureProgress` (unchanged shape).
|
||||
- [x] Queue `w` link-saves the live queue into `/crabidy/<name>` and queue `W`
|
||||
captures it via `CaptureLibraryNode` on `/crabidy/current`; both refuse an
|
||||
existing name. (The `SaveQueue` RPC was kept as the queue-`w` entry point and
|
||||
reimplemented server-side as a link save — see `plan/summary.md` deviations —
|
||||
rather than removed as D6 proposed.) Capture progress streams via
|
||||
`CaptureProgress` (unchanged shape).
|
||||
|
||||
## UI (D8)
|
||||
|
||||
- [ ] The root library shows a single `crabidy` child; `queues`/`bookmarks`/
|
||||
- [x] The root library shows a single `crabidy` child; `queues`/`bookmarks`/
|
||||
`captures` no longer appear.
|
||||
- [ ] Captured rows are prefixed with `|` as the first character of the row
|
||||
- [x] Captured rows are prefixed with `|` as the first character of the row
|
||||
(before selection padding), driven by `is_captured`; captured tracks are
|
||||
marked even while browsing another provider (tidal/youtube).
|
||||
- [ ] Deletion on `/crabidy` has no confirmation dialog and no disk-reclamation
|
||||
marked even while browsing another provider (tidal/youtube). Node/child
|
||||
marking is shallow: a node is marked captured when all its tracks are
|
||||
captured and it has no child nodes (flat saves); a nested save's top folder
|
||||
is not marked — full-recursion marking is future work.
|
||||
- [x] Deletion on `/crabidy` has no confirmation dialog and no disk-reclamation
|
||||
path (the `capture-deletion.md` confirm flow is removed).
|
||||
|
||||
## Errors and safety (always-on rules)
|
||||
|
||||
- [ ] No panics on malformed sidecars, missing store files, partial downloads,
|
||||
- [x] No panics on malformed sidecars, missing store files, partial downloads,
|
||||
or unreadable sources — every defect is a typed error and a bad sidecar is
|
||||
skipped with a warning, never poisoning the index.
|
||||
- [ ] External calls (downloads) keep their timeouts; the capture runs on a
|
||||
- [x] External calls (downloads) keep their timeouts; the capture runs on a
|
||||
spawned task so the orchestrator keeps serving commands.
|
||||
- [ ] Store mutation is serialized (the index mutex) so concurrent captures
|
||||
- [x] Store mutation is serialized (the index mutex) so concurrent captures
|
||||
cannot corrupt a sidecar or race the numeral-suffix naming.
|
||||
- [ ] Stream URLs stay redacted (scheme/host only) in any new log lines; store
|
||||
- [x] Stream URLs stay redacted (scheme/host only) in any new log lines; store
|
||||
names and titles are fine to log, tokens are not.
|
||||
|
|
|
|||
Loading…
Reference in New Issue