diff --git a/internal/library/mbidbackfill.go b/internal/library/mbidbackfill.go index 84040595..804ad39c 100644 --- a/internal/library/mbidbackfill.go +++ b/internal/library/mbidbackfill.go @@ -2,21 +2,36 @@ package library import ( "context" + "errors" "fmt" "log/slog" "os" "github.com/dhowden/tag" + "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) +// pgUniqueViolation is the SQLSTATE for a unique-constraint conflict. +const pgUniqueViolation = "23505" + +// isUniqueViolation reports whether err is a Postgres unique-constraint +// conflict. Used to distinguish "another row already owns this MBID" +// (a duplicate album/artist row in our DB, the operator should merge +// it) from genuine write failures. +func isUniqueViolation(err error) bool { + var pgErr *pgconn.PgError + return errors.As(err, &pgErr) && pgErr.Code == pgUniqueViolation +} + // BackfillMBIDsResult tallies what the one-shot worker did. type BackfillMBIDsResult struct { - Processed int // Albums considered (mbid IS NULL). - Healed int // Albums whose mbid we set. - Skipped int // Albums where the file was missing or had no MBID tag. + Processed int // Albums considered (mbid IS NULL). + Healed int // Albums whose mbid we set. + Skipped int // Albums where the file was missing or had no MBID tag. + Duplicates int // Albums whose mbid is already held by another row (operator-resolvable). } // BackfillMBIDs walks albums with NULL mbid, opens one track per album, @@ -61,6 +76,17 @@ func BackfillMBIDs(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, if err := q.SetAlbumMbidIfNull(ctx, dbq.SetAlbumMbidIfNullParams{ ID: r.AlbumID, Mbid: &m, }); err != nil { + if isUniqueViolation(err) { + // Another album row already owns this MBID — i.e. our DB + // has two rows for the same MusicBrainz release (split by + // title/artist disagreements during the pre-MBID scan). + // Operator must merge them; for now, leave this row's + // mbid NULL and skip the artist + cover-clear steps. + logger.Info("mbid backfill: duplicate album mbid (canonical row already owns it)", + "album_id", r.AlbumID, "mbid", albumMBID) + res.Duplicates++ + continue + } logger.Warn("mbid backfill: set album mbid failed", "album_id", r.AlbumID, "err", err) res.Skipped++ @@ -73,8 +99,13 @@ func BackfillMBIDs(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, if err := q.SetArtistMbidIfNull(ctx, dbq.SetArtistMbidIfNullParams{ ID: r.ArtistID, Mbid: &am, }); err != nil { - logger.Warn("mbid backfill: set artist mbid failed", - "artist_id", r.ArtistID, "err", err) + if isUniqueViolation(err) { + logger.Info("mbid backfill: duplicate artist mbid (canonical row already owns it)", + "artist_id", r.ArtistID, "mbid", artistMBID) + } else { + logger.Warn("mbid backfill: set artist mbid failed", + "artist_id", r.ArtistID, "err", err) + } } } @@ -90,14 +121,16 @@ func BackfillMBIDs(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, // Progress log every 100 healed. if (i+1)%100 == 0 { logger.Info("mbid backfill progress", - "processed", res.Processed, "healed", res.Healed, "skipped", res.Skipped) + "processed", res.Processed, "healed", res.Healed, + "skipped", res.Skipped, "duplicates", res.Duplicates) } } logger.Info("mbid backfill complete", "processed", res.Processed, "healed", res.Healed, - "skipped", res.Skipped) + "skipped", res.Skipped, + "duplicates", res.Duplicates) return res, nil } diff --git a/internal/library/mbids_test.go b/internal/library/mbids_test.go index 33494bf7..ccafeb32 100644 --- a/internal/library/mbids_test.go +++ b/internal/library/mbids_test.go @@ -15,21 +15,21 @@ type stubMeta struct { raw map[string]interface{} } -func (s stubMeta) Format() tag.Format { return s.format } -func (s stubMeta) FileType() tag.FileType { return tag.UnknownFileType } -func (s stubMeta) Title() string { return "" } -func (s stubMeta) Album() string { return "" } -func (s stubMeta) Artist() string { return "" } -func (s stubMeta) AlbumArtist() string { return "" } -func (s stubMeta) Composer() string { return "" } -func (s stubMeta) Year() int { return 0 } -func (s stubMeta) Genre() string { return "" } -func (s stubMeta) Track() (int, int) { return 0, 0 } -func (s stubMeta) Disc() (int, int) { return 0, 0 } -func (s stubMeta) Picture() *tag.Picture { return nil } -func (s stubMeta) Lyrics() string { return "" } -func (s stubMeta) Comment() string { return "" } -func (s stubMeta) Raw() map[string]interface{} { return s.raw } +func (s stubMeta) Format() tag.Format { return s.format } +func (s stubMeta) FileType() tag.FileType { return tag.UnknownFileType } +func (s stubMeta) Title() string { return "" } +func (s stubMeta) Album() string { return "" } +func (s stubMeta) Artist() string { return "" } +func (s stubMeta) AlbumArtist() string { return "" } +func (s stubMeta) Composer() string { return "" } +func (s stubMeta) Year() int { return 0 } +func (s stubMeta) Genre() string { return "" } +func (s stubMeta) Track() (int, int) { return 0, 0 } +func (s stubMeta) Disc() (int, int) { return 0, 0 } +func (s stubMeta) Picture() *tag.Picture { return nil } +func (s stubMeta) Lyrics() string { return "" } +func (s stubMeta) Comment() string { return "" } +func (s stubMeta) Raw() map[string]interface{} { return s.raw } func TestExtractMBIDs_Vorbis(t *testing.T) { // dhowden lowercases all Vorbis comment keys at parse time. diff --git a/internal/library/scanner.go b/internal/library/scanner.go index d3753269..7381f817 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -240,8 +240,15 @@ func (s *Scanner) resolveAlbum(ctx context.Context, q *dbq.Queries, artistID pgt ID: existing.ID, Mbid: &m, }); uerr != nil { - s.logger.Warn("library scan: heal album mbid failed", - "album_id", existing.ID, "err", uerr) + if isUniqueViolation(uerr) { + // Another album row already owns this MBID — duplicate + // release in the DB. Leave NULL; operator merges later. + s.logger.Info("library scan: duplicate album mbid (canonical row already owns it)", + "album_id", existing.ID, "mbid", mbid) + } else { + s.logger.Warn("library scan: heal album mbid failed", + "album_id", existing.ID, "err", uerr) + } } else { existing.Mbid = &m } diff --git a/internal/library/scanrun.go b/internal/library/scanrun.go index 1fa3fa9f..a27b7f56 100644 --- a/internal/library/scanrun.go +++ b/internal/library/scanrun.go @@ -27,9 +27,10 @@ type LibraryStageTallies struct { // MBIDBackfillStageTallies wires BackfillMBIDsResult into the scan_runs jsonb column. type MBIDBackfillStageTallies struct { - Processed int `json:"processed"` - Healed int `json:"healed"` - Skipped int `json:"skipped"` + Processed int `json:"processed"` + Healed int `json:"healed"` + Skipped int `json:"skipped"` + Duplicates int `json:"duplicates"` } // CoverEnrichStageTallies wires EnrichBatch tallies into the scan_runs jsonb column. @@ -104,7 +105,8 @@ func RunScan( captureErr("mbid_backfill", bferr) } blob, _ := json.Marshal(MBIDBackfillStageTallies{ - Processed: bfRes.Processed, Healed: bfRes.Healed, Skipped: bfRes.Skipped, + Processed: bfRes.Processed, Healed: bfRes.Healed, + Skipped: bfRes.Skipped, Duplicates: bfRes.Duplicates, }) if uerr := q.UpdateScanRunMbidBackfill(ctx, dbq.UpdateScanRunMbidBackfillParams{ ID: row.ID, MbidBackfill: blob, diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index 29d0b111..815badbd 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -196,6 +196,7 @@ export type ScanStageMbidBackfill = { processed: number; healed: number; skipped: number; + duplicates?: number; }; export type ScanStageCoverEnrich = { diff --git a/web/src/routes/admin/+page.svelte b/web/src/routes/admin/+page.svelte index aed8e7d5..09460146 100644 --- a/web/src/routes/admin/+page.svelte +++ b/web/src/routes/admin/+page.svelte @@ -360,6 +360,7 @@
{scanInFlight ? 'Pending…' : 'Skipped.'}