From 17da7a7233e34ba9a25a19a99faba2c1f2b1308c Mon Sep 17 00:00:00 2001 From: archipelago Date: Tue, 4 Aug 2026 11:15:41 -0400 Subject: [PATCH] =?UTF-8?q?docs(13-07):=20summary=20=E2=80=94=20music=20in?= =?UTF-8?q?dex=20+=20music.*=20surface=20complete,=2023/23=20green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../13-07-SUMMARY.md | 161 ++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 .planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-07-SUMMARY.md diff --git a/.planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-07-SUMMARY.md b/.planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-07-SUMMARY.md new file mode 100644 index 00000000..38724065 --- /dev/null +++ b/.planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-07-SUMMARY.md @@ -0,0 +1,161 @@ +--- +phase: 13-aiui-functional-conversational-node-control-and-content-surf +plan: 07 +subsystem: music +tags: [music-library, index, rpc, lofty, incremental-refresh, atomic-write, rust] + +# Dependency graph +requires: + - phase: 13-04 + provides: "13-MUSIC-MODEL.md (D-13 decision), music/mod.rs entity types, music/tags.rs extract_tags with media-root confinement, lofty 0.24.0" +provides: + - "core/archipelago/src/music/index.rs — MusicIndex: reindex, refresh_incremental, load (NewerSchema refusal), save_atomic (temp+fsync+rename), group_albums/group_artists, ReindexState guard, ScanStats" + - "core/archipelago/src/music/mod.rs — media_roots(Config) (filebrowser/Music + purchased-content), LibrarySnapshot" + - "music.* RPC surface: music.list-albums, music.list-artists, music.list-tracks, music.status, music.reindex — one dispatcher.rs prefix arm" +affects: [13-11, 13-12, song-grid, peer-audio] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Atomic index persistence: serialize to sibling temp + fsync + rename — concurrent readers see complete old or complete new, never partial" + - "Forward-version refusal: load returns a distinct NewerSchema error and never overwrites the newer file; explicit reindex is the only rebuild path" + - "Scan guard: AtomicBool + RAII drop-release; duplicate scans report already-running with last stats instead of queueing" + - "RPC testability without RpcHandler: module-level dispatch fn parameterized on (state, data_dir, roots), thin RpcHandler method delegating" + +key-files: + created: + - core/archipelago/src/music/index.rs + - core/archipelago/src/api/rpc/music.rs + modified: + - core/archipelago/src/music/mod.rs + - core/archipelago/src/api/rpc/dispatcher.rs + - core/archipelago/src/api/rpc/mod.rs + +key-decisions: + - "media_roots = [data_dir/filebrowser/Music, data_dir/purchased-content] — both D-13 sources as local filesystem roots; per-root MusicSource assignment (purchased-content files map to Peer{onion} from their first path component)" + - "music.reindex accepts optional incremental:true routing to refresh_incremental, so the stays-fresh truth has a production caller (default remains the on-demand full rebuild)" + - "Albums ordered by (album_artist, album, min track year); tracks by (disc, track number, title); TrackId (source, path) is the final tiebreak everywhere" + +patterns-established: + - "T-13-42 mitigation shape: never write the index in place — save_atomic is the only writer" + - "T-13-39 enforced twice: walker skips symlinks whose canonical target escapes the canonical root, and extract_tags re-checks confinement per file" + +requirements-completed: [] # AIUI-03 spans multiple plans (13-11 SongGrid wiring still pending) + +# Metrics +duration: ~3h45m wall clock (dominated by four full non-incremental archipelago compiles on the shared 4-core box; one 10-minute foreground cargo run was killed by the harness timeout and rerun) +completed: 2026-08-04 +status: complete +--- + +# Phase 13 Plan 07: Music Library Index and music.* RPC Surface Summary + +**Persisted, incrementally-refreshed music library over the D-13 model — atomic JSON index with schema-version refusal, symlink-confined scanning, derived album/artist grouping, and an authenticated five-method `music.*` surface behind a single dispatcher arm — 23/23 `music::` tests green.** + +## Performance + +- **Duration:** ~3h45m wall clock (~25 min of it authoring; the rest cargo compile time — four full `CARGO_INCREMENTAL=0` builds of the archipelago crate at ~10-14 min each on the live-node box) +- **Started:** 2026-08-04T11:27Z +- **Completed:** 2026-08-04T15:15Z +- **Tasks:** 2 (both auto/TDD) +- **Files modified:** 5 + +## Accomplishments + +- `music/index.rs`: `reindex` walks the media roots, extracts tags per audio file via 13-04's `extract_tags`, and persists `data_dir/music/index.json`; `refresh_incremental` diffs `(path, mtime, size)` so unchanged files are never re-extracted, removes rows for vanished files (derived albums vanish with their last track), and preserves the lazily-backfilled `content_hash` column on unchanged rows. +- `save_atomic` (temp sibling + fsync + rename) makes the concurrent-read truth hold: a reader sees the complete previous index or the complete new one, never a torn file — verified by a 100-writer/200-reader interleaving test. +- `load` refuses `schema_version > MUSIC_SCHEMA_VERSION` with a distinct `NewerSchema` error and never overwrites the newer file; readers serve an empty library, and only an explicit reindex rebuilds (13-MUSIC-MODEL.md's downgrade contract, T-13-43). +- Symlinks whose canonical target escapes the canonical media roots are skipped, not followed (T-13-39) — confinement enforced in the walker *and* re-checked per file inside `extract_tags`. +- One comparator everywhere: albums by (album artist, album title, min year), tracks by (disc, track, title), with the `(source, path)` identity as final tiebreak — list ordering is stable across repeated calls. +- `music.*` RPC surface: five methods behind exactly one `m if m.starts_with("music.")` dispatcher arm (the 13-01 `assistant.` pattern), placed adjacent to the `content.*` block; envelopes carry `total` + `scanned_at`; `list-tracks` clamps `limit` to [1,500] (default 100); `music.reindex` spawns and returns immediately, refusing to duplicate a running scan (T-13-41). +- Nothing `music.*` in `UNAUTHENTICATED_METHODS` — the surface rides the existing session/CSRF/RBAC gate (T-13-40), asserted by `music_methods_require_session`. + +## Task Commits + +1. **Task 1: The index — scan, group, persist, stay fresh** — `49687f7e` (feat) — `music/index.rs` + `music/mod.rs` (`media_roots`, `LibrarySnapshot`); 9 tests, one per behavior bullet +2. **Task 2: The music.* RPC surface** — `d3b0ed8a` (feat) — `api/rpc/music.rs`, single dispatcher arm, `mod music;` declaration; 7 tests + +## Files Created/Modified + +- `core/archipelago/src/music/index.rs` — `MusicIndex`, `IndexEntry`, `ScanStats`, `IndexError::NewerSchema`, `reindex`, `refresh_incremental`, `load`, `save_atomic`, `group_albums`, `group_artists`, `ReindexState`/`ReindexGuard`, `shared_state`, `INDEX_FILENAME` +- `core/archipelago/src/music/mod.rs` — `pub mod index;`, `media_roots(&Config)`, `LibrarySnapshot` +- `core/archipelago/src/api/rpc/music.rs` — `handle_music` prefix sub-dispatcher + `handle_music_list_albums` / `list_artists` / `list_tracks` / `status` / `reindex` +- `core/archipelago/src/api/rpc/dispatcher.rs` — one `music.` prefix arm (the only registration point) +- `core/archipelago/src/api/rpc/mod.rs` — `mod music;` declaration (alphabetical block) + +## Decisions Made + +- **Media roots**: `filebrowser/Music` (OwnLibrary — peer audio purchases are also auto-filed here by `content.*`'s paid-download path, so they enter the library with real filenames) plus `purchased-content` (Peer byte cache; files under it map to `MusicSource::Peer{onion}` from their first path component). Note: purchased-content stores files extensionless today, so that root yields no tracks until files there carry audio extensions — peer catalog surfacing in `SongGrid` is 13-11's job via the existing `content.*` discovery, per 13-MUSIC-MODEL.md ("reuses content.*'s existing peer-audio discovery"). +- **Grouping field names spot-checked against 13-MUSIC-MODEL.md** (plan acceptance): row key `TrackId{source, path}` (hybrid-identity) ✓; `content_hash` lazily-backfilled column preserved on unchanged rows, reset when bytes change ✓; albums derived at read time on `AlbumId{album_artist, album}`, artists on the `artist` tag — never persisted ✓; index at `data_dir/music/index.json`, pretty-printed JSON with top-level `schema_version` ✓; newer-version index treated as absent and never overwritten ✓. +- `MusicIndex::empty()` populates `scanned_at` with "now", so even a never-scanned library serves a timestamp — empty arrays + timestamp, never null, never an error. + +## Deviations from Plan + +### Auto-fixed / necessary additions + +**1. [Rule 3 - Blocking] `mod music;` added to `api/rpc/mod.rs`** +- **Found during:** Task 2 +- **Issue:** The plan's `files_modified` lists only `music.rs` + `dispatcher.rs`, but a new module must be declared in its parent. +- **Fix:** One line in the alphabetical `mod` block (between `monitoring` and `names`). +- **Commit:** `d3b0ed8a` + +**2. [Rule 2 - Missing functionality] `music.reindex` gained an optional `incremental: true` param** +- **Found during:** Task 2 (`cargo build` bin target flagged `refresh_incremental`/`ScanMode::Incremental` as dead code — the freshness machinery had no production caller, making the "stays fresh without a full rebuild" truth test-only) +- **Fix:** `music.reindex {"incremental": true}` routes to `refresh_incremental`; default stays the on-demand full rebuild. Covered by `reindex_incremental_mode_refreshes_without_full_reextraction`. Also removed all three new dead-code warnings (bin builds with zero warnings from music files). +- **Commit:** `d3b0ed8a` + +**3. [Rule 1 - Bug] `anyhow!` format-string brace escape** +- **Found during:** Task 2 first compile — `"expected { album, album_artist }"` parsed as format args. +- **Fix:** escaped to `{{ album, album_artist }}`. Caught before any commit. + +### Process notes + +- **TDD gate:** tests were authored first (one per behavior bullet) but each task landed as a single green commit rather than separate RED `test(...)` + GREEN `feat(...)` commits — this repo's CLAUDE.md hard rule ("commit each feature the moment it works — it compiles and its targeted tests pass") takes precedence over the generic RED-commit convention, matching how 13-04 Task 3 executed. See TDD Gate Compliance below. +- **Fixture reuse:** the compact FLAC byte-builder is duplicated into `index.rs` and `api/rpc/music.rs` test modules rather than refactoring `tags.rs`'s builders to be shared — `tags.rs` is outside this plan's file list. (An incidental `rustfmt` recursion into `tags.rs` was reverted to keep commits scoped.) + +**Total deviations:** 2 necessary additions + 1 trivial compile fix. No scope changes. + +## TDD Gate Compliance + +- RED evidence exists as authored-first tests (16 new tests across the two tasks), but no standalone `test(...)` commits precede the `feat(...)` commits — CLAUDE.md's green-commit rule was applied deliberately (see Process notes). Gate sequence in git log is therefore `feat(49687f7e)` → `feat(d3b0ed8a)` with tests and implementation co-committed. + +## Known Stubs + +None — no placeholder text, no hardcoded empty values flowing to UI, no unwired data paths. (The `purchased-content` root legitimately yields zero tracks today because its files are extensionless content-ids; peer audio reaches `SongGrid` via 13-11's `content.*` path per the model doc, and audio purchases already land in `filebrowser/Music` with extensions.) + +## Threat Flags + +None — no security-relevant surface beyond the plan's threat model (T-13-39/40/41/42/43/44 all mitigated as specified; T-13-45 remains accepted/deferred to 13-12's `wrap_untrusted`, and nothing here places tag text in a model context). + +## Issues Encountered + +- **Foreground cargo timeout ceiling:** a full `CARGO_INCREMENTAL=0` test compile of the archipelago crate takes ~10-14 min on this box (live node sharing 4 cores), which exceeds the 10-minute foreground tool cap — one run was SIGTERM'd mid-compile and rerun. Workaround used thereafter: detached cargo writing to a log with a `.done` marker + foreground polling loops, keeping the session turn alive. + +## Verification (plan-level) + +- `CARGO_INCREMENTAL=0 cargo test --package archipelago music::` → **`ok. 23 passed; 0 failed`** (9 index + 7 tags + 7 rpc) +- `CARGO_INCREMENTAL=0 cargo build --package archipelago` → exit 0 (`Finished dev profile`), zero warnings from music/rpc-music files +- `grep -c 'starts_with("music.")' dispatcher.rs` → **1** +- `grep -n 'music\.' middleware.rs` → no match (nothing unauthenticated) +- `git ls-files core/archipelago | grep -ciE '\.(mp3|flac|m4a|ogg)$'` → 0 (no binary fixtures) +- Index field names match `13-MUSIC-MODEL.md` (spot check recorded under Decisions Made) + +## User Setup Required + +None — no external service configuration required. + +## Next Phase Readiness + +- 13-11 can wire `SongGrid` to `music.list-albums`/`music.list-tracks` (envelopes follow `content.*`'s items+metadata shape: arrays + `total` + `scanned_at`) and trigger `music.reindex` (full or incremental). +- No music tool was added to the assistant's curated registry — deliberately out of scope per the plan (track independence, D-13). +- T-13-45 (peer tag text as untrusted model input) remains 13-12's `wrap_untrusted` responsibility. + +## Self-Check: PASSED + +- FOUND: `core/archipelago/src/music/index.rs`, `core/archipelago/src/api/rpc/music.rs`, `core/archipelago/src/music/mod.rs` (updated), `.planning/phases/13-.../13-07-SUMMARY.md` +- FOUND commits: `49687f7e`, `d3b0ed8a` + +--- +*Phase: 13-aiui-functional-conversational-node-control-and-content-surf* +*Completed: 2026-08-04*