docs(13-04): complete music model + tag extraction plan — summary, state, config
- 13-04-SUMMARY.md: all 3 tasks, broken-pipe recovery (verbatim wip
checkpoint be8f24b4), MP3 fixture off-by-one deviation, 7/7 tests green
- STATE.md: 13-04 complete (7/15 phase-13 plans), D-13 decision + lofty
gate recorded in accumulated context, next = waves 3+
- config.json: fold in pre-existing use_worktrees=false from the broken
session (wave-continue bookkeeping, intentionally kept)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
archipelago
co-authored by
Claude Fable 5
Claude Fable 5
parent
4b577493b1
commit
589cbb030f
+179
@@ -0,0 +1,179 @@
|
||||
---
|
||||
phase: 13-aiui-functional-conversational-node-control-and-content-surf
|
||||
plan: 04
|
||||
subsystem: music
|
||||
tags: [lofty, id3v2, vorbis, mp4, ogg, rust, tag-extraction, music-library]
|
||||
|
||||
# Dependency graph
|
||||
requires:
|
||||
- phase: 13-01
|
||||
provides: "mod-declaration serialization in core/archipelago/src/main.rs (binary-only crate, single declaration site) — no logical coupling"
|
||||
provides:
|
||||
- "13-MUSIC-MODEL.md — the recorded one-way D-13 decision: hybrid-identity, derived-albums, index-format-json, sources=both"
|
||||
- "core/archipelago/src/music/mod.rs — Track/Album/Artist entity types, TrackId/AlbumId/ArtistId, MusicSource, MUSIC_SCHEMA_VERSION=1"
|
||||
- "core/archipelago/src/music/tags.rs — extract_tags(path, media_roots) with root confinement, filename-stem fallback, distinct NotAudio error"
|
||||
- "lofty 0.24.0 in core/archipelago/Cargo.toml (entered via human legitimacy gate)"
|
||||
affects: [13-11, 13-12, music-index, song-grid, peer-audio]
|
||||
|
||||
# Tech tracking
|
||||
tech-stack:
|
||||
added: [lofty 0.24.0]
|
||||
patterns:
|
||||
- "Media-root confinement as a parameter (media_roots: &[PathBuf]) checked via canonicalize + starts_with BEFORE any file open"
|
||||
- "Programmatic byte-level audio fixtures built into tempdirs at test time — no binary fixtures in the repo"
|
||||
- "Untagged-but-readable audio is Ok(has_tags=false) with filename-stem title; not-audio is a distinguishable Err variant"
|
||||
|
||||
key-files:
|
||||
created:
|
||||
- .planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-MUSIC-MODEL.md
|
||||
- core/archipelago/src/music/mod.rs
|
||||
- core/archipelago/src/music/tags.rs
|
||||
modified:
|
||||
- core/archipelago/src/main.rs
|
||||
- core/archipelago/Cargo.toml
|
||||
- core/Cargo.lock
|
||||
|
||||
key-decisions:
|
||||
- "D-13 one-way half decided by the operator at the Task 1 checkpoint: hybrid-identity (path row key + lazily-backfilled content-hash dedupe column), derived-albums (no stored album/artist rows), index-format-json (data_dir/music/index.json, content_server.rs precedent), sources = both OwnLibrary and Peer"
|
||||
- "MUSIC_SCHEMA_VERSION starts at 1; a newer-versioned on-disk index is treated as absent (never reinterpreted or overwritten) by an older binary"
|
||||
- "lofty 0.24.0 approved by the operator at the Task 2 blocking-human gate (crates.io downloads + Serial-ATA/lofty-rs repo + dep tree reviewed, no networking crates)"
|
||||
|
||||
patterns-established:
|
||||
- "T-13-20 mitigation shape: filesystem confinement is a function parameter, not a constant — callers cannot bypass it by construction"
|
||||
- "Audio test fixtures are Rust functions emitting spec-correct container bytes (ID3v2.4 synchsafe frames, FLAC STREAMINFO blocks, ISO-BMFF atoms, OGG pages) — keeps licensing-clean, review-friendly tests"
|
||||
|
||||
requirements-completed: [] # AIUI-03 spans multiple plans (13-11 SongGrid wiring still pending); not complete yet
|
||||
|
||||
coverage:
|
||||
- id: D1
|
||||
description: "One-way D-13 music entity model recorded in 13-MUSIC-MODEL.md before any code (identity scheme, derived albums, index path/format, sources, reindex path, newer-version handling)"
|
||||
verification: []
|
||||
human_judgment: true
|
||||
rationale: "checkpoint:decision — the operator made and approved the decision at the gate; document content is a human artifact"
|
||||
- id: D2
|
||||
description: "lofty entered the tree only after human registry-legitimacy verification (Task 2 blocking-human gate)"
|
||||
verification: []
|
||||
human_judgment: true
|
||||
rationale: "Package legitimacy is exactly the judgment the [ASSUMED] audit fallback rule reserves for a human"
|
||||
- id: D3
|
||||
description: "Tag extraction returns a full typed record for MP3/FLAC/M4A/OGG, filename-stem fallback for untagged audio, a distinct error for non-audio, and refuses paths outside media roots before opening"
|
||||
verification:
|
||||
- kind: unit
|
||||
ref: "core/archipelago/src/music/tags.rs#mp3_id3v24_yields_full_record"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "core/archipelago/src/music/tags.rs#flac_vorbis_yields_full_record"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "core/archipelago/src/music/tags.rs#m4a_yields_full_record"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "core/archipelago/src/music/tags.rs#ogg_yields_full_record"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "core/archipelago/src/music/tags.rs#untagged_file_falls_back_to_filename_stem"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "core/archipelago/src/music/tags.rs#non_audio_returns_err_distinct_from_untagged"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "core/archipelago/src/music/tags.rs#path_outside_media_roots_is_refused"
|
||||
status: pass
|
||||
human_judgment: false
|
||||
|
||||
# Metrics
|
||||
duration: ~95min (wall clock, including a broken-pipe interruption and two full non-incremental builds on the 4-core box)
|
||||
completed: 2026-08-04
|
||||
status: complete
|
||||
---
|
||||
|
||||
# Phase 13 Plan 04: Music Entity Model, lofty Gate, and Tag Extraction Summary
|
||||
|
||||
**One-way D-13 music model decided and recorded (hybrid-identity / derived-albums / JSON index), lofty 0.24.0 admitted through a human legitimacy gate, and lofty-based tag extraction landed for MP3/FLAC/M4A/OGG with media-root confinement — 7/7 tests green.**
|
||||
|
||||
## Performance
|
||||
|
||||
- **Duration:** ~95 min wall clock (dominated by two non-incremental cargo builds and a broken-pipe recovery)
|
||||
- **Started:** 2026-08-04T09:00Z (approx — first session)
|
||||
- **Completed:** 2026-08-04T10:45Z (approx)
|
||||
- **Tasks:** 3 (1 decision checkpoint, 1 human-verify gate, 1 auto/TDD)
|
||||
- **Files modified:** 6
|
||||
|
||||
## Accomplishments
|
||||
|
||||
- The irreversible half of D-13 is now a written, operator-made decision (`13-MUSIC-MODEL.md`), not an emergent property of the first implementation: hybrid-identity (path row key + lazily-backfilled content-hash dedupe column), derived albums/artists, `data_dir/music/index.json` JSON index, both `OwnLibrary` and `Peer` sources, `MUSIC_SCHEMA_VERSION = 1` with an explicit newer-version-on-older-binary contract.
|
||||
- `lofty 0.24.0` entered the tree through the Task 2 `blocking-human` legitimacy gate (its 13-RESEARCH.md audit entry was `[ASSUMED]`), with the dependency tree reviewed for networking crates.
|
||||
- `music/mod.rs` implements the decided entity model exactly; `music/tags.rs::extract_tags` extracts title/artist/album/albumartist/track/disc/year/duration across all four formats, falls back to the filename stem for untagged audio (`Ok`, `has_tags: false`), returns a distinct `NotAudio` error for non-audio, and refuses paths outside caller-supplied `media_roots` before any file is opened (T-13-20).
|
||||
- All test fixtures are generated programmatically (byte-level ID3v2.4 / FLAC / ISO-BMFF / OGG builders into tempdirs) — zero binary audio files committed.
|
||||
|
||||
## Task Commits
|
||||
|
||||
1. **Task 1: Decide the music entity model (checkpoint:decision)** — `bca4660a` (docs) — operator selected `hybrid-identity, derived-albums, index-format-json, sources=both`
|
||||
2. **Task 2: lofty legitimacy gate (checkpoint:human-verify, blocking-human)** — `61564440` (chore) — operator approved (crates.io history + Serial-ATA/lofty-rs repo + `cargo tree -i lofty` reviewed); `cargo add lofty` → 0.24.0
|
||||
3. **Task 3: Tag extraction across four formats (auto, tdd)** — `be8f24b4` (wip checkpoint, recovered work committed verbatim) + `4b577493` (feat, fixture fix completing the task)
|
||||
|
||||
_Note: Task 3's split into `be8f24b4` + `4b577493` is a broken-pipe recovery artifact — see Issues Encountered. `be8f24b4` was already pushed and stays in history; the fix was committed on top, no rewrite._
|
||||
|
||||
## Files Created/Modified
|
||||
|
||||
- `.planning/phases/13-.../13-MUSIC-MODEL.md` — the recorded one-way decision (identity, albums, index, sources, reindex path)
|
||||
- `core/archipelago/src/music/mod.rs` — `Track`, `Album`, `Artist`, `TrackId`, `AlbumId`, `ArtistId`, `MusicSource`, `MUSIC_SCHEMA_VERSION = 1`
|
||||
- `core/archipelago/src/music/tags.rs` — `extract_tags`, `RawTags`, `TagExtractionError`, `fallback_from_filename`, 7 tests + programmatic fixture builders
|
||||
- `core/archipelago/src/main.rs` — `mod music;` in the alphabetical block (between `mod monitoring;` and `mod names;`)
|
||||
- `core/archipelago/Cargo.toml` / `core/Cargo.lock` — `lofty 0.24.0`
|
||||
|
||||
## Decisions Made
|
||||
|
||||
- All D-13 sub-decisions were made by the operator at the Task 1 gate and are recorded in `13-MUSIC-MODEL.md`; `mod.rs` implements them verbatim (spot-checked field-by-field against the document).
|
||||
- `AlbumId` is `(album_artist: Option<String>, album: String)` — a derived grouping key, never persisted as a row, per derived-albums.
|
||||
- `Track::content_hash` is `#[serde(default)] Option<String>` so the lazily-backfilled dedupe column can appear later without a schema bump.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
**1. [Rule 1 - Bug] MP3 test fixture frame length off by one (210 vs 209 bytes)**
|
||||
- **Found during:** Task 3 (`mp3_id3v24_yields_full_record` failing: `NotAudio(FileDecoding(Mpeg: "File contains an invalid frame"))`)
|
||||
- **Issue:** The programmatic MP3 fixture computed the MPEG-1 Layer III frame length for header `FF FB 52 C4` as ⌈1152·64·125/44100⌉+1 = 210, but lofty's `Header::read` truncates the integer division *before* adding the padding byte: `1152*64*125/44100 = 208`, `+1` = **209**. lofty confirms a frame sync by comparing the candidate header against the bytes exactly `len` later (`find_next_frame`/`cmp_header`), so the off-by-one made the second sync check land one byte short, exhausting the search and rejecting the file.
|
||||
- **Fix:** Fixture-only — `MP3_FRAME_LEN` 210 → 209 with a corrected doc comment citing lofty's exact formula. Production code untouched: it is a straight `lofty::read_from_path` call and is correct for real-world MP3s, whose encoders emit correctly-sized frames.
|
||||
- **Files modified:** `core/archipelago/src/music/tags.rs`
|
||||
- **Verification:** `CARGO_INCREMENTAL=0 cargo test --package archipelago music::` → `ok. 7 passed; 0 failed`
|
||||
- **Committed in:** `4b577493`
|
||||
|
||||
---
|
||||
|
||||
**Total deviations:** 1 auto-fixed (Rule 1 bug, test-fixture-only)
|
||||
**Impact on plan:** None on scope — the fixture math was verified against lofty 0.24.0's vendored source (`mpeg/header.rs`, `mpeg/read.rs`) rather than guessed.
|
||||
|
||||
## Issues Encountered
|
||||
|
||||
- **Broken-pipe interruption mid-Task-3.** The original executor session died after writing `mod.rs`/`tags.rs` and running the first test pass (6/7). The recovered working tree was checkpoint-committed **verbatim** as `wip(13-04)` `be8f24b4` before any continuation work, per the track-progress process rule; this continuation session then fixed the one failing fixture on top (`4b577493`). `be8f24b4` was already pushed, so history was not rewritten.
|
||||
- **Background cargo runs on this box are unreliable to observe:** the 600s foreground timeout moved the first test run to background where its piped `tail` output was lost; re-running in foreground after the dependency rlibs were cached completed quickly. Serial cargo only (live node on the same 4-core host).
|
||||
|
||||
## Verification (plan-level)
|
||||
|
||||
- `cargo test --package archipelago music::` → **`ok. 7 passed; 0 failed`** (test invocation also compiled the package — build exit 0)
|
||||
- `grep -c '^mod music;' core/archipelago/src/main.rs` → 1 (line 64)
|
||||
- `pub struct Track` + `MUSIC_SCHEMA_VERSION` in `mod.rs`; `pub fn extract_tags` + `media_roots` parameter in `tags.rs`
|
||||
- `git ls-files core/archipelago | grep -ciE '\.(mp3|flac|m4a|ogg)$'` → 0 (no binary fixtures)
|
||||
- `grep -c '^lofty' core/archipelago/Cargo.toml` → 1
|
||||
- Entity fields in `mod.rs` match `13-MUSIC-MODEL.md` (hybrid `TrackId`, `content_hash` column, derived `AlbumId`/`ArtistId`, `MusicSource::{OwnLibrary, Peer{onion}}`, version 1)
|
||||
|
||||
## User Setup Required
|
||||
|
||||
None - no external service configuration required.
|
||||
|
||||
## Next Phase Readiness
|
||||
|
||||
- The music domain root and tag extraction exist; `music/index.rs` (scan/persist per `content_server.rs::load_catalog`'s shape) and SongGrid wiring (13-11) can now build on a decided, written schema.
|
||||
- T-13-22 (peer tag text reaching model context) remains deliberately deferred to 13-12's `wrap_untrusted` boundary — `RawTags` strings carry no trust and nothing here puts them in a model context.
|
||||
- Waves 3+ per ROADMAP/plan dependencies are unblocked.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- FOUND: `.planning/phases/13-.../13-MUSIC-MODEL.md`, `core/archipelago/src/music/mod.rs`, `core/archipelago/src/music/tags.rs`
|
||||
- FOUND commits: `bca4660a`, `61564440`, `be8f24b4`, `4b577493`
|
||||
|
||||
---
|
||||
*Phase: 13-aiui-functional-conversational-node-control-and-content-surf*
|
||||
*Completed: 2026-08-04*
|
||||
Reference in New Issue
Block a user