Discover request surface — taste-aware, rotating, snoozable, tag-targeted (milestone #268) #116
Merged
bvandeusen
merged 14 commits from 2026-08-03 08:38:25 -04:00
dev into main
14
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  bvandeusenandClaude Opus 5
|
eec59193fa |
feat(discover): explain the taste match on both clients — #2377 (clients)
"Matches your taste in shoegaze and dream pop." replaces the seed
attribution when the candidate's own tags overlap the taste profile.
The preference order is the point of slice 6: the tag reason describes the
MUSIC ("sounds like what you like"), while seed attribution describes the
graph ("adjacent to something you played"). When we can say the former, it
is strictly the better explanation. When we can't — the common case, since
tag coverage for out-of-library artists is partial by nature (#2376) — the
card falls back to attribution rather than going blank.
Both clients share the wording, Oxford comma included, and both have tests
asserting the exact strings. That's deliberate: identical copy across two
codebases silently diverges unless something fails when it does.
Android caps at 3 tags client-side even though the server already does.
The server contract could widen; a run-on subtitle shouldn't be how we
find out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
bvandeusenandClaude Opus 5
|
ca4832e620 |
feat(discover): Discover tuning card on the admin lab — #2377 (web admin)
test-web / test (push) Successful in 35s
Rule #25/#27: the two knobs slice 6 added server-side are now touchable — taste-tag weight and snooze length, with deviation dots, save, and reset, matching the existing profile/taste cards. Copy states what each knob does AND what it doesn't: the tag-weight hint says 0 turns the term off and that an untagged candidate is never penalised, and the snooze hint says it records no opinion about the artist and never feeds the taste profile. Those are the two properties most likely to be assumed backwards by whoever turns these next. Also fixed a latent fragility the new card exposed rather than caused: all three reset buttons had the accessible name "Reset to defaults", so the existing test picked the LAST one and assumed that meant taste. Adding a card below it would have silently retargeted that assertion at the wrong scope. Each reset button now names its scope — better for screen readers too, since three identical buttons on one page is a real a11y defect — and the test selects by name instead of position. The page's test fixture needed the new `discover` key in both `snapshot` and `shipped`: the `as TuningSnapshot` cast means a missing field is not a compile error, it's every test on the page throwing inside fillForm. Noted that in the fixture so the next scope doesn't rediscover it. Includes a test that a weight of 0 is actually SENT rather than dropped as falsy — the off switch is the one value a truthiness bug would eat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
cf0d37bf8e |
fix(discover): compare against a baseline run, not a hardcoded score — #2377
TestSuggestArtists_UntaggedCandidateSurvivesAlongsideTagged asserted the untagged candidate's score was 0.9 — the raw similarity value I'd seeded. It's actually 1.61, because the pool score is signal-weighted by the seed query: ln(1+signal) x similarity, and a liked seed carries signal 5, so ln(6) x 0.9. The assertion was testing the seeding arithmetic, which is a different layer and not what the test is about. Rewritten to run the same request twice — once with the tag term disabled, once enabled — and assert the untagged candidate's score is IDENTICAL across both. That states the real property (the blend leaves untagged candidates alone) without depending on how the pool score is derived, so it survives future changes to seeding. Added a sanity assertion that the TAGGED candidate's score did move, so the comparison can't pass by both runs being trivially identical — the same "a test that cannot fail" trap recorded for this milestone. Exact-preservation at the arithmetic level is already covered where it belongs, by TestApplyTagOverlap_UntaggedCandidateScoreIsUnchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
799dab029a |
feat(discover): rank suggestions by taste-tag overlap — #2377 (server)
The payoff slice. Until now a candidate's only claim on a slot was "some
artist you play is adjacent to it in a similarity graph" — a fact that says
nothing about whether the music sounds like anything you like. Now the
candidate's own folksonomy tags (cached by slice 5) are compared against
the user's taste-profile tags, so the deck ranks on taste and can say WHY.
The blend is MULTIPLICATIVE — score × (1 + weight × overlap) — and that
choice carries the whole safety argument:
- An untagged candidate has overlap 0, so its score is EXACTLY unchanged.
Tag coverage is permanently partial (#2376); it must cost a candidate
nothing, not sink it (rule #131).
- Nothing can leapfrog on tags alone. An additive term with a large
weight would let a near-zero-similarity artist outrank a strong match
for sharing one popular tag, which reads as noise.
- Weight 0 restores pure similarity order bit-for-bit, so the operator's
knob has a real off position.
overlap = Σ(shared) candWeight × normalizedTasteWeight ÷ Σ(all) candWeight.
Normalizing the taste side by the user's strongest tag makes the score
comparable across users (taste weights accumulate with listening, so a
heavy listener's raw numbers dwarf a new user's while meaning the same
thing). Dividing by the candidate's own mass makes it comparable across
candidates, so a densely-tagged artist can't win on tag count alone.
Applied to the whole over-fetched pool BEFORE selectSuggestions, so the
rotation and diversity rules operate on blended scores — boosting only the
twelve already chosen by similarity would leave the re-ranking undone.
A query failure is returned, NOT degraded past. Graceful degradation is
for expected absence (no taste profile, no cached tags) and both are
handled explicitly as empty inputs; swallowing a real error would hide a
broken DB behind a subtly worse ranking that nothing reports.
Migration 0051 adds a FOURTH tuning scope rather than columns on
taste_tuning, because snooze_days lives here too and a snooze must never
be read as taste signal (#2374) — filing it under 'taste' would put it one
careless join from the leak that design forbids. Expanding
recommendation_tuning_audit's CHECK is in the same migration per rule #36,
and a test asserts the audit row lands, which is what would catch its
absence.
snooze_days moves out of a Go constant onto the tuning card (rule #25),
closing the deferral from #2374.
Tag-overlap tests use deliberately SKEWED fixtures: an evenly-matching pool
cannot exercise a re-ranking, since every candidate gets the same
multiplier and the order is unchanged whether the blend works or not.
Admin UI + client attribution follow in this batch — rule #27.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
bvandeusenandClaude Opus 5
|
7315e37c15 |
fix(db): apply sqlc's actual output for candidate_artist_tags — #2376
Three divergences in the hand-written generated file, all caught by
verify-generate on the first run. Two are sqlc rules I had wrong:
1. When a query's SELECT list exactly matches a table's columns in order,
sqlc REUSES the model struct rather than emitting a bespoke Row type.
So ListCandidateArtistTagsForMbids returns []CandidateArtistTag, and
ListCandidateArtistTagsForMbidsRow should never have existed.
2. models.go is ordered by GO STRUCT NAME, not table name. Table order
would put candidate_artist_tag_state before candidate_artist_tags;
sqlc emits CandidateArtistTag before CandidateArtistTagState. The
earlier slice-3 observation ("ordered by table name") was consistent
with both orderings and so never discriminated — this case does.
3. sqlc smart-quotes a doubled '' inside a promoted comment into a
typographic ”. Reworded the prose to say "the empty string" instead of
encoding a mangling into the source.
Note the integration lane PASSED on the broken push while this failed.
That is #2380's lesson landing again, and the reason the check exists:
valid SQL executing against real Postgres proves nothing about whether
the committed Go matches its source.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
bvandeusenandClaude Opus 5
|
4f9b083eec |
feat(discover): artist-tag cache for out-of-library candidates — #2376
Migration 0050 adds candidate_artist_tags + candidate_artist_tag_state: folksonomy tags for artists NOT in the library, which track_tags cannot hold because it's FK'd to tracks(id) and a Discover candidate has no local row. Slice 6 ranks against these; this slice only fills the cache. The reuse the task claimed is real and verified: MusicBrainz's fetchEntityTags(ctx, "artist", mbid, scale) already existed for the #1519 recording→artist fallback, so FetchArtistTags is a thin wrapper. Two subtleties it does NOT inherit: - Weight scale is 1.0, not artistTagWeightFactor (0.6). That discount exists because FetchTrackTags uses artist tags as a *proxy* for a track's; here the artist IS the subject. Applying it would make these weights incomparable with track_tags — exactly the comparison slice 6 depends on. Pinned by a test. - fetchEntityTags reports existing-but-untagged as (empty, nil) so the track path can fall through. There's no next level here, so empty becomes the terminal ErrNotFound; otherwise the enricher would settle a candidate as "enriched" with zero tags. ArtistTagProvider is the split TrackTagProvider's own doc comment anticipated ("e.g. artist-level tags"). Last.fm gains artist.getTopTags, which returns the same toptags envelope, so the response type and normalizer are reused unchanged. Rather than write the merge-and-classify loop twice, extracted it from EnrichTrack into runChain(). The ErrNotFound-vs-transient split is the load-bearing part — those lead to opposite persistence decisions — so it now has direct unit tests it never had while inlined. Bookkeeping is a separate table, not columns, because the "providers had nothing" outcome must be recordable for a candidate with zero tag rows, and there is no per-candidate row to hang columns off ( artist_similarity_unmatched holds many rows per candidate). Absence of a state row means "never processed", so a transient failure writes nothing and stays eligible. Two capacity realities are designed for, not papered over: - The pool is O(library artists x neighbours) and MusicBrainz allows ~1 req/s, so it can never drain in one pass. The eligibility query returns candidates in descending summed-similarity order, so the ones that can actually reach a deck are enriched first. - candidateBatch (50) is smaller than the track batch (200): tracks are finite and drain to completion, candidates are effectively unbounded and would otherwise starve the track arm forever. GC sweeps both tables — the similarity feed churns, and a candidate that joins the library has its tags in track_tags now. Tags swept before state so a mid-sweep crash leaves a valid state, not a re-fetch loop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
f17356560d |
fix(discover): "in about a month" was unreachable in both clients — #2375
The days→months threshold (45) sat above the divisor (30), so a rounded month count of 1 — which needs 15..44 days — could never be reached: every one of those day counts hit the `in N days` branch first. The singular branch was dead code on Android AND web. Lowered the threshold to 30 in both clients, which makes 30..44 days read "in about a month" instead of "in 44 days", and documented the invariant (threshold must not exceed the divisor) next to each constant so the two can't drift apart again. Found by the unit test written for that branch, which is the whole reason to assert on copy that looks obviously correct. Both suites now pin the seam from both sides — 29 days and 30 days — so the branch can't go dead again silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
18a61f1065 |
fix(discover): complete the page-test mock + drop a return from returnsIn — #2375
Two CI failures from
|
||
|
bvandeusenandClaude Opus 5
|
6e39471a70 |
feat(discover): snooze affordance on Android + web suggestion cards — #2375
Completes the snooze from slice 3 (#2374), so it's now touchable on both clients (rule #27 — the server side alone was never shippable). Copy is "Not right now" everywhere, never a dislike (rule #101). The parked list even says so out loud: "Nothing here counts against your taste profile." Both clients flip the card in place to a "Not right now" state with an Undo, rather than yanking it out of the grid under the cursor. The row leaves on the next refetch; the persistent way back is a parked-list section below the deck. That list isn't optional garnish — a snoozed candidate is by definition absent from the deck, so without it the DELETE endpoint is unreachable. Android routes the write through the offline MutationQueue per rule #100, as ONE toggle kind (SUGGESTION_SNOOZE_TOGGLE) carrying the desired state rather than two action kinds. That reuses the LIKE_TOGGLE collapse: a queued snooze the user has since undone is dropped unsent instead of replaying after the undo and re-hiding an artist they asked to see. The collapse helper is now a pure top-level function so that rule is unit tested rather than inferred. The repository does NOT enqueue on a 4xx — a permanent rejection would replay to the same failure and would raise a misleading "will sync when online" hint. The common case is a 404 from un-snoozing a row that already lapsed, which is the user's intended end state anyway. Also: an empty deck used to have one meaning (no listening signal yet). It can now also mean "you parked them all", so the empty copy branches — telling that user to go listen to something would be wrong advice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
86af79bd2f |
feat(discover): time-boxed suggestion snooze, server side — #2374
Migration 0049 adds suggestion_snoozes(user_id, candidate_mbid, candidate_name, snoozed_until), and SuggestArtistsForUser excludes rows whose snooze hasn't expired. This is NOT a dislike. Rule #101 forbids a "Not for me" / thumbs-down UI; a snooze is the approved shape instead because it records no verdict on the music, expires on its own (~90d), and never reaches the taste profile. It's acquisition triage — "not right now" — so the filter sits at the candidate stage rather than in the score, where it would become a ranking signal by the back door. Per-user throughout (rule #47): one household member parking a candidate leaves everyone else's deck untouched. candidate_name is denormalized because suggestions are out-of-library by definition — there is no artists row to resolve a display name from, and the un-snooze list has to show something. That list is why GET /discover/snoozes exists at all: a parked candidate is by definition absent from the deck, so without it the DELETE would be unreachable. Also fixes a hole in the codegen check from #2380: `git diff` ignores untracked paths, so a brand-new generated file would have passed it silently. `git add -N` first. This commit is the first to add one. Endpoints: POST /api/discover/suggestions/{mbid}/snooze (body: name, days) DELETE /api/discover/suggestions/{mbid}/snooze GET /api/discover/snoozes UI lands in slice 4 (#2375) before any of this merges — rule #27. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
e006de5d4b |
fix(db): apply sqlc's actual output for SuggestArtistsForUser — #2380
The new codegen check failed on its first run, against the slice-1 hand-edit, which is precisely why it landed on its own commit. What I got wrong: sqlc does not embed the leading `--` header block in the SQL const. It strips those lines and promotes them to the generated method's Go doc comment, gofmt-formatted — blank `//` separators around the indented list, tabs for the indent. My hand-edit left the header inside the string AND left the stale M5c doc comment sitting on the function, so the generated file described behaviour the query no longer had. Comments *inside* the statement body are kept as-is; only the header block moves. Worth knowing before slices 5 and 6 add more queries. Taken verbatim from the diff the check printed, which is the reason it prints before asserting. Round-trip cost: one CI run, no guessing. Note the integration lane passed on the previous push even with the wrong generated file — the SQL text was valid and the signature was unchanged, so executing it against real Postgres proved nothing about whether the committed Go matched its source. That gap is exactly what #2380 closes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
94e2cac03b |
ci(go): verify committed sqlc output matches its .sql sources — #2380
internal/db/dbq is 39 files and ~12k lines of generated Go covering 307 queries, and nothing checked that it still matched internal/db/queries. test-go.yml referenced sqlc.yaml only as a path trigger; sqlc never ran. So a hand-edit, a half-applied regen, or a migration changed without a regen would all pass CI while the typed layer quietly lied about the SQL underneath it — which is the single thing adopting sqlc is supposed to buy. This session's slice-1 change is an instance: its SQL const was verified byte-identical against its own .sql source by script, but never against what sqlc would actually emit. Nothing in the repo could have told the difference. make verify-generate runs ahead of vet/lint/test, because if the typed layer disagrees with its sources then everything downstream is testing a lie. generate-go runs sqlc as a Go tool rather than a container: the ci-go image already has Go, so this avoids docker-in-docker on the runner. It's pinned to the same SQLC_VERSION as the existing containerised `generate`, so both routes emit identical output and there is one version to bump — now annotated for Renovate per rule #44. The diff prints BEFORE the exit-code check on purpose. On failure the log then holds sqlc's exact expected output, so correcting it is a copy rather than a guess. That is also what makes new queries workable without installing anything: this workstation has neither Go nor sqlc. Makefile joins the workflow's paths:. Without it a Makefile-only change — including this one — would not trigger the workflow that now depends on it. Same class as #2204, where CI never ran on plugin/** changes. Landing this on its own, ahead of slice 3, so that if it fails it is unambiguous whether the drift came from slice 1 or from new code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
b27029f674 |
feat(discover): rotate the suggestion deck daily + cap one seed's share — #2373
Second half of the reported symptom: suggestions "show the same artists until you request one". The ranking was `ORDER BY total_score DESC` with no randomization and no seen-state, so the only things that could ever change the deck were a candidate entering the library or the user filing a request. The tail of the ranking was unreachable — requesting was literally the only lever. No SQL change was needed. The query already takes a limit, so it over-fetches a pool (4x the slots, capped at 60) and the selection moves to Go, where it is a pure function of (pool, limit, day) — no DB, no clock — and therefore unit testable in the fast lane instead of behind the integration gate. Three rules. The best few by score always lead, so the strongest matches never rotate out of sight (For You's head/tail shape). The remaining slots are drawn by md5(mbid + day), the same daily-stable idiom the Home rows already use: stable within a day so pull-to-refresh doesn't reshuffle, different tomorrow, and no stored state. And a per-seed cap keeps roughly a quarter of the deck attributable to any one seed artist, so twelve neighbours of a single artist can't be the whole surface. The cap is a preference, not a quota. A user whose pool hangs off one or two seeds would otherwise get a three-card surface — worse than the monoculture being avoided, and exactly the vanish-or-nothing shape rule #131 exists to prevent — so a short deck tops up in score order from what the cap set aside. This is also what keeps the existing Top12Cap integration test honest: its 30 candidates share one seed, and without the top-up it would return 3. Eight unit tests, including one that had to be rewritten mid-change: the first version asserted the cap against an evenly-spread pool, where the top-N is already diverse and the assertion could not fail. It now uses a skewed pool where one seed owns the entire top of the ranking, which is the only shape that actually exercises a cap. Dropped two //nolint:gosec directives added in passing — gosec isn't in .golangci.yml, so they suppressed nothing and only implied a check that runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bvandeusenandClaude Opus 5
|
14aa22198f |
feat(discover): seed request suggestions from the taste profile — #2372
The Discover request surface was the one recommendation surface still on its M5c implementation from early May. #796's taste profile, #1488's taste_unheard bucket and #1490's folksonomy enrichment all modernized in-library surfaces; this one was never in scope for any of them, so it still projected raw likes + plays through artist_similarity_unmatched. Two defects fall out of that signal, `5*liked + Σexp(-age/halflife)` summed over every play of the artist. It is unbounded, and contribution is signal × similarity — so a handful of heavily-played artists monopolize all twelve slots, and their share GROWS the more the user listens. The surface entrenched harder the better it knew you, which is exactly backwards and matches the reported "goes stale once it has a strong signal of your taste". It also counted every play_event with no was_skipped filter, so skipping an artist repeatedly INCREASED its signal and pushed more of its neighbours at the user. ListMostPlayedTracksForUser and the taste engine both filter skips; this query was the odd one out. Seeds now come from taste_profile_artists.weight, which the taste engine has already engagement-graded, time-decayed and signed — an artist the user drifted away from stops contributing instead of accumulating forever, and can even contribute negatively. Tiered per rule #131 rather than hard-switched: tier 1 is the profile, tier 2 is likes + completed plays for a user who has no profile rows yet (new account, or before the first daily recompute), so the surface never empties. The old unfiltered-play signal is gone, not kept behind a toggle. The signal is also log-damped, so one artist cannot take every slot even when its weight dwarfs the rest. $2 stays wired to the tier-2 decay: it is genuinely still used there, and dropping the parameter would have changed the generated signature. sqlc's image is not on this workstation and the change preserves the query signature exactly — same three params, same seven columns — so only the embedded SQL const moves. Both copies are edited and verified byte-identical rather than pulling a container onto the operator's machine; a malformed query fails the integration lane loudly, which is the real check either way. Four integration tests cover what changed: a taste weight alone seeds with no like or play; tier 2 does not run alongside tier 1; a non-positive weight never seeds (guarded by a second positive row, so an empty tier 1 can't make it pass for the wrong reason); and skip-only history seeds nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |