fix(core): move a node's id with its slot when the chain updates in place

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.
This commit is contained in:
Vance Ingalls
2026-08-08 10:42:14 -07:00
parent accac99843
commit b754e72d0c
2 changed files with 39 additions and 1 deletions
@@ -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"]);
});
});
+11 -1
View File
@@ -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;