fix(recommendation): size the candidate pool to the library
test-go / test (push) Successful in 1m5s
test-go / integration (push) Failing after 3m39s
release / Build signed APK (releases and dev) (push) Successful in 4m55s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 1m38s
test-go / test (push) Successful in 1m5s
test-go / integration (push) Failing after 3m39s
release / Build signed APK (releases and dev) (push) Successful in 4m55s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 1m38s
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. DefaultCandidateSourceLimits returns what its own comment calls
"the v1 hardcoded constants per spec" — ~170 candidates for a 500-track
library and a 100,000-track one alike. The pool therefore samples a
shrinking FRACTION of a growing collection: 17% of 1,000 tracks, 1.7% of
10,000, 0.17% of 100,000. RandomFill, whose whole job is exploration,
becomes a thinner and noisier slice at exactly the moment a library gets
more diverse — which is the "starting to feel weird" being reported.
1,000 tracks -> pool 170 (unchanged)
5,000 -> pool 170 (unchanged)
20,000 -> pool 280
80,000 -> pool 500 (ceiling)
THE SCALING IS PER-ARM, and that is the substance rather than a refinement.
A 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. LBSimilar,
SimilarArtist, TagOverlap and RandomFill grow: they are bounded by
similarity/tag data and by the library itself. LikesOverlap, UserCoplay and
TasteOverlap do not: they are bounded by the user's likes, the instance's
co-play graph and the taste profile, none of which grow when the library
does. Raising those would sample more of a set that did not change — churn,
not reach. It also keeps this from inflating the sim_score-0 share, since
TasteOverlap is one of the two zero-similarity arms.
sqrt, not linear: linear would put a 100,000-track library at a
3,400-candidate pool, long past where more candidates improve the answer.
A 4x ceiling bounds it at ~500.
Never shrinks an arm. The base limits are a floor, and #3889 makes that
load-bearing rather than tidy — shrinking an arm ordered by unseeded
random() changes pool membership between same-day rebuilds.
Library size comes from a TTL-cached count reusing CountTracksMatching with
an empty pattern (rule 28 — a new query would need sqlc regeneration, which
is blocked). The ILIKE defeats every index, so it is a full scan and must
not run per request. It degrades rather than fails: an error keeps the last
known value, a never-counted cache returns 0, and 0 scales to the base
limits — today's behaviour exactly. Nothing about sizing a pool justifies
failing the request it is sizing. Bounded by a 3s deadline (rule 156), and
a failed refresh does not stamp the clock, so a blip cannot pin a stale
value for the whole TTL.
THE REFERENCE IS ASSUMED, NOT MEASURED. libraryScaleReference = 5000 is
where growth starts, and the size the v1 constants were really tuned against
is unrecorded. #3879 should replace it; until then that constant is the one
thing to change. Deliberately conservative: below it nothing scales at all,
so no existing install changes behaviour.
Falsification caught a weak guard: the sqrt-vs-linear assertion was written
at SIXTEEN times the reference, where linear has already been clamped by the
ceiling and both curves land on 4x. It proved nothing. Moved to four times
the reference, below the ceiling for both, where sqrt gives 2x and linear
would give 4x.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
This commit is contained in:
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user