diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 29b9e11c..84a198d7 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -105,8 +105,8 @@ func run() error { logger.With("component", "scan_run"), library.RunScanConfig{ BackfillCap: 5000, - EnrichCap: cfg.Library.CoverArtBackfillCap, - ArtistEnrichCap: cfg.Library.CoverArtBackfillCap, + EnrichCap: -1, // #388: no global cap; per-provider rate limits + ArtistEnrichCap: -1, DataDir: cfg.Storage.DataDir, }, ); err != nil { @@ -122,8 +122,8 @@ func run() error { logger.With("component", "scan_run"), library.RunScanConfig{ BackfillCap: 5000, - EnrichCap: cfg.Library.CoverArtBackfillCap, - ArtistEnrichCap: cfg.Library.CoverArtBackfillCap, + EnrichCap: -1, // #388: no global cap; per-provider rate limits + ArtistEnrichCap: -1, DataDir: cfg.Storage.DataDir, }, ); err != nil { @@ -194,8 +194,8 @@ func run() error { scanCfg := library.RunScanConfig{ BackfillCap: 5000, - EnrichCap: cfg.Library.CoverArtBackfillCap, - ArtistEnrichCap: cfg.Library.CoverArtBackfillCap, + EnrichCap: -1, // #388: no global cap; per-provider rate limits + ArtistEnrichCap: -1, DataDir: cfg.Storage.DataDir, } scheduler := library.NewScheduler(pool, logger.With("component", "scheduler"), @@ -203,7 +203,7 @@ func run() error { scheduler.Start(ctx) srv := server.New(logger, pool, scanner, subsonic.Config{ AllowPlaintextPassword: cfg.Subsonic.AllowPlaintextPassword, - }, cfg.Events, cfg.Recommendation, cfg.Storage.DataDir, cfg.Branding, coverEnricher, cfg.Library.CoverArtBackfillCap, coverSettings, scanner, scanCfg, scheduler) + }, cfg.Events, cfg.Recommendation, cfg.Storage.DataDir, cfg.Branding, coverEnricher, coverSettings, scanner, scanCfg, scheduler) srv.Bus = bus srv.PlaylistScheduler = playlistScheduler httpServer := &http.Server{ diff --git a/flutter_client/pubspec.yaml b/flutter_client/pubspec.yaml index 6b5ba3d2..2e162f06 100644 --- a/flutter_client/pubspec.yaml +++ b/flutter_client/pubspec.yaml @@ -1,7 +1,7 @@ name: minstrel description: Minstrel mobile client publish_to: 'none' -version: 2026.5.15+7 +version: 2026.5.16+8 environment: sdk: '>=3.5.0 <4.0.0' diff --git a/internal/api/admin_covers.go b/internal/api/admin_covers.go index df9c8e18..d354ce31 100644 --- a/internal/api/admin_covers.go +++ b/internal/api/admin_covers.go @@ -48,19 +48,20 @@ func (h *handlers) handleAdminAlbumRefetchCover(w http.ResponseWriter, r *http.R } type adminBulkRefetchResp struct { - Queued int `json:"queued"` + Started bool `json:"started"` } // handleAdminBulkRefetchCovers implements POST /api/admin/covers/refetch-missing. -// Returns immediately; actual drain runs in a background goroutine bounded by -// the configured backfill cap. +// Returns immediately; the drain runs UNBOUNDED in a background goroutine +// (#388 — no global cap; remote providers self-throttle per provider, local +// sources run at disk speed). The count is unknowable synchronously, so the +// response just acknowledges the drain started. func (h *handlers) handleAdminBulkRefetchCovers(w http.ResponseWriter, _ *http.Request) { - cap := h.coverArtBackfillCap go func() { bgCtx := context.Background() - if _, err := h.coverart.EnrichRetryMissing(bgCtx, cap); err != nil { + if _, err := h.coverart.EnrichRetryMissing(bgCtx, -1); err != nil { h.logger.Warn("admin: bulk cover refetch failed", "err", err) } }() - writeJSON(w, http.StatusOK, adminBulkRefetchResp{Queued: cap}) + writeJSON(w, http.StatusOK, adminBulkRefetchResp{Started: true}) } diff --git a/internal/api/admin_covers_test.go b/internal/api/admin_covers_test.go index 49aa736b..7f574ce7 100644 --- a/internal/api/admin_covers_test.go +++ b/internal/api/admin_covers_test.go @@ -53,7 +53,6 @@ func testHandlersWithCovers(t *testing.T) (*handlers, *coverart.Enricher) { } enricher := coverart.NewEnricher(pool, slog.Default(), settings) h.coverart = enricher - h.coverArtBackfillCap = 100 return h, enricher } @@ -97,7 +96,7 @@ func TestAdminAlbumRefetchCover_AdminOK(t *testing.T) { } } -func TestAdminBulkRefetchCovers_AdminReturnsQueuedCount(t *testing.T) { +func TestAdminBulkRefetchCovers_AdminStartsRefetch(t *testing.T) { if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { t.Skip("MINSTREL_TEST_DATABASE_URL not set") } @@ -115,8 +114,8 @@ func TestAdminBulkRefetchCovers_AdminReturnsQueuedCount(t *testing.T) { if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v", err) } - if resp.Queued != h.coverArtBackfillCap { - t.Errorf("queued = %d, want %d (cap)", resp.Queued, h.coverArtBackfillCap) + if !resp.Started { + t.Errorf("started = false, want true (bulk refetch should acknowledge)") } } diff --git a/internal/api/api.go b/internal/api/api.go index 4f724493..9935cfc0 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -28,26 +28,25 @@ import ( // Mount attaches /api/* handlers to r. Public endpoints (login) are outside // RequireUser; everything else is gated by the middleware. The events writer // is shared with the Subsonic mount so /rest/scrobble feeds the same store. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverBackfillCap int, coverSettings *coverart.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, scheduler *library.Scheduler, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler) { +func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, scheduler *library.Scheduler, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler) { rng := rand.New(rand.NewSource(rand.Int63())) h := &handlers{ pool: pool, logger: logger, events: events, recCfg: recCfg, - rng: rng.Float64, - lidarrCfg: lidarrCfg, - lidarrRequests: lidarrReqs, - lidarrQuarantine: lidarrQuar, - tracks: tracksSvc, - playlists: playlistsSvc, - coverart: coverEnricher, - coverArtBackfillCap: coverBackfillCap, - coverSettings: coverSettings, - scanner: scanner, - scanCfg: scanCfg, - scheduler: scheduler, - dataDir: dataDir, - mailer: sender, - eventbus: bus, - playlistScheduler: playlistScheduler, + rng: rng.Float64, + lidarrCfg: lidarrCfg, + lidarrRequests: lidarrReqs, + lidarrQuarantine: lidarrQuar, + tracks: tracksSvc, + playlists: playlistsSvc, + coverart: coverEnricher, + coverSettings: coverSettings, + scanner: scanner, + scanCfg: scanCfg, + scheduler: scheduler, + dataDir: dataDir, + mailer: sender, + eventbus: bus, + playlistScheduler: playlistScheduler, } r.Route("/api", func(api chi.Router) { @@ -180,24 +179,23 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev } type handlers struct { - pool *pgxpool.Pool - logger *slog.Logger - events *playevents.Writer - recCfg config.RecommendationConfig - rng func() float64 - lidarrCfg *lidarrconfig.Service - lidarrRequests *lidarrrequests.Service - lidarrQuarantine *lidarrquarantine.Service - tracks *tracks.Service - playlists *playlists.Service - coverart *coverart.Enricher - coverArtBackfillCap int - coverSettings *coverart.SettingsService - scanner *library.Scanner - scanCfg library.RunScanConfig - scheduler *library.Scheduler - dataDir string - mailer mailer.Sender - eventbus *eventbus.Bus - playlistScheduler *playlists.Scheduler + pool *pgxpool.Pool + logger *slog.Logger + events *playevents.Writer + recCfg config.RecommendationConfig + rng func() float64 + lidarrCfg *lidarrconfig.Service + lidarrRequests *lidarrrequests.Service + lidarrQuarantine *lidarrquarantine.Service + tracks *tracks.Service + playlists *playlists.Service + coverart *coverart.Enricher + coverSettings *coverart.SettingsService + scanner *library.Scanner + scanCfg library.RunScanConfig + scheduler *library.Scheduler + dataDir string + mailer mailer.Sender + eventbus *eventbus.Bus + playlistScheduler *playlists.Scheduler } diff --git a/internal/api/library_test.go b/internal/api/library_test.go index 4cc8a3c5..845aed75 100644 --- a/internal/api/library_test.go +++ b/internal/api/library_test.go @@ -444,7 +444,7 @@ func TestRoutesRegisteredInMount(t *testing.T) { r := chi.NewRouter() w := playevents.NewWriter(h.pool, slog.New(slog.NewTextHandler(io.Discard, nil)), 30*time.Minute, 0.5, 30000) - Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverArtBackfillCap, h.coverSettings, h.scanner, h.scanCfg, nil, h.dataDir, nil, eventbus.New(), nil) + Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.scanner, h.scanCfg, nil, h.dataDir, nil, eventbus.New(), nil) paths := []string{ "/api/artists", diff --git a/internal/config/config.go b/internal/config/config.go index 4320fc20..986dad6d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -56,11 +56,10 @@ type LogConfig struct { } type LibraryConfig struct { - ScanPaths []string `yaml:"scan_paths"` - ScanOnStartup bool `yaml:"scan_on_startup"` - CoverArtFromMBCAA bool `yaml:"coverart_from_mbcaa"` // default true - CoverArtBackfillCap int `yaml:"coverart_backfill_cap"` // default 500 - ContactEmail string `yaml:"contact_email"` // optional override for MBCAA UA + ScanPaths []string `yaml:"scan_paths"` + ScanOnStartup bool `yaml:"scan_on_startup"` + CoverArtFromMBCAA bool `yaml:"coverart_from_mbcaa"` // default true + ContactEmail string `yaml:"contact_email"` // optional override for MBCAA UA } // SubsonicConfig controls wire-format concessions on the /rest/* surface. @@ -120,8 +119,7 @@ func Default() Config { RadioSizeMax: 200, }, Library: LibraryConfig{ - CoverArtFromMBCAA: true, - CoverArtBackfillCap: 500, + CoverArtFromMBCAA: true, }, } } @@ -177,11 +175,6 @@ func applyEnv(cfg *Config) { cfg.Library.CoverArtFromMBCAA = b } } - if v, ok := os.LookupEnv("MINSTREL_LIBRARY_COVERART_BACKFILL_CAP"); ok { - if n, err := strconv.Atoi(v); err == nil { - cfg.Library.CoverArtBackfillCap = n - } - } if v, ok := os.LookupEnv("MINSTREL_CONTACT_EMAIL"); ok { cfg.Library.ContactEmail = v } diff --git a/internal/coverart/artist_enricher.go b/internal/coverart/artist_enricher.go index 9b774991..48187a42 100644 --- a/internal/coverart/artist_enricher.go +++ b/internal/coverart/artist_enricher.go @@ -144,15 +144,21 @@ func (e *Enricher) recordArtistArt(ctx context.Context, artistID pgtype.UUID, th // 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: 0 = stage disabled; >0 = bounded; <0 = unbounded (#388 — +// no global cap; remote providers self-throttle per provider). func (e *Enricher) EnrichArtistBatch(ctx context.Context, limit int, dataDir string, progressCb func(processed, succeeded, failed int)) (processed, succeeded, failed int, err error) { - if limit <= 0 { + 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.ListArtistsMissingArt(ctx, dbq.ListArtistsMissingArtParams{ ArtistArtSourcesVersion: e.settings.CurrentVersion(), - Limit: int32(limit), + Limit: queryLimit, }) if qerr != nil { return 0, 0, 0, fmt.Errorf("list missing artist art: %w", qerr) diff --git a/internal/coverart/enricher.go b/internal/coverart/enricher.go index f45eafcf..cb7dda3e 100644 --- a/internal/coverart/enricher.go +++ b/internal/coverart/enricher.go @@ -213,15 +213,23 @@ func (e *Enricher) recordAlbumCover(ctx context.Context, albumID pgtype.UUID, pa // 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 { + 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: int32(limit), + Limit: queryLimit, }) if qerr != nil { return 0, 0, 0, fmt.Errorf("list missing: %w", qerr) @@ -293,12 +301,18 @@ func (e *Enricher) logBatchSummary(stage string, eligible, processed, succeeded, // 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 { + 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, int32(limit)) + rows, err := q.ListAlbumsRetryMissing(ctx, queryLimit) if err != nil { return 0, fmt.Errorf("list retry missing: %w", err) } diff --git a/internal/db/dbq/system_mixes.sql.go b/internal/db/dbq/system_mixes.sql.go index 5557b28b..3ea0e5b9 100644 --- a/internal/db/dbq/system_mixes.sql.go +++ b/internal/db/dbq/system_mixes.sql.go @@ -202,8 +202,8 @@ WITH windowed AS ( SELECT track_id, COUNT(*) AS c FROM play_events WHERE user_id = $1 AND was_skipped = false - AND started_at < now() - interval '60 days' - AND ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM now())) <= 7 + AND started_at < now() - interval '30 days' + AND ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM now())) <= 10 GROUP BY track_id ) SELECT t.id, t.album_id, t.artist_id @@ -229,9 +229,12 @@ type ListOnThisDayTracksRow struct { } // #422 On This Day: tracks the user played around this calendar date -// (±7 day-of-year) in the past, excluding the very recent (>60 days +// (±10 day-of-year) in the past, excluding the very recent (>30 days // ago) so it's nostalgic, not just "last week". Weighted by how much -// they were played in those windows. +// they were played in those windows. Floor relaxed from 60→30 days +// and window ±7→±10 so it surfaces on a months-old library instead +// of needing a full year of history; still skips cleanly (no rows → +// no playlist) when there's no qualifying history yet. // $1 user_id, $2 date string. func (q *Queries) ListOnThisDayTracks(ctx context.Context, arg ListOnThisDayTracksParams) ([]ListOnThisDayTracksRow, error) { rows, err := q.db.Query(ctx, listOnThisDayTracks, arg.UserID, arg.Column2) @@ -255,21 +258,41 @@ func (q *Queries) ListOnThisDayTracks(ctx context.Context, arg ListOnThisDayTrac const listRediscoverTracks = `-- name: ListRediscoverTracks :many WITH stats AS ( - SELECT track_id, COUNT(*) AS c, MAX(started_at) AS last_at - FROM play_events - WHERE user_id = $1 AND was_skipped = false - GROUP BY track_id + SELECT pe.track_id, COUNT(*) AS c, MAX(pe.started_at) AS last_at + FROM play_events pe + WHERE pe.user_id = $1 AND pe.was_skipped = false + GROUP BY pe.track_id +), +deep AS ( + SELECT t.id, t.album_id, t.artist_id, s.c, 0 AS tier + FROM tracks t + JOIN stats s ON s.track_id = t.id + WHERE s.c >= 5 + AND s.last_at <= now() - interval '6 months' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) +), +shallow AS ( + SELECT t.id, t.album_id, t.artist_id, s.c, 1 AS tier + FROM tracks t + JOIN stats s ON s.track_id = t.id + WHERE s.c >= 5 + AND s.last_at <= now() - interval '30 days' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) ) -SELECT t.id, t.album_id, t.artist_id - FROM tracks t - JOIN stats s ON s.track_id = t.id - WHERE s.c >= 5 - AND s.last_at <= now() - interval '6 months' - AND NOT EXISTS ( - SELECT 1 FROM lidarr_quarantine q - WHERE q.user_id = $1 AND q.track_id = t.id - ) - ORDER BY s.c DESC, t.id +SELECT id, album_id, artist_id + FROM ( + SELECT id, album_id, artist_id, c, tier FROM deep + UNION ALL + SELECT id, album_id, artist_id, c, tier FROM shallow + WHERE NOT EXISTS (SELECT 1 FROM deep) + ) u + ORDER BY tier, c DESC, id LIMIT 200 ` @@ -280,8 +303,12 @@ type ListRediscoverTracksRow struct { } // #420 Rediscover: tracks the user played a lot (>=5 non-skip) but -// not in the last 6 months. Ordered by historical affection. -// $1 user_id. +// has drifted away from. Tiered so a young library still gets a mix: +// +// tier 0 not played in the last 6 months (true rediscovery) +// tier 1 (only if tier 0 empty) not played in the last 30 days +// +// Ordered by historical affection. $1 user_id. func (q *Queries) ListRediscoverTracks(ctx context.Context, userID pgtype.UUID) ([]ListRediscoverTracksRow, error) { rows, err := q.db.Query(ctx, listRediscoverTracks, userID) if err != nil { diff --git a/internal/db/dbq/system_playlists.sql.go b/internal/db/dbq/system_playlists.sql.go index 1c2a3d4f..aa1c41cb 100644 --- a/internal/db/dbq/system_playlists.sql.go +++ b/internal/db/dbq/system_playlists.sql.go @@ -412,23 +412,55 @@ func (q *Queries) PickTopPlayedTrackForArtistByUser(ctx context.Context, arg Pic } const pickTopPlayedTracksForUser = `-- name: PickTopPlayedTracksForUser :many -SELECT t.id - FROM play_events pe - JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 - AND pe.started_at > now() - INTERVAL '7 days' - AND pe.was_skipped = false - GROUP BY t.id - ORDER BY COUNT(*) DESC, t.id +WITH recent AS ( + SELECT t.id, COUNT(*) AS c, 0 AS tier + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 + AND pe.started_at > now() - INTERVAL '30 days' + AND pe.was_skipped = false + GROUP BY t.id +), +alltime AS ( + SELECT t.id, COUNT(*) AS c, 1 AS tier + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 + AND pe.was_skipped = false + GROUP BY t.id +), +liked AS ( + SELECT gl.track_id AS id, 0::bigint AS c, 2 AS tier + FROM general_likes gl + WHERE gl.user_id = $1 +), +chosen AS ( + SELECT id, c, tier FROM recent + UNION ALL + SELECT id, c, tier FROM alltime + WHERE NOT EXISTS (SELECT 1 FROM recent) + UNION ALL + SELECT id, c, tier FROM liked + WHERE NOT EXISTS (SELECT 1 FROM recent) + AND NOT EXISTS (SELECT 1 FROM alltime) +) +SELECT id + FROM chosen + ORDER BY tier, c DESC, id LIMIT 5 ` -// For-You candidate seeds. Returns the user's top-5 most-played -// non-skipped tracks in the last 7 days; tie-break by track_id for -// determinism. The Go-side picker (pickForYouSeedForDay) chooses one -// of the returned rows as today's seed via userIDHash so the -// candidate pool rotates day-to-day while staying stable within a -// day. +// For-You candidate seeds, tiered so For-You never silently vanishes: +// +// tier 0 top non-skip plays in the last 30 days +// tier 1 (only if tier 0 empty) all-time top non-skip plays +// tier 2 (only if tiers 0+1 empty) liked tracks +// +// Returns up to 5 ids; tie-break by track_id for determinism. The +// Go-side picker (pickForYouSeedForDay) rotates one per day via +// userIDHash. Widened from a hard 7-day window, which made For-You +// disappear after a week of not listening and never recover on a +// self-hosted library with sparse history. func (q *Queries) PickTopPlayedTracksForUser(ctx context.Context, userID pgtype.UUID) ([]pgtype.UUID, error) { rows, err := q.db.Query(ctx, pickTopPlayedTracksForUser, userID) if err != nil { diff --git a/internal/db/dbq/tracks.sql.go b/internal/db/dbq/tracks.sql.go index 7e21a3f3..ffe498e3 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -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, diff --git a/internal/db/migrations/0029_tracks_mbid_nonunique.down.sql b/internal/db/migrations/0029_tracks_mbid_nonunique.down.sql new file mode 100644 index 00000000..b40e0408 --- /dev/null +++ b/internal/db/migrations/0029_tracks_mbid_nonunique.down.sql @@ -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; diff --git a/internal/db/migrations/0029_tracks_mbid_nonunique.up.sql b/internal/db/migrations/0029_tracks_mbid_nonunique.up.sql new file mode 100644 index 00000000..714789cb --- /dev/null +++ b/internal/db/migrations/0029_tracks_mbid_nonunique.up.sql @@ -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; diff --git a/internal/db/queries/system_mixes.sql b/internal/db/queries/system_mixes.sql index 62074620..b483fad0 100644 --- a/internal/db/queries/system_mixes.sql +++ b/internal/db/queries/system_mixes.sql @@ -38,24 +38,46 @@ SELECT t.id, t.album_id, t.artist_id -- name: ListRediscoverTracks :many -- #420 Rediscover: tracks the user played a lot (>=5 non-skip) but --- not in the last 6 months. Ordered by historical affection. --- $1 user_id. +-- has drifted away from. Tiered so a young library still gets a mix: +-- tier 0 not played in the last 6 months (true rediscovery) +-- tier 1 (only if tier 0 empty) not played in the last 30 days +-- Ordered by historical affection. $1 user_id. WITH stats AS ( - SELECT track_id, COUNT(*) AS c, MAX(started_at) AS last_at - FROM play_events - WHERE user_id = $1 AND was_skipped = false - GROUP BY track_id + SELECT pe.track_id, COUNT(*) AS c, MAX(pe.started_at) AS last_at + FROM play_events pe + WHERE pe.user_id = $1 AND pe.was_skipped = false + GROUP BY pe.track_id +), +deep AS ( + SELECT t.id, t.album_id, t.artist_id, s.c, 0 AS tier + FROM tracks t + JOIN stats s ON s.track_id = t.id + WHERE s.c >= 5 + AND s.last_at <= now() - interval '6 months' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) +), +shallow AS ( + SELECT t.id, t.album_id, t.artist_id, s.c, 1 AS tier + FROM tracks t + JOIN stats s ON s.track_id = t.id + WHERE s.c >= 5 + AND s.last_at <= now() - interval '30 days' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) ) -SELECT t.id, t.album_id, t.artist_id - FROM tracks t - JOIN stats s ON s.track_id = t.id - WHERE s.c >= 5 - AND s.last_at <= now() - interval '6 months' - AND NOT EXISTS ( - SELECT 1 FROM lidarr_quarantine q - WHERE q.user_id = $1 AND q.track_id = t.id - ) - ORDER BY s.c DESC, t.id +SELECT id, album_id, artist_id + FROM ( + SELECT id, album_id, artist_id, c, tier FROM deep + UNION ALL + SELECT id, album_id, artist_id, c, tier FROM shallow + WHERE NOT EXISTS (SELECT 1 FROM deep) + ) u + ORDER BY tier, c DESC, id LIMIT 200; -- name: ListNewForYouTracks :many @@ -87,16 +109,19 @@ SELECT t.id, t.album_id, t.artist_id -- name: ListOnThisDayTracks :many -- #422 On This Day: tracks the user played around this calendar date --- (±7 day-of-year) in the past, excluding the very recent (>60 days +-- (±10 day-of-year) in the past, excluding the very recent (>30 days -- ago) so it's nostalgic, not just "last week". Weighted by how much --- they were played in those windows. +-- they were played in those windows. Floor relaxed from 60→30 days +-- and window ±7→±10 so it surfaces on a months-old library instead +-- of needing a full year of history; still skips cleanly (no rows → +-- no playlist) when there's no qualifying history yet. -- $1 user_id, $2 date string. WITH windowed AS ( SELECT track_id, COUNT(*) AS c FROM play_events WHERE user_id = $1 AND was_skipped = false - AND started_at < now() - interval '60 days' - AND ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM now())) <= 7 + AND started_at < now() - interval '30 days' + AND ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM now())) <= 10 GROUP BY track_id ) SELECT t.id, t.album_id, t.artist_id diff --git a/internal/db/queries/system_playlists.sql b/internal/db/queries/system_playlists.sql index 8064dcf9..305c8b75 100644 --- a/internal/db/queries/system_playlists.sql +++ b/internal/db/queries/system_playlists.sql @@ -73,20 +73,50 @@ SELECT p.artist_id, LIMIT 5; -- name: PickTopPlayedTracksForUser :many --- For-You candidate seeds. Returns the user's top-5 most-played --- non-skipped tracks in the last 7 days; tie-break by track_id for --- determinism. The Go-side picker (pickForYouSeedForDay) chooses one --- of the returned rows as today's seed via userIDHash so the --- candidate pool rotates day-to-day while staying stable within a --- day. -SELECT t.id - FROM play_events pe - JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 - AND pe.started_at > now() - INTERVAL '7 days' - AND pe.was_skipped = false - GROUP BY t.id - ORDER BY COUNT(*) DESC, t.id +-- For-You candidate seeds, tiered so For-You never silently vanishes: +-- tier 0 top non-skip plays in the last 30 days +-- tier 1 (only if tier 0 empty) all-time top non-skip plays +-- tier 2 (only if tiers 0+1 empty) liked tracks +-- Returns up to 5 ids; tie-break by track_id for determinism. The +-- Go-side picker (pickForYouSeedForDay) rotates one per day via +-- userIDHash. Widened from a hard 7-day window, which made For-You +-- disappear after a week of not listening and never recover on a +-- self-hosted library with sparse history. +WITH recent AS ( + SELECT t.id, COUNT(*) AS c, 0 AS tier + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 + AND pe.started_at > now() - INTERVAL '30 days' + AND pe.was_skipped = false + GROUP BY t.id +), +alltime AS ( + SELECT t.id, COUNT(*) AS c, 1 AS tier + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 + AND pe.was_skipped = false + GROUP BY t.id +), +liked AS ( + SELECT gl.track_id AS id, 0::bigint AS c, 2 AS tier + FROM general_likes gl + WHERE gl.user_id = $1 +), +chosen AS ( + SELECT id, c, tier FROM recent + UNION ALL + SELECT id, c, tier FROM alltime + WHERE NOT EXISTS (SELECT 1 FROM recent) + UNION ALL + SELECT id, c, tier FROM liked + WHERE NOT EXISTS (SELECT 1 FROM recent) + AND NOT EXISTS (SELECT 1 FROM alltime) +) +SELECT id + FROM chosen + ORDER BY tier, c DESC, id LIMIT 5; -- name: PickTopPlayedTrackForArtistByUser :one diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index 4019b684..1790ddf4 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -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; diff --git a/internal/library/mbidbackfill.go b/internal/library/mbidbackfill.go index b312c1f5..5d79ff96 100644 --- a/internal/library/mbidbackfill.go +++ b/internal/library/mbidbackfill.go @@ -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) +} diff --git a/internal/library/mbids.go b/internal/library/mbids.go index 608ed0a0..3287b012 100644 --- a/internal/library/mbids.go +++ b/internal/library/mbids.go @@ -27,6 +27,20 @@ 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 the +// ListenBrainz Labs similar-recordings API 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 "" diff --git a/internal/library/scanner.go b/internal/library/scanner.go index be3476ad..5f47912a 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -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 { diff --git a/internal/library/scanrun.go b/internal/library/scanrun.go index 4b8a5db3..a6c9de3d 100644 --- a/internal/library/scanrun.go +++ b/internal/library/scanrun.go @@ -49,9 +49,11 @@ type ArtistArtEnrichStageTallies struct { Failed int `json:"failed"` } -// RunScanConfig assembles the knobs for the orchestrator. BackfillCap and -// EnrichCap mirror the existing boot-time caps (5000 / coverArtBackfillCap) -// so manual triggers don't accidentally drain the whole library. +// RunScanConfig assembles the knobs for the orchestrator. BackfillCap is +// the album/artist-MBID-backfill batch cap (5000); EnrichCap / +// ArtistEnrichCap gate cover/artist-art enrichment (0 = off, <0 = +// unbounded — #388 removed the global cover-art cap; remote providers +// self-throttle per provider). type RunScanConfig struct { BackfillCap int EnrichCap int @@ -154,6 +156,22 @@ 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. + // + // Unbounded (-1), unlike the album backfill's 5000 staged cap: this + // is a one-time whole-library heal with no progress UI, and a cap + // just means tracks.mbid stays partially NULL until N future scans + // catch up (re-reading untagged files wastefully each pass). One + // uncapped pass converges; subsequent scans only re-read the + // remaining NULL (genuinely untagged) rows, which is cheap. + if cfg.BackfillCap != 0 { + if _, tberr := BackfillTrackMBIDs(ctx, pool, logger, -1, nil); tberr != nil { + captureErr("track_mbid_backfill", tberr) + } + } + // Stage 3: cover enrichment. if enricher != nil && cfg.EnrichCap != 0 { var lastP, lastS, lastF int diff --git a/internal/playlists/system.go b/internal/playlists/system.go index c0610dad..fc55eed6 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -289,6 +289,23 @@ var systemPlaylistRegistry = []systemPlaylistKind{ {Key: "first_listens", Singleton: true, Produce: produceFirstListens}, } +// systemForYouSourceLimits is a deeper candidate pool than the radio +// default. On a self-hosted library without ListenBrainz similarity +// data the lb_similar / similar_artists sources contribute nothing, +// so the default (~130 raw, ~40 after dedup + diversity caps) can +// never fill For-You's 100-track head/tail. The raised random/tag +// fill keeps For-You ~100 deep regardless of LB enrichment; when LB +// data IS present the larger lb/similar K just makes it richer. +func systemForYouSourceLimits() recommendation.CandidateSourceLimits { + return recommendation.CandidateSourceLimits{ + LBSimilar: 80, + SimilarArtist: 80, + TagOverlap: 60, + LikesOverlap: 40, + RandomFill: 150, + } +} + // produceForYou: today's seed from the user's top-5 played tracks // (rotates daily via userIDHash), similarity candidate pool, head+ // tail composition. The base seed query failing is fatal; a @@ -311,7 +328,7 @@ func produceForYou( 1, // recentlyPlayedHours — small to avoid filtering the seed's recent neighbourhood zeroVec, []pgtype.UUID{forYouSeed}, - recommendation.DefaultCandidateSourceLimits(), + systemForYouSourceLimits(), ) if cerr != nil { logger.Warn("system playlist: for-you candidates load failed; skipping", diff --git a/internal/scrobble/listenbrainz/client.go b/internal/scrobble/listenbrainz/client.go index 42d3dd7a..28cfce5a 100644 --- a/internal/scrobble/listenbrainz/client.go +++ b/internal/scrobble/listenbrainz/client.go @@ -17,6 +17,13 @@ import ( const defaultBaseURL = "https://api.listenbrainz.org" +// defaultLabsBaseURL is the ListenBrainz *Labs* API — a DIFFERENT host +// from the main API. Similarity datasets (similar-recordings / +// similar-artists) live here, not under api.listenbrainz.org. The old +// api.listenbrainz.org/1/explore/... paths are website routes, not API +// endpoints, and 404 for every request. +const defaultLabsBaseURL = "https://labs.api.listenbrainz.org" + // Listen is one row sent to ListenBrainz. type Listen struct { ListenedAt int64 // unix seconds @@ -35,18 +42,22 @@ type Track struct { ReleaseMBID string } -// Client posts to the LB submit-listens endpoint. +// Client posts to the LB submit-listens endpoint and queries the Labs +// API for similarity. BaseURL is the main API; LabsBaseURL is the +// separate Labs host (see defaultLabsBaseURL). type Client struct { - BaseURL string - HTTP *http.Client + BaseURL string + LabsBaseURL string + HTTP *http.Client } -// NewClient returns a default-configured client (LB production base URL, -// 30s timeout). +// NewClient returns a default-configured client (LB production base +// URLs, 30s timeout). func NewClient() *Client { return &Client{ - BaseURL: defaultBaseURL, - HTTP: &http.Client{Timeout: 30 * time.Second}, + BaseURL: defaultBaseURL, + LabsBaseURL: defaultLabsBaseURL, + HTTP: &http.Client{Timeout: 30 * time.Second}, } } @@ -181,26 +192,31 @@ func buildPayload(listenType string, listens []Listen) payload { return payload{ListenType: listenType, Payload: entries} } -// LB algorithm parameter for /explore/similar-recordings/. Hardcoded for v1 -// — matches LB's documented default. -const lbSimilarRecordingsAlgorithm = "session_based_days_7500_session_30_contribution_5_threshold_15_limit_100_filter_True_skip_30" +// lbSimilarRecordingsAlgorithm is a Labs-API-valid algorithm enum (the +// value is verified against labs.api.listenbrainz.org; the old +// "_session_30_…_limit_100_filter_True…" string is NOT a permitted +// member and 400s). limit_50 is baked into the name — there is no +// separate count/limit query param. +const lbSimilarRecordingsAlgorithm = "session_based_days_9000_session_300_contribution_5_threshold_15_limit_50_skip_30" -// SimilarRecording is one entry in the /explore/similar-recordings response. +// SimilarRecording is one entry in the Labs similar-recordings response. type SimilarRecording struct { MBID string `json:"recording_mbid"` Score float64 `json:"score"` } -// SimilarRecordings fetches up to `limit` similar recordings for the given -// recording MBID. Public endpoint — no token required. Returns the same -// typed errors as SubmitListens. -func (c *Client) SimilarRecordings(ctx context.Context, mbid string, limit int) ([]SimilarRecording, error) { - base := c.BaseURL +// SimilarRecordings fetches similar recordings for the given recording +// MBID from the ListenBrainz Labs API. Public endpoint — no token +// required. The result count is fixed by the algorithm (limit_50); the +// caller applies its own top-K. Returns the same typed errors as +// SubmitListens. +func (c *Client) SimilarRecordings(ctx context.Context, mbid string, _ int) ([]SimilarRecording, error) { + base := c.LabsBaseURL if base == "" { - base = defaultBaseURL + base = defaultLabsBaseURL } - url := fmt.Sprintf("%s/1/explore/similar-recordings/%s?algorithm=%s&count=%d", - base, mbid, lbSimilarRecordingsAlgorithm, limit) + url := fmt.Sprintf("%s/similar-recordings/json?recording_mbids=%s&algorithm=%s", + base, mbid, lbSimilarRecordingsAlgorithm) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, fmt.Errorf("listenbrainz: build similar-recordings request: %w", err) @@ -234,29 +250,34 @@ func (c *Client) SimilarRecordings(ctx context.Context, mbid string, limit int) } } -// LB algorithm parameter for /explore/similar-artists/. Hardcoded for v1 -// — matches LB's documented default. -const lbSimilarArtistsAlgorithm = "session_based_days_7500_session_30_contribution_5_threshold_15_limit_100_filter_True_skip_30" +// lbSimilarArtistsAlgorithm is a Labs-API-valid algorithm enum (verified +// against labs.api.listenbrainz.org; shares the same permitted set as +// similar-recordings). The old value 400s. limit_50 is baked in — no +// separate count/limit query param. +const lbSimilarArtistsAlgorithm = "session_based_days_9000_session_300_contribution_5_threshold_15_limit_50_skip_30" -// SimilarArtist is one entry in the /explore/similar-artists response. -// Name is captured from the LB payload so M5c can render the artist's -// name on out-of-library suggestions without an extra MusicBrainz lookup. +// SimilarArtist is one entry in the Labs similar-artists response. Name +// is captured from the LB payload so M5c can render the artist's name +// on out-of-library suggestions without an extra MusicBrainz lookup. +// (The Labs payload also carries comment/type/gender/reference_mbid; +// those are intentionally ignored.) type SimilarArtist struct { MBID string `json:"artist_mbid"` Name string `json:"name"` Score float64 `json:"score"` } -// SimilarArtists fetches up to `limit` similar artists for the given -// artist MBID. Public endpoint — no token required. Returns the same -// typed errors as SubmitListens. -func (c *Client) SimilarArtists(ctx context.Context, mbid string, limit int) ([]SimilarArtist, error) { - base := c.BaseURL +// SimilarArtists fetches similar artists for the given artist MBID from +// the ListenBrainz Labs API. Public endpoint — no token required. The +// result count is fixed by the algorithm (limit_50); the caller applies +// its own top-K. Returns the same typed errors as SubmitListens. +func (c *Client) SimilarArtists(ctx context.Context, mbid string, _ int) ([]SimilarArtist, error) { + base := c.LabsBaseURL if base == "" { - base = defaultBaseURL + base = defaultLabsBaseURL } - url := fmt.Sprintf("%s/1/explore/similar-artists/%s?algorithm=%s&count=%d", - base, mbid, lbSimilarArtistsAlgorithm, limit) + url := fmt.Sprintf("%s/similar-artists/json?artist_mbids=%s&algorithm=%s", + base, mbid, lbSimilarArtistsAlgorithm) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, fmt.Errorf("listenbrainz: build similar-artists request: %w", err) diff --git a/internal/scrobble/listenbrainz/client_test.go b/internal/scrobble/listenbrainz/client_test.go index e04a83f2..f8a8939e 100644 --- a/internal/scrobble/listenbrainz/client_test.go +++ b/internal/scrobble/listenbrainz/client_test.go @@ -12,7 +12,7 @@ import ( func newTestClient(handler http.HandlerFunc) (*Client, *httptest.Server) { srv := httptest.NewServer(handler) - return &Client{BaseURL: srv.URL, HTTP: srv.Client()}, srv + return &Client{BaseURL: srv.URL, LabsBaseURL: srv.URL, HTTP: srv.Client()}, srv } func TestClient_SubmitListens_Success(t *testing.T) { @@ -196,7 +196,7 @@ func TestClient_SimilarRecordings_AlgorithmParamSet(t *testing.T) { } } -func TestClient_SimilarRecordings_LimitParamSet(t *testing.T) { +func TestClient_SimilarRecordings_MbidParamSet(t *testing.T) { var seenURL string c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { seenURL = r.URL.String() @@ -204,8 +204,13 @@ func TestClient_SimilarRecordings_LimitParamSet(t *testing.T) { }) defer srv.Close() _, _ = c.SimilarRecordings(context.Background(), "abc", 50) - if !strings.Contains(seenURL, "count=50") && !strings.Contains(seenURL, "limit=50") { - t.Errorf("URL missing count/limit param: %q", seenURL) + // Labs API takes the MBID as a query param, not a path segment; the + // result count is fixed by the algorithm (no count/limit param). + if !strings.Contains(seenURL, "recording_mbids=abc") { + t.Errorf("URL missing recording_mbids param: %q", seenURL) + } + if !strings.Contains(seenURL, "/similar-recordings/json") { + t.Errorf("URL not the Labs similar-recordings endpoint: %q", seenURL) } } @@ -291,7 +296,7 @@ func TestClient_SimilarArtists_AlgorithmParamSet(t *testing.T) { } } -func TestClient_SimilarArtists_LimitParamSet(t *testing.T) { +func TestClient_SimilarArtists_MbidParamSet(t *testing.T) { var seenURL string c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { seenURL = r.URL.String() @@ -299,8 +304,11 @@ func TestClient_SimilarArtists_LimitParamSet(t *testing.T) { }) defer srv.Close() _, _ = c.SimilarArtists(context.Background(), "abc", 50) - if !strings.Contains(seenURL, "count=50") && !strings.Contains(seenURL, "limit=50") { - t.Errorf("URL missing count/limit param: %q", seenURL) + if !strings.Contains(seenURL, "artist_mbids=abc") { + t.Errorf("URL missing artist_mbids param: %q", seenURL) + } + if !strings.Contains(seenURL, "/similar-artists/json") { + t.Errorf("URL not the Labs similar-artists endpoint: %q", seenURL) } } diff --git a/internal/server/server.go b/internal/server/server.go index 8e6f5730..cf381871 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -72,14 +72,13 @@ type Server struct { // playlist cover collages under /playlist_covers/). Empty // strings are tolerated by tests that don't exercise persisted-cover // codepaths; production callers should pass a writable directory. - DataDir string - BrandingCfg config.BrandingConfig - CoverEnricher *coverart.Enricher - CoverArtBackfillCap int - CoverSettings *coverart.SettingsService - LibraryScanner *library.Scanner - ScanCfg library.RunScanConfig - Scheduler *library.Scheduler + DataDir string + BrandingCfg config.BrandingConfig + CoverEnricher *coverart.Enricher + CoverSettings *coverart.SettingsService + LibraryScanner *library.Scanner + ScanCfg library.RunScanConfig + Scheduler *library.Scheduler // Bus is the live-event bus shared with background workers (the // lidarr reconciler, scan scheduler) constructed in cmd/minstrel/main.go. // When nil, Router() constructs a local fallback (test contexts). @@ -92,8 +91,8 @@ type Server struct { PlaylistScheduler *playlists.Scheduler } -func New(logger *slog.Logger, pool *pgxpool.Pool, scanner ScanTrigger, subCfg subsonic.Config, eventsCfg config.EventsConfig, recCfg config.RecommendationConfig, dataDir string, brandingCfg config.BrandingConfig, coverEnricher *coverart.Enricher, coverArtBackfillCap int, coverSettings *coverart.SettingsService, libraryScanner *library.Scanner, scanCfg library.RunScanConfig, scheduler *library.Scheduler) *Server { - return &Server{Logger: logger, Pool: pool, Scanner: scanner, SubsonicCfg: subCfg, EventsCfg: eventsCfg, RecommendationCfg: recCfg, DataDir: dataDir, BrandingCfg: brandingCfg, CoverEnricher: coverEnricher, CoverArtBackfillCap: coverArtBackfillCap, CoverSettings: coverSettings, LibraryScanner: libraryScanner, ScanCfg: scanCfg, Scheduler: scheduler} +func New(logger *slog.Logger, pool *pgxpool.Pool, scanner ScanTrigger, subCfg subsonic.Config, eventsCfg config.EventsConfig, recCfg config.RecommendationConfig, dataDir string, brandingCfg config.BrandingConfig, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, libraryScanner *library.Scanner, scanCfg library.RunScanConfig, scheduler *library.Scheduler) *Server { + return &Server{Logger: logger, Pool: pool, Scanner: scanner, SubsonicCfg: subCfg, EventsCfg: eventsCfg, RecommendationCfg: recCfg, DataDir: dataDir, BrandingCfg: brandingCfg, CoverEnricher: coverEnricher, CoverSettings: coverSettings, LibraryScanner: libraryScanner, ScanCfg: scanCfg, Scheduler: scheduler} } func (s *Server) Router() http.Handler { @@ -139,7 +138,7 @@ func (s *Server) Router() http.Handler { if bus == nil { bus = eventbus.New() } - api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverArtBackfillCap, s.CoverSettings, s.LibraryScanner, s.ScanCfg, s.Scheduler, s.DataDir, smtpSender, bus, s.PlaylistScheduler) + api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.LibraryScanner, s.ScanCfg, s.Scheduler, s.DataDir, smtpSender, bus, s.PlaylistScheduler) // /api/admin/scan is the only admin route owned by the server package // (it needs the Scanner). Register it as a single inline-middleware // route — using r.Route("/api/admin", ...) here would create a second diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 0be2d436..12545d21 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -24,7 +24,7 @@ import ( ) func TestHealthz(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}, nil) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{}, nil) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -73,7 +73,7 @@ func TestHealthz_IncludesMinClientVersion(t *testing.T) { } func TestRouter_ServesSPAAtRoot(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}, nil) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{}, nil) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -92,7 +92,7 @@ func TestRouter_ServesSPAAtRoot(t *testing.T) { } func TestRouter_DeepLinkFallbackReturnsSPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}, nil) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{}, nil) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -111,7 +111,7 @@ func TestRouter_DeepLinkFallbackReturnsSPA(t *testing.T) { } func TestRouter_APIPathNotSwallowedBySPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}, nil) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{}, nil) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -130,7 +130,7 @@ func TestRouter_APIPathNotSwallowedBySPA(t *testing.T) { } func TestRouter_RestPathNotSwallowedBySPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}, nil) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{}, nil) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -206,7 +206,7 @@ func TestRouter_AdminSubtreeNotShadowed(t *testing.T) { } s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), pool, - stubScanner{}, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}, nil) + stubScanner{}, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{}, nil) ts := httptest.NewServer(s.Router()) defer ts.Close() diff --git a/internal/similarity/worker.go b/internal/similarity/worker.go index e37cf5be..ba6ff365 100644 --- a/internal/similarity/worker.go +++ b/internal/similarity/worker.go @@ -1,9 +1,9 @@ // Package similarity owns the inbound ListenBrainz similarity ingest -// pipeline. A periodic worker queries LB's /explore/similar-recordings -// and /explore/similar-artists endpoints for tracks the user has played, -// filters returned MBIDs to the local library, and stores the top-K -// edges in track_similarity / artist_similarity for M4c's radio -// candidate-pool builder. +// pipeline. A periodic worker queries the LB Labs API +// (labs.api.listenbrainz.org similar-recordings / similar-artists) for +// tracks the user has played, filters returned MBIDs to the local +// library, and stores the top-K edges in track_similarity / +// artist_similarity for M4c's radio candidate-pool builder. package similarity import ( @@ -34,14 +34,16 @@ type Worker struct { } // NewWorker constructs a worker with production defaults: 1h tick, -// batch=5, topK=20. +// batch=25, topK=20. batch is tracks AND artists processed per tick; +// at 25/h a freshly-played library converges in hours, not days, while +// staying well under ListenBrainz rate limits (429s abort the tick). func NewWorker(pool *pgxpool.Pool, client *listenbrainz.Client, logger *slog.Logger) *Worker { return &Worker{ pool: pool, client: client, logger: logger, tick: 1 * time.Hour, - batch: 5, + batch: 25, topK: 20, } } diff --git a/internal/similarity/worker_test.go b/internal/similarity/worker_test.go index 3dfa0994..8e29c003 100644 --- a/internal/similarity/worker_test.go +++ b/internal/similarity/worker_test.go @@ -13,8 +13,8 @@ func TestNewWorker_DefaultsMatchSpec(t *testing.T) { if w.tick != 1*time.Hour { t.Errorf("tick = %v, want 1h", w.tick) } - if w.batch != 5 { - t.Errorf("batch = %d, want 5", w.batch) + if w.batch != 25 { + t.Errorf("batch = %d, want 25", w.batch) } if w.topK != 20 { t.Errorf("topK = %d, want 20", w.topK) diff --git a/web/src/lib/api/admin.covers.test.ts b/web/src/lib/api/admin.covers.test.ts index 5bc89734..75b2aa0e 100644 --- a/web/src/lib/api/admin.covers.test.ts +++ b/web/src/lib/api/admin.covers.test.ts @@ -25,9 +25,9 @@ describe('admin covers API', () => { }); it('refetchMissingCovers POSTs to the bulk endpoint', async () => { - (api.post as unknown as ReturnType).mockResolvedValueOnce({ queued: 7 }); + (api.post as unknown as ReturnType).mockResolvedValueOnce({ started: true }); const got = await refetchMissingCovers(); expect(api.post).toHaveBeenCalledWith('/api/admin/covers/refetch-missing', {}); - expect(got.queued).toBe(7); + expect(got.started).toBe(true); }); }); diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index ece4d1ed..20ca2da3 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -187,7 +187,7 @@ export async function refetchAlbumCover(albumId: string): Promise { diff --git a/web/src/routes/admin/+page.svelte b/web/src/routes/admin/+page.svelte index 5f570a37..54718b28 100644 --- a/web/src/routes/admin/+page.svelte +++ b/web/src/routes/admin/+page.svelte @@ -318,8 +318,10 @@ bulkBusy = true; bulkResult = null; try { - const { queued } = await refetchMissingCovers(); - bulkResult = `Queued ${queued} albums for cover refetch.`; + const { started } = await refetchMissingCovers(); + bulkResult = started + ? 'Refetching all missing covers — local sources are fast, remote providers throttle per their limits.' + : 'Could not start the cover refetch.'; await client.invalidateQueries({ queryKey: qk.coverage() }); } catch (e) { bulkResult = `Failed: ${errCode(e)}`; diff --git a/web/src/routes/admin/admin.test.ts b/web/src/routes/admin/admin.test.ts index 55e6bfbc..a0272596 100644 --- a/web/src/routes/admin/admin.test.ts +++ b/web/src/routes/admin/admin.test.ts @@ -41,7 +41,7 @@ vi.mock('$lib/api/admin', async () => { deleteQuarantineFile: vi.fn().mockResolvedValue({}), deleteQuarantineViaLidarr: vi.fn().mockResolvedValue({}), triggerScan: vi.fn().mockResolvedValue({}), - refetchMissingCovers: vi.fn().mockResolvedValue({ queued: 0 }), + refetchMissingCovers: vi.fn().mockResolvedValue({ started: true }), researchMissingArt: vi.fn().mockResolvedValue({ version: 1 }), createScanScheduleQuery: vi.fn(() => readable({