Tuner auto-open: instrument-coverage check (issue E, stage 2/3) by ChrisBeWithYou · Pull Request #656 · got-feedBack/feedBack

ChrisBeWithYou · 2026-06-30T03:00:38Z

Stacked on #655 (review/merge that first; base is its branch so this shows only the coverage diff — I'll rebase onto main once #655 lands).

Makes the opt-in auto-open tuning-coverage-aware: it now prompts only when your current physical tuning doesn't already cover the song, instead of on every tuning change.

Why

FeedBack is tune-to-song — the highway draws tab in the song's tuning (highway.js renders from songInfo.tuning and shows a "Tuning:" HUD), so the player tunes their instrument to match. An extended-range player keeps one physical tuning and plays subsets of it, so the old "different from the last song" trigger over-prompted them.

The check

Aligns the song's open-string tuning string-for-string against your instrument (read from core /api/settings — the v3 instrument selector), matched by pitch:

8-string F♯ player → 6-E or 7-B song: no prompt (your top strings already are E-/B-standard).
8-string F♯ player → Drop-A 7-string: still prompts — its low A isn't an open string you have.
6-string player → Drop D: prompts (low E must drop to D).
A432 player → A440 song, or an octave-down centOffset (now accounted for — it was ignored before): prompts.
Conservative fallback (prompt) when no instrument is declared or the lookup fails, so a real retune is never silently skipped. v3-only (the instrument selector is v3). No core changes (all in plugins/tuner/screen.js).

Testing

tests/js/tuner_auto_open.test.js: 24/24 (18 existing + 6 new — covered vs uncovered, the Drop-A case, reference-pitch mismatch, and a direct contiguous-alignment check). The sandbox now loads the real tuning-utils.js so the pitch math is real.
Full JS suite: 816 / 803 pass / 13 fail — the 13 are pre-existing on main and unrelated.

Scope / follow-up

Deferred to E1.6 to keep this reviewable: a passive "different tuning" badge cue that names the string(s) to retune (e.g. "string 2: B → A"), plus the splitscreen / no-usable-input guards. E2 is the separate playback-gate (holdAutoplay).

🤖 Generated with Claude Code

https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF

…n (tuner-E #656 review) _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>

…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>

With the opt-in auto-open on, only prompt when the player's current physical tuning doesn't already cover the song. FeedBack is tune-to-song (the highway draws tab in the song's tuning), so the check aligns the song's open-string tuning string-for-string against the player's instrument: - An 8-string F# player gets no prompt for a 6-/7-string standard song (its top strings already match those tunings). - A song needing an open string the player lacks (e.g. a Drop-A 7-string's low A on an F# 8-string) still prompts. - A whole-instrument reference difference (A440 vs A432, or an octave-down centOffset, previously ignored) also prompts. Reads the player's instrument from core /api/settings (the v3 instrument selector, a stable physical reference); conservative fallback (prompt) when undeclared or unavailable, so a real retune is never silently skipped. v3-only. All in plugins/tuner/screen.js; no core changes. Stacked on #655. Follow-up E1.6: a passive badge cue that names the strings to retune, plus splitscreen / no-usable-input guards. Tests: tests/js/tuner_auto_open.test.js (covered/uncovered, the Drop-A case, reference mismatch, direct contiguous alignment). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF

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

…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, stage 2.5/3) (#657) * Tuner: passive "different tuning" badge cue that names the retune 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 * feat(tuner): read/write the live per-instrument working tuning — both-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> * fix(tuner): transactional open + fail-closed auto-open config (tuner-E #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> * fix(tuner): coverage stays conservative when the instrument is unknown (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> * fix(tuner): badge coverage cue staleness + unknown-as-warning + dedupe (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> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Byron Gamatos <xasiklas@gmail.com>

This was referenced Jun 30, 2026

Tuner: passive "different tuning" badge cue naming the retune (issue E, stage 2.5/3) #657

Merged

feat(tuner): live per-instrument working tuning — both-directions retune prompt (working-tuning PR 3) #660

Merged

byrongamatos changed the base branch from fix/v3-tuner-auto-open-persist-toggle to main July 1, 2026 08:24

byrongamatos force-pushed the feat/v3-tuner-coverage-check branch from 3f31622 to 144a078 Compare July 1, 2026 08:25

byrongamatos merged commit 6b0e37a into main Jul 1, 2026
1 of 3 checks passed

byrongamatos deleted the feat/v3-tuner-coverage-check branch July 1, 2026 08:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Tuner auto-open: instrument-coverage check (issue E, stage 2/3)#656

Tuner auto-open: instrument-coverage check (issue E, stage 2/3)#656
byrongamatos merged 1 commit into
mainfrom
feat/v3-tuner-coverage-check

ChrisBeWithYou commented Jun 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ChrisBeWithYou commented Jun 30, 2026

Why

The check

Testing

Scope / follow-up

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants