feat(tuner): live per-instrument working tuning — both-directions retune prompt (working-tuning PR 3)#660
Merged
byrongamatos merged 1 commit intoJul 1, 2026
Conversation
This was referenced Jul 1, 2026
…-directions retune prompt (working-tuning PR 3)
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.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
byrongamatos
force-pushed
the
feat/v3-tuner-uses-working-tuning
branch
from
July 1, 2026 07:23
6cf14be to
36fd8d1
Compare
This was referenced Jul 1, 2026
byrongamatos
added a commit
that referenced
this pull request
Jul 1, 2026
…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>
byrongamatos
added a commit
that referenced
this pull request
Jul 1, 2026
…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>
byrongamatos
pushed a commit
that referenced
this pull request
Jul 1, 2026
…-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>
byrongamatos
added a commit
that referenced
this pull request
Jul 1, 2026
…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>
byrongamatos
added a commit
that referenced
this pull request
Jul 1, 2026
…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>
byrongamatos
pushed a commit
that referenced
this pull request
Jul 1, 2026
…ap Skip/Back/Esc (working-tuning PR 4) When the opt-in auto-open fires because a song needs a different tuning, playback now WAITS behind the tuner instead of starting underneath it — the "tune before you play" model. Built on a new generic core hook window.feedBack.holdAutoplay() (mirrors holdAutoExit): the tuner claims the hold synchronously on song:loading (beating the song:ready autostart) and releases it — or a 12s fail-open backstop does — so a wedged plugin can never strand a song. Generation-guarded; manual Play always wins. No one-way trap: - Skip = "I've tuned" -> plays and records the song's tuning as the instrument's current working tuning (the explicit write-point PR 3 left as 'assumed'). - Back to library / Esc -> leave the song, record nothing (reuses requestExitSong; Esc is the existing player shortcut). - The in-panel x is dropped for an auto-open — Skip/Back/Esc are the dismiss surface. This also keeps the write honest: Skip is the only on-player dismiss that records, so leaving never falsely records a tuning. Stacked on #660 (working-tuning PR 3). Core app.js gains only the generic hook (a test asserts it never references the tuner's internals); shell-agnostic. Needs a desktop smoke-test that the tuner mic doesn't contend with note_detect's scoring input under ASIO/exclusive mode (per the design charrette). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
byrongamatos
added a commit
that referenced
this pull request
Jul 1, 2026
…ap Skip/Back/Esc (working-tuning PR 4) (#666) * feat(tuner): gate playback until you've tuned — hold autoplay + no-trap Skip/Back/Esc (working-tuning PR 4) When the opt-in auto-open fires because a song needs a different tuning, playback now WAITS behind the tuner instead of starting underneath it — the "tune before you play" model. Built on a new generic core hook window.feedBack.holdAutoplay() (mirrors holdAutoExit): the tuner claims the hold synchronously on song:loading (beating the song:ready autostart) and releases it — or a 12s fail-open backstop does — so a wedged plugin can never strand a song. Generation-guarded; manual Play always wins. No one-way trap: - Skip = "I've tuned" -> plays and records the song's tuning as the instrument's current working tuning (the explicit write-point PR 3 left as 'assumed'). - Back to library / Esc -> leave the song, record nothing (reuses requestExitSong; Esc is the existing player shortcut). - The in-panel x is dropped for an auto-open — Skip/Back/Esc are the dismiss surface. This also keeps the write honest: Skip is the only on-player dismiss that records, so leaving never falsely records a tuning. Stacked on #660 (working-tuning PR 3). Core app.js gains only the generic hook (a test asserts it never references the tuner's internals); shell-agnostic. Needs a desktop smoke-test that the tuner mic doesn't contend with note_detect's scoring input under ASIO/exclusive mode (per the design charrette). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF * fix(tuner): backstop can't cut off tuning + gate race/token hardening (PR #666 review) Review fixes for the autoplay gate: - The 12s fail-open backstop could start playback UNDER a legitimately-open tuner (a slow / mic-verify retune > 12s). holdAutoplay()'s release now carries a .settle() that cancels the backstop; the tuner calls it once the tuner is confirmed open (_gateClaimed), so the hold becomes deliberate and only a dismiss / song switch releases it. (Fail-open still covers "claimed but wedged before deciding".) - The async song:ready handler could release a NEWER song's gate after its await (global _gateClaimed, no guard). It now snapshots _autoOpenGeneration and bails if a newer song took over. - holdAutoplay guarded by song generation, not per-hold — a stale release from an earlier hold could clear a later one. Each hold now mints a unique token that release()/settle() must match. Tests: source-level assertions for the token, settle(), the settle-on-open call, and the song:ready gen-guard. 45 tuner+speed 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: byrongamatos <xasiklas@gmail.com>
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.
What & why
PR 3 of the working-tuning series — the payoff. The §4 coverage check compared each song against the player's fixed instrument-profile tuning, so the tuner only ever prompted you away from a "home" tuning (E → Drop C#) and stayed silent coming back (Drop C# → E) — even though you'd physically retuned. (Surfaced in testbed play: "it only pops up E → anything-not-E.")
It now measures coverage against your instrument's live, current tuning, so it prompts both directions.
How
_playerTuning()reads the host's live per-instrument working tuning (window.feedBack.workingTuning.get(key), 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._publishWorkingTuning()writes that song's tuning as the instrument's live working tuning (assumed) — so the next song is judged against where you now are. (The write-point is the heuristic Christian chose for now; PR 4's explicit I tuned / Skip replaces it.)/api/settingstuning (today's behavior), so the 27 existing coverage tests are unchanged.Tests
tests/js/tuner_auto_open.test.js— +2: both-directions coverage (a live Drop-D tuning → a Drop-D song is covered, an E song says "tune D→E"), and publish-on-clear targets the right instrument slot. 29 pass.Stacking⚠️
feat/v3-tuner-coverage-badge(the open tuner E-stack Fix tuner auto-open flash: opt-in + persist (issue E, stage 1/3) #655→Tuner auto-open: instrument-coverage check (issue E, stage 2/3) #656→Tuner: passive "different tuning" badge cue naming the retune (issue E, stage 2.5/3) #657), where the coverage code lives — so this diff is just PR 3's changes.workingTuningcapability (#658) + the instrument→chart routing (#659). Both are feature-detected, so this degrades cleanly without them, but the both-directions behavior needs feat(core): host per-instrument workingTuning capability + read-API/event (working-tuning PR 1) #658 merged.🤖 Generated with Claude Code