From 75e0e580670015d43f53a4ad25805edd05722d53 Mon Sep 17 00:00:00 2001 From: monkey-w1n5t0n Date: Tue, 21 Jul 2026 13:22:38 +0200 Subject: [PATCH] fix(manifold): full-width input vector, dropped switches, MIDI churn, re-subscribe churn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 (S10, L19, L18, L24). - S10: EngineApi.inputVector() returned a freshly allocated [lastRawX, lastRawY] on every spine tick, so VCV bridged mode silently truncated gamepad/MIDI input to 2-D while the spine already held the full N-dim raw vector. It now returns spine.lastRawInputs (ArrayLike, documented as a live reused buffer — copy, don't retain; VcvBackend already copies), and VcvBackend tracks and dead-zones the full length. Audit correction: "32-input head" is not a constant — 32 is DEFAULT_MODE_ML.inputSize, the over-provisioned default before any mode is chosen; real per-mode widths come from the schemas. - L19: BackendManager.setActive silently dropped a switch requested while another was in flight. Now stores the latest requested id and re-runs it in the finally block (latest-caller-wins). - L18: MIDI CC messages triggered a React state update plus a snapshot allocation each. notifyBindings now fires only when the binding LIST changes. - L24: two ConsoleApp global-listener effects had no dependency array and so re-subscribed on every render, including every pointer frame. Both now read through a single ref assigned in the render body, matching the existing onMoveRef pattern. Audit correction: its suggested `[inputs]` dep would not have worked — useInputLayer returns a fresh object literal each call, so that dep changes every render too. Regression tests: input-vector-truncation.test.ts, backend-manager-switch.test.ts (a fake backend whose start() is held open, to make the in-flight switch real), midi-notify-churn.test.ts (fail-before confirmed: 51 notifications vs 1). L24 has no test — this repo has no DOM render harness to count re-subscriptions against a mounted component; verified by reading and reference-stability tracing. ALSO: manifold/package.json's test script named its test files explicitly ("bun test src tests/pipeline-golden.test.ts"), so the three new files were not run by `bun run test` or CI — regression tests that never execute. Now a glob. Deliberately `tests/*.test.ts` rather than `tests`: bun's discovery matches *.spec.ts too, which would drag the Playwright e2e specs into the unit run (verified — it fails). Unit tests go 9 -> 17. Gates: run-all-tests.sh ALL GREEN. --- manifold/package.json | 2 +- manifold/src/backends/manager.ts | 40 ++++-- manifold/src/backends/vcv-backend.ts | 34 +++-- manifold/src/console/ConsoleApp.tsx | 75 +++++++--- manifold/src/engine/engine-api.ts | 11 +- manifold/src/engine/spine.ts | 10 +- manifold/src/inputs/midi-input-source.ts | 16 ++- manifold/tests/backend-manager-switch.test.ts | 130 +++++++++++++++++ .../tests/input-vector-truncation.test.ts | 132 ++++++++++++++++++ manifold/tests/midi-notify-churn.test.ts | 86 ++++++++++++ 10 files changed, 491 insertions(+), 45 deletions(-) create mode 100644 manifold/tests/backend-manager-switch.test.ts create mode 100644 manifold/tests/input-vector-truncation.test.ts create mode 100644 manifold/tests/midi-notify-churn.test.ts diff --git a/manifold/package.json b/manifold/package.json index 524fb90..13f20cd 100644 --- a/manifold/package.json +++ b/manifold/package.json @@ -9,7 +9,7 @@ "build": "tsc --noEmit && vite build", "preview": "vite preview --port 4273", "typecheck": "tsc --noEmit", - "test": "bun test src tests/pipeline-golden.test.ts", + "test": "bun test src tests/*.test.ts", "test:e2e": "playwright test", "test:e2e:headed": "playwright test --headed" }, diff --git a/manifold/src/backends/manager.ts b/manifold/src/backends/manager.ts index fe9e7c6..3d99dfb 100644 --- a/manifold/src/backends/manager.ts +++ b/manifold/src/backends/manager.ts @@ -31,11 +31,13 @@ export interface ManagerEngine { routedOutput(): Float32Array | null; audio: { setMuted(muted: boolean): void }; /** - * Current control input vector (2-D for the fixed 2→N MLP). Optional — when - * present the VCV backend streams it to the module so the browser drives the - * module's inputs in bridged mode. + * Current FULL-width control input vector (one entry per active input axis + * — NOT fixed at 2-D). Optional — when present the VCV backend streams it + * to the module so the browser drives the module's inputs in bridged mode. + * May be a live reused buffer (ArrayLike) — the manager copies it straight + * through to `VcvBackend.setInputVector`, which itself copies. */ - inputVector?(): ReadonlyArray; + inputVector?(): ArrayLike; } export class BackendManager { @@ -46,6 +48,11 @@ export class BackendManager { private ctx: BackendContext | null = null; private unsub: (() => void) | null = null; private switching = false; + /** Latest id requested while a switch was already in flight (simplification + * audit L19) — re-run once the in-flight switch settles, so a rapid double + * click no longer silently drops the second request. Cleared before the + * re-run so it can only ever chain one hop at a time (no unbounded loop). */ + private pendingId: BackendId | null = null; private statusListeners = new Set<(id: BackendId, s: BackendStatus) => void>(); private offBackendStatus: (() => void) | null = null; @@ -126,19 +133,24 @@ export class BackendManager { /** * Switch the active backend. Tears down the old, starts the new, and applies * the synth audio gate. Idempotent for the same id. + * + * If a switch is already in flight, the request is NOT dropped: it is + * remembered as `pendingId` (the latest caller wins) and re-run once the + * in-flight switch's `finally` settles — see below. */ async setActive(id: BackendId): Promise { - if (this.activeId === id || this.switching) return; + if (this.activeId === id) return; + if (this.switching) { + this.pendingId = id; + return; + } this.switching = true; try { // Gate audio: only the synth mode drives sound. this.engine.audio.setMuted(id !== 'synth'); const next = this.backends.get(id); - if (!next) { - this.switching = false; - return; - } + if (!next) return; if (this.active) { this.offBackendStatus?.(); this.offBackendStatus = null; @@ -154,6 +166,16 @@ export class BackendManager { this.emitStatus(id, next.status()); } finally { this.switching = false; + // A newer request arrived mid-switch — chase it. Clearing `pendingId` + // BEFORE the recursive call (rather than in it) bounds this to one hop + // per settled switch: the only way to chain further is another NEW + // request arriving during that hop, which is the intended behaviour, + // not an infinite loop. + const pending = this.pendingId; + this.pendingId = null; + if (pending !== null && pending !== this.activeId) { + void this.setActive(pending); + } } } diff --git a/manifold/src/backends/vcv-backend.ts b/manifold/src/backends/vcv-backend.ts index 98aa023..ce05e50 100644 --- a/manifold/src/backends/vcv-backend.ts +++ b/manifold/src/backends/vcv-backend.ts @@ -9,7 +9,9 @@ * manifold/osc-bridge, default ws://localhost:8765, default module UDP 7001): * * browser → module - * /nisps/input the current 2-D input vector (drives the module) + * /nisps/input the current input vector, full net input + * arity (drives the module — NOT truncated to + * 2-D; simplification audit S10) * /nisps/output per-output values (CV) — sent as params batch so * the bridge maps each to /nisps/; the module * also derives its own outputs, but the browser @@ -62,13 +64,18 @@ export class VcvBackend implements OutputBackend { private specs: VcvSpec[] = []; private sendRaw = false; - /** Latest input vector the browser is driving the module with (2-D). */ + /** Latest input vector the browser is driving the module with (N-D — the + * net's full input arity, not fixed at 2; simplification audit S10). */ private inputVec: number[] = [0.5, 0.5]; private lastSent: Float32Array = new Float32Array(0); // last normalised output private batch: Array<[string, number]> = []; // reused outer; entries reused private lastSendMs = 0; - private lastInputSent: [number, number] = [-1, -1]; + /** Dead-zone sentinel per input axis, sized to `inputVec` (out-of-range -1 + * forces the first send). Tracks the FULL vector, not just the first two + * axes, so a change in axis 2+ (gamepad/MIDI beyond the XY pair) still + * triggers a resend instead of being silently swallowed. */ + private lastInputSent: number[] = []; private gotModuleReply = false; @@ -146,7 +153,7 @@ export class VcvBackend implements OutputBackend { * Set the input vector the browser drives the module with (bridged mode). The * next `send()` streams it to /nisps/input. Copied — caller may mutate. */ - setInputVector(vec: ReadonlyArray): void { + setInputVector(vec: ArrayLike): void { if (this.inputVec.length !== vec.length) this.inputVec = new Array(vec.length); for (let i = 0; i < vec.length; i++) this.inputVec[i] = vec[i]; } @@ -171,11 +178,20 @@ export class VcvBackend implements OutputBackend { this.lastSendMs = now; // 1) Stream the current input vector so the browser drives the module. - const ix = this.inputVec[0] ?? 0.5; - const iy = this.inputVec[1] ?? 0.5; - if (Math.abs(ix - this.lastInputSent[0]) >= DEAD_ZONE || Math.abs(iy - this.lastInputSent[1]) >= DEAD_ZONE) { - this.lastInputSent[0] = ix; - this.lastInputSent[1] = iy; + // Dead-zone over the FULL vector — any axis moving (not just the first + // two) triggers a resend (simplification audit S10). + if (this.lastInputSent.length !== this.inputVec.length) { + this.lastInputSent = new Array(this.inputVec.length).fill(-1); + } + let inputChanged = false; + for (let i = 0; i < this.inputVec.length; i++) { + if (Math.abs(this.inputVec[i] - this.lastInputSent[i]) >= DEAD_ZONE) { + inputChanged = true; + break; + } + } + if (inputChanged) { + for (let i = 0; i < this.inputVec.length; i++) this.lastInputSent[i] = this.inputVec[i]; // → bridge `input` verb → ONE multi-float message to /nisps/input. this.client.sendInput(this.inputVec); } diff --git a/manifold/src/console/ConsoleApp.tsx b/manifold/src/console/ConsoleApp.tsx index 6149ef0..983d25c 100644 --- a/manifold/src/console/ConsoleApp.tsx +++ b/manifold/src/console/ConsoleApp.tsx @@ -593,6 +593,40 @@ export function ConsoleApp() { const setParam = (i: number, patch: Partial) => setParams((ps) => ps.map((p, j) => (j === i ? { ...p, ...patch } : p))); + // Ref-mirror of everything the two global-listener effects below close over + // that is NOT already React-stable (verdict/navigation handlers are plain + // consts re-created every render; `pos`/`inputs.inputMode` are per-render + // values). Assigned directly in the render body — same technique as + // `onMoveRef` in Manifold.tsx — so both effects can install their + // subscriptions ONCE ([] deps) instead of tearing down + re-subscribing on + // EVERY render (previously: no dep array at all, so both effects re-ran on + // every render — including every pointer frame `onReducedInput`/`setPos` + // drive; simplification audit L24). `setActive`/`setDepth`/`setSplit`/ + // `setPos` are `useState` setters, which React guarantees are stable, so + // they're read directly and don't need mirroring here. + const liveRef = useRef({ + perturb, + commit, + undo, + reroll, + onScratchNudge, + onPlace, + onPickLocation, + pos, + inputMode: inputs.inputMode, + }); + liveRef.current = { + perturb, + commit, + undo, + reroll, + onScratchNudge, + onPlace, + onPickLocation, + pos, + inputMode: inputs.inputMode, + }; + // keyboard accelerators useEffect(() => { const onKey = (e: KeyboardEvent) => { @@ -607,10 +641,10 @@ export function ConsoleApp() { }; if (e.key === '1') { e.preventDefault(); - perturb(); + liveRef.current.perturb(); } else if (e.key === '2') { e.preventDefault(); - commit(); + liveRef.current.commit(); } else if (map[e.key]) { setActive((a) => (a === map[e.key] ? null : map[e.key])); setDepth('condensed'); @@ -626,15 +660,15 @@ export function ConsoleApp() { setSplit(0.5); } else if (e.key === ' ' || e.key === 'ArrowUp') { e.preventDefault(); - commit(); + liveRef.current.commit(); } else if (e.key === 'ArrowDown') { e.preventDefault(); - perturb(); - } else if (e.key.toLowerCase() === 'z') undo(); + liveRef.current.perturb(); + } else if (e.key.toLowerCase() === 'z') liveRef.current.undo(); }; window.addEventListener('keydown', onKey); return () => window.removeEventListener('keydown', onKey); - }); + }, []); // ---- Game-controller verdict bindings (inputs-spec) --------------------- // The gamepad's sticks already feed the input layer (→ engine); its BUTTONS @@ -644,50 +678,57 @@ export function ConsoleApp() { // (hold A, move the stick to a spot on the manifold, release to drop it). // MIDI note actions are surfaced too but left unbound (MIDI mode learns CCs // as INPUT axes; verdicts there stay on the on-screen / keyboard controls). - // The effect has no dep array (matching the keydown handler above) so each - // binding closes over the latest verdict functions + live `pos`. + // Deps are the two subscribe methods, not `inputs` itself: `inputs` is a + // fresh object literal every render (useInputLayer doesn't memoize its + // return value), so depending on the whole object would reintroduce the + // exact per-render resubscribe churn this fix removes. `onAction`/ + // `onReducedInput` ARE stable (useCallback over a ref-held layer created + // once), so this genuinely installs once; everything that DOES vary + // per-render is read fresh through `liveRef` above. useEffect(() => { const unBtn = inputs.onAction((a) => { if (a.source !== 'gamepad') return; const phase = a.phase ?? 'press'; + const live = liveRef.current; if (phase === 'press') { switch (a.id) { case 'button:4': // LB → thumbs-down - perturb(); + live.perturb(); break; case 'button:5': // RB → thumbs-up - commit(); + live.commit(); break; case 'button:2': // X → randomise / re-roll - reroll(); + live.reroll(); break; case 'button:3': // Y → nudge (scratchpad) - onScratchNudge(); + live.onScratchNudge(); break; case 'button:1': // B → undo - undo(); + live.undo(); break; case 'button:0': // A (down) → begin repositioning an example - onPlace(); + live.onPlace(); break; } } else if (phase === 'release' && a.id === 'button:0') { // A (up) → drop the example at the current (stick-driven) location. - onPickLocation(pos[0], pos[1]); + live.onPickLocation(live.pos[0], live.pos[1]); } }); // Mirror the composed gamepad/MIDI position onto the on-screen manifold so // markers + readouts track the controller (the XY pad pushes its own pos). // The callback fires every rAF frame — only re-render when it actually moves. const unPos = inputs.onReducedInput((x, y) => { - if (inputs.inputMode === 'internal') return; + if (liveRef.current.inputMode === 'internal') return; setPos((prev) => (Math.abs(prev[0] - x) < 1e-3 && Math.abs(prev[1] - y) < 1e-3 ? prev : [x, y])); }); return () => { unBtn(); unPos(); }; - }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [inputs.onAction, inputs.onReducedInput]); const onToggleAudio = () => { if (!engine) return; diff --git a/manifold/src/engine/engine-api.ts b/manifold/src/engine/engine-api.ts index 22c5d39..811e1f4 100644 --- a/manifold/src/engine/engine-api.ts +++ b/manifold/src/engine/engine-api.ts @@ -241,11 +241,14 @@ export class EngineApi { } /** - * Current control input vector (2-D for the fixed 2→N MLP). Used by the VCV - * backend (via BackendManager) to drive the module's inputs over the bridge. + * Current FULL-width control input vector (one entry per active input axis, + * up to the net's input arity — NOT fixed at 2). Live reused buffer — copy, + * don't retain. Used by the VCV backend (via BackendManager) to drive the + * module's inputs over the bridge without truncating gamepad/MIDI axes + * beyond the first two (simplification audit S10). */ - inputVector(): ReadonlyArray { - return [this.spine.lastRawX, this.spine.lastRawY]; + inputVector(): ArrayLike { + return this.spine.lastRawInputs; } /** diff --git a/manifold/src/engine/spine.ts b/manifold/src/engine/spine.ts index f3d7219..b50f604 100644 --- a/manifold/src/engine/spine.ts +++ b/manifold/src/engine/spine.ts @@ -88,8 +88,14 @@ export class Spine implements EngineSink { // Last raw input, so `EngineApi.process()` can re-tick after a weight change. lastRawX = 0.5; lastRawY = 0.5; - // Full last raw input vector (N-D) for re-ticking without losing extra axes. - private lastRawInputs: Float32Array = new Float32Array(2); + /** + * Full last raw input vector (N-D), so `process()`/`reprocess()` can re-tick + * without losing extra axes. Public + reused (live buffer — copy, don't + * retain): this is what `EngineApi.inputVector()` hands to the VCV backend + * so bridged mode drives the module's FULL input arity instead of the + * fixed 2-D pair `lastRawX`/`lastRawY` cover (simplification audit S10). + */ + lastRawInputs: Float32Array = new Float32Array(2); private mlBuf: F32 = new Float32Array(126); private routedBuf: F32 | null = null; diff --git a/manifold/src/inputs/midi-input-source.ts b/manifold/src/inputs/midi-input-source.ts index 05984ed..f5e29fd 100644 --- a/manifold/src/inputs/midi-input-source.ts +++ b/manifold/src/inputs/midi-input-source.ts @@ -216,6 +216,14 @@ export class WebMidiInputSource extends BaseSource { * Route an incoming CC: update a matching binding's value, or — if batch * learn is armed — capture it as a NEW axis (deduped). Learn stays armed so * the user can sweep their whole control surface in one pass. + * + * `notifyBindings()` fires ONLY when the binding LIST changes (a new axis + * captured here, or `clearBinding`/`clearAllBindings`) — NOT on every value + * update, which used to push a React state update + a fresh array snapshot + * per incoming CC message (simplification audit L18). `sample()` already + * pulls live values every input-layer tick; a consumer that wants a live + * per-binding readout (e.g. a meter) should read `getBindings()` on demand + * (poll/rAF) rather than rely on a notify-per-value-change contract. */ private handleCc(number: number, channel: number, value: number): void { const existing = this.bindings.find( @@ -223,7 +231,6 @@ export class WebMidiInputSource extends BaseSource { ); if (existing) { existing.value = value; - this.notifyBindings(); return; } if (this.learnArmed) { @@ -232,14 +239,17 @@ export class WebMidiInputSource extends BaseSource { } } - /** Update a learned note axis's gate value (note bindings are not auto-learned). */ + /** + * Update a learned note axis's gate value (note bindings are not + * auto-learned, so this never changes the binding LIST — see the + * notify-on-list-change contract on {@link handleCc}). + */ private updateNote(number: number, channel: number, value: number): void { const existing = this.bindings.find( (b) => b.kind === 'note' && b.number === number && b.channel === channel, ); if (existing) { existing.value = value; - this.notifyBindings(); } } diff --git a/manifold/tests/backend-manager-switch.test.ts b/manifold/tests/backend-manager-switch.test.ts new file mode 100644 index 0000000..836ad72 --- /dev/null +++ b/manifold/tests/backend-manager-switch.test.ts @@ -0,0 +1,130 @@ +/** + * BackendManager.setActive regression (simplification audit L19): a switch + * requested while another is already in flight must NOT be silently dropped. + * Run with `bun test tests/backend-manager-switch.test.ts` (see NOTE at the + * bottom of this file re: `bun run test` wiring). + */ +import { expect, test } from 'bun:test'; +import { BackendManager, type ManagerEngine } from '../src/backends/manager'; +import type { BackendContext, BackendStatus, OutputBackend } from '../src/backends/backend'; +import type { BackendId } from '../src/dock/output-state'; + +function makeCtx(): BackendContext { + return { modeId: 'test', outputCount: 0, mappings: [], names: [] }; +} + +function makeEngine(): ManagerEngine { + return { + subscribe: () => () => {}, + routedOutput: () => null, + audio: { setMuted: () => {} }, + }; +} + +/** A fake backend whose `start()` can be held open until the test releases it, + * so we can deterministically land a second `setActive` call WHILE the first + * is still in flight. */ +class FakeBackend implements OutputBackend { + readonly id: BackendId; + startCalls = 0; + teardownCalls = 0; + private release: (() => void) | null = null; + private hold: boolean; + + constructor(id: BackendId, hold = false) { + this.id = id; + this.hold = hold; + } + + isAvailable(): boolean { + return true; + } + + async start(_ctx: BackendContext): Promise { + this.startCalls++; + if (this.hold) { + await new Promise((resolve) => { + this.release = resolve; + }); + } + } + + /** Let a held `start()` resolve (simulates the backend becoming ready). */ + releaseStart(): void { + this.release?.(); + this.release = null; + } + + async teardown(): Promise { + this.teardownCalls++; + } + + send(): void {} + + status(): BackendStatus { + return { state: 'ready', message: 'fake' }; + } + + onStatusChange(): () => void { + return () => {}; + } +} + +test('a switch requested mid-switch is queued and applied, not dropped (L19)', async () => { + const midi = new FakeBackend('midi', /* hold */ true); + const osc = new FakeBackend('osc'); + const manager = new BackendManager(makeEngine(), { midi, osc }); + manager.setContext(makeCtx()); + + // Kick off a switch to 'midi' whose start() we hold open, simulating a + // switch genuinely in flight (e.g. an async backend.start()). + const first = manager.setActive('midi'); + + // While 'midi' is still starting, request 'osc'. Pre-fix this silently + // returned (BackendManager.setActive:131 `if (... || this.switching) return`) + // and 'osc' was never applied once 'midi' finished starting. + const second = manager.setActive('osc'); + + expect(manager.getActiveId()).toBe('midi'); // still mid-switch + midi.releaseStart(); + await first; + await second; + // Give the queued re-run (kicked off in setActive's `finally`) a tick to + // settle — its own start() is not held, so one microtask flush suffices. + await Promise.resolve(); + await Promise.resolve(); + + expect(manager.getActiveId()).toBe('osc'); + expect(osc.startCalls).toBe(1); +}); + +test('rapid repeated switches to the SAME pending id only apply it once', async () => { + const midi = new FakeBackend('midi', /* hold */ true); + const osc = new FakeBackend('osc'); + const manager = new BackendManager(makeEngine(), { midi, osc }); + manager.setContext(makeCtx()); + + const first = manager.setActive('midi'); + void manager.setActive('osc'); + void manager.setActive('osc'); // repeated request for the same pending id + void manager.setActive('osc'); + + midi.releaseStart(); + await first; + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(manager.getActiveId()).toBe('osc'); + // Only ONE extra switch should have actually run (no repeated re-queueing + // beyond the single pending slot — the infinite-loop guard). + expect(osc.startCalls).toBe(1); +}); + +// NOTE: `manifold/package.json`'s `test` script is `bun test src +// tests/pipeline-golden.test.ts` — an explicit file list, not a directory +// glob, so this file (like any other new file under tests/) is NOT picked up +// by `bun run test` as currently wired. Verified directly with +// `bun test tests/backend-manager-switch.test.ts`. Wiring `tests/` in as a +// whole is a one-line package.json change outside this group's file scope +// (see the handoff note in the accompanying report). diff --git a/manifold/tests/input-vector-truncation.test.ts b/manifold/tests/input-vector-truncation.test.ts new file mode 100644 index 0000000..55a51e5 --- /dev/null +++ b/manifold/tests/input-vector-truncation.test.ts @@ -0,0 +1,132 @@ +/** + * S10 regression (simplification audit): VCV bridged mode must drive the + * module with the FULL raw input vector, not just the first two axes. + * + * Two independent halves of the bug, tested separately: + * (a) `Spine.lastRawInputs` — the buffer `EngineApi.inputVector()` now + * delegates to directly — must hold every active axis, not just X/Y. + * Before the fix this field was PRIVATE (`private lastRawInputs`) and + * `EngineApi.inputVector()` returned a fresh `[lastRawX, lastRawY]` + * 2-element array, so this test would not even have compiled — a + * stronger signal than a runtime failure. (`EngineApi` itself needs a + * real loaded WASM module to construct, which this suite avoids per the + * existing convention in tests/pipeline-golden.test.ts / wasm-load.ts — + * "the ML surface is exercised elsewhere" — so this drives Spine + * directly with a minimal fake `WasmIML`, which is exactly the surface + * `EngineApi.inputVector()` now just forwards.) + * (b) `VcvBackend.send()` must dead-zone over the FULL vector, not just + * indices 0/1 — otherwise a change on axis 2+ (a gamepad stick or a + * learned MIDI CC beyond the XY pair) is silently swallowed even once + * `setInputVector` receives it. + * + * Run with `bun test tests/input-vector-truncation.test.ts` (see the NOTE at + * the bottom of tests/backend-manager-switch.test.ts re: `bun run test` + * wiring — the same applies here). + */ +import { expect, test } from 'bun:test'; +import { Spine } from '../src/engine/spine'; +import type { WasmIML } from '../src/engine/wasm-iml'; +import { VcvBackend } from '../src/backends/vcv-backend'; +import type { BackendContext, OutputMapping } from '../src/backends/backend'; + +// ---- (a) Spine.lastRawInputs ------------------------------------------- + +/** Minimal fake satisfying exactly the WasmIML surface `Spine` calls (see + * spine.ts: attach/setInputs/reprocess). No real inference — just plumbing, + * so this stays independent of the actual WASM build. */ +function makeFakeIml(outputSize = 8): WasmIML { + return { + architecture: { inputSize: 2, hidden: [0, 0, 0] as [number, number, number], outputSize, numLayers: 3 }, + setInputConfig: () => {}, + setOutputConfig: () => {}, + setOutputFreezeMask: () => {}, + resetInput: () => {}, + resetOutput: () => {}, + processInput: (x: number, y: number) => ({ x, y, frozen: false }), + setInput: () => {}, + processInto: (buf: Float32Array) => buf.fill(0), + processOutput: () => {}, + } as unknown as WasmIML; +} + +test('Spine.lastRawInputs retains the FULL N-D raw input vector, not just X/Y', () => { + const spine = new Spine(); + spine.attach(makeFakeIml(), null); + spine.setState({ inputSize: 5 }); + spine.setInputs([0.1, 0.2, 0.3, 0.4, 0.5]); + + // Compare through Float32Array on both sides — `lastRawInputs` is f32, so a + // plain f64 literal array would spuriously mismatch on rounding (e.g. 0.1 → + // 0.10000000149011612), not on the axis-count truncation this test targets. + expect(Array.from(spine.lastRawInputs)).toEqual( + Array.from(new Float32Array([0.1, 0.2, 0.3, 0.4, 0.5])), + ); + // The old 2-D extraction (`[lastRawX, lastRawY]`) is still available for + // the 2-D pad path, but must not be what a 5-D-aware consumer reads. + expect(spine.lastRawInputs.length).toBeGreaterThan(2); +}); + +// ---- (b) VcvBackend full-length dead-zone ------------------------------- + +function liveMapping(): OutputMapping { + return { state: 'live', muted: false, min: 0, max: 1, curve: 0.5, fixedValue: 0 }; +} + +function makeCtx(outputCount: number): BackendContext { + return { + modeId: 'test', + outputCount, + mappings: Array.from({ length: outputCount }, liveMapping), + names: Array.from({ length: outputCount }, (_, i) => `out${i}`), + }; +} + +/** Swap in a fake transport so `send()` runs without a real WebSocket. */ +function fakeClient() { + const sentInputs: number[][] = []; + return { connected: true, sendInput: (v: ReadonlyArray) => sentInputs.push(Array.from(v)), sendParams: () => {}, sentInputs }; +} + +/** Push `send()`'s internal 50ms send-interval timer far into the past so the + * throttle never blocks a test call regardless of how early in the process + * lifetime `performance.now()` currently reads. */ +function unthrottle(vcv: VcvBackend): void { + (vcv as unknown as { lastSendMs: number }).lastSendMs = -1e9; +} + +test('VcvBackend.send streams a 5-D input vector in full (not truncated to 2)', () => { + const vcv = new VcvBackend(); + (vcv as unknown as { ctx: BackendContext }).ctx = makeCtx(4); + const client = fakeClient(); + (vcv as unknown as { client: unknown }).client = client; + unthrottle(vcv); + + vcv.setInputVector([0.1, 0.2, 0.9, 0.4, 0.55]); + vcv.send(new Float32Array(4)); + + expect(client.sentInputs).toEqual([[0.1, 0.2, 0.9, 0.4, 0.55]]); +}); + +test('VcvBackend.send dead-zones over the FULL vector — a change on axis 3 alone still resends', () => { + const vcv = new VcvBackend(); + (vcv as unknown as { ctx: BackendContext }).ctx = makeCtx(4); + const client = fakeClient(); + (vcv as unknown as { client: unknown }).client = client; + unthrottle(vcv); + + vcv.setInputVector([0.5, 0.5, 0.5, 0.5]); + vcv.send(new Float32Array(4)); // first send always fires (sentinel init) + expect(client.sentInputs.length).toBe(1); + + // Bypass the internal 50ms send-interval throttle for the second call. + unthrottle(vcv); + + // Only axis index 3 moves; X/Y (indices 0/1) are unchanged. Before the S10 + // fix, VcvBackend's dead-zone check only ever looked at indices 0/1, so + // this change would have been silently dropped — no resend. + vcv.setInputVector([0.5, 0.5, 0.5, 0.9]); + vcv.send(new Float32Array(4)); + + expect(client.sentInputs.length).toBe(2); + expect(client.sentInputs[1]).toEqual([0.5, 0.5, 0.5, 0.9]); +}); diff --git a/manifold/tests/midi-notify-churn.test.ts b/manifold/tests/midi-notify-churn.test.ts new file mode 100644 index 0000000..edb7611 --- /dev/null +++ b/manifold/tests/midi-notify-churn.test.ts @@ -0,0 +1,86 @@ +/** + * L18 regression (simplification audit): `WebMidiInputSource` must notify + * `onBindingsChange` listeners only when the binding LIST changes (learn + * capture, clearBinding, clearAllBindings) — NOT on every incoming CC value + * update. Pre-fix, every CC message on an already-learned axis pushed a full + * bindings-array snapshot to every listener (a React state update per MIDI + * message in the real app, via useInputLayer.ts's `onBindingsChange` + * subscription). + * + * `onMessage` is private — there is no other public seam to feed synthetic + * MIDI bytes in, so this test invokes it through a narrow cast (a normal + * white-box technique for a class with no public message-injection API). + * + * Run with `bun test tests/midi-notify-churn.test.ts` (see the `bun run test` + * wiring note in tests/backend-manager-switch.test.ts — the same applies). + */ +import { expect, test } from 'bun:test'; +import { WebMidiInputSource } from '../src/inputs/midi-input-source'; + +function ccMessage(cc: number, channel1based: number, value7bit: number): MIDIMessageEvent { + const statusByte = 0xb0 | ((channel1based - 1) & 0x0f); + return { data: new Uint8Array([statusByte, cc, value7bit]) } as unknown as MIDIMessageEvent; +} + +function feedCc(midi: WebMidiInputSource, cc: number, channel: number, value7bit: number): void { + (midi as unknown as { onMessage(e: MIDIMessageEvent): void }).onMessage(ccMessage(cc, channel, value7bit)); +} + +test('repeated CC value updates on an EXISTING binding do not re-notify', () => { + const midi = new WebMidiInputSource(); + let notifyCount = 0; + midi.onBindingsChange(() => { + notifyCount++; + }); + + midi.armLearn(true); + feedCc(midi, 74, 1, 10); // new axis captured — the list changed → 1 notify + expect(notifyCount).toBe(1); + midi.armLearn(false); + + // Flood the SAME learned CC with 50 more value updates (the "wiggle a + // fader" case that used to flood React with one state update each). + for (let v = 0; v < 50; v++) feedCc(midi, 74, 1, v); + + expect(notifyCount).toBe(1); // still just the one list-change notify + expect(midi.getBindings()[0].value).toBeCloseTo(49 / 127, 5); // value still latches +}); + +test('learn capture / clearBinding / clearAllBindings still notify (the list DID change)', () => { + const midi = new WebMidiInputSource(); + let notifyCount = 0; + midi.onBindingsChange(() => { + notifyCount++; + }); + + midi.armLearn(true); + feedCc(midi, 1, 1, 10); + feedCc(midi, 2, 1, 20); + expect(notifyCount).toBe(2); // two NEW axes captured + midi.armLearn(false); + + midi.clearBinding(0); + expect(notifyCount).toBe(3); + midi.clearAllBindings(); + expect(notifyCount).toBe(4); +}); + +test('note gate updates never notify (no note binding can ever be created)', () => { + const midi = new WebMidiInputSource(); + let notifyCount = 0; + midi.onBindingsChange(() => { + notifyCount++; + }); + + // Note ON then OFF, repeatedly — no path creates a 'note' binding, so the + // list never changes and this must never notify. + const noteOn = { data: new Uint8Array([0x90, 60, 100]) } as unknown as MIDIMessageEvent; + const noteOff = { data: new Uint8Array([0x80, 60, 0]) } as unknown as MIDIMessageEvent; + const inject = midi as unknown as { onMessage(e: MIDIMessageEvent): void }; + for (let i = 0; i < 10; i++) { + inject.onMessage(noteOn); + inject.onMessage(noteOff); + } + + expect(notifyCount).toBe(0); +});