fix(studio): keep FX panel writes from clobbering each other

Three writes in the audio panel each read the source file, mutate one
attribute and write it back. Fired without ordering they read the same
content and the last one lands, dropping the others.

- Deleting an effect left its automation lanes in the attribute. Ids are
  minted lowest-free, so the next effect added took the same id and
  inherited the dead envelope: disabled and "Automated" without the
  author ever automating it, and baked into the render.
- Switching carve off wrote the chain (dropping the filters it generated)
  and the carve settings at once, so either the filters stayed with no
  carve to explain them or the settings survived with no filters.
- The three carve dials committed per input event, patching the source
  and resyncing the selection dozens of times per drag. They now preview
  live and persist on release, like the FX knobs already do.

Volume automation reads through the quiet commit too, so removing a lane
resyncs the panel instead of leaving the slider disabled.

Author: vanceingalls

packages/studio/src/components/editor/PropertyPanelFlat.tsx

@@ -257,7 +257,7 @@ export function PropertyPanelFlat({
   const showMotionEffects = gsapEffectHandlers !== null;
   const showMotionGroup = showMotionTiming || showMotionEffects;
 
-  const volumeAutomation = useVolumeAutomation(element, onSetAttribute);
+  const volumeAutomation = useVolumeAutomation(element, onSetAttributeQuiet ?? onSetAttributeLive);
 
   const groups: FlatGroupDescriptor[] = [];
   if (isTextEditable) {
@@ -436,6 +436,7 @@ export function PropertyPanelFlat({
         <AudioFxGroup
           element={element}
           onSetAttributeQuiet={onSetAttributeQuiet ?? onSetAttributeLive}
+          onSetAttributeLive={onSetAttributeLive}
         />
       ),
     });

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

@@ -40,15 +40,20 @@ function mount(dataAttributes: Record<string, string>, alone = false) {
   // restart every playing track, but with a selection resync so the panel sees
   // what it just wrote.
   const onSetAttributeQuiet = vi.fn();
+  const onSetAttributeLive = vi.fn();
   const host = document.createElement("div");
   document.body.append(host);
   const selection = audioSelection(dataAttributes, alone);
   act(() => {
     createRoot(host).render(
-      <AudioFxGroup element={selection} onSetAttributeQuiet={onSetAttributeQuiet} />,
+      <AudioFxGroup
+        element={selection}
+        onSetAttributeQuiet={onSetAttributeQuiet}
+        onSetAttributeLive={onSetAttributeLive}
+      />,
     );
   });
-  return { host, onSetAttributeQuiet };
+  return { host, onSetAttributeQuiet, onSetAttributeLive };
 }
 
 const rowFor = (host: HTMLElement, label: string): HTMLElement | null => {
@@ -182,18 +187,26 @@ describe("AudioFxGroup carve", () => {
     return block.querySelector(".hf-fx-bypass") as HTMLButtonElement;
   };
 
-  it("removes the filters it generated when carve is switched off", () => {
+  it("removes the filters it generated when carve is switched off", async () => {
     // Leaving them behind would keep dipping the bed with no carve to explain it.
     const { host, onSetAttributeQuiet } = mount({
       "fx-chain": carvedChain,
       "fx-carve": carveOn,
     });
-    act(() => carveToggle(host).click());
+    await act(async () => {
+      carveToggle(host).click();
+    });
     const chainWrite = onSetAttributeQuiet.mock.calls.find((c) => c[0] === "data-fx-chain");
     expect(chainWrite).toBeTruthy();
     const kept = JSON.parse(String(chainWrite![1])).nodes;
     expect(kept.map((n: { type: string }) => n.type)).toEqual(["lowpass"]);
-    // And the carve settings themselves go.
+    // And the carve settings themselves go — after the chain write, not
+    // alongside it: both are read-modify-writes of the same file, so fired
+    // together the later one reads pre-edit content and drops the earlier.
+    expect(onSetAttributeQuiet.mock.calls.map((c) => c[0])).toEqual([
+      "data-fx-chain",
+      "data-fx-carve",
+    ]);
     expect(onSetAttributeQuiet.mock.calls.find((c) => c[0] === "data-fx-carve")?.[1]).toBeNull();
   });
 
@@ -207,6 +220,27 @@ describe("AudioFxGroup carve", () => {
     expect(onSetAttributeQuiet.mock.calls.some((c) => c[0] === "data-fx-chain")).toBe(false);
   });
 
+  it("drags a carve dial live and persists once on release", () => {
+    // Without the split every pointermove patched the source file and resynced
+    // the selection, which is what makes the audio stutter mid-drag.
+    const { host, onSetAttributeQuiet, onSetAttributeLive } = mount({
+      "fx-chain": carvedChain,
+      "fx-carve": carveOn,
+    });
+    const dial = host.querySelector<HTMLInputElement>(".hf-fx-carve input[type=range]");
+    expect(dial).not.toBeNull();
+    act(() => {
+      // React's value tracker swallows a plain assignment, so go through the
+      // prototype setter the way the other panel tests do.
+      Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")?.set?.call(dial, "0.5");
+      dial?.dispatchEvent(new Event("input", { bubbles: true }));
+    });
+    expect(onSetAttributeLive.mock.calls.map((c) => c[0])).toEqual(["data-fx-carve"]);
+    expect(onSetAttributeQuiet.mock.calls.some((c) => c[0] === "data-fx-carve")).toBe(false);
+    act(() => dial?.dispatchEvent(new PointerEvent("pointerup", { bubbles: true })));
+    expect(onSetAttributeQuiet.mock.calls.some((c) => c[0] === "data-fx-carve")).toBe(true);
+  });
+
   it("writes carve settings live, so enabling it does not reload the preview", () => {
     const { host, onSetAttributeQuiet } = mount({ "fx-chain": carvedChain });
     act(() => carveToggle(host).click());
@@ -275,3 +309,50 @@ describe("AudioFxGroup carve visibility", () => {
     expect(host.querySelector(".hf-fx-carve")).toBeNull();
   });
 });
+
+describe("AudioFxGroup deleting an effect", () => {
+  const twoNodes = JSON.stringify({
+    version: 1,
+    nodes: [
+      { type: "lowpass", id: "n1", params: { frequency: 400, q: 0.9, poles: "2" } },
+      { type: "peaking", id: "n2", params: { frequency: 900, gain: -6, q: 1 } },
+    ],
+  });
+
+  it("takes the deleted node's lanes with it", () => {
+    // resolveAutomation only hides an orphan at read time. Left in the attribute,
+    // and with ids minted lowest-free, the next effect added takes the same id and
+    // inherits the dead envelope — disabled and "Automated" without the author
+    // ever automating it, and baked into the render.
+    const { host, onSetAttributeQuiet } = mount({
+      "fx-chain": twoNodes,
+      automation: JSON.stringify({
+        version: 1,
+        lanes: [
+          { target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] },
+          { target: "fx.n2.gain", points: [{ t: 0, v: -6 }] },
+          { target: "volume", points: [{ t: 0, v: 1 }] },
+        ],
+      }),
+    });
+    const remove = host.querySelectorAll<HTMLButtonElement>(".hf-fx-remove")[0]!;
+    act(() => remove.click());
+    const automationWrite = onSetAttributeQuiet.mock.calls.find((c) => c[0] === "data-automation");
+    expect(automationWrite).toBeTruthy();
+    expect(
+      JSON.parse(String(automationWrite![1])).lanes.map((l: { target: string }) => l.target),
+    ).toEqual(["fx.n2.gain", "volume"]);
+  });
+
+  it("leaves automation alone when the deleted node had none", () => {
+    const { host, onSetAttributeQuiet } = mount({
+      "fx-chain": twoNodes,
+      automation: JSON.stringify({
+        version: 1,
+        lanes: [{ target: "fx.n2.gain", points: [{ t: 0, v: -6 }] }],
+      }),
+    });
+    act(() => host.querySelectorAll<HTMLButtonElement>(".hf-fx-remove")[0]!.click());
+    expect(onSetAttributeQuiet.mock.calls.some((c) => c[0] === "data-automation")).toBe(false);
+  });
+});

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

@@ -48,6 +48,7 @@ import { FxSection, type AudioTrackOption } from "./propertyPanelFxSection.js";
 export function AudioFxGroup({
   element,
   onSetAttributeQuiet,
+  onSetAttributeLive,
 }: {
   element: DomEditSelection;
   /**
@@ -61,6 +62,8 @@ export function AudioFxGroup({
    * edit would work from a pre-edit value and appear to do nothing.
    */
   onSetAttributeQuiet: (attr: string, value: string | null) => void | Promise<void>;
+  /** Continuous, non-persisting write for a dial being dragged. */
+  onSetAttributeLive: (attr: string, value: string | null) => void | Promise<void>;
 }) {
   const chain = ((): HfAudioFxChain => {
     const raw = element.dataAttributes?.["fx-chain"];
@@ -108,6 +111,42 @@ export function AudioFxGroup({
     writeAutomation(withoutLane(automation, fxAutomationTarget(nodeId, paramKey)));
   };
 
+  /**
+   * Turn carve on or off.
+   *
+   * Switching off drops the filters it generated — left behind they keep dipping
+   * the bed with nothing in the panel to explain them — but that is a second
+   * attribute, and each write is a read-modify-write against the same source
+   * file. Fired together, both read the same content and the later one drops the
+   * earlier: either the carve settings went and the filters stayed, or the
+   * reverse. Awaiting the first means the second reads the file it produced.
+   *
+   * One commit carrying both would also close the window where a failure of just
+   * the second leaves them half-applied; that needs a multi-attribute quiet
+   * commit, which does not exist yet.
+   */
+  const setCarve = async (next: HfCarveSettings | null): Promise<void> => {
+    if (!next) {
+      const kept = chain.nodes.filter((n) => !n.fromCarve);
+      if (kept.length !== chain.nodes.length) {
+        await onSetAttributeQuiet(
+          HF_AUDIO_FX_ATTR,
+          kept.length ? serializeAudioFxChain({ version: 1, nodes: kept }) : null,
+        );
+      }
+    }
+    await onSetAttributeQuiet(HF_AUDIO_CARVE_ATTR, next ? JSON.stringify(next) : null);
+  };
+
+  /** Every lane belonging to a node that is going away. */
+  const removeNodeAutomation = (nodeId: string): void => {
+    const prefix = `fx.${nodeId}.`;
+    const kept = automation.lanes.filter((lane) => !lane.target.startsWith(prefix));
+    if (kept.length !== automation.lanes.length) {
+      writeAutomation({ version: 1, lanes: kept });
+    }
+  };
+
   const carve = ((): HfCarveSettings | null => {
     const raw = element.dataAttributes?.["fx-carve"];
     if (!raw) return null;
@@ -178,6 +217,7 @@ export function AudioFxGroup({
       automatedTargets={automatedTargets}
       onAutomateParam={automateParam}
       onRemoveParamAutomation={removeParamAutomation}
+      onRemoveNodeAutomation={removeNodeAutomation}
       onChainChange={(next) =>
         // Live for the same reason as automation above: adding, removing or
         // bypassing an effect is applied to the running graph, so a reload would
@@ -188,28 +228,14 @@ export function AudioFxGroup({
         )
       }
       onChainPreview={(next) =>
-        // Live writes skip the preview refresh, so dragging a knob no longer
-        // reloads the composition and restarts playback on every pixel.
-        onSetAttributeQuiet(
-          HF_AUDIO_FX_ATTR,
-          next.nodes.length ? serializeAudioFxChain(next) : null,
-        )
+        // Live writes skip the preview refresh entirely, so dragging a knob no
+        // longer reloads the composition and restarts playback on every pixel.
+        // The gesture-end write above is the one that resyncs.
+        onSetAttributeLive(HF_AUDIO_FX_ATTR, next.nodes.length ? serializeAudioFxChain(next) : null)
       }
       carve={carve}
-      onCarveChange={(next) => {
-        // Turning carve off drops the filters it generated; leaving them behind
-        // would keep dipping the bed with no carve to explain it.
-        if (!next) {
-          const kept = chain.nodes.filter((n) => !n.fromCarve);
-          if (kept.length !== chain.nodes.length) {
-            onSetAttributeQuiet(
-              HF_AUDIO_FX_ATTR,
-              kept.length ? serializeAudioFxChain({ version: 1, nodes: kept }) : null,
-            );
-          }
-        }
-        void onSetAttributeQuiet(HF_AUDIO_CARVE_ATTR, next ? JSON.stringify(next) : null);
-      }}
+      onCarveChange={(next) => void setCarve(next)}
+      onCarvePreview={(next) => onSetAttributeLive(HF_AUDIO_CARVE_ATTR, JSON.stringify(next))}
       sourceOptions={sourceOptions}
       onAnalyseCarve={() => void analyse()}
       analysing={analysing}

packages/studio/src/components/editor/propertyPanelFxSection.tsx

@@ -269,12 +269,18 @@ export interface FxSectionProps {
   onAutomateParam?(nodeId: string, paramKey: string): void;
   /** Delete one effect parameter's lane. */
   onRemoveParamAutomation?(nodeId: string, paramKey: string): void;
+  /** Delete every lane belonging to a node that is being removed. */
+  onRemoveNodeAutomation?(nodeId: string): void;
   /** Structural edits and gesture-end writes; this is the one that persists. */
   onChainChange(chain: HfAudioFxChain): void;
   /** Continuous updates while a control is being dragged. */
   onChainPreview?(chain: HfAudioFxChain): void;
   carve: HfCarveSettings | null;
+  /** Gesture-end write; this is the one that persists. */
   onCarveChange(carve: HfCarveSettings | null): void;
+  /** Continuous updates while a carve slider is dragged. Without this every
+   *  pointermove patched the source file and resynced the selection. */
+  onCarvePreview?(carve: HfCarveSettings): void;
   /** Other audio elements that could act as the carve source. */
   sourceOptions: AudioTrackOption[];
   /** Re-run analysis against the current source audio. */
@@ -288,15 +294,21 @@ export function FxSection({
   automatedTargets,
   onAutomateParam,
   onRemoveParamAutomation,
+  onRemoveNodeAutomation,
   onChainChange,
   onChainPreview,
   carve,
   onCarveChange,
+  onCarvePreview,
   sourceOptions,
   onAnalyseCarve,
   analysing,
   disabled,
 }: FxSectionProps) {
+  // Falls back to the persisting write when no preview handler is supplied, which
+  // keeps the control working rather than going dead.
+  const previewCarve = onCarvePreview ?? onCarveChange;
+
   // Nothing to carve against means nothing to show — see the block below.
   const showCarve = sourceOptions.length > 0 || carve !== null;
 
@@ -343,10 +355,17 @@ export function FxSection({
 
   const removeNode = useCallback(
     (index: number) => {
+      // The node's lanes go with it. `resolveAutomation` only hides an orphan at
+      // read time; left in the attribute, and with ids minted lowest-free, the
+      // next effect added takes the same id and inherits the dead envelope —
+      // arriving with its control disabled and "Automated" without the author
+      // ever automating it, and baked into the render.
+      const removedId = chain.nodes[index]?.id;
+      if (removedId) onRemoveNodeAutomation?.(removedId);
       mutate(chain.nodes.filter((_, i) => i !== index));
       setOpenNode(null);
     },
-    [chain.nodes, mutate],
+    [chain.nodes, mutate, onRemoveNodeAutomation],
   );
 
   const moveNode = useCallback(
@@ -478,7 +497,8 @@ export function FxSection({
                 }}
                 value={carve.maxCutDb}
                 disabled={disabled}
-                onChange={(_k, v) => onCarveChange({ ...carve, maxCutDb: Number(v) })}
+                onChange={(_k, v) => previewCarve({ ...carve, maxCutDb: Number(v) })}
+                onCommit={(_k, v) => onCarveChange({ ...carve, maxCutDb: Number(v) })}
               />
               <FxParamRow
                 param={{
@@ -493,7 +513,8 @@ export function FxSection({
                 }}
                 value={carve.bands}
                 disabled={disabled}
-                onChange={(_k, v) => onCarveChange({ ...carve, bands: Number(v) })}
+                onChange={(_k, v) => previewCarve({ ...carve, bands: Number(v) })}
+                onCommit={(_k, v) => onCarveChange({ ...carve, bands: Number(v) })}
               />
               <FxParamRow
                 param={{
@@ -509,7 +530,8 @@ export function FxSection({
                 }}
                 value={carve.intelligibilityBias}
                 disabled={disabled}
-                onChange={(_k, v) => onCarveChange({ ...carve, intelligibilityBias: Number(v) })}
+                onChange={(_k, v) => previewCarve({ ...carve, intelligibilityBias: Number(v) })}
+                onCommit={(_k, v) => onCarveChange({ ...carve, intelligibilityBias: Number(v) })}
               />
               <button
                 type="button"