From ce7c9e4867672f0c519ea3a50c7df6d0ace95ef3 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sat, 8 Aug 2026 12:53:00 -0700 Subject: [PATCH] refactor(core): route the graph's enabled-node filter through one helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `enabledAudioFxNodes()` existed and was used only by the engine, while audioFxGraph restated the same predicate three times in three shapes — `.filter((n) => n.enabled !== false)` in `shapeOf`, `if (node.enabled === false) continue` in `buildFxChain`, and the filter again in `update`. All three now call the helper. Also drops the `shape = shapeOf(next)` at the end of `update`. The early return above it already established that `shapeOf(next)` equals `shape`, so this was a full normalise-and-join of the chain per observer tick to write back the string that was already there. `shape` is `const` now, which is the compiler enforcing the same thing. Consolidating found a genuine gap: `update`'s filter had NO test. A bypassed node is not in the graph, so the update must walk the ENABLED nodes to stay aligned with what was built — walking every node shifts each one's parameters into its neighbour, and the shape is identical either way so nothing forces a rebuild that would expose it. Both other call sites failed under mutation; this one passed. Now covered, and falsified: the lowpass keeps its old cutoff while the bypassed peaking's values land on it. Co-Authored-By: Claude Opus 5 (1M context) --- packages/core/src/audio/audioFxGraph.test.ts | 33 ++++++++++++++++++++ packages/core/src/audio/audioFxGraph.ts | 16 +++++----- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/packages/core/src/audio/audioFxGraph.test.ts b/packages/core/src/audio/audioFxGraph.test.ts index 316deff7a1..f1eca2b743 100644 --- a/packages/core/src/audio/audioFxGraph.test.ts +++ b/packages/core/src/audio/audioFxGraph.test.ts @@ -216,6 +216,39 @@ describe("buildFxChain", () => { expect(c.created.find((x) => x.kind === "biquad")!.frequency.value).toBe(2000); }); + it("lines new values up against the built nodes, skipping a bypassed one", () => { + // A bypassed node is not in the graph, so the update has to walk the + // ENABLED nodes to stay aligned with what was built. Walking `next.nodes` + // instead shifts everything after the bypass by one and pushes each node's + // parameters into its neighbour — and the shape is unchanged either way, + // so nothing forces a rebuild that would hide it. + const c = ctx(); + const withBypass = (frequency: number): HfAudioFxChain => ({ + version: 1, + nodes: [ + { + type: "highpass", + enabled: true, + params: { ...defaultAudioFxParams("highpass"), frequency: 100 }, + }, + { type: "peaking", enabled: false, params: defaultAudioFxParams("peaking") }, + { + type: "lowpass", + enabled: true, + params: { ...defaultAudioFxParams("lowpass"), frequency }, + }, + ], + }); + const h = buildFxChain(asCtx(c), withBypass(8000)); + const biquads = c.created.filter((n) => n.kind === "biquad"); + expect(biquads).toHaveLength(2); + + expect(h.update(withBypass(3000))).toBe(true); + // The lowpass took the new cutoff; the highpass was left where it was. + expect(biquads[0]!.frequency.value).toBe(100); + expect(biquads[1]!.frequency.value).toBe(3000); + }); + it("reports that a rebuild is needed when the chain shape changes", () => { const c = ctx(); const h = buildFxChain(asCtx(c), chain("peaking")); diff --git a/packages/core/src/audio/audioFxGraph.ts b/packages/core/src/audio/audioFxGraph.ts index 027064ebe4..f37f394214 100644 --- a/packages/core/src/audio/audioFxGraph.ts +++ b/packages/core/src/audio/audioFxGraph.ts @@ -9,6 +9,7 @@ */ import { + enabledAudioFxNodes, getAudioFxDef, normalizeAudioFxParams, type HfAudioFxChain, @@ -480,8 +481,7 @@ export interface FxChainHandle { * into the running nodes; when it changes, the caller rebuilds. */ function shapeOf(chain: HfAudioFxChain): string { - return chain.nodes - .filter((node) => node.enabled !== false) + return enabledAudioFxNodes(chain) .map((node) => { const p = normalizeAudioFxParams(node.type, node.params); const poles = p.poles !== undefined ? `:${p.poles}` : ""; @@ -506,8 +506,7 @@ export function buildFxChain(ctx: BaseAudioContext, chain: HfAudioFxChain): FxCh const handles: { id?: string; type: string; handle: FxNodeHandle }[] = []; let tail: AudioNode = input; - for (const node of chain.nodes) { - if (node.enabled === false) continue; + for (const node of enabledAudioFxNodes(chain)) { const handle = buildFxNode(ctx, node.type, node.params ?? {}); tail.connect(handle.input); tail = handle.output; @@ -515,7 +514,7 @@ export function buildFxChain(ctx: BaseAudioContext, chain: HfAudioFxChain): FxCh } tail.connect(output); - let shape = shapeOf(chain); + const shape = shapeOf(chain); return { input, @@ -523,8 +522,7 @@ export function buildFxChain(ctx: BaseAudioContext, chain: HfAudioFxChain): FxCh nodes: handles, update(next) { if (shapeOf(next) !== shape) return false; - const active = next.nodes.filter((node) => node.enabled !== false); - active.forEach((node, i) => { + enabledAudioFxNodes(next).forEach((node, i) => { const held = handles[i]; if (!held) return; held.handle.update(normalizeAudioFxParams(node.type, node.params)); @@ -537,7 +535,9 @@ export function buildFxChain(ctx: BaseAudioContext, chain: HfAudioFxChain): FxCh if (node.id === undefined) delete held.id; else held.id = node.id; }); - shape = shapeOf(next); + // `shape` is not reassigned: the early return above already established + // that `shapeOf(next)` equals it, so recomputing was a whole normalise + + // join per observer tick to write back the string that was already there. return true; }, dispose() {