fix(studio): harden carve and FX rack behavior (#3452)

* fix(core): harden audio FX and group identity

* fix(core): address audio group review feedback

* fix(core): align preview transport with grouped audio

* test(core): pin audio group gain ceiling

* fix(core): preserve solo bridge through stack

* fix(engine): harden grouped audio rendering

* docs(engine): explain grouped mix fallback invariant

* test(engine): allow grouped mixes to finish on Windows

* feat(lint): validate audio group membership and timing

* test(lint): pin audio group membership guards

* fix(studio): unify audio IDs and group state

* fix(studio): make audio-group edits transactional

* fix(studio): keep preview state synchronized

* fix(studio): align audio rows, automation lanes and headers

* fix(studio): stabilize timeline audio derivations

* refactor(studio): simplify group metadata memoization

* style(studio): keep timeline layout within size gate

* fix(studio): keep timeline preset apply off auditions

* fix(studio): harden carve and FX rack behavior

* fix(studio): repeat audio FX reveal requests

Author: vanceingalls

packages/studio/src/components/editor/audioFxRevealTarget.test.ts

@@ -0,0 +1,59 @@
+import { describe, expect, it } from "vitest";
+import { audioFxRevealTarget } from "./audioFxRevealTarget";
+import type { HfAudioFxChain } from "@hyperframes/core/audio-fx";
+
+const chain = (nodes: HfAudioFxChain["nodes"]): HfAudioFxChain => ({ version: 1, nodes });
+
+describe("audioFxRevealTarget", () => {
+  it("points a hand-built effect's lane at its own row, by chain index", () => {
+    const c = chain([
+      { type: "highpass", id: "n1", params: {} },
+      { type: "peaking", id: "n2", params: {} },
+    ]);
+    expect(audioFxRevealTarget("fx.n2.gain", c)).toEqual({ kind: "node", index: 1, nodeId: "n2" });
+  });
+
+  // The case that made this a resolver rather than an index lookup: a carve's
+  // bands are filtered out of the rack's node list, so opening `openNode` on
+  // one opens nothing. The carve module is what renders them.
+  it("points a carve band's lane at the carve module", () => {
+    const c = chain([{ type: "peaking", id: "n1", fromCarve: true, params: {} }]);
+    expect(audioFxRevealTarget("fx.n1.gain", c)).toEqual({ kind: "carve" });
+  });
+
+  it("points an EQ band's lane at its EQ module", () => {
+    const c = chain([{ type: "peaking", id: "n1", fromEq: "eq1", params: {} }]);
+    expect(audioFxRevealTarget("fx.n1.gain", c)).toEqual({ kind: "eq", eqId: "eq1" });
+  });
+
+  it("points a preset node's lane at its run, keyed like collapsedRuns", () => {
+    const c = chain([
+      { type: "highpass", id: "n1", params: {} },
+      { type: "peaking", id: "n2", fromPreset: "clean-voice", params: {} },
+      { type: "gain", id: "n3", fromPreset: "clean-voice", params: {} },
+    ]);
+    // Keyed by the run's FIRST node, not the automated one.
+    expect(audioFxRevealTarget("fx.n3.gain", c)).toEqual({
+      kind: "preset",
+      runKey: "clean-voice-1",
+    });
+  });
+
+  it("resolves a preset-level lane through the preset it names", () => {
+    const c = chain([{ type: "peaking", id: "n1", fromPreset: "clean-voice", params: {} }]);
+    expect(audioFxRevealTarget("fx.preset.clean-voice", c)).toEqual({
+      kind: "preset",
+      runKey: "clean-voice-0",
+    });
+  });
+
+  it("treats the track's own volume as the rack itself", () => {
+    expect(audioFxRevealTarget("volume", chain([]))).toEqual({ kind: "volume" });
+  });
+
+  it("resolves nothing for a lane whose effect is gone, or an unparseable target", () => {
+    expect(audioFxRevealTarget("fx.gone.gain", chain([]))).toBeNull();
+    expect(audioFxRevealTarget("nonsense", chain([]))).toBeNull();
+    expect(audioFxRevealTarget("fx.n1.gain", null)).toBeNull();
+  });
+});

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

@@ -0,0 +1,113 @@
+/**
+ * Where in the rack an automation lane's parameter actually lives.
+ *
+ * A lane names `fx.<nodeId>.<param>`, but the rack does not show one flat list
+ * of nodes: the carve is one module standing for the filters it compiled, EQ
+ * bands are folded into their own module, and preset runs are collapsible
+ * groups. A node id therefore resolves to one of several surfaces, and the
+ * caller has to open the RIGHT one — opening `openNode` on a carve band, whose
+ * row is filtered out of `handBuilt`, would open nothing at all and read as the
+ * click doing nothing.
+ */
+
+import { parseAutomationTarget } from "@hyperframes/core/audio-automation";
+import { escapeCssString } from "./domEditingDom";
+import type { HfAudioFxChain } from "@hyperframes/core/audio-fx";
+
+export type AudioFxRevealTarget =
+  /** A hand-built effect, addressed by its index in the chain. */
+  | { kind: "node"; index: number; nodeId: string }
+  /** An EQ module, addressed by the id its bands share. */
+  | { kind: "eq"; eqId: string }
+  /** A preset run, addressed the way `collapsedRuns` keys it. */
+  | { kind: "preset"; runKey: string }
+  /** The carve module, which owns every `fromCarve` node collectively. */
+  | { kind: "carve" }
+  /** The track's own volume — no rack row; the reveal is the rack itself. */
+  | { kind: "volume" };
+
+/**
+ * Resolve a lane target to the surface that shows it, or null when the chain
+ * does not contain it (a stale lane, or one whose effect was removed).
+ */
+export function audioFxRevealTarget(
+  target: string,
+  chain: HfAudioFxChain | null,
+): AudioFxRevealTarget | null {
+  const parsed = parseAutomationTarget(target);
+  if (!parsed) return null;
+  if (parsed.kind === "volume") return { kind: "volume" };
+  if (parsed.kind === "preset") {
+    // A preset-level lane names the preset, not a node inside it: find its run
+    // by the first node that belongs to it, which is how `runKey` is built.
+    const index = (chain?.nodes ?? []).findIndex((node) => node.fromPreset === parsed.presetId);
+    return index >= 0 ? { kind: "preset", runKey: `${parsed.presetId}-${index}` } : null;
+  }
+  const index = (chain?.nodes ?? []).findIndex((node) => node.id === parsed.nodeId);
+  const node = index >= 0 ? chain?.nodes[index] : undefined;
+  if (!node) return null;
+  // Order matters: a carve band can also carry `fromEq`/`fromPreset` tags, and
+  // the carve module is the one that actually renders it.
+  if (node.fromCarve) return { kind: "carve" };
+  if (node.fromEq) return { kind: "eq", eqId: node.fromEq };
+  if (node.fromPreset) {
+    const first = (chain?.nodes ?? []).findIndex((n) => n.fromPreset === node.fromPreset);
+    return { kind: "preset", runKey: `${node.fromPreset}-${first}` };
+  }
+  return { kind: "node", index, nodeId: parsed.nodeId };
+}
+
+/**
+ * The DOM selector for the row a reveal target lives in, or null when the target
+ * names no surface this panel renders.
+ *
+ * A preset run is keyed by the preset id the run element actually exposes, not by
+ * `runKey`, which is the collapse map's key and carries an index suffix.
+ */
+function revealRowSelector(where: AudioFxRevealTarget | null): string | null {
+  if (!where) return null;
+  switch (where.kind) {
+    case "node":
+      return where.nodeId ? `[data-fx-node-id="${escapeCssString(where.nodeId)}"]` : null;
+    case "eq":
+      return `[data-fx-eq="${escapeCssString(where.eqId)}"]`;
+    case "carve":
+      return ".hf-fx-carve-module";
+    case "preset":
+      return `[data-fx-preset="${escapeCssString(where.runKey.replace(/-\d+$/, ""))}"]`;
+    default:
+      return null;
+  }
+}
+
+/**
+ * Scroll the row that owns `target` into view, and report whether it was found.
+ *
+ * The target is resolved against the chain again rather than remembered: which
+ * surface owns a parameter is a fact about the chain, and the chain may have
+ * been edited between the reveal request and the pass that can act on it. A
+ * false return means the row has not mounted yet, so the caller keeps the
+ * request pending.
+ */
+export function scrollRevealedRowIntoView(
+  root: HTMLElement | null,
+  target: string,
+  chain: HfAudioFxChain,
+): boolean {
+  const selector = revealRowSelector(audioFxRevealTarget(target, chain));
+  // `querySelector` throws SyntaxError on a malformed selector, and this runs
+  // inside a render-phase effect — an unescapable id would take the whole
+  // property panel down instead of leaving the request pending, which is what
+  // returning false means. `parseAudioFxNode` accepts any non-empty string as
+  // an id and `parseAutomationTarget` only splits on `.`, so a hand- or
+  // LLM-authored chain can carry one.
+  let row: HTMLElement | null = null;
+  try {
+    row = selector ? (root?.querySelector<HTMLElement>(selector) ?? null) : null;
+  } catch {
+    return false;
+  }
+  if (!row) return false;
+  row.scrollIntoView({ block: "nearest", behavior: "smooth" });
+  return true;
+}

packages/studio/src/components/editor/audioFxSignalPath.test.ts

@@ -0,0 +1,50 @@
+import { describe, expect, it } from "vitest";
+import type { HfAudioGroup } from "@hyperframes/core/audio-groups";
+import { audioFxSignalPath } from "./audioFxSignalPath";
+
+const group = (over: Partial<HfAudioGroup> = {}): HfAudioGroup => ({
+  id: "voiceover",
+  label: "Voiceover",
+  memberIds: ["vo-1", "vo-2"],
+  volume: 1,
+  hidden: false,
+  ...over,
+});
+
+describe("audioFxSignalPath", () => {
+  // The design doc's §5 mockup, both columns.
+  // Copy taken from the rendered designs, which are more specific than the
+  // ASCII stand-ins in the markdown: "vo-1 and vo-2, together" / "into
+  // Voiceover", not "vo-1, vo-2" / "to Voiceover".
+  it("names what a group sums, and sends it to the mix", () => {
+    expect(audioFxSignalPath("hf-audio-group", "voiceover", [group()])).toEqual({
+      inLabel: "vo-1 and vo-2, together",
+      outLabel: "to mix",
+      subject: "group",
+    });
+  });
+
+  it("names the group a member feeds, so routing reads from either end", () => {
+    expect(audioFxSignalPath("audio", "vo-1", [group()])).toEqual({
+      inLabel: "this track",
+      outLabel: "into Voiceover",
+      subject: "track",
+    });
+  });
+
+  it("leaves an ungrouped clip on the shipped clip labels", () => {
+    expect(audioFxSignalPath("audio", "music-bed", [group()])).toEqual({
+      inLabel: "this track",
+      outLabel: "to mix",
+      subject: "track",
+    });
+  });
+
+  // The state an author is in the instant after making a group. It must not
+  // read as a failure to resolve.
+  it("says a memberless group holds nothing yet", () => {
+    expect(
+      audioFxSignalPath("hf-audio-group", "empty", [group({ id: "empty", memberIds: [] })]),
+    ).toMatchObject({ inLabel: "nothing yet", subject: "group" });
+  });
+});

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

@@ -0,0 +1,71 @@
+/**
+ * What the rack's `IN` and `OUT` lines say, per selected element.
+ *
+ * The rack brackets its chain with the signal path because the ORDER is the
+ * point (see `propertyPanelFxRackChain`). Those two lines were hardcoded to a
+ * clip's answer — "in this track", "out to mix" — which is wrong at both ends
+ * once groups exist, and the design doc's §5 mockup spells out both:
+ *
+ *     GROUP: Voiceover          CLIP: vo-1
+ *     IN   vo-1, vo-2           IN   this track
+ *     OUT  to mix               OUT  to Voiceover
+ *
+ * A group's IN names what it sums, which is the only thing on screen that says
+ * a bus is a sum rather than a copy of the chain on each member. A member's OUT
+ * names the group it feeds, "so the routing is readable from either end".
+ */
+
+import type { HfAudioGroup } from "@hyperframes/core/audio-groups";
+
+export interface AudioFxSignalPath {
+  /** After the word "In". */
+  inLabel: string;
+  /** After the word "Out". */
+  outLabel: string;
+  /** The thing the empty-state sentence is about: "No effects on this …". */
+  subject: string;
+}
+
+/** What a plain, ungrouped clip has always said, and the default everywhere. */
+export const CLIP_SIGNAL_PATH: AudioFxSignalPath = {
+  inLabel: "this track",
+  outLabel: "to mix",
+  subject: "track",
+};
+
+/**
+ * `groups` is the resolved set from the composition; `elementId` and `tag` come
+ * from the selection. Pure so the labels can be asserted without a DOM.
+ */
+/** "a", "a and b", "a, b and c" — how the designs read a member list aloud. */
+function joinNatural(items: readonly string[]): string {
+  if (items.length <= 1) return items[0] ?? "";
+  return `${items.slice(0, -1).join(", ")} and ${items[items.length - 1]}`;
+}
+
+export function audioFxSignalPath(
+  tag: string | undefined,
+  elementId: string | undefined,
+  groups: readonly HfAudioGroup[],
+): AudioFxSignalPath {
+  if (tag === "hf-audio-group") {
+    const group = groups.find((g) => g.id === elementId);
+    // A group with no members yet still reads as a group — "nothing yet" is the
+    // honest answer, and it is also the state the author is in right after
+    // making one, so it must not look like a bug.
+    const members = group?.memberIds ?? [];
+    return {
+      // "vo-1 and vo-2, together" — the rendered design's exact phrasing, not a
+      // comma list. The trailing "together" is the point: it says the group is
+      // ONE signal hearing both, which is the thing two separate copies of a
+      // chain cannot do, and it says it without "sum" or "bus".
+      inLabel: members.length > 0 ? `${joinNatural(members)}, together` : "nothing yet",
+      outLabel: "to mix",
+      subject: "group",
+    };
+  }
+  const owner = elementId ? groups.find((g) => g.memberIds.includes(elementId)) : undefined;
+  // "into Voiceover", not "to" — a member feeds the group, and the design uses
+  // the preposition that says so.
+  return owner ? { ...CLIP_SIGNAL_PATH, outLabel: `into ${owner.label}` } : CLIP_SIGNAL_PATH;
+}

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

@@ -8,6 +8,14 @@ const el = (dataAttributes: Record<string, string>): DomEditSelection =>
 const chain = (nodes: unknown[]) => JSON.stringify({ version: 1, nodes });
 
 describe("audioFxSummary", () => {
+  // The designs give a member's rack the summary "in Voiceover" — it answers
+  // "where does this go?" before anything is opened, the same job the rack's
+  // OUT does from the other end, and it outranks the effect count because a
+  // member with no effects of its own is still in the group.
+  it("names the group a clip belongs to, ahead of any effect count", () => {
+    expect(audioFxSummary(el({}), "Voiceover")).toBe("in Voiceover");
+  });
+
   it("counts a carve as one module, not as the filters behind it", () => {
     // Six bands and a level stage reading "7 effects" is the misreading the
     // grouping exists to prevent.

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

@@ -10,27 +10,39 @@
 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?.[HF_AUDIO_FX_DATA_KEY];
-  const carveAttr = element.dataAttributes?.["fx-carve"];
-  let handBuilt = 0;
-  let carveNodes = 0;
-  if (raw) {
-    try {
-      for (const node of parseAudioFxChain(raw).nodes) {
-        if (node.enabled === false) continue;
-        if (node.fromCarve) carveNodes += 1;
-        else handBuilt += 1;
-      }
-    } catch {
-      return "unreadable";
+/** Enabled nodes split by who authored them, or null when the chain won't parse. */
+function countEnabledNodes(raw: string | undefined): { handBuilt: number; carve: number } | null {
+  if (!raw) return { handBuilt: 0, carve: 0 };
+  try {
+    let handBuilt = 0;
+    let carve = 0;
+    for (const node of parseAudioFxChain(raw).nodes) {
+      if (node.enabled === false) continue;
+      if (node.fromCarve) carve += 1;
+      else handBuilt += 1;
     }
+    return { handBuilt, carve };
+  } catch {
+    return null;
   }
+}
+
+export function audioFxSummary(element: DomEditSelection, groupLabel?: string): string {
+  // A clip inside a group reads "in Voiceover" — the designs use this line to
+  // answer "where does this go?" before the author opens anything, which is
+  // the same job the rack's OUT does from the other end. It outranks the effect
+  // count: a member with no effects of its own is still IN the group, and that
+  // is the more useful thing to say about it.
+  if (groupLabel) return `in ${groupLabel}`;
+  const counts = countEnabledNodes(element.dataAttributes?.[HF_AUDIO_FX_DATA_KEY]);
+  if (!counts) return "unreadable";
   const parts: string[] = [];
-  if (handBuilt > 0) parts.push(`${handBuilt} effect${handBuilt === 1 ? "" : "s"}`);
+  if (counts.handBuilt > 0) {
+    parts.push(`${counts.handBuilt} effect${counts.handBuilt === 1 ? "" : "s"}`);
+  }
   // One name for the module however many filters are behind it. Named when the
   // carve is switched on at all, because the control is in this section whether or
   // not it has compiled to anything yet.
-  if (carveNodes > 0 || carveAttr) parts.push("carve");
+  if (counts.carve > 0 || element.dataAttributes?.["fx-carve"]) parts.push("carve");
   return parts.length > 0 ? parts.join(" + ") : "none";
 }