test(h3d-carve-8): fix Toby r1 findings — _applyBloom extract + test 7 vacuity

F1 (MED — declared gap confirmed and widened):
  Extract _bloomEnsure's .then() body into named _applyBloom([EC, RP, UB, OP])
  at factory scope. _applyBloom added to return set and screen.js destructure.
  Test 9 calls _applyBloom directly with mock module objects (no import() needed).
  All 4 write-back mutations now RED:
    sever setBloomPass(bp)    → RED
    sever setBloomW(w)        → RED
    sever setBloomH(h)        → RED
    sever setComposer(comp)   → RED

F2 (MED — vacuous OR clause in test 7):
  Old:
       The  arm is always true (no living sparks) → NO-OP passes.
  Fix: replaced with independent AND assertions on BOTH needsUpdate flags.
  NO-OP mutation (return; at top of _sparkUpdate) now goes RED.

DI beyond-subst count +5: _applyBloom reads T/ren/scene/cam/highwayCanvas
from live-accessors (instead of inheriting _bloomEnsure's locals).
fx.js return set: 6 → 7 symbols (added _applyBloom).
Suite: 1267/1269 (2 pre-existing failures unchanged).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014uZ169yfoFYArXz962g7KW
This commit is contained in:
byrongamatos
2026-09-05 15:51:42 +02:00
co-authored by Claude Sonnet 4.6
parent 9b98d78cbe
commit d060c49238
3 changed files with 100 additions and 42 deletions
+1 -1
View File
@@ -6381,7 +6381,7 @@ import { createFx } from './src/fx.js'; // h3d-carve-8
/* ── h3d-carve-8: Q-helpers (lighting/FX) → src/fx.js ─────────── */
/* buildBoard deferred: see plans/highway3d-carve.md §3 row 16. */
const { _h3dHexOrDefault, _applyCinematic, _timingHex, _sparkBurst, _sparkUpdate, _bloomEnsure } = createFx({
const { _h3dHexOrDefault, _applyCinematic, _timingHex, _sparkBurst, _sparkUpdate, _applyBloom, _bloomEnsure } = createFx({
BG_DEFAULTS, K,
getT: () => T,
getAmbLight: () => ambLight, getDirLight: () => dirLight, getCinematic: () => _cinematic,
+30 -23
View File
@@ -99,44 +99,51 @@ export function createFx({
_sparkPts.geometry.attributes.color.needsUpdate = true;
_sparkPts.visible = any;
}
// #4 Bloom — .then() body extracted for test harness reach (Toby r1 F1).
// Called with [EC, RP, UB, OP] when all four postprocessing modules resolve.
// DI rewire: T/ren/scene/cam/highwayCanvas from live-accessors (per-call).
function _applyBloom([EC, RP, UB, OP]) {
// VERBATIM MOVE of the .then() handler body from _bloomEnsure.
// DI rewire: T/ren/scene/cam/highwayCanvas read from live-accessors.
try {
const T = getT(), highwayCanvas = getHighwayCanvas();
const ren = getRen(), scene = getScene(), cam = getCam();
const sz = canvasSize(highwayCanvas) || { w: 1280, h: 720 };
const w = Math.max(2, sz.w | 0), h = Math.max(2, sz.h | 0);
// Multisampled (WebGL2 MSAA) HalfFloat target so anti-aliasing
// survives the bloom path — EffectComposer's default target has no
// `samples`, which is why bloom-on looked jagged (worst on non-Retina
// DPR1 displays that have no supersampling cushion).
const _bloomRT = new T.WebGLRenderTarget(w, h, { type: T.HalfFloatType, samples: 4 });
const comp = new EC.EffectComposer(ren, _bloomRT);
comp.addPass(new RP.RenderPass(scene, cam));
const bp = new UB.UnrealBloomPass(new T.Vector2(w, h), 0.65, 0.5, 0.82); // strength, radius, threshold (high → only emissive blooms)
setBloomPass(bp);
comp.addPass(bp);
comp.addPass(new OP.OutputPass());
comp.setSize(w, h);
setBloomW(w); setBloomH(h); setComposer(comp);
} catch (e) { console.warn('[3D-Hwy] bloom init failed', e); setComposer(null); }
}
// #4 Bloom: lazy-load the vendored postprocessing addons and build an
// EffectComposer (RenderPass -> UnrealBloomPass -> OutputPass/ACES). Returns
// the composer once ready, or null (caller falls back to a direct render).
function _bloomEnsure() {
// VERBATIM MOVE. DI rewire: all factory-scope vars via get/set accessors.
// _composer, _bloomLoad, _bloomPass, _bloomW, _bloomH are REASSIGNED here
// so setters are required — these must NOT silently become locals.
// _composer, _bloomLoad, _bloomPass, _bloomW, _bloomH are REASSIGNED via
// _applyBloom — setters required; must NOT silently become locals.
if (getComposer()) return getComposer();
const ren = getRen(), scene = getScene(), cam = getCam();
if (getBloomLoad() || !ren || !scene || !cam) return null;
const T = getT(), highwayCanvas = getHighwayCanvas();
const A = '/static/vendor/three/addons/';
setBloomLoad(Promise.all([
import(A + 'postprocessing/EffectComposer.js'),
import(A + 'postprocessing/RenderPass.js'),
import(A + 'postprocessing/UnrealBloomPass.js'),
import(A + 'postprocessing/OutputPass.js'),
]).then(([EC, RP, UB, OP]) => {
try {
const sz = canvasSize(highwayCanvas) || { w: 1280, h: 720 };
const w = Math.max(2, sz.w | 0), h = Math.max(2, sz.h | 0);
// Multisampled (WebGL2 MSAA) HalfFloat target so anti-aliasing
// survives the bloom path — EffectComposer's default target has no
// `samples`, which is why bloom-on looked jagged (worst on non-Retina
// DPR1 displays that have no supersampling cushion).
const _bloomRT = new T.WebGLRenderTarget(w, h, { type: T.HalfFloatType, samples: 4 });
const comp = new EC.EffectComposer(ren, _bloomRT);
comp.addPass(new RP.RenderPass(scene, cam));
const bp = new UB.UnrealBloomPass(new T.Vector2(w, h), 0.65, 0.5, 0.82); // strength, radius, threshold (high → only emissive blooms)
setBloomPass(bp);
comp.addPass(bp);
comp.addPass(new OP.OutputPass());
comp.setSize(w, h);
setBloomW(w); setBloomH(h); setComposer(comp);
} catch (e) { console.warn('[3D-Hwy] bloom init failed', e); setComposer(null); }
}).catch((e) => console.warn('[3D-Hwy] bloom modules failed', e)));
]).then(_applyBloom).catch((e) => console.warn('[3D-Hwy] bloom modules failed', e)));
return null;
}
return { _h3dHexOrDefault, _applyCinematic, _timingHex, _sparkBurst, _sparkUpdate, _bloomEnsure };
return { _h3dHexOrDefault, _applyCinematic, _timingHex, _sparkBurst, _sparkUpdate, _applyBloom, _bloomEnsure };
}
+69 -18
View File
@@ -3,17 +3,18 @@
//
// Class-killers guaranteed:
// 1. Module exports createFx (source)
// 2. createFx return set covers all 6 required symbols (source)
// 2. createFx return set covers all 7 required symbols (source)
// 3. Stranded-caller: every returned symbol in screen.js destructure (source)
// 4. Private-guard: factory-depth-1 privates not bare in screen.js (source)
// 5. _timingHex returns EARLY tint when ts='EARLY' and _timingFx truthy (behavioural)
// 6. _sparkBurst writes to getSparkPos() array NOT a stale init-time capture
// (live-accessor class-killer: set new arrays after factory init → RED if stale-cached)
// 7. _sparkUpdate sets .visible on getSparkPts() object (live-accessor)
// 7. _sparkUpdate sets needsUpdate=true on pts reached via live getSparkPts()
// (F2 fix: AND assertions; no longer vacuously true via .visible === false)
// 8. _bloomEnsure calls setBloomLoad (synchronous write-back: assignment silenced → RED)
// 8b. _bloomEnsure reads getComposer() live (not init-cached: setComposer after init → RED)
// GAP: setComposer(comp) inside the async .then() is not tested synchronously —
// would require mocking dynamic import(); declared to Toby/Creed for assessment.
// 9. _applyBloom calls all 4 write-backs (setBloomPass/setBloomW/setBloomH/setComposer)
// (F1 fix: named function testable with mock modules; each sever → RED)
const { test } = require('node:test');
const assert = require('node:assert/strict');
@@ -39,7 +40,7 @@ test('fx.js exports createFx', () => {
// ── 2. Return set covers all 6 required symbols ──────────────────────────────
test('createFx returns all 6 required symbols', () => {
const stripped = stripComments(src());
const REQUIRED = ['_h3dHexOrDefault', '_applyCinematic', '_timingHex', '_sparkBurst', '_sparkUpdate', '_bloomEnsure'];
const REQUIRED = ['_h3dHexOrDefault', '_applyCinematic', '_timingHex', '_sparkBurst', '_sparkUpdate', '_applyBloom', '_bloomEnsure'];
// Factory-level return block: 4-space indent inside createFx body.
const retMatch = stripped.match(/\n {4}return\s*\{\s*([^}]+)\}/);
assert.ok(retMatch, 'factory-level return block must be present');
@@ -184,32 +185,38 @@ test('_sparkBurst writes to the sparkPos array returned by getSparkPos (live, no
'if 0 it cached the initial array at factory init time (live-accessor broken)');
});
// ── 7. _sparkUpdate live-accessor class-killer ────────────────────────────────
test('_sparkUpdate reads sparkPts from getter each call (not init-cached)', () => {
// Mutation: if getSparkPts() result is cached at factory init, setSparkPts(newPts)
// after init → _sparkUpdate still references stale (null) pts → .visible never set.
// ── 7. _sparkUpdate live-accessor class-killer (F2 fix) ───────────────────────
test('_sparkUpdate sets needsUpdate=true on pts reached via live getSparkPts()', () => {
// Mutation that goes RED: if getSparkPts() result is cached at factory init,
// setSparkPts(newPts) after init → _sparkUpdate still references stale (null) pts
// → needsUpdate flags never set → BOTH assertions fail → RED.
//
// F2 fix: previous test used `|| pts.visible === false` which is vacuously true
// (no living sparks → visible stays false) — a NO-OP _sparkUpdate passed the test.
// Now we assert needsUpdate=true (set unconditionally after the loop) via AND.
const di = makeDi();
const { _sparkUpdate } = loadFxModule(di);
// No sparkPts yet — _sparkUpdate should short-circuit.
_sparkUpdate(0.016); // must not throw
// No sparkPts yet — short-circuit (must not throw).
_sparkUpdate(0.016);
// Now provide sparkPts (simulates buildBoard completing).
// Now provide sparkPts (simulates buildBoard completing after factory init).
const pts = {
geometry: { attributes: {
position: { needsUpdate: false },
color: { needsUpdate: false },
}},
visible: false,
visible: true,
};
di.setSparkPts(pts);
_sparkUpdate(0.016);
// Even with no living sparks, the needsUpdate flags must have been set.
assert.ok(pts.geometry.attributes.position.needsUpdate === true ||
pts.geometry.attributes.color.needsUpdate === true ||
pts.visible === false,
'_sparkUpdate must reach the pts object provided via setSparkPts after factory init');
// _sparkUpdate sets needsUpdate unconditionally after the particle loop.
// If _sparkUpdate cached the stale null ptr at factory init, both stay false.
assert.ok(pts.geometry.attributes.position.needsUpdate === true,
'_sparkUpdate must set position.needsUpdate=true (reached via live getSparkPts)');
assert.ok(pts.geometry.attributes.color.needsUpdate === true,
'_sparkUpdate must set color.needsUpdate=true (reached via live getSparkPts)');
});
// ── 8. _bloomEnsure write-back class-killer: setBloomLoad called ──────────────
@@ -251,3 +258,47 @@ test('_bloomEnsure returns composer set via setComposer (reads live via getCompo
'_bloomEnsure must return composer set via setComposer; ' +
'null means it cached the initial null value at factory init time');
});
// ── 9. _applyBloom calls all 4 write-backs (F1 fix) ──────────────────────────
test('_applyBloom calls setBloomPass/setBloomW/setBloomH/setComposer with mock modules', () => {
// Mutation scenarios (all 4 must go RED when severed individually):
// sever setBloomPass(bp) → getBloomPass() stays null → RED
// sever setBloomW(w) → getBloomW() stays 0 → RED
// sever setBloomH(h) → getBloomH() stays 0 → RED
// sever setComposer(comp)→ getComposer() stays null → RED
//
// F1 Toby fix: _applyBloom is named at factory scope and in the return set,
// so the harness calls it directly with mock [EC, RP, UB, OP] — no import() needed.
const di = makeDi();
di._state.T = {
WebGLRenderTarget: function(w, h, opts) { return { _w: w, _h: h }; },
HalfFloatType: 1,
Vector2: function(w, h) { return { w, h }; },
};
di._state.ren = { isRenderer: true };
di._state.scene = { isScene: true };
di._state.cam = { isCamera: true };
const { _applyBloom } = loadFxModule(di);
// Mock module objects matching the destructure [EC, RP, UB, OP].
const fakeComp = { addPass() {}, setSize() {} };
const fakePass = { isBloomPass: true };
const mods = [
{ EffectComposer: function(ren, rt) { return fakeComp; } },
{ RenderPass: function(scene, cam) { return {}; } },
{ UnrealBloomPass: function(v2, s, r, t) { return fakePass; } },
{ OutputPass: function() { return {}; } },
];
_applyBloom(mods);
assert.equal(di.getComposer(), fakeComp,
'setComposer must be called: if severed, getComposer() stays null → bloom never activates');
assert.equal(di.getBloomPass(), fakePass,
'setBloomPass must be called: if severed, pass ref lost → resize/tuning broken');
assert.ok(di.getBloomW() > 0,
'setBloomW must be called with positive width');
assert.ok(di.getBloomH() > 0,
'setBloomH must be called with positive height');
});