mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-23 13:21:21 +00:00
* refactor(highway): lift 79 per-instance closure vars into `hwState` (R3c H lift) Collapses createHighway()'s 79 mutable closure `let`s into one per-instance `hwState` object. Scope-resolved rewrite via acorn + eslint-scope: 1059 edits (1057 references + 79 defs - the deleted `let canvas, ctx, ws`), with the four names shadowed in inner scopes (chartTime/ctx/notes/chordTemplates) resolved correctly so only closure-bound refs move. Enables the later module split: extracted renderer/ws modules close over `hwState` as a factory arg, so multi-panel plugins (highway_3d, note_detect, splitscreen) don't share one highway's state. Container is `hwState`, NOT `H` — `H` is already canvas height (70 uses). The frame-time gate caught that collision instantly (0 draws, `H._drawHooks is not iterable` in the shared draw-hook path). PERF (the whole risk): identical to the pre-lift baseline. Draw p50 2.1-2.2 ms, p95 2.7-3.0 ms (pre-lift 2.7-3.2), measured on the Arcturus feedpak, headless. Each closure-slot read became a `hwState.<slot>` monomorphic property load; the hot 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 + the monotonic-clock vm sandbox now use `hwState.<slot>` (the const _CHART_MAX_INTERP_MS etc. stay top-level, not lifted). node --test: 1030/1030 green. Two self-inflicted over-replacements caught and reverted (`_lefty` is a prefix of the 3D-local `_leftyCached`; `STRING_COLORS` a suffix of `DEFAULT_STRING_COLORS`) — substring replaces on the brace-extract regexes need word care. Transformer saved at ~/.local/share/feedback-editor/highway-h-lift.mjs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(highway): pin the setNoteStateProvider assertion to hwState._noteStateProvider (CodeRabbit) The [^}]* form matched an unqualified _noteStateProvider =, so a regression to closure-level state could still pass. Require the hwState-qualified assignment. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
60 lines
3.3 KiB
JavaScript
60 lines
3.3 KiB
JavaScript
// Source-level guards for the 2D highway chord render cache
|
|
// (`_ensureChordRenderCache`). Locks in the invalidation contract so a
|
|
// regression that drops one of the three keys, or forgets to reset the
|
|
// derived state, will fail in CI.
|
|
//
|
|
// Background: see feedBack#412 and the Copilot review thread that
|
|
// surfaced the `chordTemplates` ordering edge case (templates can land
|
|
// after the final `chords` chunk; `isOpen()`-derived `nonZeroNotes`
|
|
// would otherwise stay stale until the next chord transition).
|
|
//
|
|
// Like the other highway tests in this directory, these inspect the
|
|
// source rather than executing — the createHighway() closure owns
|
|
// canvas + WebGL lifecycle that's too heavy for a vm sandbox.
|
|
|
|
const { test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('node:fs');
|
|
const path = require('node:path');
|
|
|
|
const highwayJs = path.join(__dirname, '..', '..', 'static', 'highway.js');
|
|
|
|
test('_ensureChordRenderCache keys off src, _inverted, AND chordTemplates', () => {
|
|
const src = fs.readFileSync(highwayJs, 'utf8');
|
|
// The cache key triple must include chordTemplates — without it, a
|
|
// late-arriving `chord_templates` WS message leaves cached
|
|
// nonZeroNotes / nonZeroFrets stale until the next chord transition.
|
|
//
|
|
// Match either operand order (`A === B` or `B === A`) so a future
|
|
// stylistic refactor that flips sides doesn't trip these guards —
|
|
// the semantic invariant is the comparison, not its placement.
|
|
const eqEither = (a, b) => new RegExp(
|
|
`\\b${a}\\b\\s*===\\s*\\b${b}\\b|\\b${b}\\b\\s*===\\s*\\b${a}\\b`
|
|
);
|
|
const neqEither = (a, b) => new RegExp(
|
|
`\\b${a}\\b\\s*!==\\s*\\b${b}\\b|\\b${b}\\b\\s*!==\\s*\\b${a}\\b`
|
|
);
|
|
assert.match(src, eqEither('hwState\\._chordRenderCacheSrc', 'src'), 'cache must key on src');
|
|
assert.match(src, eqEither('hwState\\._chordRenderCacheInverted', 'hwState\\._inverted'), 'cache must key on _inverted');
|
|
assert.match(src, neqEither('hwState\\._chordRenderCacheTemplates', 'hwState\\.chordTemplates'),
|
|
'cache must key on chordTemplates (detected via !== for change-flag)');
|
|
});
|
|
|
|
test('chordTemplates change resets fretline preview and frame-mismatch warner', () => {
|
|
const src = fs.readFileSync(highwayJs, 'utf8');
|
|
// The cache-invalidation block must clear both _chordFretLineNotes
|
|
// (so _updateFretLinePreview re-publishes with corrected isOpen
|
|
// classification) and _frameMismatchWarned (so a chord ID warned
|
|
// against stale templates re-validates against the corrected ones).
|
|
// Non-greedy `[\s\S]*?` instead of `[^}]*` so a future nested
|
|
// block inside the `if (templatesChanged) { … }` branch (e.g. an
|
|
// inner conditional reset) doesn't break the match by introducing
|
|
// a `}` before the symbol we're checking for.
|
|
assert.match(src, /if\s*\(\s*templatesChanged\s*\)\s*\{[\s\S]*?hwState\._chordFretLineNotes\s*=\s*\[\][\s\S]*?\}/,
|
|
'templatesChanged branch must reset _chordFretLineNotes');
|
|
assert.match(src, /if\s*\(\s*templatesChanged\s*\)\s*\{[\s\S]*?hwState\._lastChordOnFretLine\s*=\s*null[\s\S]*?\}/,
|
|
'templatesChanged branch must null _lastChordOnFretLine');
|
|
assert.match(src, /if\s*\(\s*templatesChanged\s*\)\s*\{[\s\S]*?_frameMismatchWarned\.clear\(\)[\s\S]*?\}/,
|
|
'templatesChanged branch must clear _frameMismatchWarned');
|
|
});
|