refactor(core): route the graph's enabled-node filter through one helper

`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) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-08-08 12:53:00 -07:00
co-authored by Claude Opus 5
parent ac0e372474
commit ce7c9e4867
2 changed files with 41 additions and 8 deletions
@@ -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"));
+8 -8
View File
@@ -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() {