mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 12:21:49 +00:00
main
36 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Matthew Harris Glover
|
270cb39f41
|
Enable Linux nightly AppImage auto-update in the System settings UI (#999)
Some checks are pending
ship-ci / ci (push) Waiting to run
* fix(settings): enable Linux nightly AppImage auto-update in System settings
Fixes the Settings → System "App updates" panel so it actually works on
Linux, and adds Nightly as a selectable channel — previously missing
entirely, so Linux self-update couldn't be reached from this UI at all.
- The channel dropdown no longer gets permanently disabled the moment
the desktop bridge reports 'unsupported', which is the normal state
whenever the channel isn't Nightly on Linux. It stays enabled so the
user can switch to Nightly, the only way out of that state.
- Shows live download progress ("Downloading update… N%") and an
explicit button state machine (Check → grayed out while busy →
Restart now once staged), instead of a frozen "Checking…" during the
~1.5GB background download.
- Renders every status update from the triggering action's own return
value (checkNow()/setChannel()'s result) rather than a separate
follow-up getStatus() call, which can race against other state
changes and show a stale result even after a real success.
- setupAppUpdates() no longer re-syncs the channel to the backend on
every Settings-panel re-render — only once per page load — so a
redundant sync can no longer stomp an in-flight download's state.
- Routes update-flow events into the existing diagnostics.js
console-capture + contribute() snapshot API, so the user's existing
"Export Diagnostics" button now captures the full update decision
trace end to end — no new UI or log file. This diagnostic tracing is
what actually root-caused the bugs above, from real on-device
captures rather than guesswork.
Companion PR in feedBack-desktop (the underlying update engine).
Verified end-to-end on a Steam Deck: channel switch → check → live
download progress → restart button → relaunch onto the new build,
confirmed via a real Export Diagnostics capture showing a clean,
fully-accounted-for trace.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(settings): extract + unit-test the app-update status view; dedupe diag log
- Extract the status→UI state machine from renderFrom into a pure, exported
_appUpdateStatusView() (DOM-free) and cover it with tests/js — settings.js's
large module graph made importing it for a full harness impractical, so the
pure function is the testable seam. Behavior-preserving; renderFrom applies
the returned shape to the DOM exactly as before.
- Dedupe the [update-diag] renderFrom console line so the ~1.5s download poll
no longer floods the diagnostics ring buffer with byte-identical entries;
every real state/percent change still logs, and the structured contribute()
snapshot stays unconditional.
Left the 'audio_engine' diagnostics key as-is: the server export filters
client contributions to loaded plugin ids (diagnostics_bundle.py path-traversal
guard), so a dedicated key would be silently dropped from the bundle.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
---------
Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
|
||
|
Joe Hallahan
|
05be9ebdbe
|
Add new chart-transform plugin capability (#1000)
* Chart-transform plugin capability * PR comments * Cleanup * Fix markdown * CodeRabbit feedback Signed-off-by: Joe <jphinspace@gmail.com> --------- Signed-off-by: Joe <jphinspace@gmail.com> Co-authored-by: Byron Gamatos <xasiklas@gmail.com> |
||
|
ChrisBeWithYou
|
cc75cb876a
|
fix(library): tuning filter answers for your instrument, not always guitar (#1003)
* fix(library): tuning filter answers for your instrument, not always guitar The library indexed exactly one tuning per song, chosen guitar-first (lead > rhythm > combo, bass only as a last resort), and nothing consulted the player's instrument. A bassist filtering by tuning was shown the guitar chart's tuning, so playlists built by tuning contained songs needing a retune. Reported by a tester building bass practice sets; Covet "Shibuya" is the clean case, with a custom guitar tuning over a standard bass chart. Indexes each arrangement role's own tuning and makes the facet, filter, sort and labels answer for one perspective. `guitar-lead` reads the original unprefixed columns and adds no payload keys, so the default response is unchanged. The same defect existed inside guitar -- lead and rhythm charts can disagree -- so perspective is three-valued (guitar-lead, guitar-rhythm, bass) driven by one PERSPECTIVES table rather than parallel column families. Songs with no chart for the perspective fall back to the song-level tuning rather than vanishing (18 of 59 packs in the test library have no bass chart), but the fallback is marked inferred in the facet counts and on the row instead of being silently coalesced. "Only real charts" reuses the existing `arrangements_has` filter rather than adding one. Bass-specific handling, from measured content: - Bass tuning arrays are padded to six entries; charts never reference string index 4 or 5. Truncated to four before naming and grouping. - Grouping uses a canonical open-pitch key, so [-2,0,0,0] and [-2,0,0,0,0,0] are one facet row instead of two. - Offsets above +1 semitone are refused a name. Bassists tune down, near never up; one pack ships [5,5,5,5,4,4] (A-D-G-C, unplayable, and its own notes sit in the song's real key under standard tuning). Naming that would send a player to retune to a tuning that does not exist. Rhythm deliberately does not truncate -- padding is a bass finding, and cutting a seven-string array would invent a tuning the chart lacks. Adds an opt-in `tuning_match=playable` mode alongside exact match: a chart is offered when your lowest open pitch is at or below its lowest open pitch, so a five-string bass covers four-string standard and drop-D with no retune. Open strings only -- note range is not indexed and the scan stays manifest-only -- so it fails conservative: unknown low pitch is excluded, and the upper bound is unchecked and documented rather than guessed. Existing installs would otherwise never populate: the tree-signature fast path reports "unchanged" forever on a settled library. Rows with NULL marker columns re-extract, and the fast path is disabled until that backfill converges (writes use '' rather than NULL, so it self-clears). 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> * test(v3): accept the tuning-perspective indirection in the badge guard The album-art badge now reads shownTuningName(), so the source-pattern guard no longer matched the inline `tuning_name || tuning` form and CI went red. Accept the helper, and pin the helper's own fallback in a companion test so the guard still fails if a guitar player's tuning label is ever dropped. Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> --------- Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
e14ef64224
|
fix(playback): the song queue must survive a playSong wrapper that drops options (#977)
Some checks are pending
ship-ci / ci (push) Waiting to run
Tester: "Passports does not advance in the song queue." The play queue tells playSong "don't clear the queue I'm driving" by passing options.fromQueue. But window.playSong is wrapped by a CHAIN of plugins — nam_tone, midi_amp, fretboard, invert_highway, tabview — and each wrapper forwards only (filename, arrangement), silently dropping the options object. So fromQueue never reached playSong: it cleared the queue the instant its first song started, and a gig/album/playlist never advanced. Reproduced on the real build via a queue.start + a hooked clear(): the queue went inactive with 0 remaining immediately after start, and the clear stack ran through nam_tone -> midi_amp -> invert_highway -> fretboard -> session.js. Fixing six plugin wrappers is whack-a-mole and the next plugin re-breaks it. Fix it at the source instead: the queue raises an out-of-band flag (_consumeInternalPlay, one-shot) beside the wrapper chain, not through it, and playSong's clear-guard honours it. options.fromQueue stays as the in-band path. The flag is consumed on read so a later MANUAL play still abandons the queue. Verified on the real build: the gig queue stays active after start and advances on song:ended (Iron Maiden -> Blind Guardian), and a manual play still clears. Tests drive the real clear-guard against the queue for: a dropped-options wrapper (the bug), the one-shot manual-play-still-clears invariant, and the in-band fromQueue path on its own. All 3 fail on the pre-fix source. JS 1211/1211. |
||
|
Byron Gamatos
|
6272af8d33
|
feat(career): profile passport wall, home career card, shareable PNG card (#955)
* feat(career): profile passport wall, home career card, shareable PNG card Career v3, WS2. The identity artifact leaves the plugin tab: - Profile: #v3-profile-passports-mount (core, one div) filled by career on v3:profile-rendered — per-instrument shelves of earned covers, hours, gig count, open-career link. Absent-not-empty. - Home: the plugin-count stat tile becomes #v3-dash-career-slot with the old stat as fallback content; career replaces it with a trading-card tile (leather + foil shine, badge count, hours, closest-stamp ask) on the existing v3:dashboard-rendered event. - Shareable card: static/js/blob-io.js (downloadBlob lifts the idiom duplicated verbatim in settings-io/diagnostics-export — both refactored; copyImageBlob wraps ClipboardItem, returns false to signal the download fallback). Earned passports get Save/Copy card: a natively-drawn 480×640 canvas (leather, stamp ring, stubs+hours line); copy falls back to download with a notice when the clipboard refuses. - Mount-point convention documented in docs/plugin-v3-ui.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(career): external surfaces stay absent until a passport exists CodeRabbit on #955: a bare commitment produced a zero-passport wall and replaced the dashboard fallback. Docs also now say mounts may hold fallback content and plugins REPLACE, never append. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
gionnibgud
|
f8012a8ce4 |
feat(v3): add desktop-only "Start in fullscreen" system option
Adds a "Start in fullscreen" toggle to the Settings → System panel,
addressing the desktop request in feedBack-desktop#97: users want the
app to launch fullscreen without hitting the OS hotkey every time.
The block ships hidden and is gated exactly like the App-updates block:
setupWindowOptions() only unhides + wires it when the feedBack-desktop
bridge exposes window.feedBackDesktop.window.{getStartFullscreen,
setStartFullscreen}. Web/Docker builds have no such bridge, so the
section never appears there. Persistence lives desktop-side because
only the Electron main process can read the pref at window-creation
time — core just proxies through the bridge.
The desktop bridge + launch behaviour land in a follow-up
feedBack-desktop PR.
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
|
||
|
Byron Gamatos
|
8d3db5f42c
|
fix(nav): nobody may monkey-patch window.showScreen — add screen:changing, make the shell listen (#924) (#925)
Some checks are pending
ship-ci / ci (push) Waiting to run
* fix(nav): the library sometimes showed the legacy screen — map 'home' inside showScreen
Testers: "randomly, when moving to the library from another menu option, the library shows the
old interface — never when a song ends."
━━━ WHAT WAS ACTUALLY HAPPENING ━━━
#home is the PRE-V3 library screen. The v3 shell replaced it with #v3-songs, and the mapping DID
exist — but only inside WRAPPERS on window.showScreen, and only for callers that go through
`window`. THREE independent parties monkey-patch it, each capturing whatever happens to be there
at the time:
app.js publishes the raw function
-> shell.js wraps it, adding the home -> v3-songs mapping
-> the stems plugin wraps it AGAIN (src/main.js:1029), capturing the current value
Plugins load ASYNCHRONOUSLY. The chain links up in whatever order the race settles, and any
capture taken before shell.js installs — or any re-assignment after it — silently drops the
mapping. Hence "randomly".
AND THE INTERNAL CALLERS NEVER TOUCHED window.showScreen AT ALL. closeCurrentSong and the
Esc-from-settings shortcut call the IMPORTED showScreen, which no wrapper ever sees. Reproduced
in a browser: the unwrapped function with 'home' lands on the dead legacy screen EVERY time.
"Never when a song ends" is the tell, and it is what identified the mechanism: closeCurrentSong
resolves its target through _resolvePlayerOrigin(), which ALREADY applies this mapping. That one
path was fine — which is exactly why the bug looked random rather than total.
PRE-EXISTING, not a regression from the module carve: the onclick="showScreen('home')" links and
the wrapper-only mapping both date to 2026-06-22.
━━━ THE FIX ━━━
The guard lives inside showScreen now: ONE place, in the function every caller routes through,
instead of a chain of monkey-patches that must each remember. Wrapper order stops mattering, and
the module-internal callers are covered for the first time.
Verified in a browser: the raw, unwrapped showScreen('home') — which reproduced as #home — now
lands on #v3-songs, and cannot be undone by any wrapper order.
━━━ AND A [P1] I INTRODUCED, WHICH CODEX CAUGHT ━━━
My first cut mapped BOTH 'home' and 'v3-home', copied straight from _resolvePlayerOrigin.
That is correct THERE and wrong HERE. _resolvePlayerOrigin computes where to RETURN TO after a
song, and landing on the Songs list from the dashboard is the right behaviour. But #v3-home is
the v3 DASHBOARD — a real screen that the shell's Home nav, the onboarding tour and the dashboard
re-render listener all target. Redirecting it would have made Home unreachable.
A LEGACY ALIAS IS NOT THE SAME THING AS A RETURN TARGET. Only 'home' is mapped now, and a test
pins that: re-adding 'v3-home' to the guard fails it.
4 tests, bite-tested both ways.
node 1049, pytest 2425, ESLint 0, Codex 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(nav): nobody may monkey-patch window.showScreen — add screen:changing, make the shell listen (#924)
window.showScreen was wrapped by THREE independent parties, each capturing whatever happened to be
there at the time:
app.js publishes the raw function
-> static/v3/shell.js wrapped it (to call syncActive, and to map home -> v3-songs)
-> the stems plugin wrapped it AGAIN (to tear down on leaving the player)
Plugins load ASYNCHRONOUSLY, so the chain linked up in whatever order the race settled. A capture
taken before shell.js installed silently dropped the mapping it carried — and the library opened on
the dead legacy #home screen. Testers saw that as "randomly, the library shows the old interface"
(#923).
#923 fixed the symptom by moving the mapping inside showScreen. This removes the CAUSE: neither
wrapper ever needed to be one.
━━━ TWO EVENTS, AND THE DISTINCTION IS THE WHOLE POINT ━━━
screen:changing emitted BEFORE anything happens. "I am leaving `from`." Teardown/cancel here.
screen:changed emitted after the DOM and data settle. "I am on `id`." Now carries `from`.
screen:changing is new, and it exists because Codex caught me collapsing the two. The stems plugin
tore down its audio graph BEFORE showScreen did anything; screen:changed fires at the very END,
after core awaits library and provider loads — so moving the plugin onto it would have delayed
teardown behind a slow fetch, or skipped it entirely if that fetch threw, and stems would keep
playing on a non-player screen. A test pins the ordering: screen:changing must precede the first
await.
shell.js is a plain screen:changed listener now, like app.js, audio-mixer.js and tour-engine.js
already were. window.showScreen is an unwrapped function again, and tests/js/
no_showscreen_monkeypatch.test.js fails CI if anything in static/ ever assigns to it again — so the
hazard is structurally impossible rather than merely avoided.
━━━ AND A FALLBACK THAT COULD NEVER FIRE ━━━
My retry-if-the-bus-is-late path listened for `slopsmith:capabilities:ready`. Core dispatches
`feedBack:capabilities:ready` (capabilities.js:1536) — the slopsmith: name is the PRE-DMCA event
and nothing has emitted it since the rename. Codex caught it. A guard that cannot fire is worse
than no guard: it reads as protection and is decoration.
(The same dead-event bug turned out to be sitting in THREE of the stems plugin's fallbacks, where
it has silently disabled its lifecycle wiring whenever the bus was late. Fixed in
feedback-plugin-stems#38.)
VERIFIED. A/B against origin/main: the nav highlight and topbar title follow IDENTICALLY with
shell.js as a listener; screen:changing -> screen:changed fire in order with the right {id, from};
window.showScreen is unwrapped; and showScreen('home') still lands on v3-songs.
node 1053, pytest 2425, ESLint 0, Codex 0.
Closes #924
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
f27d4f623c
|
fix(nav): the library sometimes showed the legacy screen — map 'home' inside showScreen (#923)
Testers: "randomly, when moving to the library from another menu option, the library shows the
old interface — never when a song ends."
━━━ WHAT WAS ACTUALLY HAPPENING ━━━
#home is the PRE-V3 library screen. The v3 shell replaced it with #v3-songs, and the mapping DID
exist — but only inside WRAPPERS on window.showScreen, and only for callers that go through
`window`. THREE independent parties monkey-patch it, each capturing whatever happens to be there
at the time:
app.js publishes the raw function
-> shell.js wraps it, adding the home -> v3-songs mapping
-> the stems plugin wraps it AGAIN (src/main.js:1029), capturing the current value
Plugins load ASYNCHRONOUSLY. The chain links up in whatever order the race settles, and any
capture taken before shell.js installs — or any re-assignment after it — silently drops the
mapping. Hence "randomly".
AND THE INTERNAL CALLERS NEVER TOUCHED window.showScreen AT ALL. closeCurrentSong and the
Esc-from-settings shortcut call the IMPORTED showScreen, which no wrapper ever sees. Reproduced
in a browser: the unwrapped function with 'home' lands on the dead legacy screen EVERY time.
"Never when a song ends" is the tell, and it is what identified the mechanism: closeCurrentSong
resolves its target through _resolvePlayerOrigin(), which ALREADY applies this mapping. That one
path was fine — which is exactly why the bug looked random rather than total.
PRE-EXISTING, not a regression from the module carve: the onclick="showScreen('home')" links and
the wrapper-only mapping both date to 2026-06-22.
━━━ THE FIX ━━━
The guard lives inside showScreen now: ONE place, in the function every caller routes through,
instead of a chain of monkey-patches that must each remember. Wrapper order stops mattering, and
the module-internal callers are covered for the first time.
Verified in a browser: the raw, unwrapped showScreen('home') — which reproduced as #home — now
lands on #v3-songs, and cannot be undone by any wrapper order.
━━━ AND A [P1] I INTRODUCED, WHICH CODEX CAUGHT ━━━
My first cut mapped BOTH 'home' and 'v3-home', copied straight from _resolvePlayerOrigin.
That is correct THERE and wrong HERE. _resolvePlayerOrigin computes where to RETURN TO after a
song, and landing on the Songs list from the dashboard is the right behaviour. But #v3-home is
the v3 DASHBOARD — a real screen that the shell's Home nav, the onboarding tour and the dashboard
re-render listener all target. Redirecting it would have made Home unreachable.
A LEGACY ALIAS IS NOT THE SAME THING AS A RETURN TARGET. Only 'home' is mapped now, and a test
pins that: re-adding 'v3-home' to the guard fails it.
4 tests, bite-tested both ways.
node 1049, pytest 2425, ESLint 0, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
57e7db5c2a
|
refactor(app): carve the keyboard-shortcuts subsystem into static/js/shortcuts.js (R3d) (#922)
19 declarations + 23 TOP-LEVEL STATEMENTS. 922 lines. app.js 3,243 -> 2,325 (-28%). The panel registry, both global keydown dispatchers, the library arrow-nav, and the whole plugin-facing shortcut API. ━━━ MOST OF THIS SUBSYSTEM WAS NOT DECLARATIONS ━━━ A declaration-seeded dependency closure reports this cluster as 10 names, 246 lines. It is 42 statements and 922. window.registerShortcut, createShortcutPanel, getAllShortcuts, unregisterShortcut, clearWindowShortcuts, the panel registry, and BOTH global keydown dispatchers are bare TOP-LEVEL STATEMENTS at app.js's top level. A call-graph scan sees NONE of them. That blind spot has now cost three times: * it nearly shipped a dead library A-Z rail (#896) — 43 of library.js's exports were referenced only from app.js's window contract; * it threw "Assignment to constant variable" in the session carve (#921), where the autoplay gate's top-level statements wrote state that had just become a read-only import; * and here it under-reported the slice by 3x. The extractor takes them by construction now — any top-level statement that TOUCHES a moved binding comes along — and the SEED is closed to a FIXED POINT, because those statements have their own dependencies (_modifiersMatch, _isShortcutActive, _handleLibArrowNav, _gridColumns…) that the declaration closure never walked. Seed -> pull the statements -> the statements need more names -> re-seed. Iterate until it stops growing. ━━━ syncLibrarySong GOES ACROSS THE SEAM, NOT THROUGH AN IMPORT ━━━ The library arrow-nav calls it on Enter. It cannot be imported: syncLibrarySong reaches showScreen/playSong, and a module importing app.js closes a cycle. It is the ONE name here that had to stay behind, so it comes across the host seam — which is exactly what the seam is for. host.js throws loudly if the wiring is ever dropped, and tests/js/host_contract.test.js fails in CI if the hook drifts. VERIFIED. A/B against origin/main in two browsers, IDENTICAL, zero page errors — and driven for real, not merely present: the plugin API (register / unregister / getAll / panels), THE GLOBAL KEYDOWN DISPATCHER actually firing a registered shortcut, that same shortcut correctly SUPPRESSED while typing in a text input, and `?` opening the help modal. node 1045, pytest 2425, ESLint 0 (no-cycle clean), host contract 2/2, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
545e569ad6
|
refactor(app): carve the song session out of app.js — playSong, showScreen, closeCurrentSong (R3d) (#921)
36 declarations + the 4 autoplay/auto-exit gate statements. 359 lines. app.js 3,772 -> 3,242. Bodies VERBATIM. ━━━ THIS WAS "THE UNCUTTABLE HEART", AND IT IS 359 LINES ━━━ At the start of this epic, seeding a dependency closure from count-in, from loops, from section-practice, or from the JUCE seek shim all returned the SAME 178-function, 3,360-line set. playSong and showScreen called each other; everything called them; nothing could be cut anywhere. The conclusion — correct at the time — was that NO closure-based carve could touch it at any seed, and the answer was a host seam. That was true THEN. Every slice taken out since (transport, loops, count-in, section-practice, the library, the edit modal, settings) removed edges, and the strongly-connected component DISSOLVED. This closure is 36 declarations with an interface width of FOUR. The lesson is not that the seam was wrong — the seam is what MADE this possible, by letting the carves proceed against a cyclic core instead of stalling on it. The lesson is to RE-MEASURE. An SCC is a fact about a graph at a moment, not a property of the code. ━━━ THE BUG NO SCAN COULD SEE, AND THE A/B DID ━━━ First cut passed every gate — no-undef clean, no-cycle clean, 1045/1045, pytest green — and THREW IN THE BROWSER: "Assignment to constant variable." window.feedBack.holdAutoplay / holdAutoExit and their two event handlers are TOP-LEVEL STATEMENTS, not declarations. They WRITE this cluster's state (_autoplayHeld, _autoExitTimer, …), and an imported binding is READ-ONLY — so left behind in app.js, every one threw the instant the module existed. A dependency scan that walks DECLARATIONS cannot see them. Mine didn't. This is the same blind spot that nearly shipped a dead library A-Z rail (#896): app.js keeps its public API in top-level statements, and a call-graph is blind to every one of them. The extractor now finds them by construction — any top-level statement that WRITES a moved binding comes with the carve — and the gate statements live beside the machinery they drive, which is where they belonged anyway. ━━━ ZERO OUTSIDE WRITES, BY MOVING THE BOUNDARY RATHER THAN BUILDING MACHINERY ━━━ The autoplay scalars and the wake-lock state were written from outside the cluster, which would have forced a setter or a state container. But the writers — _releaseAutoplay, _acquireWakeLock — plainly belong here. Pulling them in left ZERO outside writes, so every export is a plain import. Same move as settings (#920): measure the writers before you reach for a container. VERIFIED. A/B against origin/main in two browsers, IDENTICAL, zero page errors — including the autoplay gate driven end to end: a plugin HOLDS autoplay, the song loads but does not start, the RELEASE fires it, and a stale release is a no-op. That is the exact machinery that was throwing. node 1045, pytest 2425, ESLint 0 (no-cycle clean), host contract 2/2, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
84fe29688c
|
refactor(app): carve settings into static/js/settings.js (R3d) (#920)
22 declarations, 446 lines. app.js 4,218 -> 3,772. Bodies VERBATIM. Settings load/save, the AV-offset nudge, the default-arrangement pin, the instrument pathway, and the app-update channel. ━━━ INTERFACE WIDTH 1, AND IT GOT THERE BY DRAWING THE BOUNDARY IN THE RIGHT PLACE ━━━ app.js calls loadSettings() and nothing else. The first cut was NOT clean: _defaultArrangement was written from OUTSIDE the cluster, and an imported binding is READ-ONLY, so that one write would have forced a setter or a state container — as it did for the player (player-state.js) and the library (library-state.js). But the writers were saveSettings and pinCurrentArrangementDefault, which ARE settings functions. Widening the slice to include them left ZERO outside writes. Every export is now a plain read-only import and no container is needed. Worth naming, because I reached for a container twice before: the fix for "this binding is written from outside" is sometimes a container, and sometimes it just means the boundary is in the wrong place. Measure the writers before you build machinery. ━━━ handleSliderInput STAYS A HOST HOOK, DELIBERATELY ━━━ It lives in settings now (it is a settings control), but player-controls.js must NOT import it: this module already imports player-controls (_applyMastery, _autoplayExitEnabled, …), so a direct back-import would close a cycle. player-controls keeps reading it through the host seam, and app.js — the root, which imports both — wires it. That is exactly what the seam is for, and the contract test proves the wiring survived. VERIFIED. A/B against origin/main in two browsers: the window contract, the settings screen rendering, the AV-offset and default-arrangement controls present, and a real `input` event dispatched on a slider — which is the path that goes through the host seam. IDENTICAL, zero page errors. node 1045, pytest 2425, ESLint 0 (no-cycle clean), host contract 2/2, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
69aac32278
|
refactor(app): carve the edit-song modal into static/js/edit-modal.js (R3d) (#919)
4 functions, 234 lines. app.js 4,452 -> 4,218. Bodies VERBATIM. INTERFACE WIDTH ZERO — nothing in app.js calls into this cluster. app.js needs only the names on the window contract, so the markup's onclick= handlers resolve. That is what makes it the cleanest slice left. AND IT ONLY BECAME CLEAN BECAUSE THE LIBRARY CAME OUT FIRST (#896). Every dependency the modal has is a module now: it reads six bindings out of ./library.js (loadLibrary, loadFavorites, loadTreeView, _removeLibCardsForFilename, libView, _lastLibSelected) plus dom.js and the L container. Before that carve, extracting this would have dragged the whole library with it. Checked, and it matters: the modal never WRITES any of those six. An imported binding is READ-ONLY, so a single write would have forced a setter or a state container. Every use is a read, so plain imports suffice. Acyclic: edit-modal -> { dom, library-state, library }, and library imports none of them back. VERIFIED. A/B against origin/main in two browsers: the window contract, the modal actually OPENING off a real library row, its title and year fields rendering, and the data-edit-save wiring (rather than an inline onclick embedding the filename — the fix this cluster's harness exists to guard). IDENTICAL, no new page errors. node 1045, pytest 2425, ESLint 0 (no-cycle clean), host contract 2/2, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
36cf77dc44
|
refactor(highway): carve the 2D drawing layer into highway-draw.js (R3c) (#917)
18 functions, 1,245 lines. highway.js 3,972 -> 2,727 (-31%). The biggest R3c slice: notes,
sustains, chords, strum groups, unison bends and lyrics — everything the default renderer
paints each frame.
━━━ MUTABILITY, NOT LOCATION, DECIDES WHERE A THING BELONGS ━━━
Three per-instance caches came out with this slice, and they are why it needed care:
_frameMismatchWarned a warn-once Set of chord ids (feedBack#88)
_chordRenderInfo a WeakMap of chord -> chain info
_lyricMeasureCache Map<fontSize, Map<text, width>>
All three are MUTATED. Left at module scope they would be SHARED ACROSS PANELS — one
highway's lyric widths and chord chains stomping another's, silently, with nothing throwing.
createHighway() is a factory (the constitution publishes window.createHighway so a plugin can
build a second highway), so they are lifted onto hwState, which is exactly what hwState is for.
The shimmer LUT went the OTHER way — to MODULE scope in highway-geometry.js. It is a
deterministic xorshift table, byte-for-byte identical for every instance, so sharing it is not
merely safe but BETTER: built once for the page rather than once per panel.
Same slice, opposite directions, decided entirely by whether the thing mutates.
━━━ MY SCRIPT WAS WRONG TWICE. THE GATES CAUGHT BOTH. ━━━
1. HAND-LISTED THE MOVE SET. I listed 10 functions and missed six that drawChords needs
(_ensureChordRenderCache, bsearchChords, getChordTemplateInfo, _computeChordBox,
_updateFretLinePreview, _drawFretLineChordPreview). The no-undef gate named every one. The
set is now DERIVED from the dependency closure — 18, not 10.
2. JUDGED PURITY TOO EARLY, and this one is subtle. I classified _computeChordBox as pure
because its ORIGINAL body never mentions hwState. Then the call-site rewriter injected
`fretX(hwState, …)` INTO it — fretX takes hwState now (#916) — leaving a function that
references an hwState it was never given. Purity has to be judged from the body AS IT WILL
BE, so the classifier iterates to a fixed point: a function needs hwState if it mentions it,
OR calls anything that now takes it. That moved _computeChordBox to the stateful side.
VERIFIED. A/B against origin/main: IDENTICAL, zero page errors. The PLUGIN BUNDLE contract is
byte-identical (b.fretX arity 3, b.getNoteState arity 2, both stable references, both correct
under the old calling convention). PERF GATE PASSES AT 1.92ms against its 12ms budget — and
this is the slice that could really have cost something: the ENTIRE per-frame drawing path is
now cross-module. It costs nothing measurable.
TESTS. highway_teaching_marks follows strumGroupBuckets to the new module. The two source-shape
harnesses now read highway.js AND every static/js/highway-*.js, rather than being re-pinned at
whichever file currently holds a function — re-pinning breaks again next time, and a shape
assertion that silently stops finding its target is indistinguishable from one that passes.
node 1045, pytest 2416, ESLint 0, no-undef 0, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
12eb73aee9
|
refactor(highway): carve the STATEFUL primitives, threading hwState explicitly (R3c) (#916)
fretX, fillTextReadable, _noteState, _paintGemGlow -> static/js/highway-state-primitives.js.
50 call sites rewritten. highway.js 4,105 -> 3,965.
The first slice that changes signatures. Each of these four gains hwState as an explicit
FIRST PARAMETER.
━━━ hwState IS A PARAMETER, NOT AN IMPORT ━━━
createHighway() is a FACTORY. The constitution publishes window.createHighway so a plugin can
build a SECOND highway for its own panel, and highway.js says so itself. Import hwState as a
module singleton and two panels silently share one clock, one render scale, one string
palette — each driving the other. Nothing throws. The picture is just wrong, in a way no test
would catch.
(The exact opposite of the app.js carve, where player-state.js and library-state.js ARE
module singletons — correctly, because there is exactly one app. Same epic, same language,
opposite answer, decided entirely by whether the thing is a factory.)
━━━ THE PLUGIN BUNDLE NEARLY BROKE, SILENTLY ━━━
The renderer bundle hands two of these STRAIGHT TO PLUGINS:
b.fretX = fretX;
b.getNoteState = _noteState; // stable reference
highway_3d calls both EVERY FRAME, with the old arity. Handing out the new 3-arg versions
would have passed `note` where hwState belongs — no throw, no error, just wrong geometry and
wrong judgment state INSIDE A PLUGIN, which no core test would ever see. Green CI, broken 3D
highway.
So hwState is bound ONCE per instance, in the factory, and the bundle hands out those views.
A per-frame arrow would have fixed the arity and reintroduced exactly the per-frame allocation
the bundle's stable-reference contract (feedBack#254) exists to prevent. b.project needs none
of this — project() is pure and its arity never changed.
VERIFIED IN A BROWSER, against the real bundle, on both builds:
fretX arity 3 3 (NOT 4 — the bound view preserves it)
getNoteState arity 2 2
fretX(5,1,800) in 0..800 True True
getNoteState null w/o provider True True
getNoteState honours provider True True
fretX is a stable reference True True
IDENTICAL. Without the bound views fretX would have reported arity 4 and computed garbage.
Also caught on the way: my generated module imported STRING_BRIGHT_FALLBACK, a name
highway-constants.js does not export. ESLint does not flag that — but importing a name a
module does not export is a runtime SyntaxError that kills the WHOLE module. These four need
no constants at all; the import is gone.
TESTS. highway_note_state pins the signature AND the stable-reference contract — it caught the
bundle break. Retargeted at the module and the new arity; both contracts still asserted, and
the "no fresh arrow per frame" rule is now asserted explicitly rather than implied by
`getNoteState: _noteState`.
PERF GATE PASSES AT 1.94ms against its 12ms budget — fretX and _noteState are now CROSS-MODULE
calls, per note, per frame. It costs nothing measurable.
node 1045, pytest 2416, ESLint 0, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
1a386c272d
|
refactor(highway): carve the PURE geometry primitives into highway-geometry.js (R3c) (#915)
6 functions, 53 lines. highway.js 4,158 -> 4,105. NOT ONE CALL SITE CHANGES. project, roundRect, bnvNormalizedPoints, teachingFingerLabel, teachingDegreeLabel, chordHarmonyLabels — the shared primitives every drawing function leans on. ━━━ PURITY IS THE WHOLE POINT OF THIS SLICE ━━━ Every one of these is a pure function of its arguments. None touches hwState. None closes over the canvas context — roundRect() already took `ctx` explicitly, and the rest need nothing but numbers. project() reads only the module-level constants from #914. That matters because createHighway() is a FACTORY: a plugin can build a second highway for its own panel, so anything holding per-instance state must be PASSED hwState rather than importing it, or two panels silently share one clock and palette. These six hold no state at all, so they move VERBATIM — the module boundary is invisible to every caller. The asserts are mechanical and in the extractor: it REFUSES to move a function whose body mentions hwState, or that references `ctx` without taking it as a parameter. Purity is checked, not assumed. ━━━ WHAT IS DELIBERATELY LEFT BEHIND ━━━ The four primitives that DO need hwState — fretX, fillTextReadable, _noteState, _paintGemGlow — stay in the factory for now. They need an explicit hwState parameter threaded through 53 call sites, which is a real behavioural change and belongs in its own commit rather than smuggled in beside a provably-identical move. Separating the provable from the risky is the whole discipline of this epic. TESTS. Three harnesses brace-match these functions out of the source and run them in a sandbox; they now read static/js/highway-geometry.js. `export function x` still contains `function x`, so the extractor needed no change — only the path. VERIFIED. A/B against origin/main: 15 probes IDENTICAL, zero page errors. PERF GATE PASSES AT 1.91ms against its 12ms budget — and this is the one that could plausibly have cost something: project() runs for every visible note on every frame and is now a CROSS-MODULE call. It costs nothing measurable. That is the answer #910 was built to give. node 1045, pytest 2416, ESLint 0, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
8e89b39ad3
|
refactor(highway): carve the constants into static/js/highway-constants.js (R3c) (#914)
29 constants, 190 lines. highway.js 4,267 -> 4,158. The first real slice, and the one that
every later one imports.
━━━ WHY ONLY THE CONSTANTS MAY LIVE AT MODULE SCOPE ━━━
createHighway() is a FACTORY, not a singleton. The constitution publishes
window.createHighway precisely so a plugin can build a SECOND highway for its own panel, and
highway.js already says so at the top of the closure:
// R3c: per-instance mutable state in one object, so extracted renderer/ws
// modules can close over it as a factory arg without cross-panel sharing.
So hwState — all 79 mutable properties — must NEVER become a module-level singleton: two
highways would silently share it, and one panel would drive the other's clock, scale and
colour tables. Extracted functions will take it as an ARGUMENT.
That is the OPPOSITE of the app.js carve, where a single state container (player-state.js,
library-state.js) was exactly right, because there is exactly one app. Same epic, same
language, opposite answer — because one is a singleton and the other is a factory.
These 29 are pure literals: numbers, strings and colour tables, never reassigned, never
mutated. Sharing them across instances is not merely safe, it is what you want — one copy of
the shimmer LUT bounds and the string palettes rather than one per panel. Anything with a
runtime dependency (document, window, performance, localStorage) stays in the factory;
checked, and none of these has one.
ESLint now knows static/highway.js is a module. It could not have known before this commit:
the flip (#913) changed the SCRIPT TAG, but the file had no import/export yet, so it still
parsed as a script and lint stayed green. The first `import` is what makes the config wrong.
TESTS. Four source-shape harnesses asserted `const _AUTO_SCALE_MIN = …` etc. lived in
highway.js. They now read highway.js AND every static/js/highway-*.js — deliberately, rather
than being re-pinned at whichever file currently holds a constant. Re-pinning just breaks
again on the next carve, and a source-shape assertion that silently stops finding its target
is indistinguishable from one that passes. Bite-tested: renaming two constants away fails
them.
VERIFIED. A/B against origin/main: 15 probes IDENTICAL, zero page errors. AND THE PERF GATE
PASSES AT 1.97ms against its 12ms budget — which is the point of having built it (#910)
first: these constants moved from closure scope to module scope, and V8 does not treat those
identically. It does here. Now I know rather than hope.
node 1045, pytest 2416, ESLint 0, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
d9fa6d3f55
|
refactor(highway): make the highway global explicit before the module flip (R3c) (#912)
73 bare `highway.x` references -> `window.highway.x`, across app.js and 10 other files. Provably a NO-OP today. It is the precondition for flipping highway.js to a module. ━━━ WHY THIS HAS TO LAND FIRST ━━━ highway.js is a CLASSIC script. Its top-level `const highway = createHighway()` therefore creates a GLOBAL LEXICAL BINDING — visible as a bare name to every other classic script AND to every ES module. 73 call sites quietly rely on that. The moment highway.js becomes a module, that binding is gone. `const` in a module is module-scoped, not global. Every one of those 73 sites becomes a ReferenceError, and the flip is impossible until they say what they mean. `window.highway = highway` is already set, to the same object, on the same line. So this is an identity rewrite — verified in the browser below. ━━━ THE REWRITE BIT ME THREE TIMES. REGEX IS NOT ENOUGH FOR THIS. ━━━ 1. A SHADOWED LOCAL. capabilities/note-detection.js does `const highway = window.highway`. Its 9 bare uses are LOCAL and already correct; a blind rewrite would have emitted `const window.highway = window.highway`. Excluded. 2. HALF-CONVERTED GUARDS — the dangerous one. Six sites read `typeof highway !== 'undefined' && highway && typeof highway.setTime === 'function'`. The regex converted the CONSEQUENT and left the TEST, which is WORSE than not touching them: after the flip `typeof highway` is 'undefined', so each guard is PERMANENTLY FALSE and the code behind it silently never runs. transport.js's was the seek->setTime sync: the chart clock would have quietly desynced after every seek, with nothing failing. All six now test window.highway. 3. TWO MORE BARE REFERENCES, found by Codex [P2] and confirmed by an AST scan: app.js:3114 and :3176 use `highway && typeof window.highway.getSections === 'function'`. My grep searched for `typeof highway`, not `highway &&`. After the flip these throw, the catch swallows it, and the editor silently falls back to a ±4s edit window and arrangement 0. Regex missed a shadow, a half-conversion, and two bare reads. The final check is an AST pass that resolves scopes and reports every `highway` identifier not bound locally. It now reports ZERO. VERIFIED. A/B against origin/main in two browsers, 15 probes, IDENTICAL, zero page errors: window.highway is the same object as the bare global, the whole API surface resolves, a real song plays, the chart clock advances, getPerf().drawMs > 0 — and `seek syncs chart` passes, which is the exact guard I nearly broke in (2). node 1045, pytest 2416, ESLint 0, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
OmikronApex
|
34215fbd32 |
fix(static): request raw stereo audio for the loopback capture track
Chromium treats a getDisplayMedia audio track as a voice call by default: echo cancellation, noise suppression, auto gain control and mono downmix. Music through that pipeline is the tester-reported "tin can" sound on ASIO/exclusive outputs. Request the raw path explicitly (EC/NS/AGC off, stereo, 48 kHz) — all constraints are best-effort so unsupported ones degrade silently instead of failing the capture. The [asio-diag] loopback line now dumps track.getSettings() so logs prove which processing actually applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Byron Gamatos
|
756588678b
|
fix(plugins): make a module plugin actually re-evaluate on reload (#879) (#897)
A plugin reload silently did nothing for scriptType:"module" plugins. ES modules are evaluated ONCE PER URL PER DOCUMENT, so re-inserting a <script type="module"> whose src the module map has already seen fires `load` without re-running the body — and the loader then recorded the reload as applied. A no-op that reported success. THE ISSUE UNDERSTATES IT. #879 says "upgrades are fine — a new version yields a new URL". That is true of screen.js and FALSE of the plugin. I drove a real browser through install(1.0.0) -> upgrade(1.1.0) -> rollback(1.0.0), counting evaluations of src/main.js: ONE. Not three, not two. The upgrade re-runs the one-line screen.js shim at its new ?v= URL; the shim does `import './src/main.js'`; a relative specifier resolves against the base URL WITH THE QUERY DROPPED; that is the same URL as before; the module map hands back the already-evaluated v1.0.0 module. The plugin's own code never re-ran. Busting the entry point cannot fix this, whatever token you hang off it. So the token goes in the PATH: /api/plugins/<id>/g/<n>/screen.js. From there './src/main.js' resolves to /api/plugins/<id>/g/<n>/src/main.js — every relative import inherits it, at every depth, for free. No import-specifier rewriting (which could never see `import(expr)` anyway). Same browser drive after the fix: THREE evaluations. Keyed on the plugin ID, not id@version: EVERY re-load of a module plugin needs a fresh path, not just a rollback. First load keeps the stable ?v= URL, so the ETag/304 live-edit caching the R0 rails depend on is untouched. Classic-script plugins are not affected and never take a /g/ path. ━━━ A PATH REWRITE, NOT TWO MIRRORED ROUTES ━━━ Codex caught this, and it was right. The token shifts the BASE URL, so EVERYTHING the module graph resolves relatively moves with it — not only imports. `new URL('../assets/worklet.js', import.meta.url)` from /api/plugins/x/g/1/src/main.js resolves to /api/plugins/x/g/1/assets/worklet.js. Mirroring only screen.js and src/ would have fixed imports and 404'd every asset, worklet and wasm file the graph reaches — and would have broken again the next time someone added a plugin route. So the /g/<token> segment is STRIPPED BEFORE ROUTING. Every plugin route, present and future, works under the prefix with no extra wiring. The token is opaque and never joined into a filesystem path, so containment still rests entirely on the same safe_join. Codex then caught a [P3] in that: eagerly re-encoding raw_path with latin-1 raises UnicodeEncodeError on a valid plugin file like src/工具.js, 500ing a request the plain route serves fine. raw_path is informational and Starlette routes on scope["path"], so the mutation is simply gone — and leaving raw_path as the client sent it is more truthful for logs anyway. TESTS. tests/js/plugin_module_rollback.test.js (5) + 8 in test_plugin_src_route.py: identical bytes under the prefix, the whole graph one and two levels deep, ASSETS (the Codex [P2]), every plugin route, non-ASCII filenames (the [P3]), an opaque token, and containment asserted as PARITY with the un-prefixed route rather than a guessed 404 — `../screen.js` legitimately 200s on both, because the URL normalises before routing. All bite-tested: reverting the fix fails the rollback tests, disabling the rewrite fails the asset tests. Two harnesses re-anchored on `script.src = _pluginScriptUrl(` — the URL literal they keyed on now lives in the helper, further down the file, so their slice ran off the end. node 1045, pytest 2404, ESLint 0, Codex 0. Closes #879 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
bd830328f0
|
refactor(app): carve the library out of app.js (R3a) (#896)
static/js/library.js (1,988) + static/js/library-state.js (29) — bodies VERBATIM.
app.js 6,313 -> 4,451.
THE BIGGEST SLICE OF THE CARVE: 145 declarations, ~1,900 lines, 30% of what was left.
The grid, the artist tree, the A-Z rail, filters, pagination, selection, favourites, the
scan banner, and the library-provider plumbing.
A LOW module: it imports only leaves (./dom.js, ./format.js, ./library-state.js,
./tuning-display.js — all four import nothing themselves) and needs ZERO host hooks. It
calls nothing in app.js. That is not luck; it is why this cluster was picked. Two entry
points that WOULD have dragged the playback core in were left behind in app.js:
* syncLibrarySong reaches showScreen/playSong
* _handleLibArrowNav Enter on a selected row plays the song
Both are one hop from the library, and app.js is the root, so it imports from both sides
for free. Pulling them in swallows playSong, showScreen and the whole remaining core — I
measured it: the closure jumps from 145 declarations to 189.
library-state.js holds exactly FIVE fields. An imported binding is read-only, and of the
library's outward bindings only these five are genuinely WRITTEN from outside — by
showScreen, deleteSongFromModal and syncLibrarySong, none of which can move in. The other
23 are read-only from outside, so they stay plain exports (ES live bindings mean app.js
still sees every reassignment).
━━━ THE EXPORT LIST NEARLY SHIPPED A DEAD A-Z RAIL ━━━
59 exports — and 43 of them CANNOT be found by a call-graph scan. They are referenced only
from app.js's TOP-LEVEL statements: the Object.assign(window, {...}) contract and the
scattered window.X = X lines, which live outside every function, so a closure walk over
declarations never sees them. Among them are the four handler names app.js composes AT
RUNTIME into onclick="" strings — filterTreeLetter, filterFavTreeLetter, goTreePage,
goFavTreePage — the library A-Z rail and its pagination. No static tool can see those at
all. Had I trusted the call-graph, the rail would have died silently on click with nothing
failing in CI.
━━━ AND MY OWN SCANNER LIED ━━━
The cycle-risk pass reported "(none)" for this carve. It was wrong, and it could not have
been right: a dangling `else if` bound to an inner `if` instead of the outer chain, so its
`imported` map was ALWAYS empty and the check reported clean no matter what. A guard that
cannot fail is worse than no guard. Fixed, and it then found the real edges — dom.js,
format.js, tuning-display.js, library-state.js. All four are leaves, so the carve is
genuinely acyclic; I just now know it instead of assuming it.
(The AST rewriter had its own trap: `MAP[name]` with an object literal and name ===
'constructor' hits Object.prototype.constructor — truthy — and it happily rewrote
`constructor(id)` into `L.function Object() { [native code] }(id)`. Every identifier in
the file is looked up, so the lookup must not see the prototype chain. It is a Map now.)
TESTS. legacy_shim_hits SPLIT (loadLibraryProviders + setLibraryProvider -> the module;
syncLibrarySong stayed in app.js). v3_library_refresh now reads app.js AND the module,
rather than being re-pinned to whichever file happens to hold the emit this week.
VERIFIED. A/B against origin/main in two browsers: the whole window contract, cards render,
grid/tree/sort/filter/clear round-trip — and, specifically, the A-Z rail: 28 onclick
handlers composed at runtime, identical on both, and a real .click() on a letter works.
IDENTICAL on all 33 + 7 probes, no new page errors.
pytest 2396, node 1040/1040, host contract 2/2, ESLint 0 (no-cycle clean).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
09f7e450a5
|
refactor(app): give formatTime a home — a leaf format.js, one fewer host hook (R3a) (#895)
static/js/format.js (17). One function. Retires the formatTime hook: 12 -> 11. WHY A MODULE FOR ONE FUNCTION. formatTime was a host hook — loops.js and section-practice.js both reached back through the seam for it. It is ALSO, by pure accident of who calls it, inside the dependency closure of the library carve that comes next. Leaving it there would have made loops.js and section-practice.js import the LIBRARY in order to format a timestamp — nonsense, and a cycle waiting to happen. Same rule as the transport carve: a hook is a cycle you agreed to live with; an import is a dependency you actually have. formatTime has a real owner. It just isn't app.js, and it certainly isn't the library. Give it a home and both consumers import it directly. A leaf on purpose. Anything else that turns out to be a shared pure formatter belongs here too; nothing does yet (I checked — formatBadge, _safeImageUrl and _fetchJsonOrThrow have no callers outside the library), so nothing else is here. node 1040/1040, host contract 2/2, ESLint 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
8bec8d2466
|
refactor(app): carve the playback transport out of app.js — and RETIRE 8 host hooks (R3a) (#894)
static/js/transport.js (377) — bodies VERBATIM. app.js 6,643 → 6,316.
THIS IS THE FIRST CARVE THAT SUBTRACTS HOOKS INSTEAD OF ADDING THEM.
Every carve before this one added host hooks: a module pulled out of app.js still had
to call back into it. But four modules were all reaching through the seam for the SAME
handful of names — _audioSeek, _audioTime, setPlayButtonState, _songEventPayload,
jucePlayer. Those names have an owner, and it isn't app.js. Give them one, and the
consumers import them directly:
count-in.js 5 hooks -> 0 (host import deleted)
juce-audio.js 4 hooks -> 0 (host import deleted)
loops.js 6 hooks -> 4
section-practice.js 10 hooks -> 7
----------------------------------------------------------
configureHost() 20 hooks -> 12
A hook is a cycle you agreed to live with. An import is a dependency you actually have.
Prefer the import whenever the name has a real owner.
_audioSeekGen now stays PRIVATE. It has exactly one writer — _resetAudioSeekState(),
which moved with it — so readers get audioSeekGen() and nobody outside can desync it.
Strictly better than the hook it replaces, which handed out a getter and left the writer
behind in app.js.
THE SCAN HAD A HOLE, AND IT BIT. Picking the carve by dependency closure over app.js's
own top-level decls said this cluster was downward-closed. It wasn't:
_currentPlaybackSnapshot reads loopA/loopB — which live in ./js/loops.js, and loops.js
imports transport. The scan saw nothing, because loopA STOPPED BEING an app.js decl the
moment loops.js was carved out. Any dependency scan of a partly-carved monolith has to
resolve the imports too, or it will confidently hand you a cycle. Added that pass; it
found exactly one back-edge, and _currentPlaybackSnapshot stays in app.js (as does
restartCurrentSong, which calls _cancelCountIn). app.js is the root — it imports both
sides for free.
TESTS. Four harnesses retargeted (play_button_reroute_guard, song_event_payload,
song_seek -> transport.js; playback_app_adapter SPLIT, since
_installPlaybackTransportAdapter stayed behind).
The two CENSUS tests — "≥8 song:* emit sites", "every seek callsite passes a reason" —
now scan app.js AND every static/js/*.js, not one file. Pointed at a single file, their
count silently shrinks as code leaves, which reads as "someone deleted an emit" or, worse,
passes while genuinely missing sites. Both bite-tested: stripping a _songEventPayload()
from an emit and adding a reason-less _audioSeek() each fail the suite.
VERIFIED. A/B against origin/main, real song, real playback: song:play payload is exactly
{audioT, chartT, perfNow, time}; song:seek carries reason "seek-by" with finite from/to;
all five song:* events fire; seekBy advances the clock; restartCurrentSong returns to zero;
the play button's aria-pressed tracks state. IDENTICAL on all 21 probes, zero page errors.
pytest 2396, node 1040/1040, host contract 2/2, ESLint 0 (no-cycle clean), Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
8d0e270345
|
refactor(app): carve the JUCE/desktop audio shims out of app.js (R3a) (#893)
* refactor(app): carve resume-session out of app.js (R3a)
static/js/resume-session.js (157) — the snapshot taken when you leave a song and the
pill that offers it back. Bodies VERBATIM. app.js 7,727 → 7,601.
Fifth slice out of the strongly-connected core. ONE hook (playSong) + a
currentFilename getter.
S.pendingResume JOINS THE CONTAINER — on demand, exactly as intended. app.js WRITES
it (playSong({ resume }) arms it; the song:ready listener consumes it) while this
module reads it, so it cannot be a plain export: an imported binding is read-only.
Same reason isPlaying is there. The container grows one field per carve that needs
it, never speculatively.
THE CONTRACT TEST CAUGHT THE MISSING HOOK, again on a path nothing executes:
"playSong is read by a module but never wired by app.js — it would throw at runtime".
Second time it has caught a real wiring gap the moment it appeared.
A REAL TRAP, worth remembering: I first did the S.pendingResume rewrite by feeding
acorn's identifier RANGES from node into python, and it corrupted the file
(`_pS.pendingResume null;`). **Acorn's offsets are UTF-16 code units; Python's string
indices are code points.** static/app.js contains emoji, so every offset past one
drifts. Do an AST-driven rewrite in the SAME language that produced the offsets.
`node --check` caught it; a silent version of that bug is very easy to imagine.
VERIFIED. A/B against origin/main in two browsers, real song: the window API
(resumeLastSession / _snapshotResumeSession / _readResumeSession /
_clearResumeSession), snapshot, read-back, and clear — IDENTICAL, zero page errors.
HONEST LIMIT: my probe never got the snapshot to actually PERSIST (there is a guard
beyond the 3s minimum position that a scripted playSong does not satisfy), so that
path is verified only as identical-to-main, not as observed-working. The real
coverage is tests/browser/resume-session.spec.ts, which drives the flow properly.
Zero harnesses broke. pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean),
tailwind clean, Codex 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(app): carve the JUCE/desktop audio shims out of app.js (R3a)
static/js/juce-audio.js (994) — bodies VERBATIM. app.js 7,603 → 6,643.
THE LARGEST SINGLE SLICE of the whole carve phase: 960 lines, ~13% of what was left.
Three self-installing IIFEs:
_installJuceEngineRoutingWatcher (444) routes a song to the JUCE engine or HTML5 as
the desktop output enters/leaves exclusive/ASIO
_installRendererBusFeeder (337) feeds the highway renderer bus from whichever
transport is actually running
_installJuceAudioElementShim (156) patches audio.play/pause so the rest of the app
keeps talking to the <audio> element while JUCE
owns the transport
They EXPORT NOTHING — all three publish through `window.*` (_juceMode,
_reevaluateJuceRouting, _reevaluateRendererBus, …). So app.js needs only a
side-effect import plus the one binding it actually uses
(_resetJuceAudioShimChain, which the shim IIFE assigns).
THE ORDERING QUESTION, CHECKED RATHER THAN ASSUMED. Importing this module runs the
IIFEs EARLIER than before: imports evaluate ahead of app.js's body, and therefore
ahead of configureHost(). A hook read at IIFE-execution time would THROW. So I walked
the AST at IIFE-body depth to see what they actually touch when they run: nothing but
listener registration, and `audio.play`/`audio.pause` patching — and `audio` is itself
an imported module now. Verified in the browser: both are patched on the carved build
exactly as on main, which proves the shim installs correctly at its new, earlier point.
(Had I got this wrong, host.js throws loudly rather than silently misbehaving — which
is the whole reason it has no no-op defaults.)
VERIFIED. A/B against origin/main in two browsers: the entire window.* surface the
IIFEs publish (_juceMode, _juceOutputIsExclusive, _reevaluateJuceRouting,
_reevaluateRendererBus, _clearJuceRerouteMemo), audio.play/pause patched, a real song
loading and togglePlay driving the public mirror — IDENTICAL, zero page errors.
Harnesses: juce_engine_reroute (19 tests) + renderer_bus_feeder (13) slice the IIFEs by
signature — retargeted, and each sandbox gains a `host` object routed at its EXISTING
stubs so every assertion holds unchanged. test_plugin_runtime_idempotence is SPLIT: 3 of
its 4 source-asserts stayed in app.js, the sm.emit('song:resume') one moved.
pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean, Codex 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
dc429ecd16
|
refactor(app): carve the player controls out of app.js (R3a) (#891)
static/js/player-controls.js (229) — the speed + mastery sliders and the four
playback-preference reads (autoplay-exit, up-next, countdown-before-song,
confirm-exit). Bodies VERBATIM. app.js 7,914 → 7,727.
The fourth slice out of the strongly-connected core, and by far the easiest: ONE
hook (handleSliderInput) and NO shared mutable state. The three groups are the same
surface — the controls under the highway — and the preference reads are the
one-line localStorage lookups half of app.js consults before deciding whether to
auto-start, show the Up Next pill, run a count-in, or confirm on exit. They travel
with the controls that set them.
Zero missed members on the first build (the no-undef pass was clean), which is the
first time that has happened in this phase.
TWO HARNESSES ARE SPLIT, and both taught something:
* speed_reset spans BOTH files — playSong (app.js) resets the speed controls
(module). Its presence GUARDS still read `src.includes('function setSpeed')`
against app.js, so once the code moved they silently evaluated FALSE and the
helpers were quietly dropped from the sandbox. A guard that disables itself is
worse than no guard. Repointed at the file the code actually lives in.
* Its `host.handleSliderInput` stub had to route at the sandbox's EXISTING spy,
not a fresh `() => {}`. The test asserts the slider was actually refreshed
(`deepEqual(__sliderInputs, ['speed-slider'])`); a fresh stub swallows the call
and the assertion passes VACUOUSLY. Same failure mode as a no-op host default —
the thing this whole seam design exists to prevent.
* autoplay_exit is split too: _autoplayExitEnabled moved, but the auto-exit
machinery around it (_clearAutoExit, holdAutoExit, _resolvePlayerOrigin) stayed.
VERIFIED. A/B against origin/main in two browsers, real song: setSpeed(0.75) ->
playbackRate 0.75; applySpeedPreset(100) -> 1; the speed slider; setMastery;
setAutoplayExit / setCountdownBeforeSong / setShowUpNext — IDENTICAL, zero page errors.
pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
11f8c36b61
|
refactor(app): carve count-in (and the song-credits overlay) out of app.js (R3a) (#890)
static/js/count-in.js (389) — bodies VERBATIM. app.js 8,223 → 7,913. The third slice out of the strongly-connected core, and the first that WRITES shared state rather than only reading it. #889's container is what makes it possible. imports: loops (setLoop/loopA/loopB — a count-in inside an A-B loop must begin at A), audio-el, player-state, host hooks : _audioSeek, setPlayButtonState, _songEventPayload, togglePlay + a jucePlayer getter Nothing imports count-in back — app.js and section-practice both reach it through the seam — so the graph stays acyclic. app.js's autoplay path used to reach IN and set this module's credits timers itself (_creditsTimer, _creditsHideOnPlay) and read _countingIn. It cannot now, and should not have to, so the module exports the OPERATIONS instead — armCreditsHideOnPlay(), scheduleCreditsHide(), holdCreditsThen(start), isCountingIn() — and owns its own timer invariants. Third time this has happened (section-practice's resetSelection, loops' state) and each time the constraint produced better code than was there before: the module keeps its own promises instead of trusting a caller 6,000 lines away to zero the right fields. THE no-undef GATE FOUND FIVE MISSED MEMBERS, one at a time: showSongCreditsOverlay and startSongCountIn (my name regex matched startCountIn, not startSongCountIn), then _creditLineLabel, _CREDITS_MAX_MS, and _CREDIT_ROLE_VERBS. A call-graph closure does not see a const table; only the undefined-symbol pass does. AND A REAL TRAP: I computed _CREDIT_ROLE_VERBS's span against the ALREADY-MODIFIED app.js and applied it to the clean one — the line numbers had drifted, so the slice would have cut somewhere else entirely. Recomputed every span from the clean file with acorn. Never carry line numbers across an edit. VERIFIED. A/B against origin/main in two browsers with a real song: playback state, the public feedBack.isPlaying mirror, audio position, cancel-count-in — IDENTICAL, zero page errors. Unit coverage moved with the code: loop_restart's count-in cancellation-token test and the 5 song_credits_overlay tests now read count-in.js; loop_restart's sandbox gains a `host` object routed at its EXISTING stubs, so every assertion is unchanged. HONEST LIMIT: I could not make the count-in OVERLAY actually render headlessly — its autoplay path needs a fresh-load _pendingAutostart that a scripted playSong() never arms. Behaviour is identical to main on every probe and the unit tests cover the logic, but the on-screen 1-2-3-4 and the credits card want a human look. pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
5fb28d5c5a
|
refactor(app): lift the shared player state onto a container (R3a) (#889)
static/js/player-state.js — one exported object, two fields. app.js's 70 reference
sites rewritten. Provably a no-op; nothing shrinks.
WHY NOW. Every slice carved out of app.js so far only ever READ the state it shared
(loopA/loopB, _audioSeekGen, currentFilename), so a read-only getter hook was enough
and no container was needed — twice I checked and twice I got away with it. That
runs out at count-in: it genuinely WRITES `isPlaying` (it starts and stops playback,
4 sites) and `lastAudioTime` (2). `import { isPlaying }` then `isPlaying = true`
THROWS — an imported binding cannot be assigned to. So the state has to live on an
object: `S.isPlaying = true` is a property write, and works from any module holding
the same S. Same shape stems, studio, and editor all converged on.
DELIBERATELY SMALL. app.js has ~104 top-level `let` scalars; lifting all of them is
a ~977-site rewrite for no benefit, because most are private to one cluster and
travel with it. Only what a carved module must WRITE goes here. Add on demand.
THE REWRITE IS AST-DRIVEN, NOT TEXTUAL — and that is not fussiness. Of 100 textual
occurrences of these two names, only 70 resolve to the module binding:
* 22 are member accesses (`someObj.isPlaying`, `window.feedBack.isPlaying`)
* 4 are the LOCAL PARAMETER of `function setPlayButtonState(isPlaying)` — a blind
replace yields `function setPlayButtonState(S.isPlaying)`
* 1 is an object key
* 2 are shorthand properties `{ isPlaying }`, which must become
`{ isPlaying: S.isPlaying }` — and acorn gives a shorthand's key and value the
SAME range, so rewriting both produced `isPlaying: S.isPlaying: S.isPlaying`
until I deduped by range
A find-and-replace corrupts all 29. The rewrite walks the AST, skips shadows, member
properties and keys, and replaces identifier RANGES.
`window.feedBack.isPlaying` — the PUBLIC mirror — is a different thing and is
untouched. Two test sandboxes stub it; those were left alone deliberately.
VERIFIED WITH REAL PLAYBACK. A/B against origin/main in two browsers, real song:
togglePlay -> the public mirror goes true -> false -> true across two toggles, the
audio element's paused state follows, seekBy works — IDENTICAL, zero page errors.
Harnesses: 8 vm-sandbox suites slice playback code out of app.js and now see
S.isPlaying — juce_engine_reroute, loop_restart, play_button_reroute_guard,
playback_app_adapter, song_restart, song_seek, speed_reset, and the python
idempotence source-assert. Each gets the same container in its sandbox; every
assertion is unchanged.
pytest 2396, node 1040/1040, ESLint 0, tailwind clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
cb236e6c04
|
refactor(app): carve the A–B loop out of app.js (R3a) (#888)
static/js/loops.js (261) — bodies VERBATIM. app.js 8,421 → 8,224.
The second slice out of the strongly-connected core.
It OWNS the loop state — loopA, loopB, _loopMutationGen. Nothing outside writes
them: restartCurrentSong() looked like it did, but it declares its own local `let
loopA/loopB` shadows, so the module-level bindings only ever change in setLoop /
setLoopStart / setLoopEnd / clearLoop. All four move here. No state container.
DIRECTION IS THE WHOLE DESIGN. loops and section-practice are mutually dependent —
the SCC in miniature. clearLoop() must drop section-practice's selection, and
practiceSection() must call setLoop(). Both edges cannot be imports or no-cycle
(rightly) rejects it. So:
section-practice -> reaches loops through the HOST SEAM (host.setLoop, …)
loops -> imports section-practice DIRECTLY
section-practice is the higher-level feature — a consumer of loops, not the reverse
— so it is the one that takes the indirection. app.js hands the loop module's
exports across into the seam for it. Graph stays acyclic; no-cycle passes.
THE CONTRACT TEST EARNED ITS KEEP IMMEDIATELY. It failed on the first build with
"these hooks are wired by app.js but no module reads them: playSong". My dependency
scan had counted a mention of playSong() inside a COMMENT in loops.js as a real
call. Wired but unused is precisely the "fossil of a rename" case the test exists
for — and it caught it on a path no test executes.
VERIFIED BY DRIVING BOTH SIDES OF THE SEAM. A/B against origin/main in two browsers,
real song loaded:
* setLoop(5,12) -> true; getLoop() -> 5,12 — IDENTICAL
* clearLoop() (loops -> section-practice, a direct import) -> getLoop() ->
null,null — IDENTICAL
* onPhraseNext() (section-practice -> loops, ACROSS THE SEAM) -> ok — IDENTICAL
* loadSavedLoop / saveCurrentLoop / deleteSelectedLoop on window — IDENTICAL
* zero page errors either side. An unwired hook throws, so a live app is itself
proof the seam is wired.
Harness: loop_api extracts the loop helpers by signature — retargeted to loops.js,
`export` stripped for the vm sandbox, and the sandbox's existing _audioSeek /
_audioTime / formatTime spies are now routed through a `host` object so every
assertion holds unchanged, just through the indirection the real code uses. It is
SPLIT: one test still reads app.js for the window.feedBack API surface, which stayed.
pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
64f04565e2
|
refactor(app): the host seam + carve section practice out of app.js (R3a) (#887)
static/js/host.js (99) + static/js/section-practice.js (1,214).
app.js 9,461 → 8,409.
THE FIRST SLICE OUT OF THE STRONGLY-CONNECTED CORE. What is left in app.js is not
a tree, it is a cycle: seeding a dependency closure from section-practice, from
loops, from count-in, or from the JUCE seek shim all return the SAME 178-function
set, and setLoop() and practiceSection() call each other directly. No closure-based
carve can cut it at any seed. So it is cut BY NAME, and the calls back into app.js
go through a host seam.
61 functions + its own 24 _sectionPractice*/_sectionParents* scalars (read nowhere
else) move out. 11 hooks come back in. 4 of those are read-only GETTERS —
loopA/loopB/_audioSeekGen/_loopMutationGen are only ever READ here, never written,
so app.js keeps owning them and NO state container is needed (a 977-site lift
avoided).
app.js used to reach IN and reset the module's state by hand (clearLoop() zeroed the
selection; changeArrangement() invalidated the parent count). It cannot now — an
imported binding is read-only — so those are exported as resetSelection() and
invalidateParentCount(). Strictly better: the module owns its own invariants instead
of trusting two callers on the far side of the file to zero the right three fields.
═══ THE SILENT-NO-OP PROBLEM, SOLVED ═══
The obvious host seam is an object of no-op defaults. That is a TRAP and we walked
into it once: the plugin loader's seam defaulted populateVizPicker to `() => {}`, so
a dropped wiring line would have left the viz picker quietly not refreshing with NO
test, boot check, or bot noticing. Two layers stop it here:
1. RUNTIME — host.js is a Proxy with NO defaults and NO stubs. Reading an unwired
hook THROWS. An unwired hook cannot degrade into a no-op because there is
nothing to degrade INTO. configureHost() also rejects a non-function at WIRE
time, and refuses to run twice.
2. STATIC — tests/js/host_contract.test.js asserts the hooks the modules USE are
exactly the hooks app.js WIRES. This is the layer that matters: a runtime throw
only fires if the broken path executes, and the whole danger of a seam is the
paths that never run in a smoke test. VERIFIED TO BITE in all three drift
directions: drop a hook from configureHost -> fails; rename host.setLoop in the
module -> fails; wire a hook nobody uses -> fails.
Writing that guard took three tries and each failure is instructive: (a) the
configureHost regex anchored `});` at column 0, ran past the indented close, and
swallowed app.js's 66-name window contract — 77 "hooks"; (b) an import-stripping
regex with `[\s\S]*?` ate 14,000 characters INCLUDING the drift the bite test was
meant to catch — a guard with a hole is worse than no guard, because you trust it;
(c) `host.js'` in the import path backtracked from `js` to a "hook" called `j`.
The bite tests are what surfaced all three.
CODEX FOUND A REAL RACE [P2]. configureHost() was inside the async boot function,
after several awaits — but the window handlers (onPhraseNext, …) go live during
app.js's SYNCHRONOUS module evaluation. A user clicking one in that window would hit
"[host] … was read before configureHost() ran". It is now a bare top-level statement
sitting immediately before the window contract, so the seam is always wired before a
handler can be reached. Verified live: invoking a handler 1.2s in — well before the
boot awaits settle — works.
VERIFIED. A/B against origin/main in two browsers with a REAL song loaded: popover
toggle, practice-mode change, phrase-next, and clearLoop (all of which cross the seam
— setLoop/clearLoop/_audioTime/loopA/loopB) — IDENTICAL, zero page errors. Since an
unwired hook throws, a live app is itself proof the seam is wired.
Harnesses: section_practice_dismiss retargeted; loop_api's clearLoop sandbox gains a
resetSelection SPY (not a stub) and ASSERTS it fires — the guarantee is still tested,
just through the seam.
pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
f53d566dbc
|
refactor(app): give the <audio> element a module of its own (R3a) (#886)
static/js/audio-el.js — one exported const. app.js's diff is 5 lines.
This is a HINGE, not a carve: nothing shrinks, but almost everything left in
app.js is blocked behind it.
WHY. `audio` is `document.getElementById('audio')` with 162 references in app.js
and 173 outside any one cluster. Every remaining cluster measured — settings,
app-updates, count-in (208 fns), exit-confirm (212), library-render (220) — lists
`audio` among its inbound symbols, because they all touch playback and playback
reaches for the element directly. A module that needs it cannot import app.js to
get it (that closes a cycle and fails import-x/no-cycle), so today the only way to
carve any of them would be a host seam — the exact thing #878 had to build and
#880 had to tear out.
WHY IT'S SAFE. `audio` is a `const` and is NEVER reassigned anywhere in core, so a
read-only import binding is exactly right and no state container is needed. The
162 call sites are untouched — the binding keeps its name, it is just imported
instead of declared. (Contrast the reassigned scalars — isPlaying, _avOffsetMs —
which CANNOT be shared this way: an imported binding cannot be written to. Those
still need containers, and that is the next problem, not this one.)
TIMING. app.js is <script type="module">, so it evaluates after the HTML is parsed
and its imports evaluate just before its body — the same moment app.js used to run
this exact lookup. If the element had not been in the document, `audio` would be
null and app.js's top-level `audio.addEventListener(...)` calls would throw and
kill the module. They don't.
VERIFIED WITH REAL PLAYBACK, not a boot check. A/B against origin/main in two
browsers: app alive with zero page errors (which is itself the proof the import
resolved), #audio is an AUDIO element, togglePlay/seekBy live, and playSong() on a
real library song sets audio.src and the element reports a duration — IDENTICAL on
both sides.
pytest 2396, node 1038/1038, ESLint 0 (no-cycle clean), tailwind clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
b5dd585d25
|
refactor(app): carve settings backup + plugin updates out of app.js (R3a) (#885)
Two leaves, one PR. app.js 9,651 → 9,457.
static/js/settings-io.js (155) — exportSettings + importSettings, the Settings
backup bundle. Imports nothing. The two-phase rationale comment (server first and
atomic; then a best-effort localStorage merge) is the contract and moved with the
code.
plugin-updates → INTO static/js/plugin-loader.js, not a module of its own.
checkPluginUpdates + updatePlugin are plugin MANAGEMENT; they belong with the code
that loads plugins. A new file for 50 lines would have been a file for its own
sake.
All four are inline handlers on the Settings screen and already in app.js's window
contract, so app.js re-exposes the imported bindings unchanged.
VERIFIED BY DRIVING BOTH FLOWS. A/B against origin/main in two browsers:
* checkPluginUpdates() -> hits the API and settles the button back to
"Check for Updates" — IDENTICAL
* exportSettings() -> POSTs /api/settings/export and writes
"Exported feedBack-settings…" to #backup-status — IDENTICAL (fetch intercepted
so the assertion is on the real call, not a stub)
* all four resolve on window — IDENTICAL
* zero console/page errors either side
Zero harnesses broke. pytest 2396, node 1038/1038, ESLint 0, tailwind clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
d47883c5e5
|
refactor(app): carve the tuning-display helpers out of app.js (R3a) (#884)
static/js/tuning-display.js (228 lines) — bodies VERBATIM. app.js 9,838 → 9,650.
A LEAF: imports nothing.
Tuning NAME resolution (Drop D / Eb Standard / raw-offset fallback), bass
detection, effective string count, and the target FREQUENCIES + note names the
tuner checks against. Pure functions over a small MIDI/note-name table; the 3
_TUNING_* tables are read nowhere else and move in.
NOT A SLICE — a node-level extract. The span 2309-2535 INTERLEAVES the functions
with the `window.*` / `window.feedBack.*` assignments that publish them, and one
of those is `window.feedBack = window.feedBack || {}` — the BUS BOOTSTRAP, not
tuning code at all. Every ExpressionStatement stays exactly where it was; only the
16 functions and 3 tables move. app.js re-exposes the imported bindings from the
same lines, so the public surface and its ordering are untouched (constitution II
names window.feedBack).
app.js -> { plugin-loader, viz, diagnostics-export, dom, highway-colors,
tuning-display }
HARNESSES — 4 broke, and 3 of them broke in the SAME informative way: they sliced
app.js from `function isBassArrangement(` UP TO the marker
`window.feedBack.parseRawTuningOffsets = parseRawTuningOffsets;` — an end-marker
that (correctly) stayed behind in app.js. The module is now nothing BUT the tuning
helpers, so there is no block to slice: they read it whole and strip `export ` so
the vm sandbox still evaluates it as a script.
tuner_auto_open is SPLIT — its autoplay-gate test still reads app.js, so it keeps
APP_JS and gains TUNING_JS. Retargeting its path wholesale (my first attempt)
silently pointed the autoplay test at the wrong file.
VERIFIED BY DRIVING THE CONTRACT. A/B against origin/main in two browsers, through
the real window surface: displayTuningName -> "E Standard" / "Drop D" /
"Eb Standard", parseRawTuningOffsets('-2,0,0,0,0,0') -> [-2,0,0,0,0,0],
isBassArrangement, effectiveStringCount, displayTuningTargets, and
window.feedBack.displayTuningName / .songTuningContext — IDENTICAL on both, zero
console/page errors either side.
pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
ebbfc8da6f
|
refactor(app): carve the highway string-colours out of app.js (R3a) (#883)
static/js/highway-colors.js (601 lines) — bodies VERBATIM. app.js 10,415 → 9,837. Under 10k. DECOMPOSED, not sliced. The "settings" blob measured 47 fns / 19 inbound and was not carvable as-is. Seeding the closure from a FUNCTION (initHighwayColors) found only 18 fns and left 4 HWC_* constants used outside it — i.e. the seed was wrong, not the cluster. Re-seeding from the STATE (every function touching HWC_*/`_hwc*`) found the true cluster: 45 top-level nodes, lines 2751-3331, CONTIGUOUS, with ZERO foreign nodes inside the span. INBOUND: 0. EXPORTS: 2 (initHighwayColors, hwcInitSettingsUI). The other 43 symbols — the HWC_* tables, the 12 presets, the theme store, the share codec, the picker handlers, the window.feedBack.highwayColors facade — are used nowhere else in core and stay private. No inline on*= handlers here (the Settings buttons are wired by addEventListener inside hwcInitSettingsUI), so nothing needed re-exposing on window. The three bus listeners register inside initHighwayColors, which app.js calls — not at module top level — so no ordering change. THE no-undef GATE EARNED ITS KEEP. My closure said INBOUND=0; the module actually uses `uiPrompt` (the "name this theme" prompt). It was missed because uiPrompt is no longer an app.js DECLARATION — it's an IMPORT BINDING (from #882's dom.js), and I was collecting declarations only. `no-undef` with typeof:true caught it. => Lesson for the next carve: seed `tops` from ImportDeclaration bindings too. => And it VALIDATES carving dom.js early: this module just imports uiPrompt from it. Had dom.js still been stranded in app.js, this carve would have needed a host seam. app.js -> { plugin-loader, viz, diagnostics-export, dom, highway-colors } plugin-loader -> viz highway-colors -> dom viz, diagnostics-export, dom -> (leaves) VERIFIED BY DRIVING THE FACADE. A/B against origin/main in two browsers: window.feedBack.highwayColors installed, identical method surface, 12 presets, identical default slot colours, and a share-code encode→decode round-trip returning #112233 — IDENTICAL on both, zero console/page errors either side. Harnesses: highway_colors_facade + highway_string_colors retargeted (both brace-extract blocks out of the source by signature). pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
14b4058bc6
|
refactor(app): carve the DOM/modal primitives out of app.js (R3a) (#882)
static/js/dom.js (203 lines) — esc, _escAttr, _isElementVisible, _trapFocusInModal,
_confirmDialog, uiPrompt. Bodies VERBATIM. app.js 10,593 → 10,414.
A GATHER, not a slice — the six lived in six different places (108, 635, 659,
2617, 2623, 8892). They belong together because they are the BOTTOM of the UI
stack: `esc` alone has 25 call sites and `_escAttr` 23, and every later carve that
renders HTML will need them.
That is the actual point of doing this one now. Give them a home and the next
carve imports them; leave them in app.js and the next carve that renders HTML has
to invent a host seam to reach back into app.js — exactly the trap the
plugin-loader carve had to work around until the viz layer became a module. This
is the cheapest possible way to stop that recurring.
app.js -> { plugin-loader, viz, diagnostics-export, dom }
plugin-loader -> viz
viz, diagnostics-export, dom -> (nothing)
Zero imports. Six exports (every one is used outside the cluster).
VERIFIED BY DRIVING THE MODALS, not just booting — they are interactive, so a
green suite says little. A/B against origin/main in two browsers:
* window.uiPrompt / _confirmDialog / _trapFocusInModal all resolve
* uiPrompt() mounts its modal, accepts typed input, and resolves with the typed
value ('typed') — IDENTICAL on both
* _confirmDialog() mounts and resolves true on confirm — IDENTICAL
* zero console/page errors either side
pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
bfb31a8b89
|
refactor(app): carve the diagnostics-bundle export out of app.js (R3a) (#881)
static/js/diagnostics-export.js (280 lines) — bodies VERBATIM.
app.js 10,858 → 10,592.
Chosen BY MEASUREMENT, not by eye. Ran the transitive closure over four candidate
clusters and took the one with the smallest interface:
diagnostics 7 fns 235 lines span 4110-4378 imports 1 exports 2
shortcuts-modal 5 fns 235 lines span 104-9897 imports 4 exports 5
settings+updates 47 fns 1012 lines span 1409-7636 imports 19 exports 30
library-render 220 fns 4064 lines span 20-10536 imports 126 exports 117
diagnostics is contiguous and nearly closed; its one inbound symbol
(_DIAG_FILE_LABELS) lives inside the region and is read only by _renderDiagPreview,
so it moves in and the module ends up a LEAF — imports nothing.
app.js -> { plugin-loader, viz, diagnostics-export }
plugin-loader -> viz
viz, diagnostics-export -> (nothing)
Exports exactly 2: previewDiagnostics + exportDiagnostics, both already in app.js's
window contract (they're inline handlers in the Settings screen) — so app.js keeps
re-exposing them, now as imported bindings. The preview renderer, the file-label
table, and the byte/HTML formatters are used NOWHERE else in core and stay private.
VERIFIED BY DRIVING IT, not just booting. Zero harnesses broke — because the
diagnostics export flow had NO source-level test at all, which is exactly why a
green suite proves nothing here. So the flow was exercised for real: A/B against
origin/main in two browsers, window.previewDiagnostics() invoked, the preview
container rendered identical content on both, both entry points resolve on window,
zero console/page errors either side.
pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0.
NOTE for the next carve: library-render is NOT a cluster — 220 functions and 126
inbound symbols is most of app.js entangled together. It cannot be carved as a
unit; it needs decomposing from the inside first.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
a222b45c02
|
refactor(app): carve the viz layer out of app.js — and delete the loader seam (R3a) (#880)
static/js/viz.js (770 lines) — the viz picker, renderer selection, Auto-match, the WebGL2 probe, the 3D-promotion nag, the notation hints. Bodies VERBATIM. app.js 11,603 → 10,857. THE SEAM IS GONE. #878's plugin-loader needed configurePluginLoader({ populateVizPicker }) purely because _populateVizPicker lived in app.js and importing app.js would have closed a cycle. viz.js is a LEAF — it imports NOTHING — so plugin-loader now imports _populateVizPicker straight from it. The _host object, the configure function, its loud-default guard, and the wiring line in app.js are all deleted. The second carve simplifies the first. app.js -> { plugin-loader, viz } plugin-loader -> viz viz -> (nothing) NOT A PURE MOVE — one listener block had to be SPLIT. app.js had a single top-level `if (window.feedBack) { … }` registering four handlers, and only two were viz. song:loaded / arrangement:changed / song:ready (the mastery slider) stay in app.js and now call the imported _autoMatchViz / _maybeShowNotationViewHint. The viz:reverted handler MOVES, because it REASSIGNS _cancelPendingAutoLabel and an imported binding is read-only — `_cancelPendingAutoLabel = null` would throw if the listener stayed behind while the state moved. ORDER CHECKED, NOT ASSUMED: viz.js's song:ready listener now registers BEFORE app.js's own (imports evaluate first). Safe — _pendingPromotionNag is only ever set inside _populateVizPicker, which runs at boot/plugin-refresh, never from inside the other song:ready handler, so the two are independent. VERIFIED — the listeners are the risk here, so they were DRIVEN, not just booted. A/B against origin/main in two browsers: * viz picker: 6 options (auto|default|venue|drum_highway_3d|keys_highway_3d| highway_3d), selected highway_3d, Auto label — IDENTICAL. This alone proves plugin-loader's direct import of viz.js works. * emit('viz:reverted') -> picker resets to default, localStorage resets to default, the warning logs — IDENTICAL. The MOVED listener fires. * emit('song:ready') -> mastery slider enables, no throw — IDENTICAL. The SPLIT listener still does both halves. * plugin screens, module injections, 37 capability participants — IDENTICAL. * zero console/page errors on both. pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean. no-cycle re-bitten on the 3-module graph (viz -> plugin-loader fails). Codex preflight raised a [P2] claiming viz.js's top-level bus guards would be false because "app.js only creates the event bus later" — FALSE POSITIVE. app.js does not create the bus; capabilities.js does, from its own <script type="module"> at index.html:122, and module scripts execute in document order, so the bus exists long before app.js's import graph evaluates. Instrumented the setter: by viz.js's turn `window.feedBack.on` is already a function, and the viz:reverted listener is provably attached (firing it resets the picker). The ordering is also enforced by test_app_shell_loads_capability_registry_before_app_runtime. Harnesses: 5 tests retargeted to viz.js across legacy_shim_hits, venue_scene_3d, venue_viz (each SPLIT — their non-viz tests still read app.js). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
38772f604a
|
refactor(app): carve the plugin loader out of app.js into static/js/ (R3a) (#878)
The first carve, and deliberately the riskiest: app.js IS the plugin loader (the
R0 host rails), so it goes first while the module graph is still one edge deep.
static/js/plugin-loader.js (829 lines) — bodies VERBATIM. app.js 12,217 → 11,439.
Core's first `static/js/` module, exactly as constitution II anticipates.
CLOSURE (measured with acorn, not regex — brace-matching stripped source drifted):
the block at app.js:11246-12031 is contiguous and self-contained. It needs only
TWO things from the rest of app.js, and exports only TWO:
exports: loadPlugins (the window contract), bootstrapPluginsAndUi (boot)
inbound: window.showScreen — already the public host contract (constitution II),
so it is called through `window`, not re-coupled as an import
_populateVizPicker — injected via configurePluginLoader()
WHY A SEAM, NOT AN IMPORT. plugin-loader must not import app.js: app.js imports
it, so that would close a cycle. I checked whether _populateVizPicker could just
move into the module instead (which would delete the seam entirely) — it drags 9
further symbols (_canRun3D, _autoMatchViz, _showPromotionNag, …), i.e. a whole
viz cluster. That is its own carve, so the seam stays.
THE SEAM'S DEFAULT IS LOUD, ON PURPOSE. A no-op stub is the classic silent
failure for this pattern (see the editor's setHostHooks trap, hit twice): drop the
wiring call and the loader keeps working while the viz picker quietly stops
refreshing — no test, no boot check says a word. The default now console.errors,
so the smoke harness catches it. VERIFIED BY BITE TEST: removing
configurePluginLoader() from app.js surfaces
"[plugin-loader] host seam not configured" at boot. The seam IS exercised on the
plugin-startup path, so an unwired hook cannot pass silently.
no-cycle is now LIVE on core's own graph for the first time. eslint.config.js
gains `static/app.js` + `static/js/**` to the module block — app.js now `import`s,
so parsing it as a script would be a syntax error. VERIFIED BY BITE TEST: making
plugin-loader import app.js back fails with "Dependency cycle detected".
HARNESSES (the R3a note said budget one conversion per carve — it was five):
retargeted capability_inspector_nav, plugin_hydration_wipe,
plugin_loader_script_type, plugin_style_injection, legacy_shim_hits (SPLIT — one
test needs the loader, one still needs app.js) + test_plugin_runtime_idempotence.
legacy_shim_hits was missed by a symbol-name grep because it greps for a code
STRING; only the failing run found it. test_capability_events' NEGATIVE asserts
now span app.js + the loader — carving code out of app.js would otherwise make
them vacuous instead of failing.
VERIFIED: A/B against origin/main in two browsers — mounted plugin screens, 14
loaded plugin scripts, the 3 module plugins injected as <script type="module">,
37 capability participants, 14 shims, window.loadPlugins: IDENTICAL, zero
console/page errors on both. /static/js/plugin-loader.js serves 200; R0 rails
intact (src/main.js 200, conditional GET 304, script_type passthrough).
pytest 2396, node 1032/1032, ESLint 0, Codex 0.
Codex preflight caught a REAL [P1] first pass: static/js/plugin-loader.js was
untracked, so a checkout would have served an app.js importing a nonexistent
module — a failed static import kills the whole module and every window handler
with it. Now tracked.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|