Tuner: passive "different tuning" badge cue naming the retune (issue E, stage 2.5/3)#657
Merged
Merged
Conversation
This was referenced Jun 30, 2026
This was referenced Jul 1, 2026
Closed
Merged
byrongamatos
added a commit
that referenced
this pull request
Jul 1, 2026
…e (tuner-E #657 review) Three review fixes for the passive "different tuning" badge cue stage: - Stale async cue (#678): _refreshCoverageCue awaited coverageReport then wrote the DOM unconditionally, so a slow /api/settings fetch could restore the previous song's amber ring after song:loading / leaving the player. Add a monotonic token bumped on every refresh and both clear paths; apply the awaited report only if the token still matches. - "Unknown" rendered as "needs retune" (#679): the plugin returns a conservative all-false report on a fetch hiccup; the cue painted that as an amber "retune the reference pitch" ring. Collapse a no-signal report (not covered, no reference / retune / cantCover) to null (no cue) via _meaningfulReport(). A genuine not-covered report always carries reference / retune / cantCover, so real cues are preserved. - Duplicate /api/settings fetch (#680): the auto-open gate and the badge cue both call coverageReport() per song:ready. Cache the coverage promise per song (keyed by session + tuning + centOffset) so they share one fetch; invalidate on song:loading, instrument:changed, and working-tuning-changed so it can't go stale within a song. Tests: tuner_auto_open.test.js — concurrent reports share one fetch, a new song refetches (fails without the cache). 34 JS tests green. Codex-reviewed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos
added a commit
that referenced
this pull request
Jul 1, 2026
…e (tuner-E #657 review) (#683) Three review fixes for the passive "different tuning" badge cue stage: - Stale async cue (#678): _refreshCoverageCue awaited coverageReport then wrote the DOM unconditionally, so a slow /api/settings fetch could restore the previous song's amber ring after song:loading / leaving the player. Add a monotonic token bumped on every refresh and both clear paths; apply the awaited report only if the token still matches. - "Unknown" rendered as "needs retune" (#679): the plugin returns a conservative all-false report on a fetch hiccup; the cue painted that as an amber "retune the reference pitch" ring. Collapse a no-signal report (not covered, no reference / retune / cantCover) to null (no cue) via _meaningfulReport(). A genuine not-covered report always carries reference / retune / cantCover, so real cues are preserved. - Duplicate /api/settings fetch (#680): the auto-open gate and the badge cue both call coverageReport() per song:ready. Cache the coverage promise per song (keyed by session + tuning + centOffset) so they share one fetch; invalidate on song:loading, instrument:changed, and working-tuning-changed so it can't go stale within a song. Tests: tuner_auto_open.test.js — concurrent reports share one fetch, a new song refetches (fails without the cache). 34 JS tests green. Codex-reviewed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 1, 2026
Closed
byrongamatos
force-pushed
the
feat/v3-tuner-coverage-check
branch
from
July 1, 2026 08:25
3f31622 to
144a078
Compare
Building on the coverage check: when you enter a song your current
instrument doesn't cover, the topbar tuner badge gets an amber ring + a
tooltip naming the change (e.g. "retune B->A", or "the reference pitch"
for an A440 vs A432 mismatch). Advisory only -- it never auto-opens the
panel; recomputed on song:ready, cleared on song-load / leaving the
player.
Refactors the coverage check into a structured report
(window._tunerAutoOpen.coverageReport -> { covered, retune:[{from,to}],
reference, cantCover }); the boolean gate now wraps it. The cue is
CSS-free (inline ring + native tooltip, no Tailwind rebuild) and no-ops
when the tuner plugin is absent.
Touches static/v3/badges.js (cue) + plugins/tuner/screen.js (report).
v3-only. Stacked on #656 (issue E stage 2.5/3). The splitscreen-suppress
and no-usable-input guards move to E2 (the playback gate).
Tests: tests/js/tuner_auto_open.test.js (report names the strings,
reference mismatch, badge wiring).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
…-directions retune prompt (working-tuning PR 3) (#660) The §4 coverage check compared each song against the player's fixed instrument-profile tuning, so the tuner only ever prompted *away* from a "home" tuning (E -> Drop C#) and stayed silent coming back (Drop C# -> E), even though the player had physically retuned. _playerTuning() now reads the host's live per-instrument working tuning (window.feedBack.workingTuning, keyed by the selected instrument from /api/settings) instead of re-deriving from the static settings tuning, so coverage is measured against what the instrument is ACTUALLY in and prompts both directions. On clearing an auto-opened tuner, _publishWorkingTuning() writes that song's tuning as the instrument's live working tuning ('assumed' — PR 4's explicit "I tuned / Skip" refines the write-point), so the next song is judged against where the player now is. Per-instrument (guitar vs bass tracked separately). Feature-detected: falls back to the static /api/settings tuning when the working-tuning capability is absent, so the 27 existing coverage tests are unchanged. Builds on PR 1 (host workingTuning) + PR 2 (instrument->chart routing). Tests: tests/js/tuner_auto_open.test.js — +2 (both-directions coverage via a live Drop-D working tuning; publish-on-clear targets the right instrument slot); 29 pass total. Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…#655 review) (#681) Two review fixes for the auto-open opt-in+persist stage: - enable() wasn't transactional. The panel (with the ×/Skip buttons) is shown before `await _tunerAudio.start()`, and `_state.enabled` was only set after it. A ×/Skip dismiss during that await hit disable() with wasEnabled=false, then enable() completed and flipped enabled on — an enabled-but-hidden zombie. Guard the open with an `_openGen` token bumped on every enable()/disable(); after the audio-start await, bail if superseded instead of enabling. Closes #675. - Config wasn't fail-closed. routes.py normalized the opt-in with bool(data.get("autoOpenOnTuningChange", False)), so "false"/"0"/junk coerced to True. Accept only a real JSON boolean. Closes #676. Tests: tuner_auto_open.test.js (dismiss-mid-open stays disabled — fails without the token guard), test_config.py (auto-open default-false + fail-closed on non-bool). 34 JS + 24 config tests green. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n (tuner-E #656 review) (#682) _playerTuning() is documented as conservative ("missing data → not covered → still prompt"), but when /api/settings carried no instrument identity (a fresh profile: _default_settings() omits instrument/string_count/tuning) it invented guitar/6/440/ standard, so an unconfigured player was treated as 6-string E-standard and coverage suppressed the auto-open (and badge cue) for matching songs. The post-#660 rewrite only returned null when the whole fetch failed (!s), not when settings existed but lacked an instrument. Now return null unless there's a confident identity — any of instrument/string_count/ tuning in settings, or live working-tuning offsets. A configured standard guitar still covers a standard song (no regression). Closes #677. Tests: tuner_auto_open.test.js — empty-settings → not covered (fails without the fix); configured standard guitar → still covered. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e (tuner-E #657 review) (#683) Three review fixes for the passive "different tuning" badge cue stage: - Stale async cue (#678): _refreshCoverageCue awaited coverageReport then wrote the DOM unconditionally, so a slow /api/settings fetch could restore the previous song's amber ring after song:loading / leaving the player. Add a monotonic token bumped on every refresh and both clear paths; apply the awaited report only if the token still matches. - "Unknown" rendered as "needs retune" (#679): the plugin returns a conservative all-false report on a fetch hiccup; the cue painted that as an amber "retune the reference pitch" ring. Collapse a no-signal report (not covered, no reference / retune / cantCover) to null (no cue) via _meaningfulReport(). A genuine not-covered report always carries reference / retune / cantCover, so real cues are preserved. - Duplicate /api/settings fetch (#680): the auto-open gate and the badge cue both call coverageReport() per song:ready. Cache the coverage promise per song (keyed by session + tuning + centOffset) so they share one fetch; invalidate on song:loading, instrument:changed, and working-tuning-changed so it can't go stale within a song. Tests: tuner_auto_open.test.js — concurrent reports share one fetch, a new song refetches (fails without the cache). 34 JS tests green. Codex-reviewed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos
force-pushed
the
feat/v3-tuner-coverage-badge
branch
from
July 1, 2026 08:27
0013c9b to
00851b5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #656 (which is stacked on #655) — review/merge those first; base is the #656 branch so this shows only the badge-cue diff. I'll rebase down the chain as each merges.
Building on the instrument-coverage check, the topbar tuner badge now passively flags when a song needs a different tuning — and names the change.
What it does
When you enter a song your current instrument doesn't cover, the tuner badge gets an amber ring + a tooltip naming the retune:
It's purely advisory — it never auto-opens the panel (tap the badge to tune). Recomputed on
song:ready, cleared when a new song loads or you leave the player.How
coverageReporton the tuner plugin (window._tunerAutoOpen.coverageReport→{ covered, retune: [{ from, to }], reference, cantCover }); the boolean gate now wraps it.static/v3/badges.jsconsumes the report onsong:readyand renders the cue. CSS-free (inline ring + nativetitle— no Tailwind rebuild), and no-ops when the tuner plugin isn't installed.static/v3/badges.js) in addition to the plugin — first of the tuner-family PRs to do so. v3-only.Testing
tests/js/tuner_auto_open.test.js: 27/27 — incl. the report yielding{ from:'B', to:'A' }for the Drop-A case, a reference-mismatch report, and the badge wiring.mainand unrelated.Scope / follow-up
The splitscreen-suppress and no-usable-input guards move to E2 (the playback gate), where the no-input guard actually matters for its no-trap rule.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF