From b754e72d0c3a749b42c1fd2f08fb7b5e5cb44a6d Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sat, 8 Aug 2026 10:42:14 -0700 Subject: [PATCH] fix(core): move a node's id with its slot when the chain updates in place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reordering two effects of the same type leaves `shapeOf` identical — it carries type, poles and the one-pole frequency, but no id — so the chain updates in place instead of rebuilding. That is right for the audio: the params move with the position. The ids did not move with them. They were captured once at build time, so after a reorder each handle named whichever effect used to sit in that slot, and scheduleChainAutomation built its byId map from those stale names: a lane on `fx.n2.frequency` drove the band that is now n1. This is the invariant HfAudioFxNode.id documents itself as protecting — "reordering the chain never re-points a lane at a different effect" — and it is easiest to hit on the all-peaking chains the voiceover carve produces. Preview only, since the render rebuilds from scratch, so the symptom is preview quietly ceasing to predict the render. Fixed by following the id, not by adding it to the shape: a same-type reorder genuinely does not need a rebuild, only a correct id map. --- packages/core/src/audio/audioFxGraph.test.ts | 28 ++++++++++++++++++++ packages/core/src/audio/audioFxGraph.ts | 12 ++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/core/src/audio/audioFxGraph.test.ts b/packages/core/src/audio/audioFxGraph.test.ts index 4d85eca285..1a8d235196 100644 --- a/packages/core/src/audio/audioFxGraph.test.ts +++ b/packages/core/src/audio/audioFxGraph.test.ts @@ -461,3 +461,31 @@ describe("phaser automation targets", () => { expect(handle.automation?.out_gain?.[0]?.param.value).toBeCloseTo(0.5, 6); }); }); + +describe("chain update keeps ids with their effects", () => { + const band = (id: string, frequency: number) => ({ + type: "peaking", + id, + enabled: true, + params: { ...defaultAudioFxParams("peaking"), frequency }, + }); + + /** + * Reordering two effects of the same type leaves the shape string identical, + * so the chain updates in place rather than rebuilding — correct for the + * audio, since the params move with the position. The ids have to move too: + * a lane addresses its effect by id, and an id captured at build time names + * whichever effect used to occupy that slot. The scheduler would then drive + * `fx.n2.frequency` into the band that is now n1 — the exact swap that + * HfAudioFxNode.id documents itself as preventing, and the one the voiceover + * carve's all-peaking chains make easy to hit. + */ + it("moves an id with its slot when same-type effects are reordered", () => { + const chain: HfAudioFxChain = { version: 1, nodes: [band("n1", 200), band("n2", 4000)] }; + const handle = buildFxChain(ctx() as unknown as BaseAudioContext, chain); + + const swapped: HfAudioFxChain = { version: 1, nodes: [band("n2", 4000), band("n1", 200)] }; + expect(handle.update(swapped)).toBe(true); + expect(handle.nodes.map((n) => n.id)).toEqual(["n2", "n1"]); + }); +}); diff --git a/packages/core/src/audio/audioFxGraph.ts b/packages/core/src/audio/audioFxGraph.ts index abb1f2e5f0..09f3c91a7a 100644 --- a/packages/core/src/audio/audioFxGraph.ts +++ b/packages/core/src/audio/audioFxGraph.ts @@ -515,7 +515,17 @@ export function buildFxChain(ctx: BaseAudioContext, chain: HfAudioFxChain): FxCh if (shapeOf(next) !== shape) return false; const active = next.nodes.filter((node) => node.enabled !== false); active.forEach((node, i) => { - handles[i]?.handle.update(normalizeAudioFxParams(node.type, node.params)); + const held = handles[i]; + if (!held) return; + held.handle.update(normalizeAudioFxParams(node.type, node.params)); + // The id follows the position, because the params just did. Reordering + // two effects of the same type leaves the shape identical, so the graph + // is updated in place — but a lane addresses its effect BY id, and an id + // captured at build time then names whichever effect used to be here. + // The scheduler would drive `fx.n2.frequency` into the band that is now + // n1: exactly what HfAudioFxNode.id documents itself as preventing. + if (node.id === undefined) delete held.id; + else held.id = node.id; }); shape = shapeOf(next); return true;