feat(playlists): flag songs that are not in your current tuning - #1009
Conversation
Making the library's tuning filter instrument-aware does not repair playlists
already built under the old guitar-first behaviour. Those keep their
wrong-tuning songs, so a player still hits a surprise retune mid-practice and
reasonably concludes nothing was fixed.
Adds a per-playlist check: each row is marked against the player's current
tuning, with a summary ("3 of 24 songs are not in your tuning"), a filter to
show only those, and an explicit removal that lists every affected song by
title and states they stay in the library. Flagging is the feature -- nothing
is ever removed without being asked for, and removal reuses the existing
per-song DELETE rather than adding a bulk destructive endpoint.
Reuses the tuner capability's coverage report and `window.feedBack
.workingTuning`, the same pair the library cards already score against,
rather than introducing another source of truth.
Two deliberate departures:
- A coverage report reads "not covered" both for a real mismatch and for a
bail-out it could not evaluate. Only a report carrying an actual reason
counts as a mismatch; the rest render as unknown. This differs from the
library grid, which paints every not-covered song amber -- acceptable on a
grid, not on a hand-curated playlist where a false warning costs trust.
- With no tuning perspective available it makes no claim at all, rather than
defaulting to guitar and reproducing the original bug in a new place.
Playlist rows carry `tuning_offsets` and `bass_only`; a tuning *name* cannot
be scored, since two "Custom Tuning" rows are different tunings.
Fully correct once the instrument-aware tuning filter lands. That dependency
is confined to `rowTuningForCheck()` in static/v3/playlists.js, marked SEAM.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFDokqh2H6mEjk1Kgbi6JW
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
CI's tailwind-fresh gate rebuilds static/tailwind.min.css and hard-fails if the committed file differs. The new chip/summary/filter markup introduces classes the previous build never saw. Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Reverts the regenerated static/tailwind.min.css and reworks the tuning-check markup to use only classes already in the committed sheet. Regenerating that file is not reproducible off CI: nothing pins tailwindcss, autoprefixer or caniuse-lite, so a local `npx -y tailwindcss@3.4.19` resolves different browser data and rewrites unrelated bytes -- a clean checkout of main rebuilds with the -webkit-backdrop-filter prefixes dropped. Committing that output fails the tailwind-fresh gate no matter how many times it is regenerated. Six utilities were new: bg-fb-good/10, border-fb-accent/50, hover:bg-fb-accent/10, list-disc, list-inside, max-h-48, plus gap-x-3/gap-y-2. Substituted bg-fb-good/30, the amber border already used by the mismatch state, hover:bg-fb-card, a literal bullet in a div, max-h-32 and gap-3. Visual intent is unchanged. The removal-confirm test pinned the <li> markup; it now accepts either wrapper, since what it guards is that every song is named and escaped ahead of any DELETE, not which element wraps it. Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Why
Making the library's tuning filter instrument-aware (#1003) does not repair playlists already built under the old guitar-first behaviour. Those keep their wrong-tuning songs, so a player still hits a surprise retune mid-practice and reasonably concludes nothing was fixed.
This matters more on bass than it sounds: heavy strings take minutes to settle after a retune, and detuning far on standard gauges goes floppy and unusable. "Group songs so I don't retune" isn't a convenience — it's the difference between practising and doing setup work.
What this does
A per-playlist check against the player's current tuning:
Flagging is the feature. Nothing is ever removed without being asked for — silent auto-repair of curated user data was considered and rejected. Removal loops the existing per-song
DELETE /api/playlists/{pid}/songs/{fn}rather than adding a bulk destructive endpoint, and targets mismatches only, never unknowns.Reuses the tuner capability's coverage report plus
window.feedBack.workingTuning— the same pair the library cards already score against indecorateTuningChips()— rather than introducing another source of truth.Two deliberate departures
1.
unknownis derived, not reported. The coverage report returns the same "not covered" shape both for a real mismatch and for a bail-out it couldn't evaluate. Only a report carrying an actual reason (named string changes, a reference-pitch gap, too few strings) counts as a mismatch; the rest render as unknown. This differs from the library grid, which paints every not-covered song amber — acceptable on a grid, not on a hand-curated playlist where a false ⚠ costs trust in the whole feature.2. No perspective → no claim. Without
workingTuningor tuner coverage the view renders exactly as before, rather than defaulting to guitar and reproducing the original bug in a new place.States carry text markers (⚠ / ?) as well as colour.
Backend
Playlist rows now carry
tuning_offsetsandbass_only. A tuning name can't be scored — two"Custom Tuning"rows are different tunings — and the check needs to know whether to measure against bass or guitar base pitches._resolve_album_orphannow reports the resolved chart's tuning rather than the dead pin's.Testing
tests/js/v3_playlist_tuning_check.test.js— 20 tests driving the real functions lifted from source viaextractBlock+vm, no re-implementation. Coverage is the one stubbed boundary, and a contract test pins its report shape againstplugins/tuner/screen.jsso fixtures can't drift."",None,"Bassoon","BASS").KeyError: 'tuning_offsets'/KeyError: 'bass_only'against unmodifiedmain.One env note:
eslintcan't run in a fresh worktree (dev dependencies aren't installed there), so the JS is unlinted — pre-existing, but flagging it since style wasn't machine-verified.🤖 Generated with Claude Code
https://claude.ai/code/session_01SFDokqh2H6mEjk1Kgbi6JW