Recommendation relevance, the rollback unit, and a version that names what shipped #131

Merged
bvandeusen merged 11 commits from dev into main 2026-09-10 23:37:57 -04:00
Owner

Eleven commits in three strands. CI green on 4ce47397 across every lane, integration included.

Recommendation relevance (milestone #398)

Operator report, 2026-09-10: "the songs like section algorithm is starting to feel weird", a radio session that came back entirely from one artist, and a hypothesis — "is the pool that we draw from somehow scaled to the amount of music in the library". All three were right.

Songs-like wandered (#3881) because it shared the daily_mix weight profile with For-You, and the two surfaces want opposite things. Under the shared weights an unrelated track you'd liked and not played recently scored 1.0 + 2.0 + 1.0 = 4.0, while a perfect similarity match you hadn't liked scored 1.0 + 1.5 = 2.5. Liking something outranked sounding like the seed. It now has its own DB-backed profile where similarity outranges every seed-independent term combined: the same pair scores 5.00 vs 2.00. The seed artist's own music is eligible too — bounded at 3 of 25 by the diversity cap rather than excluded, since those are usually the seed track's nearest neighbours.

Radio had no per-artist cap (#3882) while discover.go, you_might_like.go and home.go all did. On the fixture that reproduces the report, 10 tracks from 1 artist becomes 10 from 8. Two-pass selection, so the caps change which tracks are picked and never how many — a hard cap would have returned a six-track "radio" for a fifty-track request.

The pool didn't scale (#3880). DefaultCandidateSourceLimits returned what its own comment called "the v1 hardcoded constants per spec" — ~170 candidates for a 500-track library and a 100,000-track one alike, so the pool sampled a shrinking fraction of a growing collection. Now sqrt-scaled per arm, bounded at 4x, and only for arms that library size can actually help.

Release machinery

Image tags follow family rules 145 and 147dev:dev, main:latest + :<sha>, tag → :latest. No :<version>, no :main. The :<sha> is new and is the rollback unit; this repo previously had none.

The version names what shipped. ci/version.sh derived from bare HEAD, so a CI-only or docs-only commit moved it. Now a denylist pathspec over the shipped file set, tests included — they don't ship, so they must not re-version an artifact.

.dockerignore excluded the wrong CI directory (.forgejo/ and .github/, neither of which exists here) and had lost the pattern for the mobile client. Build context: 42.66 MB → 21.71 MB. An 18 MB binary was also tracked; it isn't now.

What to expect on main

The rebundle path is exercised for the first time under the new tag map, and the tag build will publish the first :<sha> rollback image. Scribe #3890 has ci-runner-f1 watching this merge for android.yml's debug-APK artifact.

Carrying caveats forward

The recommendation numbers are defensible starting points, not measured optima — spike #3879 still gates them, and libraryScaleReference = 5000 is an assumption about a library size nobody recorded. Two known defects stay open: likes_overlap assigns a flat 0.6 similarity it never computed (#3879), and four candidate arms order by unseeded random() so same-day determinism is accidental (#3889).

🤖 Generated with Claude Code

https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH

Eleven commits in three strands. CI green on `4ce47397` across every lane, integration included. ## Recommendation relevance (milestone #398) Operator report, 2026-09-10: *"the songs like section algorithm is starting to feel weird"*, a radio session that came back entirely from one artist, and a hypothesis — *"is the pool that we draw from somehow scaled to the amount of music in the library"*. All three were right. **Songs-like wandered** (#3881) because it shared the `daily_mix` weight profile with For-You, and the two surfaces want opposite things. Under the shared weights an unrelated track you'd liked and not played recently scored `1.0 + 2.0 + 1.0 = 4.0`, while a perfect similarity match you hadn't liked scored `1.0 + 1.5 = 2.5`. Liking something outranked sounding like the seed. It now has its own DB-backed profile where similarity outranges every seed-independent term combined: the same pair scores **5.00 vs 2.00**. The seed artist's own music is eligible too — bounded at 3 of 25 by the diversity cap rather than excluded, since those are usually the seed track's nearest neighbours. **Radio had no per-artist cap** (#3882) while `discover.go`, `you_might_like.go` and `home.go` all did. On the fixture that reproduces the report, 10 tracks from 1 artist becomes 10 from 8. Two-pass selection, so the caps change *which* tracks are picked and never *how many* — a hard cap would have returned a six-track "radio" for a fifty-track request. **The pool didn't scale** (#3880). `DefaultCandidateSourceLimits` returned what its own comment called "the v1 hardcoded constants per spec" — ~170 candidates for a 500-track library and a 100,000-track one alike, so the pool sampled a shrinking fraction of a growing collection. Now sqrt-scaled per arm, bounded at 4x, and only for arms that library size can actually help. ## Release machinery **Image tags follow family rules 145 and 147** — `dev` → `:dev`, `main` → `:latest + :<sha>`, tag → `:latest`. No `:<version>`, no `:main`. The `:<sha>` is new and is the rollback unit; this repo previously had none. **The version names what shipped.** `ci/version.sh` derived from bare `HEAD`, so a CI-only or docs-only commit moved it. Now a denylist pathspec over the shipped file set, tests included — they don't ship, so they must not re-version an artifact. **`.dockerignore` excluded the wrong CI directory** (`.forgejo/` and `.github/`, neither of which exists here) and had lost the pattern for the mobile client. Build context: 42.66 MB → 21.71 MB. An 18 MB binary was also tracked; it isn't now. ## What to expect on `main` The rebundle path is exercised for the first time under the new tag map, and the tag build will publish the first `:<sha>` rollback image. Scribe #3890 has `ci-runner-f1` watching this merge for `android.yml`'s debug-APK artifact. ## Carrying caveats forward The recommendation numbers are **defensible starting points, not measured optima** — spike #3879 still gates them, and `libraryScaleReference = 5000` is an assumption about a library size nobody recorded. Two known defects stay open: `likes_overlap` assigns a flat `0.6` similarity it never computed (#3879), and four candidate arms order by unseeded `random()` so same-day determinism is accidental (#3889). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
bvandeusen added 11 commits 2026-09-10 23:37:52 -04:00
fix(release): drop version image tags, mint the rollback unit on main
test-go / test (push) Successful in 1m43s
test-web / test (push) Successful in 1m13s
test-go / integration (push) Successful in 4m12s
release / Build signed APK (releases and dev) (push) Successful in 5m11s
release / Build + push container image (push) Successful in 38s
release / Verify release artifacts (tag releases only) (push) Skipped
aeb8781c4e
The image tag map was the inverse of family rules 145 and 147 on every
count: it published :vYYYY.MM.DD.HHMM that nobody pinned, published :main
that rule 147 says should not exist, and published no commit-addressable
image at all — so the rollback unit the rule names did not exist in this
repo. A bad main push had nothing to roll back to but the previous
release tag, which may be many commits back.

The whole map is now:

  dev  → :dev
  main → :latest + :<sha>
  tag  → :latest

A release refreshes the channel and mints nothing else. The tag build
rebuilds the SAME SOURCE as main's build minutes earlier, differing only
in which APK is baked in, so rule 145's immutability clause applies
directly: move the channel tag, never re-push a commit-addressable one.
:latest has to move here rather than waiting for the next main push, or
the channel would carry the previous release's APK indefinitely — a
channel that cannot refresh itself (rule 146).

Two consequences that are not optional:

The verify job asserted the :<version> image existed. With version tags
gone that would fail every release for a tag nothing mints. Re-pointed at
the :<sha> image rather than deleted — deleting it is the tempting way to
make a failing guard go green, and it earns its keep twice now: it still
catches an image push that silently did not happen, and it additionally
proves the ordering, since a tag cut on a commit whose main build never
completed has no rollback target.

The server's self-reported version was the literal string "main" or
"dev". That was survivable while :vYYYY.MM.DD.HHMM existed to identify a
build; with version tags gone it is the ONLY thing that says which build
is running, and two dev images months apart were indistinguishable. It
now carries the derived name from ci/version.sh on every lane, with the
channel as a sibling field (rule 149) rather than folded into the string.
Surfaced at /healthz and beside the version in Settings.

Guards added for each arm of the policy, and every one was falsified
against the specific regression it names before committing. That caught
two real bugs in the guards themselves: stepBody cut at the next
`- name:`, which returns an EMPTY body for the last step in a job and
made the assertions pass vacuously, and its replacement cut at any blank
line followed by indentation, which truncated a step mid-run-block. The
helper now refuses an empty body outright.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
fix(ci): artifacts move to stock upload-artifact@v7 / download-artifact@v8
test-go / test (push) Successful in 1m4s
test-go / integration (push) Successful in 3m54s
android / Build + lint + test (push) Successful in 4m58s
release / Build signed APK (releases and dev) (push) Successful in 5m11s
release / Build + push container image (push) Successful in 1m52s
release / Verify release artifacts (tag releases only) (push) Skipped
8f4b76a638
android.yml's debug upload and release.yml's minstrel-apk pair went
through the bvandeusen fork mirrors, with comments saying stock actions
refuse this hostname, that the pair had to be matched on the bundled
@actions/artifact major, and that download v7 was off-limits for node24.
None of that holds on gitea/runner 3.x: the runner edits the GHES refusal
out of the action bundles, every download major v4-v8 reads every upload
major v4-v7 (Scribe spike #3843, CI-runner run 6312), and every CI image
carries Node 24. The mirror pair itself was last verified at tag run 6286.

Same artifact names, paths and if-no-files-found. ci-requirements.md
drops the pairing table and keeps what is still true: @v3 is invisible.

Scribe snippet #2271, milestone 395.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DwoKYuw3qJmUUYsJeNherB
fix(ci): version derives from the shipped set; untrack an 18MB binary
test-go / test (push) Successful in 1m5s
test-go / integration (push) Successful in 3m30s
release / Build signed APK (releases and dev) (push) Successful in 5m10s
release / Build + push container image (push) Successful in 1m31s
release / Verify release artifacts (tag releases only) (push) Skipped
17212e9eb4
Three build-hygiene fixes that turned up while explaining the pathspec.

**version.sh derives from what SHIPPED.** It read bare HEAD, so any commit
moved the version — including one touching only CI or a README. Rules 148
and 149 both specify the pathspec form. Now a denylist, and the direction
is the point: as an allowlist the list must be updated by whoever adds a
directory and nothing fails if they don't, so the failure mode is a changed
artifact keeping its old version silently on a green run. Inverted, new
content counts by default.

android/ is deliberately NOT excluded, and that is the subtle part. This
repo ships TWO artifacts from ONE derivation: android/ is in no server
image, but it is the APK's entire source, and excluding it would stop an
Android-only commit from moving the APK's own version — the silent
downgrade the versioning rework exists to prevent. So the list is the
union: exclude only what ships in neither, and accept that an Android
commit also nudges the server's reported version. Over-inclusion across the
two, which is the harmless direction. roundtable/roundtable-android each
keep tighter lists because they are one-artifact repos; don't copy theirs.

**.dockerignore excluded the wrong CI directory.** It named .forgejo/ and
.github/, neither of which this repo has. Gitea Actions reads .gitea/, so
the one directory that exists was the one not excluded. The "Flutter mobile
client" block had also lost its PATTERN when flutter_client/ was deleted,
leaving a comment describing an exclusion that was not happening — android/
never took its place, so 4.1MB of Gradle project entered the context and
busted the `COPY . .` layer on every Android-only change. bin/ excluded too.

**bin/minstrel was tracked** — an 18MB binary last refreshed by a commit
about web test mocks, and re-dirtied by every `make build` since. Untracked
and ignored; the file stays on disk.

Guards are behavioural rather than textual: they build throwaway repos with
pinned commit timestamps and run version.sh against them, so they break when
the derivation changes rather than when the wording does. Falsified — drop
the .gitea exclusion and the CI-only commit moves the version; add an
android exclusion and an Android commit stops moving it; exclude everything
and a source commit refuses.

One honest note on the refusal test: the script already refused an empty
result via the downstream date check, so the new explicit check improves the
diagnostic ("no commit touches the shipped file set — shallow clone?") and
not the safety. The test pins the property, which is defended in depth.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
fix(ci): tests do not ship, so they must not re-version an artifact
test-go / test (push) Successful in 1m1s
test-go / integration (push) Successful in 3m17s
release / Build signed APK (releases and dev) (push) Successful in 4m41s
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
270ad7a71b
Completes the pathspec. 17212e9e excluded CI, docs and tooling but left
tests in the shipped set, so its own commit re-versioned the image on the
strength of a _test.go file. `go build` drops *_test.go outright and the
Vite build never imports a .test.ts — neither reaches an image or an APK.

Globs over files rather than a directory exclusion, because this repo has
no tests/ tree to exclude: Go tests sit inline beside the code they cover
(158 files) and the web suite beside its modules (115). Patterns match what
exists and nothing speculative — there are no .spec.* files, no __tests__/
directories and no androidTest/ tree. If any appear they re-version until
named, which is the harmless direction and the point of a denylist.

The guard that matters is not "a test-only commit is inert" — it is that a
commit touching a test AND its source still moves the version. `':!internal'`
would satisfy every inertness assertion while silently excluding the entire
server, which is the stale-version-on-changed-artifact failure this whole
derivation exists to prevent.

Falsified: drop the Go exclusion and a _test.go commit moves the version;
drop the web one and a .test.ts does; replace the globs with `':!internal'`
and the source-alongside-test case breaks.

That last check failed first time, on a bug in the FIXTURE rather than the
derivation, and it is worth recording because it makes a test pass for the
wrong reason. Both commit helpers wrote the constant "x\n", so re-writing a
file with identical bytes recorded NOTHING — the "source and test together"
commit actually contained only the test, and the assertion was quietly
checking the case it was meant to contrast against. Content is now derived
from the commit's epoch, and the test asserts HEAD really contains both
paths before drawing any conclusion from it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
fix(recommendation): Songs-like gets its own profile so it stops wandering
test-web / test (push) Successful in 1m7s
test-go / test (push) Successful in 1m31s
test-go / integration (push) Failing after 4m21s
release / Build signed APK (releases and dev) (push) Successful in 5m11s
release / Build + push container image (push) Successful in 1m52s
release / Verify release artifacts (tag releases only) (push) Skipped
f367eeaa9d
Operator, 2026-09-10: "when I play it I'm expecting to get a consistent
sound and style from the experience... I was getting a seeming wide variety
of music from each one when I was hoping to stay in a certain neighborhood."

Songs-like shared the `daily_mix` weight profile with For-You, and that
sharing WAS the bug. The two surfaces want opposite things: For-You answers
"what will they enjoy today" and is supposed to roam; Songs-like answers
"what sounds like THIS". Under one profile the broad answer wins.

The arithmetic, from the shared weights:

    unrelated track, liked, not played recently → 1.0 + 2.0 + 1.0 = 4.0
    PERFECT similarity match, not liked         → 1.0 + 1.5       = 2.5

Liking something outranked sounding like the seed, because LikeBoost (2.0)
exceeded SimilarityWeight's whole range (1.5) and TasteWeight (1.5, and
seed-INDEPENDENT) matched it outright. Under the new profile the same pair
scores 5.00 vs 2.00.

Two levers, because either alone leaves the other's failure intact:

POOL. Songs-like now takes its own CandidateSourceLimits. The default gave
~29% of candidates a sim_score of literally zero — `taste_overlap` and
`random_fill` are both `0.0::float8` in recommendation.sql, seed-independent
by construction. Same total pool size; composition shifts to arms that
measure distance from the seed, LBSimilar doubled.

WEIGHTS. A third profile beside radio and daily_mix, DB-backed and live per
rule 25, with the property that similarity's range exceeds the combined
range of every seed-independent differentiator — so a closer match cannot
be beaten on likes, freshness and taste alone, while tracks within ~0.39
similarity of each other still get ordered by what the user likes.

Rule 131 changed the pool design mid-way and for the better. Zeroing the
two seed-independent arms was the first instinct and is exactly the
vanish-or-nothing shape that rule forbids: a seed with thin ListenBrainz
coverage would yield a short mix or none. They are the tier-3 FLOOR — cut
hard, never removed — and the weights keep them at the bottom of the
ranking rather than out of the pool. "A few tracks further from the seed
than we'd like" beats "no playlist".

Caught while wiring it: switching only pickTopN's final Score would have
been nearly INERT. scoreAndSortCandidates does the selection sort, and the
caller caps and truncates in that order — so the playlist would still have
been chosen by daily_mix and merely relabelled with songs_like numbers. It
now takes the profile as a parameter, and each surface passes its own.

Also corrects the daily_mix card's blurb, which claimed Songs-like as one
of its surfaces and no longer is.

Guards pin behaviour rather than the numbers, since numbers get retuned:
that similarity beats an unrelated liked track, that daily_mix still
DOESN'T (or the split buys nothing), that the tier-3 floor is non-zero,
and that the UI card shows its own values rather than falling back. Each
falsified against its named regression first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
fix(recommendation): don't shrink a candidate arm ordered by unseeded random()
test-go / test (push) Successful in 1m10s
test-go / integration (push) Successful in 3m35s
release / Build signed APK (releases and dev) (push) Successful in 5m6s
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
ecfa056d4d
Fixes the integration failure from f367eeaa:
"same-day rebuild produced different track lists".

Cutting RandomFill 30→10 for Songs-like broke
TestBuildSystemPlaylists_DailyNonceDeterminism, and the reason is worth
stating because the number is not the bug.

`likes_overlap` and `random_fill` end in a bare `ORDER BY random()` with no
daily seed (recommendation.sql:118, :161). Such an arm returns a STABLE set
only while its LIMIT exceeds the rows eligible for it — then it returns all
of them, and the random order stops mattering because scoreAndSortCandidates
sorts by track id before drawing jitter. Below that threshold the arm
returns a random SUBSET, and two builds on the same day draw different ones.

So the test was green by accident. It seeds ~20 tracks against a default
RandomFill of 30; the limit exceeded the library, so the arm returned
everything. Determinism held for a reason unrelated to the code being right.

Which means it does NOT hold in production. Any real library is larger than
30, so same-day rebuilds have been drawing different mixes since that arm
was written — invisible, because a mix changing after a refresh looks like a
feature. Filed as #3889; the fix is a seeded ordering per (user, day), which
needs a .sql change and sqlc regeneration and so cannot land from here.

The correction: grow an arm freely, never shrink one whose ordering is
unseeded random. LikesOverlap and RandomFill go back to the defaults;
TasteOverlap stays halved because it sorts by `tpa.weight DESC, t.id` and is
genuinely deterministic. Guarded by a test that names the reasoning, so the
next person to trim these has to read why first — and it should be DELETED
once #3889 lands rather than worked around.

The cost is honest: the seed-independent share of the Songs-like pool falls
from 29% to 20% instead of the intended cut. That matters less than it
sounds. The pool only biases the draw; the songs_like WEIGHTS are what
actually demote sim_score-0 candidates, and they are untouched here — a
perfect match still scores 5.00 against an unrelated favourite's 2.00.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
feat(recommendation): Songs-like can include the seed artist's own music
test-go / test (push) Successful in 1m16s
test-go / integration (push) Failing after 3m55s
release / Build signed APK (releases and dev) (push) Successful in 5m11s
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
31190657d8
Operator, 2026-09-10: "it should also be able to include music from the same
artist." Completes #3881 — the weights and pool landed in f367eeaa; this is
the eligibility half.

produceSeedMixes filtered the seed artist out entirely:

    // "Songs like X" excludes X's own songs.
    if !pgtypeUUIDEqual(c.Track.ArtistID, artistID) { ... }

That reads as obviously right and is not. The seed is a TRACK — the artist's
top-played one — and the tracks most likely to sound like it are usually the
rest of that artist's catalogue. The filter threw away the seed's nearest
neighbours, then reached FURTHER OUT to replace them. On the one surface
whose job is staying in a neighbourhood, that is backwards, and it worked
against the coherence tuning rather than with it.

Domination is bounded by the cap instead of by exclusion, which is the
distinction that makes this safe rather than a new problem:
capCandidatesByAlbumAndArtist already allows at most 3 tracks per artist in
a 25-track mix, so the seed artist gets 12% at most — a presence, not a
takeover. Without that bound this would just be the radio failure (#3882)
arriving on a different surface. The seed track itself still cannot appear;
it is passed to LoadCandidatesFromSimilarity as an exclusion.

Guarded end-to-end rather than by reading the source, for two reasons: the
check has to survive the filter returning in a different shape, and an
absence check would now match the comment that explains why the filter is
gone — rule 167's prose trap exactly. The test asserts both directions, that
at least one mix contains its seed artist and that none exceeds the cap.

Its falsification is by construction rather than by execution: under the
previous code every mix's own-artist count was necessarily zero, so the
assertion could not have passed. Running it needs Postgres, which is the
integration lane's job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
test(playlists): the same-artist guard needed a fixture that has same artists
test-go / test (push) Successful in 1m25s
test-go / integration (push) Successful in 4m46s
release / Build signed APK (releases and dev) (push) Successful in 5m45s
release / Build + push container image (push) Successful in 17s
release / Verify release artifacts (tag releases only) (push) Skipped
f5dd4462de
Fixes the integration failure from 31190657. The test was wrong, not the
code: it could not have passed whatever produceSeedMixes did.

seedActiveLibrary builds its tracks through seedTrack, whose own comment
says "artist and album are not deduplicated across calls (mbid-less
upsert)". So every track gets a fresh artist row despite sharing a name —
4 artists x 5 tracks is really 20 artists with one track each. A seed
artist's only track IS the seed, which is excluded from its own mix, so
"does this mix contain a track by its seed artist" was structurally
answerable only as no.

That is the failure mode worth naming: the assertion was measuring the
fixture, not the behaviour, and it reported the behaviour as broken.

seedSharedArtistLibrary upserts each artist ONCE and reuses the id across
its tracks, so a seed artist genuinely owns five others. Albums are still
not deduplicated, which suits this test — the per-album cap never binds, so
the per-artist cap (3) is unambiguously what is under test. Noted in the
fixture, because "tidying" the album titles into something shared would
silently change which cap the assertion measures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
fix(radio): cap any one artist's share of a radio session
test-go / test (push) Successful in 1m13s
test-go / integration (push) Successful in 3m53s
release / Build signed APK (releases and dev) (push) Successful in 5m9s
release / Build + push container image (push) Successful in 1m52s
release / Verify release artifacts (tag releases only) (push) Skipped
633d4f591f
Operator, 2026-09-10: started radio from a song and "literally all of the
songs in the playlist after that were from a single artist which was not
expected."

There was no per-artist cap anywhere in the radio path. radio.go built the
pool and handed it straight to Shuffle, which scores, sorts and takes the
top N — nothing between those steps bounded any artist's share, so a pool
dominated by one artist produced an output dominated by it. The asymmetry
was the tell: discover.go, you_might_like.go and home.go all cap; radio
never got one.

With the fixture that reproduces it — 20 liked tracks by one artist plus 10
by ten others — the old path returns 10 tracks from 1 artist. It now returns
10 from 8.

TWO PASSES, and that is the whole design. A hard cap was the easy mistake:
radio asks for 50 tracks by default and 200 at most, so capping at three per
artist over a concentrated pool would hand back a six-track "radio". Pass
one takes candidates that fit under the caps; pass two fills any remaining
slots from those it skipped, still in score order. The result always holds
min(limit, len(candidates)) — the caps change WHICH tracks are picked, never
HOW MANY. Rule 131's principle past the system mixes it was written for.

The caps SCALE with the requested length rather than being a constant.
Three-per-artist is a sensible 12% of a 25-track mix and an absurd 1.5% of a
200-track radio, where every selection would sit in the relaxation path and
the cap would be decorative. RadioDiversityCaps holds the system mixes'
proportion at any length: 3/2 at 25, 6/4 at 50, 24/16 at 200, with floors so
a very short radio is not capped down to one track per artist.

A BOUND, NOT AN EXCLUSION — the operator asked for the opposite of removal:
"again it should be able to add songs from the same artist." The dominant
artist still appears, just not exclusively. Guarded, because the tempting
wrong fix is the filter songs-like used to carry.

Shuffle grew the parameter rather than gaining a capped twin: radio is its
only production caller, so a second function would have left the original
dead (rule 22).

Falsified against each named regression: uncapped gives 10/10 to one artist;
a hard cap returns 3 of 10 on a single-artist pool; a cap-as-exclusion drops
the artist entirely; a fixed cap stays 3 where the scaled one reaches 24.

Caught while writing the guards: the artist-key constant was hand-written
hex and wrong — the fixture's artist UUID carries 0001 in its fourth group,
so the lookup missed and the assertion measured nothing. Derived from the
same construction the fixture uses now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
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
721154847e
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
fix(recommendation): a nil LibrarySize must degrade, not panic
test-go / test (push) Successful in 1m4s
test-go / integration (push) Successful in 3m41s
release / Build signed APK (releases and dev) (push) Successful in 4m37s
release / Build + push container image (push) Successful in 1m58s
release / Verify release artifacts (tag releases only) (push) Skipped
4ce47397a9
Fixes the integration failure from 72115484: a SIGSEGV inside handleRadio
took down TestHandleRadio_ColdStart_OnlySeedReturned.

    recommendation.(*LibrarySize).Get(0x0, ...)
      library_scale.go:146
    api.(*handlers).handleRadio(...)
      radio.go:95

internal/api builds its handlers struct directly in a dozen tests, none of
which know about every field, so librarySize arrives nil there. Get took
l.mu.Lock() straight off the nil receiver.

The shape of the bug is what matters more than the nil check. This value's
entire contract is that it degrades — an errored count keeps the last known
number, a never-counted cache returns 0, and 0 scales to the base limits,
i.e. today's behaviour. A pool-sizing HINT then turned a request into a
crash, which is the precise opposite of that.

A nil receiver is now VALID and means "no cache": the count still runs, it
is just not memoised. Correct-but-uncached rather than zero, so a wiring
miss in production would cost a query per request, not silently unscale
every pool — a performance bug is findable, a quietly-wrong pool is not.

Patching the test constructors was the alternative and is worse: a dozen
call sites, and the next test to build a handlers literal reintroduces it.

Guarded with the nil path exercised directly, including that it counts
again rather than memoising, and still returns 0 on a failed count. The old
shape fails it by panicking.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
bvandeusen merged commit f70df9f827 into main 2026-09-10 23:37:57 -04:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bvandeusen/minstrel#131