refactor(audio): the review's cleanup list, and the design record behind the rack

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

@@ -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"));

packages/core/src/audio/audioFxGraph.ts

@@ -9,6 +9,7 @@
  */
 
 import {
+  enabledAudioFxNodes,
   getAudioFxDef,
   normalizeAudioFxParams,
   type HfAudioFxChain,
@@ -484,8 +485,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}` : "";
@@ -510,25 +510,23 @@ 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;
     handles.push({ ...(node.id ? { id: node.id } : {}), type: node.type, handle });
   }
   tail.connect(output);
 
-  let shape = shapeOf(chain);
+  const shape = shapeOf(chain);
 
   return {
     input,
     output,
     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));
@@ -541,7 +539,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() {

packages/core/src/audioAutomation.test.ts

@@ -3,6 +3,8 @@ import {
   applyCurve,
   shapeProgress,
   fxAutomationTarget,
+  HF_AUDIO_AUTOMATION_ATTR,
+  HF_AUDIO_AUTOMATION_DATA_KEY,
   isConstantLane,
   parseAutomation,
   parseAutomationTarget,
@@ -30,6 +32,14 @@ const lane = (points: HfAutomationLane["points"], target = "volume"): HfAutomati
   points,
 });
 
+describe("the automation attribute's two spellings", () => {
+  it("names the same attribute either way", () => {
+    // Same split as the FX chain: written as an attribute, read as a dataset
+    // key. Derived, so a rename cannot half-land.
+    expect(HF_AUDIO_AUTOMATION_ATTR).toBe(`data-${HF_AUDIO_AUTOMATION_DATA_KEY}`);
+  });
+});
+
 describe("targets", () => {
   it("reads volume and fx targets, and rejects anything else", () => {
     expect(parseAutomationTarget("volume")).toEqual({ kind: "volume" });

packages/core/src/audioAutomation.ts

@@ -11,10 +11,13 @@
  * curve, three consumers, or the picture and the sound disagree.
  */
 
-import { getAudioFxDef, type HfAudioFxChain, type HfAudioFxNumberParam } from "./audioFx.js";
+import { getAudioFxDef, type HfAudioFxChain } from "./audioFx.js";
 
 export const HF_AUDIO_AUTOMATION_ATTR = "data-automation";
 
+/** The same attribute as a `dataset` / `dataAttributes` key. See `HF_AUDIO_FX_DATA_KEY`. */
+export const HF_AUDIO_AUTOMATION_DATA_KEY = HF_AUDIO_AUTOMATION_ATTR.slice("data-".length);
+
 /** Automation files are versioned; a reader must refuse a version it doesn't know. */
 export const HF_AUDIO_AUTOMATION_VERSION = 1;
 
@@ -138,15 +141,14 @@ export function resolveAutomationRange(
   const def = getAudioFxDef(node.type);
   const param = def?.params.find((p) => p.key === parsed.param);
   if (!param || param.kind !== "number") return null;
-  const p = param as HfAudioFxNumberParam;
   return {
-    min: p.min,
-    max: p.max,
-    step: p.step,
-    unit: p.unit,
-    label: `${def?.label ?? node.type} · ${p.label}`,
-    scale: p.scale === "log" && p.min > 0 ? "log" : "linear",
-    default: p.default,
+    min: param.min,
+    max: param.max,
+    step: param.step,
+    unit: param.unit,
+    label: `${def?.label ?? node.type} · ${param.label}`,
+    scale: param.scale === "log" && param.min > 0 ? "log" : "linear",
+    default: param.default,
   };
 }
 

packages/core/src/audioFx.test.ts

@@ -5,7 +5,9 @@ import {
   enabledAudioFxNodes,
   getAudioFxDef,
   HF_AUDIO_FX,
+  HF_AUDIO_FX_ATTR,
   HF_AUDIO_FX_CHAIN_VERSION,
+  HF_AUDIO_FX_DATA_KEY,
   HF_AUDIO_FX_IDS,
   normalizeAudioFxParams,
   parseAudioFxChain,
@@ -47,6 +49,15 @@ describe("effect registry", () => {
   });
 });
 
+describe("the chain attribute's two spellings", () => {
+  it("names the same attribute either way", () => {
+    // The studio WRITES through the attribute and READS through the dataset
+    // key, so the read side used to carry its own `"fx-chain"` literal and a
+    // rename would only half-land. Derived now; this is what keeps it derived.
+    expect(HF_AUDIO_FX_ATTR).toBe(`data-${HF_AUDIO_FX_DATA_KEY}`);
+  });
+});
+
 describe("normalizeAudioFxParams", () => {
   it("fills missing keys with defaults", () => {
     expect(normalizeAudioFxParams("peaking", {})).toEqual(defaultAudioFxParams("peaking"));

packages/core/src/audioFx.ts

@@ -16,6 +16,16 @@
 
 export const HF_AUDIO_FX_ATTR = "data-fx-chain";
 
+/**
+ * The same attribute as a `dataset` / `dataAttributes` key — the `data-` prefix
+ * is not part of that spelling.
+ *
+ * Derived rather than restated: the studio writes through
+ * `HF_AUDIO_FX_ATTR` and reads through the key, so a hardcoded `"fx-chain"`
+ * on the read side is a rename waiting to half-land.
+ */
+export const HF_AUDIO_FX_DATA_KEY = HF_AUDIO_FX_ATTR.slice("data-".length);
+
 /** Chain files are versioned; a reader must refuse a version it doesn't know. */
 export const HF_AUDIO_FX_CHAIN_VERSION = 1;
 

packages/studio/src/components/editor/audioFxSummary.ts

@@ -7,11 +7,11 @@
  * grouping exists to prevent — that they are seven things to manage.
  */
 
-import { parseAudioFxChain } from "@hyperframes/core/audio-fx";
+import { HF_AUDIO_FX_DATA_KEY, parseAudioFxChain } from "@hyperframes/core/audio-fx";
 import type { DomEditSelection } from "./domEditingTypes";
 
 export function audioFxSummary(element: DomEditSelection): string {
-  const raw = element.dataAttributes?.["fx-chain"];
+  const raw = element.dataAttributes?.[HF_AUDIO_FX_DATA_KEY];
   const carveAttr = element.dataAttributes?.["fx-carve"];
   let handBuilt = 0;
   let carveNodes = 0;

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

@@ -1,3 +1,3 @@
 /**
  * The audio FX panel's bridge to the element/attribute world.
  *
@@ -11,6 +11,7 @@
 import {
   defaultAudioFxParams,
   HF_AUDIO_FX_ATTR,
+  HF_AUDIO_FX_DATA_KEY,
   mintAudioFxNodeId,
   parseAudioFxChain,
   serializeAudioFxChain,
@@ -41,6 +42,7 @@
   automatedTargetsOf,
   automationAttrValue,
   HF_AUDIO_AUTOMATION_ATTR,
+  HF_AUDIO_AUTOMATION_DATA_KEY,
   readPanelAutomation,
   resolveAutomationRange,
   withoutLane,
@@ -109,7 +111,7 @@
   onSetAttributeLive: (attr: string, value: string | null) => void | Promise<void>;
 }) {
   const chain = ((): HfAudioFxChain => {
-    const raw = element.dataAttributes?.["fx-chain"];
+    const raw = element.dataAttributes?.[HF_AUDIO_FX_DATA_KEY];
     if (!raw) return { version: 1, nodes: [] };
     try {
       return parseAudioFxChain(raw);
@@ -120,7 +122,10 @@
     }
   })();
 
-  const automation = readPanelAutomation(element.dataAttributes?.["automation"], chain);
+  const automation = readPanelAutomation(
+    element.dataAttributes?.[HF_AUDIO_AUTOMATION_DATA_KEY],
+    chain,
+  );
   const automatedTargets = automatedTargetsOf(automation);
 
   /**
@@ -485,11 +490,24 @@
   const analyse = async (active: HfCarveSettings | null = carve): Promise<void> => {
     if (!active?.sources.length) return;
     const doc = element.element?.ownerDocument;
+    if (!doc) return;
     // Every named voice that is actually there with something to decode. A source
     // naming a deleted track is skipped rather than failing the whole analysis.
-    const voices = active.sources
-      .map((id) => doc?.getElementById(id) as HTMLAudioElement | null)
-      .filter((el): el is HTMLAudioElement => Boolean(el?.getAttribute("src")));
+    //
+    // Read out to plain values here rather than carrying elements around: it is
+    // what lets the src and the start be non-null by construction downstream
+    // instead of by assertion.
+    const voices: { src: string; start: string | null }[] = [];
+    for (const id of active.sources) {
+      const el = doc.getElementById(id);
+      // By tag name, not `instanceof HTMLAudioElement`: these elements belong to
+      // the composition's iframe document, so the constructor they were made
+      // from is not this realm's and the instanceof is false for every one.
+      if (el?.tagName !== "AUDIO") continue;
+      const src = el.getAttribute("src");
+      if (!src) continue;
+      voices.push({ src, start: el.getAttribute("data-start") });
+    }
     if (voices.length === 0) return;
     setAnalysing(true);
     try {
@@ -502,7 +520,7 @@
           .webkitOfflineAudioContext;
       if (!Ctor) return;
       const decode = async (relative: string): Promise<AudioBuffer> => {
-        const res = await fetch(new URL(relative, doc!.baseURI).href);
+        const res = await fetch(new URL(relative, doc.baseURI).href);
         return new Ctor(1, 1, DECODE_SAMPLE_RATE).decodeAudioData(await res.arrayBuffer());
       };
       const bedStart = clipStart(element.dataAttributes?.["start"]);
@@ -512,9 +530,9 @@
       // is also what lets the bands and the envelopes stay a single set: the chain is
       // fixed, so there is no per-voice filter to switch between.
       const decoded = await Promise.all(
-        voices.map(async (el) => ({
-          samples: (await decode(el.getAttribute("src")!)).getChannelData(0),
-          offsetSeconds: clipStart(el.getAttribute("data-start")) - bedStart,
+        voices.map(async (voice) => ({
+          samples: (await decode(voice.src)).getChannelData(0),
+          offsetSeconds: clipStart(voice.start) - bedStart,
         })),
       );
       const voiceMix = mixCarveSources(decoded, DECODE_SAMPLE_RATE);

packages/studio/src/components/editor/propertyPanelAutomation.ts

@@ -8,6 +8,7 @@
 
 import {
   HF_AUDIO_AUTOMATION_ATTR,
+  HF_AUDIO_AUTOMATION_DATA_KEY,
   parseAutomation,
   resolveAutomation,
   resolveAutomationRange,
@@ -77,4 +78,4 @@ export function automationAttrValue(automation: HfAutomation): string {
   return automation.lanes.length > 0 ? serializeAutomation(automation) : "";
 }
 
-export { HF_AUDIO_AUTOMATION_ATTR, resolveAutomationRange };
+export { HF_AUDIO_AUTOMATION_ATTR, HF_AUDIO_AUTOMATION_DATA_KEY, resolveAutomationRange };

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

@@ -165,6 +165,55 @@ describe("FxSection chain", () => {
     expect(next.nodes.map((n) => n.type)).toEqual(["reverb", "peaking"]);
   });
 
+  it("keeps a half-typed value with its own effect across a reorder", () => {
+    // Rows used to be keyed `${type}-${index}`, so two effects of the same type
+    // kept their keys through a reorder and React reused each row where it
+    // stood. The controls hold real state — a number field mid-edit is held as
+    // text — so the buffer stayed at the position and landed on whichever
+    // effect moved into it.
+    const peaking = (id: string, frequency: number) => ({
+      type: "peaking",
+      id,
+      enabled: true,
+      params: { ...defaultAudioFxParams("peaking"), frequency },
+    });
+    const a = peaking("pa", 400);
+    const b = peaking("pb", 1600);
+    const chainOfNodes = (...nodes: unknown[]): HfAudioFxChain =>
+      ({ version: 1, nodes }) as HfAudioFxChain;
+
+    const host = document.createElement("div");
+    document.body.append(host);
+    const root = createRoot(host);
+    const render = (chain: HfAudioFxChain) =>
+      act(() => {
+        root.render(
+          <FxSection
+            chain={chain}
+            onChainChange={vi.fn()}
+            onChainPreview={vi.fn()}
+            carve={null}
+            onCarveChange={vi.fn()}
+            sourceOptions={[]}
+          />,
+        );
+      });
+
+    render(chainOfNodes(a, b));
+    // Only the first card is open, which is the one being edited.
+    const openFrequency = (): HTMLInputElement =>
+      host.querySelector<HTMLInputElement>(".hf-fx-node .hf-fx-number")!;
+
+    expect(openFrequency().value).toBe("400");
+    typeInto(openFrequency(), "123");
+    expect(openFrequency().value).toBe("123");
+
+    // The author moves that effect down; the other one takes the open slot.
+    render(chainOfNodes(b, a));
+
+    expect(openFrequency().value).toBe("1600");
+  });
+
   it("cannot move the ends past themselves", () => {
     const { host } = mount({ chain: chainOf("peaking", "reverb") });
     const ups = host.querySelectorAll('.hf-fx-move[title="Move up"]');

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

@@ -1,3 +1,3 @@
 /**
  * The FX section for an audio element: the chain, plus the voiceover carve.
  *
@@ -795,7 +795,13 @@
           handBuilt.map(({ node, i }) => {
             return (
               <FxNodeRow
-                key={`${node.type}-${i}`}
+                // Keyed by id, as the carve module's list above already is.
+                // On `${type}-${index}` two effects of the same type keep their
+                // keys through a reorder, so React reuses each row where it
+                // stands — and the controls hold real state (a half-typed
+                // number, an in-flight drag), which then lands on whichever
+                // effect moved into that slot.
+                key={node.id ?? `${node.type}-${i}`}
                 node={node}
                 index={i}
                 automatedTargets={automatedTargets}