fix: seven defects a max-effort review found in the preset work

All seven are in code from this stack, and five are in the preset wet/dry
wrap and its removal path — the newest work, and the least exercised.

**Two produce a wrong render.**

`shapeOf` carried no preset identity, so a reorder that preserved the
effect-type sequence took the in-place update path and left the wet/dry
wrap bracketing the wrong node. Add "Tame Boominess" (a peaking node),
add the "Add Clarity" job (also peaking), move one past the other: the
shape string is unchanged, so preview keeps its old wiring and blends out
the author's own effect while the render, which rebuilds, blends out the
preset's. Same class as the one-pole cutoff and the phaser waveform
already encoded there — state that is WIRED at construction has to be in
the signature.

`FxChainHandle.presets` was a Record keyed by preset id, but `presetRuns`
legitimately emits several runs for one preset once a reorder splits it.
The second wrap overwrote the first, and `update` looked wraps up with
`find`, so a split preset wrote one wrap twice and never touched the
other: the switch read "Off" while the audio was unchanged, and an
`fx.preset.<id>` lane drove one fragment. Targets accumulate per id now,
and the update walks wraps in build order.

**Two lose or resurrect automation lanes.**

`removeRun` called `onRemoveNodeAutomation` per node in a loop. Each call
recomputes from the same render-time snapshot and replaces the whole
attribute, so every write but the last was discarded and the earlier
nodes' lanes survived as orphans — which the next effect added inherits,
because ids are minted lowest-free, arriving "Automated" with an envelope
nobody drew. One batched call now takes every id at once. `removeEq` had
the identical loop; it was only unreachable because EQ bands expose no
automation toggle.

It also never removed the whole-preset `fx.preset.<id>` lane, which
belongs to no node, so re-applying the preset later resurrected an old
ramp. The batched call takes the preset id too.

**Three more.**

The levelling script measured the whole decoded file from sample 0 while
lane times are clip-local, so any clip with `data-media-start` got an
envelope offset by exactly the trim — corrections landing on the wrong
passages, worse than not levelling. It now measures the clip's own window.

`audioFxProfileStrength` inverted every profile with a straight line, but
the reverb's `size` curve is piecewise because the design's anchors are
not evenly spaced. Setting Space to 0.5 read back as 0.46, and each
reopen moved the sound again. It searches the curve at the knob's own
0.01 resolution instead — searching only reachable values is what makes
the round trip exact rather than one step off.

The `audio_volume_double_automation` probe was narrowed to `[^;)]` in
this stack to stop it blaming the wrong element in a chained timeline.
That silenced it for any object holding a call —
`{ duration: fadeTime(2), volume: 0.2 }` — which is ordinary rather than
exotic. A depth-counted scan crosses nested parens and still stops at the
end of the selector's own call, which neither fixed bound could do.

Each fix has a test that fails without it; every one was falsified by
reverting the fix and watching it fail.

Not fixed, and reported as a false positive: the claim that a node id of
`"preset"` would have its lanes reinterpreted. `mintAudioFxNodeId` only
ever produces `n1`, `n2`, …, which is why that slot was reserved.

core 1774 · studio 3732 + 18 todo · engine services 739 + 3 · lint 520.

Author: vanceingalls

packages/core/src/audio/audioFxGraph.test.ts

@@ -698,3 +698,66 @@ describe("a preset's run is wrapped in a wet/dry blend", () => {
     expect(live).toEqual([]);
   });
 });
+
+describe("a preset's wrap stays with the nodes it belongs to", () => {
+  const peak = (over: Partial<HfAudioFxNode>): HfAudioFxNode =>
+    ({
+      type: "peaking",
+      enabled: true,
+      params: defaultAudioFxParams("peaking"),
+      ...over,
+    }) as HfAudioFxNode;
+
+  it("rebuilds when a reorder moves a node across a preset boundary", () => {
+    // The type sequence is identical either way, so without preset identity in
+    // the shape the chain updated in place and left the wet/dry wrap bracketing
+    // the hand-added effect instead of the preset's — preview blending out the
+    // author's own node while the render, which rebuilds, blended out the
+    // preset's.
+    const before: HfAudioFxChain = {
+      version: 1,
+      nodes: [peak({ id: "p1", fromPreset: "boom-tame" }), peak({ id: "own" })],
+    };
+    const after: HfAudioFxChain = {
+      version: 1,
+      nodes: [peak({ id: "own" }), peak({ id: "p1", fromPreset: "boom-tame" })],
+    };
+    expect(buildFxChain(asCtx(ctx()), before).update(after)).toBe(false);
+  });
+
+  it("gives every run of one preset its own blend", () => {
+    // A preset pulled apart by a reorder occupies two runs. Keyed by id and
+    // assigned, the second wrap overwrote the first, so a whole-preset lane
+    // reached one fragment and the switch silently left the rest applied.
+    const split: HfAudioFxChain = {
+      version: 1,
+      nodes: [
+        peak({ id: "t1", fromPreset: "telephone" }),
+        { type: "reverb", id: "own", enabled: true, params: defaultAudioFxParams("reverb") },
+        peak({ id: "t2", fromPreset: "telephone" }),
+      ],
+    };
+    const built = buildFxChain(asCtx(ctx()), split);
+    // Two wraps, so two wet/dry pairs — four params under the one id.
+    expect(built.presets.telephone).toHaveLength(4);
+  });
+
+  it("pushes an amount into each run of a split preset, not one of them twice", () => {
+    const at = (amount: number): HfAudioFxChain => ({
+      version: 1,
+      nodes: [
+        peak({ id: "t1", fromPreset: "telephone", presetAmount: amount }),
+        { type: "reverb", id: "own", enabled: true, params: defaultAudioFxParams("reverb") },
+        peak({ id: "t2", fromPreset: "telephone", presetAmount: amount }),
+      ],
+    });
+    const built = buildFxChain(asCtx(ctx()), at(1));
+    expect(built.update(at(0))).toBe(true);
+    // Every wet leg off and every dry leg fully open: switching the preset off
+    // has to silence all of it, not the last fragment only.
+    const wets = (built.presets.telephone ?? []).filter((_, i) => i % 2 === 0);
+    const drys = (built.presets.telephone ?? []).filter((_, i) => i % 2 === 1);
+    for (const t of wets) expect(t.param.value).toBe(0);
+    for (const t of drys) expect(t.param.value).toBe(1);
+  });
+});

packages/core/src/audio/audioFxGraph.ts

@@ -595,7 +595,16 @@ function shapeOf(chain: HfAudioFxChain): string {
       // same reason: pushed into the running graph it would be a no-op, and
       // preview would keep sweeping on a triangle while the render used a sine.
       const wave = node.type === "phaser" ? `~${p.type}` : "";
-      return `${node.type}${poles}${fixedFreq}${wave}`;
+      // Which preset run this node belongs to, for the same reason again: the
+      // wet/dry wrap is WIRED around a run at construction, so moving a node
+      // across a preset boundary changes the graph's shape even when the type
+      // sequence is identical. Without this, reordering a hand-added peaking
+      // filter past a preset's peaking filter kept the in-place update path and
+      // left the wrap bracketing the wrong effect — preview blending out the
+      // author's own node while the render, which rebuilds, blended out the
+      // preset's.
+      const run = node.fromPreset ? `%${node.fromPreset}` : "";
+      return `${node.type}${poles}${fixedFreq}${wave}${run}`;
     })
     .join("|");
 }
@@ -670,8 +679,14 @@ export function buildFxChain(
 
   const shape = shapeOf(chain);
 
+  // Accumulated, not assigned. A preset pulled apart by a reorder occupies more
+  // than one run, and each run gets its own wrap — keying by id and assigning
+  // dropped every wrap but the last, so a whole-preset lane drove one fragment
+  // and left the rest at full strength while the switch read "Off".
   const presetTargets: Record<string, FxParamTarget[]> = {};
-  for (const p of presets) presetTargets[p.id] = mixTargets(p.wet.gain, p.dry.gain);
+  for (const p of presets) {
+    (presetTargets[p.id] ??= []).push(...mixTargets(p.wet.gain, p.dry.gain));
+  }
 
   return {
     input,
@@ -696,10 +711,14 @@ export function buildFxChain(
       // The blend is a value like any other: switching a preset off writes
       // `presetAmount`, and pushing it into the running graph is what keeps that
       // from being a rebuild — and from restarting the audio underneath it.
+      // Walked in step with the build, not looked up by id: `find` returned the
+      // first wrap for every run sharing a preset id, so a split preset wrote
+      // one wrap twice and never touched the other.
+      let wrapIndex = 0;
       for (const run of presetRuns(enabledAudioFxNodes(next))) {
         if (!run.preset) continue;
-        const wrap = presets.find((p) => p.id === run.preset);
-        if (!wrap) continue;
+        const wrap = presets[wrapIndex++];
+        if (!wrap || wrap.id !== run.preset) continue;
         wrap.wet.gain.value = run.amount;
         wrap.dry.gain.value = 1 - run.amount;
       }

packages/core/src/audioFxProfiles.test.ts

@@ -87,14 +87,32 @@ describe("derived one-knob profiles", () => {
     // so it has to be authoritative. Reopening a project has to put the knob
     // back where it was.
     for (const [id, profile] of Object.entries(HF_AUDIO_FX_PROFILES)) {
+      // Tight, because the failure this guards against is small and cumulative:
+      // a piecewise curve read back with a straight line put the reverb's Space
+      // knob at 0.46 when the author set 0.5, and every reopen moved it again.
+      // One knob step is 0.01, so anything beyond that is a value the author
+      // did not choose.
       for (const s of [0, 0.25, 0.5, 0.75, 1]) {
         const params = applyAudioFxProfile(id, s, defaultAudioFxParams(id));
-        expect(audioFxProfileStrength(id, params), `${id} at ${s}`).toBeCloseTo(s, 1);
+        expect(
+          Math.abs(audioFxProfileStrength(id, params) - s),
+          `${id} at ${s} read back as ${audioFxProfileStrength(id, params)}`,
+        ).toBeLessThanOrEqual(0.01);
       }
       void profile;
     }
   });
 
+  it("reads a piecewise curve back at the value that produced it", () => {
+    // The reverb's `size` is piecewise — the design's anchors 0.25/0.55/0.90 are
+    // not evenly spaced — so a linear inverse is wrong by construction, and it
+    // was: 0.5 in, 0.46 out. This is the case the general round-trip above
+    // cannot isolate.
+    const params = applyAudioFxProfile("reverb", 0.5, defaultAudioFxParams("reverb"));
+    expect(params.size).toBe(0.55);
+    expect(audioFxProfileStrength("reverb", params)).toBe(0.5);
+  });
+
   it("passes through the figures the design proposed", () => {
     // The curves replaced a three-point table, and these are the points it
     // named. Continuous beats three settings, but not at the price of landing

packages/core/src/audioFxProfiles.ts

@@ -202,8 +202,28 @@ export function audioFxProfileStrength(type: string, params: HfAudioFxParamValue
   if (key === undefined) return 0.5;
   const value = params[key];
   if (typeof value !== "number") return 0.5;
-  const low = profile.at(0)[key];
-  const high = profile.at(1)[key];
-  if (typeof low !== "number" || typeof high !== "number" || low === high) return 0.5;
-  return to2(Math.min(1, Math.max(0, (value - low) / (high - low))));
+  // Searched, not inverted algebraically: a curve is free to be piecewise — the
+  // reverb's `size` is, because the design's three anchors are not evenly
+  // spaced — and a straight line between the endpoints reads such a curve back
+  // at the wrong place. Setting Space to 0.5 wrote size 0.55 and reopening the
+  // project drew the knob at 0.46, so every reopen nudged the sound.
+  //
+  // The grid IS the knob's own resolution (0.01), not something finer: a finer
+  // grid lands between two settable positions and rounds to a neighbour, which
+  // is how a search can be off by a step even where the curve is exact.
+  // Searching only reachable values makes the round trip exact.
+  const STEPS = 100;
+  let best = 0.5;
+  let bestErr = Infinity;
+  for (let i = 0; i <= STEPS; i += 1) {
+    const s = to2(i / STEPS);
+    const at = profile.at(s)[key];
+    if (typeof at !== "number") continue;
+    const err = Math.abs(at - value);
+    if (err < bestErr) {
+      bestErr = err;
+      best = s;
+    }
+  }
+  return bestErr === Infinity ? 0.5 : to2(best);
 }

packages/lint/src/rules/media.test.ts

@@ -457,6 +457,18 @@ describe("audio_volume_double_automation", () => {
     }
   });
 
+  it("still warns when another value in the same call is a function result", async () => {
+    // Bounding the scan at the first `)` to fix the chained-timeline case
+    // silenced the rule for the ordinary shape of a tween whose object holds a
+    // call — the paren closing `fadeTime(2)` ended the match before `volume`.
+    // The lane and the tween still both drive volume, and the author still gets
+    // no warning about it.
+    const res = await lintHyperframeHtml(
+      withScript(LANE, `tl.to("#bgm", { duration: fadeTime(2), volume: 0.2 });`),
+    );
+    expect(res.findings.some((f) => f.code === "audio_volume_double_automation")).toBe(true);
+  });
+
   it("does not blame the wrong element in a chained timeline", async () => {
     // A chain has no semicolon until its very end, so a run that could cross `)`
     // reached the `volume` in a LATER call and reported the element from an

packages/lint/src/rules/media.ts

@@ -2,6 +2,35 @@ import type { LintContext, HyperframeLintFinding } from "../context";
 import { readAttr, readDecodedAttr, stripJsComments, truncateSnippet, isMediaTag } from "../utils";
 import { validateColorGradingContract } from "@hyperframes/parsers/color-grading-contract";
 
+/**
+ * Does the GSAP call that names `#id` also set `volume` in the same call?
+ *
+ * Depth-counted rather than regex-bounded: the selector opens somewhere inside a
+ * call, and the interesting region ends when THAT call closes — a nested
+ * `fadeTime(2)` opens and closes on the way and must not end the scan. A regex
+ * cannot count parens, and both fixed bounds were wrong in opposite directions:
+ * unbounded blamed a later element, first-paren missed a whole ordinary shape.
+ */
+function tweensVolumeInSameCall(script: string, id: string): boolean {
+  const selector = new RegExp(`#${escapeRegExp(id)}(?![\\w-])`, "g");
+  for (let hit = selector.exec(script); hit; hit = selector.exec(script)) {
+    let depth = 0;
+    // Cap the scan so a malformed script cannot walk the whole file.
+    const limit = Math.min(script.length, hit.index + 2000);
+    for (let i = hit.index; i < limit; i += 1) {
+      const ch = script[i];
+      if (ch === "(") depth += 1;
+      else if (ch === ")") {
+        // Past the end of the call the selector sits in.
+        if (depth === 0) break;
+        depth -= 1;
+      } else if (ch === ";" && depth === 0) break;
+      else if (ch === "v" && /^volume\s*:/.test(script.slice(i))) return true;
+    }
+  }
+  return false;
+}
+
 function escapeRegExp(value: string): string {
   return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
 }
@@ -624,15 +653,15 @@ function findVolumeDoubleAutomationFindings(ctx: LintContext): HyperframeLintFin
     // the element's own selector, rather than by parsing the timeline. It reads
     // the same call the runtime's own probe would pick up, and the rule only
     // warns, so a miss costs nothing.
-    const escaped = escapeRegExp(id);
-    // `[^;)]`, not `[^;]`: a chained timeline has no semicolon until the end of
-    // the whole chain, so a run that could cross `)` matched `volume` in a LATER
-    // `.to()` call and named the wrong element — and this rule's fixHint tells
-    // the author to delete their lane. Refusing to cross the closing paren keeps
-    // the match inside the call the selector belongs to. It costs a false
-    // negative when some other value in the same object is a call result, which
-    // is the safe direction for a warning that already only guesses.
-    const tweened = new RegExp(`#${escaped}(?![\\w-])[^;)]{0,200}?\\bvolume\\s*:`).test(script);
+    // Scan to the end of the call the selector opened, rather than to the first
+    // `)`. A chained timeline has no semicolon until the end of the whole chain,
+    // so an unbounded run matched `volume` in a LATER `.to()` and named the
+    // wrong element — but stopping at the first `)` instead silenced the rule
+    // for any object holding a call, e.g.
+    // `gsap.to("#bgm", { duration: fadeTime(2), volume: 0.2 })`, which is the
+    // ordinary case rather than an exotic one. Counting depth keeps the match
+    // inside the selector's own call AND lets it cross a nested one.
+    const tweened = tweensVolumeInSameCall(script, id);
     if (!tweened) continue;
     findings.push({
       code: "audio_volume_double_automation",

packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx

@@ -435,6 +435,109 @@ describe("AudioFxGroup dynamic carve", () => {
    * nobody asked to level, through a channel that does not persist: audible,
    * absent from the document, and gone on the next reload.
    */
+  it("levels the part of the file the clip plays, not the file from its start", async () => {
+    // A lane's `t` is seconds from the start of the CLIP, but the decode is the
+    // whole file — so a trimmed clip got an envelope offset by exactly
+    // `media-start`, and every correction landed early.
+    //
+    // The file is loud 0-2s, quiet 2-5s, loud again 5-8s, and the clip trims the
+    // first 2s. Measured from the clip's own zero, t=0.5 sits in the quiet
+    // passage and wants a real lift; measured from the file's zero it sits in
+    // the loud head and wants none. That gap is the bug.
+    const sampleRate = 48000;
+    const data = new Float32Array(sampleRate * 8);
+    for (let i = 0; i < data.length; i++) {
+      const t = i / sampleRate;
+      const amp = t < 2 ? 0.5 : t < 5 ? 0.05 : 0.5;
+      data[i] = amp * Math.sin(2 * Math.PI * 300 * t);
+    }
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async () => ({ arrayBuffer: async () => new ArrayBuffer(8) })),
+    );
+    vi.stubGlobal(
+      "OfflineAudioContext",
+      class {
+        decodeAudioData = async () => ({ sampleRate, getChannelData: () => data });
+      },
+    );
+
+    const { host, onSetAttributeQuiet } = mount({
+      "fx-chain": CHAIN,
+      "media-start": "2",
+      duration: "6",
+    });
+    document.getElementById("bed")?.setAttribute("src", "bed.wav");
+    act(() => byTextButton(host, "Audio FX")?.click());
+    act(() => byTextButton(host, "Add effect")?.click());
+    await act(async () => {
+      byTextButton(host, "Even Out Levels")?.click();
+      await new Promise((r) => setTimeout(r, 0));
+      await new Promise((r) => setTimeout(r, 0));
+    });
+
+    const write = onSetAttributeQuiet.mock.calls.filter((c) => c[0] === "data-automation").at(-1);
+    if (!write) throw new Error("no levelling lane written");
+    const lane = (
+      JSON.parse(String(write[1])).lanes as { target: string; points: { t: number; v: number }[] }[]
+    ).find((l) => l.target.startsWith("fx."));
+    if (!lane) throw new Error("no fx lane");
+    const near = (t: number) =>
+      lane.points.reduce((best, p) => (Math.abs(p.t - t) < Math.abs(best.t - t) ? p : best));
+    // The quiet passage, from the clip's zero, gets its lift.
+    expect(near(0.5).v).toBeGreaterThan(4);
+  });
+
+  it("removes every lane a preset owned, not just the last node's", () => {
+    // Each write is computed from the same render-time snapshot and replaces the
+    // whole attribute, so removing lanes one node at a time kept only the final
+    // write — the earlier nodes' lanes survived as orphans, and with ids minted
+    // lowest-free the next effect added inherited one, arriving "Automated" with
+    // an envelope nobody drew and baked into the render.
+    const chain = {
+      version: 1,
+      nodes: [
+        {
+          type: "highpass",
+          id: "n1",
+          fromPreset: "telephone",
+          params: { frequency: 300, q: 0.707, poles: "2" },
+        },
+        {
+          type: "peaking",
+          id: "n2",
+          fromPreset: "telephone",
+          params: { frequency: 1200, gain: 6, q: 1.2 },
+        },
+      ],
+    };
+    const automation = {
+      version: 1,
+      lanes: [
+        { target: "fx.n1.frequency", points: [{ t: 0, v: 300 }] },
+        { target: "fx.n2.gain", points: [{ t: 0, v: 6 }] },
+        { target: "fx.preset.telephone", points: [{ t: 0, v: 1 }] },
+        { target: "volume", points: [{ t: 0, v: 0.5 }] },
+      ],
+    };
+    const { host, onSetAttributeQuiet } = mount({
+      "fx-chain": JSON.stringify(chain),
+      automation: JSON.stringify(automation),
+    });
+    act(() => byTextButton(host, "Audio FX")?.click());
+    act(() => host.querySelector<HTMLElement>(".hf-fx-preset-run-remove")?.click());
+
+    const write = onSetAttributeQuiet.mock.calls.filter((c) => c[0] === "data-automation").at(-1);
+    const lanes = JSON.parse(String(write?.[1] ?? '{"lanes":[]}')).lanes as { target: string }[];
+    const targets = lanes.map((l) => l.target);
+    // Both nodes gone, and the whole-preset lane with them.
+    expect(targets).not.toContain("fx.n1.frequency");
+    expect(targets).not.toContain("fx.n2.gain");
+    expect(targets).not.toContain("fx.preset.telephone");
+    // The track's own volume lane is untouched.
+    expect(targets).toContain("volume");
+  });
+
   describe("auditioning starts the transport when it has to", () => {
     const store = () => usePlayerStore.getState();