Compare commits

...
11 Commits
Author SHA1 Message Date
bvandeusen aa9f534f3c Merge pull request 'Genre index: sort A–Z, and repair casing damage at scan time' (#124) from dev into main
test-web / test (push) Successful in 49s
test-go / test (push) Successful in 1m9s
test-go / integration (push) Successful in 5m1s
release / Build signed APK (tag releases only) (push) Successful in 4m2s
release / Build + push container image (push) Successful in 14s
release / Verify release artifacts (tag releases only) (push) Successful in 2s
2026-08-07 21:40:33 -04:00
bvandeusen 8e1d25a772 fix(scanner): repair acronym and apostrophe casing on genre tags — #2468
test-go / test (push) Successful in 53s
test-go / integration (push) Successful in 5m2s
Operator decision: keep the ID3v1 table canonical, fix the casing.

The operator's library carries "Edm", "Idm", "Aor", "Uk Garage", "Uk
Hardcore", "Trap Edm", "Glitch Hop Edm" and "Children'S Music" — an external
tag editor title-cased the whole genre field. The "'S" is the giveaway.

Fixed at SCAN time, not in the display layer: taste_profile.sql reads
tracks.genre directly, so a cosmetic-only fix would leave the taste
vocabulary holding "Edm" while the UI showed "EDM", and any correctly
tagged file would contribute a second, separate tag.

trueUpCasing only ever changes case, never letters, so it cannot silently
turn one genre into a different one — that is what separates it from the
label-remapping idea this task rejected. Two narrow rules:

- A short, evidence-led acronym list, matched case-insensitively so "edm",
  "Edm" and "EDM" all land on "EDM". This is a deliberate exception to the
  project's rule that genre case is exposed as the file says it: "Rock" and
  "rock" still stay separate rows, because folding those is a judgement about
  labels, whereas there is no genre named "Edm".
- Apostrophe suffixes from a FIXED contraction list, so "Children'S" is
  repaired while "O'Brien" and "D'Angelo" keep their capital. A blanket
  "lowercase after an apostrophe" would have broken both.

Matching uses the word's letter core rather than the raw word, so "(Edm)"
and "Edm," are repaired and their punctuation re-attached. Interior
punctuation stays in the core, so "Lo-Fi" and "R&B" are compared whole and
cannot match a fragment by accident. My first version missed this and a test
expecting "(Live EDM)" caught it.

Names resolved from the ID3v1 table are deliberately NOT re-cased, per the
operator's call — entry 40's "AlternRock" stays as the table spells it, with
a test pinning that so a later tidy-up doesn't quietly "fix" it.

tagReadVersion 1 -> 2, so this reaches the existing library on the next scan
rather than new files only. That re-read reuses stored durations, so it costs
tag reads and no ffprobe.
2026-08-07 14:43:36 -04:00
bvandeusen 4509f740f8 feat(web): sort the genre index A–Z as well as by count — #2468
test-web / test (push) Successful in 38s
Operator decision: the taxonomy this task proposed is cancelled. With their
repaired library measured — 391 genres, 90,774 tag applications over ~24,185
tracks, so ~3.7 genres per track — multi-membership already puts each track
under everything it claims, and grouping would add nothing while destroying
real specificity (Neurofunk, Wassoulou, Soukous are not noise).

The task's premise was also wrong. It argued from case variants, "Alt. Rock"
abbreviations and a junk tail; none exist. That apparent mess was the scanner
welding multi-value tags (#2499) plus ghost rows from deleted files (#2523),
both ours, both now fixed. A category system would have papered over both.

So the ask reduces to sorting and search. The search box already existed
(QuickFilter, with its own no-matches state), so this adds only the sort:
count-first by default — the server's order, and the right default since the
head is where you're going — or A–Z for when you can already name the thing
but can't find it among 391 rows. The filtered count now reads "12 of 391"
so a filter's effect is visible.

Client-side only: /api/library/genres is unpaged and already returns the whole
set (~12KB), so neither control needs a round trip or a server change.

Two things the tests pin down:

- The sort COPIES before sorting. With no filter applied the derived list is
  the very array held by the query cache, and Array.sort mutates in place —
  sorting it directly would reorder cached data under every other consumer.
- Count mode passes the server's order through rather than re-sorting. The
  fixture is deliberately not in count order so the test asserts pass-through
  instead of coincidence.

Verified locally: svelte-check 0 errors, 110 files / 788 tests.
2026-08-07 13:11:10 -04:00
bvandeusen 011b4d9a9c Merge pull request 'ci(release): verify a tag release actually shipped its artifacts' (#123) from dev into main
release / Build signed APK (tag releases only) (push) Successful in 3m38s
release / Build + push container image (push) Successful in 1m31s
release / Verify release artifacts (tag releases only) (push) Successful in 2s
2026-08-07 08:32:48 -04:00
bvandeusen a254cb2273 ci(release): close the verify blind spot, check preconditions before the build
Auditing the gating turned up two problems.

verify-release only checked the APK. Because it runs with `always()`, it
runs even when image-release FAILED — so android succeeding while the image
push died would have reported "verified" on a release with no immutable
:vYYYY.MM.DD image. That is exactly half of what was missing when
v2026.08.07 had to be re-cut, so the guard would have caught the incident we
had and waved through its mirror image. Now checks the image too, via
docker manifest inspect.

"Attach APK to gitea Release" resolves the release by tag and fails if it is
absent — but it is the LAST step, so a bare `git push origin vX` built an
APK for several minutes before discovering it had nowhere to put it. Same
check now runs immediately after version computation: seconds, not minutes.
Releases created through the API create tag and release together and pass it.

The rest of the gating audits clean, and one part is worth not "fixing":
image-release's `if: !failure() && !cancelled()` looks odd next to
`needs: [android-release]` but is correct. On main pushes android-release is
SKIPPED, and a skipped dependency is not success() — so the obvious
`if: success()` would silently stop main from ever publishing :latest.
Steps 4/5 vs 6 are mutually exclusive on the tag context, and every image
step gates on the Dockerfile+go.mod guard.

Validated: YAML parses, and `bash -n` over every run: block in all three
jobs is clean.
2026-08-07 08:27:09 -04:00
bvandeusen e368b82f0a ci(release): fail loudly when a tag release ends up without its APK
v2026.08.07 had to be re-cut, and the tag build's android-release job
never started — no job log was written at all, so all eight steps reported
`failure` with none executed and image-release showed `skipped`.

The run was red, but the release PAGE rendered fine and main's own push
build had already moved :latest, so the code was deployable and nothing
looked obviously wrong. What was actually missing — the attached APK and
the immutable :vYYYY.MM.DD image — is easy to skim past, and I nearly did.

This cannot prevent that. The cause was a runner failing to launch a
container, not anything in this file, and it did not reproduce on an
unchanged re-run. What this does is make the CONSEQUENCE legible: an
incomplete release now fails with a named error instead of eight mystery
step failures, and the message says to re-run the run rather than delete
and re-create the tag.

`if: always()` is load-bearing — the job has to report precisely when the
jobs above did not succeed.

Correcting the record while here: I first blamed this on the workflow's
`cancel-in-progress` concurrency block. That was wrong. Cancellation needs
a NEWER run in the same group, and there was exactly one run on the tag
ref (total_count 632 -> 633 on release creation); the main-push runs sit
in a different group. Plausible mechanism, unchecked precondition.

Validated locally: YAML parses, `bash -n` clean, and the asset-parsing
logic unit-checked against a release with an APK, one with no assets, and
one with a non-APK asset.
2026-08-07 08:23:11 -04:00
bvandeusen d5aa081157 Merge pull request 'Recommendation metrics: publish the margin of error on every delta' (#122) from dev into main
test-web / test (push) Successful in 52s
test-go / test (push) Successful in 1m10s
test-go / integration (push) Successful in 4m53s
release / Build signed APK (tag releases only) (push) Successful in 3m57s
release / Build + push container image (push) Successful in 1m37s
2026-08-06 21:50:41 -04:00
bvandeusen 304de88c50 test(tuning): assert headers by exact accessible name — #2495
test-web / test (push) Successful in 34s
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.
2026-08-06 21:27:22 -04:00
bvandeusen 96abb48086 test(tuning): query the window/last-week headers as column headers — #2495
test-web / test (push) Failing after 33s
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.
2026-08-06 21:20:18 -04:00
bvandeusen a094d5f8b0 test(metrics): target deltas by test id, not by glyph — #2495
test-web / test (push) Failing after 35s
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.
2026-08-06 21:14:53 -04:00
bvandeusen 481f906059 feat(metrics): publish margin of error on every delta — #2495, #2524
test-web / test (push) Failing after 43s
test-go / test (push) Successful in 1m0s
test-go / integration (push) Successful in 4m58s
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.
2026-08-06 21:08:58 -04:00
16 changed files with 941 additions and 61 deletions
+101
View File
@@ -98,6 +98,31 @@ jobs:
echo "code=${COMMIT_COUNT}" >> "$GITHUB_OUTPUT" echo "code=${COMMIT_COUNT}" >> "$GITHUB_OUTPUT"
echo "::notice::APK version: ${VERSION_NAME} (code=${COMMIT_COUNT})" echo "::notice::APK version: ${VERSION_NAME} (code=${COMMIT_COUNT})"
# Checked BEFORE the expensive work, not after it. "Attach APK to gitea
# Release" below resolves the release by tag and fails if it is absent —
# but that is the final step, so a tag pushed without a release built an
# APK for several minutes first and only then discovered it had nowhere to
# put it. Same check, seconds in instead of minutes.
#
# Releases are normally created through the API (which creates the tag and
# the release together, so this passes). A bare `git push origin vX` is the
# case this catches.
- name: Release must exist for this tag
shell: bash
working-directory: ${{ github.workspace }}
env:
CI_TOKEN: ${{ secrets.CI_TOKEN }}
run: |
set -euo pipefail
TAG="${GITHUB_REF#refs/tags/}"
if ! curl -fsSL -o /dev/null \
-H "Authorization: token ${CI_TOKEN}" \
"https://git.fabledsword.com/api/v1/repos/${GITHUB_REPOSITORY}/releases/tags/${TAG}"; then
echo "::error::no release exists for ${TAG}. Create the release (which creates the tag) rather than pushing a bare tag — otherwise there is nothing to attach the APK to."
exit 1
fi
echo "::notice::release found for ${TAG}"
- name: Cache Gradle dirs - name: Cache Gradle dirs
uses: actions/cache@v4 uses: actions/cache@v4
with: with:
@@ -322,3 +347,79 @@ jobs:
docker buildx build \ docker buildx build \
--build-arg MINSTREL_VERSION="${{ steps.tags.outputs.version }}" \ --build-arg MINSTREL_VERSION="${{ steps.tags.outputs.version }}" \
--push ${{ steps.tags.outputs.args }} . --push ${{ steps.tags.outputs.args }} .
# Verifies a tag release actually ended up complete, and names the specific
# thing that's missing if not.
#
# Added 2026-08-07 after v2026.08.07 was re-cut. The android-release job never
# started — no log was written at all — so all eight of its steps reported
# `failure` with none executed and image-release showed `skipped`. The run was
# red, but the *release page rendered fine*, and `main`'s own push build had
# already moved `:latest`, so the code was deployable and nothing looked
# obviously wrong. The release was simply missing its APK and its immutable
# `:vYYYY.MM.DD` image, which is easy to skim past.
#
# This job cannot prevent that (the cause was a runner failing to launch, not
# anything in this file). What it does is turn an incomplete release into an
# explicit, named error instead of eight mystery step failures — so the
# consequence is legible without having to infer it.
#
# `if: always()` is the whole point: it has to report precisely when the jobs
# above did NOT succeed.
verify-release:
name: Verify release artifacts (tag releases only)
needs: [android-release, image-release]
if: ${{ always() && startsWith(github.ref, 'refs/tags/v') }}
runs-on: go-ci
container:
image: git.fabledsword.com/bvandeusen/ci-go:1.26
steps:
- name: Release must have an APK attached
shell: bash
env:
CI_TOKEN: ${{ secrets.CI_TOKEN }}
run: |
set -euo pipefail
TAG="${GITHUB_REF#refs/tags/}"
REPO="${GITHUB_REPOSITORY}"
REL_JSON="$(curl -fsSL \
-H "Authorization: token ${CI_TOKEN}" \
"https://git.fabledsword.com/api/v1/repos/${REPO}/releases/tags/${TAG}" || true)"
if [ -z "${REL_JSON}" ]; then
echo "::error::no release found for ${TAG} — the tag exists but nothing was published"
exit 1
fi
APK="$(printf '%s' "${REL_JSON}" \
| grep -oP '"browser_download_url":\s*"\K[^"]+' \
| grep -E '\.apk$' | head -1 || true)"
if [ -z "${APK}" ]; then
echo "::error::release ${TAG} has NO APK attached — in-app update will offer nothing, and the bundled-APK path on future :latest builds has no source."
echo "::error::Fix by RE-RUNNING this workflow run. Do NOT delete and re-create the tag; if it fails again the runner never started the container, and the evidence is in act_runner on the host (Gitea will hold no job log)."
exit 1
fi
echo "::notice::APK attached: ${APK}"
# The other half. Checking only the APK would report success on a release
# whose image push failed — which is precisely the second thing that was
# missing when v2026.08.07 had to be re-cut. `always()` on this job means
# it runs even when image-release failed, so without this the guard would
# cheerfully verify an incomplete release.
- name: Immutable image tag must exist
shell: bash
run: |
set -euo pipefail
TAG="${GITHUB_REF#refs/tags/}"
IMAGE="git.fabledsword.com/bvandeusen/minstrel"
echo "${{ secrets.CI_TOKEN }}" \
| docker login git.fabledsword.com -u "${{ github.actor }}" --password-stdin
if ! docker manifest inspect "${IMAGE}:${TAG}" > /dev/null 2>&1; then
echo "::error::image ${IMAGE}:${TAG} was never pushed — the release tag has no immutable image, so there is nothing to pin or roll back to. Re-run this workflow run."
exit 1
fi
echo "::notice::image verified: ${IMAGE}:${TAG}"
+46 -3
View File
@@ -39,6 +39,14 @@ type surfaceMetric struct {
SkipRate float64 `json:"skip_rate"` // skips / plays, [0,1] SkipRate float64 `json:"skip_rate"` // skips / plays, [0,1]
AvgCompletion float64 `json:"avg_completion"` // mean completion ratio, [0,1] AvgCompletion float64 `json:"avg_completion"` // mean completion ratio, [0,1]
LowConfidence bool `json:"low_confidence"` // plays < recMetricsLowVolume LowConfidence bool `json:"low_confidence"` // plays < recMetricsLowVolume
// SkipDelta / CompletionDelta are this row's difference from the manual
// baseline WITH its margin of error (#2495). nil on the baseline row
// itself, and whenever the samples are too thin for a margin to mean
// anything. Computed server-side so both clients read the same arithmetic
// instead of each re-deriving it — and so `low_confidence` is no longer
// mistaken for a decision threshold, which it never was.
SkipDelta *metricDelta `json:"skip_delta,omitempty"`
CompletionDelta *metricDelta `json:"completion_delta,omitempty"`
// Breakdown splits the family into the pick-kind populations its // Breakdown splits the family into the pick-kind populations its
// builder stamped (#1249, generalized #1270): For You's taste/fresh, // builder stamped (#1249, generalized #1270): For You's taste/fresh,
// Discover's buckets, tier1-3 for tiered mixes — plus earlier plays // Discover's buckets, tier1-3 for tiered mixes — plus earlier plays
@@ -119,6 +127,10 @@ type familyAccum struct {
// completionSum is avg*count re-expanded, so merging N raw rows // completionSum is avg*count re-expanded, so merging N raw rows
// reduces to a single weighted division at the end. // reduces to a single weighted division at the end.
completionSum float64 completionSum float64
// completionSqSum is the sum of squared completion ratios, which is what
// makes the variance mergeable across raw source rows (#2495). Standard
// deviations cannot be combined; sums of squares add exactly.
completionSqSum float64
} }
func (a *familyAccum) add(row dbq.RecommendationSourceMetricsForUserRow) { func (a *familyAccum) add(row dbq.RecommendationSourceMetricsForUserRow) {
@@ -126,6 +138,7 @@ func (a *familyAccum) add(row dbq.RecommendationSourceMetricsForUserRow) {
a.skips += row.Skips a.skips += row.Skips
a.completionN += row.CompletionN a.completionN += row.CompletionN
a.completionSum += row.AvgCompletion * float64(row.CompletionN) a.completionSum += row.AvgCompletion * float64(row.CompletionN)
a.completionSqSum += row.CompletionSqsum
} }
func (a *familyAccum) metric() surfaceMetric { func (a *familyAccum) metric() surfaceMetric {
@@ -145,6 +158,33 @@ func (a *familyAccum) metric() surfaceMetric {
return m return m
} }
// completionVariance is the sample variance of this family's completion ratios.
func (a *familyAccum) completionVariance() float64 {
return sampleVariance(a.completionSum, a.completionSqSum, a.completionN)
}
// applyDeltas attaches baseline-relative deltas + margins to a metric.
// Split out so every row — parent surfaces and breakdown rows alike — goes
// through the identical arithmetic; a breakdown arm is exactly where the old
// card was most misleading, because those are the thinnest samples on screen.
func applyDeltas(m *surfaceMetric, acc *familyAccum, baseline *familyAccum) {
if baseline == nil || baseline.plays == 0 {
return
}
m.SkipDelta = proportionDelta(
m.SkipRate, acc.plays,
float64(baseline.skips)/float64(baseline.plays), baseline.plays,
)
baseMean := 0.0
if baseline.completionN > 0 {
baseMean = baseline.completionSum / float64(baseline.completionN)
}
m.CompletionDelta = meanDelta(
m.AvgCompletion, acc.completionVariance(), acc.completionN,
baseMean, baseline.completionVariance(), baseline.completionN,
)
}
// handleGetRecommendationMetrics implements GET /api/me/recommendation-metrics. // handleGetRecommendationMetrics implements GET /api/me/recommendation-metrics.
// Bucketed per-surface-family outcomes for the caller over the last `days` // Bucketed per-surface-family outcomes for the caller over the last `days`
// (default 30, capped at 365), grouped by surface intent and anchored by the // (default 30, capped at 365), grouped by surface intent and anchored by the
@@ -214,7 +254,7 @@ func pickKindFamily(parent recFamily, kind string) recFamily {
// Breakdown rows. Attached only when at least one attributed play // Breakdown rows. Attached only when at least one attributed play
// exists — an all-unattributed breakdown would just repeat the parent // exists — an all-unattributed breakdown would just repeat the parent
// row, and families that never stamp (radio, direct plays) stay flat. // row, and families that never stamp (radio, direct plays) stay flat.
func pickKindBreakdown(picks map[string]*familyAccum) []surfaceMetric { func pickKindBreakdown(picks map[string]*familyAccum, baseline *familyAccum) []surfaceMetric {
attributed := int64(0) attributed := int64(0)
for kind, acc := range picks { for kind, acc := range picks {
if kind != "" { if kind != "" {
@@ -227,7 +267,9 @@ func pickKindBreakdown(picks map[string]*familyAccum) []surfaceMetric {
out := make([]surfaceMetric, 0, len(picks)) out := make([]surfaceMetric, 0, len(picks))
for _, kind := range pickKindOrder { for _, kind := range pickKindOrder {
if acc, ok := picks[kind]; ok && acc.plays > 0 { if acc, ok := picks[kind]; ok && acc.plays > 0 {
out = append(out, acc.metric()) m := acc.metric()
applyDeltas(&m, acc, baseline)
out = append(out, m)
} }
} }
return out return out
@@ -287,7 +329,8 @@ func bucketMetricsResponse(
for _, acc := range families { for _, acc := range families {
if acc.fam.intent == g.intent { if acc.fam.intent == g.intent {
m := acc.metric() m := acc.metric()
m.Breakdown = pickKindBreakdown(picks[acc.fam.key]) applyDeltas(&m, acc, baseline)
m.Breakdown = pickKindBreakdown(picks[acc.fam.key], baseline)
group.Surfaces = append(group.Surfaces, m) group.Surfaces = append(group.Surfaces, m)
} }
} }
+117
View File
@@ -0,0 +1,117 @@
package api
import "math"
// Uncertainty on the deltas the recommendation-metrics card shows (#2495).
//
// Why this exists: the card had exactly one volume threshold,
// recMetricsLowVolume = 20, and it was doing two jobs. Twenty plays is enough to
// be worth DISPLAYING — below that a skip rate is anecdote — but it is nowhere
// near enough to ACT on. Detecting the ~13pp differences that actually matter
// needs roughly 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 led directly
// to a recommendation the data didn't support, and any reader with the same
// numbers would have made the same call.
//
// The fix is to publish the margin of error next to the delta and flag when the
// delta is smaller than it — i.e. not distinguishable from zero. Computed here,
// server-side, so both clients agree rather than each re-deriving it.
//
// recMetricsLowVolume stays exactly as it was. This is a second, independent
// signal, not a replacement: "too thin to show" and "too thin to act on" are
// different questions and deserve different answers.
// deltaZ is the two-sided 95% normal critical value. Normal rather than
// Student's t: at the sample sizes where a delta is worth acting on (n in the
// hundreds) the difference is immaterial, and a household dashboard does not
// need a t-table.
const deltaZ = 1.96
// metricDelta is a difference from the baseline, with its uncertainty.
//
// Both figures are in PERCENTAGE POINTS, matching how the card reads them out —
// a skip rate of 0.153 against a baseline of 0.270 is "-11.7", not "-0.117".
type metricDelta struct {
// DeltaPP is surface minus baseline. Negative skip is better; negative
// completion is worse. The client owns that colouring.
DeltaPP float64 `json:"delta_pp"`
// MarginPP is the 95% margin of error on DeltaPP. Read the delta as
// DeltaPP ± MarginPP.
MarginPP float64 `json:"margin_pp"`
// Distinguishable reports |DeltaPP| >= MarginPP: the interval excludes
// zero, so the difference is worth reading as a difference. When false the
// number may be pure noise no matter how large it looks.
Distinguishable bool `json:"distinguishable"`
}
// proportionDelta compares two rates (skips/plays) as a two-proportion
// difference. Returns nil when either sample is empty, or when either rate is
// degenerate (0 or 1) — a rate with no observed variation has an SE of 0 on its
// side, which would report a spuriously narrow margin rather than an honest one.
func proportionDelta(rate1 float64, n1 int64, rate2 float64, n2 int64) *metricDelta {
if n1 <= 0 || n2 <= 0 {
return nil
}
v1 := rate1 * (1 - rate1) / float64(n1)
v2 := rate2 * (1 - rate2) / float64(n2)
se := math.Sqrt(v1 + v2)
if se <= 0 {
// Both rates are 0 or both are 1. The delta is exactly zero and the
// margin is meaningless; reporting nothing is more honest than
// reporting certainty.
return nil
}
return newDelta((rate1-rate2)*100, deltaZ*se*100)
}
// meanDelta compares two means (average completion ratio) using Welch's
// standard error, which does not assume equal variances between the two groups.
//
// Note the margins here are wider than intuition suggests, and that is correct:
// completion is strongly bimodal — a play is either abandoned early (≈0.05) or
// finished (≈1.0), with little in between — so its standard deviation is large
// (~0.4) even though the mean looks stable.
func meanDelta(mean1 float64, variance1 float64, n1 int64, mean2 float64, variance2 float64, n2 int64) *metricDelta {
// Two observations minimum per side: a sample variance needs n-1 > 0.
if n1 < 2 || n2 < 2 {
return nil
}
se := math.Sqrt(variance1/float64(n1) + variance2/float64(n2))
if se <= 0 || math.IsNaN(se) || math.IsInf(se, 0) {
return nil
}
return newDelta((mean1-mean2)*100, deltaZ*se*100)
}
func newDelta(deltaPP, marginPP float64) *metricDelta {
return &metricDelta{
DeltaPP: deltaPP,
MarginPP: marginPP,
// >= rather than >: a delta exactly equal to its margin sits on the
// boundary, and calling the boundary "distinguishable" is the
// conventional reading of a 95% interval that just excludes zero.
Distinguishable: math.Abs(deltaPP) >= marginPP,
}
}
// sampleVariance recovers the sample variance from the aggregates the SQL
// returns. sum is mean×n rather than a selected column, which keeps the query to
// one extra expression.
//
// The subtraction can go very slightly negative through floating-point
// cancellation when every observation is identical, so the result is clamped —
// a negative variance would produce NaN downstream.
func sampleVariance(sum, sqSum float64, n int64) float64 {
if n < 2 {
return 0
}
nf := float64(n)
v := (sqSum - (sum * sum / nf)) / (nf - 1)
if v < 0 {
return 0
}
return v
}
+152
View File
@@ -0,0 +1,152 @@
package api
import (
"math"
"testing"
)
func TestProportionDelta_ReproducesTheDiscoverCase(t *testing.T) {
// The comparison that motivated #2495: Discover taste-matched (59 plays,
// 15.3% skip) vs random-unheard (70 plays, 28.6%). A 13.3pp gap that the old
// card rendered as a confident coloured number, sitting at p ≈ 0.06.
d := proportionDelta(0.153, 59, 0.286, 70)
if d == nil {
t.Fatal("expected a delta for two real samples")
}
if math.Abs(d.DeltaPP-(-13.3)) > 0.1 {
t.Errorf("DeltaPP = %.2f, want ≈ -13.3", d.DeltaPP)
}
// This is the assertion the whole task exists for: at these sample sizes the
// margin swallows the difference.
if d.Distinguishable {
t.Errorf("13.3pp on n=59/70 reported as distinguishable (margin %.2f) — "+
"this is exactly the false confidence #2495 set out to remove", d.MarginPP)
}
if d.MarginPP <= 13.3 {
t.Errorf("MarginPP = %.2f, expected it to exceed the 13.3pp delta", d.MarginPP)
}
}
// Same effect size, ~10x the volume: now it is real. Proves the flag tracks
// sample size rather than just the size of the gap.
func TestProportionDelta_SameGapBecomesDistinguishableWithVolume(t *testing.T) {
d := proportionDelta(0.153, 600, 0.286, 700)
if d == nil {
t.Fatal("expected a delta")
}
if !d.Distinguishable {
t.Errorf("13.3pp on n=600/700 should be distinguishable (margin %.2f)", d.MarginPP)
}
}
func TestProportionDelta_SignAndDirection(t *testing.T) {
// Surface skips MORE than baseline -> positive delta (worse for skip rate).
worse := proportionDelta(0.40, 500, 0.25, 500)
if worse == nil || worse.DeltaPP <= 0 {
t.Fatalf("expected a positive delta, got %+v", worse)
}
better := proportionDelta(0.10, 500, 0.25, 500)
if better == nil || better.DeltaPP >= 0 {
t.Fatalf("expected a negative delta, got %+v", better)
}
}
func TestProportionDelta_EmptySamples(t *testing.T) {
if d := proportionDelta(0.2, 0, 0.3, 100); d != nil {
t.Errorf("n1=0 produced a delta: %+v", d)
}
if d := proportionDelta(0.2, 100, 0.3, 0); d != nil {
t.Errorf("n2=0 produced a delta: %+v", d)
}
}
// Two degenerate rates have zero standard error, which would report a margin of
// 0 and therefore "distinguishable" for a delta of exactly 0. Reporting nothing
// is the honest answer.
func TestProportionDelta_DegenerateRates(t *testing.T) {
if d := proportionDelta(0, 50, 0, 50); d != nil {
t.Errorf("both rates 0 produced a delta: %+v", d)
}
if d := proportionDelta(1, 50, 1, 50); d != nil {
t.Errorf("both rates 1 produced a delta: %+v", d)
}
// One degenerate side is still informative — the other side carries variance.
if d := proportionDelta(0, 200, 0.3, 200); d == nil {
t.Error("one degenerate rate should still yield a delta")
}
}
func TestMeanDelta(t *testing.T) {
// Completion is bimodal, so ~0.16 variance (sd ≈ 0.4) is realistic.
const v = 0.16
thin := meanDelta(0.82, v, 59, 0.54, v, 70)
if thin == nil {
t.Fatal("expected a delta")
}
if math.Abs(thin.DeltaPP-28.0) > 0.1 {
t.Errorf("DeltaPP = %.2f, want ≈ 28.0", thin.DeltaPP)
}
// 28pp is large enough to survive even a wide margin at this n.
if !thin.Distinguishable {
t.Errorf("28pp on n=59/70 with sd 0.4 should be distinguishable (margin %.2f)", thin.MarginPP)
}
// A small completion gap at the same volume should not be.
small := meanDelta(0.56, v, 59, 0.54, v, 70)
if small == nil {
t.Fatal("expected a delta")
}
if small.Distinguishable {
t.Errorf("2pp on n=59/70 reported as distinguishable (margin %.2f)", small.MarginPP)
}
}
// A sample variance needs at least two observations per side.
func TestMeanDelta_NeedsTwoObservations(t *testing.T) {
if d := meanDelta(0.8, 0.1, 1, 0.5, 0.1, 100); d != nil {
t.Errorf("n1=1 produced a delta: %+v", d)
}
if d := meanDelta(0.8, 0.1, 100, 0.5, 0.1, 1); d != nil {
t.Errorf("n2=1 produced a delta: %+v", d)
}
}
func TestMeanDelta_ZeroVarianceBothSides(t *testing.T) {
if d := meanDelta(0.8, 0, 50, 0.5, 0, 50); d != nil {
t.Errorf("zero variance on both sides produced a delta: %+v", d)
}
}
func TestSampleVariance(t *testing.T) {
// Observations 0, 1: mean 0.5, sample variance 0.5.
if got := sampleVariance(1.0, 1.0, 2); math.Abs(got-0.5) > 1e-9 {
t.Errorf("sampleVariance = %v, want 0.5", got)
}
// Identical observations -> zero variance, and must not go negative through
// floating-point cancellation.
if got := sampleVariance(4.0, 4.0, 4); got != 0 {
t.Errorf("identical observations gave variance %v, want 0", got)
}
if got := sampleVariance(0, 0, 1); got != 0 {
t.Errorf("n=1 gave variance %v, want 0", got)
}
}
// Clamping matters: a negative variance would become NaN in the square root and
// propagate into the JSON as a null-ish number.
func TestSampleVariance_NeverNegative(t *testing.T) {
// sqSum slightly below sum²/n, as cancellation can produce.
if got := sampleVariance(10.0, 24.999999999, 4); got < 0 {
t.Errorf("variance went negative: %v", got)
}
}
func TestNewDelta_BoundaryCountsAsDistinguishable(t *testing.T) {
d := newDelta(5.0, 5.0)
if !d.Distinguishable {
t.Error("a delta exactly equal to its margin should count as distinguishable")
}
d = newDelta(4.999, 5.0)
if d.Distinguishable {
t.Error("a delta just inside its margin should not count as distinguishable")
}
}
+11 -1
View File
@@ -18,7 +18,8 @@ SELECT
count(*)::bigint AS plays, count(*)::bigint AS plays,
count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips, count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips,
count(pe.completion_ratio)::bigint AS completion_n, count(pe.completion_ratio)::bigint AS completion_n,
COALESCE(avg(pe.completion_ratio), 0)::float8 AS avg_completion COALESCE(avg(pe.completion_ratio), 0)::float8 AS avg_completion,
COALESCE(sum(pe.completion_ratio * pe.completion_ratio), 0)::float8 AS completion_sqsum
FROM play_events pe FROM play_events pe
WHERE pe.user_id = $1 WHERE pe.user_id = $1
AND pe.started_at > now() - ($2::float8 * INTERVAL '1 day') AND pe.started_at > now() - ($2::float8 * INTERVAL '1 day')
@@ -38,12 +39,20 @@ type RecommendationSourceMetricsForUserRow struct {
Skips int64 Skips int64
CompletionN int64 CompletionN int64
AvgCompletion float64 AvgCompletion float64
CompletionSqsum float64
} }
// $1 user_id, $2 window_days. plays/skips are counts; avg_completion is the // $1 user_id, $2 window_days. plays/skips are counts; avg_completion is the
// mean completion ratio over the completion_n plays that recorded one. // mean completion ratio over the completion_n plays that recorded one.
// pick_kind splits For You plays into taste/fresh/unattributed (#1249); // pick_kind splits For You plays into taste/fresh/unattributed (#1249);
// it is NULL for every other source, so those still group to one row. // it is NULL for every other source, so those still group to one row.
//
// completion_sqsum carries the sum of SQUARED completion ratios so the Go
// handler can compute a variance — needed for the margin of error on a
// completion delta (#2495). It is the sum rather than `stddev_samp` on purpose:
// raw source rows get merged into surface families in Go, and sums of squares
// add across groups exactly, whereas standard deviations cannot be combined
// without them. Variance = (sqsum - sum²/n) / (n-1), with sum = avg × n.
func (q *Queries) RecommendationSourceMetricsForUser(ctx context.Context, arg RecommendationSourceMetricsForUserParams) ([]RecommendationSourceMetricsForUserRow, error) { func (q *Queries) RecommendationSourceMetricsForUser(ctx context.Context, arg RecommendationSourceMetricsForUserParams) ([]RecommendationSourceMetricsForUserRow, error) {
rows, err := q.db.Query(ctx, recommendationSourceMetricsForUser, arg.UserID, arg.Column2) rows, err := q.db.Query(ctx, recommendationSourceMetricsForUser, arg.UserID, arg.Column2)
if err != nil { if err != nil {
@@ -60,6 +69,7 @@ func (q *Queries) RecommendationSourceMetricsForUser(ctx context.Context, arg Re
&i.Skips, &i.Skips,
&i.CompletionN, &i.CompletionN,
&i.AvgCompletion, &i.AvgCompletion,
&i.CompletionSqsum,
); err != nil { ); err != nil {
return nil, err return nil, err
} }
@@ -44,13 +44,21 @@ ORDER BY 1, 2;
-- mean completion ratio over the completion_n plays that recorded one. -- mean completion ratio over the completion_n plays that recorded one.
-- pick_kind splits For You plays into taste/fresh/unattributed (#1249); -- pick_kind splits For You plays into taste/fresh/unattributed (#1249);
-- it is NULL for every other source, so those still group to one row. -- it is NULL for every other source, so those still group to one row.
--
-- completion_sqsum carries the sum of SQUARED completion ratios so the Go
-- handler can compute a variance — needed for the margin of error on a
-- completion delta (#2495). It is the sum rather than `stddev_samp` on purpose:
-- raw source rows get merged into surface families in Go, and sums of squares
-- add across groups exactly, whereas standard deviations cannot be combined
-- without them. Variance = (sqsum - sum²/n) / (n-1), with sum = avg × n.
SELECT SELECT
pe.source, pe.source,
pe.pick_kind, pe.pick_kind,
count(*)::bigint AS plays, count(*)::bigint AS plays,
count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips, count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips,
count(pe.completion_ratio)::bigint AS completion_n, count(pe.completion_ratio)::bigint AS completion_n,
COALESCE(avg(pe.completion_ratio), 0)::float8 AS avg_completion COALESCE(avg(pe.completion_ratio), 0)::float8 AS avg_completion,
COALESCE(sum(pe.completion_ratio * pe.completion_ratio), 0)::float8 AS completion_sqsum
FROM play_events pe FROM play_events pe
WHERE pe.user_id = $1 WHERE pe.user_id = $1
AND pe.started_at > now() - ($2::float8 * INTERVAL '1 day') AND pe.started_at > now() - ($2::float8 * INTERVAL '1 day')
+96 -3
View File
@@ -89,7 +89,7 @@ func normaliseGenreValue(v string) []string {
for strings.HasPrefix(v, "(") { for strings.HasPrefix(v, "(") {
// "((" is the spec's escape for a literal "(" — the rest is plain text. // "((" is the spec's escape for a literal "(" — the rest is plain text.
if strings.HasPrefix(v, "((") { if strings.HasPrefix(v, "((") {
return append(out, strings.TrimSpace(v[1:])) return append(out, trueUpCasing(strings.TrimSpace(v[1:])))
} }
end := strings.IndexByte(v, ')') end := strings.IndexByte(v, ')')
if end < 0 { if end < 0 {
@@ -106,7 +106,7 @@ func normaliseGenreValue(v string) []string {
if err != nil { if err != nil {
// Parenthesised but not a reference, e.g. "(Live)". Keep the // Parenthesised but not a reference, e.g. "(Live)". Keep the
// whole remainder as written. // whole remainder as written.
return append(out, v) return append(out, trueUpCasing(v))
} }
if name, ok := id3v1GenreName(n); ok { if name, ok := id3v1GenreName(n); ok {
out = append(out, name) out = append(out, name)
@@ -120,11 +120,104 @@ func normaliseGenreValue(v string) []string {
} }
if n, err := strconv.Atoi(v); err == nil { if n, err := strconv.Atoi(v); err == nil {
if name, ok := id3v1GenreName(n); ok { if name, ok := id3v1GenreName(n); ok {
// Canonical table name, deliberately NOT re-cased — see id3v1Genres.
return append(out, name) return append(out, name)
} }
return out return out
} }
return append(out, v) return append(out, trueUpCasing(v))
}
// genreAcronyms are tokens that belong in caps. Tag editors that title-case the
// whole genre field turn "EDM" into "Edm" and "UK Garage" into "Uk Garage", and
// the operator's library carries all of these (#2468).
//
// Matched case-INSENSITIVELY, so "edm", "Edm" and "EDM" all land on "EDM". That
// is a deliberate, narrow exception to the project's rule that genre case is
// exposed as the file says it: "Rock" and "rock" still stay separate rows,
// because folding those would be a judgement about labels. Fixing a token that
// is unambiguously an initialism is not — there is no genre named "Edm".
//
// Kept short and evidence-led. Add a token here only when a real library shows
// it damaged; a speculative list risks mangling a word that legitimately looks
// like an acronym.
var genreAcronyms = map[string]string{
"edm": "EDM",
"idm": "IDM",
"aor": "AOR",
"uk": "UK",
"us": "US",
"ebm": "EBM",
}
// contractionSuffixes are the word-endings that follow an apostrophe in normal
// English. Title-casing capitalises the letter after ANY non-letter, which is
// how "Children's Music" became "Children'S Music".
//
// Deliberately a fixed list rather than "lowercase whatever follows an
// apostrophe": that broader rule would break "O'Brien" and "D'Angelo", which are
// correctly capitalised after the apostrophe.
var contractionSuffixes = map[string]bool{
"s": true, "t": true, "re": true, "ll": true, "ve": true, "d": true, "m": true,
}
// trueUpCasing repairs casing damage done by tag editors that title-case the
// genre field. It only ever changes case, never the letters — so it cannot
// silently turn one genre into a different one, which is what separates it from
// the label-remapping idea #2468 rejected.
func trueUpCasing(s string) string {
if s == "" {
return s
}
words := strings.Split(s, " ")
for i, w := range words {
if w == "" {
continue
}
// Match the word's letter core, not the raw word, so surrounding
// punctuation doesn't hide the acronym: "(Edm)" and "Edm," both need
// fixing, and re-attaching the trimmed edges keeps them intact.
lead, core, trail := splitWordCore(w)
if fixed, ok := genreAcronyms[strings.ToLower(core)]; ok {
words[i] = lead + fixed + trail
continue
}
words[i] = fixApostrophe(w)
}
return strings.Join(words, " ")
}
// splitWordCore peels leading and trailing non-alphanumerics off a word.
// Interior punctuation stays in the core, so "Lo-Fi" and "R&B" are compared
// whole rather than being split into fragments that might match by accident.
func splitWordCore(w string) (lead, core, trail string) {
isCore := func(r rune) bool {
return (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9')
}
start := 0
for start < len(w) && !isCore(rune(w[start])) {
start++
}
end := len(w)
for end > start && !isCore(rune(w[end-1])) {
end--
}
return w[:start], w[start:end], w[end:]
}
// fixApostrophe lowercases a capitalised contraction or possessive suffix:
// "Children'S" -> "Children's". Leaves "O'Brien" alone, since "Brien" is not a
// contraction suffix.
func fixApostrophe(w string) string {
idx := strings.LastIndexByte(w, '\'')
if idx <= 0 || idx == len(w)-1 {
return w
}
suffix := w[idx+1:]
if !contractionSuffixes[strings.ToLower(suffix)] {
return w
}
return w[:idx+1] + strings.ToLower(suffix)
} }
func id3v1GenreName(n int) (string, bool) { func id3v1GenreName(n int) (string, bool) {
+99
View File
@@ -419,3 +419,102 @@ func equalStrings(a, b []string) bool {
} }
return true return true
} }
func TestTrueUpCasing(t *testing.T) {
tests := []struct {
name string
in string
want string
}{
// The damage actually present in the operator's library (#2468).
{"edm acronym", "Edm", "EDM"},
{"idm acronym", "Idm", "IDM"},
{"aor acronym", "Aor", "AOR"},
{"uk prefix", "Uk Garage", "UK Garage"},
{"uk hardcore", "Uk Hardcore", "UK Hardcore"},
{"acronym mid-phrase", "Trap Edm", "Trap EDM"},
{"acronym at the end", "Glitch Hop Edm", "Glitch Hop EDM"},
{"possessive", "Children'S Music", "Children's Music"},
// Already correct input must be left exactly alone.
{"correct acronym", "EDM", "EDM"},
{"correct possessive", "Children's Music", "Children's Music"},
// Case-insensitive, so a lower-cased tag also lands on the canonical
// form rather than becoming a third variant.
{"lowercase acronym", "edm", "EDM"},
// Names with an apostrophe followed by a real word are NOT contractions
// and must keep their capital — this is why the suffix list is fixed
// rather than "lowercase anything after an apostrophe".
{"irish surname", "O'Brien Core", "O'Brien Core"},
{"french elision", "D'Angelo Soul", "D'Angelo Soul"},
// Ordinary genres pass through untouched. Case is otherwise exposed as
// the file says it — "Rock" vs "rock" stays a real distinction.
{"plain", "Alternative Rock", "Alternative Rock"},
{"lowercase plain", "rock", "rock"},
{"hyphenated", "Lo-Fi Hip Hop", "Lo-Fi Hip Hop"},
{"ampersand", "R&B", "R&B"},
{"empty", "", ""},
// Punctuation around a word must not hide the acronym inside it.
{"parenthesised acronym", "Hip Hop (Edm)", "Hip Hop (EDM)"},
{"acronym with comma", "Edm, Trap", "EDM, Trap"},
// Interior punctuation stays in the core, so these are compared whole
// and cannot match a fragment by accident.
{"hyphenated stays whole", "Lo-Fi", "Lo-Fi"},
{"ampersand stays whole", "Drum & Bass", "Drum & Bass"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := trueUpCasing(tc.in); got != tc.want {
t.Errorf("trueUpCasing(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}
// The casing repair must never touch a name resolved from the ID3v1 table. The
// operator chose to keep that table canonical, so entry 40's 1990s spelling
// "AlternRock" stays as-is even though it reads like damage.
func TestNormaliseGenreValue_CanonicalTableNamesNotRecased(t *testing.T) {
if got := normaliseGenreValue("40"); !equalStrings(got, []string{"AlternRock"}) {
t.Errorf("bare 40 = %q, want [AlternRock]", got)
}
if got := normaliseGenreValue("(40)"); !equalStrings(got, []string{"AlternRock"}) {
t.Errorf("(40) = %q, want [AlternRock]", got)
}
}
// Casing runs on values that came from the file, including the parenthesised
// and refinement paths.
func TestNormaliseGenreValue_CasingAppliedToFileText(t *testing.T) {
tests := []struct {
in string
want []string
}{
{"Edm", []string{"EDM"}},
{"(17)Uk Garage", []string{"Rock", "UK Garage"}},
// Punctuation around the acronym must not hide it — the letter core is
// what gets matched, and the trimmed edges are re-attached.
{"(Live Edm)", []string{"(Live EDM)"}},
{"((Children'S Music", []string{"(Children's Music"}},
}
for _, tc := range tests {
t.Run(tc.in, func(t *testing.T) {
if got := normaliseGenreValue(tc.in); !equalStrings(got, tc.want) {
t.Errorf("normaliseGenreValue(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}
// Two spellings of one acronym in the same file collapse to a single tag rather
// than surviving as near-duplicates.
func TestNormaliseGenres_AcronymVariantsDedupe(t *testing.T) {
if got := normaliseGenres([]string{"Edm", "EDM", "edm"}); !equalStrings(got, []string{"EDM"}) {
t.Errorf("genres = %q, want [EDM]", got)
}
}
+18 -1
View File
@@ -52,7 +52,11 @@ var audioExtensions = map[string]bool{
// ("Alternative Rock" + "Rock" -> "Alternative RockRock"), which corrupted // ("Alternative Rock" + "Rock" -> "Alternative RockRock"), which corrupted
// the genre browse axis and polluted the taste profile's tag vocabulary, // the genre browse axis and polluted the taste profile's tag vocabulary,
// and left bare ID3v1 numeric references unresolved (#2499). // and left bare ID3v1 numeric references unresolved (#2499).
const tagReadVersion int16 = 1 // 2: acronym and apostrophe casing repaired on genre values (#2468) — "Edm" ->
// "EDM", "Children'S Music" -> "Children's Music". Bumped rather than left
// to new files only because taste_profile.sql reads tracks.genre directly,
// so a half-repaired library would carry both spellings as separate tags.
const tagReadVersion int16 = 2
type Stats struct { type Stats struct {
Scanned int `json:"scanned"` Scanned int `json:"scanned"`
@@ -414,8 +418,21 @@ func (s *Scanner) resolveArtist(ctx context.Context, q *dbq.Queries, name, mbid
ID: existing.ID, ID: existing.ID,
Mbid: &m, Mbid: &m,
}); uerr != nil { }); uerr != nil {
if isUniqueViolation(uerr) {
// Another artist row already owns this MBID — two rows that
// should be merged (usually two spellings of one name).
// Expected, not a fault: leave NULL and let the operator
// merge. Mirrors resolveAlbum, which has always handled it
// this way — without this branch the identical benign
// condition logged a generic warning plus a Postgres ERROR
// line on every scan, which teaches an operator to ignore
// database errors (#2524).
s.logger.Info("library scan: duplicate artist mbid (canonical row already owns it)",
"artist_id", existing.ID, "artist", name, "mbid", mbid)
} else {
s.logger.Warn("library scan: heal artist mbid failed", s.logger.Warn("library scan: heal artist mbid failed",
"artist_id", existing.ID, "err", uerr) "artist_id", existing.ID, "err", uerr)
}
} else { } else {
existing.Mbid = &m existing.Mbid = &m
} }
+18
View File
@@ -4,6 +4,20 @@ import { api } from './client';
// Mirrors internal/api/me_recommendation_metrics.go: raw play sources are // Mirrors internal/api/me_recommendation_metrics.go: raw play sources are
// bucketed server-side into stable surface families, grouped by intent, and // bucketed server-side into stable surface families, grouped by intent, and
// anchored by the manual-plays baseline (milestone #127). // anchored by the manual-plays baseline (milestone #127).
// A difference from the baseline, with its uncertainty (#2495). Both figures
// are already in percentage points — the server does the arithmetic so both
// clients read the same numbers.
//
// `distinguishable: false` means |delta_pp| < margin_pp: the delta cannot be
// told apart from zero, however large it looks. That distinction is the whole
// point of this type — `low_confidence` answers "is this worth showing?", which
// is a much lower bar than "is this worth acting on?".
export type MetricDelta = {
delta_pp: number;
margin_pp: number;
distinguishable: boolean;
};
export type SurfaceMetric = { export type SurfaceMetric = {
key: string; key: string;
label: string; label: string;
@@ -12,6 +26,10 @@ export type SurfaceMetric = {
skip_rate: number; skip_rate: number;
avg_completion: number; avg_completion: number;
low_confidence: boolean; low_confidence: boolean;
// Absent on the baseline row itself, and whenever the samples are too thin
// for a margin to mean anything.
skip_delta?: MetricDelta;
completion_delta?: MetricDelta;
// Present when the surface's builder stamps pick-kind provenance and // Present when the surface's builder stamps pick-kind provenance and
// the window holds attributed plays (#1249, generalized #1270): For // the window holds attributed plays (#1249, generalized #1270): For
// You's taste/fresh split, Discover's candidate buckets, the tiered // You's taste/fresh split, Discover's candidate buckets, the tiered
+21 -7
View File
@@ -216,9 +216,17 @@
return plays > 0 ? hits / plays : 0; return plays > 0 ? hits / plays : 0;
} }
function latest(s: TrendSeries): { skip: number; completion: number } { // The "latest" columns are ONE WEEK while the Plays column is the whole
// window, which is a trap: a 40% skip rate off 17 plays sat next to a
// four-figure Plays total and read as a solid signal. It isn't — I misread
// exactly this and briefly concluded Deep cuts was the worst surface, when
// over 180 days it's one of the best (#2495). So the week's own play count
// comes back with the rates and is rendered beside them.
function latest(s: TrendSeries): { skip: number; completion: number; plays: number } {
const last = s.points[s.points.length - 1]; const last = s.points[s.points.length - 1];
return last ? { skip: last.skip_rate, completion: last.avg_completion } : { skip: 0, completion: 0 }; return last
? { skip: last.skip_rate, completion: last.avg_completion, plays: last.plays }
: { skip: 0, completion: 0, plays: 0 };
} }
function pct(v: number): string { function pct(v: number): string {
@@ -423,6 +431,9 @@
Skip rate per surface over the last {trends?.weeks ?? 12} weeks (lower is better; all Skip rate per surface over the last {trends?.weeks ?? 12} weeks (lower is better; all
users aggregated, rates only). Dashed ticks mark tuning changes. Taste hit is the share users aggregated, rates only). Dashed ticks mark tuning changes. Taste hit is the share
of plays whose artist fits the current taste profile. of plays whose artist fits the current taste profile.
<span class="font-medium">The skip and completion columns show the most recent week
alone</span>, not the whole window — the figure after the skip rate is that week's
play count, so a rate drawn from a handful of listens reads as what it is.
</p> </p>
</div> </div>
{#if trendsFailed} {#if trendsFailed}
@@ -439,10 +450,10 @@
<tr class="text-left text-text-secondary"> <tr class="text-left text-text-secondary">
<th class="py-1 font-medium">Surface</th> <th class="py-1 font-medium">Surface</th>
<th class="py-1 font-medium">Skip rate by week</th> <th class="py-1 font-medium">Skip rate by week</th>
<th class="py-1 text-right font-medium">Plays</th> <th class="py-1 text-right font-medium">Plays<span class="font-normal text-xs"> (window)</span></th>
<th class="py-1 text-right font-medium">Latest skip</th> <th class="py-1 text-right font-medium">Skip<span class="font-normal text-xs"> (last wk)</span></th>
<th class="py-1 text-right font-medium">Latest completion</th> <th class="py-1 text-right font-medium">Completion<span class="font-normal text-xs"> (last wk)</span></th>
<th class="py-1 text-right font-medium">Taste hit</th> <th class="py-1 text-right font-medium">Taste hit<span class="font-normal text-xs"> (window)</span></th>
</tr> </tr>
</thead> </thead>
<tbody> <tbody>
@@ -493,7 +504,10 @@
</svg> </svg>
</td> </td>
<td class="py-1.5 text-right tabular-nums">{s.plays}</td> <td class="py-1.5 text-right tabular-nums">{s.plays}</td>
<td class="py-1.5 text-right tabular-nums">{pct(latest(s).skip)}</td> <td class="py-1.5 text-right tabular-nums">
{pct(latest(s).skip)}
<span class="text-xs text-text-secondary">/{latest(s).plays}</span>
</td>
<td class="py-1.5 text-right tabular-nums">{pct(latest(s).completion)}</td> <td class="py-1.5 text-right tabular-nums">{pct(latest(s).completion)}</td>
<td class="py-1.5 text-right tabular-nums">{pct(windowTasteHitRate(s))}</td> <td class="py-1.5 text-right tabular-nums">{pct(windowTasteHitRate(s))}</td>
</tr> </tr>
+23 -3
View File
@@ -195,9 +195,29 @@ describe('Admin tuning page', () => {
await waitFor(() => expect(screen.getByText('Weekly trends')).toBeInTheDocument()); await waitFor(() => expect(screen.getByText('Weekly trends')).toBeInTheDocument());
expect(screen.getByTestId('sparkline-radio')).toBeInTheDocument(); expect(screen.getByTestId('sparkline-radio')).toBeInTheDocument();
expect(screen.getByTestId('sparkline-discover')).toBeInTheDocument(); expect(screen.getByTestId('sparkline-discover')).toBeInTheDocument();
// Latest skip rate column for radio = 40% (also discover's latest // The skip column is the LAST WEEK's rate, now carrying that week's play
// completion, hence getAllBy). // count so a rate off a handful of plays reads as what it is (#2495).
expect(screen.getAllByText('40%').length).toBeGreaterThan(0); // Radio's latest week: 40% skip over 15 plays; Discover's: 60% over 5.
expect(screen.getByText('/15')).toBeInTheDocument();
expect(screen.getByText('/5')).toBeInTheDocument();
// Completion columns are unchanged and still bare percentages — radio 70%.
expect(screen.getByText('70%')).toBeInTheDocument();
// '40%' is now genuinely ambiguous: radio's latest SKIP rate and discover's
// latest COMPLETION are both 40%. testing-library matches an element's own
// direct text nodes, so the skip cell still matches despite its trailing
// play-count span. Assert the count rather than pretending it's unique.
expect(screen.getAllByText('40%')).toHaveLength(2);
// The window/last-week distinction has to be visible in the headers, or the
// Plays total reads as the denominator of the skip rate. That misreading is
// what #2495 was filed over.
//
// Queried as column headers rather than by text: the caption below also
// mentions "Plays", and matching on the word finds the prose too.
// Exact accessible names: /^Skip/ also matches the "Skip rate by week"
// sparkline column, and /Plays/ matched the caption prose before that.
expect(screen.getByRole('columnheader', { name: 'Plays (window)' })).toBeInTheDocument();
expect(screen.getByRole('columnheader', { name: 'Skip (last wk)' })).toBeInTheDocument();
expect(screen.getByRole('columnheader', { name: 'Completion (last wk)' })).toBeInTheDocument();
// The knob turn is listed under the chart AND tooltipped on each // The knob turn is listed under the chart AND tooltipped on each
// sparkline's marker tick, hence getAllBy. // sparkline's marker tick, hence getAllBy.
expect( expect(
+49 -5
View File
@@ -30,6 +30,31 @@
return genres.filter((g: GenreCount) => g.genre.toLowerCase().includes(q)); return genres.filter((g: GenreCount) => g.genre.toLowerCase().includes(q));
}); });
// Count-first by default because that's what the server returns and it's the
// right default: the head of this list is genuinely where you're going. But a
// real library runs to several hundred genres with a long tail of one-offs
// (391 / ~3.7 per track on the operator's), and at that size "I know roughly
// what it's called" needs A-Z as much as filtering does.
//
// View state only, not a query parameter — same as `filter` above. Neither is
// worth making shareable, and putting one in the URL and not the other would
// be the inconsistent choice.
let sortMode = $state<'count' | 'name'>('count');
const visibleGenres = $derived.by(() => {
// Copy before sorting. With no filter applied `filteredGenres` IS the array
// held by the query cache, and Array.sort mutates in place — sorting it
// directly would reorder cached data for every other consumer.
const list = [...filteredGenres];
if (sortMode === 'name') {
// sensitivity 'base' so case and accents don't split neighbours apart.
return list.sort((a: GenreCount, b: GenreCount) =>
a.genre.localeCompare(b.genre, undefined, { sensitivity: 'base' })
);
}
return list; // server order: count DESC, then name
});
let albums = $state<AlbumRef[]>([]); let albums = $state<AlbumRef[]>([]);
let total = $state(0); let total = $state(0);
let loading = $state(false); let loading = $state(false);
@@ -153,12 +178,30 @@
<h1 class="font-display text-2xl font-medium text-text-primary">Genres</h1> <h1 class="font-display text-2xl font-medium text-text-primary">Genres</h1>
{#if !index.isPending && !index.isError} {#if !index.isPending && !index.isError}
<p class="text-sm text-text-secondary"> <p class="text-sm text-text-secondary">
{#if filter.trim()}
{visibleGenres.length} of {genres.length} genres
{:else}
{genres.length} {genres.length === 1 ? 'genre' : 'genres'}, straight from your file tags {genres.length} {genres.length === 1 ? 'genre' : 'genres'}, straight from your file tags
{/if}
</p> </p>
{/if} {/if}
</div> </div>
{#if genres.length > 0} {#if genres.length > 0}
<div class="flex flex-wrap items-center gap-2">
<QuickFilter bind:value={filter} placeholder="Filter genres" /> <QuickFilter bind:value={filter} placeholder="Filter genres" />
<label class="flex items-center gap-1.5 text-xs text-text-secondary">
Sort
<select
bind:value={sortMode}
aria-label="Sort genres"
class="rounded border border-border bg-surface px-2 py-1 text-sm text-text-primary
focus-visible:outline focus-visible:outline-2 focus-visible:outline-accent"
>
<option value="count">Most tracks</option>
<option value="name">AZ</option>
</select>
</label>
</div>
{/if} {/if}
</header> </header>
@@ -180,16 +223,17 @@
</a> </a>
{/snippet} {/snippet}
</EmptyState> </EmptyState>
{:else if filter.trim() && filteredGenres.length === 0} {:else if filter.trim() && visibleGenres.length === 0}
<p class="text-text-secondary"> <p class="text-text-secondary">
No genres match <span class="font-medium">'{filter.trim()}'</span>. No genres match <span class="font-medium">'{filter.trim()}'</span>.
</p> </p>
{:else} {:else}
<!-- Ordered by track count, not alphabetically: raw tags carry a long <!-- Defaults to track count rather than alphabetical: raw tags carry a
tail of one-offs, so alphabetical would bury the handful of genres long tail of one-offs, so A-Z would bury the handful of genres you
you actually have a library's worth of. --> actually have a library's worth of. The Sort control lets you ask for
A-Z when you already know roughly what you're looking for. -->
<ul class="grid grid-cols-1 gap-2 sm:grid-cols-2 lg:grid-cols-3"> <ul class="grid grid-cols-1 gap-2 sm:grid-cols-2 lg:grid-cols-3">
{#each filteredGenres as g (g.genre)} {#each visibleGenres as g (g.genre)}
<li> <li>
<a <a
href={genreHref(g.genre)} href={genreHref(g.genre)}
@@ -98,6 +98,57 @@ describe('/library/genres index', () => {
expect(screen.getByRole('link', { name: /^rock 2$/ })).toBeInTheDocument(); expect(screen.getByRole('link', { name: /^rock 2$/ })).toBeInTheDocument();
}); });
// #2468: the operator's real library is 391 genres at ~3.7 per track, so the
// count-first default buries anything you can already name. AZ is the ask.
test('AZ sort reorders the list without mutating the source array', async () => {
const data = [
{ genre: 'Rock', track_count: 8974 },
{ genre: 'Ambient', track_count: 646 },
{ genre: 'jazz', track_count: 1406 }
];
asMock(createGenresQuery).mockReturnValue(mockQuery({ data }));
render(GenresPage);
const order = () =>
screen.getAllByRole('link').map((a) => a.textContent?.trim().split(/\s+/)[0]);
// Count mode PRESERVES the server's order rather than re-sorting client
// side — the server already returns count DESC, name ASC, and duplicating
// that here would be two orderings to keep in step. The fixture is
// deliberately not in count order so this asserts pass-through, not luck.
expect(order()).toEqual(['Rock', 'Ambient', 'jazz']);
await fireEvent.change(screen.getByLabelText('Sort genres'), { target: { value: 'name' } });
// Case-insensitive, so 'jazz' sorts between Ambient and Rock rather than
// after both.
expect(order()).toEqual(['Ambient', 'jazz', 'Rock']);
// The query cache's own array must not have been reordered in place —
// Array.sort mutates, and with no filter applied the derived list IS that
// array.
expect(data.map((d) => d.genre)).toEqual(['Rock', 'Ambient', 'jazz']);
});
test('filtering reports the visible subset against the total', async () => {
asMock(createGenresQuery).mockReturnValue(
mockQuery({
data: [
{ genre: 'Rock', track_count: 10 },
{ genre: 'Ska Punk', track_count: 5 },
{ genre: 'Jazz', track_count: 3 }
]
})
);
render(GenresPage);
expect(screen.getByText(/3 genres, straight from your file tags/)).toBeInTheDocument();
await fireEvent.input(screen.getByLabelText('Filter genres'), { target: { value: 'punk' } });
// QuickFilter debounces by 120ms.
await waitFor(() => expect(screen.getByText('1 of 3 genres')).toBeInTheDocument());
});
test('empty library explains where genres come from', () => { test('empty library explains where genres come from', () => {
asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] })); asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] }));
render(GenresPage); render(GenresPage);
+52 -27
View File
@@ -12,7 +12,7 @@
createRecommendationMetricsQuery, createRecommendationMetricsQuery,
type RecommendationMetrics, type RecommendationMetrics,
type SurfaceIntent, type SurfaceIntent,
type SurfaceMetric type MetricDelta
} from '$lib/api/metrics'; } from '$lib/api/metrics';
import { theme, setTheme, type ThemePreference } from '$lib/stores/theme.svelte'; import { theme, setTheme, type ThemePreference } from '$lib/stores/theme.svelte';
import { player, setCrossfade } from '$lib/player/store.svelte'; import { player, setCrossfade } from '$lib/player/store.svelte';
@@ -45,20 +45,39 @@
return `${(v * 100).toFixed(0)}%`; return `${(v * 100).toFixed(0)}%`;
} }
// Delta in percentage points vs the baseline, signed ("+12" / "5"). // Deltas come from the server with their margin of error (#2495). The client
function deltaPts(value: number, baseline: number): string { // no longer subtracts rates itself: the margin needs the sample sizes and
const pts = Math.round((value - baseline) * 100); // variances, and having both clients re-derive it invites them to disagree.
return pts > 0 ? `+${pts}` : `${pts}`; //
// A delta that isn't distinguishable from zero is prefixed "≈" and dimmed.
// That is the point of this whole change — the card used to render a 12 on
// 59 plays exactly as boldly as a 6 on 400, and the first of those is noise.
function deltaText(d: MetricDelta | undefined): string {
if (!d) return '';
const pts = Math.round(d.delta_pp);
const signed = pts > 0 ? `+${pts}` : `${pts}`;
return d.distinguishable ? signed : `≈${signed}`;
} }
// A surface's skip delta is "worse" when it skips more than the function deltaTitle(d: MetricDelta | undefined): string | undefined {
// baseline; completion delta is "worse" when it completes less. if (!d) return undefined;
function skipDeltaClass(m: SurfaceMetric, baseline: SurfaceMetric): string { const range = `${d.delta_pp.toFixed(1)} ± ${d.margin_pp.toFixed(1)} points vs baseline`;
return m.skip_rate > baseline.skip_rate ? 'text-danger' : 'text-text-secondary'; return d.distinguishable
? `${range} (95% confidence)`
: `${range} — not distinguishable from zero at 95% confidence, so read this as no measured difference.`;
} }
function completionDeltaClass(m: SurfaceMetric, baseline: SurfaceMetric): string { // A skip delta is "worse" above the baseline; a completion delta is "worse"
return m.avg_completion < baseline.avg_completion ? 'text-danger' : 'text-text-secondary'; // below it. Neither gets a colour unless it's distinguishable — colouring
// noise red is what made the old card misleading.
function skipDeltaClass(d: MetricDelta | undefined): string {
if (!d?.distinguishable) return 'text-text-secondary opacity-60';
return d.delta_pp > 0 ? 'text-danger' : 'text-text-secondary';
}
function completionDeltaClass(d: MetricDelta | undefined): string {
if (!d?.distinguishable) return 'text-text-secondary opacity-60';
return d.delta_pp < 0 ? 'text-danger' : 'text-text-secondary';
} }
// Pick-kind breakdowns are collapsed by default (#1270): with every // Pick-kind breakdowns are collapsed by default (#1270): with every
@@ -384,18 +403,18 @@
<td class="py-1 text-right tabular-nums">{m.plays}</td> <td class="py-1 text-right tabular-nums">{m.plays}</td>
<td class="py-1 text-right tabular-nums"> <td class="py-1 text-right tabular-nums">
{pct(m.skip_rate)} {pct(m.skip_rate)}
{#if baseline} {#if m.skip_delta}
<span class="ml-1 text-xs {skipDeltaClass(m, baseline)}"> <span class="ml-1 text-xs {skipDeltaClass(m.skip_delta)}"
{deltaPts(m.skip_rate, baseline.skip_rate)} data-testid="skip-delta-{m.key}"
</span> title={deltaTitle(m.skip_delta)}>{deltaText(m.skip_delta)}</span>
{/if} {/if}
</td> </td>
<td class="py-1 text-right tabular-nums"> <td class="py-1 text-right tabular-nums">
{pct(m.avg_completion)} {pct(m.avg_completion)}
{#if baseline} {#if m.completion_delta}
<span class="ml-1 text-xs {completionDeltaClass(m, baseline)}"> <span class="ml-1 text-xs {completionDeltaClass(m.completion_delta)}"
{deltaPts(m.avg_completion, baseline.avg_completion)} data-testid="completion-delta-{m.key}"
</span> title={deltaTitle(m.completion_delta)}>{deltaText(m.completion_delta)}</span>
{/if} {/if}
</td> </td>
</tr> </tr>
@@ -420,18 +439,18 @@
<td class="py-1 text-right text-xs tabular-nums">{b.plays}</td> <td class="py-1 text-right text-xs tabular-nums">{b.plays}</td>
<td class="py-1 text-right text-xs tabular-nums"> <td class="py-1 text-right text-xs tabular-nums">
{pct(b.skip_rate)} {pct(b.skip_rate)}
{#if baseline} {#if b.skip_delta}
<span class="ml-1 {skipDeltaClass(b, baseline)}"> <span class="ml-1 {skipDeltaClass(b.skip_delta)}"
{deltaPts(b.skip_rate, baseline.skip_rate)} data-testid="skip-delta-{b.key}"
</span> title={deltaTitle(b.skip_delta)}>{deltaText(b.skip_delta)}</span>
{/if} {/if}
</td> </td>
<td class="py-1 text-right text-xs tabular-nums"> <td class="py-1 text-right text-xs tabular-nums">
{pct(b.avg_completion)} {pct(b.avg_completion)}
{#if baseline} {#if b.completion_delta}
<span class="ml-1 {completionDeltaClass(b, baseline)}"> <span class="ml-1 {completionDeltaClass(b.completion_delta)}"
{deltaPts(b.avg_completion, baseline.avg_completion)} data-testid="completion-delta-{b.key}"
</span> title={deltaTitle(b.completion_delta)}>{deltaText(b.completion_delta)}</span>
{/if} {/if}
</td> </td>
</tr> </tr>
@@ -442,6 +461,12 @@
</table> </table>
</div> </div>
{/each} {/each}
<p class="text-xs text-text-secondary">
Deltas compare each surface with your manual plays. A delta marked
<span class="opacity-60"></span> is smaller than its own margin of error at this
sample size — it can't be told apart from no difference, however big it looks.
Hover any delta for its range.
</p>
{:else} {:else}
<p class="text-sm text-text-secondary"> <p class="text-sm text-text-secondary">
No plays recorded yet. Play something from For You, Discover, or a mix. No plays recorded yet. Play something from For You, Discover, or a mix.
+68
View File
@@ -241,6 +241,74 @@ describe('Settings page — Recommendation metrics card', () => {
expect(screen.queryByText(/Taste picks/)).not.toBeInTheDocument(); expect(screen.queryByText(/Taste picks/)).not.toBeInTheDocument();
}); });
// #2495: the card used to render a delta computed client-side with no notion
// of uncertainty, so a -12 on 59 plays looked exactly as solid as a -6 on 400.
// Deltas now arrive from the server with a margin, and an indistinguishable
// one is marked with "≈" and dimmed rather than coloured.
test('a delta smaller than its margin is marked as indistinguishable', async () => {
setupPage();
metricsMock.data = {
window_days: 30,
baseline: metric('manual', 'Manual library plays', { plays: 400, skip_rate: 0.27 }),
groups: [
{
intent: 'discovery',
label: 'Discovery mixes',
surfaces: [
metric('discover', 'Discover', {
plays: 59,
skip_rate: 0.153,
// 13.3pp gap, but the margin at n=59 is wider than the gap.
skip_delta: { delta_pp: -13.3, margin_pp: 14.5, distinguishable: false },
completion_delta: { delta_pp: 28.0, margin_pp: 12.1, distinguishable: true }
})
]
}
]
};
render(SettingsPage);
await waitFor(() => expect(screen.getByText('Discover')).toBeInTheDocument());
// Targeted by test id rather than text: the legend below the table also
// contains a "≈", so matching on the glyph finds the explanation instead of
// the delta. (It did, on the first attempt at this test.)
const skip = screen.getByTestId('skip-delta-discover');
expect(skip).toHaveTextContent('≈-13');
expect(skip).toHaveAttribute('title', expect.stringContaining('not distinguishable from zero'));
// It must NOT be coloured as a real regression/improvement.
expect(skip.className).toContain('opacity-60');
// The completion delta clears its margin, so it renders plainly.
const completion = screen.getByTestId('completion-delta-discover');
expect(completion).toHaveTextContent('+28');
expect(completion.className).not.toContain('opacity-60');
// And the legend explains the glyph rather than leaving it a mystery.
expect(screen.getByText(/smaller than its own margin of error/i)).toBeInTheDocument();
});
// A delta is omitted entirely when the samples are too thin for a margin to
// mean anything — the server decides that, and the cell must simply show the
// rate rather than a bare "0".
test('a surface with no delta shows its rate and nothing else', async () => {
setupPage();
metricsMock.data = {
window_days: 30,
baseline: metric('manual', 'Manual library plays', { plays: 400 }),
groups: [
{
intent: 'go_to',
label: 'Go-to surfaces',
surfaces: [metric('radio', 'Radio', { plays: 1, skip_rate: 0 })]
}
]
};
render(SettingsPage);
await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument());
expect(screen.queryByTestId('skip-delta-radio')).not.toBeInTheDocument();
expect(screen.queryByTestId('completion-delta-radio')).not.toBeInTheDocument();
});
test('surfaces without a breakdown render no toggle and no sub-rows', async () => { test('surfaces without a breakdown render no toggle and no sub-rows', async () => {
setupPage(); setupPage();
metricsMock.data = { metricsMock.data = {