# XZBT Daily Code Review — 2026-09-06 (America/Los_Angeles)

## Review scope

- Local date/time (America/Los_Angeles, PDT): 2026-09-06 ~11:30. Branch: `main`. HEAD: `0af58da` (feat(visual): implement the slice 4d renderer core).
- Reviewed commits (all within LA 2026-09-06, per author dates): `1bc4901` docs(visual) sections 17-19 review triage @ rev 0.8; `6587d3e` schema/validator/resolution aligned to the 0.8 visual contract; `699492f` standalone-artifact bundle of contract+validation; `0af58da` slice 4d renderer core. Earlier same-session commits `3e27a66`…`527220e` are LA 2026-09-05 20:57-21:30 and were treated as the contract baseline, not reviewed work.
- Uncommitted changes: nothing staged; 17 tracked files modified (+3188/-119), ~35 untracked paths, including 9 new runtime modules (visual-noise/behaviors/distributions/fields/motion/systems/effects/automation/lifecycle), 3 new test files (phase4-procedural/execution/challenges), tools/visual-challenge-fixtures.mjs, tools/build-visual-acceptance.mjs, exhibits/exhibit-a..d.xzbt, prototypes/phase4/, docs/evidence/phase4/*.md, regenerated XZBT.html. This is the phase 4e-g layer.
- Dating of uncommitted work: file mtimes (2026-09-06 09:44-11:13 LA) and the self-dated evidence docs are consistent with a single this-morning session, and XZBT.html is byte-identical to a fresh build of current sources, so the tree is coherent; but git cannot strictly prove when each edit was made, and some files may have been touched over several hours.
- Scope limitations: commits were not checked out individually (no interference); everything was reviewed at HEAD + working tree. Canvas-pixel output was not observed on a real display; claims about rendering are from code reading plus the mock-context test oracles.

## Findings

### P1-1 — `morph` corrupts point `z` to NaN on z-less geometry
- File: `src/runtime/visual-behaviors.js:348` (untracked, phase 4e).
- Condition: any morph between geometries whose points omit `z` (the normal 2D case), ≥1 tick, rendered under `camera.projection: "perspective"`.
- Impact: `item.points[index].z += ((target[index].z ?? 0) - item.points[index].z) * amount` reads an undefined base (`undefined + …` → NaN); NaN then flows through `primitiveSubpaths` (visual-motion.js:73) into plan coordinates; under perspective the object renders NaN (invisible/poisoned), orthographic hides it, so the current suite (no perspective-morph case) is green. Reproduced by probe: morphed point `z` became NaN after 3 ticks.
- Fix: guard the base read as well: `item.points[index].z += ((target[index].z ?? 0) - (item.points[index].z ?? 0)) * amount;` (as `point-wander` already does at line 235).

### P1-2 — Velocity-accumulating behaviors are silently inert on graphic objects and repeater copies
- Files: `src/runtime/visual-motion.js:47` (graphic-object host runs `advanceItem` with a fake `{type:'repeater'}` context), `src/runtime/visual-systems.js:314-325` (integrator gated by `this.type !== 'repeater'`), `src/runtime/visual-behaviors.js:307-319, 332` (attract/repel/field-follow write `item.vx` only).
- Condition: spec 18.6 accumulating behaviors (`attract`, `repel`, `bounce`, `field-follow` force/velocity, `wander`) on a graphic object, graphic-system content, or repeater copy — validation accepts them (spec 18.6 attaches behaviors to objects/items; no host restriction enforced).
- Impact: the behavior accumulates velocity forever with no displacement: probe — graphic point with `attract {x:500,y:300}, strength:80` had `vx ≈ 0.235` but still sat at (0,0) after 2 s of ticks (the identical behavior moves particles); `drift` on the same host works, which masks the gap in fixtures. Silent, no diagnostic, violates 18.6 ("the integrator of 18.2 turns into displacement").
- Fix: integrate `vx/vy` for object/repeater hosts in `advanceVisualMotion`, or reject these behaviors on hosts without an integrator at semantic validation with the documented code.

### P1-3 — Per-item/system behavior arrays escape semantic validation (wrong stage; silent no-op channels)
- Files: `src/runtime/visual-validation.js:332-333` (systems: only the `≤ 8` count check) vs `:653`/`:789` (`validateBehaviors` invoked only for object hosts).
- Condition: any spec-18.9 semantic violation inside a `particles`/`emitter`/`repeater` `behaviors` array.
- Impact: 18.9/18.10 trace 12 requires `ERR_INVALID_BEHAVIOR_TYPE`, `ERR_INVALID_BEHAVIOR_TARGET`, `ERR_INVALID_REFERENCE`, `ERR_SCHEMA_VALIDATION` at import; instead probe: six cases (bogus type on particles/repeater, invalid property channel, undeclared field, point-wander on point render, follow-path speed+duration) all import clean. Unknown types then throw inside `createBehavior` at activation (whole-system failure at the wrong stage); invalid property channels (e.g. `style.hue`) write inert keys silently; undeclared fields throw per tick until `failSystem`.
- Fix: run the object-host `validateBehaviors` over each procedural system's `behaviors` at import with field context.

### P2-1 — Canvas 2D backend never consumes per-object buffer grants; 17.12 stage order realized per-op
- Files: `src/runtime/visual-canvas2d.js:88-96, 97-106, 227-241` (no read of `node.buffered` anywhere in src/runtime) vs `src/runtime/visual-engine.js:1171-1201` (allocates per-object buffers, sets `plan.buffered`, sheds beyond 16).
- Condition: any object/group with non-default `blend`, `blur`, `glow`, or `filters` inside an unbuffered layer (the common case).
- Impact: fill, stroke, glow, and filter ops each composite individually against the live backdrop under the node's blend/alpha instead of the object being rasterized once and composited once (spec 17.12 buffer table/stage order; "A buffer is an offscreen surface the renderer must allocate to compute a stage correctly"). Observable: double-blending on translucent/blended stroke-over-fill overlap and on glow under non-normal blend; grant/shed bookkeeping has no rendering effect (granted and shed nodes differ only by feature presence). Only the layer-buffer and mask paths get real isolation.
- Fix: when `node.buffered` is true, draw the node (subtree for groups) into a pool surface with default state, then composite once with the node's alpha/blend, mirroring the layer path at visual-canvas2d.js:234-241.

### P2-2 — Glow paints over the object interior and is silently dropped on text
- File: `src/runtime/visual-canvas2d.js:162-181`.
- Condition: `style.glow` on a point with a fill, on any strokable shape, or on text.
- Impact: 17.12 adds glow *around* the result; here the point disc is re-filled with `glow.color` (full-strength disc at alpha 1/strength 1 — a white point with red glow renders as a solid red disc), shapes are re-stroked on the identical path with glow color (`lineWidth = max(strokeWidth,1)`), overpainting the real stroke, and text gets no glow branch at all (silent, while the engine still grants text a buffer — visual-engine.js:901). Verified by code read.
- Fix: draw only the halo via a zero-offset blurred shadow of the original geometry without re-filling/re-stroking the path; apply the same to `fillText`/`strokeText` for text.

### P2-3 — Boolean leaves accept any literal; `visible: 0` renders visible, `enabled: "false"` executes
- Files: `src/runtime/visual-validation.js:233` (effect `enabled` and object/layer/system `visible` are only structure-validated as ValueSpecs); consumers use `=== false` semantics: `visual-engine.js:529` (effectPlan), `:643-645` (systemVisible), `:698` (layerValue).
- Condition: authoring `visible: 0`/`1`/`"no"` on any object/layer/system, or `enabled: 0`/`1`/`"false"` on an effect entry.
- Impact: spec section 2 bans coercion — "a non-zero number will not be coerced into a boolean. Type mismatches produce `ERR_TYPE_MISMATCH`". Probes: all of the above import VALID, and the runtime draws a `visible: 0` object (expectation: hidden) and executes an `enabled: "false"`/`enabled: 0` effect (exact `false` is the only skip). No diagnostic anywhere.
- Fix: type-check boolean leaves against the declared type at validation (`ERR_TYPE_MISMATCH`), i.e. apply the effect `color`-literal check pattern (visual-validation.js:244-247) to booleans.

### P2-4 — `face-motion` snaps on its first tick and reverts on zero velocity
- File: `src/runtime/visual-behaviors.js:279-283`.
- Condition: `face-motion` with any `smoothing`, including 1 ("never turns", spec 18.6).
- Impact: `const current = instance.state.rotation ?? target;` initializes the follow state to the *target*, so the first tick contributes the full turn regardless of smoothing (probe: base 45, +y velocity → rotation 90 on tick 1 for smoothing 0, 0.5 and 1); when XY speed drops below 1e-6 the behavior `break`s, the fresh rotation bucket was zeroed, and the object pops back to the authored rotation instead of holding the previous one (18.6: "An object with zero velocity holds its previous rotation").
- Fix: initialize `instance.state.rotation` lazily to the current drawn rotation on first tick; on the zero-velocity branch emit the held value.

### P2-5 — Trail `fade` is a flat uniform alpha; head never keeps the item's own opacity
- File: `src/runtime/visual-engine.js:1014` (points mode per-sample taper), `:1053` (ribbon), `:1064` (line).
- Condition: any trail in `line`/`ribbon` mode, or any trail on an item whose opacity ramps/fades.
- Impact: spec 18.8: "`fade` … Opacity multiplier at the tail; the head keeps the item's own opacity." Line/ribbon draw the whole trail at the flat tail factor and nothing multiplies by the item's ramped `opacityMultiplier`; probe: item at ramped opacity 0.81 rendered a full-strength head point but a line trail at flat alpha 0.4 (default fade 1.0 would leave the trail at 1.0 while the head fades to 0). Fading particles leave permanent-looking trails.
- Fix: per-vertex/per-segment alpha from head `itemOpacity` to tail `itemOpacity * fade` (segmented strokes), or state the uniform-color approximation with `WARN_VISUAL_APPROXIMATION`.

### P2-6 — Procedural system admission gaps at import: required fields and emission semantics deferred to activation
- Files: `src/runtime/visual-validation.js` (system per-type field checks, cf. `:332-333` region) vs `src/runtime/visual-systems.js:96-99` (`ERR_UNBOUNDED_EMISSION` raised at construction).
- Condition/probes (all import VALID): `particles` without `render` (spec 18.2 table: required), `emitter` without `emit` (spec 18.4: required), `particles` with `rate` and neither `lifetime` nor `limit`.
- Impact: spec 18.2/18.4 render/emit-required rows and the semantic-stage list (18.9) demand import-time rejection; the docs instead pass validation and only fail (or silently draw nothing, in the render-less case) at activation/instantiation — wrong stage, and one case is silent.
- Fix: enforce per-type required fields and the static half of the unbounded-emission rule at import (runtime check stays for stochastic rates).

### P2-7 — Resolved non-integer creation counts silently rounded
- File: `src/runtime/visual-systems.js:59, 63` (`Math.round` on resolved `count` and burst `count`).
- Condition: `count`/`burst[].count` authored as a ValueSpec resolving non-integer (e.g. `{random:{min:2.4,max:2.6}}`).
- Impact: spec 18.2: "an integer field whose resolved value is not an integer is `ERR_TYPE_MISMATCH` at that boundary rather than being rounded". Runtime rounds silently, so the same seed is reproducible but the authoring error is unmarked.
- Fix: after sampling, throw `RuntimeFault('ERR_TYPE_MISMATCH', …)` when `!Number.isInteger(resolved)` for `count` and each burst count.

### P2-8 — `ring` with equal start/end angles draws a visible radial spoke
- File: `src/runtime/visual-geometry.js:348-351`.
- Condition: `{type:'ring', startAngle: 90, endAngle: 90, …}` (validation accepts; `directedSweep` returns sweep 0).
- Impact: the partial-sector branch emits outer-point → `lineTo` inner-point → closed, which strokes as a straight radial line; spec 17.9: "`d == 0` draws nothing and raises no diagnostic" (the sibling `arc` primitive handles sweep 0 correctly via `arcSegments`).
- Fix: return no segments when `sweep === 0` (keep the full-annulus branch).

### P3 findings
1. Diagnostics `raised` list grows unbounded and is copied every frame — `src/runtime/visual-diagnostics.js:19, 39, 63` (push), `:45-52` (`endTick` prunes nothing), `src/runtime/visual-engine.js:734` (per-frame `slice()`). Slow memory/frame-cost growth over long sessions with sustained sheds/onces. Fix: clear or ring-buffer after the per-tick plan copy.
2. Degenerate path arcs: `src/runtime/visual-geometry.js:144-149, 153-186` — a path `arc` whose `to` equals the current point (`dx = dy = 0`) forces `acos(0)` and emits a phantom 90°/270° cubic bulge; and `Math.abs` is applied to the radius before the `≤ 0` throw, so a negative radius component (spec 17.13: `ERR_OUT_OF_BOUNDS`, "not silently degraded") draws a mirrored arc instead of failing. Fix: early-return on coincident endpoints; check the raw radii before `Math.abs`.
3. Masked-group rendering dereferences a null surface — `src/runtime/visual-canvas2d.js:97-106, 233` — `surface.create()` is unguarded; with no usable `createSurface` (Node/headless, no `document`/`OffscreenCanvas`; subsystem fallback at visual-subsystem.js:63-65 can return undefined) a granted masked group throws `TypeError` out of `renderFrame`, and the app treats any render exception as fatal: `src/runtime/app.js:42-49` calls `visual.deactivate()` with no reattach, permanently blacking visuals for the activation (a transient fault should not be terminal; audio faults only diagnose). Fix: guard/null-skip the mask with a warning; consider re-attaching visuals on a later frame.
4. Post-effect transient memory is unbounded at the backing ceiling — `src/runtime/visual-effects.js:6-39`: each blur pass allocates a full-frame copy + `Float32Array` + output (bloom adds a bright pass + glow blur); at the 4096-device-px ceiling this is on the order of several hundred MB per effect with no tie-in to the 19.5 pass budget. Fix: tile the blur or cap effect radius relative to the backing store.
5. `window-traffic` challenge fixture: per-copy `wrap` bounds (tools/visual-challenge-fixtures.mjs, ~lines 56-60) are ±30 px around each copy's own origin, so cars cycle in place every ~3 s and never cross the window — the authored bounds appear intended to be the window rect. The 4g suite only checks finiteness/equality, so this passes. Hedged: if cell-stutter was intended it should be documented.
6. `schema/xzbt-0.1.schema.json` (visual additions, committed in 6587d3e) has no consumer anywhere in src/tools/tests — the hand-written validator is the sole admission authority, and the schema already diverges from it (schema marks per-system fields like `render`/`emit` required that the validator admits without, cf. P2-6), so the schema can silently rot. Consider validating the exhibit fixtures against it in CI. (Schema itself parses cleanly: no duplicate keys; all 191 `$ref`s resolve.)

## Verification performed

- Full suite in a temp copy of the repo (`node --test test/*.test.mjs`, node v24.11.0): 212/212 pass, 0 fail; Phase 4 = exactly 109 tests (renderer 22, visual-contract 29, procedural 19, execution 20, challenges 19) + gc2-runner's single test covering 12 fixtures → evidence's "223 checks" accounting (212 tests + 11 extra counted GC2 fixture sub-checks) is consistent.
- Artifact integrity: two clean `npm run build` runs are byte-identical and identical to the working-tree `XZBT.html` (SHA-256 `46619d8ff2fbadf188136409d5cfe658bf5492ba0ba111d58ad488e95b343770`); `npm run build:visual-acceptance` output matches the evidence hash `42981219…db74` (18 fixtures embedded). Evidence claims in docs/evidence/phase4/2026-09-06-phase4f-execution.md verified. `git diff --check` clean; nothing staged.
- `python tools/verify-spec-contract.py`: 0 unresolved cross-references, fences/tables balanced, 46 codes declared / 43 used (3 declared-only: ERR_INVALID_ARITY, ERR_INVALID_OPERATOR, ERR_UNSUPPORTED_VERSION).
- Independent probes (node, read-only): attract-on-graphic inertness (P1-2); six-case behavior-array import acceptance (P1-3); `visible:0`/`enabled:0|1|"false"` accepted (P2-3); particles/emitter/repeater required-field admission (P2-6); morph NaN via code trace (P1-1); plus positive controls: audio-bus draws are bit-identical with and without visual effects blocks; both automation call sites use consistent ms time bases; 120-tick runs at 1/2/4 draws/frame agree (execution suite).
- Clean areas (verified by direct reading/tests): math/geometry conventions and sweep normalization; layer ordering and buffer-refusal order (nearest-first grants, farthest-first refusals); fog/gradient/alpha arithmetic; determinism story (keyed RNG substreams, no Math.random/Date/performance.now in the visual path); effect pass budget and per-effect warn routing; build determinism; phase1 bundler top-level-identifier collision scan is a good net for the concatenated artifact.
- Not verified: per-commit isolation of the four reviewed commits; real-browser pixel output, post-effect numerics on a real surface, and hardware performance (evidence doc itself disclaims these); memory footprint measurements; exact provenance/dating of each uncommitted edit.

## Testing gaps (relevant to the findings)

No test covers: perspective-projected morph (P1-1); velocity behaviors on graphic/repeater hosts (P1-2); semantic-validation cases on per-item behavior arrays (P1-3); backend rendering of buffered nodes, glow, blur, filters, or masked groups through a real surface pool (P2-1, P2-2, P3.3); zero-sweep ring / degenerate path arcs (P2-8, P3.2); typed-boolean leaves (P2-3); face-motion smoothing/zero-velocity (P2-4); trail head-opacity/fade taper in line/ribbon (P2-5); non-integer resolved counts (P2-7); long-run diagnostics growth (P3.1); post-effect runs against real readback (P3.4). phase4-procedural/execution trace coverage is strong where it operates (draw counts, closed-form motion, budgets, ceilings, cadence).