refactor(highway): carve the PURE geometry primitives into highway-geometry.js (R3c)#915
Conversation
…ometry.js (R3c) 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>
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
1 participant
6 functions, 53 lines.
highway.js4,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 tookctxexplicitly, 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 passedhwStaterather 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.
And the purity is checked, not assumed — the extractor refuses to move a function whose body mentions
hwState, or that referencesctxwithout taking it as a parameter.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
hwStateparameter 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 xstill containsfunction x, so the extractor needed no change — only the path.Verification
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 slice that could plausibly have cost something:
project()runs for every visible note on every frame, and it is now a cross-module call. It costs nothing measurable.That is precisely the answer #910 was built to give.
node 1045 · pytest 2416 · ESLint 0 · Codex 0.
🤖 Generated with Claude Code