Tuner: passive "different tuning" badge cue naming the retune (issue 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 commit is contained in:
ChrisBeWithYou
2026-07-01 10:28:04 +02:00
committed by GitHub
co-authored by Claude Opus 4.8 Byron Gamatos
parent 6b0e37aa35
commit 6aed8510d7
6 changed files with 439 additions and 39 deletions
+2
View File
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Added
- **The tuner now tracks what tuning your instrument is *actually* in, so it prompts you to retune in BOTH directions — down to a song's tuning, and back up when the next song needs it.** The coverage check used to compare each song against your fixed instrument-profile tuning, so it only ever prompted you *away* from "home" (e.g. E → Drop C#) and stayed silent coming back (Drop C# → E), even though you'd physically retuned. It now reads the host's live **per-instrument working tuning** (`window.feedBack.workingTuning`) — what your selected instrument is currently in — so coverage is measured against your *actual* tuning and fires both ways. When you clear an auto-opened tuner, the tuner publishes that song's tuning as your instrument's live working tuning (`assumed` — an explicit "I tuned / Skip" refines it in a later PR), so the next song is judged against where you now are. **Per-instrument** — your guitar's and bass's tunings are tracked separately (keyed like the selector), so switching instruments uses the right one. Feature-detected: on a host without the working-tuning capability it falls back to the static `/api/settings` tuning (today's behavior). `plugins/tuner/screen.js` (`_playerTuning` reads `workingTuning` keyed by the selected instrument; `_publishWorkingTuning` writes on clear). Builds on the host `workingTuning` foundation (PR 1 of the series) + the instrument→chart routing (PR 2). Tests: `tests/js/tuner_auto_open.test.js` (both-directions coverage via a live Drop-D working tuning; publish-on-clear targets the right instrument slot) — 29 pass.
- **`.jsonc` support for feedpak data files** (feedpak-spec §8, FEP #3 / PR #13). Hand-edited packs may now use the `.jsonc` extension (JSON with C-style `//` line and `/* */` block comments) for any data file the manifest points at — arrangements, notation sidecars, `drum_tab`, `song_timeline`, `lyrics`, and `keys`. New shared `lib/jsonc.py` provides `parse_jsonc(text)` + `load_json(path)` (auto-detects `.jsonc` by suffix, string-aware so comment-like text inside JSON string values is preserved) and is now used by every reader in `lib/sloppak.py` (six side-file sites) and `scripts/lift_keys_notation.py` (three arrangement / song_timeline read sites). The strip regex mirrors the reference validator in `feedpak-spec/tools/validate.py`. This is an additive (MINOR) change: `.jsonc` is opt-in, so any pack that keeps its data files as `.json` is unaffected and needs no regeneration. Note that a `.jsonc` file containing real comments only loads on a reader that implements §8 — a pre-this-change reader calls bare `json.loads` and fails on the comments rather than ignoring them, so don't hand out `.jsonc` packs to older hosts. Tests: `tests/test_sloppak_jsonc_load.py` (covers all six side-file types, the lift helper, and the string-boundary preservation rule end-to-end).
- **The highway now loads the part that matches your selected instrument — a bass player gets the Bass arrangement, not the default Lead/guitar chart.** When you open a song without an explicit arrangement, the WebSocket handler (`server.py` `highway_ws`) reads your selected `instrument` from `config.json` (the same file it already reads for your default-arrangement preference) and routes to the matching part: **bass → the Bass arrangement**; guitar — and any unknown/future instrument (drums, keys) — falls through to the existing preference/most-notes default, which already lands on a guitar part. Previously the instrument selector only fed the tuner, so a bass player was handed a guitar chart (and a tune/coverage check then compared a 4-string bass against a 6-string part). An **explicit arrangement request always wins** (a manual arrangement switch is untouched), and a bass player's saved default-arrangement preference is still honored **within** the bass parts (so a preferred `Bass 2` / `Alt. Bass` wins over the canonical Bass), so this only changes the *default* part chosen on load. Server-only — every launch path already flows through the WS, so there's no client change. This is the instrument↔chart-routing piece the working-tuning series leans on (otherwise coverage compares across instruments). Tests: `tests/test_highway_ws_instrument_routing.py` (bass→Bass, bass-honors-pref, bass-no-bass-part→guitar, guitar→default, explicit-wins).
- **Host "working tuning" — a live, app-wide record of what tuning your instrument is *actually* in right now (foundation; no behavior change yet).** Introduces `window.feedBack.workingTuning`, a host-owned, session-lived state distinct from any one song's tuning and from a soft opt-in default: the offsets + string-count + reference pitch the player's instrument is currently in, plus an `assumed`/`verified` provenance flag. It's **per-instrument** — your guitar's current tuning and your bass's are kept *separately* (keyed like the instrument selector, e.g. `guitar-6` / `bass-4`), so switching instruments surfaces that instrument's own remembered tuning and you only ever deal with the one you've selected. It exists so a retune — or an instrument swap mid-session — is reflected **everywhere** (the highway, the library/song-picker, and plugins like the tuner, Virtuoso, and the minigames) instead of being re-derived per surface or wrongly assumed from a fixed profile. Modeled on the shipped `tuning` capability + the `feedBack.theme` read-API: a **synchronous `get(instrument?)`** (returns the selected instrument's state, defaulting to the seed until known), a `set(state, {provenance, instrument})` mutator (the tuner becomes the sole writer in the next change), `setCurrentInstrument()` for the selector, `resetToDefault()`, and a `working-tuning-changed` event that fires on every change **and once on hydration** (carrying which instrument changed) so a late-mounting consumer is never stuck on stale state. State is **in-memory, seeded from `/api/settings` on boot and reset on restart** — a stale "you're in drop-A" assumption is worse than re-asking. Registered as a separate `working-tuning` **exclusive-owner** capability (tuner = writer, the rest = requesters). This is the foundation (plumbing only — nothing writes to it yet) of the working-tuning series, which fixes the tuner gate only ever prompting *away from* a fixed "home" tuning (never back) and makes the current tuning a first-class signal the whole app shares. Offsets use the same per-string semitone vocabulary as song tunings, so fully custom/extended tunings (e.g. a drop-A 8-string) are first-class. Frontend-only: new `static/capabilities/working-tuning.js`, loaded from `static/index.html` + `static/v3/index.html`.
@@ -52,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- **Tuner auto-open is now opt-in and persists instead of flashing open-then-shut.** When you entered a song (or switched arrangement) whose tuning differed from the last, the tuner auto-opened and — for some testers — vanished ~1s later (reported macOS+Windows, 0.3.0). Root cause: the tuner closes itself on `song:play` (`plugins/tuner/utils/ui.js` — you don't tune while playing), so a **song switch** fired autoplay → `song:play` → the just-auto-opened tuner closed; an **arrangement switch** (which never arms autoplay) had no `song:play`, so it stayed open — exactly why two testers saw opposite behaviour (it wasn't the mic). Now: (1) the feature is a **new opt-in setting** ("Auto-open on tuning change", in the tuner's Settings panel, persisted as `autoOpenOnTuningChange`, **default OFF**); (2) an **auto**-opened tuner *persists* — it ignores the autoplay `song:play`, stray outside-clicks, and same-screen re-emits, closing only via the new in-panel **`×`** / **"Skip"** buttons or when you leave the song. A *manually* opened tuner keeps its classic click-away / play-to-close behaviour. The panel previously had no in-box close at all; this adds one (`×` + contextual Skip). All in the tuner plugin (`routes.py` config, `screen.js` gate + persist, `utils/ui.js` buttons + `song:play` guard, `settings.html` toggle) — **no core `app.js` changes**. Tests: `tests/js/tuner_auto_open.test.js` (opt-in gate, `{ auto: true }` persist mode, play/click-proofing). **Default (opt-in vs opt-out) is teed up for Byron to decide — flip one boolean.**
- **Tuner auto-open is now tuning-coverage-aware — extended-range players aren't nagged for songs their instrument already covers.** With the opt-in auto-open on, it now prompts only when your **current physical tuning** (from your instrument selection in Settings) 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 your instrument: an **8-string F♯-standard** player gets **no** prompt for a 6- or 7-string standard song (its top strings already match those tunings), while a song needing an open string you don't have (e.g. a **Drop-A 7-string**, whose low A isn't an open string on an F♯ 8-string) **still** prompts. A whole-instrument reference difference also prompts — A440 vs A432, or an octave-down `centOffset` (which the auto-open now accounts for; it was previously ignored). The player's instrument is read from core **`/api/settings`** (the v3 instrument selector — a stable physical reference, not the tuner's song-tracking selection); when nothing's declared or the lookup is unavailable it falls back to a conservative prompt, so a real retune is never silently skipped. **v3-only** (the instrument selector is v3). All in the tuner plugin (`plugins/tuner/screen.js`) — **no core changes**. Tests: `tests/js/tuner_auto_open.test.js` (covered vs uncovered, the Drop-A case, reference-pitch mismatch, contiguous alignment). _Follow-up (E1.6): a passive "different tuning" badge cue that names the string(s) to retune, plus the splitscreen / no-usable-input guards._
- **The tuner badge now passively flags when a song needs a different tuning — and 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 and a tooltip that **names the change** — e.g. *"retune B→A"* for a Drop-A song on an F♯ 8-string, or *"the reference pitch"* for an A440-vs-A432 mismatch. It's purely **advisory** (it never auto-opens the panel — tap the badge to tune), recomputed on `song:ready` and cleared when a new song loads or you leave the player. The retune diff comes from the tuner plugin's coverage report (`window._tunerAutoOpen.coverageReport``{ covered, retune: [{ from, to }], reference, cantCover }`); the cue is CSS-free (an inline ring + native tooltip — no Tailwind rebuild) and no-ops when the tuner plugin isn't installed. **v3-only.** Touches `static/v3/badges.js` (the cue) + `plugins/tuner/screen.js` (the report). Tests: `tests/js/tuner_auto_open.test.js` (the report names the strings; reference mismatch; the badge wiring). _(The splitscreen-suppress and no-usable-input guards move to the playback-gate stage, where they matter for its no-trap rule.)_
- **v3 player: opening another rail popover now closes the Section Practice popover (no more two stacked popovers).** Opening the **Practice** pill's popover and then clicking a different player-rail icon (e.g. **Plugins**) left the Practice popover open underneath the new one — looked broken (reported on macOS, 0.3.0 / 2026-06-28). The rail icons call `e.stopPropagation()` in their click handler (`static/v3/player-chrome.js`), which killed bubbling before it reached the Practice popover's outside-click dismiss bound on `document`. The dismiss (`_installSectionPracticeDismiss` in `static/app.js`) now binds in the **capture phase**, which runs before the target's handler so a descendant's `stopPropagation()` can't swallow it — mirroring how the audio-mixer popover already dismisses. Esc handling stays bubble-phase (the player's Escape-to-exit ordering is unchanged). v2 shares `app.js` and is only hardened (no rail `stopPropagation` there). Tests: `tests/js/section_practice_dismiss.test.js`.
- **v3 UI no longer lets you accidentally text-select the chrome.** Dragging or double-clicking across the interface used to marquee-highlight buttons, labels, the sidebar, the transport, and the note-highway HUD — which looks broken (reported on Mac + Windows). The v3 shell now defaults to `user-select: none` on `html` (`static/v3/v3.css`), then opts *content* back in — so chrome is non-selectable but the text you actually copy still works. Decided by a 4-lens panel (UX / accessibility / dev-ops / plugin-ecosystem); the guardrails are deliberate: **form fields are always re-enabled** (never break the caret / IME — no `* { user-select:none }`, which trips a WebKit input bug); **plugin screens (`.screen[id^="plugin-"]`) stay selectable by default** so a plugin's copyable text (lyrics, chord names, results) — including community plugins that don't know about this — isn't silently locked; and **core read-only content opts back in by container** via a new hand-authored **`.fb-selectable`** class — applied to the whole **Settings** panel (paths, device names, version, diagnostics, About — answering "is settings still copyable?": yes), the **now-playing song metadata** (with `pointer-events` re-enabled so the HUD text is actually reachable), and the focused **modals / dialogs / toasts / scan banner** that carry copyable errors, IDs, paths, and file names. It's cosmetic only (it protects nothing) and never used to lock copy-worthy text — errors, IDs, paths, versions, and metadata stay selectable per WCAG 2.2 (copy-paste as a permitted mechanism). Dense card lists (library grid, dashboard, profile) stay non-selectable by design — making them selectable would reintroduce the marquee-mess across cards. **v3-only** (v2 unchanged); plain CSS, no Tailwind rebuild; no desktop changes (standard OS-framed window). Plugin authors: `.fb-selectable` is documented in `CLAUDE.md` for re-enabling copyable content rendered outside a plugin screen. Tests: `tests/js/v3_user_select_policy.test.js`.
- **Input-setup wizard no longer collapses an audio device's driver-type variants into one entry.** On Windows the desktop engine enumerates the same interface once per host API (ASIO / Windows Audio / DirectSound), and the wizard's audio picker (`plugins/input_setup/screen.js`) de-duped the source list by display **label** — so the variants (which share a name) collapsed to a single choice, silently keeping whichever sorted first (often *not* the low-latency ASIO one the player wants). The audio-input capability already collapses true duplicates by `logicalSourceKey` (`_visibleInputSources` in `static/capabilities/audio-session.js`), and the variants each have a **distinct** key, so the wizard's extra label-collapse was redundant for real dupes and destructive for these — it also could drop the variant that was actually `selected`. Removed it; the picker now lists every selectable input. Pairs with feedBack-desktop's change to label each source with its driver type (e.g. "Focusrite (ASIO)") so the now-distinct entries are legible.