From 34615fffbdb1aa5e0b5ed84ad8094edc969db7af Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 4 May 2026 19:40:27 -0400 Subject: [PATCH] feat(server/m7-380): one-shot MBID backfill worker for existing libraries Adds a boot-time goroutine that walks albums with NULL mbid, re-reads tags from one track per album via dhowden/tag, and persists album + artist MBIDs. Healed albums also get their cover_art_source='none' cleared so the enricher's next batch retries the MBCAA fetch. Caps at 5000 albums per boot to avoid stalling startup on huge libraries. Co-Authored-By: Claude Sonnet 4.6 --- cmd/minstrel/main.go | 10 +++ internal/db/dbq/albums.sql.go | 53 +++++++++++++ internal/db/dbq/covers.sql.go | 14 ++++ internal/db/queries/albums.sql | 20 +++++ internal/db/queries/covers.sql | 8 ++ internal/library/mbidbackfill.go | 123 +++++++++++++++++++++++++++++++ 6 files changed, 228 insertions(+) create mode 100644 internal/library/mbidbackfill.go diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 3905aa42..e5e20cf5 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -83,6 +83,16 @@ func run() error { }) coverEnricher := coverart.NewEnricher(pool, logger.With("component", "coverart"), coverFetcher, cfg.Library.CoverArtFromMBCAA) + // One-shot MBID backfill: walks albums with NULL mbid, reads tags from + // one track per album, persists the MBID. Filling in MBIDs unblocks the + // cover enricher for libraries that were imported before scanner-side + // MBID extraction landed (M7 #380). + go func() { + if _, err := library.BackfillMBIDs(ctx, pool, logger.With("component", "mbid_backfill"), 5000); err != nil { + logger.Warn("mbid backfill failed", "err", err) + } + }() + if cfg.Library.ScanOnStartup && len(cfg.Library.ScanPaths) > 0 { go func() { if _, err := scanner.Scan(ctx); err != nil { diff --git a/internal/db/dbq/albums.sql.go b/internal/db/dbq/albums.sql.go index 2bb5a100..91d8513f 100644 --- a/internal/db/dbq/albums.sql.go +++ b/internal/db/dbq/albums.sql.go @@ -347,6 +347,59 @@ func (q *Queries) ListAlbumsByGenre(ctx context.Context, arg ListAlbumsByGenrePa return items, nil } +const listAlbumsMissingMbidWithTrack = `-- name: ListAlbumsMissingMbidWithTrack :many +SELECT a.id AS album_id, + a.artist_id AS artist_id, + a.title AS title, + t.file_path AS track_file_path + FROM albums a + JOIN LATERAL ( + SELECT file_path + FROM tracks + WHERE album_id = a.id + ORDER BY disc_number NULLS LAST, track_number NULLS LAST, id + LIMIT 1 + ) t ON true + WHERE a.mbid IS NULL + ORDER BY a.created_at ASC + LIMIT $1 +` + +type ListAlbumsMissingMbidWithTrackRow struct { + AlbumID pgtype.UUID + ArtistID pgtype.UUID + Title string + TrackFilePath string +} + +// One-shot MBID backfill: returns each album where mbid IS NULL alongside +// one of its tracks' file_path so the worker can re-read tags. LIMIT +// supplied by caller for batching/progress purposes. +func (q *Queries) ListAlbumsMissingMbidWithTrack(ctx context.Context, limit int32) ([]ListAlbumsMissingMbidWithTrackRow, error) { + rows, err := q.db.Query(ctx, listAlbumsMissingMbidWithTrack, limit) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAlbumsMissingMbidWithTrackRow + for rows.Next() { + var i ListAlbumsMissingMbidWithTrackRow + if err := rows.Scan( + &i.AlbumID, + &i.ArtistID, + &i.Title, + &i.TrackFilePath, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listAlbumsNewest = `-- name: ListAlbumsNewest :many SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source FROM albums ORDER BY created_at DESC LIMIT $1 OFFSET $2 ` diff --git a/internal/db/dbq/covers.sql.go b/internal/db/dbq/covers.sql.go index f661984b..f726cc03 100644 --- a/internal/db/dbq/covers.sql.go +++ b/internal/db/dbq/covers.sql.go @@ -25,6 +25,20 @@ func (q *Queries) ClearAlbumCover(ctx context.Context, id pgtype.UUID) error { return err } +const clearAlbumCoverNone = `-- name: ClearAlbumCoverNone :exec +UPDATE albums + SET cover_art_source = NULL + WHERE id = $1 AND cover_art_source = 'none' +` + +// M7 #380: when MBID backfill heals an album, clear its 'none' cover_art_source +// back to NULL so the enricher's next batch retries the MBCAA fetch. +// Idempotent — only updates rows currently stamped 'none'. +func (q *Queries) ClearAlbumCoverNone(ctx context.Context, id pgtype.UUID) error { + _, err := q.db.Exec(ctx, clearAlbumCoverNone, id) + return err +} + const countAlbumCoverSources = `-- name: CountAlbumCoverSources :one SELECT COUNT(*) FILTER (WHERE cover_art_source IS NULL) ::bigint AS untried, diff --git a/internal/db/queries/albums.sql b/internal/db/queries/albums.sql index 640b8e16..527fb4e5 100644 --- a/internal/db/queries/albums.sql +++ b/internal/db/queries/albums.sql @@ -104,3 +104,23 @@ UPDATE albums SET mbid = $2, updated_at = now() WHERE id = $1 AND mbid IS NULL; + +-- name: ListAlbumsMissingMbidWithTrack :many +-- One-shot MBID backfill: returns each album where mbid IS NULL alongside +-- one of its tracks' file_path so the worker can re-read tags. LIMIT +-- supplied by caller for batching/progress purposes. +SELECT a.id AS album_id, + a.artist_id AS artist_id, + a.title AS title, + t.file_path AS track_file_path + FROM albums a + JOIN LATERAL ( + SELECT file_path + FROM tracks + WHERE album_id = a.id + ORDER BY disc_number NULLS LAST, track_number NULLS LAST, id + LIMIT 1 + ) t ON true + WHERE a.mbid IS NULL + ORDER BY a.created_at ASC + LIMIT $1; diff --git a/internal/db/queries/covers.sql b/internal/db/queries/covers.sql index 1fbc2ec3..10a55410 100644 --- a/internal/db/queries/covers.sql +++ b/internal/db/queries/covers.sql @@ -44,6 +44,14 @@ UPDATE albums cover_art_source = NULL WHERE id = $1; +-- name: ClearAlbumCoverNone :exec +-- M7 #380: when MBID backfill heals an album, clear its 'none' cover_art_source +-- back to NULL so the enricher's next batch retries the MBCAA fetch. +-- Idempotent — only updates rows currently stamped 'none'. +UPDATE albums + SET cover_art_source = NULL + WHERE id = $1 AND cover_art_source = 'none'; + -- name: CountAlbumCoverSources :one -- Diagnostic for the admin overview: counts by source, including NULL. -- Returns one row with named tallies. diff --git a/internal/library/mbidbackfill.go b/internal/library/mbidbackfill.go new file mode 100644 index 00000000..6715be65 --- /dev/null +++ b/internal/library/mbidbackfill.go @@ -0,0 +1,123 @@ +package library + +import ( + "context" + "fmt" + "log/slog" + "os" + + "github.com/dhowden/tag" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// 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. +} + +// BackfillMBIDs walks albums with NULL mbid, opens one track per album, +// extracts MusicBrainz IDs from the tags, and persists them. Also heals +// the artist's mbid when the same tag-read yields one. When a row is +// healed, any 'none' cover_art_source on the album is cleared back to +// NULL so the enricher batch retries. +// +// Intended use: one-shot at server boot. Re-running it after the first +// pass costs the same N file reads but is a no-op for healed rows. +// +// limit caps how many albums are processed in one call. Pass -1 for no +// cap. The caller normally batches (e.g. limit=500 per boot) to avoid +// stalling startup on huge libraries. +func BackfillMBIDs(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, limit int) (BackfillMBIDsResult, error) { + q := dbq.New(pool) + queryLimit := int32(limit) + if limit < 0 { + queryLimit = 1<<31 - 1 // effectively unbounded + } + + rows, err := q.ListAlbumsMissingMbidWithTrack(ctx, queryLimit) + if err != nil { + return BackfillMBIDsResult{}, fmt.Errorf("list missing mbid: %w", err) + } + + var res BackfillMBIDsResult + for i, r := range rows { + if ctx.Err() != nil { + return res, ctx.Err() + } + res.Processed++ + + albumMBID, artistMBID := readMBIDsForFile(r.TrackFilePath, logger) + if albumMBID == "" { + res.Skipped++ + continue + } + + // Heal album. + m := albumMBID + if err := q.SetAlbumMbidIfNull(ctx, dbq.SetAlbumMbidIfNullParams{ + ID: r.AlbumID, Mbid: &m, + }); err != nil { + logger.Warn("mbid backfill: set album mbid failed", + "album_id", r.AlbumID, "err", err) + res.Skipped++ + continue + } + + // Heal artist (if we got one). + if artistMBID != "" { + am := artistMBID + 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) + } + } + + // Clear cover_art_source = 'none' so the cover enricher's next + // batch retries the MBCAA fetch for this album. + if err := q.ClearAlbumCoverNone(ctx, r.AlbumID); err != nil { + logger.Warn("mbid backfill: clear cover none failed", + "album_id", r.AlbumID, "err", err) + } + + res.Healed++ + + // Progress log every 100 healed. + if (i+1)%100 == 0 { + logger.Info("mbid backfill progress", + "processed", res.Processed, "healed", res.Healed, "skipped", res.Skipped) + } + } + + logger.Info("mbid backfill complete", + "processed", res.Processed, + "healed", res.Healed, + "skipped", res.Skipped) + return res, nil +} + +// readMBIDsForFile opens a single audio file, reads tags via dhowden/tag, +// and returns extracted album + artist MBIDs. Returns ("", "") when the +// file can't be opened or the tag read fails — those errors are logged +// at Warn level but not propagated, so a single broken file doesn't +// halt the whole backfill. +func readMBIDsForFile(path string, logger *slog.Logger) (albumMBID, artistMBID string) { + f, err := os.Open(path) + if err != nil { + logger.Warn("mbid backfill: open failed", "path", path, "err", err) + return "", "" + } + defer func() { _ = f.Close() }() + + meta, err := tag.ReadFrom(f) + if err != nil { + logger.Warn("mbid backfill: tag read failed", "path", path, "err", err) + return "", "" + } + return extractMBIDs(meta.Raw()) +}