diff --git a/.planning/research/ARCHITECTURE.md b/.planning/research/ARCHITECTURE.md index 9549116..30854d9 100644 --- a/.planning/research/ARCHITECTURE.md +++ b/.planning/research/ARCHITECTURE.md @@ -1,118 +1,469 @@ -# Architecture Research: Multi-Library Integration +# Architecture Patterns: Tag Editing Integration -**Researched:** 2026-03-08 -**Confidence:** HIGH +**Domain:** Audio metadata editing in existing music player +**Researched:** 2026-03-16 +**Confidence:** HIGH (based on full codebase analysis of existing architecture) -## Design Decision: Hybrid Model +## Recommended Architecture -- **`library_id` on `audio_files` only** — physical file binding -- **Artists, albums, recordings, genres stay global** — shared reference data -- **Unified presentation by default** — optional library filter -- **Cross-library playlists** — playlists reference audio_file_id -- **Phantom tracks on removal** — playlist entries preserved with metadata +Tag editing is a **cross-cutting operation** that touches files, database entities, the FTS5 search index, the cover art pipeline, and the frontend cache — all from a single user action. The architecture adds a new `backend/tageditor/` package that orchestrates the full write pipeline, keeping the existing `library`, `metadata`, and `database` packages focused on their current responsibilities. -## Database Changes +### High-Level Data Flow -### New Table: libraries +``` +UI: track-details "Save" click + → Wails binding: tageditor.EditTrack(filePath, changes) + → 1. Validate input + resolve audio_file by path + → 2. Write tags to temp file, rename over original (safe write) + → 3. Update DB entities in single transaction: + a. Upsert artist_credit + artist (if artist changed) + b. Upsert release_group (if album changed) + c. Update recording fields (title, year, track#, etc.) + d. Update genre links (delete old, insert new) + e. Update release_group_recordings link (if album changed) + f. Handle cover art (if image provided) + → 4. Update FTS5 search_index (re-insert with same rowid) + → 5. Emit TagsUpdated event with affected file paths + → Frontend: libraryStore receives event, patches cached tracks in-place + → All views re-render with updated metadata +``` + +### Component Boundaries + +| Component | Responsibility | Communicates With | +|-----------|---------------|-------------------| +| `backend/tageditor/` (NEW) | Orchestrates tag write pipeline: file write + DB update + FTS5 + events | `metadata/`, `database/`, `events/`, `coverart/`, Wails runtime | +| `backend/tageditor/writer.go` (NEW) | Format-specific tag writing (MP3/FLAC/OGG) via external libraries | File system, `bogem/id3v2`, `go-flac/go-flac` + `go-flac/flacvorbis` | +| `backend/metadata/tags.go` (EXISTING) | Tag reading via `dhowden/tag` — **no changes needed** | File system | +| `backend/library/library.go` (EXISTING) | Scan pipeline, entity upsert helpers — **reuse `processMetadata` pattern** | `database/`, `metadata/` | +| `backend/database/search.go` (EXISTING) | FTS5 index operations — **add `UpdateSearchIndex` method** | SQLite | +| `backend/events/events.go` (EXISTING) | Event constants — **add tag editing events** | Nothing | +| `frontend/src/components/track-details/` (EXISTING) | Edit UI — **wire Save to backend, add batch mode** | `tageditor` Wails binding | +| `frontend/src/store/library-store.ts` (EXISTING) | Track cache — **add event handler for in-place patch** | Wails events | + +## New Package: `backend/tageditor/` + +### Why a Separate Package + +The tag editing flow does NOT fit cleanly into the existing `library` package because: + +1. **Different lifecycle**: Scans are bulk, batch-oriented operations. Tag edits are individual, user-initiated, synchronous operations. +2. **Different entity update strategy**: Scans always CREATE new recordings. Tag edits must UPDATE existing recordings and handle shared entity reference changes. +3. **Different file I/O pattern**: Scans read files. Tag edits write files with safety guarantees (temp + rename). +4. **Wails binding boundary**: Tag editor needs its own binding registration for a clean API surface. + +However, the tag editor REUSES logic from existing packages: +- Entity upsert helpers from `library` (either extracted to shared code or duplicated with attribution) +- FTS5 operations from `database/search.go` +- Cover art pipeline from `library/coverart.go` and `coverart/` + +### Package Structure + +``` +backend/tageditor/ +├── tageditor.go # Service struct, EditTrack(), EditTracks(), SetCoverArt() +├── writer.go # Format-specific tag writing (MP3, FLAC, OGG) +└── writer_test.go # Tests for safe file write + tag round-trip +``` + +### Service API (Wails-Bound) + +```go +// Package tageditor provides audio file tag editing with safe +// file writes and inline database synchronization. +package tageditor + +// EditRequest describes changes to apply to a single track. +type EditRequest struct { + FilePath string `json:"filePath"` + Title *string `json:"title,omitempty"` + Artist *string `json:"artist,omitempty"` + Album *string `json:"album,omitempty"` + Genre *string `json:"genre,omitempty"` + Year *int `json:"year,omitempty"` + TrackNumber *int `json:"trackNumber,omitempty"` + DiscNumber *int `json:"discNumber,omitempty"` + Composer *string `json:"composer,omitempty"` + // CoverArt is set separately via SetCoverArt() +} + +// EditResult reports the outcome of a tag edit operation. +type EditResult struct { + FilePath string `json:"filePath"` + Success bool `json:"success"` + Error string `json:"error,omitempty"` +} + +// Service orchestrates tag editing operations. +type Service struct { + ctx context.Context + logger *slog.Logger + db *database.DB +} + +// EditTrack applies metadata changes to a single audio file. +func (s *Service) EditTrack(req EditRequest) EditResult + +// EditTracks applies shared field changes to multiple files (batch). +func (s *Service) EditTracks(reqs []EditRequest) []EditResult + +// SetCoverArt embeds an image file into one or more audio files. +func (s *Service) SetCoverArt(filePaths []string, imagePath string) []EditResult +``` + +Pointer fields (`*string`, `*int`) distinguish "not changed" (nil) from "set to empty/zero" (pointer to zero value). This is critical for batch editing where you only want to change shared fields. + +### Two-Phase Initialization + +Follows the existing `NewService()` + `SetContext()` pattern: + +```go +// In NewYellowJacketApp(): +yjApp.tagEditor = tageditor.NewService(logger, db) + +// In OnStartup(): +yj.tagEditor.SetContext(ctx) + +// In FEBindings: +yjApp.FEBindings = []any{ + // ... existing bindings ... + yjApp.tagEditor, +} +``` + +## File Writing Strategy + +### Write-to-Temp-Then-Rename (Corruption Safety) + +``` +1. Write modified tags to temporary file in same directory: + /music/track.mp3 → /music/.track.mp3.yjtmp +2. fsync the temp file +3. os.Rename temp file over original (atomic on same filesystem) +4. If any step fails, delete temp file and return error +``` + +Why same directory: `os.Rename` is atomic only within the same filesystem. Writing to a temp directory on a different mount would require a full copy. + +### Format-Specific Writers + +| Format | Library | Write Strategy | +|--------|---------|----------------| +| MP3 (ID3v2) | `github.com/bogem/id3v2/v2` (v2.1.4) | Open → parse existing → modify frames → Save() writes to same file. Use WriteTo() to write to temp file instead. | +| FLAC (Vorbis Comments) | `github.com/go-flac/go-flac/v2` + `github.com/go-flac/flacvorbis/v2` | ParseFile → find/create VorbisComment metablock → set fields → Save() to temp file | +| OGG (Vorbis Comments) | Custom or `dhowden/tag`-compatible approach | OGG Vorbis uses same comment format as FLAC. May need lower-level OGG page rewriting. **Needs deeper research at implementation time.** | + +**Confidence notes:** +- MP3 via `bogem/id3v2`: HIGH — mature library (359 stars, v2.1.4, 57 importers), well-documented read+write API, supports ID3v2.3 and v2.4, picture frames, UTF-8 encoding. +- FLAC via `go-flac/go-flac` + `go-flac/flacvorbis`: MEDIUM — smaller community (12 stars on flacvorbis), but clean API for metadata block manipulation. `flac.Save(filename)` writes back to disk. +- OGG Vorbis: LOW — no well-established pure-Go OGG tag writing library. May need to shell out to a tool or implement custom OGG page rewriting. **Consider deferring OGG write support to a follow-up if complexity is high.** + +### Cover Art Embedding + +For cover art, the writer embeds the image data directly into the audio file: + +- **MP3**: `id3v2.PictureFrame` with `PTFrontCover` type +- **FLAC**: `flac.MetaDataBlockPicture` (FLAC picture metadata block) + +After writing to the audio file, the cover art pipeline also: +1. Saves the image to the covers directory (hash-based filename) +2. Generates size variants (sm/md/lg) +3. Upserts the `cover_art` DB record +4. Updates `release_groups.cover_art_id` if needed + +## Database Update Strategy + +### The Shared Entity Problem + +The normalized schema means entities are shared across tracks: + +``` +artist_credit "The Beatles" ← referenced by 200 recordings +release_group "Abbey Road" ← referenced by 17 recordings +genre "Rock" ← referenced by 5000 recordings +``` + +When a user changes a track's artist from "The Beatles" to "The Beetles" (typo fix), we must NOT modify the existing `artist_credit` row — that would change the artist name for all 200 tracks. + +### Update Rules + +| Field Changed | DB Operation | +|---------------|-------------| +| Title | UPDATE `recordings.name` directly (recording is per-track) | +| Track Number | UPDATE `recordings.track_number` directly | +| Disc Number | UPDATE `recordings.disc_number` directly | +| Year | UPDATE `recordings.year` directly | +| Composer | UPDATE `recordings.composer` directly | +| Artist | Upsert new `artist_credit` + `artist`, UPDATE `recordings.artist_credit_id` to point to new credit. Old credit is NOT deleted (may be used by other recordings). | +| Album | Upsert new `release_group`, update `release_group_recordings` link. Old release group is NOT deleted. | +| Genre | Delete existing `recording_genres` links for this recording, upsert new genres, create new links. Old genres NOT deleted (shared). | +| Cover Art | Process through cover art pipeline, update `release_groups.cover_art_id` | + +### Orphan Cleanup Strategy + +After tag edits, orphaned entities (artist credits, release groups, genres with zero references) accumulate. Two options: + +**Option A: Lazy cleanup (RECOMMENDED)** +- Orphans are harmless — they don't appear in queries because all views JOIN through `audio_files → recordings → ...` +- Clean up during the next library rescan (existing orphan cleanup phase) +- Zero additional complexity in the tag edit path + +**Option B: Eager cleanup** +- After each edit, run reference-counting DELETE queries for affected entities +- Adds complexity and transaction time to every edit +- Only worthwhile if orphans cause visible problems (they don't) + +**Decision: Option A.** The existing rescan orphan cleanup handles this. Tag editing should be fast and simple. + +### Transaction Shape + +Single transaction per track edit: ```sql -CREATE TABLE IF NOT EXISTS libraries ( - id INTEGER PRIMARY KEY, - name TEXT NOT NULL, - path TEXT NOT NULL UNIQUE, - scan_concurrency TEXT NOT NULL DEFAULT 'auto', - created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, - last_scanned_at DATETIME -); +BEGIN; +-- 1. Upsert artist_credit (if artist changed) +INSERT INTO artist_credit(text) VALUES(?) ON CONFLICT(text) DO UPDATE SET text=text RETURNING *; +INSERT INTO artists(name) VALUES(?) ON CONFLICT(name) DO UPDATE SET name=name RETURNING *; +INSERT OR IGNORE INTO artist_credit_artist(artist_id, credit_id) VALUES(?, ?); + +-- 2. Upsert release_group (if album changed) +INSERT INTO release_groups(name, album_artist_credit_id) VALUES(?, ?) + ON CONFLICT(name, album_artist_credit_id) DO UPDATE SET name=name RETURNING *; + +-- 3. Update recording +UPDATE recordings SET name=?, artist_credit_id=?, track_number=?, disc_number=?, + year=?, genre=?, composer=? WHERE id=?; + +-- 4. Update genre links (if genre changed) +DELETE FROM recording_genres WHERE recording_id = ?; +INSERT INTO genres(name) VALUES(?) ON CONFLICT(name) DO UPDATE SET name=name RETURNING *; +INSERT INTO recording_genres(recording_id, genre_id) VALUES(?, ?); + +-- 5. Update release_group_recordings (if album changed) +DELETE FROM release_group_recordings WHERE recording_id = ?; +INSERT INTO release_group_recordings(release_group_id, recording_id, track_number, disc_number) VALUES(?, ?, ?, ?); + +-- 6. FTS5 update (re-insert with same rowid) +INSERT INTO search_index(rowid, file_path, title, artist, album) VALUES(?, ?, ?, ?, ?); +COMMIT; ``` -### Migration 6: Multi-Library Support +### FTS5 Update Pattern -Order of operations: -1. Create `libraries` table -2. Read TOML `DirectoryPath`, insert as default library -3. `ALTER TABLE audio_files ADD COLUMN library_id INTEGER NOT NULL DEFAULT {defaultLibID}` -4. Create index on `audio_files(library_id)` -5. Rebuild `playlist_tracks` with SET NULL FK + phantom columns -6. Drop and recreate `track_metadata` VIEW with `library_id` -7. Set `PRAGMA user_version = 6` +The current `search_index` is contentless (`content=''`), which means: +- DELETE is not supported +- INSERT with an existing rowid adds a new entry; the old one becomes stale +- Stale entries are filtered out by the JOIN against `track_metadata` in search queries -### playlist_tracks Rebuild (for phantom support) +This works correctly for tag editing: re-INSERT with the same `audio_files.id` as rowid. The stale entry for the old metadata is harmless and filtered by the VIEW JOIN. -```sql -CREATE TABLE playlist_tracks_new ( - id INTEGER PRIMARY KEY, - playlist_id INTEGER NOT NULL, - audio_file_id INTEGER, -- NOW NULLABLE - position INTEGER NOT NULL, - phantom_file_path TEXT, - phantom_title TEXT, - phantom_artist TEXT, - phantom_album TEXT, - FOREIGN KEY(playlist_id) REFERENCES playlists(id) ON DELETE CASCADE, - FOREIGN KEY(audio_file_id) REFERENCES audio_files(id) ON DELETE SET NULL -); +**No FTS5 schema changes needed.** + +## Events + +### New Events + +```go +// Tag editing events. +const ( + TagsUpdated = "TagsUpdated" // Single or batch edit complete + TagEditFailed = "TagEditFailed" // Edit failed (file write error, etc.) +) ``` -Two-phase library removal: -1. Populate phantom metadata BEFORE deleting audio_files -2. Delete audio_files -> SET NULL triggers -> phantom columns preserve display info +### Event Payloads -### track_metadata VIEW (updated) +```go +// TagsUpdated payload: +type TagsUpdatedPayload struct { + FilePaths []string `json:"filePaths"` // All affected file paths +} -Add `af.library_id` to SELECT list. Same JOIN structure. Consumers get library_id for filtering. - -## Scan Pipeline Changes - -- `Scan()` -> `ScanLibrary(libraryID int64)` — accepts library ID, loads path from DB -- `ScanAllLibraries()` — sequential iteration, one at a time -- Orphan cleanup scoped to library being scanned -- Entity cache (artists, albums) remains per-scan and works correctly (shared entities) -- Progress events include library_id and library_name - -## Orphan Cleanup (Library Removal) - -Reference-counting bottom-up deletes in single transaction: -``` -audio_files (library_id = X) -> DELETE -recordings (no remaining audio_files) -> DELETE -release_group_recordings (orphaned) -> DELETE -recording_genres (orphaned) -> DELETE -release_groups (no remaining recordings) -> DELETE -artist_credit (no remaining references) -> DELETE -artist_credit_artist (orphaned) -> DELETE -artists (no remaining credits) -> DELETE -genres (no remaining recording links) -> DELETE -cover_art (no remaining release_groups) -> DELETE +// TagEditFailed payload: +type TagEditFailedPayload struct { + FilePath string `json:"filePath"` + Error string `json:"error"` +} ``` -## FTS5 Search Index +### Frontend Event Handling -Contentless FTS5 (`content=''`) works naturally: -- Search queries JOIN `search_index` on `track_metadata` (which now has `library_id`) -- Library-filtered search: add `AND tm.library_id = ?` to WHERE clause -- Stale entries after library removal filtered out by JOIN (same as current orphan behavior) -- Consider migrating to `contentless_delete=1` (SQLite 3.43.0+) for per-row DELETE support +When `TagsUpdated` fires: +1. `libraryStore` re-fetches all data (simplest approach for v1) +2. OR `libraryStore` patches affected tracks in-place from the payload (more complex but avoids full reload) -## Frontend Architecture +**Recommendation:** Start with full re-fetch on `TagsUpdated`. Optimize to incremental patch later if performance is an issue. The existing `LibraryScanComplete` handler already does a full re-fetch, so this is consistent. -- `libraryStore` gains: library list, active filter (null = all), persistence in localStorage -- Backend filtering (not frontend) — pass libraryID to backend queries -- `library-manager` component redesigned: library list view, add/remove/rename, per-library scan -- All browse views check active filter when fetching data -- New events: LibraryAdded, LibraryRemoved, LibraryRenamed -- Existing scan events gain library_id in payload +## Frontend Integration -## Config Migration +### Existing `track-details` Component -- TOML `[Library].DirectoryPath` read once during migration 6, inserted as default library -- Post-migration: library management through DB only -- `ScanConcurrency` moves per-library (DB column) with global default fallback -- `SetLibraryDirectory()` and `GetLibraryDirectory()` deprecated +The component already has: +- Edit mode toggle with input fields for all editable metadata +- `editValues` state tracking changes +- `saveEdit()` method (currently a no-op TODO) -## Build Order +Changes needed: +1. Wire `saveEdit()` to call `tageditor.EditTrack()` via Wails binding +2. Add loading/saving state for the save button +3. Add error display if the edit fails +4. Close dialog and emit refresh on success +5. Add cover art upload: file picker → `tageditor.SetCoverArt()` -1. Schema & Migration (foundation) -2. Backend scan pipeline (per-library scanning) -3. Backend API (CRUD, filtered queries, events) -4. Frontend (library manager, filter, store updates) +### Batch Editing (Multi-Select) + +The track list already has multi-select via `SelectionController`. Batch editing needs: + +1. New context menu item: "Edit Tags" (when multiple tracks selected) +2. A batch edit dialog variant of `track-details` that: + - Shows "Multiple Values" placeholder for fields that differ across selected tracks + - Only sends changed fields (using the `*string`/`*int` nil-means-no-change pattern) + - Calls `tageditor.EditTracks()` for all selected files + +### Store Updates + +`library-store.ts` needs: +```typescript +// In constructor, add event listener: +EventsOn(Events.TagsUpdated, () => { + // Re-fetch all data to reflect changes + this.eagerFetch(); +}); +``` + +This ensures all views (tracks, albums, artists, genres) reflect the updated metadata without manual cache invalidation. + +## Integration Points Summary + +| Existing Component | Change Type | What Changes | +|-------------------|-------------|-------------| +| `backend/app.go` | MODIFY | Add `tagEditor` field, wire in `NewYellowJacketApp`/`OnStartup`, add to `FEBindings` | +| `backend/events/events.go` | MODIFY | Add `TagsUpdated`, `TagEditFailed` constants | +| `frontend/src/events.ts` | MODIFY (auto-generated) | Mirror new event constants | +| `backend/database/search.go` | MINOR MODIFY | No changes needed — existing `InsertSearchIndex` works for re-insert | +| `backend/metadata/tags.go` | NO CHANGE | Read-only, continues to work as-is | +| `backend/library/library.go` | MINOR MODIFY | Extract `processMetadata` helpers to be reusable, or duplicate in tageditor with attribution | +| `backend/library/query.go` | NO CHANGE | Query methods work as-is | +| `frontend/src/components/track-details/` | MODIFY | Wire save to backend, add loading states, error handling | +| `frontend/src/store/library-store.ts` | MODIFY | Add `TagsUpdated` event listener for cache refresh | +| `go.mod` | MODIFY | Add `bogem/id3v2/v2`, `go-flac/go-flac/v2`, `go-flac/flacvorbis/v2` | + +## Patterns to Follow + +### Pattern 1: Pointer Fields for Optional Updates +**What:** Use `*string` and `*int` in `EditRequest` to distinguish "no change" from "set to empty/zero" +**When:** Any API that partially updates a record +**Example:** +```go +type EditRequest struct { + Title *string `json:"title,omitempty"` + Year *int `json:"year,omitempty"` +} + +// nil = don't change, non-nil = set to this value +if req.Title != nil { + recording.Name = *req.Title +} +``` + +### Pattern 2: Write-to-Temp-Then-Rename +**What:** Write to a temporary file in the same directory, then atomically rename +**When:** Any file modification that must not corrupt the original on failure +**Example:** +```go +tmpPath := filepath.Join(dir, "."+base+".yjtmp") +// Write to tmpPath... +if err := os.Rename(tmpPath, originalPath); err != nil { + os.Remove(tmpPath) + return err +} +``` + +### Pattern 3: Upsert-and-Relink for Shared Entities +**What:** Create new shared entity (artist/album/genre) and update the FK reference, rather than modifying the shared entity in place +**When:** Editing a field that maps to a shared/normalized entity +**Why:** Modifying a shared row would change data for all tracks referencing it + +## Anti-Patterns to Avoid + +### Anti-Pattern 1: Modifying Shared Entity Rows In-Place +**What:** `UPDATE artists SET name = ? WHERE id = ?` to change an artist name +**Why bad:** Changes the name for ALL tracks by that artist, not just the edited track +**Instead:** Upsert a new artist_credit, update the recording's FK to point to the new one + +### Anti-Pattern 2: Full Library Rescan After Tag Edit +**What:** Triggering a library scan to pick up tag changes +**Why bad:** Scans take seconds to minutes. Creates new recordings instead of updating existing ones. Terrible UX. +**Instead:** Inline DB update in the same transaction as the file write + +### Anti-Pattern 3: Frontend-Side Tag File Writing +**What:** Reading/writing audio files from TypeScript via File API +**Why bad:** Wails WebView doesn't have full filesystem access. Tag writing libraries are Go-native. +**Instead:** All file I/O happens in Go backend; frontend sends edit requests via Wails bindings + +### Anti-Pattern 4: Deleting and Recreating Recordings on Edit +**What:** DELETE the old recording, CREATE a new one with updated metadata +**Why bad:** Changes the recording ID, breaking all references (audio_files.recording_id, release_group_recordings, recording_genres, queue, playlists referencing file paths) +**Instead:** UPDATE the existing recording row in place + +## Scalability Considerations + +| Concern | Single Track Edit | Batch Edit (100 tracks) | Batch Edit (1000 tracks) | +|---------|-------------------|------------------------|--------------------------| +| File I/O | ~50ms (one file read+write) | ~5s (sequential, safe) | ~50s (consider progress bar) | +| DB Transaction | <10ms | <100ms (single transaction) | <500ms (batch in groups of 100) | +| FTS5 Update | <1ms | <10ms | <50ms | +| Frontend Refresh | Instant (single event) | Single event, full re-fetch | Single event, full re-fetch | +| Memory | Negligible | ~100MB if all cover arts loaded | Consider streaming cover art | + +For batch edits of >50 tracks, the UI should show a progress indicator. The backend should emit progress events similar to scan progress. + +## Build Order (Dependency-Aware) + +1. **Tag writing library integration** (`backend/tageditor/writer.go`) + - Add dependencies to `go.mod` + - Implement format-specific writers (MP3, FLAC) + - Write-to-temp-then-rename safety wrapper + - Unit tests with real audio files + +2. **DB update logic** (`backend/tageditor/tageditor.go`) + - Shared entity upsert (reuse or extract from library package) + - Recording UPDATE query (existing `UpdateRecordingFull` in sqlc) + - Genre re-linking + - Release group re-linking + - FTS5 re-index (existing `InsertSearchIndex`) + - Transaction wrapper + +3. **Events** (`backend/events/events.go`) + - Add `TagsUpdated`, `TagEditFailed` constants + - Run codegen to update `frontend/src/events.ts` + +4. **Service wiring** (`backend/app.go`) + - Create and bind `tageditor.Service` + - Two-phase init (NewService + SetContext) + +5. **Frontend: single track edit** (`frontend/src/components/track-details/`) + - Wire `saveEdit()` to `tageditor.EditTrack()` + - Loading/error states + - `library-store` event handler for refresh + +6. **Frontend: batch edit** (new or extended component) + - Multi-select context menu action + - Batch edit dialog + - `tageditor.EditTracks()` call + +7. **Cover art editing** (builds on phases 1-5) + - File picker for image selection + - `tageditor.SetCoverArt()` implementation + - Cover art pipeline integration (save to disk, generate variants, update DB) + +## Sources + +- Codebase analysis: `backend/library/library.go` (scan pipeline, entity upsert pattern) +- Codebase analysis: `backend/database/search.go` (FTS5 contentless behavior) +- Codebase analysis: `backend/metadata/tags.go` (read-only tag extraction via dhowden/tag) +- Codebase analysis: `frontend/src/components/track-details/track-details.ts` (existing edit UI stub) +- `bogem/id3v2/v2`: https://pkg.go.dev/github.com/bogem/id3v2/v2 (v2.1.4, MIT, 359 stars, 57 importers) +- `go-flac/go-flac`: https://github.com/go-flac/go-flac (FLAC metadata manipulation) +- `go-flac/flacvorbis`: https://github.com/go-flac/flacvorbis (Vorbis comment read/write for FLAC) +- SQLite FTS5 contentless tables: https://www.sqlite.org/fts5.html#contentless_tables diff --git a/.planning/research/FEATURES.md b/.planning/research/FEATURES.md index 68b5062..db4da48 100644 --- a/.planning/research/FEATURES.md +++ b/.planning/research/FEATURES.md @@ -1,73 +1,300 @@ -# Features Research: Multi-Library Support +# Feature Landscape: Tag Editing -**Researched:** 2026-03-08 -**Confidence:** HIGH +**Domain:** Metadata tag editing in desktop music players +**Researched:** 2026-03-16 +**Confidence:** HIGH (based on analysis of MusicBee, foobar2000, Kid3, Mp3tag, Picard patterns + Hydrogenaudio tag standards + existing YellowJacket codebase) -## How Mature Players Handle Multiple Libraries +## How Desktop Music Players Implement Tag Editing -| Player | Model | Libraries Separate? | Cross-Library Playlists? | -|--------|-------|--------------------|-----------------------| -| foobar2000 | Multiple folders, one merged library | No — all folders merge | N/A (one library) | -| MusicBee | Multiple folders per library, separate library databases | Yes (separate DBs) | No | -| Plex | Separate typed libraries, multiple folders each | Yes (fully isolated) | No | -| Jellyfin | Virtual collections with multiple paths | Yes | No | -| Navidrome | Named libraries with user access control | Yes (with multi-select merge) | Yes | -| Roon | Watched folders, one unified library | No — all merge | N/A (one library) | +### Reference Players Analyzed -### Desktop Player Pattern (foobar2000, Roon) +| Player | Single Edit | Batch Edit | Cover Art Edit | Auto-Tag | Tag Format Handling | +|--------|------------|------------|---------------|----------|-------------------| +| foobar2000 | Properties dialog | Multi-select → Properties (shared fields) | Embed/remove from Properties | Via plugins | ID3v2, Vorbis, APEv2; configurable write format | +| MusicBee | Inline + dialog | Multi-select → Edit panel (keep/clear/set) | Drag-drop + file picker + paste | Built-in | ID3v2.3/2.4, Vorbis; auto-convert on write | +| Kid3 | Side panel + dialog | Multi-select → panel applies to all | File picker + paste + drag | MusicBrainz/Discogs | ID3v1/v2, Vorbis, APEv2; shows raw frames | +| Mp3tag | List view + panel | Inherent (panel always applies to selection) | Drag-drop + file picker + clipboard | Tag Sources | All formats; extended tag view | +| Picard | Panel per file/album | Album-level batch via MusicBrainz match | Automatic via MusicBrainz + manual | Core feature | All formats; submission to MusicBrainz | -All folders contribute to one unified library. No folder-level filtering in default UI. User never thinks about "which folder." +### Common Patterns Across All Players -### Server Pattern (Plex, Jellyfin, Navidrome) +**Single-track editing:** +- Dialog/panel with labeled fields, plain text inputs +- Title, artist, album shown prominently (larger/bolder) +- Cover art displayed alongside fields (150-250px) +- Numeric fields (year, track #, disc #) use number inputs or constrained text +- Genre usually free-text (not dropdown — genre lists are opinionated and incomplete) +- Non-editable properties shown separately (bitrate, sample rate, file path, file size) +- Save button writes to file → updates database +- Cancel discards all changes -Separate libraries with access control. More suited to multi-user server apps. +**Batch editing (the critical UX challenge):** +- Select multiple tracks → open editor +- Fields show current value if identical across selection, blank/placeholder if mixed +- A "keep" / "don't change" / "mixed" indicator distinguishes "empty because cleared" from "empty because mixed" +- User types a value → it applies to ALL selected tracks on save +- Fields left unchanged preserve each track's individual value +- Common pattern: three-state per field — "keep original" (default), "set to value", "clear" +- Track number is special: batch edit typically excludes it (each track needs unique number) OR offers auto-number (sequential from N) -### YellowJacket Fit +**Cover art editing:** +- Display current embedded art (or "no cover" placeholder) +- Replace from file: file picker (JPEG, PNG) +- Remove embedded art (less common, but available in Kid3/Mp3tag) +- Cover art in batch edit: applies same image to all selected tracks (common for fixing an album) +- No crop/resize UI — users prepare images externally +- Players typically accept any size but recommend 500-1000px square -Desktop player = **merged by default**, with optional filter. Follows foobar2000/Roon pattern but adds Navidrome-style library selector for power users. +**File safety:** +- Write-to-temp-then-rename (atomic write) is universal best practice +- Some players (foobar2000) create backups before writing +- All players update their internal database after successful file write (no rescan) -## Feature Classification +### Universal Editable Fields (from Hydrogenaudio Tag Mapping + player analysis) -### Table Stakes (Must Have) +**Basic (ID3v1-level, universal compatibility):** +- Title, Artist, Album, Year, Genre, Track Number, Comment -| Feature | Complexity | Dependencies | -|---------|-----------|-------------| -| Add multiple watched folders | Low | Config to DB migration | -| Unified merged view (default) | Medium | All browse views, search aggregate | -| Per-folder independent scan | Medium | Scan pipeline scoping | -| Remove folder without data loss | Low | Phantom tracks for playlists | -| Folder status indicators | Low | Scan event system | -| Graceful offline handling | Medium | Guard orphan cleanup | -| Existing playlists unaffected | Low | File-path references already work | +**Standard (ID3v2/Vorbis, widely supported):** +- Album Artist, Composer, Disc Number, Track Total, Disc Total, Lyrics -### Differentiators (Nice to Have) +**Extended (advanced users, format-dependent):** +- BPM, Initial Key, Mood, Label, Catalog Number, ISRC, MusicBrainz IDs -| Feature | Complexity | Dependencies | -|---------|-----------|-------------| -| Filter/narrow by source folder | Medium | UI filter chip, query-level filtering | -| Named libraries | Low | DB stores name + path | -| Per-folder scan concurrency | Low | Extend existing ScanConcurrency per library | -| Folder health dashboard | Medium | Aggregate scan metrics | +## Table Stakes -### Anti-Features (Do NOT Build) +Features users expect. Missing = product feels incomplete. -| Anti-Feature | Why Avoid | -|--------------|-----------| -| Separate databases per library | Breaks unified browse, doubles query logic | -| User/access control per library | Desktop app is single-user | -| Auto-merge/deduplicate across folders | Complex, error-prone, unexpected | -| Library-specific settings/themes | Over-engineering | +| Feature | Why Expected | Complexity | Dependencies | Notes | +|---------|-------------|------------|--------------|-------| +| Single track tag editing (title, artist, album, genre, year, track#, disc#, composer) | Every player with tag editing supports these 8 fields minimum | Medium | Tag writing library, DB update queries, FTS5 reindex | Existing `track-details` dialog has edit mode UI scaffolded (save is no-op TODO) | +| Write tags to MP3 (ID3v2) | MP3 is the most common format; must-have | High | Need tag writing library (dhowden/tag is read-only) | Format-specific: must write ID3v2.3 or ID3v2.4 frames | +| Write tags to FLAC (Vorbis Comments) | FLAC is the standard lossless format | High | Same writing library | Vorbis comments in FLAC metadata block | +| Write tags to OGG (Vorbis Comments) | Already supported for reading | Medium | Same writing library | Same Vorbis comment format as FLAC | +| Write-to-temp-then-rename | File corruption on crash/power loss = unacceptable data loss | Low | `os.Rename` after writing to temp file | Universal best practice; Go stdlib handles this well | +| Inline DB + FTS5 update after tag write | Users expect immediate UI update; forcing rescan is unacceptable | Medium | UPDATE queries for recordings, artist_credit, release_groups, genres; FTS5 search_index rebuild for affected rows | Must update the `track_metadata` VIEW's source tables | +| Batch editing shared fields across multiple selected tracks | Every tag editor supports this; multi-select already exists in track list | High | Batch editor UI component, backend batch write endpoint, progress tracking | The hard UX problem: mixed-value indicators, three-state fields | +| Save confirmation / error feedback | User must know if write succeeded or failed | Low | Event emission, toast/notification UI | Especially important for read-only files or permission errors | +| Cover art set/replace from image file | Fundamental tag editing feature; cover art is visually prominent | Medium | File picker (already have `FrontendUtil.OpenFileDialog`), image embedding in tag write, cover art cache update | Must update both embedded tag and cover art cache + thumbnails | -## Library Removal Patterns +## Differentiators -All players that support library removal: -1. Show confirmation dialog -2. Remove tracks from DB (or mark as missing) -3. Handle playlist references (delete, mark phantom, or leave as-is) -4. Don't delete files from disk +Features that set the product apart. Not expected, but valued. -**YellowJacket approach:** Phantom tracks (preserving metadata) for playlists. Queue tracks cascade-deleted (ephemeral). Orphan cleanup for shared entities via reference counting. +| Feature | Value Proposition | Complexity | Dependencies | Notes | +|---------|------------------|------------|--------------|-------| +| Album artist field editing | Distinguishes VA compilations; power users expect it | Low | One additional field in edit form; already extracted by `dhowden/tag` | Not in PROJECT.md active list but low-hanging fruit | +| Lyrics field editing | Multi-line text editing for embedded lyrics | Low | Textarea in dialog; lyrics field already in `recordings` schema… wait, it's in `TrackMetadata` struct but not shown in track-details UI | Would need multiline input; niche but straightforward | +| Comment field editing | Standard tag field, some users store notes | Low | Already extracted, just needs UI input | Very low effort to include | +| Auto-number tracks in batch edit | Select album tracks → auto-assign sequential track numbers | Low | Frontend logic to generate sequential numbers, apply in batch write | Huge time-saver when retagging an album | +| Dirty indicator / unsaved changes warning | Prevent accidental dialog close with unsaved edits | Low | Track `editValues` diff vs original values | MusicBee and foobar2000 both do this | +| Undo last tag write (restore from backup) | Safety net for mistakes; builds user trust | Medium | Write original tag values to a backup store before overwriting | Most players don't do this — would be a genuine differentiator | +| Cover art remove (strip embedded art) | Some users want to remove bloated embedded art | Low | Write tags without picture data | Available in Kid3/Mp3tag but not most players | +| Cover art paste from clipboard | Quick workflow: copy image from browser → paste into editor | Medium | Clipboard API in WebView, image data extraction | MusicBee supports this; convenient for web-sourced art | +| Progress indicator for batch operations | Visual feedback during multi-file writes (batch of 20+ tracks) | Low | Progress event emission, progress bar in UI | Important when writing to many files (can take seconds per file for FLAC) | +| Total Tracks / Total Discs fields | Part of standard tag spec; power users tag these | Low | Two additional number fields; already in `TrackMetadata` struct | Mp3tag and Kid3 expose these; foobar2000 uses "X/Y" format | -## Offline Handling +## Anti-Features -Universal pattern: Don't delete tracks when source goes offline. Mark as unavailable. Auto-recover on next scan when source returns. Never auto-delete on temporary unavailability. +Features to explicitly NOT build. + +| Anti-Feature | Why Avoid | What to Do Instead | +|--------------|-----------|-------------------| +| Inline editing in track list columns | Extremely complex (virtual scrolling + inline inputs + focus management + multi-select conflicts); fragile UX | Use the existing modal dialog approach — click to open editor. This is what foobar2000 does. | +| MusicBrainz auto-tagging / lookup | Massive scope expansion (API integration, fuzzy matching, network dependency); separate milestone material | Defer to future "MusicBrainz browser" milestone already in PROJECT.md | +| Genre dropdown with predefined list | Genre lists are subjective, never complete, frustrate users who use custom genres | Free-text input with optional suggestions from existing genres in DB (future enhancement) | +| Tag format conversion (ID3v1→v2, strip APEv2) | Edge case tool feature; desktop tagger territory (Mp3tag) | Write the "correct" format for each file type; don't expose format internals to users | +| Raw tag frame editing | Power-user-only feature; complex UI for marginal value | Edit semantic fields (title, artist, etc.); abstract away ID3 frames vs Vorbis comments | +| Custom/arbitrary tag field editing | Requires extensible UI, arbitrary field names, format-specific storage concerns | Support the standard fields; users with custom tags use Mp3tag | +| Filename renaming from tags | Common in dedicated taggers (Mp3tag, Kid3) but orthogonal to tag editing; adds file system mutation risk | Out of scope; would need separate file operations system | +| ReplayGain scanning/writing | Separate audio analysis feature, not tag editing | Future milestone if ever; requires DSP analysis | +| Drag-and-drop cover art from external apps | Complex browser/WebView drag interop; unreliable across platforms | File picker is the reliable universal approach | +| Multi-value field editing (multiple artists/genres as separate entries) | ID3v2 and Vorbis support multiple values per field, but the UI complexity is enormous | Store as single string; genre already uses `||` separator internally | + +## Feature Dependencies + +``` +Single Track Edit ──→ Tag Writing Library (MP3/FLAC/OGG) + ──→ DB Update Queries (recordings, artist_credit, release_groups, genres) + ──→ FTS5 Reindex (search_index) + ──→ Event Emission (UI refresh) + +Batch Edit ────────→ Single Track Edit (batch = N × single with shared values) + ────────→ Mixed-value UI (three-state field indicators) + ────────→ Multi-select (already exists in track-list) + +Cover Art Edit ───→ Tag Writing Library (picture frame embedding) + ───→ Cover Art Cache Update (saveCoverArt + thumbnail generation) + ───→ File Picker Dialog (already exists: FrontendUtil.OpenFileDialog) + +Write Safety ─────→ Temp file + os.Rename (no dependencies on existing code) + +DB Update ────────→ Existing schema: recordings, artist_credit, artists, + release_groups, release_group_recordings, genres, genre_recordings, + cover_art, audio_files + ────→ FTS5 search_index rebuild for affected rows + ────→ track_metadata VIEW reflects changes automatically (it's a VIEW) +``` + +### Critical Dependency Chain +``` +Tag Writing Library → Single Track Edit → Batch Edit + → Cover Art Edit +``` + +The tag writing library choice gates everything. Until a library can write ID3v2 and Vorbis comments, no editing features can ship. + +### Dependency on Existing Architecture + +| Existing Feature | How Tag Editing Uses It | +|-----------------|----------------------| +| `track-details` component | Already has edit mode scaffolded with input fields, edit/save/cancel buttons, and `editValues` state. Save handler is a TODO stub. | +| Multi-select in track-list | Entry point for batch editing — selected file paths already accessible via `selection.getSelectedKeysOrdered()` | +| Context menu system | "Edit Tags" menu item for single or multi-select (currently shows "Track Details" for single) | +| `FrontendUtil.OpenFileDialog` | File picker for cover art image selection | +| `Library.saveCoverArt` + thumbnail pipeline | Reusable for cover art embedding — same hash-based cache, same thumbnail generation | +| Event system | New events needed: `TagsWritten`, `TagWriteProgress`, `TagWriteError` | +| `backend/metadata/tags.go` | `TrackMetadata` struct defines all writable fields; `ExtractTags` used for reading | + +## Batch Editing UX Patterns (Deep Dive) + +The batch editor is the highest-complexity feature. Here's how mature players handle it: + +### Three-State Field Model + +For each editable field in batch mode: +1. **Keep** (default): Shows "[Mixed]" or "[Various]" if values differ, shows the common value if all tracks share it. On save, each track retains its original value. +2. **Set**: User has typed a new value. On save, all selected tracks get this value. +3. **Clear**: User explicitly cleared the field. On save, all selected tracks have this field emptied. + +**Implementation approach:** +```typescript +type FieldState = 'keep' | 'set' | 'clear'; + +interface BatchField { + state: FieldState; + value: string; // The new value (only meaningful when state === 'set') + commonValue: string; // Value shared across all tracks (empty if mixed) + isMixed: boolean; // Whether tracks have different values +} +``` + +### Backend Batch Write Contract + +```go +// TagEdits contains the fields to write. nil = don't change, empty string = clear. +type TagEdits struct { + Title *string + Artist *string + Album *string + Genre *string + Year *int + TrackNumber *int + DiscNumber *int + Composer *string + CoverArt *CoverArtEdit // nil = keep, non-nil = set/remove +} + +type CoverArtEdit struct { + ImageData []byte // empty = remove cover art + MIMEType string +} +``` + +Using pointer fields: `nil` = keep original, non-nil = set to this value (empty string/zero = clear). This is the standard Go pattern for optional updates and maps directly to the three-state UI model. + +### Batch Write Ordering + +1. Validate all edits before writing any files (fail fast) +2. Write files sequentially (not concurrently — avoids disk thrashing and simplifies error handling) +3. For each file: read → modify → write-to-temp → rename +4. After ALL files written successfully: batch-update DB + FTS5 +5. Emit success event with count +6. On error: stop, report which file failed, files already written are committed (no rollback — file writes are atomic individually) + +## Cover Art Editing Workflow + +### Set/Replace Cover Art (Table Stakes) + +1. User clicks "Change Cover" in edit dialog +2. File picker opens (filter: `*.jpg, *.jpeg, *.png`) +3. User selects image file +4. Preview shown in dialog (replacing current art) +5. On save: + a. Read image bytes from selected file + b. Embed in audio file tag (APIC frame for ID3v2, METADATA_BLOCK_PICTURE for FLAC/OGG) + c. Save to cover art cache (via existing `saveCoverArt` pipeline → hash, dedupe, thumbnails) + d. Update `cover_art` table if hash changed + e. Update UI with new cover art URLs + +### Batch Cover Art (Same Image to All Selected Tracks) + +Common use case: fixing an album where some tracks have wrong/missing cover art. +1. In batch editor, cover art section shows "[Mixed]" or common art +2. User selects new image → applies to ALL selected tracks on save +3. This is the same flow as single-track, just repeated N times + +### What NOT to Build for Cover Art + +- No crop/resize — users use external tools (GIMP, Preview, etc.) +- No web search — would require API integration (future MusicBrainz milestone could add this) +- No multiple picture types (front, back, booklet) — only front cover. ID3v2 supports picture types but the complexity isn't worth it for v1. + +## Field Mapping: Tag Format → Database Schema + +Understanding how edited fields map through the system: + +| Edit Field | Tag (ID3v2) | Tag (Vorbis) | DB Table | DB Column | Notes | +|-----------|------------|-------------|----------|-----------|-------| +| Title | TIT2 | TITLE | `recordings` | `name` | | +| Artist | TPE1 | ARTIST | `artist_credit` → `artists` | `text` / `name` | May need to create new artist_credit + artist rows | +| Album | TALB | ALBUM | `release_groups` | `name` | May need to create new release_group row | +| Album Artist | TPE2 | ALBUMARTIST | (not currently stored separately) | — | Would need schema addition or use existing artist credit | +| Genre | TCON | GENRE | `genres` + `genre_recordings` | `name` | Multiple genres: split on `;` or `,` | +| Year | TYER/TDRC | DATE | `recordings` | `year` | | +| Track # | TRCK | TRACKNUMBER | `recordings` | `track_number` | | +| Disc # | TPOS | DISCNUMBER | `recordings` | `disc_number` | | +| Composer | TCOM | COMPOSER | `recordings` | `composer` | | +| Cover Art | APIC | METADATA_BLOCK_PICTURE | `cover_art` | `file_path` | Binary data; separate storage | +| Comment | COMM | COMMENT | `recordings` | `comment` | | +| Lyrics | USLT | LYRICS | `recordings` | `lyrics` | | + +### Schema Update Complexity + +Simple fields (title, year, track#, disc#, composer, comment, lyrics) → UPDATE `recordings` directly. + +Relational fields (artist, album, genre) → must handle entity lifecycle: +- **Artist change:** Look up or create new `artists` + `artist_credit` rows, update `recordings.artist_credit_id` +- **Album change:** Look up or create new `release_groups` row, update `release_group_recordings` link +- **Genre change:** Parse genre string, look up or create `genres` rows, update `genre_recordings` links + +This entity lookup logic already exists in `library.go`'s `processMetadata` / `saveAudioFile` pipeline — it should be extracted and reused. + +## MVP Recommendation + +**Prioritize (Phase 1 — Tag Editing Core):** +1. Tag writing library integration (MP3 + FLAC + OGG) +2. Single track editing (the 8 active fields from PROJECT.md) +3. Write-to-temp-then-rename safety +4. DB + FTS5 inline update +5. Cover art set/replace from file + +**Prioritize (Phase 2 — Batch Editing):** +6. Batch editing with three-state field model +7. Progress feedback for batch operations +8. Error handling and partial-success reporting + +**Defer:** +- Album artist editing (schema question, low priority) +- Lyrics/comment editing (easy to add later, niche) +- Auto-numbering tracks (convenience, not core) +- Undo/backup system (nice-to-have, not table stakes) +- Cover art paste from clipboard (WebView clipboard API complexity) + +## Sources + +- Hydrogenaudio Knowledgebase: Tag Mapping (https://wiki.hydrogenaud.io/index.php/Tag_Mapping) — HIGH confidence, authoritative tag format reference +- Hydrogenaudio Knowledgebase: foobar2000 Encouraged Tag Standards (https://wiki.hydrogenaud.io/index.php/Foobar2000:Encouraged_Tag_Standards) — HIGH confidence +- Hydrogenaudio Knowledgebase: Tag (metadata) (https://wiki.hydrogenaud.io/index.php/Tag) — HIGH confidence, basic/advanced/personalized field categorization +- YellowJacket codebase analysis: `track-details.ts`, `tags.go`, `coverart.go`, `library.go`, database schemas — PRIMARY source for dependency analysis +- MusicBee, foobar2000, Kid3, Mp3tag, Picard — feature set analysis from training data (MEDIUM confidence on specific UI details) diff --git a/.planning/research/PITFALLS.md b/.planning/research/PITFALLS.md index 597db06..3f8863f 100644 --- a/.planning/research/PITFALLS.md +++ b/.planning/research/PITFALLS.md @@ -1,100 +1,382 @@ -# Pitfalls Research: Multi-Library Support +# Domain Pitfalls: Tag Editing -**Researched:** 2026-03-08 -**Confidence:** HIGH +**Domain:** Adding tag editing to an existing music player with normalized DB +**Researched:** 2026-03-16 +**Confidence:** HIGH (based on codebase analysis + format specifications + SQLite FTS5 docs) ## Critical Pitfalls -### P1: SQLite ALTER TABLE ADD COLUMN with NOT NULL Requires DEFAULT +Mistakes that cause data loss, file corruption, or require architectural rework. -SQLite requires NOT NULL columns added via ALTER TABLE to have a default. Create `libraries` table and insert default library BEFORE adding `library_id` to `audio_files`. Use `DEFAULT {id}` where id is the auto-created library's ID. +### P1: FLAC Tag Writes Require Full File Rewrite -**Phase:** Schema & Migration +**What goes wrong:** FLAC stores Vorbis Comments in a METADATA_BLOCK after the STREAMINFO block. Unlike MP3 (which has padding in ID3v2 headers), FLAC metadata blocks are tightly packed with no padding by default. Changing a tag that increases the metadata size requires rewriting the entire file — moving every audio frame forward. A crash or power loss during this rewrite corrupts the file irrecoverably. -### P2: Table Rebuild for CASCADE to SET NULL Must Audit ALL Tables +**Why it happens:** FLAC spec doesn't mandate padding blocks. Most FLAC files in the wild have zero padding. Even if padding exists, adding cover art (which can be 100KB+) almost always exceeds it. -Both `playlist_tracks` AND `queue_tracks` have `ON DELETE CASCADE` on `audio_file_id`. Decision: playlist_tracks -> SET NULL (phantom support), queue_tracks -> keep CASCADE (queue is ephemeral). Must explicitly document this choice. +**Consequences:** Corrupted FLAC files that won't play. Audio data intact on disk but offset table is wrong, so decoders can't find frames. -**Phase:** Schema & Migration +**Prevention:** +1. **Write-to-temp-then-rename (mandatory for all formats).** Write modified file to a temp file in the same directory (same filesystem), then `os.Rename()` atomically. This is already listed in PROJECT.md as a target feature. +2. For FLAC specifically: read entire file → write new metadata blocks → copy audio frames → rename. There is no in-place shortcut. +3. Verify the written file can be opened and has correct duration before replacing the original. +4. Consider adding a PADDING metadata block after writing (e.g. 8KB) so small future edits can be done in-place. This is what tools like `metaflac` do. -### P3: FTS5 Contentless Table Cannot Delete Individual Rows +**Detection:** File size changes unexpectedly; beep decoder fails to open the file after write; duration changes after write (offset corruption). -After removing a library with 10K tracks, 10K stale FTS5 entries remain. Current JOIN filtering handles this, but FTS5 scoring is affected. Consider migrating to `contentless_delete=1` (SQLite 3.43.0+). Alternative: full rebuild after library removal. +**Phase:** File write layer (earliest phase) -**Phase:** Schema & Migration +--- -### P4: Orphan Cleanup Through Entity Graph Is Complex +### P2: Currently-Playing File Cannot Be Written On Windows (And Shouldn't On Any Platform) -Reference-counting deletes must handle shared entities. Two libraries with same artist — removing one must not delete the artist if the other still references it. Use `NOT IN (SELECT ...)` or `LEFT JOIN ... WHERE ... IS NULL` pattern. Single transaction required. +**What goes wrong:** The player holds an `os.File` handle on the currently playing track (`p.currentFile` in `player.go:461`). On Windows, the OS enforces mandatory file locking — `os.Rename()` will fail with "The process cannot access the file because it is being used by another process." On Linux, the rename succeeds but the player continues reading the old inode (now unlinked), which works until something closes and reopens the path. -**Phase:** Backend API / Library CRUD +**Why it happens:** The player opens files with `os.Open()` and holds them open for the duration of playback (streaming audio data). The beep library reads from this file handle continuously. -### P5: Existing User Migration Must Be Seamless +**Consequences:** On Windows: tag write fails silently or with confusing error. On Linux: tag write succeeds but the player sees stale data, and if the user seeks, the streamer may read garbage from the new file at old offsets. -First launch after update: migration 6 reads TOML DirectoryPath, creates library row, backfills audio_files.library_id. Test on real user database snapshot, not just fresh DB. +**Prevention:** +1. **Check if the target file is currently playing before writing.** Compare `player.currentTrackPath` against the edit target. +2. If the file IS playing: stop playback, close the file handle, perform the write, then reload and seek to the previous position. This creates a brief audio glitch but is the only safe approach. +3. For batch edits that include the current track: edit all other files first, handle the playing file last with the stop-write-reload dance. +4. Alternative (simpler): refuse to edit the currently playing file and show a user-facing message. Less ideal UX but avoids complexity. -**Phase:** Schema & Migration +**Detection:** `os.Rename()` returns error on Windows. On Linux, no error but playback becomes corrupted after seek. + +**Phase:** File write layer + player integration + +--- + +### P3: FTS5 Contentless Table Cannot UPDATE or DELETE Individual Rows + +**What goes wrong:** The current `search_index` is a contentless FTS5 table (`content=''`). The existing `DeleteSearchIndex()` method is literally a no-op (see `search.go:120-127`). After editing a track's title from "Love Song" to "Heart Song", searching for "Love Song" still returns the track because the old FTS5 entry cannot be removed. The stale entry points to a valid rowid, and the JOIN against `track_metadata` will return the row (now with different data), so the user sees a search result that doesn't match their query. + +**Why it happens:** Contentless FTS5 (`content=''`) stores only the index, not the original text. Without the original text, FTS5 can't compute what tokens to remove from the index. The current design relies on full rebuilds during rescan, which is fine for the read-only case but breaks for incremental edits. + +**Consequences:** Search returns false positives after tag edits. The more edits the user makes, the worse search quality gets — until the next full rescan rebuilds the index. + +**Prevention — Two Options:** + +**Option A: Migrate to `contentless_delete=1` (Recommended)** +SQLite 3.43.0+ supports `contentless_delete=1` which enables DELETE and INSERT OR REPLACE. This requires a schema migration (drop + recreate the FTS5 table). The `modernc.org/sqlite` driver bundles SQLite 3.45+, so this is available. After migration, tag edit can do: DELETE the old row, INSERT the new row. This is the `DELETE + INSERT` pattern already noted in the milestone context. + +**Option B: Rebuild the entire index after each edit session** +Call `RebuildSearchIndex()` after completing all tag writes. This is expensive (reads all tracks) but correct. Could be batched — rebuild once after a batch edit, not per-track. + +**Recommendation:** Option A. The migration is straightforward and makes individual updates O(1) instead of O(n). The existing `RebuildSearchIndex()` becomes the migration step. + +**Detection:** Search for old tag values — if they return results with the new values, the index is stale. + +**Phase:** Schema migration (do first, before any tag write code) + +--- + +### P4: Shared Entity Fan-Out — Editing Artist on One Track Affects Zero or Fifty Others + +**What goes wrong:** The normalized schema shares entities across tracks. An `artist_credit` row with text "The Beatles" may be referenced by 200 recordings via `recordings.artist_credit_id`. If the user edits the artist field on one track from "The Beatles" to "Beatles, The", the system must decide: (a) update the shared `artist_credit` row (changing all 200 tracks), (b) create a new `artist_credit` and repoint only this track's recording, or (c) something else. + +**Why it happens:** The MusicBrainz-inspired schema (`artists` → `artist_credit` → `recordings`) is designed for read-heavy workloads where entities are shared. Tag editing breaks this assumption by making per-track changes that may or may not be intended as global changes. + +**Consequences:** +- If you update the shared row: user edits one track, 199 other tracks silently change. Terrifying. +- If you create new rows: orphaned entities accumulate (old `artist_credit` row with only 199 refs, then 198, etc.). The artist browse view shows "The Beatles" AND "Beatles, The" as separate entries. +- If you try to be smart about it: complex merge/split logic that's hard to get right. + +**Prevention:** +1. **Tag editing always creates new entity rows for the edited track.** Create a new `recording`, new `artist_credit` (if changed), new `release_group_recordings` link, new `genre_recordings` links. Point the `audio_file.recording_id` at the new recording. This is the safest approach and matches what the scan pipeline already does (it always creates new recordings). +2. **Orphan cleanup after edit.** After repointing the audio_file, check if the old recording is still referenced by any audio_file. If not, delete it (and cascade to its genre links, release_group links). Same for artist_credit, artists, genres, release_groups. +3. **Never mutate shared entities in-place** during single-track or batch-within-same-album editing. The only exception is intentional "rename this artist across all tracks" which should be a separate, explicit feature (not part of v1.2). + +**Detection:** After editing one track's artist, check if other tracks in the same album now show the wrong artist. + +**Phase:** Database update layer (core architecture decision — must be settled before writing any DB update code) + +--- + +### P5: Race Condition — Scan Runs While Tags Are Being Written + +**What goes wrong:** User starts editing tags. While the edit is in progress (writing files, updating DB), a library scan starts (either from the scan queue, soft scan on launch, or user-initiated). The scan reads the file's tags (which may be half-written or already-written-but-DB-not-yet-updated), creates new entity rows, and overwrites the DB state that the tag editor just carefully set up. + +**Why it happens:** The scan pipeline (`scanInternal`) and tag editing are independent operations. The scan loads existing files from DB, walks the filesystem, extracts metadata, and writes to DB. If a file's on-disk tags differ from the DB (because the edit just wrote new tags), the scan treats it as needing an update and overwrites the recording. + +**Consequences:** Tag edits silently reverted. Or worse: the scan creates duplicate recordings (one from the edit, one from the scan) because the scan's entity cache doesn't know about the edit's newly-created entities. + +**Prevention:** +1. **Mutual exclusion between tag editing and scanning.** While tag writes are in progress, block scan start (or vice versa). The existing `l.mu` mutex protects scan state; extend it to cover "edit in progress" state. +2. **Simpler: Use the existing scan queue coordinator.** Tag edits happen on the main goroutine (via Wails binding). Scans run in background goroutines. Since SQLite has `SetMaxOpenConns(1)`, DB writes are already serialized. The risk is the scan re-reading the file AFTER the tag write but BEFORE the DB update. Solution: perform the file write and DB update atomically (in the same critical section), and have the scan skip files that were recently edited (timestamp check or "edited" flag). +3. **Best approach: Pause/cancel active scan during tag edit, resume after.** The existing `PauseScan()`/`ResumeScan()` mechanism can be leveraged. Pause the scan, do the edit (file write + DB update), resume the scan. + +**Detection:** Edit a tag, immediately trigger a scan, check if the edit survives. + +**Phase:** Tag write integration with scan pipeline + +--- + +### P6: Temp File Rename Fails Across Filesystem Boundaries + +**What goes wrong:** `os.Rename()` is atomic only when source and dest are on the same filesystem. If the temp file is created in `/tmp` (default `os.CreateTemp` behavior) but the music file is on `/mnt/music`, the rename becomes a copy+delete — no longer atomic, and if interrupted, you lose the file. + +**Why it happens:** Many developers use `os.CreateTemp("", ...)` which defaults to the system temp directory, which is often a different filesystem/partition from where music files live. + +**Consequences:** Non-atomic write. Power loss during copy = corrupted or missing file. + +**Prevention:** +1. **Create the temp file in the same directory as the target file.** Use `os.CreateTemp(filepath.Dir(targetPath), ".yj-edit-*")` to ensure same-filesystem rename. +2. Clean up temp files on startup (find files matching `.yj-edit-*` pattern in library directories — these are orphaned from crashed edits). +3. Use the temp file pattern: `/.yj-edit-` → write → `os.Rename()` → done. If rename fails, the temp file is deleted. The original is untouched. + +**Detection:** Check if `os.Rename()` returns `EXDEV` (cross-device link) error. + +**Phase:** File write layer (earliest phase) ## Moderate Pitfalls -### P6: Frontend Memory Pressure with Multiple Large Libraries +Mistakes that cause bugs, degraded UX, or significant rework. -`libraryStore.eagerFetch()` loads ALL data. 150K tracks x ~500 bytes = 75MB. Use backend filtering (pass library_id to queries). When "All Libraries" is selected, this is unavoidable for now — pagination is a future optimization. +### P7: ID3v2 Encoding Mismatch — UTF-8 Written Where Latin-1 Expected -**Phase:** Frontend +**What goes wrong:** ID3v2.3 (the most common version) defaults to ISO-8859-1 (Latin-1) encoding for text frames. If the tag writing library writes UTF-8 text into a Latin-1 frame without setting the encoding byte to UTF-8/UTF-16, players that strictly follow the spec will display garbled text (mojibake). Conversely, some players write Latin-1 tags that `dhowden/tag` reads as UTF-8, causing garbled reads. -### P7: Scan Coordination — No Concurrent Scans +**Why it happens:** ID3v2.3 only officially supports ISO-8859-1 and UTF-16. UTF-8 support was added in ID3v2.4. Many real-world files are ID3v2.3 with UTF-8 text (spec violation that most players tolerate). When writing tags, the library must match the encoding scheme to the ID3v2 version. -Single `scanActive` bool, single entity cache, single writer SQLite. Enforce one-scan-at-a-time globally with scan coordinator. Track which library is scanning for UI display. +**Consequences:** Non-ASCII characters (accents, CJK, Cyrillic) display as garbage in other players after editing with YellowJacket. -**Phase:** Backend Scan Pipeline +**Prevention:** +1. **Use `bogem/id3v2` (aka `n10v/id3v2`) for MP3 tag writing.** This library handles encoding correctly — it auto-selects UTF-8 for v2.4 and UTF-16 for v2.3, or allows explicit control. +2. When writing ID3v2.3 tags with non-ASCII content, use UTF-16 encoding (the only Unicode encoding ID3v2.3 supports). +3. Consider upgrading all written tags to ID3v2.4 (which supports UTF-8 natively). This is what most modern taggers do. +4. **Read the existing tag version and preserve it** unless the user explicitly requests an upgrade. -### P8: Phantom Track Resolution with Multiple Library Roots +**Detection:** Edit a track with non-ASCII characters, open in another player (VLC, foobar2000), check for garbled text. -`LibraryDirProvider` returns single string. M3U8 path resolution checks one root. With multi-library, try all library roots for phantom resolution. Store phantom_file_path as absolute path to avoid ambiguity. +**Phase:** Tag writing layer -**Phase:** Frontend / Playlist Integration +--- -### P9: Queue and Now-Playing During Library Removal +### P8: Cover Art Embedding Size and Format Incompatibilities -If currently playing track belongs to removed library: stop playback, advance to next non-removed track. Check queue and player state before proceeding with removal. +**What goes wrong:** +- **JPEG vs PNG:** Both ID3v2 and FLAC Vorbis Comments support JPEG and PNG cover art. However, some older players only handle JPEG. If the user selects a PNG, it should work but may not display in all contexts. +- **Image size:** Users may select a 10MB PNG file as cover art. Embedding this in every track of a 50-track album creates 500MB of overhead. The file write becomes extremely slow, and the FLAC rewrite (P1) is even worse because the entire file must be rewritten. +- **FLAC cover art is stored as a PICTURE metadata block** with specific structure (picture type, MIME type, description, width, height, color depth, data). Getting any of these fields wrong causes players to not display the art. +- **Vorbis Comments in OGG:** Cover art in OGG Vorbis files is stored as a base64-encoded METADATA_BLOCK_PICTURE in a Vorbis Comment field. This is a different mechanism than FLAC's native PICTURE block, despite both using "Vorbis Comments." -**Phase:** Backend API / Library CRUD +**Consequences:** Cover art doesn't display in other players. Enormous file size increase. Slow writes. -### P10: Cross-Library Entity Deduplication +**Prevention:** +1. **Resize cover art before embedding.** Cap at 800x800 or 1000x1000 pixels. Convert to JPEG (quality 90) for embedding — better compression than PNG for photos. +2. **Validate image before embedding.** Decode it, check dimensions, re-encode if needed. Use `image/jpeg` and `image/png` standard library packages (already in use for thumbnail generation via `golang.org/x/image`). +3. **For FLAC:** Populate ALL required PICTURE block fields (picture type=3 "front cover", MIME type, width, height, bit depth, data). +4. **For OGG:** Base64-encode the FLAC PICTURE block structure into a `METADATA_BLOCK_PICTURE` Vorbis Comment field. +5. **Show file size impact preview** in the UI before confirming cover art change on batch operations. -Same artist in two libraries -> one `artists` row (UNIQUE constraint handles this). Removing one library's audio_files must NOT delete shared artist. Reference-counting cleanup handles this correctly. +**Detection:** Embed cover art, open in another player, check if art displays. Check file size increase. -**Phase:** Backend API / Library CRUD +**Phase:** Cover art write layer + +--- + +### P9: Batch Edit Creates Hundreds of Orphaned Entity Rows + +**What goes wrong:** User selects 50 tracks from an album and changes the artist name. Following P4's approach (create new entities, repoint audio_file), this creates 50 new recordings, 1 new artist_credit, and 50 new release_group_recordings links. The old recording rows (and their genre links) are now orphaned — nothing references them. Without cleanup, the artists/albums/genres views show ghost entries. + +**Why it happens:** The "always create new" approach from P4 is correct for safety but generates garbage. The existing scan pipeline never updates entities — it only creates them. There's no existing orphan cleanup for recordings/artists/genres (only for audio_files during scan). + +**Consequences:** Ghost artists, albums, and genres appear in browse views. Database grows over time. Genre list fills with duplicates if genre spelling varies slightly across edits. + +**Prevention:** +1. **Run entity orphan cleanup after every edit (or batch edit).** In a single transaction: + - Delete recordings not referenced by any audio_file + - Delete release_group_recordings referencing deleted recordings + - Delete recording_genres referencing deleted recordings + - Delete artist_credits not referenced by any recording or release_group + - Delete artists not referenced by any artist_credit_artist + - Delete genres not referenced by any recording_genres + - Delete release_groups not referenced by any release_group_recordings + - Delete cover_art not referenced by any release_groups +2. **Use `LEFT JOIN ... WHERE ... IS NULL` pattern** (same approach documented in P4 of the multi-library PITFALLS). +3. **Batch the cleanup** — run once per edit session, not per-track. + +**Detection:** After batch edit, check that the old artist/album/genre no longer appears in browse views (unless other tracks still reference them). + +**Phase:** Database update layer (immediately after P4's approach is implemented) + +--- + +### P10: Genre Storage Mismatch — Comma-Separated String vs Multi-Value + +**What goes wrong:** The `recordings.genre` column stores genre as a free-text string. The existing `metadata.ParseGenres()` splits on `,` and `;` and normalizes to title case. But the `recording_genres` M:N junction table stores individual genre links. These two representations can diverge: the string says "Rock, Pop" but the junction table has links to "Rock" and "Pop" as separate genre entities. After a tag edit, if only the string is updated (or only the junction table), they fall out of sync. + +**Why it happens:** Dual representation — the raw string in `recordings.genre` and the normalized M:N links in `recording_genres`. The scan pipeline populates both, but an edit might only update one. + +**Consequences:** Genre filtering (which uses `recording_genres`) shows different results than the genre string displayed in the track list (which comes from `recordings.genre` via `track_metadata` VIEW). + +**Prevention:** +1. **Always update both representations in the same transaction.** When the user sets genre to "Rock, Pop": + - Update `recordings.genre` = "Rock, Pop" + - Delete all `recording_genres` rows for this recording + - Insert new `recording_genres` rows for "Rock" and "Pop" (via `ParseGenres()`) +2. **Use `ParseGenres()` consistently** for both display and storage. +3. **When writing to the audio file**, join the individual genre names with the format's conventional separator (`;` for Vorbis Comments multi-value, `,` for ID3v2 TCON frame). + +**Detection:** Edit genre, verify both the displayed genre string and the genre filter show consistent results. + +**Phase:** Database update layer + +--- + +### P11: Database Update After File Write — Partial Failure Leaves Inconsistency + +**What goes wrong:** The tag edit flow is: (1) write new tags to temp file, (2) rename temp to original, (3) update DB entities, (4) update FTS5 index. If step 2 succeeds but step 3 fails (e.g., SQLite busy, constraint violation), the file on disk has new tags but the DB shows old values. The next scan will "fix" this by re-reading the file, but until then the UI shows stale data. + +**Why it happens:** File writes and DB writes can't be in the same transaction (they're different systems). The rename is the point of no return for the file. + +**Consequences:** UI shows old metadata for edited tracks. Search returns old values. User thinks the edit failed and tries again (potentially fine since the file is already correct). + +**Prevention:** +1. **DB update first approach:** Update the DB entities BEFORE writing the file. If DB update fails, don't write the file — clean rollback. If DB update succeeds but file write fails, revert the DB change. This makes the DB the "leader" and the file the "follower." +2. **Alternative: Accept eventual consistency.** Write file, update DB, if DB fails log a warning and mark the file for re-scan. The scan pipeline already handles files-on-disk-differ-from-DB. +3. **For batch edits:** Use a two-phase approach — first update all DBs in a transaction, then write all files. If any file write fails, the DB is already correct for the others. Report per-file errors to the user. +4. **Recommendation:** Option 1 (DB first) is simpler and more correct. The file write is the expensive/risky step; the DB update is fast and transactional. + +**Detection:** Kill the app mid-edit (during file write), restart, verify DB and file are consistent. + +**Phase:** Tag write integration layer + +--- + +### P12: `dhowden/tag` Is Read-Only — Need Separate Write Libraries Per Format + +**What goes wrong:** The existing `github.com/dhowden/tag` library is read-only. It extracts tags but cannot write them. Developers may assume the existing dependency can handle writes, waste time trying, then discover late that a separate library is needed. + +**Why it happens:** `dhowden/tag` explicitly only supports reading. Its API has `ReadFrom()` but no `WriteTo()`. + +**Consequences:** Need to add 1-2 new dependencies for tag writing, each with different APIs and behaviors per format. + +**Prevention:** +1. **MP3 (ID3v2):** Use `github.com/bogem/id3v2/v2` (also available as `github.com/n10v/id3v2/v2`). 359 stars, actively maintained, supports read+write for ID3v2.3 and v2.4, handles encoding correctly, supports picture frames. Pure Go. +2. **FLAC:** Use `github.com/go-flac/flactag` or handle FLAC metadata blocks manually. FLAC's metadata format is simpler than ID3v2 (well-defined block structure). May need to write a thin wrapper that reads STREAMINFO + other blocks, modifies VORBIS_COMMENT block, and rewrites. +3. **OGG Vorbis:** Use `github.com/go-flac/go-ogg` or a Vorbis Comment library. OGG wraps Vorbis Comments in OGG pages, which adds framing complexity. +4. **WAV:** WAV tag support is minimal in practice. Defer WAV tag writing (not in v1.2 scope per PROJECT.md which lists MP3, FLAC, OGG only). +5. **Keep `dhowden/tag` for reading.** Don't replace it — use it alongside the write libraries. + +**Phase:** Stack decision (before implementation begins) ## Minor Pitfalls -### P11: Config Migration — TOML to DB Split Creates Two Sources of Truth +Mistakes that cause minor issues, confusion, or suboptimal UX. -Move ALL library-related config to DB. TOML only for app-level settings (theme, shortcuts, window). TOML `[Library]` section is migration source only. +### P13: Cover Art Cache Invalidation After Embedded Art Change -**Phase:** Schema & Migration +**What goes wrong:** Cover art is cached by content hash in `~/.local/share/yellowjacket/covers/`. If the user replaces embedded cover art, the old cached thumbnails (sm/md/lg) still exist and may be served from cache. The cover art hash changes (new image = new hash), so a new cache entry is created, but the `release_groups.cover_art_id` must be updated to point to the new `cover_art` record. -### P12: Library Filter State Interacting with Everything +**Why it happens:** The cover art system is designed for initial extraction during scan. It doesn't expect art to change after initial import. -Single filter state in libraryStore. All data-fetching functions accept the filter. Trigger invalidate+refetch on filter change. +**Prevention:** +1. After writing new cover art to the file, extract it back, compute the new hash, create the new `cover_art` record, update `release_groups.cover_art_id`, generate new thumbnails. +2. Delete old `cover_art` record and files only if no other release_group references them (same orphan cleanup as P9). +3. Emit an event so the frontend refreshes cover art display (invalidate any cached cover art URLs). -**Phase:** Frontend +**Phase:** Cover art write layer -### P13: Scan-While-Remove Race Condition +--- -Before removing a library, cancel any active scan on it and wait for completion. Serialize scan and remove operations. +### P14: Undo/Redo Expectations — Users Expect to Revert Tag Edits -**Phase:** Backend API +**What goes wrong:** User changes artist name, saves, realizes it was wrong, expects Ctrl+Z to work. But tag editing writes to the actual audio file — there's no undo buffer. -### P14: Cover Art Files Not Library-Scoped +**Why it happens:** File writes are destructive. The temp-file-rename approach ensures atomicity but not reversibility. -Cover art stored by content hash (shared). Removing a library: only delete cover_art DB rows that are truly orphaned (no remaining release_groups reference them). Then delete corresponding files. +**Prevention:** +1. **For v1.2: Don't implement undo.** It's complex (would need to store original tag values per-edit) and users of tag editors generally don't expect undo. +2. **Show a confirmation dialog before writing**, especially for batch edits. "You are about to modify 47 files. This cannot be undone." +3. **Log what changed.** Write structured log entries like `"tag edit: file=/path/to/song.mp3, field=artist, old=Beatles, new=The Beatles"`. This gives users a recovery path (manual). +4. **Future milestone consideration:** Backup original files before edit (copy to `.yj-backup/` directory). Add a "restore original" option. -**Phase:** Backend API +**Phase:** UX design -### P15: Testing Gaps +--- -Create "two-library fixture" test helper. Test: add two libraries with overlapping artists -> remove one -> verify other is intact. Test migration on pre-multi-library DB snapshot. +### P15: Track Number and Disc Number Edge Cases -**Phase:** All phases (accompanying tests) +**What goes wrong:** Track number is stored as `sql.NullInt64` in the DB and as `int` in `TrackMetadata`. User enters "1/12" in the track number field (common display format). If parsed as a raw int, this fails. If split on `/`, `TotalTracks` must also be stored. The existing `toNullInt64()` treats 0 as NULL, so track 0 is impossible to store (rare but exists in some compilations). + +**Consequences:** Track numbers display incorrectly or can't be set to certain values. + +**Prevention:** +1. Parse "N/M" format: split on `/`, store track number and total separately. +2. Validate inputs: track number must be positive integer (or blank for null). +3. Consider whether `toNullInt64()` treating 0 as NULL is correct for the edit case. For display it's fine, but for editing, the user might explicitly set track number to 0. Probably not worth changing for v1.2. + +**Phase:** Frontend input validation + backend write layer + +--- + +### P16: Multiple Audio Files Sharing the Same Recording (1:1 Assumption) + +**What goes wrong:** The scan pipeline creates a new `recordings` row for every audio file (see `processMetadata()` at `library.go:1178`). This means the relationship is effectively 1:1 (each audio_file has its own recording). But the schema allows N:1 (multiple audio_files can share a recording_id). If a future change or manual DB edit creates shared recordings, editing one track's metadata would affect the other track sharing that recording. + +**Why it happens:** The schema was designed for MusicBrainz-style data where multiple releases of the same recording share a recording ID. The scan pipeline doesn't implement this sharing, but the schema allows it. + +**Prevention:** +1. **Before editing a recording, check how many audio_files reference it.** If more than one, create a new recording for this audio_file (fork the entity). +2. This is already handled by P4's "always create new" approach, but worth calling out as a specific guard. + +**Phase:** Database update layer + +--- + +### P17: Frontend Store Refresh After Tag Edit + +**What goes wrong:** After a tag edit updates the DB, the frontend `libraryStore` still holds the old cached data (tracks, albums, artists, genres). Without a refresh, the UI shows stale values until the user navigates away and back, or triggers a full reload. + +**Why it happens:** The `libraryStore.eagerFetch()` loads all data at startup. There's no mechanism for partial updates — the store either shows cached data or refetches everything. + +**Prevention:** +1. **Emit a `TagsUpdated` event** from the backend after successful tag edit, with the list of affected file paths. +2. The frontend store listens for this event and either: + - (a) Refetches the full data (simple but expensive for large libraries), or + - (b) Patches the affected rows in-place (more complex but instant) +3. **Recommendation for v1.2:** Option (a) — full refetch. The existing `eagerFetch()` path is proven. Optimize to partial updates in a future milestone if performance is an issue. +4. Also update: search results (refetch if search is active), queue track metadata (emit `QueueTracksModified`), now-playing display (emit `TrackChanged` if the edited track is playing). + +**Phase:** Frontend integration (last phase) + +## Phase-Specific Warnings + +| Phase Topic | Likely Pitfall | Mitigation | +|-------------|---------------|------------| +| Schema migration | P3: FTS5 contentless can't delete | Migrate to `contentless_delete=1` first | +| File write layer | P1: FLAC full rewrite, P6: temp file same dir | Write-to-temp-then-rename in same directory | +| File write layer | P2: Currently playing file | Check player state before write, stop if needed | +| Tag library selection | P12: dhowden/tag is read-only | Use bogem/id3v2 for MP3, format-specific libs for FLAC/OGG | +| DB update design | P4: Shared entities, P9: Orphan cleanup | Always create new entities, clean up orphans per-edit | +| DB update design | P10: Genre dual representation | Update both recordings.genre and recording_genres atomically | +| Write + DB integration | P5: Scan race condition | Pause scan during edit, or mutual exclusion | +| Write + DB integration | P11: Partial failure | DB update first, then file write | +| Cover art writes | P8: Size/format compat, P13: Cache invalidation | Resize before embed, invalidate cache after write | +| Encoding | P7: ID3v2 Latin-1 vs UTF-8 | Use UTF-16 for v2.3, UTF-8 for v2.4 | +| Frontend | P17: Stale cache after edit | Emit event, full refetch | +| UX design | P14: No undo for file writes | Confirmation dialog, structured logging | + +## Ordering Implications + +The pitfalls strongly suggest this phase ordering: + +1. **FTS5 migration first** (P3) — enables all subsequent DB updates to be clean +2. **File write layer** (P1, P2, P6) — the atomic write-to-temp-rename mechanism, independent of DB +3. **Tag library integration** (P7, P8, P12) — per-format write support using new dependencies +4. **DB update design** (P4, P9, P10, P11, P16) — entity creation, orphan cleanup, genre sync +5. **Scan pipeline integration** (P5) — mutual exclusion between edit and scan +6. **Frontend** (P14, P15, P17) — UI, events, cache refresh + +## Sources + +- SQLite FTS5 documentation: contentless tables section (sqlite.org/fts5.html#contentless_tables) — HIGH confidence +- SQLite FTS5 contentless_delete: sqlite.org/fts5.html#contentless_delete_tables — HIGH confidence +- YellowJacket codebase analysis: search.go, library.go, player.go, schema files — HIGH confidence +- FLAC format spec: metadata block structure, PICTURE block format — HIGH confidence (well-established spec) +- ID3v2.3/2.4 spec: encoding requirements for text frames — HIGH confidence +- `bogem/id3v2` GitHub (n10v/id3v2): read+write ID3v2 library, 359 stars — MEDIUM confidence (verified repo exists and has write support) +- `dhowden/tag` API: read-only confirmed from codebase usage — HIGH confidence diff --git a/.planning/research/STACK.md b/.planning/research/STACK.md index 3c55708..2949ec4 100644 --- a/.planning/research/STACK.md +++ b/.planning/research/STACK.md @@ -1,62 +1,203 @@ -# Stack Research: Multi-Library Support +# Technology Stack: Tag Editing -**Researched:** 2026-03-08 -**Confidence:** HIGH +**Project:** YellowJacket v1.2 Tag Editing +**Researched:** 2026-03-16 -## Summary +## Recommended Stack -Zero new Go packages needed. The existing stack handles everything required for multi-library support. +### MP3 Tag Writing — `bogem/id3v2/v2` -## Existing Stack (No Changes) +| Technology | Version | Purpose | Why | +|------------|---------|---------|-----| +| `github.com/bogem/id3v2/v2` | v2.1.4 | ID3v2.3/v2.4 read + write for MP3 | Only mature pure-Go ID3v2 writing library. 359 stars, 57 importers, 579 commits. Supports SetTitle/SetArtist/SetAlbum/SetGenre/SetYear, AddAttachedPicture (cover art embedding), AddTextFrame (track/disc numbers, composer via TRCK/TPOS/TCOM), and tag.Save(). | -| Component | Package | Version | Role in Multi-Library | -|-----------|---------|---------|----------------------| -| Database | modernc.org/sqlite | v1.46.1 | ALTER TABLE, new tables, migration 6 | -| Query Gen | sqlc | v1.30.0 | New query files for libraries CRUD + filtered queries | -| Config | BurntSushi/toml | v1.6.0 | Migration source (DirectoryPath to DB) | -| Desktop | Wails | v2.10.2 | Binding patterns for library CRUD | -| Frontend | Lit | 3.2.1 | Reactive controllers for library state | -| Audio | gopxl/beep | v2 | No changes needed | +**Key capabilities verified (HIGH confidence — pkg.go.dev docs):** +- `tag.SetArtist()`, `tag.SetTitle()`, `tag.SetAlbum()`, `tag.SetGenre()`, `tag.SetYear()` — direct setters +- `tag.AddTextFrame("TRCK", id3v2.EncodingUTF8, "5/12")` — track number +- `tag.AddTextFrame("TPOS", id3v2.EncodingUTF8, "1/2")` — disc number +- `tag.AddTextFrame("TCOM", id3v2.EncodingUTF8, "Bach")` — composer +- `tag.AddAttachedPicture(PictureFrame{...})` — cover art embedding with MIME type, picture type (front cover), and raw image bytes +- `tag.Save()` — writes modified tag back to file +- `tag.DeleteFrames(id)` — remove specific frame types (needed for replacing cover art) +- ID3v2.3 and v2.4 version support with `tag.SetVersion()` +- UTF-8 encoding default for v2.4, ISO-8859-1 for v2.3 +- `id3v2.Open(path, Options{Parse: true})` — open existing file, parse all frames, then modify and save -## Do NOT Add +**Integration with existing dhowden/tag:** +- dhowden/tag stays for READ operations (already integrated in `backend/metadata/tags.go`) +- bogem/id3v2 used ONLY for WRITE operations +- No conflict: dhowden/tag reads from `io.ReadSeeker`, bogem/id3v2 reads from file path and writes back +- Read flow unchanged: `dhowden/tag.ReadFrom()` → `TrackMetadata` struct +- Write flow new: `id3v2.Open()` → modify → `tag.Save()` → close -- No ORM or query builder (fights existing sqlc architecture) -- No migration framework (goose, golang-migrate) — PRAGMA user_version works well with 5 existing migrations -- No UUID package — INTEGER PRIMARY KEY is the pattern +**Dependency footprint:** Only dependency is `golang.org/x/text` (already in go.mod). Pure Go, no CGo. -## SQLite Migration Patterns +### FLAC Tag Writing — `go-flac/go-flac` + `go-flac/flacvorbis` + `go-flac/flacpicture` -### Adding library_id to audio_files +| Technology | Version | Purpose | Why | +|------------|---------|---------|-----| +| `github.com/go-flac/go-flac/v2` | v2.x | FLAC metadata block manipulation (parse + save) | Purpose-built for FLAC metadata manipulation. Parses metadata blocks separately from audio frames. `f.Save()` writes back metadata blocks + raw audio frames without re-encoding. 44 stars, clean API. | +| `github.com/go-flac/flacvorbis/v2` | v2.x | Vorbis Comment read/write for FLAC metadata blocks | Companion to go-flac. Provides `ParseFromMetaDataBlock()`, `Add()`, `Marshal()` for Vorbis Comment manipulation. Has field constants (`FIELD_TITLE`, `FIELD_ARTIST`, etc.). | +| `github.com/go-flac/flacpicture` | latest | PICTURE metadata block manipulation for FLAC | Companion to go-flac. `NewFromImageData()` creates PICTURE blocks, `Marshal()` serializes for embedding. | -```sql -ALTER TABLE audio_files ADD COLUMN library_id INTEGER NOT NULL DEFAULT 0; -CREATE INDEX IF NOT EXISTS idx_audio_files_library_id ON audio_files(library_id); +**Why go-flac over mewkiz/flac for WRITING:** +- `mewkiz/flac` is primarily a FLAC **codec** (encoder/decoder). Its `Encode()` API re-encodes audio data, which is unacceptably slow and potentially lossy for metadata-only edits. +- `go-flac/go-flac` is specifically designed for **metadata manipulation**. It stores audio frames as raw bytes and copies them verbatim on save — no re-encoding. +- `mewkiz/flac` stays as an indirect dependency (via beep) for FLAC **decoding** during playback and duration extraction. No conflict. + +**Key capabilities verified (HIGH confidence — GitHub README + examples):** +- `flac.ParseFile(fileName)` — returns `File` with `Meta` (metadata blocks) and `Frames` (raw audio data) +- `flacvorbis.ParseFromMetaDataBlock(*meta)` — parse existing Vorbis Comment block +- `cmts.Add(flacvorbis.FIELD_TITLE, "New Title")` — add/modify comment fields +- `cmts.Marshal()` — serialize back to MetaDataBlock +- `f.Meta[idx] = &cmtsmeta` — replace metadata block in-place +- `f.Save(fileName)` — write modified file (metadata blocks + raw audio frames, no re-encoding) +- `flacpicture.NewFromImageData(PictureTypeFrontCover, "Front cover", imgData, "image/jpeg")` — create picture block +- `picture.Marshal()` → append to `f.Meta` — embed cover art + +**FLAC tag writing approach — metadata block replacement:** +1. `flac.ParseFile(path)` — parses metadata blocks + stores audio frames as raw bytes +2. Find existing VorbisComment block in `f.Meta` slice, or create new via `flacvorbis.New()` +3. Modify/add comment fields via `cmts.Add()` (handles field replacement) +4. Marshal back: `f.Meta[idx] = &cmts.Marshal()` +5. For cover art: create via `flacpicture.NewFromImageData()`, append to `f.Meta` +6. `f.Save(tmpPath)` — writes "fLaC" + metadata blocks + raw audio frames to temp file +7. Atomic rename temp file over original + +**Critical detail:** `go-flac/go-flac`'s `Save()` copies audio frames as raw bytes — no re-encoding. A metadata-only edit of a 50MB FLAC file takes ~50ms, not minutes. + +### OGG Vorbis Tag Writing — Custom Implementation Required + +| Technology | Version | Purpose | Why | +|------------|---------|---------|-----| +| Custom OGG page rewriter | n/a | OGG Vorbis Comment + Picture writing | No pure-Go OGG tag writing library exists. `jfreymuth/oggvorbis` is decode-only. OGG tag writing requires parsing OGG pages, modifying the Vorbis Comment header packet, and rewriting pages. | + +**Why custom OGG writing is necessary:** +- `jfreymuth/oggvorbis` (existing indirect dep) is a **decoder only** — no write API +- No other pure-Go OGG Vorbis tag writer exists in the ecosystem +- OGG Vorbis comments are stored in the second header packet (comment header), which is an OGG page +- Modifying comments changes page sizes, requiring page-level rewriting + +**OGG tag writing approach:** +1. Parse OGG pages to find the three Vorbis header packets (identification, comment, setup) +2. Decode existing Vorbis Comment from the comment header packet +3. Modify comment fields (same key=value format as FLAC Vorbis Comments) +4. Re-encode comment packet into new OGG pages +5. Copy identification and setup headers unchanged +6. Copy all audio data pages unchanged +7. Write to temp file, atomic rename + +**Complexity assessment:** MEDIUM-HIGH. OGG page framing is well-documented but requires careful implementation. The Vorbis Comment format itself is simple (same as FLAC). The OGG page CRC and segment tables are the tricky parts. + +**Cover art in OGG:** Stored as `METADATA_BLOCK_PICTURE` Vorbis Comment tag (base64-encoded FLAC Picture block). Same encoding as FLAC Picture but base64-wrapped in a comment field. + +**Recommendation:** Implement OGG writing LAST. Start with MP3 and FLAC (libraries exist). OGG uses the same Vorbis Comment format as FLAC, so the comment serialization code is shared — only the OGG page framing is new work. + +### Atomic File Writing — No New Dependency + +| Technology | Version | Purpose | Why | +|------------|---------|---------|-----| +| `os.CreateTemp` + `os.Rename` (stdlib) | Go 1.25 | Write-to-temp-then-rename pattern | The stdlib approach is simpler and sufficient. `natefinch/atomic` is already an indirect dep but provides `WriteFile(filename, io.Reader)` which doesn't match our use case (we need to write to temp first, THEN rename). The stdlib pattern gives more control over temp file location (same directory as target for same-filesystem rename). | + +**Pattern:** +```go +// Create temp file in same directory as target (ensures same filesystem for atomic rename) +dir := filepath.Dir(targetPath) +tmp, err := os.CreateTemp(dir, ".yj-tag-*.tmp") +// ... write tag data to tmp ... +tmp.Close() +// Atomic rename (POSIX guarantees atomicity for same-filesystem rename) +os.Rename(tmp.Name(), targetPath) ``` -SQLite limitation: `ALTER TABLE ADD COLUMN` cannot add FK constraints. Enforce at application level. +**Why NOT `natefinch/atomic`:** It's designed for `io.Reader` → file workflows. Our workflow is: read original → write modified to temp → rename. The stdlib `os.CreateTemp` + `os.Rename` is the right primitive. `natefinch/atomic` also uses `os.Rename` internally on Unix anyway. -### track_metadata VIEW +**Why same-directory temp file matters:** `os.Rename` is only atomic when source and destination are on the same filesystem. Music files could be on any mount point. Creating the temp file in the same directory guarantees this. -Must DROP VIEW + CREATE VIEW (SQLite doesn't support ALTER VIEW). Add `af.library_id` to SELECT list. Follows migration 5 pattern exactly. +## Alternatives Considered -### playlist_tracks Table Rebuild +| Category | Recommended | Alternative | Why Not | +|----------|-------------|-------------|---------| +| MP3 write | bogem/id3v2/v2 | dhowden/tag | dhowden/tag is read-only. No write API. Would require forking. | +| MP3 write | bogem/id3v2/v2 | go-id3 (mikkyang) | Dead project, archived, no v2 module support, last commit 2015 | +| FLAC write | go-flac/go-flac + flacvorbis | mewkiz/flac | mewkiz/flac is a codec (encoder/decoder); its Encode() re-encodes audio. go-flac is purpose-built for metadata manipulation — copies audio frames as raw bytes. | +| FLAC write | go-flac/go-flac + flacvorbis | Custom FLAC writer | go-flac handles the format correctly with proven Save(); reinventing would be fragile | +| OGG write | Custom | CGo (libvorbis) | Violates no-CGo constraint | +| OGG write | Custom | dhowden/tag fork | dhowden/tag OGG parsing is minimal, not designed for writing | +| Atomic write | stdlib os.CreateTemp+Rename | natefinch/atomic | Doesn't match our write pattern; stdlib is sufficient | +| Atomic write | stdlib os.CreateTemp+Rename | renameio | Unnecessary dep for a 5-line pattern | -Change `ON DELETE CASCADE` to `ON DELETE SET NULL` on `audio_file_id`. Requires full table rebuild (migration 5 pattern: PRAGMA foreign_keys OFF -> create _new -> copy -> drop -> rename -> PRAGMA foreign_keys ON). +## What NOT To Add -## sqlc Query Patterns +| Library | Why Avoid | +|---------|-----------| +| Any CGo-based tag library (taglib-go, etc.) | Violates pure-Go constraint from PROJECT.md | +| go-id3 (mikkyang/id3-go) | Archived, unmaintained since 2015, no module support | +| Any "universal tag writer" that wraps TagLib via CGo | Violates pure-Go constraint | +| natefinch/atomic as direct dep | Already indirect; stdlib pattern is more appropriate for this use case | +| goflac (CGo wrapper around libFLAC) | Violates pure-Go constraint | -- Create `sql/queries/libraries.sql` for CRUD -- Create `ByLibrary` variants of key queries (GetAllTracksWithFullMetadataByLibrary, GetAllAlbumsWithDetailsByLibrary, etc.) -- Separate queries preferred over dynamic WHERE (cleaner types, better query plans) -- Hand-crafted FTS5 queries get `AND tm.library_id = ?` filter +## Existing Dependencies Leveraged (No Version Changes) -## Frontend Patterns +| Library | Current Use | New Use in Tag Editing | +|---------|------------|----------------------| +| `dhowden/tag` v0.0.0-20240417 | Tag reading during library scan | Unchanged — still used for all READ operations | +| `mewkiz/flac` v1.0.12 (indirect via beep) | FLAC audio decoding during playback + duration extraction | Unchanged — remains indirect for decoding only | +| `golang.org/x/image` v0.12.0 | Cover art thumbnail generation | Image validation before embedding (ensure valid JPEG/PNG) | +| `natefinch/atomic` v1.0.1 (indirect) | Not directly used | Remains indirect; not needed for our pattern | -- `LibraryStore` gains `selectedLibraryId` state (null = all libraries) -- Backend filtering (not frontend) — don't load 150K tracks when viewing one library -- Persist selection in localStorage -- `invalidate()` on library switch triggers refetch +## Installation -## Config Migration +```bash +# New direct dependencies +go get github.com/bogem/id3v2/v2@v2.1.4 +go get github.com/go-flac/go-flac/v2 +go get github.com/go-flac/flacvorbis/v2 +go get github.com/go-flac/flacpicture +``` -Libraries stored in SQLite (not TOML). Config `[Library].DirectoryPath` read once during migration, then deprecated. All library management through DB-backed methods. +## Format Coverage Matrix + +| Format | Text Tags | Cover Art Embed | Library | Confidence | +|--------|-----------|-----------------|---------|------------| +| MP3 (ID3v2) | ✓ Full | ✓ APIC frame | bogem/id3v2/v2 | HIGH | +| FLAC | ✓ Full | ✓ Picture block | go-flac/go-flac + flacvorbis + flacpicture | HIGH | +| OGG Vorbis | ✓ Full | ✓ METADATA_BLOCK_PICTURE | Custom (built on Vorbis Comment format) | MEDIUM | +| WAV | ✗ Not supported | ✗ Not supported | n/a — WAV has no standard tag format | n/a | + +**WAV exclusion rationale:** WAV files have no widely-adopted metadata standard. Some players use INFO chunks, some use ID3v2 headers prepended to WAV. The project already supports WAV playback but doesn't extract meaningful tags from WAV during scanning. Tag editing for WAV is out of scope. + +## Vorbis Comment Field Mapping + +Both FLAC and OGG use Vorbis Comments. Field names are standardized: + +| YellowJacket Field | Vorbis Comment Key | ID3v2 Frame ID | +|--------------------|-------------------|----------------| +| Title | TITLE | TIT2 | +| Artist | ARTIST | TPE1 | +| Album | ALBUM | TALB | +| Album Artist | ALBUMARTIST | TPE2 | +| Genre | GENRE | TCON | +| Year | DATE | TDRC (v2.4) / TYER (v2.3) | +| Track Number | TRACKNUMBER | TRCK | +| Total Tracks | TRACKTOTAL | TRCK (as "N/Total") | +| Disc Number | DISCNUMBER | TPOS | +| Total Discs | DISCTOTAL | TPOS (as "N/Total") | +| Composer | COMPOSER | TCOM | +| Comment | COMMENT | COMM | +| Lyrics | LYRICS | USLT | + +## Sources + +- bogem/id3v2: https://pkg.go.dev/github.com/bogem/id3v2/v2 (HIGH confidence — official docs) +- bogem/id3v2 GitHub: https://github.com/n10v/id3v2 (HIGH confidence — 359 stars, v2.1.4 release Feb 2023) +- go-flac/go-flac GitHub: https://github.com/go-flac/go-flac (HIGH confidence — 44 stars, metadata manipulation library with Save()) +- go-flac/flacvorbis GitHub: https://github.com/go-flac/flacvorbis (HIGH confidence — Vorbis Comment add/parse/marshal, v2 module) +- go-flac/flacpicture GitHub: https://github.com/go-flac/flacpicture (HIGH confidence — PICTURE block creation from image data) +- mewkiz/flac GitHub: https://github.com/mewkiz/flac (HIGH confidence — confirmed codec, not suitable for metadata-only writes) +- dhowden/tag GitHub: https://github.com/dhowden/tag (HIGH confidence — confirmed read-only, no write API) +- jfreymuth/oggvorbis GitHub: https://github.com/jfreymuth/oggvorbis (HIGH confidence — confirmed decode-only) +- natefinch/atomic GitHub: https://github.com/natefinch/atomic (HIGH confidence — confirmed API mismatch for our use case) +- Vorbis Comment spec: https://www.xiph.org/vorbis/doc/v-comment.html +- FLAC format spec: https://www.xiph.org/flac/format.html +- OGG framing spec: https://www.xiph.org/ogg/doc/framing.html diff --git a/.planning/research/SUMMARY.md b/.planning/research/SUMMARY.md index 4ccec2b..8cebf66 100644 --- a/.planning/research/SUMMARY.md +++ b/.planning/research/SUMMARY.md @@ -1,50 +1,190 @@ -# Research Summary: Multi-Library Support +# Project Research Summary -**Synthesized:** 2026-03-08 -**Sources:** STACK.md, FEATURES.md, ARCHITECTURE.md, PITFALLS.md +**Project:** YellowJacket v1.2 Tag Editing +**Domain:** Audio metadata editing in a desktop music player (Go/Wails) +**Researched:** 2026-03-16 +**Confidence:** HIGH + +## Executive Summary + +Tag editing for YellowJacket is a cross-cutting feature that touches file I/O (three audio formats), a normalized relational database, an FTS5 search index, a cover art cache pipeline, and the frontend state — all from a single user action. Mature desktop music players (foobar2000, MusicBee, Mp3tag, Kid3) converge on a consistent pattern: modal dialog editing with atomic file writes, inline DB updates (no rescan), and batch editing with three-state field semantics (keep/set/clear). The existing codebase has substantial scaffolding already in place — the `track-details` component has an edit mode UI stub with a no-op save handler, multi-select works in the track list, and the cover art pipeline is fully operational. + +The recommended approach uses **three external libraries** for tag writing — `bogem/id3v2/v2` for MP3, `go-flac/go-flac` + `go-flac/flacvorbis` + `go-flac/flacpicture` for FLAC — plus a **custom OGG page rewriter** (deferred to last, since no pure-Go OGG tag writing library exists). A new `backend/tageditor/` package orchestrates the full pipeline: validate → write tags to temp file → atomic rename → update DB entities in a single transaction → update FTS5 → emit event → frontend refreshes. This keeps the existing `library`, `metadata`, and `database` packages unchanged. + +The dominant risks are: (1) **FLAC files require full rewrite** for tag changes (no in-place edit), making atomic write-to-temp-then-rename mandatory; (2) the **FTS5 contentless index cannot delete rows**, requiring a schema migration to `contentless_delete=1` before any tag writing code ships; (3) the **normalized schema shares entities** (artists, albums, genres) across tracks, so editing one track must create new entity rows and repoint references rather than mutating shared rows in-place; and (4) a **race condition** between tag editing and library scanning requires mutual exclusion. All four risks have well-understood mitigations documented in the research. ## Key Findings -### Stack -- **Zero new packages needed** — existing SQLite, sqlc, Wails, Lit stack handles everything -- Migration 6 follows existing PRAGMA user_version pattern (5 precedents) -- sqlc queries: create ByLibrary variants for ~5 key queries +### Recommended Stack -### Features -- **Desktop player pattern:** Merged view by default (foobar2000, Roon), optional filter (Navidrome) -- **Table stakes:** Multiple folders, unified view, per-folder scan, graceful removal, offline handling -- **Anti-features:** Separate databases, user access control, auto-dedup -- Cross-library playlists are expected by users (Navidrome, foobar2000) +Pure-Go tag writing is well-supported for MP3 and FLAC via established libraries. OGG Vorbis tag writing requires custom implementation but shares the Vorbis Comment format with FLAC, so serialization code is reusable. No new dependencies beyond `golang.org/x/text` (already in go.mod) are pulled in transitively. The existing `dhowden/tag` library stays for all READ operations — no conflict with the new write libraries. -### Architecture -- **Hybrid model:** `library_id` on `audio_files` only; artists/albums/genres stay global -- **Migration 6:** Create libraries table -> add library_id column -> rebuild playlist_tracks for SET NULL + phantom columns -> recreate track_metadata VIEW -- **Scan pipeline:** `ScanLibrary(id)` replaces `Scan()`, sequential coordination -- **Orphan cleanup:** Reference-counting bottom-up deletes for shared entities -- **FTS5:** Contentless table works via JOIN filtering; consider contentless_delete migration +**Core technologies:** +- **`bogem/id3v2/v2`** (v2.1.4): MP3 ID3v2 read+write — 359 stars, 57 importers, handles encoding (UTF-8/UTF-16) correctly, supports picture frames. HIGH confidence. +- **`go-flac/go-flac/v2` + `go-flac/flacvorbis/v2` + `go-flac/flacpicture`**: FLAC metadata manipulation — copies audio frames as raw bytes (no re-encoding), ~50ms for metadata-only edits on large files. HIGH confidence. +- **Custom OGG page rewriter**: No pure-Go OGG tag writer exists. OGG page framing (CRC, segment tables) is the only new work — Vorbis Comment serialization is shared with FLAC. MEDIUM confidence. +- **stdlib `os.CreateTemp` + `os.Rename`**: Atomic file write pattern — temp file in same directory guarantees same-filesystem rename. No external dependency needed. + +**Critical version requirements:** +- SQLite ≥ 3.43.0 for `contentless_delete=1` FTS5 support (bundled `modernc.org/sqlite` provides 3.45+, so already satisfied) + +### Expected Features + +**Must have (table stakes):** +- Single track tag editing (title, artist, album, genre, year, track#, disc#, composer) +- Write tags to MP3 (ID3v2) and FLAC (Vorbis Comments) +- Write-to-temp-then-rename corruption safety +- Inline DB + FTS5 update after tag write (no rescan) +- Batch editing with three-state field model (keep/set/clear) +- Cover art set/replace from image file +- Save confirmation and error feedback + +**Should have (differentiators):** +- Progress indicator for batch operations (20+ files) +- Auto-number tracks in batch edit +- Cover art remove (strip embedded art) +- Dirty indicator / unsaved changes warning +- Album artist, comment, lyrics field editing (low-effort additions) +- Total tracks / total discs fields + +**Defer (v2+):** +- MusicBrainz auto-tagging (separate milestone already in PROJECT.md) +- Undo/backup system for tag edits +- Cover art paste from clipboard +- Inline editing in track list columns (fragile UX, complex) +- Raw tag frame editing, custom fields, filename renaming +- OGG Vorbis tag writing (implement last due to custom work required) + +### Architecture Approach + +Tag editing is implemented as a new `backend/tageditor/` package that orchestrates the full write pipeline, keeping existing packages focused on their current responsibilities. The service exposes `EditTrack()`, `EditTracks()`, and `SetCoverArt()` as Wails bindings. It uses pointer fields (`*string`, `*int`) to distinguish "no change" (nil) from "set to empty" — mapping directly to the three-state UI model for batch editing. + +**Major components:** +1. **`backend/tageditor/tageditor.go`** — Service orchestrator: validates input, coordinates file write → DB update → FTS5 → events +2. **`backend/tageditor/writer.go`** — Format-specific tag writing (MP3 via bogem/id3v2, FLAC via go-flac, OGG via custom) +3. **`backend/events/events.go`** (modified) — New `TagsUpdated` and `TagEditFailed` event constants +4. **`frontend/src/components/track-details/`** (modified) — Wire existing edit UI stub to backend, add batch edit variant +5. **`frontend/src/store/library-store.ts`** (modified) — Listen for `TagsUpdated` event, full re-fetch on change + +**Key patterns:** +- Write-to-temp-then-rename (temp in same directory as target) +- Upsert-and-relink for shared entities (never mutate shared artist/album/genre rows) +- Pointer fields for optional partial updates +- Lazy orphan cleanup (defer to next rescan) ### Critical Pitfalls -1. ALTER TABLE ADD COLUMN requires DEFAULT for NOT NULL — create libraries first -2. Table rebuild must audit ALL CASCADE FKs (playlist_tracks AND queue_tracks) -3. FTS5 contentless can't DELETE rows — stale entries accumulate after library removal -4. Orphan cleanup must not delete shared entities across libraries -5. Existing user migration must be seamless (TOML to DB) -6. Scan coordination must serialize (single-writer SQLite) -## Architecture Decision Record +1. **FTS5 contentless index can't delete rows (P3)** — Migrate to `contentless_delete=1` before writing any tag edit code. Without this, search returns stale results after every edit. This is a prerequisite schema migration. +2. **FLAC requires full file rewrite (P1)** — No in-place edit possible. Write-to-temp-then-rename is mandatory. Temp file must be in the same directory for atomic rename. Verify written file before replacing original. +3. **Shared entity fan-out (P4)** — Editing one track's artist must NOT modify the shared `artist_credit` row (would silently change 200 other tracks). Always create new entity rows and repoint the edited track's foreign keys. +4. **Currently-playing file lock (P2)** — On Windows, `os.Rename()` fails if the player holds an open file handle. Must check player state and stop playback before editing the current track. +5. **Scan/edit race condition (P5)** — A library scan running during tag editing can overwrite changes. Pause scan during edits using the existing `PauseScan()`/`ResumeScan()` mechanism. -| Decision | Rationale | -|----------|-----------| -| library_id on audio_files only | Physical files belong to libraries; logical entities (artists, albums) are global | -| Libraries in DB, not TOML | CRUD through UI shouldn't require TOML manipulation; DB is already source of truth | -| SET NULL for playlist_tracks FK | Phantom tracks preserve playlist structure when library removed | -| CASCADE for queue_tracks FK | Queue is ephemeral, not user-curated like playlists | -| Sequential scanning | SQLite single-writer makes parallel scans pointless | -| Backend filtering, not frontend | Don't load 150K tracks when viewing one library | +## Implications for Roadmap -## Build Order +Based on research, suggested phase structure: -1. **Schema & Migration** — Foundation everything else depends on -2. **Backend Scan Pipeline** — Per-library scanning before exposing in UI -3. **Backend API** — CRUD, filtered queries, events, orphan cleanup -4. **Frontend** — Library manager, filter, store updates, phantom display +### Phase 1: Schema Migration & Write Safety Foundation + +**Rationale:** The FTS5 migration (P3) is a hard prerequisite — without `contentless_delete=1`, tag edits degrade search quality. The atomic file write mechanism (P1, P6) is the foundation all tag writing depends on. These are small, testable, independent pieces that de-risk everything downstream. +**Delivers:** FTS5 schema migration; atomic write-to-temp-then-rename utility; temp file cleanup on startup +**Addresses:** Write-to-temp-then-rename (table stakes), FTS5 inline update capability +**Avoids:** P3 (stale search), P1 (file corruption), P6 (cross-filesystem rename failure) + +### Phase 2: Tag Writing Library Integration + +**Rationale:** With the write safety layer in place, integrate the format-specific tag writing libraries. MP3 first (most common format, best library), then FLAC. This phase is pure backend — no UI changes yet. Unit tests with real audio files validate round-trip correctness. +**Delivers:** `backend/tageditor/writer.go` with MP3 + FLAC tag writing; encoding handling (P7); cover art embedding capability +**Uses:** `bogem/id3v2/v2`, `go-flac/go-flac/v2` + `go-flac/flacvorbis/v2` + `go-flac/flacpicture` +**Avoids:** P7 (encoding mismatch), P8 (cover art format issues), P12 (dhowden/tag is read-only) + +### Phase 3: Single Track Edit Pipeline + +**Rationale:** Wire the full pipeline end-to-end for a single track: backend service → file write → DB entity update → FTS5 re-index → event emission → frontend refresh. This is the core loop that all other features build on. Includes the shared entity upsert-and-relink pattern (P4) and genre dual-representation sync (P10). +**Delivers:** `backend/tageditor/tageditor.go` service; `EditTrack()` Wails binding; wired `track-details` save handler; `TagsUpdated` event; library store refresh +**Implements:** Tageditor service, DB update logic, event system, frontend integration +**Avoids:** P4 (shared entity mutation), P5 (scan race), P10 (genre mismatch), P11 (partial failure), P17 (stale frontend cache) + +### Phase 4: Cover Art Editing + +**Rationale:** Cover art embedding builds on Phase 2's writer and Phase 3's pipeline but adds image validation, the cover art cache pipeline integration, and file picker UX. Separated because cover art has its own pitfalls (P8, P13) and is independently testable. +**Delivers:** `SetCoverArt()` binding; image resize/validation before embed; cover art cache invalidation and thumbnail regeneration; cover art remove capability +**Avoids:** P8 (oversized images, format issues), P13 (stale cached thumbnails) + +### Phase 5: Batch Editing + +**Rationale:** Batch editing is the highest-complexity UI feature (three-state field model, mixed-value indicators, progress tracking). It depends on the single-track pipeline being solid. The backend is straightforward (loop over `EditTrack()`), but the frontend UX is where the complexity lives. +**Delivers:** Batch edit dialog with three-state fields; `EditTracks()` binding; progress indicator; auto-number tracks; confirmation dialog for destructive batch operations +**Addresses:** Batch editing (table stakes), progress indicator (differentiator), auto-number (differentiator) +**Avoids:** P9 (orphan entity accumulation — run cleanup after batch), P14 (no undo — confirmation dialog) + +### Phase 6: OGG Vorbis Tag Writing (Stretch) + +**Rationale:** OGG tag writing requires a custom OGG page rewriter — MEDIUM-HIGH complexity with no library support. The Vorbis Comment serialization is shared with FLAC (Phase 2), so only the OGG page framing is new. This can ship after the core MP3/FLAC editing is stable. +**Delivers:** Custom OGG page rewriter; OGG Vorbis tag writing support; full format coverage (MP3 + FLAC + OGG) +**Avoids:** Scope creep — if OGG proves too complex, MP3 + FLAC cover the vast majority of user libraries + +### Phase Ordering Rationale + +- **Schema migration first** because FTS5 `contentless_delete=1` is a hard prerequisite that must be in place before any DB update code is written for tag editing. +- **Write safety before tag libraries** because the atomic write mechanism is tested independently of any format-specific code. +- **MP3 before FLAC before OGG** because library quality/maturity decreases in that order, and MP3 covers the largest user base. +- **Single track before batch** because batch editing is N × single with UI complexity on top — the underlying pipeline must be solid. +- **Cover art as a separate phase** because it has independent pitfalls (image validation, cache invalidation) and is testable in isolation. +- **OGG last** because it requires custom implementation and MP3 + FLAC cover the majority of use cases. + +### Research Flags + +Phases likely needing deeper research during planning: +- **Phase 2 (Tag Writing):** `go-flac` libraries have smaller communities (44 stars) — verify FLAC write round-trip with edge cases (large files, existing padding blocks, multiple PICTURE blocks) during implementation. +- **Phase 6 (OGG Writing):** Custom OGG page rewriter needs specification-level research (OGG framing RFC). Consider prototyping before committing to scope. + +Phases with standard patterns (skip research-phase): +- **Phase 1 (Schema Migration):** Well-documented SQLite FTS5 migration. `contentless_delete=1` is a one-line schema change. +- **Phase 3 (Single Track Edit):** The architecture is fully designed — pointer fields, upsert-and-relink, event emission are all standard Go/Wails patterns. +- **Phase 5 (Batch Editing):** The three-state field model is well-understood from foobar2000/MusicBee analysis. Frontend-heavy but no novel backend work. + +## Confidence Assessment + +| Area | Confidence | Notes | +|------|------------|-------| +| Stack | HIGH | MP3 library (bogem/id3v2) verified via pkg.go.dev docs with 359 stars/57 importers. FLAC libraries verified via GitHub READMEs. OGG is the only gap (custom work). | +| Features | HIGH | Cross-referenced 5 desktop music players + Hydrogenaudio tag standards + existing codebase analysis. Table stakes are unambiguous. | +| Architecture | HIGH | Based on full codebase analysis — every integration point verified against actual source files (library.go, search.go, tags.go, track-details.ts, player.go). | +| Pitfalls | HIGH | All critical pitfalls derived from format specifications (FLAC, ID3v2, OGG, FTS5) and codebase analysis (shared entities, file locking, scan race). Mitigations are concrete. | + +**Overall confidence:** HIGH + +### Gaps to Address + +- **OGG Vorbis tag writing:** No pure-Go library exists. Custom implementation complexity is estimated at MEDIUM-HIGH but not prototyped. Validate feasibility during Phase 6 planning — consider whether OGG support is worth the custom code, or whether to accept MP3+FLAC-only for v1.2. +- **`go-flac` edge cases:** The go-flac library has 44 stars and a small community. Round-trip testing with edge-case FLAC files (files with existing PADDING blocks, multiple PICTURE blocks, unusual metadata block orders) should be done early in Phase 2 to surface any library bugs. +- **Windows file locking behavior:** The currently-playing-file lock (P2) is well-understood conceptually but the exact interaction between Go's `os.Open`, beep's streamer, and Windows mandatory locking needs validation on a Windows build. +- **Album artist storage:** FEATURES.md notes album artist editing is low-hanging fruit, but ARCHITECTURE.md flags that album artist isn't currently stored as a separate entity. Schema implications should be resolved during Phase 3 planning. + +## Sources + +### Primary (HIGH confidence) +- `bogem/id3v2` (n10v/id3v2): https://pkg.go.dev/github.com/bogem/id3v2/v2 — API docs, v2.1.4, write support verified +- `go-flac/go-flac`: https://github.com/go-flac/go-flac — metadata manipulation, Save() copies audio frames as raw bytes +- `go-flac/flacvorbis`: https://github.com/go-flac/flacvorbis — Vorbis Comment add/parse/marshal +- `go-flac/flacpicture`: https://github.com/go-flac/flacpicture — PICTURE block creation from image data +- SQLite FTS5 docs: https://www.sqlite.org/fts5.html — contentless tables, contentless_delete=1 +- FLAC format spec: https://www.xiph.org/flac/format.html — metadata block structure +- Vorbis Comment spec: https://www.xiph.org/vorbis/doc/v-comment.html — field format +- OGG framing spec: https://www.xiph.org/ogg/doc/framing.html — page structure +- Hydrogenaudio Tag Mapping: https://wiki.hydrogenaud.io/index.php/Tag_Mapping — field name standards +- YellowJacket codebase: library.go, search.go, tags.go, player.go, track-details.ts, schema files — architecture analysis + +### Secondary (MEDIUM confidence) +- `dhowden/tag`: https://github.com/dhowden/tag — confirmed read-only, no write API +- `mewkiz/flac`: https://github.com/mewkiz/flac — confirmed codec (encoder/decoder), unsuitable for metadata-only writes +- `jfreymuth/oggvorbis`: https://github.com/jfreymuth/oggvorbis — confirmed decode-only +- MusicBee, foobar2000, Kid3, Mp3tag, Picard — feature pattern analysis + +### Tertiary (LOW confidence) +- OGG Vorbis custom writer feasibility — estimated MEDIUM-HIGH complexity based on spec analysis, not prototyped + +--- +*Research completed: 2026-03-16* +*Ready for roadmap: yes*