From 2439c2a728e6e8497fbb2466e5bf8e8ca5277820 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Thu, 5 Mar 2026 10:27:25 -0500 Subject: [PATCH] docs(quick-10): diagnose duplicate album merging bug - albums with same name but different artists shown as single entry --- .../10-PLAN.md | 191 ++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 .planning/quick/10-diagnose-duplicate-album-merging-bug-alb/10-PLAN.md diff --git a/.planning/quick/10-diagnose-duplicate-album-merging-bug-alb/10-PLAN.md b/.planning/quick/10-diagnose-duplicate-album-merging-bug-alb/10-PLAN.md new file mode 100644 index 0000000..de911f8 --- /dev/null +++ b/.planning/quick/10-diagnose-duplicate-album-merging-bug-alb/10-PLAN.md @@ -0,0 +1,191 @@ +--- +phase: quick-10 +plan: 10 +type: execute +wave: 1 +depends_on: [] +files_modified: + - backend/database/sql/schemas/release_groups.sql + - backend/database/sql/queries/release_groups.sql + - backend/database/sql/sqlcgen/release_groups.sql.go + - backend/database/database.go + - backend/library/library.go +autonomous: true +requirements: [] + +must_haves: + truths: + - "Two albums with the same name but different artists are stored as separate release_groups rows" + - "Scanning a library with two 'Classics' albums (Aphex Twin + Ratatat) produces two distinct entries" + - "The cover grid shows both albums as separate entries with correct artist names" + - "Opening each album shows only its own tracks, not tracks from the other" + artifacts: + - path: "backend/database/sql/schemas/release_groups.sql" + provides: "UNIQUE constraint on (name, album_artist_credit_id) instead of name alone" + - path: "backend/database/sql/queries/release_groups.sql" + provides: "UpsertReleaseGroup with ON CONFLICT(name, album_artist_credit_id)" + - path: "backend/database/database.go" + provides: "Migration 5 to rebuild release_groups table with new unique constraint" + - path: "backend/library/library.go" + provides: "Entity cache keyed by album name + artist credit ID" + key_links: + - from: "backend/library/library.go" + to: "backend/database/sql/sqlcgen/release_groups.sql.go" + via: "UpsertReleaseGroup call in resolveReleaseGroup" + pattern: "UpsertReleaseGroup" + - from: "backend/database/database.go" + to: "backend/database/sql/schemas/release_groups.sql" + via: "Migration 5 rebuilds release_groups with new constraint" + pattern: "migration.*5" +--- + + +Fix the album merging bug where albums with the same name but different artists are incorrectly stored as a single entry. Root cause: the `release_groups` table has `UNIQUE(name)` instead of `UNIQUE(name, album_artist_credit_id)`, causing `ON CONFLICT` to merge distinct albums. + +Purpose: Two users' "Classics" albums (Aphex Twin and Ratatat) should appear as separate entries in the cover grid, each with correct cover art, artist name, and track listing. + +Output: Schema migration, updated SQL queries, regenerated sqlc code, and fixed entity cache. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/STATE.md +@backend/database/sql/schemas/release_groups.sql +@backend/database/sql/queries/release_groups.sql +@backend/database/database.go +@backend/library/library.go + + + + + + Task 1: Fix schema, queries, and regenerate sqlc + + backend/database/sql/schemas/release_groups.sql + backend/database/sql/queries/release_groups.sql + backend/database/sql/sqlcgen/release_groups.sql.go + + +1. **Update `release_groups.sql` schema** (line 3): Remove `UNIQUE` from the `name` column definition. Add a composite unique constraint at the table level: + ```sql + name TEXT NOT NULL, + ``` + And after the FOREIGN KEY lines, before the closing `);`: + ```sql + UNIQUE(name, album_artist_credit_id) + ``` + + **IMPORTANT**: SQLite treats each NULL as unique in UNIQUE constraints, so albums without an album_artist_credit_id will each get their own row. This is the desired behavior — an album with no tagged artist should not conflict with named-artist albums. + +2. **Update `release_groups.sql` queries**: + - `UpsertReleaseGroup` (line 22): Change `ON CONFLICT(name)` to `ON CONFLICT(name, album_artist_credit_id)`. This ensures upsert only matches when BOTH album name and artist match. + - `GetReleaseGroupByName` (lines 15-17): Add an `album_artist_credit_id` parameter. Rename to `GetReleaseGroupByNameAndArtist`: + ```sql + -- name: GetReleaseGroupByNameAndArtist :one + SELECT * FROM release_groups + WHERE name = ? AND album_artist_credit_id = ? LIMIT 1; + ``` + **Check first**: grep codebase for any callers of `GetReleaseGroupByName`. If there are callers, update them to pass the artist credit ID. If no callers exist outside generated code, safe to rename. + +3. **Regenerate sqlc**: Run `sqlc generate` from `backend/database/` directory: + ```bash + cd backend/database && sqlc generate + ``` + Verify the generated `release_groups.sql.go` has the updated function signatures (UpsertReleaseGroup params unchanged since it already takes album_artist_credit_id; GetReleaseGroupByNameAndArtist now takes two params). + +**SAFETY NOTE (hand-crafted SQL follows in Task 2)**: The schema file change only affects NEW databases. Existing databases need the migration in Task 2. + + + - `sqlc generate` completes without errors from `backend/database/` + - `go build ./...` passes from project root + - Schema file has `UNIQUE(name, album_artist_credit_id)` instead of `name TEXT NOT NULL UNIQUE` + - UpsertReleaseGroup query uses `ON CONFLICT(name, album_artist_credit_id)` + + Schema and queries updated for composite uniqueness, sqlc regenerated, project compiles. + + + + Task 2: Add migration 5 and fix entity cache + + backend/database/database.go + backend/library/library.go + + +1. **Add migration 5 in `database.go`** after the migration 4 block (after line 286). Follow the existing migration pattern (check `version < 5`, bump to `PRAGMA user_version = 5`). + + Migration 5 must: + - **SAFETY**: This is hand-crafted SQL for a schema migration. SQLite cannot ALTER a UNIQUE constraint, so we must rebuild the table. + - Create `release_groups_new` with the corrected schema (matching the updated `release_groups.sql` exactly — same columns, same foreign keys, but `UNIQUE(name, album_artist_credit_id)` instead of `UNIQUE(name)`). + - Copy all data: `INSERT INTO release_groups_new SELECT * FROM release_groups` + - Drop old table: `DROP TABLE release_groups` + - Rename: `ALTER TABLE release_groups_new RENAME TO release_groups` + - Recreate both indexes: + ```sql + CREATE INDEX IF NOT EXISTS idx_release_groups_cover_art_id ON release_groups(cover_art_id); + CREATE INDEX IF NOT EXISTS idx_release_groups_album_artist_credit_id ON release_groups(album_artist_credit_id); + ``` + - Set `PRAGMA user_version = 5` + - Log: `"applying migration 5: release_groups composite unique constraint"` + - Log completion: `"migration 5 complete"` + + **NOTE**: The migration does NOT split already-merged albums. That requires a full library rescan which the user triggers manually. The migration just removes the bad constraint so future scans work correctly. + + **NOTE**: The `release_group_recordings` table has a foreign key `REFERENCES release_groups(id)`. Since we're dropping and recreating, we need to handle this. SQLite defers FK checks by default when foreign_keys is ON. Wrap the migration in: + ```go + // Temporarily disable FK checks for table rebuild. + db.ExecContext(ctx, "PRAGMA foreign_keys = OFF") + // ... migration steps ... + db.ExecContext(ctx, "PRAGMA foreign_keys = ON") + ``` + +2. **Fix entity cache in `library.go`**: + - Line 44: Change cache type from `map[string]sqlcgen.ReleaseGroup` to `map[string]sqlcgen.ReleaseGroup` (type stays same, but key semantics change). + - In `resolveReleaseGroup()` (lines 1253-1327): Change all cache key accesses from `tags.Album` to a composite key. Create a helper or inline: + ```go + // Build composite cache key: "albumName\x00artistCreditID" (or "albumName\x00-1" if no artist). + artistID := int64(-1) + if albumArtistCreditID.Valid { + artistID = albumArtistCreditID.Int64 + } + cacheKey := fmt.Sprintf("%s\x00%d", tags.Album, artistID) + ``` + - Replace all 3 occurrences of `cache.releaseGroups[tags.Album]` with `cache.releaseGroups[cacheKey]`: + - Line 1265: cache lookup + - Line 1283: cache update after cover art + - Line 1324: cache store after upsert + + + - `go build ./...` passes + - `go test ./backend/database/...` passes (existing migration tests should still work since migration 5 is additive) + - `go test ./backend/library/...` passes + - `go vet ./...` passes + + Migration 5 rebuilds release_groups with composite unique constraint. Entity cache uses composite key (album name + artist credit ID). Existing databases upgraded on next app start. User triggers full rescan to split previously merged albums. + + + + + +- `go build ./...` — project compiles +- `go test ./...` — all tests pass +- `go vet ./...` — no issues +- Schema file reflects `UNIQUE(name, album_artist_credit_id)` +- UpsertReleaseGroup uses `ON CONFLICT(name, album_artist_credit_id)` +- Migration 5 exists and rebuilds the release_groups table +- Entity cache key includes artist credit ID + + + +- Two albums named "Classics" by different artists stored as separate release_groups rows after rescan +- Each album shows only its own tracks when opened +- Cover grid displays both albums as distinct entries +- Existing databases migrated safely (constraint changed, rescan needed to split merged data) + + + +After completion, create `.planning/quick/10-diagnose-duplicate-album-merging-bug-alb/10-SUMMARY.md` +