Settings → System → Career panel with the 'Crowd sound reactions' toggle
(writes the localStorage key the crowd layer reads). Pack manifests may
carry sfx {up, down} mp3s, validated like intro files.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rank up → the venue's cheer sample, rank down → boos, gated behind the
feedBack-venue-crowd-sfx setting (default off), rate-limited by the
existing transition hysteresis and suppressed during the intro. Sounds
ship per-venue in the pack manifest (sfx: {up, down}).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(career): career plugin — stars from song_stats, venue tiers, pack downloads (career mode PR2)
Bundled plugin: per-song stars from best_accuracy (60/75/85% → 1/2/3★),
cumulative stars unlock bar → club → arena (data-driven venues.json).
Venue packs (UE-rendered crowd loops) download on demand to
CONFIG_DIR/plugin_uploads/career/ on a background thread with sha256 +
zip-slip validation, served via FileResponse. Career screen (promoted
sidebar entry) shows progress and pushes the active venue's manifest
into the crowd video layer (v3VenueCrowd, PR1) — degrades cleanly when
either side is absent. Pack URLs land in venues.json in PR3.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): keep manifest cleanup path alive on delete; badge only for installed venues
Codex preflight: nulling _appliedManifestVenue on delete skipped
pushCrowdManifest's setManifest(null) cleanup, leaving the crowd layer on
a deleted pack; and the 'playing here' badge showed for an override venue
whose pack was removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): generation-guard in-flight manifest fetches
Codex preflight: a manifest fetch resolving after a newer refresh (pack
deleted, venue switched) could re-apply a stale pack over the user's
newer selection — fetches now carry a generation token and bail when
superseded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): exclude orphaned song_stats from star totals
Codex preflight: scans hide rather than delete stats of removed songs, so
stars now apply the same existing-song filter other stats surfaces use
(filename IN (SELECT filename FROM songs)).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(career): 50/150 star thresholds + star collection overview
Byron's progression tuning: club at 50★, arena at 150★. /state now
returns star_detail rows (title/artist joined from the library, stars,
best accuracy, next-star threshold) sorted closest-to-next-star first,
and the career screen renders a collection panel: tier summary plus a
per-song list with a 'N% to next star' practice hint.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(career): venue select/unselect UX, intro manifest support, fullmatch guards
- 'Play here' now also defaults the visualization to Venue (remembering
the prior viz); active venues show 'Leave venue' which restores it and
sets the '__none__' override so no installed venue silently reapplies.
- Pack manifests may ship an intro block (flyover video + ambience mp3);
files validate like loops/stingers, .mp3 added to the serving whitelist.
- Codex preflight: whitelist regexes use fullmatch (trailing-newline names
could validate but 500 on serving).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): let pushCrowdManifest clear the manifest on Leave venue
Codex preflight: nulling _appliedManifestVenue before refresh skipped the
setManifest(null) cleanup branch, leaving the crowd playing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): refresh tailwind output
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(venue): reactive crowd video layer behind the 3D highway (career mode PR1)
Two crossfading video backdrop planes in the highway_3d venue background
style, driven by a new venue-crowd.js state machine that maps
v3:live-performance-state to crowd states (bored/neutral/engaged/ecstatic)
with 3s stability + 8s dwell hysteresis, plus one-shot reaction stingers
on streak milestones and end-of-song accuracy. Inert without a venue pack
manifest (career plugin, PR2) or the feedBack-venue-crowd-dev flag — the
static bg plate behaves exactly as before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): retry renderer binding + preserve mid-stinger transitions
Codex preflight P2s: (1) videos created before highway_3d registered its
globals never reached the backdrop planes — binding is now idempotent and
retried from start/perf-event/re-activation paths; (2) a crowd-state
switch committing while a stinger played was dropped because the machine
had already advanced — it is now deferred and played when the stinger ends.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): per-video load tokens + unbind renderer on stop
Codex preflight round 2: (1) the global load token let a stinger cancel a
committed loop load on the other layer — tokens are now per-element, and a
stinger preempting an in-flight loop on its own layer requeues that loop
for when the stinger ends; (2) setManifest(null)/deactivate left the last
crowd frame bound and visible over the static plate — stop() now unbinds
both layers from the renderer and zeroes the mix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): flush deferred loop on stinger failure, source accuracy from perf events
Codex preflight round 3: (1) a failed/timed-out stinger left a deferred
loop switch queued forever; the failure path now flushes it. (2)
stats:recorded only carries {filename, arrangement}, so the end-of-song
reaction now uses the accuracyPct from the song's last
v3:live-performance-state event (a real percentage) instead of a field
that never existed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): requeue mid-crossfade loops preempted by stingers; hard-stop on manifest swap
Codex preflight round 4: (1) idleLayer() still points at the fading-in
layer during a crossfade, so a stinger firing mid-fade overwrote the new
loop with nothing requeued — the fading loop is now tracked and requeued
like an in-flight load; (2) swapping venue packs while active now goes
through stop() so _stopGen invalidates the old manifest's in-flight loads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): generation-gate stinger handlers; recrop on video size change
Codex preflight round 5: (1) an ended/timeout handler orphaned by stop()
could fire into a later stinger's lifecycle on the reused element — handlers
now detach unconditionally and carry a generation token; (2) the renderer
only re-applied cover-crop on camera aspect changes, so a src swap with a
different intrinsic size kept stale repeat/offset — it now recrops when
videoWidth/Height change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): bail loop-fade completion when a stinger preempted the layer
Codex preflight round 6: the loop crossfade's completion callback could
still run between a stinger's start and its canplaythrough, promoting the
stinger's layer to active and pausing the real loop — it now bails when
the fading loop was preempted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): keep rear video layer opaque during crossfades
Two half-transparent layers let the static bg plate bleed through (~25%
at mid-fade) — visible as a flash of the old still image on every state
transition. The crossfade is now always the front layer fading over an
opaque rear layer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): reset active layer with mix on stop
Codex preflight: stop() zeroed the mix but left _activeLayer at 1, so a
restart flashed layer 0's stale frame until the new loop loaded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): reset crowd mood to neutral on song load
Codex preflight: a song ending in ecstatic/bored left the next song's
crowd stuck in that mood until the hysteresis window passed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): cancel in-flight fade when a stinger preempts it
Codex preflight: the orphaned ramp kept pushing the mix toward the layer
whose src the stinger had just replaced.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): don't let null accuracy resets wipe the end-of-song value
Codex preflight: Number(null) is 0, so idle HUD resets overwrote
_lastAccuracyPct before stats:recorded consumed it, suppressing the
end-of-song stinger.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): abort stale stinger state on song load
Codex preflight: a stinger straddling a song change could fade back into
the previous song's layer or flush its pending loop.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): always detach load listeners, gate only the callback
Codex preflight: superseded loads left canplaythrough/error listeners
attached to the persistent video elements — unbounded growth over a
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(venue-crowd): flyover intro with crowd-ambience ducking
On song:loaded, an optional pack intro plays once: a camera flyover video
(idle layer, one-shot) with bar-crowd ambience audio that ducks out on
song:play, near the flyover's landing, or at handoff — whichever first.
Machine commits and stingers defer during the intro; stop()/song-change
abort it. Packs without an intro behave as before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(venue-crowd): fall back to the loop when the intro fails to load
Codex preflight: a failed/timed-out intro left the song with no crowd
loop at all.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: nulling _appliedManifestVenue before refresh skipped the
setManifest(null) cleanup branch, leaving the crowd playing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- 'Play here' now also defaults the visualization to Venue (remembering
the prior viz); active venues show 'Leave venue' which restores it and
sets the '__none__' override so no installed venue silently reapplies.
- Pack manifests may ship an intro block (flyover video + ambience mp3);
files validate like loops/stingers, .mp3 added to the serving whitelist.
- Codex preflight: whitelist regexes use fullmatch (trailing-newline names
could validate but 500 on serving).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On song:loaded, an optional pack intro plays once: a camera flyover video
(idle layer, one-shot) with bar-crowd ambience audio that ducks out on
song:play, near the flyover's landing, or at handoff — whichever first.
Machine commits and stingers defer during the intro; stop()/song-change
abort it. Packs without an intro behave as before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Byron's progression tuning: club at 50★, arena at 150★. /state now
returns star_detail rows (title/artist joined from the library, stars,
best accuracy, next-star threshold) sorted closest-to-next-star first,
and the career screen renders a collection panel: tier summary plus a
per-song list with a 'N% to next star' practice hint.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: superseded loads left canplaythrough/error listeners
attached to the persistent video elements — unbounded growth over a
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: a stinger straddling a song change could fade back into
the previous song's layer or flush its pending loop.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: Number(null) is 0, so idle HUD resets overwrote
_lastAccuracyPct before stats:recorded consumed it, suppressing the
end-of-song stinger.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: the orphaned ramp kept pushing the mix toward the layer
whose src the stinger had just replaced.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: a song ending in ecstatic/bored left the next song's
crowd stuck in that mood until the hysteresis window passed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: stop() zeroed the mix but left _activeLayer at 1, so a
restart flashed layer 0's stale frame until the new loop loaded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two half-transparent layers let the static bg plate bleed through (~25%
at mid-fade) — visible as a flash of the old still image on every state
transition. The crossfade is now always the front layer fading over an
opaque rear layer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* 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>
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>
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>
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>
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>
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>
The runtime stylesheet moves to CONFIG_DIR. static/tailwind.min.css is never written again.
━━━ TWO DIFFERENT THINGS WERE SHARING ONE PATH ━━━
static/tailwind.min.css a BUILD ARTEFACT. Committed, image-baked, generated by scanning
the in-tree plugins only. CI's tailwind-fresh check verifies it.
the RUNTIME sheet PER-INSTALL STATE. Additionally scans whatever the user installed
into FEEDBACK_PLUGINS_DIR, so it differs machine to machine.
Writing the second over the first meant that MERELY RUNNING THE DEV SERVER from a git checkout
silently modified a tracked file. `git add -A` then swept a 100KB reshuffle of minified CSS
into the commit and ci/tailwind-fresh went red with a diff that explains nothing — on a PR
whose real change touched no Tailwind classes at all. It also wrote app state into the app
directory, which is read-only in some deploys.
A new route serves the runtime sheet when there is one and falls back to the committed one
otherwise. It is registered BEFORE the /static mount, which would otherwise swallow the path.
━━━ A PERSISTED SHEET MUST NOT OUTLIVE ITS REASON (Codex [P2] x2) ━━━
1. THE USER REMOVES THEIR PLUGINS. Startup only rebuilds when user plugins exist, so nothing
would ever overwrite the stale sheet — and it still carries classes for plugins that are
gone. With no user plugins the COMMITTED sheet is complete by definition. Guarded.
2. THE APP IS UPGRADED, and my first guard for this was WRONG. I compared mtimes. Codex: that
is not a freshness signal across install methods — archives and container images routinely
PRESERVE SOURCE MTIMES, so a just-shipped stylesheet can carry an OLDER timestamp than a
runtime sheet a user built days ago. The mtime check then calls the stale one FRESH and it
masks the new core CSS indefinitely — permanently, if no Tailwind toolchain is present to
trigger a rebuild.
Freshness is decided by CONTENT now. Each runtime build stamps a sidecar with the sha256 of
the committed sheet it was made from. Core ships new CSS -> that file changes -> the hash
changes -> the runtime sheet is correctly judged stale. Timestamps only gesture at the
question that hashing answers.
Falling back to the committed sheet is always safe: at worst it lacks a just-installed plugin's
classes for the seconds until the async rebuild lands.
VERIFIED END TO END. Ran the real dev server with 3 plugins installed: it rebuilt Tailwind over
them (123,291 bytes), wrote the sheet + sidecar to CONFIG_DIR, still served /static/
tailwind.min.css at 200 — and `git diff` on the tracked file came back CLEAN.
8 tests. Bite-tested: reverting to the shared path fails 3, dropping the staleness guards fails
2 more.
pytest 2425, pyflakes 0, Codex 0.
Closes#911
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
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>
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>
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>
Two lines. index.html: defer -> type="module". highway.js: one explicit assignment.
highway.js can now `import`, which is the whole point — the carve can begin.
━━━ THE ONE THING THE FLIP ACTUALLY BREAKS: window.createHighway ━━━
A top-level `function createHighway()` in a CLASSIC script IMPLICITLY becomes
window.createHighway. In a module it does not — module declarations are module-scoped, and
the name vanishes from the global object the instant the tag grows type="module".
The constitution names window.createHighway as PUBLIC EXTENSION CONTRACT (alongside
window.playSong / showScreen / feedBack). NOTHING IN-TREE CALLS IT. That is exactly why this
would have shipped: the only consumers are third-party plugins rendering their own highway
panel, and I cannot grep those. Green CI, green tests, and a broken plugin API.
Verified by removing the assignment and reloading:
flip WITHOUT an explicit assignment: window.createHighway === undefined <-- gone
flip WITH it: window.createHighway === function
So it is assigned explicitly now — same object, same behaviour, no longer an accident of how
the file happens to be loaded.
━━━ AND A CORRECTION TO #912 ━━━
#912 (merged) rewrote 73 bare `highway.x` -> `window.highway.x` on the stated grounds that
the flip would turn every one of them into a ReferenceError. HAVING NOW ACTUALLY FLIPPED IT,
THAT WAS WRONG. highway.js already did `window.highway = highway`, which puts the name on the
GLOBAL OBJECT — and bare-identifier resolution falls back to the global object whether or not
a lexical global binding exists. Measured on both builds: bare `highway` resolves either way.
#912 is defensible as hygiene and it does not hurt, but it was not a precondition and it fixed
no latent bug. A correction is posted on the PR so its commit message does not mislead. The
real hazard was the factory, not the instance — same class of breakage, wrong name.
ORDERING is unchanged: classic-defer and non-async type="module" share ONE post-parse
execution queue, in document order, so highway.js keeps its position at index.html:1244.
VERIFIED. A/B against origin/main: 15 probes IDENTICAL, zero page errors — window.highway,
window.createHighway, the full API surface, a real song playing, the chart clock advancing,
and the seek->setTime sync. THE PERF GATE PASSES at 1.85ms against its 12ms budget (module
evaluation costs nothing at render time), which is exactly what #910 was built to tell me.
node 1045, pytest 2416, ESLint 0, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
highway.getPerf() (additive) + tests/browser/highway-perf-baseline.spec.ts.
No behaviour change. This lands BEFORE highway.js is touched, because a perf-gated refactor
without a perf gate is just a refactor.
━━━ FRAME RATE IS THE WRONG THING TO MEASURE ━━━
The highway AUTO-SCALES. When the smoothed draw cost passes _DRAW_BUDGET_HI_MS (12ms) it
LOWERS THE RENDER RESOLUTION to protect the frame rate (#654). Exactly right for players —
and it means a real perf regression does NOT show up as dropped frames. It shows up as a
BLURRIER PICTURE at a perfectly healthy 60fps.
Benchmark fps and you measure the feedback loop, not the renderer, and conclude nothing
changed while the image quietly degrades.
So the gate pins the scale (setRenderScale(1) + setMinRenderScale(1), which clamps autoScale
to [1,1]) and measures drawMs — the renderer's own cost. None of that was reachable before:
neither drawMs nor the effective scale escaped the closure. Hence getPerf().
The threshold is the app's OWN: _DRAW_BUDGET_HI_MS is the cost at which the highway itself
starts sacrificing resolution in production. Exceeding it is not an arbitrary benchmark line
— it is the renderer failing its own budget. Current cost ~2.2ms, so ~5x headroom: far more
than headless-CI variance, far less than any regression worth shipping.
━━━ I WROTE THIS GATE WRONG THREE TIMES. EACH TIME IT PASSED. ━━━
1. VACUOUS ASSERTION. First cut asserted "the auto-scaler wasn't forced to intervene", i.e.
effectiveScale == 1. I injected a 10x regression (drawMs 2.4 -> 22.4ms, nearly DOUBLE the
budget) and it PASSED. Of course it did: setMinRenderScale(1) sets the scaler's FLOOR to
1, so effectiveScale CANNOT drop below it. The very pinning that stops the scaler hiding
a regression also stops it ever reporting one. A guard that cannot fail.
2. MEASURING AN IDLE RENDERER (Codex [P2]). playSong() takes ~3-4s to actually start — it is
fetching and decoding stems. My "if not playing after 2s, togglePlay()" fired BEFORE
autoplay, started playback, and then the app's own autoplay toggled it straight back to
PAUSED. The renderer idled through the entire measurement. Now it WAITS for playback
rather than racing it, and asserts the chart clock advanced DURING the sampling window —
not merely at some point beforehand, which the first fix would have accepted.
3. UNENCODED FILENAME (Codex [P2]). playSong() decodes its argument before building the
/ws/highway path, so every real caller passes encodeURIComponent(filename)
(app.js:2879, 4137). Raw, a name containing # ? % or / yields an invalid WebSocket URL,
the song never loads — and on those libraries the gate would have silently measured an
idle renderer instead of failing.
Every one of those bugs made the gate PASS. That is the whole hazard of a perf test: it
fails safe in the wrong direction.
BITE-TESTED, and this is the only reason I trust it: a 10x regression injected into the draw
path FAILS the gate under live playback (22.0ms vs the 12ms budget) and the clean build
passes at ~2.1ms with the chart clock advancing 5.6s across the sample.
node 1045, pytest 2412, ESLint 0, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The guard in startup_events() read:
if getenv_compat("FEEDBACK_DEMO_MODE") or getenv_compat("FEEDBACK_DEMO_MODE") == "1" \
and not _DEMO_JANITOR_STARTED:
`and` binds tighter than `or`, so that is `A or (B and C)`. The not-already-started half
never ran when the env var was truthy — the only case that reaches it at all. A second
startup started a SECOND janitor thread, overwrote the handle, and shutdown then joined
only the last: the first leaked and kept firing registered hooks hourly, forever.
The guard now lives INSIDE start_janitor(). A caller cannot get operator precedence wrong
if there is nothing left for it to get wrong.
━━━ THREE WAYS TO WRITE THIS GUARD WRONG. I HIT ALL THREE. ━━━
1. NO GUARD — the original bug. Double-start, orphaned thread.
2. GUARD ON THE FLAG (`if _DEMO_JANITOR_STARTED: return`). Codex [P2]. stop_janitor()
DELIBERATELY leaves that flag True when a hook outruns its join timeout, so that a later
startup cannot spawn a janitor beside a live one. But the hook usually finishes a moment
later: the thread exits and the flag is stale. A flag-keyed guard then refuses to start a
replacement for the rest of the process — demo cleanup silently dead. (The original bug
accidentally MASKED this by always starting.)
3. GUARD ON LIVENESS ALONE (`if thread.is_alive(): return`). Codex [P2], second pass. A
timed-out stop leaves the old thread ALIVE BUT DOOMED — its stop event is set and it
exits as soon as its current hook returns. Treating that as a running janitor skips the
replacement, and we are back at (2) a second later.
So: a janitor counts as running only if its thread is alive AND it has not been told to stop.
━━━ AND EACH JANITOR NOW OWNS ITS STOP EVENT ━━━
start_janitor() used to `_DEMO_JANITOR_STOP.clear()` a single SHARED Event. Start a
replacement while a doomed thread is still finishing a hook and that clear RESURRECTS it: it
loops back to wait(), sees the flag cleared, and carries on. Two janitors — the exact bug we
started from. A fresh Event per janitor makes it impossible; the old thread waits on its own
event, which stays set, so it can only exit.
Env semantics UNCHANGED, verified across every value ("", "1", "0", "true", "false", "off"):
the old expression and demo_mode_enabled() agree on all of them. The only behavioural change
is the idempotency fix.
FOUR tests, and each of the three wrong guards fails a different subset:
no guard -> 2 fail (double start; orphaned thread)
guard on the flag -> 2 fail (never restarts after a timed-out stop)
liveness alone -> 1 fail (no replacement for a doomed janitor)
liveness + not-stopping -> all pass
pytest 2416, pyflakes 0, Codex 0.
Closes#902
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex preflight: scans hide rather than delete stats of removed songs, so
stars now apply the same existing-song filter other stats surfaces use
(filename IN (SELECT filename FROM songs)).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: a manifest fetch resolving after a newer refresh (pack
deleted, venue switched) could re-apply a stale pack over the user's
newer selection — fetches now carry a generation token and bail when
superseded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: nulling _appliedManifestVenue on delete skipped
pushCrowdManifest's setManifest(null) cleanup, leaving the crowd layer on
a deleted pack; and the 'playing here' badge showed for an override venue
whose pack was removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bundled plugin: per-song stars from best_accuracy (60/75/85% → 1/2/3★),
cumulative stars unlock bar → club → arena (data-driven venues.json).
Venue packs (UE-rendered crowd loops) download on demand to
CONFIG_DIR/plugin_uploads/career/ on a background thread with sha256 +
zip-slip validation, served via FileResponse. Career screen (promoted
sidebar entry) shows progress and pushes the active venue's manifest
into the crowd video layer (v3VenueCrowd, PR1) — degrades cleanly when
either side is absent. Pack URLs land in venues.json in PR3.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
Codex preflight round 6: the loop crossfade's completion callback could
still run between a stinger's start and its canplaythrough, promoting the
stinger's layer to active and pausing the real loop — it now bails when
the fading loop was preempted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight round 5: (1) an ended/timeout handler orphaned by stop()
could fire into a later stinger's lifecycle on the reused element — handlers
now detach unconditionally and carry a generation token; (2) the renderer
only re-applied cover-crop on camera aspect changes, so a src swap with a
different intrinsic size kept stale repeat/offset — it now recrops when
videoWidth/Height change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lib/demo_mode.py (342). server.py 1,870 -> 1,649.
The read-only request guard (its 96-entry blocked-route table + the middleware) and the
hourly session janitor (registry, hook runner, thread). Bodies VERBATIM.
THE MIDDLEWARE NEEDS `app`, SO THE MODULE TAKES IT. _demo_mode_guard is an
@app.middleware("http") and cannot exist without an app object. Rather than have a module
under lib/ reach for a global, it exposes install(app) and server.py — which owns the app —
hands it over. The janitor is symmetrical: start_janitor() / stop_janitor(), called from
server.py's startup and shutdown hooks, where the process lifecycle actually lives.
register_demo_janitor_hook IS PART OF THE PLUGIN CONTRACT. It is a key in plugin_context,
so plugins hold it as a LIVE REFERENCE from setup(). server.py imports this exact object
and puts it in the dict unchanged — identity preserved, and
tests/test_plugin_context_contract.py (#898, merged) fails if that ever stops being true.
This is the first carve that guard has actually protected.
━━━ stop_janitor()'s ORDER IS LOAD-BEARING ━━━
The obvious way to write it — clear the "started" flag, then join — is WRONG, and I wrote
it that way first. server.py's original deliberately returns EARLY, leaving
_DEMO_JANITOR_STARTED True and the thread handle intact, when the thread outlives the join:
# Leave _DEMO_JANITOR_STARTED True so a new janitor is not
# spawned by a subsequent startup while the old one is alive.
Clearing the flag first quietly reintroduces exactly the double-janitor leak the flag
exists to prevent. Preserved byte-for-byte, and the reason is now written down at the
function rather than only at its single call site.
━━━ A BUG MOVED VERBATIM, ON PURPOSE ━━━
if getenv_compat("FEEDBACK_DEMO_MODE") or getenv_compat("FEEDBACK_DEMO_MODE") == "1" \
and not _DEMO_JANITOR_STARTED:
`and` binds tighter than `or`, so this is `A or (B and C)` — the not-already-started
re-entry guard is DEAD whenever the env var is truthy, which is the only case that runs.
A second startup leaks a janitor thread (the handle is overwritten, so shutdown joins only
the last). Verified. Preserved exactly and filed as issue #902: a carve whose whole value
is being provably behaviour-neutral is not the place to change behaviour.
pyflakes caught three more missing imports on the way in (uuid, warnings x2). Five carves,
ten missing imports, every one a NameError on a live path.
pytest 2399, pyflakes 0, Codex 0.
Refs #48
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos