Recommendation metrics: publish the margin of error on every delta #122

2026-08-06T21:50:32-04:00

bvandeusen commented

2026-08-06 21:50:32 -04:00

Closes #2495 and #2524. Four commits, no schema migration. Server + web.

CI green on 304de88c (web, run 3491). The Go lane was green at 481f9060 (run 3486) and nothing Go or SQL changed after it, so both lanes hold at HEAD.

#2495 — the card invited a wrong conclusion

recMetricsLowVolume = 20 was doing two jobs. Twenty plays is enough to be worth displaying — below that a skip rate is anecdote — but the card then presented deltas as though it were also a decision floor. Those differ by an order of magnitude: detecting the ~13pp differences that matter needs ~133 plays per arm for 80% power at α=0.05.

So Discover's taste-matched (59 plays) and random-unheard (70) both rendered as full-confidence rows with a bold delta beside them, and that comparison sits at p ≈ 0.06. The card said "signal"; the arithmetic said "maybe". It produced a recommendation the data didn't support, and any reader with the same numbers would have made the same call.

Every surface and every breakdown row now carries skip_delta / completion_delta as {delta_pp, margin_pp, distinguishable}. Breakdown arms are the thinnest samples on screen and where the old card misled most.

Computed server-side so both clients read one implementation rather than each re-deriving it. Skip rate is a two-proportion difference; completion is Welch, which needs a variance — hence completion_sqsum in the query. It's the sum of squares rather than stddev_samp on purpose: raw source rows are merged into surface families in Go, and sums of squares combine across groups exactly whereas standard deviations cannot.

Edge cases that are refusals rather than numbers: an empty sample, and two degenerate rates (both 0 or both 1) whose zero SE would report a spuriously narrow margin — and therefore "distinguishable" — for a delta of exactly zero. sampleVariance is clamped at zero because floating-point cancellation can push it slightly negative when every observation is identical, which would become NaN in the square root.

recMetricsLowVolume is untouched. "Too thin to show" and "too thin to act on" are different questions and now have different answers.

Web: an indistinguishable delta renders dimmed and prefixed ≈, with its range on hover and a legend explaining the glyph. Colour is withheld unless the delta clears its margin — colouring noise red is what made the old card misleading.

#2495 — the admin trends view had it worse

/admin/tuning's "Latest skip"/"Latest completion" are one week while the adjacent Plays column is the whole window. I misread exactly that and briefly concluded Deep cuts was the worst surface, from ~17 plays in a single week; over 180 days it's one of the best.

Headers now read Plays (window) / Skip (last wk) / Completion (last wk), and the skip cell carries that week's play count so a rate drawn from a handful of listens reads as what it is.

#2524 — duplicate-MBID log noise

resolveArtist now recognises an isUniqueViolation on SetArtistMbidIfNull as the expected condition it is, matching resolveAlbum. Two rows mapping to one MusicBrainz artist is a merge candidate, not a fault; without the branch it logged a generic warning plus a Postgres ERROR line on every scan, which teaches an operator to ignore database errors. Three lines, and isUniqueViolation already existed.

Tests

12 Go cases. The load-bearing one, TestProportionDelta_ReproducesTheDiscoverCase, asserts the exact 59-vs-70-plays / 13.3pp comparison comes back not distinguishable — the false confidence is now a regression test rather than a comment. Paired with a same-effect-at-10×-volume case proving the flag tracks sample size, not gap size.

2 web cases, targeting deltas by data-testid.

Three of these four commits are test fixes for my own new tests: testing-library queries that matched more elements than intended (a <td>'s own direct text nodes still match despite a child span; /≈/ matched the legend explaining the glyph; /^Skip/ matched the sparkline column too). Kept as separate commits because each records a distinct trap. The underlying cause was iterating against CI rather than locally — the web lane uses the same ci-go image, so npx vitest run works locally in ~7s, and note #2465 has been corrected to say so.

Verified locally before the final push: svelte-check 0 errors, 110 files / 786 tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N6vZoJ4Se5YyaqdtGVkap5

Closes #2495 and #2524. Four commits, **no schema migration**. Server + web. CI green on `304de88c` (web, run 3491). The Go lane was green at `481f9060` (run 3486) and nothing Go or SQL changed after it, so both lanes hold at HEAD. ## #2495 — the card invited a wrong conclusion `recMetricsLowVolume = 20` was doing two jobs. Twenty plays is enough to be worth **displaying** — below that a skip rate is anecdote — but the card then presented deltas as though it were also a **decision** floor. Those differ by an order of magnitude: detecting the ~13pp differences that matter needs ~133 plays per arm for 80% power at α=0.05. So Discover's taste-matched (59 plays) and random-unheard (70) both rendered as full-confidence rows with a bold delta beside them, and that comparison sits at p ≈ 0.06. The card said "signal"; the arithmetic said "maybe". It produced a recommendation the data didn't support, and any reader with the same numbers would have made the same call. Every surface **and every breakdown row** now carries `skip_delta` / `completion_delta` as `{delta_pp, margin_pp, distinguishable}`. Breakdown arms are the thinnest samples on screen and where the old card misled most. Computed server-side so both clients read one implementation rather than each re-deriving it. Skip rate is a two-proportion difference; completion is Welch, which needs a variance — hence `completion_sqsum` in the query. It's the **sum of squares** rather than `stddev_samp` on purpose: raw source rows are merged into surface families in Go, and sums of squares combine across groups exactly whereas standard deviations cannot. Edge cases that are refusals rather than numbers: an empty sample, and two degenerate rates (both 0 or both 1) whose zero SE would report a spuriously narrow margin — and therefore "distinguishable" — for a delta of exactly zero. `sampleVariance` is clamped at zero because floating-point cancellation can push it slightly negative when every observation is identical, which would become NaN in the square root. `recMetricsLowVolume` is untouched. "Too thin to show" and "too thin to act on" are different questions and now have different answers. **Web:** an indistinguishable delta renders dimmed and prefixed `≈`, with its range on hover and a legend explaining the glyph. Colour is withheld unless the delta clears its margin — colouring noise red is what made the old card misleading. ## #2495 — the admin trends view had it worse `/admin/tuning`'s "Latest skip"/"Latest completion" are one **week** while the adjacent Plays column is the whole window. I misread exactly that and briefly concluded Deep cuts was the worst surface, from ~17 plays in a single week; over 180 days it's one of the best. Headers now read `Plays (window)` / `Skip (last wk)` / `Completion (last wk)`, and the skip cell carries that week's play count so a rate drawn from a handful of listens reads as what it is. ## #2524 — duplicate-MBID log noise `resolveArtist` now recognises an `isUniqueViolation` on `SetArtistMbidIfNull` as the expected condition it is, matching `resolveAlbum`. Two rows mapping to one MusicBrainz artist is a merge candidate, not a fault; without the branch it logged a generic warning plus a Postgres `ERROR` line on every scan, which teaches an operator to ignore database errors. Three lines, and `isUniqueViolation` already existed. ## Tests 12 Go cases. The load-bearing one, `TestProportionDelta_ReproducesTheDiscoverCase`, asserts the exact 59-vs-70-plays / 13.3pp comparison comes back **not distinguishable** — the false confidence is now a regression test rather than a comment. Paired with a same-effect-at-10×-volume case proving the flag tracks sample size, not gap size. 2 web cases, targeting deltas by `data-testid`. Three of these four commits are test fixes for my own new tests: testing-library queries that matched more elements than intended (a `<td>`'s own direct text nodes still match despite a child span; `/≈/` matched the legend explaining the glyph; `/^Skip/` matched the sparkline column too). Kept as separate commits because each records a distinct trap. The underlying cause was iterating against CI rather than locally — the web lane uses the same `ci-go` image, so `npx vitest run` works locally in ~7s, and note #2465 has been corrected to say so. Verified locally before the final push: svelte-check 0 errors, 110 files / 786 tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01N6vZoJ4Se5YyaqdtGVkap5

bvandeusen added 4 commits 2026-08-06 21:50:32 -04:00

feat(metrics): publish margin of error on every delta — #2495 , #2524

test-web / test (push) Failing after 43s

Details

test-go / test (push) Successful in 1m0s

Details

test-go / integration (push) Successful in 4m58s

Details

481f906059

The metrics card had one volume threshold doing two jobs.
recMetricsLowVolume = 20 is a DISPLAY floor — below that a skip rate is
anecdote — but the card then presented deltas as though it were also a
DECISION floor. Those differ by an order of magnitude: detecting the ~13pp
differences that matter needs ~133 plays per arm for 80% power at a=0.05.

So Discover's taste-matched (59 plays) and random-unheard (70) both rendered
as full-confidence rows with a bold delta beside them, and that comparison
sits at p ~ 0.06. The card said "signal"; the arithmetic said "maybe". It
produced a recommendation the data didn't support, and any reader with the
same numbers would have made the same call.

Deltas now carry a 95% margin of error and a `distinguishable` flag, computed
server-side so both clients read the same arithmetic instead of each
re-deriving it. Skip rate is a two-proportion difference; completion is
Welch, which needs a variance — hence completion_sqsum in the query. It is
the sum of squares rather than stddev_samp on purpose: raw source rows are
merged into surface families in Go, and sums of squares combine across groups
exactly whereas standard deviations cannot.

recMetricsLowVolume is untouched. "Too thin to show" and "too thin to act on"
are different questions.

Web renders an indistinguishable delta as dimmed and prefixed "≈", with the
range on hover and a legend explaining the glyph. Colour is withheld unless
the delta clears its margin — colouring noise red is what made the old card
misleading. Breakdown rows go through the same path; those are the thinnest
samples on screen and where the old card misled most.

Also fixes the admin trends view, which had the same problem worse: its
"Latest skip"/"Latest completion" columns are one WEEK while the adjacent
Plays column is the whole window. I misread exactly that and briefly
concluded Deep cuts was the worst surface, from ~17 plays in a single week —
over 180 days it is one of the best. Headers now name their period and the
skip cell carries that week's play count.

#2524: resolveArtist now recognises a duplicate-MBID unique violation as the
expected condition it is, matching resolveAlbum. Two rows mapping to one
MusicBrainz artist is a merge candidate, not a fault; without the branch it
logged a generic warning plus a Postgres ERROR line on every scan, which
teaches an operator to ignore database errors.

test(metrics): target deltas by test id, not by glyph — #2495

test-web / test (push) Failing after 35s

Details

a094d5f8b0

Two CI failures, both in my own new tests, both informative.

settings: queryByText(/≈/) matched the LEGEND explaining the glyph rather
than a delta, so the "no delta" case failed on the explanation being
present. Delta spans now carry data-testid so a test can name what it
means instead of pattern-matching prose that sits next to it.

tuning: getByText("40%") found two elements. testing-library matches an
element and its OWN direct text nodes, so the skip cell still matches
"40%" despite the trailing play-count span — and discover late-week
completion is also 40%. Genuinely ambiguous now; assert the count.

test(tuning): query the window/last-week headers as column headers — #2495

test-web / test (push) Failing after 33s

Details

96abb48086

getByText(/Plays/) matched my own new caption as well as the header, since
the caption explains which columns cover the window. Query by columnheader
role instead, which is what the assertion actually means.

Also reordered the caption: prepending the clarification turned it into a
run-on that opened mid-explanation before saying what the chart was.

test(tuning): assert headers by exact accessible name — #2495

test-web / test (push) Successful in 34s

Details

304de88c50

Third attempt at the same assertion, so I stopped guessing and got vitest
running locally instead: the web lane uses the same ci-go image, so
`docker run ... -w /src/web ci-go:1.26 npx vitest run` works and turns a
5-minute CI round trip into 7 seconds.

/^Skip/ matched the "Skip rate by week" sparkline column as well as
"Skip (last wk)", just as /Plays/ had matched the caption. Exact names
say what the assertion means and cannot drift onto a neighbour.

Verified locally before pushing: svelte-check 0 errors, 110 files /
786 tests pass.

bvandeusen merged commit d5aa081157 into main

2026-08-06 21:50:42 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/minstrel#122