Files
bvandeusen 005965d6de feat(coverart): #388 remove global cover-art backfill cap
Operator feedback (2026-05-09): the 500-album cap meant cover art rolled
in over many nightly runs even when local sources (sidecar/embedded)
could finish in minutes. Remove the global cap; rely on the existing
per-provider HTTP throttle (coverart httpClient MinInterval) so local
art is disk-speed and remote providers stay TOS-friendly.

- enricher.go / artist_enricher.go: EnrichBatch, EnrichArtistBatch,
  EnrichRetryMissing now treat limit<0 as unbounded (0 still = stage
  disabled). The cap was a SQL LIMIT; unbounded uses max int32.
- main.go: RunScanConfig EnrichCap/ArtistEnrichCap = -1 (unbounded).
- Drop LibraryConfig.CoverArtBackfillCap + the
  MINSTREL_LIBRARY_COVERART_BACKFILL_CAP env var.
- Drop the now-dead coverBackfillCap param threaded through
  server.New + api.Mount + the handlers struct.
- Admin bulk refetch (/api/admin/covers/refetch-missing) now drains
  unbounded; response {queued:int} → {started:bool} (the count is
  unknowable synchronously for a fire-and-forget drain). Web copy +
  client type + Go/web tests updated to match.

No doc refs existed (config.example.yaml / docker-compose / README
never documented the env var).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 18:41:21 -04:00

375 lines
13 KiB
Go

package coverart
import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"path/filepath"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// Enricher orchestrates the sidecar → provider chain → record cycle
// for one or many albums. Owns no DB pool itself; receives one per
// call so the boot backfill, post-scan trigger, and admin endpoints
// share state.
//
// DataDir is the optional managed-cache root. When set, sidecar writes
// that fail (e.g. read-only music mount) fall back to
// <DataDir>/album-art/<album_id>/cover.jpg so enrichment isn't gated
// on the music directory being writable. Empty disables fallback;
// tests use empty since they operate on writable tempdirs.
type Enricher struct {
pool *pgxpool.Pool
logger *slog.Logger
settings *SettingsService
DataDir string
}
// NewEnricher constructs an Enricher.
func NewEnricher(pool *pgxpool.Pool, logger *slog.Logger, settings *SettingsService) *Enricher {
return &Enricher{pool: pool, logger: logger, settings: settings}
}
// EnrichAlbum runs the chain for a single album.
//
// Eligibility:
// - cover_art_source IS NULL → eligible
// - cover_art_source IN (sidecar/embedded/ → SKIP (found; version irrelevant)
// mbcaa/theaudiodb)
// - cover_art_source = 'none' → flip to NULL via ClearAlbumCoverNone,
// then enrich (the DB-level query ListAlbumsMissingCover already
// guards the version check for batch callers; direct callers such
// as RetryAlbum clear via ClearAlbumCover first so we never see
// a current-version 'none' in normal flow)
//
// Chain: sidecar → enabled AlbumCoverProviders in registration order.
// First success returns. All-ErrNotFound settles to 'none'. Any
// ErrTransient leaves the row NULL for next-pass retry.
func (e *Enricher) EnrichAlbum(ctx context.Context, albumID pgtype.UUID) error {
q := dbq.New(e.pool)
row, err := q.GetAlbumWithFirstTrackPath(ctx, albumID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return nil // album was deleted between queueing and processing
}
return fmt.Errorf("get album: %w", err)
}
currentVersion := e.settings.CurrentVersion()
// Eligibility check.
currentSource := ""
if row.CoverArtSource != nil {
currentSource = *row.CoverArtSource
}
switch currentSource {
case "sidecar", "embedded", "mbcaa", "theaudiodb":
return nil // already found; version irrelevant
case "none":
// Flip to NULL and proceed. ListAlbumsMissingCover already
// filters to stale-version 'none' rows for batch callers;
// RetryAlbum clears to NULL before calling us.
if err := q.ClearAlbumCoverNone(ctx, albumID); err != nil {
return fmt.Errorf("clear stale none: %w", err)
}
}
// Sidecar layer (always tried first; not a Provider).
if row.TrackFilePath != nil && *row.TrackFilePath != "" {
albumDir := filepath.Dir(*row.TrackFilePath)
if path := FindSidecar(albumDir); path != "" {
return e.recordAlbumCover(ctx, albumID, path, "sidecar", currentVersion)
}
}
// Embedded layer: read the picture stored inside the audio file's
// tag block (ID3v2 APIC, FLAC METADATA_BLOCK_PICTURE, MP4 covr).
// Picard/Lidarr-tagged libraries typically carry the canonical art
// here, so this beats every external provider for accuracy and
// avoids the network round-trip.
if row.TrackFilePath != nil && *row.TrackFilePath != "" {
body, eerr := ExtractEmbeddedAlbumArt(*row.TrackFilePath)
if eerr == nil {
dest, werr := e.writeAlbumArt(albumID, *row.TrackFilePath, body)
if werr != nil {
e.logger.Error("coverart: embedded write failed; leaving NULL",
"album_id", uuidString(albumID), "err", werr)
return nil
}
return e.recordAlbumCover(ctx, albumID, dest, "embedded", currentVersion)
}
if !errors.Is(eerr, ErrNoEmbeddedArt) {
e.logger.Debug("coverart: embedded extract failed; falling through",
"album_id", uuidString(albumID), "err", eerr)
}
// No embedded art (or extract failed): continue to provider chain.
}
// Build the ref. Every provider receives it and decides whether it
// can act on the fields available (MBID-only providers return
// ErrNotFound when MBID is empty; name-based providers fall back to
// ArtistName + AlbumTitle).
ref := AlbumRef{
ArtistName: row.ArtistName,
AlbumTitle: row.Title,
}
if row.Mbid != nil {
ref.MBID = *row.Mbid
}
// Provider chain. allWere404 tracks whether every provider
// returned ErrNotFound — only then do we settle to 'none'. Any
// transient failure leaves the row NULL for next-pass retry.
allWere404 := true
for _, provider := range e.settings.EnabledAlbumProviders() {
body, perr := provider.FetchAlbumCover(ctx, ref)
if perr == nil {
if row.TrackFilePath == nil || *row.TrackFilePath == "" {
// Defensive: shouldn't happen if MBID was set on an
// album with no tracks, but record 'none' rather than
// crash.
return e.recordAlbumCover(ctx, albumID, "", "none", currentVersion)
}
dest, werr := e.writeAlbumArt(albumID, *row.TrackFilePath, body)
if werr != nil {
e.logger.Error("coverart: write file failed; leaving NULL",
"album_id", uuidString(albumID),
"provider", provider.ID(), "err", werr)
return nil
}
return e.recordAlbumCover(ctx, albumID, dest, provider.ID(), currentVersion)
}
if !errors.Is(perr, ErrNotFound) {
allWere404 = false
e.logger.Warn("coverart: provider fetch failed; trying next",
"album_id", uuidString(albumID),
"provider", provider.ID(), "err", perr)
}
// ErrNotFound: continue to next provider; allWere404 stays true.
}
if allWere404 {
return e.recordAlbumCover(ctx, albumID, "", "none", currentVersion)
}
return nil // at least one transient failure; leave NULL for retry
}
// writeAlbumArt persists art bytes for an album, preferring the sidecar
// location next to the track file. Falls back to the managed cache
// (<DataDir>/album-art/<id>/cover.jpg) when the sidecar location
// isn't writable (e.g. read-only music mount in a container) and
// DataDir is set. Returns the path that was actually written.
//
// Used by both the embedded-art layer and the external provider
// chain — both produce the same dest-and-fallback semantics.
func (e *Enricher) writeAlbumArt(albumID pgtype.UUID, trackFilePath string, body []byte) (string, error) {
albumDir := filepath.Dir(trackFilePath)
dest := filepath.Join(albumDir, "cover.jpg")
werr := os.WriteFile(dest, body, 0o644)
if werr != nil && e.DataDir != "" {
fallbackDir := filepath.Join(e.DataDir, "album-art", uuidString(albumID))
if mkErr := os.MkdirAll(fallbackDir, 0o755); mkErr != nil {
return "", fmt.Errorf("managed-cache mkdir failed (sidecar: %v): %w", werr, mkErr)
}
dest = filepath.Join(fallbackDir, "cover.jpg")
werr = os.WriteFile(dest, body, 0o644)
}
if werr != nil {
return "", fmt.Errorf("write cover: %w", werr)
}
return dest, nil
}
// recordAlbumCover writes the new cover_art_path / cover_art_source /
// cover_art_sources_version atomically.
func (e *Enricher) recordAlbumCover(ctx context.Context, albumID pgtype.UUID, path, source string, version int32) error {
q := dbq.New(e.pool)
src := source
var pathArg *string
if path != "" {
pathArg = &path
}
return q.SetAlbumCoverWithVersion(ctx, dbq.SetAlbumCoverWithVersionParams{
ID: albumID,
CoverArtPath: pathArg,
CoverArtSource: &src,
CoverArtSourcesVersion: version,
})
}
// EnrichBatch drains up to limit albums whose eligibility loop
// determines they need processing. Iterates serially. Returns
// per-batch tallies for the orchestrator's scan-run record.
//
// Emits a single Info log line at end-of-batch with a category
// breakdown (succeeded / settled-none / left-NULL / errored). The
// JSON tally written to scan_runs keeps the existing
// processed/succeeded/failed shape for backwards compat with the
// admin overview UI; the log line is the operator's diagnostic.
// limit semantics: 0 = stage disabled (skip); >0 = bounded batch;
// <0 = unbounded — walk every album needing art. The global cap was
// removed (#388); external providers self-throttle via their
// per-provider httpClient MinInterval, local sources run at disk speed.
func (e *Enricher) EnrichBatch(ctx context.Context, limit int,
progressCb func(processed, succeeded, failed int)) (processed, succeeded, failed int, err error) {
if limit == 0 {
return 0, 0, 0, nil
}
queryLimit := int32(limit)
if limit < 0 {
queryLimit = 1<<31 - 1 // unbounded
}
q := dbq.New(e.pool)
rows, qerr := q.ListAlbumsMissingCover(ctx, dbq.ListAlbumsMissingCoverParams{
CoverArtSourcesVersion: e.settings.CurrentVersion(),
Limit: queryLimit,
})
if qerr != nil {
return 0, 0, 0, fmt.Errorf("list missing: %w", qerr)
}
var settledNone, leftNull, errored int
for _, id := range rows {
if ctx.Err() != nil {
e.logBatchSummary("album", len(rows), processed, succeeded, settledNone, leftNull, errored)
return processed, succeeded, failed, ctx.Err()
}
processed++
if eerr := e.EnrichAlbum(ctx, id); eerr != nil {
e.logger.Warn("coverart: batch entry failed",
"album_id", uuidString(id), "err", eerr)
failed++
errored++
if progressCb != nil {
progressCb(processed, succeeded, failed)
}
continue
}
// Re-read to classify.
row, rerr := q.GetAlbumWithFirstTrackPath(ctx, id)
if rerr != nil {
failed++
errored++
if progressCb != nil {
progressCb(processed, succeeded, failed)
}
continue
}
switch {
case row.CoverArtSource != nil &&
(*row.CoverArtSource == "sidecar" ||
*row.CoverArtSource == "mbcaa" ||
*row.CoverArtSource == "embedded" ||
*row.CoverArtSource == "theaudiodb"):
succeeded++
case row.CoverArtSource != nil && *row.CoverArtSource == "none":
settledNone++
failed++
default:
leftNull++
failed++
}
if progressCb != nil {
progressCb(processed, succeeded, failed)
}
}
e.logBatchSummary("album", len(rows), processed, succeeded, settledNone, leftNull, errored)
return processed, succeeded, failed, nil
}
// logBatchSummary emits a single Info line at end-of-batch with the
// full category breakdown. Operator-side diagnostic; the
// processed/succeeded/failed tally collapses too much to debug
// "0 successes" symptoms by itself.
func (e *Enricher) logBatchSummary(stage string, eligible, processed, succeeded, settledNone, leftNull, errored int) {
e.logger.Info("coverart: enrichment batch complete",
"stage", stage,
"eligible", eligible,
"processed", processed,
"succeeded", succeeded,
"settled_none", settledNone,
"left_null", leftNull,
"errored", errored)
}
// EnrichRetryMissing drains both NULL and 'none' rows. Used by the
// admin bulk-retry endpoint. Clears 'none' to NULL first so
// EnrichAlbum picks them up.
// limit: 0 = no-op; >0 = bounded; <0 = unbounded (retry every
// missing/none album). Admin bulk-retry passes <0 post-#388.
func (e *Enricher) EnrichRetryMissing(ctx context.Context, limit int) (int, error) {
if limit == 0 {
return 0, nil
}
queryLimit := int32(limit)
if limit < 0 {
queryLimit = 1<<31 - 1 // unbounded
}
q := dbq.New(e.pool)
rows, err := q.ListAlbumsRetryMissing(ctx, queryLimit)
if err != nil {
return 0, fmt.Errorf("list retry missing: %w", err)
}
for _, r := range rows {
if err := q.ClearAlbumCover(ctx, r.ID); err != nil {
e.logger.Warn("coverart: clear before retry failed",
"album_id", uuidString(r.ID), "err", err)
}
}
processed := 0
for _, r := range rows {
if ctx.Err() != nil {
return processed, ctx.Err()
}
if err := e.EnrichAlbum(ctx, r.ID); err != nil {
e.logger.Warn("coverart: retry-missing entry failed",
"album_id", uuidString(r.ID), "err", err)
}
processed++
}
return processed, nil
}
// RetryAlbum is the admin per-album retry path.
func (e *Enricher) RetryAlbum(ctx context.Context, albumID pgtype.UUID) error {
q := dbq.New(e.pool)
row, err := q.GetAlbumWithFirstTrackPath(ctx, albumID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return ErrNotFound
}
return fmt.Errorf("get album: %w", err)
}
// Delete the file only if we previously wrote it (mbcaa or
// theaudiodb). Operator sidecars stay; embedded extracts (future)
// live in a managed cache with its own lifecycle.
if row.CoverArtSource != nil &&
(*row.CoverArtSource == "mbcaa" || *row.CoverArtSource == "theaudiodb") &&
row.CoverArtPath != nil && *row.CoverArtPath != "" {
if err := os.Remove(*row.CoverArtPath); err != nil && !os.IsNotExist(err) {
e.logger.Warn("coverart: remove old file failed",
"album_id", uuidString(albumID),
"path", *row.CoverArtPath, "err", err)
}
}
if err := q.ClearAlbumCover(ctx, albumID); err != nil {
return fmt.Errorf("clear cover: %w", err)
}
return e.EnrichAlbum(ctx, albumID)
}
// uuidString carries over from the previous enricher.
func uuidString(u pgtype.UUID) string {
if !u.Valid {
return "<nil>"
}
return fmt.Sprintf("%x-%x-%x-%x-%x",
u.Bytes[0:4], u.Bytes[4:6], u.Bytes[6:8], u.Bytes[8:10], u.Bytes[10:16])
}