diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 81be0df..583007f 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -14,7 +14,7 @@ Requirements for v1.2 Tag Editing milestone. Each maps to roadmap phases. ### Tag Writing -- [ ] **WRITE-01**: Write metadata tags to MP3 files via ID3v2 (title, artist, album, genre, year, track#, disc#, composer) +- [x] **WRITE-01**: Write metadata tags to MP3 files via ID3v2 (title, artist, album, genre, year, track#, disc#, composer) - [x] **WRITE-02**: Write metadata tags to FLAC files via Vorbis Comments - [ ] **WRITE-03**: Write metadata tags to OGG Vorbis files via custom page rewriter - [x] **WRITE-04**: Embed cover art image (JPEG/PNG) in MP3 and FLAC files @@ -93,7 +93,7 @@ Which phases cover which requirements. Updated during roadmap creation. |-------------|-------|--------| | SCHEMA-01 | Phase 15 | Complete | | SCHEMA-02 | Phase 15 | Complete | -| WRITE-01 | Phase 16 | Pending | +| WRITE-01 | Phase 16 | Complete | | WRITE-02 | Phase 16 | Complete | | WRITE-03 | Phase 19 | Pending | | WRITE-04 | Phase 16 | Complete | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index f757381..7762b95 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -72,7 +72,7 @@ Plans: 3. Cover art images (JPEG/PNG) can be embedded in both MP3 and FLAC files — the embedded image is readable back and the existing cover art pipeline (extraction, thumbnails) works with the newly embedded art 4. After a tag write, the database reflects the new metadata within the same operation: artist/album/genre entities are created or relinked (never mutated in-place), orphaned entities with zero remaining references are cleaned up, and the FTS5 index is updated — no library rescan needed 5. If the currently-playing track is being edited, playback is stopped before the file write begins — the user does not experience a crash or corrupted audio stream -**Plans:** 1/3 plans executed +**Plans:** 2/3 plans executed Plans: - [ ] 16-01-PLAN.md — Tagwriter foundation + sqlc queries + MP3 writer (Wave 1) - [ ] 16-02-PLAN.md — FLAC writer with go-flac ecosystem (Wave 1) @@ -129,7 +129,7 @@ Plans: | 13. Library Views & Phantom Tracks | v1.1 | 2/2 | Complete | 2026-03-16 | | 14. Performance Optimization | v1.1 | 4/4 | Complete | 2026-03-15 | | 15. Schema Migration & Write Safety | 2/2 | Complete | 2026-03-16 | - | -| 16. Tag Writing & Database Sync | 1/3 | In Progress| | - | +| 16. Tag Writing & Database Sync | 2/3 | In Progress| | - | | 17. Single Track Edit | v1.2 | 0/? | Not started | - | | 18. Batch Edit | v1.2 | 0/? | Not started | - | | 19. OGG Vorbis Tag Writing | v1.2 | 0/? | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 464c1c3..0a7b748 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,12 +3,12 @@ gsd_state_version: 1.0 milestone: v1.2 milestone_name: Tag Editing status: unknown -last_updated: "2026-03-16T22:21:06.448Z" +last_updated: "2026-03-17T14:42:46.662Z" progress: - total_phases: 1 + total_phases: 2 completed_phases: 1 - total_plans: 2 - completed_plans: 2 + total_plans: 5 + completed_plans: 4 --- # YellowJacket — Project State @@ -23,9 +23,9 @@ See: .planning/PROJECT.md (updated 2026-03-16) ## Current Position Phase: Phase 16 — Tag Writing & Database Sync (in progress) -Plan: 2 of 3 in progress -Status: Executing Phase 16 plans — Plan 02 (FLAC writer) complete -Last activity: 2026-03-17 — Completed 16-02 (FLAC tag writer) +Plan: 2 of 3 complete +Status: Executing Phase 16 plans — Plans 01 (MP3 writer + sqlc) and 02 (FLAC writer) complete +Last activity: 2026-03-17 — Completed 16-01 (MP3 writer + orphan queries) ### Phase Overview @@ -59,6 +59,7 @@ Last activity: 2026-03-17 — Completed 16-02 (FLAC tag writer) |-------|------|----------|-------|-------| | 15 | 01 | 15min | 2 | 5 | | 15 | 02 | 16min | 2 | 2 | +| 16 | 01 | 28min | 2 | 14 | | 16 | 02 | 20min | 2 | 6 | ## Accumulated Context @@ -85,6 +86,8 @@ Decisions from v1.0 and v1.1 are archived in PROJECT.md Key Decisions table. Key | `*slog.Logger` as first param for AtomicWrite | Matches codebase convention — all packages accept logger as first arg | | go-flac `WriteTo(io.Writer)` for AtomicWrite integration | Pipes directly into temp file callback; avoids `Save(path)` file path conflicts | | `replaceVorbisComment` as filter+add pattern | flacvorbis has no Set/Replace — must remove existing entries then Add new value | +| id3v2 WriteTo + manual audio copy for AtomicWrite | `tag.Save()` writes to original file; use `WriteTo(tmp)` + seek past tag + `io.Copy` audio data | +| Snapshot tag size before `id3v2.Open()` | `originalSize` is unexported; read 10-byte ID3v2 header and decode synchsafe size ourselves | ### v1.2 Roadmap Decisions @@ -125,8 +128,8 @@ Decisions from v1.0 and v1.1 are archived in PROJECT.md Key Decisions table. Key ### Last Session **Date:** 2026-03-17 -**What happened:** Executed Phase 16 Plan 02 — FLAC tag writer using go-flac ecosystem with Vorbis Comments, PICTURE blocks, AtomicWrite integration, and 7 round-trip tests. -**Where we stopped:** Completed 16-02-PLAN.md — Plan 03 remaining +**What happened:** Executed Phase 16 Plan 01 — MP3 tag writer with n10v/id3v2, orphan-counting sqlc queries, and 5 round-trip tests. Plans 01 and 02 now both complete. +**Where we stopped:** Completed 16-01-PLAN.md — Plan 03 remaining **Next action:** Execute 16-03-PLAN.md (WriteTrackTags entry point + DB sync) --- @@ -141,4 +144,4 @@ Decisions from v1.0 and v1.1 are archived in PROJECT.md Key Decisions table. Key | 19 | fix phantom playlist tracks with multi-root path resolution | 2026-03-16 | 9144ded | [19-fix-phantom-playlist-tracks](./quick/19-fix-phantom-playlist-tracks/) | Last activity: 2026-03-16 - Completed quick task 19: fix phantom playlist tracks with multi-root path resolution -*Last updated: 2026-03-17 — Completed 16-02 (FLAC tag writer) — Phase 16 in progress* +*Last updated: 2026-03-17 — Completed 16-01 (MP3 writer + orphan queries) — Phase 16 in progress (2/3)* diff --git a/.planning/phases/16-tag-writing-database-sync/16-01-SUMMARY.md b/.planning/phases/16-tag-writing-database-sync/16-01-SUMMARY.md new file mode 100644 index 0000000..b64e426 --- /dev/null +++ b/.planning/phases/16-tag-writing-database-sync/16-01-SUMMARY.md @@ -0,0 +1,132 @@ +--- +phase: 16-tag-writing-database-sync +plan: 01 +subsystem: database, tagwriter +tags: [sqlc, id3v2, mp3, atomicwrite, tag-writing] + +# Dependency graph +requires: + - phase: 15-schema-migration-write-safety + provides: AtomicWrite utility for crash-safe file writes +provides: + - TagChanges type and field name constants for diff-map API + - writeMp3Tags function with ID3v2 + AtomicWrite integration + - Orphan-counting sqlc queries (CountArtistCreditReferences, CountReleaseGroupRecordings, CountGenreReferences, DeleteGenre) + - detectMIME helper for JPEG/PNG magic byte detection + - id3v2OriginalTagSize helper for locating audio data offset in MP3 files +affects: [16-tag-writing-database-sync, 17-single-track-edit] + +# Tech tracking +tech-stack: + added: [github.com/bogem/id3v2/v2] + patterns: [diff-map tag changes, synchsafe integer decoding, ID3v2 WriteTo + audio copy for atomic rewrite] + +key-files: + created: + - backend/tagwriter/tagwriter.go + - backend/tagwriter/mp3.go + - backend/tagwriter/mp3_test.go + modified: + - backend/database/sql/queries/recordings.sql + - backend/database/sql/queries/artist_credit.sql + - backend/database/sql/queries/release_groups.sql + - backend/database/sql/queries/genres.sql + - backend/database/sql/sqlcgen/recordings.sql.go + - backend/database/sql/sqlcgen/artist_credit.sql.go + - backend/database/sql/sqlcgen/release_groups.sql.go + - backend/database/sql/sqlcgen/genres.sql.go + - go.mod + - go.sum + +key-decisions: + - "Used id3v2.WriteTo + manual audio data copy for AtomicWrite integration instead of tag.Save()" + - "Snapshot original tag size before opening for edit to reliably locate audio data offset" + - "Shared test helpers in helpers_test.go for both MP3 and FLAC test files" + +patterns-established: + - "id3v2 WriteTo + copyAudioData pattern: write new tag to temp file, seek past original tag in source, copy audio data, atomic rename" + - "Synchsafe integer decoding for ID3v2 header parsing" + +requirements-completed: [WRITE-01, WRITE-04] + +# Metrics +duration: 28min +completed: 2026-03-17 +--- + +# Phase 16 Plan 01: Tagwriter Foundation + MP3 Writer Summary + +**MP3 tag writer with n10v/id3v2 using AtomicWrite for crash-safe ID3v2 rewriting, plus orphan-counting sqlc queries for entity cleanup** + +## Performance + +- **Duration:** 28 min +- **Started:** 2026-03-17T14:12:10Z +- **Completed:** 2026-03-17T14:40:39Z +- **Tasks:** 2 +- **Files modified:** 14 + +## Accomplishments +- Orphan-counting sqlc queries for artist credits, release groups, and genres (5 new queries across 4 SQL files) +- MP3 tag writing for all 8 text fields + cover art embed/clear via n10v/id3v2 with AtomicWrite crash safety +- Round-trip tests verify tags written by id3v2 are readable by dhowden/tag (metadata.ExtractTags) +- TagChanges diff-map type and field constants established as the public API for callers + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Add orphan-counting sqlc queries** - `3642cbe` (feat) — queries were included in an earlier commit alongside tagwriter foundation +2. **Task 2: Create tagwriter package with types and MP3 writer** - `6bd65a6` (feat) — mp3.go and mp3_test.go with 5 round-trip tests + +**Plan metadata:** (this commit) + +_Note: Tasks 1 and 2 were committed by a concurrent session that also executed Plan 02 (FLAC writer). The sqlc queries and tagwriter.go were committed in `3642cbe` alongside FLAC work; the MP3 writer was committed in `6bd65a6` alongside Plan 02's summary._ + +## Files Created/Modified +- `backend/tagwriter/tagwriter.go` — Package types (TagChanges, AudioFormat), field constants, format detection, MIME detection, ID3v2 tag size helper +- `backend/tagwriter/mp3.go` — writeMp3Tags with applyTextChanges, applyCoverArtChanges, copyAudioData +- `backend/tagwriter/mp3_test.go` — 5 tests: text fields, cover art, clear art, partial update, atomic safety +- `backend/tagwriter/helpers_test.go` — Shared test helpers (testLogger, tinyJPEG, assertEqual, assertStrField, assertIntField) +- `backend/database/sql/queries/recordings.sql` — CountRecordingsByArtistCredit query +- `backend/database/sql/queries/artist_credit.sql` — CountArtistCreditReferences query +- `backend/database/sql/queries/release_groups.sql` — CountReleaseGroupRecordings query +- `backend/database/sql/queries/genres.sql` — CountGenreReferences and DeleteGenre queries +- `go.mod` / `go.sum` — Added github.com/bogem/id3v2/v2 v2.1.4 + +## Decisions Made +- **id3v2 WriteTo + manual audio copy** — The n10v/id3v2 library's `Save()` method writes directly to the original file, bypassing AtomicWrite. Instead, we use `WriteTo(tmpFile)` to write the new tag, then read the original file's audio data (skipping past the original ID3v2 header using `id3v2OriginalTagSize()`) and copy it into the temp file. AtomicWrite renames the temp file over the original. +- **Snapshot tag size before Open** — `id3v2.Tag.originalSize` is unexported. We read the ID3v2 header ourselves (10-byte header with synchsafe size integer) to determine where audio data starts. This is done before `id3v2.Open()` to avoid any interference. +- **Shared test helpers across formats** — Created `helpers_test.go` with common test utilities (logger, JPEG generator, assertion functions) used by both mp3_test.go and flac_test.go. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking] Pre-existing untracked FLAC writer files from aborted session** +- **Found during:** Task 2 (MP3 writer implementation) +- **Issue:** The `backend/tagwriter/` directory already contained `tagwriter.go`, `flac.go`, `flac_test.go`, and `helpers_test.go` from a previous aborted session that had executed Plan 02 before Plan 01. These files were untracked but present on disk, causing compilation conflicts. +- **Fix:** Integrated with the existing file layout — used helpers from `helpers_test.go` instead of duplicating, and ensured `mp3.go` fit into the existing package structure. +- **Files modified:** mp3_test.go (adapted to use existing shared helpers) +- **Verification:** All 12 tests pass, lint clean +- **Committed in:** 6bd65a6 + +--- + +**Total deviations:** 1 auto-fixed (1 blocking) +**Impact on plan:** Minimal — the pre-existing FLAC writer code was from Plan 02 which would have been next anyway. Integration was straightforward. + +## Issues Encountered +None — tests and lint passed on first run after integration. + +## User Setup Required +None — no external service configuration required. + +## Next Phase Readiness +- Plan 01 (sqlc queries + MP3 writer) and Plan 02 (FLAC writer) are both complete +- Ready for Plan 03 (WriteTrackTags entry point, DB sync pipeline, player safety, scan mutex, events) +- All format-specific writers are tested and lint-clean + +--- +*Phase: 16-tag-writing-database-sync* +*Completed: 2026-03-17*