bvandeusen/minstrel
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 38 Wiki Activity

fix(server,web/m7-382): handle duplicate-MBID conflicts during backfill

Browse Source 
Forward-fix from running the prior two commits in production. Backfill
on a real library exposed two further issues:

(1) gofmt -s flagged the column-aligned stubMeta methods in
    mbids_test.go. Switched to the gofmt-canonical single-space form.

(2) Many albums failed to heal with SQLSTATE 23505 (unique constraint
    albums_mbid_unique). Cause: the operator's library has duplicate
    album rows in the DB — same MusicBrainz release, split into
    multiple rows by the pre-MBID scanner because of subtle title or
    artist disagreements between files. When backfill now correctly
    extracts the MBID, two rows want to claim the same one and the
    partial unique index rejects the second.

    The conflict is correct DB behavior — we shouldn't have two rows
    with the same MBID — but it's not a write failure to alarm the
    operator about. Detect 23505 specifically:
      - downgrade the log line from Warn to Info
      - track these in a new BackfillMBIDsResult.Duplicates counter
        (separate from Skipped, which retains its "no MBID in tag"
        meaning)
      - leave the duplicate row's mbid NULL; merging duplicates is a
        separate (future) operator workflow

    Same handling threaded through the scanner heal path so a regular
    rescan doesn't generate the noise either.

    JSON tally + admin UI gain a "Duplicates" line so the operator
    can see how many duplicate rows their library carries.

Follow-up scope (separate task): a duplicate-album merge UX that
reparents the duplicate's tracks to the canonical row and deletes
the orphaned album row.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Van Deusen
2026-05-04 21:45:15 -04:00
parent cc12ab7e96
commit 4481874482
6 changed files with 72 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/library/mbidbackfill.go
+40 -7
View File
@@ -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
}
internal/library/mbids_test.go
+15 -15
View File
@@ -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.
internal/library/scanner.go
+9 -2
View File
@@ -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
}
internal/library/scanrun.go
+6 -4
View File
@@ -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,
web/src/lib/api/admin.ts
+1
View File
@@ -196,6 +196,7 @@ export type ScanStageMbidBackfill = {
processed: number;
healed: number;
skipped: number;
duplicates?: number;
};
export type ScanStageCoverEnrich = {
web/src/routes/admin/+page.svelte
+1
View File
@@ -360,6 +360,7 @@
<div class="flex justify-between"><dt class="text-text-secondary">Processed</dt><dd>{scan.mbid_backfill.processed}</dd></div>
<div class="flex justify-between"><dt class="text-text-secondary">Healed</dt><dd>{scan.mbid_backfill.healed}</dd></div>
<div class="flex justify-between"><dt class="text-text-secondary">Skipped</dt><dd>{scan.mbid_backfill.skipped}</dd></div>
<div class="flex justify-between"><dt class="text-text-secondary">Duplicates</dt><dd>{scan.mbid_backfill.duplicates ?? 0}</dd></div>
</dl>
{:else}
<p class="mt-2 text-sm text-text-muted">{scanInFlight ? 'Pending…' : 'Skipped.'}</p>