refactor(highway): lift 79 per-instance closure vars into hwState (R3c H lift)

[byrongamatos]

The H-container lift — the enabler for splitting highway.js. Collapses createHighway()'s 79 mutable closure lets into one per-instance hwState object, so extracted renderer/ws modules can later close over hwState as a factory argument without multi-panel plugins (highway_3d, note_detect, splitscreen) sharing one highway's state.

Depends conceptually on the frame-time gate in #848 (the --song harness mode); functionally independent.

The transform

Scope-resolved rewrite via acorn + eslint-scope (not regex — that would corrupt the 4 shadowed names): 1059 edits, 1057 references + 79 declaration ids − the deleted let canvas, ctx, ws. The four names shadowed in inner scopes (chartTime, ctx, notes, chordTemplates) are resolved correctly, so only closure-bound references move. Verified: re-parses, 0 leftover mutable closure vars, and the added lines are all hwState. (plus 2 comment lines).

Container is hwState, not H — H is already the canvas-height local (70 uses). The frame gate caught the collision instantly (0 draws, H._drawHooks is not iterable in the shared draw-hook path).

Perf — the whole risk — holds

Frame-time gate (docs/perf-baseline.md), Arcturus feedpak, headless:

	draw p50	draw p95
pre-lift	~2.2 ms	2.7–3.2 ms
post-lift	2.1–2.2 ms	2.7–3.0 ms

Each closure-slot read became a hwState.<slot> monomorphic property load on a stable-shape object; the 60fps loop pays nothing. On-device: Byron confirmed the 2D highway plays smoothly.

Tests

The ~30 highway JS suites brace-extract functions/patterns from the source; their state references and the monotonic_clock vm sandbox now use hwState.<slot> (consts like _CHART_MAX_INTERP_MS stay top-level — not lifted). node --test: 1030/1030 green. Two self-inflicted over-replacements caught and reverted along the way (_lefty is a prefix of the 3D-local _leftyCached; STRING_COLORS a suffix of DEFAULT_STRING_COLORS) — substring replaces on brace-extract regexes need word care.

npm run lint 0 errors; Codex 0 findings. Transformer saved for reproducibility.

Next (R3c cont.): with hwState in place, the renderer/ws/geometry modules can be extracted as createRenderer2d(hwState) etc.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved renderer stability during WebSocket reconnects and renderer mode swaps.
  • Fixed timing/pause detection and made draw-loop behavior more consistent across playback states.
  • Improved visibility throttling and ensured HUD/canvas state resets correctly.
  • Enhanced adaptive render scaling behavior while playing.
• Tests
  • Updated assertions to match the new instance-scoped renderer state across timing, visibility, note/chord/handshape rendering, renderer swapping, and adaptive scaling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6a7633b1-d5ce-4789-acc2-f2a0973e6bad

📥 Commits

Reviewing files that changed from the base of the PR and between d8b398b and de49cd7.

📒 Files selected for processing (1)

• tests/js/highway_note_state.test.js

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/js/highway_note_state.test.js

---

📝 Walkthrough

Walkthrough

static/highway.js refactors mutable renderer, timing, connection, song, and API state into a per-instance hwState object. Renderer lifecycle, WebSocket generation handling, clock interpolation, drawing, adaptive scaling, and related source-level tests are updated accordingly.

Changes

Highway state refactor

Layer / File(s)	Summary
State container and renderer bundle static/highway.js, tests/js/highway_note_state.test.js, tests/js/highway_string_colors.test.js, tests/js/highway_filtered_notes.test.js	Mutable state, palettes, note providers, filtered arrays, and renderer bundle values are read from hwState.
Renderer lifecycle and draw scheduling static/highway.js, tests/js/highway_renderer_swap_canvas_reset.test.js, tests/js/highway_visibility.test.js, tests/js/highway_pause_throttle.test.js	Renderer switching, canvas replacement, visibility throttling, pause gating, and draw scheduling use hwState.
Note, chord, sustain, and lyric rendering static/highway.js, tests/js/highway_chord_render_cache.test.js, tests/js/handshapes_pipeline.test.js, tests/js/highway_3d_lefty.test.js	Drawing paths and chord caches use instance timing, context, palettes, filtered data, and state-qualified bundle fields.
Initialization, clock, connection, and public API static/highway.js, tests/js/highway_monotonic_clock.test.js, tests/js/beats_loaded.test.js, tests/js/highway_adaptive_scale.test.js, tests/js/highway_phrases.test.js, tests/js/highway_3d_smooth_clock_pause.test.js	Initialization, adaptive scaling, clock state, WebSocket message accumulation, generation checks, API accessors, and reset flows are backed by hwState.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WebSocket
  participant JUCE routing
  participant hwState
  participant Renderer
  WebSocket->>JUCE routing: route incoming message
  JUCE routing->>hwState: update song and connection state
  hwState->>hwState: validate connection generation and message chain
  hwState->>Renderer: expose updated bundle and readiness
  Renderer->>hwState: read timing, canvas, and filtered data

Loading

Possibly related PRs

• got-feedBack/feedBack#729: Both changes modify sustain rendering in static/highway.js; this PR also refactors that path around hwState.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main refactor: moving per-instance Highway closure state into hwState.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/r3-highway-h-lift

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@static/highway.js`:
- Line 1501: static/highway.js exceeds the repository’s max-lines limit and must
be reduced below 1500 lines. Extract cohesive functionality—starting with
drawing primitives and chord-cache helpers such as drawNote, drawSustains,
drawChords, and drawLyrics—into modules whose factories accept hwState and
required helpers, then update highway.js imports and call sites while preserving
behavior.

In `@tests/js/highway_note_state.test.js`:
- Line 42: Update the setter assertion in the test to require assignment
specifically to hwState._noteStateProvider, rather than any unqualified
_noteStateProvider. Adjust the regex for setNoteStateProvider so it matches the
hwState-qualified property assignment and continues verifying the setter
behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f13af998-b59f-4ffd-8ce7-1723726f1b9a

📥 Commits

Reviewing files that changed from the base of the PR and between cbc6545 and d8b398b.

📒 Files selected for processing (15)

• static/highway.js
• tests/js/beats_loaded.test.js
• tests/js/handshapes_pipeline.test.js
• tests/js/highway_3d_lefty.test.js
• tests/js/highway_3d_smooth_clock_pause.test.js
• tests/js/highway_adaptive_scale.test.js
• tests/js/highway_chord_render_cache.test.js
• tests/js/highway_filtered_notes.test.js
• tests/js/highway_monotonic_clock.test.js
• tests/js/highway_note_state.test.js
• tests/js/highway_pause_throttle.test.js
• tests/js/highway_phrases.test.js
• tests/js/highway_renderer_swap_canvas_reset.test.js
• tests/js/highway_string_colors.test.js
• tests/js/highway_visibility.test.js

[byrongamatos]

On the two threads:

max-lines (4224 vs 1500) — expected and tracked. highway.js is the planned-split monolith; it sits in docs/size-exemptions.md under Planned, NOT exempt with a pre-approved post-split renderer-2d.js ceiling of 3,000. This PR is the state-lift prerequisite (the hwState container is what lets the renderer/ws/geometry modules become createRenderer2d(hwState) factories); the module split that brings the residual under the ceiling is the next R3c step. max-lines is a warning, not an error — CI lint is green. The net line delta here is ~+3 (895 ins / 892 del).

note_state:42 — fixed in <pushed>: the assertion now requires hwState._noteStateProvider =, so a regression to closure/shared state fails the test instead of matching an unqualified form.