mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-10 18:59:56 +00:00
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>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
c6963fdf30
commit
8e89b39ad3
@@ -27,16 +27,33 @@ function extractBlock(src, signature) {
|
||||
return src.slice(start, i);
|
||||
}
|
||||
|
||||
|
||||
// R3c: highway.js is being carved into modules, so its source is no longer ONE file. Read the
|
||||
// whole set. Re-pinning these assertions at whichever file currently holds a constant just
|
||||
// means they break again on the next carve — and worse, a source-shape assertion that silently
|
||||
// stops finding its target is indistinguishable from one that passes.
|
||||
function highwaySources() {
|
||||
const root = path.join(__dirname, '..', '..');
|
||||
const jsDir = path.join(root, 'static', 'js');
|
||||
const parts = [fs.readFileSync(path.join(root, 'static', 'highway.js'), 'utf8')];
|
||||
for (const f of fs.readdirSync(jsDir).sort()) {
|
||||
if (f.startsWith('highway-') && f.endsWith('.js')) {
|
||||
parts.push(fs.readFileSync(path.join(jsDir, f), 'utf8'));
|
||||
}
|
||||
}
|
||||
return parts.join('\n');
|
||||
}
|
||||
|
||||
test('highway declares adaptive-scale state with a floor', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
assert.match(src, /hwState\._autoScale\s*=\s*1/, 'missing _autoScale multiplier');
|
||||
assert.match(src, /const\s+_AUTO_SCALE_MIN\s*=\s*0?\.25/, 'missing _AUTO_SCALE_MIN floor (0.25)');
|
||||
assert.match(src, /const\s+_DRAW_BUDGET_HI_MS\s*=\s*\d+/, 'missing high draw budget');
|
||||
assert.match(src, /const\s+_DRAW_BUDGET_LO_MS\s*=\s*\d+/, 'missing low draw budget');
|
||||
assert.match(src, /(?:export\s+)?const\s+_AUTO_SCALE_MIN\s*=\s*0?\.25/, 'missing _AUTO_SCALE_MIN floor (0.25)');
|
||||
assert.match(src, /(?:export\s+)?const\s+_DRAW_BUDGET_HI_MS\s*=\s*\d+/, 'missing high draw budget');
|
||||
assert.match(src, /(?:export\s+)?const\s+_DRAW_BUDGET_LO_MS\s*=\s*\d+/, 'missing low draw budget');
|
||||
});
|
||||
|
||||
test('_effectiveRenderScale clamps user ceiling * auto factor to [MIN, 1]', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
const fn = extractBlock(src, 'function _effectiveRenderScale()');
|
||||
// Derives from the (sanitized) user ceiling and auto factor.
|
||||
assert.match(fn, /_renderScale/, 'effective scale must derive from the user _renderScale');
|
||||
@@ -47,7 +64,7 @@ test('_effectiveRenderScale clamps user ceiling * auto factor to [MIN, 1]', () =
|
||||
});
|
||||
|
||||
test('min render scale floor is user-configurable + exposed on the api (#654)', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
// Hard floor constant kept; configurable floor read from localStorage.
|
||||
assert.match(src, /hwState\._autoScaleMin\s*=/, 'missing configurable _autoScaleMin');
|
||||
assert.match(src, /localStorage\.getItem\('highwayMinRenderScale'\)/,
|
||||
@@ -65,7 +82,7 @@ test('min render scale floor is user-configurable + exposed on the api (#654)',
|
||||
});
|
||||
|
||||
test('_adaptRenderScale uses the draw budget + cooldown and re-applies via resize', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
const fn = extractBlock(src, 'function _adaptRenderScale(');
|
||||
assert.match(fn, /_DRAW_BUDGET_HI_MS/, 'must scale down past the high budget');
|
||||
assert.match(fn, /_DRAW_BUDGET_LO_MS/, 'must scale up below the low budget');
|
||||
@@ -74,27 +91,27 @@ test('_adaptRenderScale uses the draw budget + cooldown and re-applies via resiz
|
||||
});
|
||||
|
||||
test('draw() only adapts during active playback and feeds the HUD', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
const fn = extractBlock(src, 'function draw()');
|
||||
assert.match(fn, /if\s*\(\s*!_paused\s*\)\s*_adaptRenderScale/, 'must skip adaptation while paused');
|
||||
assert.match(fn, /_updatePerfHud\(\)/, 'must update the perf HUD each drawn frame');
|
||||
});
|
||||
|
||||
test('bundle + canvas sizing use the effective scale, not the raw user value', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
assert.match(src, /renderScale\s*[:=]\s*_effectiveRenderScale\(\)/, 'bundle.renderScale must be the effective scale');
|
||||
assert.match(src, /canvas\.width\s*=\s*Math\.round\(w\s*\*\s*_effectiveRenderScale\(\)\)/, 'canvas backing store must use effective scale');
|
||||
});
|
||||
|
||||
test('api exposes effective scale + perf stats', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
assert.match(src, /getEffectiveRenderScale\(\)\s*\{\s*return\s+_effectiveRenderScale\(\)/, 'api.getEffectiveRenderScale missing');
|
||||
assert.match(src, /getPerfStats\(\)\s*\{/, 'api.getPerfStats missing');
|
||||
});
|
||||
|
||||
// Robustness fixes from the #655 Copilot review.
|
||||
test('render scale is sanitized on load and effective scale guards non-finite', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
assert.match(src, /parseFloat\(localStorage\.getItem\('renderScale'\)[\s\S]{0,160}?Number\.isFinite/,
|
||||
'render scale load must validate via Number.isFinite + clamp');
|
||||
const eff = extractBlock(src, 'function _effectiveRenderScale()');
|
||||
@@ -102,7 +119,7 @@ test('render scale is sanitized on load and effective scale guards non-finite',
|
||||
});
|
||||
|
||||
test('stop() tears down the perf HUD and resets per-session accumulators', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
assert.match(src, /stop\(\)\s*\{[\s\S]{0,400}?_perfHud\.remove\(\)/,
|
||||
'stop() must remove the perf HUD so it cannot strand in the DOM');
|
||||
assert.match(src, /stop\(\)\s*\{[\s\S]{0,1200}?_autoScale\s*=\s*1/,
|
||||
@@ -112,7 +129,7 @@ test('stop() tears down the perf HUD and resets per-session accumulators', () =>
|
||||
});
|
||||
|
||||
test('perf HUD throttles its localStorage flag read off the hot path', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
const fn = extractBlock(src, 'function _updatePerfHud()');
|
||||
assert.match(fn, /_hudFlagAt/, 'HUD must cache the flag and re-read on an interval, not every frame');
|
||||
});
|
||||
|
||||
@@ -60,7 +60,7 @@ function buildClockSandbox(perfNowImpl) {
|
||||
performance: { now: perfNowImpl },
|
||||
};
|
||||
vm.createContext(sandbox);
|
||||
const src = fs.readFileSync(HIGHWAY_JS, 'utf8');
|
||||
const src = highwaySources();
|
||||
const setTimeBody = extractBlock(src, 'setTime(t) {');
|
||||
const getTimeBody = extractBlock(src, 'getTime() {');
|
||||
// Strip trailing comma if present (object-literal method declarations).
|
||||
@@ -72,8 +72,25 @@ function buildClockSandbox(perfNowImpl) {
|
||||
return sandbox;
|
||||
}
|
||||
|
||||
|
||||
// R3c: highway.js is being carved into modules, so its source is no longer ONE file. Read the
|
||||
// whole set. Re-pinning these assertions at whichever file currently holds a constant just
|
||||
// means they break again on the next carve — and worse, a source-shape assertion that silently
|
||||
// stops finding its target is indistinguishable from one that passes.
|
||||
function highwaySources() {
|
||||
const root = path.join(__dirname, '..', '..');
|
||||
const jsDir = path.join(root, 'static', 'js');
|
||||
const parts = [fs.readFileSync(path.join(root, 'static', 'highway.js'), 'utf8')];
|
||||
for (const f of fs.readdirSync(jsDir).sort()) {
|
||||
if (f.startsWith('highway-') && f.endsWith('.js')) {
|
||||
parts.push(fs.readFileSync(path.join(jsDir, f), 'utf8'));
|
||||
}
|
||||
}
|
||||
return parts.join('\n');
|
||||
}
|
||||
|
||||
test('highway declares chart anchor + stall-detect + rate state', () => {
|
||||
const src = fs.readFileSync(HIGHWAY_JS, 'utf8');
|
||||
const src = highwaySources();
|
||||
// Both anchor fields use NaN sentinels — _chartAnchorAudioT in
|
||||
// particular MUST start as NaN, not 0, otherwise setTime(0) on the
|
||||
// very first 60 Hz tick fails the `t !== _chartAnchorAudioT` check
|
||||
@@ -82,11 +99,11 @@ test('highway declares chart anchor + stall-detect + rate state', () => {
|
||||
assert.match(src, /hwState\._chartAnchorPerfNow\s*=\s*NaN/, 'missing _chartAnchorPerfNow (NaN sentinel)');
|
||||
assert.match(src, /hwState\._chartLastAdvanceAt\s*=\s*0/, 'missing _chartLastAdvanceAt (pause detection)');
|
||||
assert.match(src, /hwState\._chartObservedRate\s*=\s*1/, 'missing _chartObservedRate (playback rate awareness)');
|
||||
assert.match(src, /const\s+_CHART_MAX_INTERP_MS\s*=\s*100/, 'missing _CHART_MAX_INTERP_MS cap');
|
||||
assert.match(src, /(?:export\s+)?const\s+_CHART_MAX_INTERP_MS\s*=\s*100/, 'missing _CHART_MAX_INTERP_MS cap');
|
||||
});
|
||||
|
||||
test('getTime scales interpolation by _chartObservedRate (speed-slider safe)', () => {
|
||||
const src = fs.readFileSync(HIGHWAY_JS, 'utf8');
|
||||
const src = highwaySources();
|
||||
const m = src.match(/getTime\(\)\s*\{[\s\S]+?\n\s*\},/);
|
||||
assert.ok(m, 'getTime() body not found');
|
||||
const slice = m[0];
|
||||
@@ -98,7 +115,7 @@ test('getTime scales interpolation by _chartObservedRate (speed-slider safe)', (
|
||||
});
|
||||
|
||||
test('setTime re-anchors and updates _chartLastAdvanceAt only when t actually changes', () => {
|
||||
const src = fs.readFileSync(HIGHWAY_JS, 'utf8');
|
||||
const src = highwaySources();
|
||||
// Repeated setTime calls with the same value must not refresh the
|
||||
// anchor (else interpolation stutters); they also must not refresh
|
||||
// _chartLastAdvanceAt (else getTime would never detect a stalled
|
||||
@@ -115,7 +132,7 @@ test('setTime re-anchors and updates _chartLastAdvanceAt only when t actually ch
|
||||
});
|
||||
|
||||
test('getTime falls back to chartTime when audio has stalled (paused)', () => {
|
||||
const src = fs.readFileSync(HIGHWAY_JS, 'utf8');
|
||||
const src = highwaySources();
|
||||
// Find the actual getTime body. Match the whole brace-balanced
|
||||
// method (using a generous greedy slice to ensure we capture both
|
||||
// the stall check and the interpolation expression below it).
|
||||
@@ -139,7 +156,7 @@ test('getTime falls back to chartTime when audio has stalled (paused)', () => {
|
||||
});
|
||||
|
||||
test('api.stop() clears the chart anchor state so re-init starts fresh', () => {
|
||||
const src = fs.readFileSync(HIGHWAY_JS, 'utf8');
|
||||
const src = highwaySources();
|
||||
// Use the brace-balanced extractor so the assertions are scoped to
|
||||
// the actual stop() body — a fixed-size slice would falsely match
|
||||
// resets that landed in an adjacent method.
|
||||
|
||||
@@ -29,14 +29,31 @@ function extractBlock(src, signature) {
|
||||
return src.slice(start, i);
|
||||
}
|
||||
|
||||
|
||||
// R3c: highway.js is being carved into modules, so its source is no longer ONE file. Read the
|
||||
// whole set. Re-pinning these assertions at whichever file currently holds a constant just
|
||||
// means they break again on the next carve — and worse, a source-shape assertion that silently
|
||||
// stops finding its target is indistinguishable from one that passes.
|
||||
function highwaySources() {
|
||||
const root = path.join(__dirname, '..', '..');
|
||||
const jsDir = path.join(root, 'static', 'js');
|
||||
const parts = [fs.readFileSync(path.join(root, 'static', 'highway.js'), 'utf8')];
|
||||
for (const f of fs.readdirSync(jsDir).sort()) {
|
||||
if (f.startsWith('highway-') && f.endsWith('.js')) {
|
||||
parts.push(fs.readFileSync(path.join(jsDir, f), 'utf8'));
|
||||
}
|
||||
}
|
||||
return parts.join('\n');
|
||||
}
|
||||
|
||||
test('highway declares the paused-render throttle state', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
assert.match(src, /const\s+_PAUSED_FRAME_INTERVAL_MS\s*=\s*\d+/, 'missing _PAUSED_FRAME_INTERVAL_MS cap');
|
||||
const src = highwaySources();
|
||||
assert.match(src, /(?:export\s+)?const\s+_PAUSED_FRAME_INTERVAL_MS\s*=\s*\d+/, 'missing _PAUSED_FRAME_INTERVAL_MS cap');
|
||||
assert.match(src, /hwState\._lastPausedDrawAt\s*=\s*0/, 'missing _lastPausedDrawAt accumulator');
|
||||
});
|
||||
|
||||
test('draw() throttles full renders while the audio clock is stalled', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
const fn = extractBlock(src, 'function draw()');
|
||||
// Reuse getTime()'s pause signal rather than inventing a parallel one.
|
||||
assert.match(fn, /_chartLastAdvanceAt/, 'throttle must key off _chartLastAdvanceAt (the advance timestamp)');
|
||||
@@ -46,7 +63,7 @@ test('draw() throttles full renders while the audio clock is stalled', () => {
|
||||
});
|
||||
|
||||
test('throttle runs after the ready gate, before bundle/draw', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
const fn = extractBlock(src, 'function draw()');
|
||||
// Regex landmarks (not exact-string indexOf) so harmless spacing /
|
||||
// semicolon changes don't break the ordering guard — matches the
|
||||
|
||||
@@ -37,18 +37,35 @@ function extractBlock(src, signature) {
|
||||
|
||||
// ── 2D highway (static/highway.js) ────────────────────────────────────────
|
||||
|
||||
|
||||
// R3c: highway.js is being carved into modules, so its source is no longer ONE file. Read the
|
||||
// whole set. Re-pinning these assertions at whichever file currently holds a constant just
|
||||
// means they break again on the next carve — and worse, a source-shape assertion that silently
|
||||
// stops finding its target is indistinguishable from one that passes.
|
||||
function highwaySources() {
|
||||
const root = path.join(__dirname, '..', '..');
|
||||
const jsDir = path.join(root, 'static', 'js');
|
||||
const parts = [fs.readFileSync(path.join(root, 'static', 'highway.js'), 'utf8')];
|
||||
for (const f of fs.readdirSync(jsDir).sort()) {
|
||||
if (f.startsWith('highway-') && f.endsWith('.js')) {
|
||||
parts.push(fs.readFileSync(path.join(jsDir, f), 'utf8'));
|
||||
}
|
||||
}
|
||||
return parts.join('\n');
|
||||
}
|
||||
|
||||
test('2D palette arrays are mutable (let) with frozen DEFAULT_* originals', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
assert.match(src, /const\s+DEFAULT_STRING_COLORS\s*=/, 'DEFAULT_STRING_COLORS must exist for reset');
|
||||
assert.match(src, /const\s+DEFAULT_STRING_DIM\s*=/, 'DEFAULT_STRING_DIM must exist for reset');
|
||||
assert.match(src, /const\s+DEFAULT_STRING_BRIGHT\s*=/, 'DEFAULT_STRING_BRIGHT must exist for reset');
|
||||
const src = highwaySources();
|
||||
assert.match(src, /(?:export\s+)?const\s+DEFAULT_STRING_COLORS\s*=/, 'DEFAULT_STRING_COLORS must exist for reset');
|
||||
assert.match(src, /(?:export\s+)?const\s+DEFAULT_STRING_DIM\s*=/, 'DEFAULT_STRING_DIM must exist for reset');
|
||||
assert.match(src, /(?:export\s+)?const\s+DEFAULT_STRING_BRIGHT\s*=/, 'DEFAULT_STRING_BRIGHT must exist for reset');
|
||||
assert.match(src, /hwState\.STRING_COLORS\s*=\s*DEFAULT_STRING_COLORS\.slice\(\)/, 'STRING_COLORS must be a mutable copy of the defaults');
|
||||
assert.match(src, /hwState\.STRING_DIM\s*=\s*DEFAULT_STRING_DIM\.slice\(\)/, 'STRING_DIM must be a mutable copy of the defaults');
|
||||
assert.match(src, /hwState\.STRING_BRIGHT\s*=\s*DEFAULT_STRING_BRIGHT\.slice\(\)/, 'STRING_BRIGHT must be a mutable copy of the defaults');
|
||||
});
|
||||
|
||||
test('2D public API exposes getStringColors / setStringColors', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
assert.match(src, /getStringColors\s*\(\s*\)\s*\{\s*return\s+hwState\.STRING_COLORS\.slice\(\)/, 'getStringColors must return a copy');
|
||||
const fn = extractBlock(src, 'setStringColors(arr)');
|
||||
// Each provided index sets base + derived dim/bright; missing → default.
|
||||
@@ -110,7 +127,7 @@ test('app.js color manager name-maps to both highways, with identity no-op + bui
|
||||
// ── Executable: dim/bright derivation math ────────────────────────────────
|
||||
|
||||
function loadColorMath() {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const src = highwaySources();
|
||||
const snippet = [
|
||||
extractBlock(src, 'function _clampByte(n)'),
|
||||
extractBlock(src, 'function _parseHex(hex)'),
|
||||
|
||||
Reference in New Issue
Block a user