diff --git a/internal/api/api.go b/internal/api/api.go index d9d99990..2f71c3e2 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -24,6 +24,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/playevents" "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" "git.fabledsword.com/bvandeusen/minstrel/internal/reacquisition" + "git.fabledsword.com/bvandeusen/minstrel/internal/recommendation" "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" "git.fabledsword.com/bvandeusen/minstrel/internal/tags" "git.fabledsword.com/bvandeusen/minstrel/internal/tracks" @@ -55,6 +56,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev streamSecret: streamSecret, netSettings: netSettings, reacqSettings: reacqSettings, + librarySize: recommendation.NewLibrarySize(nil), } r.Route("/api", func(api chi.Router) { @@ -268,12 +270,16 @@ 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 - recSettings *recsettings.Service - rng func() float64 + pool *pgxpool.Pool + logger *slog.Logger + events *playevents.Writer + recCfg config.RecommendationConfig + recSettings *recsettings.Service + rng func() float64 + // librarySize memoises the track count that sizes the candidate pool + // (#3880). Held here rather than counted per request: the count is a + // full table scan, and library size only moves when a scan runs. + librarySize *recommendation.LibrarySize lidarrCfg *lidarrconfig.Service lidarrRequests *lidarrrequests.Service lidarrQuarantine *lidarrquarantine.Service diff --git a/internal/api/radio.go b/internal/api/radio.go index 8f4d194c..218dadd8 100644 --- a/internal/api/radio.go +++ b/internal/api/radio.go @@ -87,7 +87,17 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) { currentVec.DeviceClass = latestDeviceClass(r.Context(), q, user.ID, h.logger) exclude := parseExcludeParam(r.URL.Query().Get("exclude")) - limits := recommendation.DefaultCandidateSourceLimits() + // Size the pool to the library (#3880). A fixed ~170 candidates samples a + // shrinking fraction of a growing collection, which is what made the + // recommendations feel less relevant as the library grew. Degrades to the + // base limits if the count is unavailable — never fails the request over a + // sizing hint. + librarySize := h.librarySize.Get(r.Context(), func(ctx context.Context) (int64, error) { + return recommendation.CountLibraryTracks(ctx, q) + }) + limits := recommendation.ScaleForLibrary( + recommendation.DefaultCandidateSourceLimits(), librarySize, + ) candidates, err := recommendation.LoadCandidatesFromSimilarity( r.Context(), q, user.ID, seedID, h.recCfg.RecentlyPlayedHours, currentVec, exclude, limits, diff --git a/internal/playlists/system.go b/internal/playlists/system.go index 7bac0a1e..f0708793 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -238,6 +238,11 @@ var ( ContextTimeWeight: 0.5, } systemTasteConfig = taste.DefaultConfig() + + // Sizes the candidate pool to the library (#3880). Cached because the + // count is a full table scan and the daily build runs it once per user; + // within the TTL every user in a build shares one count. + systemLibrarySize = recommendation.NewLibrarySize(nil) ) // SetSystemMixWeights installs the current daily_mix scoring weights. @@ -675,6 +680,13 @@ func produceSeedMixes( seedPool := pickSeedArtistsFromRows(seedRowsLocal) seeds := pickSeedArtistsForDay(seedPool, userID, dateStr) + // Once per build rather than once per seed artist — six mixes would + // otherwise mean six full-table counts for a number that cannot have + // changed between them. + librarySize := systemLibrarySize.Get(ctx, func(c context.Context) (int64, error) { + return recommendation.CountLibraryTracks(c, q) + }) + out := make([]builtPlaylist, 0, len(seeds)) for _, artistID := range seeds { artistRow, aerr := q.GetArtistByID(ctx, artistID) @@ -694,9 +706,16 @@ func produceSeedMixes( // size; the composition shifts toward arms that actually measure // distance from the seed. The default gave ~29% of candidates a // sim_score of literally 0. + // + // Then scaled to the library (#3880): a bigger collection should put + // more genuinely-similar candidates in reach, not the same ~170 + // regardless. The weights still rank sim_score-0 arms last, so the + // growth reaches coherence rather than working against it. cands, cerr := recommendation.LoadCandidatesFromSimilarity( ctx, q, userID, seedTrack, 1, zeroVec, []pgtype.UUID{seedTrack}, - recommendation.SongsLikeCandidateSourceLimits(), + recommendation.ScaleForLibrary( + recommendation.SongsLikeCandidateSourceLimits(), librarySize, + ), ) if cerr != nil { logger.Warn("system playlist: seed candidates load failed; skipping", diff --git a/internal/recommendation/library_scale.go b/internal/recommendation/library_scale.go new file mode 100644 index 00000000..d648171f --- /dev/null +++ b/internal/recommendation/library_scale.go @@ -0,0 +1,168 @@ +package recommendation + +import ( + "context" + "math" + "sync" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Scaling the candidate pool to the library (#3880). +// +// DefaultCandidateSourceLimits returns what its own comment calls "the v1 +// hardcoded constants per spec" — ~170 candidates, identical for a 500-track +// library and a 100,000-track one. Operator, 2026-09-10: "is the pool that we +// draw from somehow scaled to the amount of music in the library... my earlier +// understanding of the tuning and work may have been skewed by what was in my +// library." +// +// It was not, and the consequence compounds. The pool samples a shrinking +// FRACTION of the library as it grows — 17% of 1,000 tracks, 1.7% of 10,000, +// 0.17% of 100,000 — so the ceiling on how much of a collection can ever +// surface stays flat while the collection does not. RandomFill, whose entire +// job is exploration, becomes a thinner and noisier slice of a more diverse +// corpus at exactly the moment diversity rises. + +const ( + // libraryScaleReference is the library size at which the base limits + // apply unchanged. Below it nothing scales, so small libraries keep + // today's behaviour exactly. + // + // ASSUMED, NOT MEASURED. The size the v1 constants were actually tuned + // against is unrecorded; 5,000 is a plausible mid-size library and a + // deliberately conservative place to start growing. #3879 should replace + // this with the real number, and until it does, this constant is the + // single thing to change. + libraryScaleReference = 5000 + + // maxLibraryScale bounds growth so a very large library does not turn + // every recommendation query into a huge scan. At 4x the pool tops out + // around 500 candidates, which is still cheap to score in memory. + maxLibraryScale = 4.0 +) + +// libraryScale is sqrt rather than linear on purpose. Linear growth would put +// a 100,000-track library at a 3,400-candidate pool — a slow query, slow +// scoring, and far past the point where more candidates improve the answer. +// Square-root growth keeps the pool meaningfully proportional while staying +// bounded: 1x at the reference, 2x at four times it, 4x at sixteen times. +// +// Never returns below 1: the base limits are a floor, not a midpoint. That +// also keeps this safe against #3889 — growing an arm ordered by unseeded +// random() is fine, shrinking one is what breaks same-day determinism. +func libraryScale(libraryTracks int64) float64 { + if libraryTracks <= libraryScaleReference { + return 1.0 + } + f := math.Sqrt(float64(libraryTracks) / float64(libraryScaleReference)) + return math.Min(f, maxLibraryScale) +} + +// ScaleForLibrary grows the library-bounded arms of a limit set for a library +// of the given size, and leaves the rest alone. +// +// THE SCALING IS NOT UNIFORM, and that is the substance of it rather than a +// refinement. An arm's limit only matters if there are rows for it to cut off, +// so what an arm is BOUNDED BY decides whether library size can help it: +// +// scaled LBSimilar, SimilarArtist, TagOverlap — bounded by similarity +// and tag data, which grows as the library does. A bigger library +// means more of ListenBrainz's returned MBIDs survive the +// local-library filter in similarity/worker.go. +// scaled RandomFill — the exploration arm, and the one the complaint is +// really about. This is where "fraction of the library" lives. +// unscaled LikesOverlap — bounded by the USER's likes. +// unscaled UserCoplay — bounded by the INSTANCE's co-play graph. +// unscaled TasteOverlap — bounded by the taste profile's artists. +// +// Raising the last three would not sample more of the library; it would +// sample more of a set that did not grow, which is churn rather than reach. +// Leaving TasteOverlap alone has a second benefit: it and RandomFill are the +// two arms carrying sim_score 0, so this does not inflate the +// seed-independent share as fast as a uniform scale would. +func ScaleForLibrary(base CandidateSourceLimits, libraryTracks int64) CandidateSourceLimits { + f := libraryScale(libraryTracks) + grow := func(n int) int { return int(float64(n) * f) } // f >= 1, so never shrinks + return CandidateSourceLimits{ + LBSimilar: grow(base.LBSimilar), + SimilarArtist: grow(base.SimilarArtist), + TagOverlap: grow(base.TagOverlap), + RandomFill: grow(base.RandomFill), + LikesOverlap: base.LikesOverlap, + UserCoplay: base.UserCoplay, + TasteOverlap: base.TasteOverlap, + } +} + +// CountLibraryTracks returns the whole library's track count. +// +// Reuses CountTracksMatching with an empty pattern — `title ILIKE '%%'` +// matches every row — rather than adding a query, because a new one would +// need sqlc regeneration (rule 28, and see #3889 for where that blocks). +// The user id is left invalid so the quarantine anti-join is skipped: this +// number sizes a pool, and a per-user view of it would be false precision. +func CountLibraryTracks(ctx context.Context, q *dbq.Queries) (int64, error) { + return q.CountTracksMatching(ctx, dbq.CountTracksMatchingParams{ + Column1: "", + UserID: pgtype.UUID{}, // invalid → NULL → no quarantine filter + }) +} + +// librarySizeTTL is how long a counted size is reused. Library size changes +// only when a scan runs, so minutes are plenty — and the count is a full +// table scan (the ILIKE defeats every index), which is why it is not done +// per request. +const librarySizeTTL = 5 * time.Minute + +// librarySizeTimeout bounds the count itself. Rule 156: the caller is a user +// waiting on a radio, and a pool-sizing hint is never worth hanging for. +const librarySizeTimeout = 3 * time.Second + +// LibrarySize memoises the library track count. +// +// Degrades rather than fails: a count that errors or times out leaves the +// previous value in place, and a zero (never yet counted) scales to the base +// limits — which is exactly today's behaviour. Nothing about sizing a +// candidate pool justifies failing the request it is sizing. +type LibrarySize struct { + mu sync.Mutex + now func() time.Time // injectable for tests + at time.Time + size int64 +} + +// NewLibrarySize returns an empty cache. The zero value works too; this +// exists so tests can pin the clock. +func NewLibrarySize(now func() time.Time) *LibrarySize { + return &LibrarySize{now: now} +} + +// Get returns the cached size, refreshing through load when stale. +func (l *LibrarySize) Get(ctx context.Context, load func(context.Context) (int64, error)) int64 { + l.mu.Lock() + defer l.mu.Unlock() + + now := time.Now + if l.now != nil { + now = l.now + } + if !l.at.IsZero() && now().Sub(l.at) < librarySizeTTL { + return l.size + } + + cctx, cancel := context.WithTimeout(ctx, librarySizeTimeout) + defer cancel() + n, err := load(cctx) + if err != nil { + // Keep the last known value and re-try at the next call rather than + // stamping `at`, so a transient failure does not pin a stale number + // for the whole TTL. + return l.size + } + l.size, l.at = n, now() + return l.size +} diff --git a/internal/recommendation/library_scale_test.go b/internal/recommendation/library_scale_test.go new file mode 100644 index 00000000..00cd6745 --- /dev/null +++ b/internal/recommendation/library_scale_test.go @@ -0,0 +1,177 @@ +package recommendation + +import ( + "context" + "errors" + "testing" + "time" +) + +// Small libraries must behave exactly as before. Scaling is meant to help a +// growing collection, not to change what every existing install already does. +func TestScaleForLibrary_SmallLibrariesAreUntouched(t *testing.T) { + base := DefaultCandidateSourceLimits() + for _, size := range []int64{0, 1, 500, libraryScaleReference} { + if got := ScaleForLibrary(base, size); got != base { + t.Errorf("library of %d changed the limits: %+v, want %+v", size, got, base) + } + } +} + +// The complaint, restated as a property: a bigger library must reach further +// into itself. Operator, 2026-09-10 — "is the pool that we draw from somehow +// scaled to the amount of music in the library". +func TestScaleForLibrary_BigLibrariesGetABiggerPool(t *testing.T) { + base := DefaultCandidateSourceLimits() + small := ScaleForLibrary(base, libraryScaleReference) + big := ScaleForLibrary(base, libraryScaleReference*16) + + if big.RandomFill <= small.RandomFill { + t.Errorf("RandomFill %d did not grow for a 16x library (was %d); that arm "+ + "IS the fraction-of-library problem", big.RandomFill, small.RandomFill) + } + if big.LBSimilar <= small.LBSimilar { + t.Errorf("LBSimilar %d did not grow for a 16x library (was %d)", + big.LBSimilar, small.LBSimilar) + } +} + +// THE SUBSTANCE: scaling is per-arm, decided by what each arm is BOUNDED BY. +// Raising a limit only helps if there are rows for it to cut off, so arms +// bounded by the user's likes, the instance's co-play graph or the taste +// profile gain nothing from a bigger library — raising them would sample more +// of a set that did not grow. +// +// A uniform scale would look right and be wrong, which is why this is pinned +// separately from "the pool got bigger". +func TestScaleForLibrary_OnlyLibraryBoundedArmsGrow(t *testing.T) { + base := DefaultCandidateSourceLimits() + big := ScaleForLibrary(base, libraryScaleReference*16) + + for _, tc := range []struct { + arm string + got, want int + why string + }{ + {"LikesOverlap", big.LikesOverlap, base.LikesOverlap, "bounded by the user's likes"}, + {"UserCoplay", big.UserCoplay, base.UserCoplay, "bounded by the instance's co-play graph"}, + {"TasteOverlap", big.TasteOverlap, base.TasteOverlap, "bounded by the taste profile's artists"}, + } { + if tc.got != tc.want { + t.Errorf("%s scaled to %d (base %d) but is %s — a bigger library gives it "+ + "nothing more to return", tc.arm, tc.got, tc.want, tc.why) + } + } +} + +// Growth is bounded, or a huge library turns every recommendation query into +// a huge scan. sqrt keeps it proportional; the ceiling keeps it affordable. +func TestScaleForLibrary_GrowthIsBounded(t *testing.T) { + base := DefaultCandidateSourceLimits() + huge := ScaleForLibrary(base, 100_000_000) + + if huge.RandomFill > int(float64(base.RandomFill)*maxLibraryScale) { + t.Errorf("RandomFill %d exceeds the %.0fx ceiling on base %d", + huge.RandomFill, maxLibraryScale, base.RandomFill) + } + // sqrt, not linear — and the test point matters. At SIXTEEN times the + // reference both curves land on 4x, because linear has already been + // clamped by the ceiling; asserting there proves nothing. Four times the + // reference is below the ceiling for both, so the curves separate: sqrt + // gives 2x, linear would give 4x. + four := ScaleForLibrary(base, libraryScaleReference*4) + if four.RandomFill != base.RandomFill*2 { + t.Errorf("4x library gave RandomFill %d, want %d — sqrt growth (linear "+ + "would give %d)", four.RandomFill, base.RandomFill*2, base.RandomFill*4) + } +} + +// Never below the base. The base limits are a floor, not a midpoint — and +// #3889 makes this load-bearing rather than tidy: shrinking an arm ordered by +// unseeded random() changes pool membership between same-day rebuilds. +func TestScaleForLibrary_NeverShrinksAnArm(t *testing.T) { + base := DefaultCandidateSourceLimits() + for _, size := range []int64{0, 1, 100, 4999, 5001, 1_000_000} { + got := ScaleForLibrary(base, size) + for _, tc := range []struct { + arm string + got, want int + }{ + {"LBSimilar", got.LBSimilar, base.LBSimilar}, + {"SimilarArtist", got.SimilarArtist, base.SimilarArtist}, + {"TagOverlap", got.TagOverlap, base.TagOverlap}, + {"RandomFill", got.RandomFill, base.RandomFill}, + {"LikesOverlap", got.LikesOverlap, base.LikesOverlap}, + {"UserCoplay", got.UserCoplay, base.UserCoplay}, + {"TasteOverlap", got.TasteOverlap, base.TasteOverlap}, + } { + if tc.got < tc.want { + t.Errorf("library %d shrank %s to %d (base %d)", size, tc.arm, tc.got, tc.want) + } + } + } +} + +// A pool-sizing hint must never fail the request it is sizing. A count that +// errors leaves the previous value in place, and a never-counted cache +// returns 0 — which scales to the base limits, i.e. exactly today's +// behaviour. +func TestLibrarySize_DegradesOnFailure(t *testing.T) { + clock := time.Now() + c := NewLibrarySize(func() time.Time { return clock }) + + boom := func(context.Context) (int64, error) { return 0, errors.New("db is down") } + if got := c.Get(context.Background(), boom); got != 0 { + t.Errorf("first failure returned %d, want 0 (which scales to the base limits)", got) + } + if got := ScaleForLibrary(DefaultCandidateSourceLimits(), 0); got != DefaultCandidateSourceLimits() { + t.Error("a zero library size did not scale to the base limits") + } + + // A good count, then a failure: the last known value survives. + if got := c.Get(context.Background(), func(context.Context) (int64, error) { return 40_000, nil }); got != 40_000 { + t.Fatalf("got %d, want 40000", got) + } + clock = clock.Add(librarySizeTTL + time.Second) + if got := c.Get(context.Background(), boom); got != 40_000 { + t.Errorf("a failed refresh returned %d, discarding the last known 40000", got) + } +} + +// The count is a full table scan, so it must not run per request. +func TestLibrarySize_CachesWithinTheTTL(t *testing.T) { + clock := time.Now() + c := NewLibrarySize(func() time.Time { return clock }) + calls := 0 + load := func(context.Context) (int64, error) { calls++; return 1234, nil } + + for i := 0; i < 5; i++ { + c.Get(context.Background(), load) + } + if calls != 1 { + t.Errorf("counted %d times within the TTL, want 1 — this is a full table scan", calls) + } + + clock = clock.Add(librarySizeTTL + time.Second) + c.Get(context.Background(), load) + if calls != 2 { + t.Errorf("counted %d times after the TTL expired, want 2 — the size never refreshes", calls) + } +} + +// A transient failure must not pin a stale value for the whole TTL: the +// failed refresh does not stamp the clock, so the next call retries. +func TestLibrarySize_RetriesAfterAFailedRefresh(t *testing.T) { + clock := time.Now() + c := NewLibrarySize(func() time.Time { return clock }) + + c.Get(context.Background(), func(context.Context) (int64, error) { return 100, nil }) + clock = clock.Add(librarySizeTTL + time.Second) + c.Get(context.Background(), func(context.Context) (int64, error) { return 0, errors.New("blip") }) + + // Immediately after, with no clock movement — a stamped failure would + // serve the stale 100 until the TTL expired again. + if got := c.Get(context.Background(), func(context.Context) (int64, error) { return 900, nil }); got != 900 { + t.Errorf("got %d after a failed refresh, want 900 — the failure pinned a stale value", got) + } +}