fix(studio): unwind to the file's value, and mirror a group write to sub-comp members

Review findings 15 and 14.

**15 — the unwind restored the value it was supposed to undo.** `previousValue`
came from `readLive()`, but every live-write caller patches the DOM BEFORE
committing: a fader drag is `setLive` per frame, hovering a preset auditions the
whole chain. So by commit time the live DOM already held the in-progress value,
`previousValue === value`, and the unwind was a no-op — and `setQuiet`'s catch,
which deliberately re-mirrors the store off the live DOM, then mirrored that
same never-saved value. The group audibly had the preset, the panel agreed, and
a reload dropped it. It reads the value out of `before` — the file content the
target check just fetched — with `readAttributeByTarget`, which is file truth.

`readLive` is gone from the input shape: it existed only for this, and leaving
it would invite the same mistake back. Both callers drop it.

**14 — the mirror was a no-op for a group declared in a sub-composition.** Those
members never enter the flat store (`childGroupState` keeps them out), so their
`audioGroup*` fields come from the `DomClipChild` record — and
`syncStoredGroupAttribute` only mapped `elements`. The header kept the pre-write
chain, its FX button showed the old count, and `laneCount` stayed 0 so the lane
disclosure never appeared for automation that now existed. Verbatim the symptom
that function's own docblock claims to have fixed, fixed only for flat members.
It now writes both, and only touches `domClipChildren` when the group actually
has one (no notification for the common flat case).

Two tests, each verified against a revert. studio: 390 files, 4389 tests.
fallow clean.

packages/studio/src/hooks/timelineAudioGroupVolume.test.ts

@@ -138,6 +138,35 @@ describe("group attribute writes reach the store", () => {
  * index.html", because the runtime inlines a sub-comp as its own root element
  * that carries only the composition ID — the FILE is on the host above it.
  */
+describe("the mirror reaches sub-composition members", () => {
+  /**
+   * A group declared inside a sub-composition has no FLAT member to mirror onto
+   * — `childGroupState` keeps those members out of `elements` — so mirroring
+   * only `elements` made this whole function a no-op for it: the header kept the
+   * pre-write chain and `laneCount` stayed 0, so the lane disclosure never
+   * appeared for automation that now existed.
+   */
+  it("mirrors a group write onto DomClipChild members as well as flat ones", async () => {
+    const react = await import("react");
+    const { renderToStaticMarkup } = await import("react-dom/server");
+    const harness = makeSetter();
+    renderToStaticMarkup(react.createElement(harness.Probe));
+    const setter = harness.get();
+    usePlayerStore.getState().setElements([member("flat-1", 0)]);
+    usePlayerStore.getState().setDomClipChildren([
+      { id: "sub-1", parentId: "host", hostId: "host", label: "Sub 1", audioGroup: "voiceover" },
+      { id: "other", parentId: "host", hostId: "host", label: "Other", audioGroup: "sfx" },
+    ]);
+
+    setter?.setLive("voiceover", "data-label", "Voices");
+
+    const children = usePlayerStore.getState().domClipChildren;
+    expect(children.find((c) => c.id === "sub-1")?.audioGroupLabel).toBe("Voices");
+    // A member of another group is untouched.
+    expect(children.find((c) => c.id === "other")?.audioGroupLabel).toBeUndefined();
+  });
+});
+
 describe("resolveGroupSourceFile", () => {
   function livePreviewShape(): Document {
     const doc = document.implementation.createHTMLDocument("preview");

packages/studio/src/hooks/timelineAudioGroupVolume.ts

@@ -79,9 +79,28 @@ function syncStoredGroupAttribute(groupId: string, attr: string, value: string |
   // a 500-clip composition was 1500 object spreads and 3 store notifications per
   // drag frame — at ~60/s, with every `elements`-keyed memo downstream
   // recomputing each time.
-  usePlayerStore.setState((state) => ({
-    elements: state.elements.map((el) => (el.audioGroup === groupId ? { ...el, ...patch } : el)),
-  }));
+  // BOTH stores, because a group declared inside a sub-composition has no flat
+  // member to mirror onto: `childGroupState` keeps those members out of
+  // `elements` entirely, so their `audioGroup*` fields come from the
+  // `DomClipChild` record instead. Mirroring only `elements` made this whole
+  // function a no-op for such a group — the header kept the pre-write chain, its
+  // FX button showed the old count, and `laneCount` stayed 0 so the lane
+  // disclosure never appeared for automation that now existed. Verbatim the
+  // symptom this docblock claims to have fixed, fixed only for flat members.
+  //
+  // `setDomClipChildren` has one other writer, inside `processTimelineMessage`,
+  // which a live attribute patch does not trigger.
+  usePlayerStore.setState((state) => {
+    const next: Partial<typeof state> = {
+      elements: state.elements.map((el) => (el.audioGroup === groupId ? { ...el, ...patch } : el)),
+    };
+    if (state.domClipChildren.some((child) => child.audioGroup === groupId)) {
+      next.domClipChildren = state.domClipChildren.map((child) =>
+        child.audioGroup === groupId ? { ...child, ...patch } : child,
+      );
+    }
+    return next;
+  });
 }
 
 /**
@@ -172,8 +191,6 @@ async function setAudioGroupAttribute({
     domEditSaveTimestampRef,
     pendingTimelineEditPathRef,
     patchLive: (v) => patchLiveGroupAttribute(previewIframe, groupId, attr, v),
-    readLive: () =>
-      previewIframe?.contentDocument?.getElementById(groupId)?.getAttribute(attr) ?? null,
   });
 }
 

packages/studio/src/hooks/timelineEditingHelpers.test.ts

@@ -455,7 +455,6 @@ describe("persistElementAttribute", () => {
         domEditSaveTimestampRef: { current: 0 },
         pendingTimelineEditPathRef: { current: new Set() },
         patchLive,
-        readLive: () => null,
       }),
     ).rejects.toThrow("Unable to patch element in index.html");
 
@@ -464,3 +463,49 @@ describe("persistElementAttribute", () => {
     fetchSpy.mockRestore();
   });
 });
+
+describe("persistElementAttribute — unwind value", () => {
+  /**
+   * The unwind has to restore the value on DISK, not the one in the preview.
+   *
+   * Every live-write caller patches the DOM before committing (a fader drag is
+   * `setLive` per frame; hovering a preset auditions the whole chain), so by
+   * commit time the live DOM already holds the in-progress value. Reading it as
+   * `previousValue` made the unwind a no-op, and the group writer's catch —
+   * which deliberately re-mirrors the store off the live DOM — then mirrored the
+   * never-saved value: the panel agreed with the preview, and a reload dropped it.
+   */
+  it("restores the file's value, not the audition already in the live DOM", async () => {
+    const fetchSpy = vi
+      .spyOn(globalThis, "fetch")
+      .mockResolvedValue(
+        new Response(
+          JSON.stringify({ content: `<body><audio id="bgm" data-volume="0.25"></audio></body>` }),
+        ),
+      );
+    const patched: Array<string | null> = [];
+    const writeProjectFile = vi.fn(() => Promise.reject(new Error("save failed")));
+
+    await expect(
+      persistElementAttribute({
+        projectId: "p",
+        targetPath: "index.html",
+        patchTarget: { id: "bgm" },
+        attr: "data-volume",
+        value: "0.9",
+        label: "Set volume",
+        writeProjectFile,
+        recordEdit: vi.fn(),
+        domEditSaveTimestampRef: { current: 0 },
+        pendingTimelineEditPathRef: { current: new Set() },
+        // The live DOM is ALREADY at the new value when the commit runs — that
+        // is what `setLive` does on every drag frame.
+        patchLive: (v) => patched.push(v),
+      }),
+    ).rejects.toThrow("save failed");
+
+    // First the optimistic write, then the unwind — back to what the FILE said.
+    expect(patched).toEqual(["0.9", "0.25"]);
+    fetchSpy.mockRestore();
+  });
+});

packages/studio/src/hooks/timelineEditingHelpers.ts

@@ -410,8 +410,6 @@ export interface PersistElementAttributeInput {
   pendingTimelineEditPathRef: { current: Set<string> };
   /** Write the attribute directly on the live preview DOM node. */
   patchLive: (value: string | null) => void;
-  /** Read the attribute's current value off the live preview DOM node. */
-  readLive: () => string | null;
 }
 
 /**
@@ -420,7 +418,7 @@ export interface PersistElementAttributeInput {
  * `setAudioGroupAttribute` (a group id addressed by its own DOM id) and
  * `useSetElementAttribute` (an arbitrary timeline clip) — same shape, only
  * how the live node is found and where the patch target resolves to differs,
- * which is exactly what `patchLive`/`readLive`/`patchTarget` parameterize.
+ * which is exactly what `patchLive`/`patchTarget` parameterize.
  */
 export async function persistElementAttribute({
   projectId,
@@ -434,10 +432,7 @@ export async function persistElementAttribute({
   domEditSaveTimestampRef,
   pendingTimelineEditPathRef,
   patchLive,
-  readLive,
 }: PersistElementAttributeInput): Promise<string[]> {
-  const previousValue = readLive();
-
   // Resolve the target BEFORE patching the live DOM. The optimistic patch used
   // to run first, and only the save was wrapped in the unwind — so an
   // unresolvable target threw with the live preview (and, through the callers'
@@ -447,6 +442,17 @@ export async function persistElementAttribute({
   if (readTagSnippetByTarget(before, patchTarget) === undefined) {
     throw new Error(`Unable to patch element in ${targetPath}`);
   }
+  // The unwind value comes from the FILE, not from `readLive()`.
+  //
+  // Every live-write caller patches the DOM before committing — a fader drag is
+  // `setLive` per frame, hovering a preset auditions the whole chain — so by the
+  // time this runs the live DOM already holds the in-progress value. Reading it
+  // here made `previousValue === value`, so the unwind below was a no-op, and
+  // `setQuiet`'s catch (which deliberately re-mirrors the store from the live
+  // DOM) then mirrored that same never-saved value. The group audibly had the
+  // preset, the panel agreed, and a reload dropped it — the failure class the
+  // target check above was added to close, still open on the live-write path.
+  const previousValue = readAttributeByTarget(before, patchTarget, attr) ?? null;
   patchLive(value);
 
   const operation: PatchOperation = { type: "attribute", property: attr, value };

packages/studio/src/hooks/timelineElementFxAttribute.ts

@@ -75,9 +75,6 @@ async function setElementAttribute({
     domEditSaveTimestampRef,
     pendingTimelineEditPathRef,
     patchLive: (v) => patchLiveElementAttribute(previewIframe, element, attr, v, activeCompPath),
-    readLive: () =>
-      findTimelineElementInIframe(previewIframe, element, activeCompPath)?.getAttribute(attr) ??
-      null,
   });
 }