fix(library): extract recording MBID → tracks.mbid; unblock LB similarity

tracks.mbid was 100% NULL: the scanner only extracted album + artist MBIDs, never the recording MBID. The ListenBrainz similarity worker is gated on tracks.mbid IS NOT NULL, so track_similarity could never populate — starving For-You/radio's strongest candidate source (lb_similar) on every library. - mbids.go: add extractRecordingMBID (mbz.Recording / Picard musicbrainz_recordingid). Separate fn so extractMBIDs' signature + unit tests stay untouched. - scanner.go: persist recording MBID via UpsertTrack (heals on the file_path conflict, so re-scans backfill for free). - BackfillTrackMBIDs: one-shot pass mirroring BackfillMBIDs, wired as scan Stage 2b (idempotent via SetTrackMbidIfNull, gated by BackfillCap, log-only progress). - migration 0029: tracks_mbid_unique (0002, written when the column was always NULL) wrongly assumes one MBID == one track. A recording appears on multiple releases, so rows legitimately share a recording MBID. Replace with a non-unique partial index. Zero-risk: column is 100% NULL at migration time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 13:49:38 -04:00
parent 2e7b81fdfe
commit e2432caa65
8 changed files with 214 additions and 0 deletions
@@ -376,6 +376,41 @@ func (q *Queries) ListTracksByAlbum(ctx context.Context, arg ListTracksByAlbumPa
 	return items, nil
 }

+const listTracksMissingMbidWithPath = `-- name: ListTracksMissingMbidWithPath :many
+SELECT id, file_path
+  FROM tracks
+ WHERE mbid IS NULL
+ ORDER BY id
+ LIMIT $1
+`
+
+type ListTracksMissingMbidWithPathRow struct {
+	ID       pgtype.UUID
+	FilePath string
+}
+
+// Track recording-MBID backfill: tracks with NULL mbid that still have
+// a file to re-read. $1 caps the batch (mirrors the album backfill).
+func (q *Queries) ListTracksMissingMbidWithPath(ctx context.Context, limit int32) ([]ListTracksMissingMbidWithPathRow, error) {
+	rows, err := q.db.Query(ctx, listTracksMissingMbidWithPath, limit)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	var items []ListTracksMissingMbidWithPathRow
+	for rows.Next() {
+		var i ListTracksMissingMbidWithPathRow
+		if err := rows.Scan(&i.ID, &i.FilePath); err != nil {
+			return nil, err
+		}
+		items = append(items, i)
+	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+	return items, nil
+}
+
 const searchTracks = `-- name: SearchTracks :many
 SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at FROM tracks
 WHERE title ILIKE '%' || $1::text || '%'
@@ -437,6 +472,24 @@ func (q *Queries) SearchTracks(ctx context.Context, arg SearchTracksParams) ([]T
 	return items, nil
 }

+const setTrackMbidIfNull = `-- name: SetTrackMbidIfNull :exec
+UPDATE tracks
+   SET mbid = $2, updated_at = now()
+ WHERE id = $1 AND mbid IS NULL
+`
+
+type SetTrackMbidIfNullParams struct {
+	ID   pgtype.UUID
+	Mbid *string
+}
+
+// Heal a track's recording MBID only while still NULL — idempotent, so
+// re-running the backfill is a no-op for already-healed rows.
+func (q *Queries) SetTrackMbidIfNull(ctx context.Context, arg SetTrackMbidIfNullParams) error {
+	_, err := q.db.Exec(ctx, setTrackMbidIfNull, arg.ID, arg.Mbid)
+	return err
+}
+
 const upsertTrack = `-- name: UpsertTrack :one
 INSERT INTO tracks (
    title, album_id, artist_id, track_number, disc_number,
@@ -0,0 +1,6 @@
+-- Reverts to the unique partial index. This FAILS if any recording
+-- MBID is shared across releases in the library (the exact case 0029
+-- exists to allow) — dedupe tracks.mbid before rolling back.
+
+DROP INDEX IF EXISTS tracks_mbid_idx;
+CREATE UNIQUE INDEX tracks_mbid_unique ON tracks (mbid) WHERE mbid IS NOT NULL;
@@ -0,0 +1,15 @@
+-- tracks.mbid holds the MusicBrainz *recording* MBID — the id the
+-- ListenBrainz similarity pipeline (ListPlayedTracksNeedingSimilarity,
+-- GetTracksByMBIDs) matches on. A single recording legitimately appears
+-- on multiple releases (album + compilation + single), so multiple
+-- tracks rows share one recording MBID.
+--
+-- tracks_mbid_unique (added in 0002, when this column was always NULL
+-- and never populated) wrongly assumes one MBID == one track. Now that
+-- the scanner extracts the recording MBID, that uniqueness throws
+-- 23505 for any recording present on >1 release. Replace it with a
+-- non-unique partial index serving the same lookups. Zero-risk: the
+-- column is 100% NULL at migration time.
+
+DROP INDEX IF EXISTS tracks_mbid_unique;
+CREATE INDEX IF NOT EXISTS tracks_mbid_idx ON tracks (mbid) WHERE mbid IS NOT NULL;
@@ -19,6 +19,22 @@ ON CONFLICT (file_path) DO UPDATE SET
    updated_at = now()
 RETURNING *;

+-- name: ListTracksMissingMbidWithPath :many
+-- Track recording-MBID backfill: tracks with NULL mbid that still have
+-- a file to re-read. $1 caps the batch (mirrors the album backfill).
+SELECT id, file_path
+  FROM tracks
+ WHERE mbid IS NULL
+ ORDER BY id
+ LIMIT $1;
+
+-- name: SetTrackMbidIfNull :exec
+-- Heal a track's recording MBID only while still NULL — idempotent, so
+-- re-running the backfill is a no-op for already-healed rows.
+UPDATE tracks
+   SET mbid = $2, updated_at = now()
+ WHERE id = $1 AND mbid IS NULL;
+
 -- name: GetTrackByID :one
 SELECT * FROM tracks WHERE id = $1;

@@ -167,3 +167,96 @@ func readMBIDsForFile(path string, logger *slog.Logger) (albumMBID, artistMBID s
 	}
 	return extractMBIDs(meta)
 }
+
+// BackfillTrackMBIDsResult tallies the track recording-MBID backfill.
+type BackfillTrackMBIDsResult struct {
+	Processed int // Tracks considered (mbid IS NULL).
+	Healed    int // Tracks whose recording MBID we set.
+	Skipped   int // Tracks whose file was missing / untagged / write failed.
+}
+
+// BackfillTrackMBIDs walks tracks with NULL mbid, re-reads each file's
+// tags, and persists the MusicBrainz recording MBID. This is the
+// counterpart to BackfillMBIDs (which heals album/artist MBIDs); it
+// unblocks the ListenBrainz similarity pipeline, which is gated on
+// tracks.mbid IS NOT NULL.
+//
+// Idempotent via SetTrackMbidIfNull: re-running costs the same N file
+// reads but is a no-op for already-healed rows. limit caps one call;
+// pass -1 for unbounded (caller normally batches per scan).
+func BackfillTrackMBIDs(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger,
+	limit int, progressCb func(BackfillTrackMBIDsResult)) (BackfillTrackMBIDsResult, error) {
+	q := dbq.New(pool)
+	queryLimit := int32(limit)
+	if limit < 0 {
+		queryLimit = 1<<31 - 1 // effectively unbounded
+	}
+
+	rows, err := q.ListTracksMissingMbidWithPath(ctx, queryLimit)
+	if err != nil {
+		return BackfillTrackMBIDsResult{}, fmt.Errorf("list tracks missing mbid: %w", err)
+	}
+
+	var res BackfillTrackMBIDsResult
+	for i, r := range rows {
+		if ctx.Err() != nil {
+			return res, ctx.Err()
+		}
+		res.Processed++
+
+		rec := readRecordingMBIDForFile(r.FilePath, logger)
+		if rec == "" {
+			res.Skipped++
+			if progressCb != nil {
+				progressCb(res)
+			}
+			continue
+		}
+
+		m := rec
+		if uerr := q.SetTrackMbidIfNull(ctx, dbq.SetTrackMbidIfNullParams{
+			ID: r.ID, Mbid: &m,
+		}); uerr != nil {
+			logger.Warn("track mbid backfill: set failed",
+				"track_id", r.ID, "err", uerr)
+			res.Skipped++
+			if progressCb != nil {
+				progressCb(res)
+			}
+			continue
+		}
+
+		res.Healed++
+		if (i+1)%500 == 0 {
+			logger.Info("track mbid backfill progress",
+				"processed", res.Processed, "healed", res.Healed, "skipped", res.Skipped)
+		}
+		if progressCb != nil {
+			progressCb(res)
+		}
+	}
+
+	logger.Info("track mbid backfill complete",
+		"processed", res.Processed, "healed", res.Healed, "skipped", res.Skipped)
+	return res, nil
+}
+
+// readRecordingMBIDForFile opens one audio file and returns its
+// MusicBrainz recording MBID, or "" when the file can't be opened, the
+// tag read fails, or the tag is absent. Errors are logged at Warn, not
+// propagated — one broken file must not halt the backfill.
+func readRecordingMBIDForFile(path string, logger *slog.Logger) string {
+	f, err := os.Open(path)
+	if err != nil {
+		logger.Warn("track mbid backfill: open failed", "path", path, "err", err)
+		return ""
+	}
+	defer func() { _ = f.Close() }()
+
+	meta, err := tag.ReadFrom(f)
+	if err != nil {
+		logger.Warn("track mbid backfill: tag read failed", "path", path, "err", err)
+		return ""
+	}
+	return extractRecordingMBID(meta)
+}
@@ -27,6 +27,19 @@ func extractMBIDs(m tag.Metadata) (albumMBID, artistMBID string) {
 	return cleanMBID(info.Get(mbz.Album)), cleanMBID(info.Get(mbz.Artist))
 }

+// extractRecordingMBID reads the MusicBrainz *recording* ID — Picard's
+// musicbrainz_recordingid, surfaced by dhowden/tag as mbz.Recording
+// (tag display name "MusicBrainz Track Id"). This is the id ListenBrainz
+// /explore/similar-recordings keys on and what tracks.mbid stores.
+//
+// Deliberately NOT mbz.Track ("MusicBrainz Release Track Id"): that is
+// per-release-track, whereas similarity is per-recording. Kept separate
+// from extractMBIDs so its existing (album, artist) signature and unit
+// tests stay untouched.
+func extractRecordingMBID(m tag.Metadata) string {
+	return cleanMBID(mbz.Extract(m).Get(mbz.Recording))
+}
+
 func cleanMBID(s string) string {
 	if s == "" {
 		return ""
@@ -142,6 +142,7 @@ func (s *Scanner) scanFile(ctx context.Context, q *dbq.Queries, path string, sta
 		return fmt.Errorf("tag read: %w", err)
 	}
 	albumMBID, artistMBID := extractMBIDs(meta)
+	recordingMBID := extractRecordingMBID(meta)

 	artistName := meta.Artist()
 	if artistName == "" {
@@ -195,6 +196,13 @@ func (s *Scanner) scanFile(ctx context.Context, q *dbq.Queries, path string, sta
 	if g := meta.Genre(); g != "" {
 		params.Genre = &g
 	}
+	// Recording MBID feeds the ListenBrainz similarity pipeline.
+	// UpsertTrack heals mbid on the file_path conflict, so a re-scan
+	// of a previously-untagged-into-DB track backfills it for free.
+	if recordingMBID != "" {
+		m := recordingMBID
+		params.Mbid = &m
+	}

 	track, err := q.UpsertTrack(ctx, params)
 	if err != nil {
@@ -154,6 +154,16 @@ func RunScan(
 		}
 	}

+	// Stage 2b: track recording-MBID backfill. Unblocks the ListenBrainz
+	// similarity pipeline (gated on tracks.mbid IS NOT NULL). Log-only
+	// progress — no scan_runs jsonb column to avoid a schema addition;
+	// it's idempotent and reruns each scan until every file is healed.
+	if cfg.BackfillCap != 0 {
+		if _, tberr := BackfillTrackMBIDs(ctx, pool, logger, cfg.BackfillCap, nil); tberr != nil {
+			captureErr("track_mbid_backfill", tberr)
+		}
+	}
+
 	// Stage 3: cover enrichment.
 	if enricher != nil && cfg.EnrichCap != 0 {
 		var lastP, lastS, lastF int