fix(studio): make sub-composition audio groups actually work

Browser-verified the one surface that had never been run: a group and
its members declared entirely inside a sub-composition. Both fixes
written for that case were broken, and both of their tests passed —
because I wrote fixtures that matched my assumption instead of the DOM.

Membership never arrived. `hostElementState` inherits from the flat store
twin, and a sub-comp that declares its own group keeps those members OUT
of the flat store — the store held three elements (panel, sub-comp host,
bed) and neither voice. So there was nothing to inherit from and no group
row appeared at all. Membership now rides `DomClipChild`, captured during
the DOM walk that is the only place holding the child's live element,
with the flat twin still preferred when it exists.

Routing never worked either. `getTimelineElementSourceFile` stops at the
nearest `[data-composition-id]`, which for an inlined sub-composition is
its own ROOT element — that carries the composition id but not the file.
The file sits on the HOST above it:

  hf-audio-group#voiceover   (no composition attrs)
  section#voices-root        data-composition-id="voices"           <- stopped here
  div#voices-host            data-composition-file="...voices.html" <- file is here
  body                       data-composition-id="<root>"

My unit fixture put the file on the sub-comp root, so the test passed
while the studio still threw "Unable to patch element in index.html" on
every mute, fader move and FX preset. The resolver climbs composition
ancestors until one names a file, and returns undefined for a root-level
group so the caller still falls back to activeCompPath.

The new tests use the ancestor shape copied from a live preview, and both
fixes were mutation-checked. Verified end to end in the studio: the group
row appears with its members nested, mute writes `data-hidden` into
compositions/voices.html and not index.html, unmute removes it, and the
fader writes data-volume="0.35" to the same file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-08-20 16:39:43 -07:00
co-authored by Claude Opus 5
parent 6056ec7463
commit 6de75fd4b4
6 changed files with 208 additions and 14 deletions
@@ -16,7 +16,7 @@
import { afterEach, describe, expect, it } from "vitest";
import { usePlayerStore, type TimelineElement } from "../player";
import { useSetAudioGroupAttribute } from "./timelineAudioGroupVolume";
import { resolveGroupSourceFile, useSetAudioGroupAttribute } from "./timelineAudioGroupVolume";
afterEach(() => {
usePlayerStore.getState().reset();
@@ -128,3 +128,48 @@ describe("group attribute writes reach the store", () => {
expect(byId.get("sfx")?.audioGroupVolume).toBe(1);
});
});
/**
* The ancestor shape here is COPIED FROM A LIVE PREVIEW, not imagined.
*
* The first attempt at this fix used `getTimelineElementSourceFile` and a
* fixture in which the sub-composition root carried `data-composition-file`.
* The test passed; the studio still threw "Unable to patch element in
* 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("resolveGroupSourceFile", () => {
function livePreviewShape(): Document {
const doc = document.implementation.createHTMLDocument("preview");
doc.body.setAttribute("data-composition-id", "subcomp-group-qa");
doc.body.innerHTML = `
<div id="voices-host" data-composition-id="voices-host" data-composition-file="compositions/voices.html">
<section id="voices-root" data-composition-id="voices">
<hf-audio-group id="voiceover"></hf-audio-group>
</section>
</div>
<hf-audio-group id="root-group"></hf-audio-group>
`;
return doc;
}
it("climbs past the sub-comp root to the host that names the file", () => {
const doc = livePreviewShape();
expect(resolveGroupSourceFile(doc.getElementById("voiceover"))).toBe(
"compositions/voices.html",
);
});
// A group in the root composition: body names an id but no file, so the
// caller falls back to activeCompPath. Returning body's id here would route
// every root-composition group write at a path that does not exist.
it("returns undefined for a group in the root composition", () => {
const doc = livePreviewShape();
expect(resolveGroupSourceFile(doc.getElementById("root-group"))).toBeUndefined();
});
it("is safe on a detached or missing element", () => {
expect(resolveGroupSourceFile(null)).toBeUndefined();
expect(resolveGroupSourceFile(document.createElement("hf-audio-group"))).toBeUndefined();
});
});
@@ -3,7 +3,6 @@ import { HF_AUDIO_FX_ATTR } from "@hyperframes/core/audio-fx";
import { usePlayerStore } from "../player";
import type { TimelineElementPatch } from "../player/store/timelineElement";
import { invalidateGroupInfoCache } from "../player/lib/timelineGroupInfo";
import { getTimelineElementSourceFile } from "../player/lib/timelineElementHelpers";
import {
buildPatchTarget,
persistElementAttribute,
@@ -83,6 +82,36 @@ function syncStoredGroupAttribute(groupId: string, attr: string, value: string |
}));
}
/**
* The composition FILE that contains a group element, walking up through
* composition ancestors until one names a file.
*
* `getTimelineElementSourceFile` stops at the nearest `[data-composition-id]`,
* which for an inlined sub-composition is its own ROOT element — that carries
* the composition id but not the file. The file is on the HOST one level above
* it. Measured on a live preview:
*
* hf-audio-group#voiceover (no composition attrs)
* section#voices-root data-composition-id="voices" <- stops here
* div#voices-host data-composition-file="…/voices.html" <- file is here
* body data-composition-id="<root>"
*
* Returns undefined for a group in the root composition (body names an id but
* no file), which is exactly when the caller should fall back to activeCompPath.
*/
export function resolveGroupSourceFile(groupEl: Element | null): string | undefined {
let node: Element | null = groupEl?.parentElement ?? null;
while (node) {
const owner: Element | null = node.closest("[data-composition-id]");
if (!owner) return undefined;
const file =
owner.getAttribute("data-composition-file") ?? owner.getAttribute("data-composition-src");
if (file) return file;
node = owner.parentElement;
}
return undefined;
}
interface SetAudioGroupAttributeInput {
projectId: string;
activeCompPath: string | null;
@@ -125,8 +154,7 @@ async function setAudioGroupAttribute({
// FX preset throws "Unable to patch element in index.html". Every sibling
// timeline writer already routes `element.sourceFile || activeCompPath`.
const groupEl = previewIframe?.contentDocument?.getElementById(groupId) ?? null;
const targetPath =
(groupEl ? getTimelineElementSourceFile(groupEl) : undefined) || activeCompPath || "index.html";
const targetPath = resolveGroupSourceFile(groupEl) || activeCompPath || "index.html";
const patchTarget = buildPatchTarget({ domId: groupId });
if (!patchTarget) return [];
@@ -598,4 +598,67 @@ describe("buildExpandedElements — collision-free synthetic rows (cross-file la
// Distinct ordered rows per child.
expect(children[0].track).not.toBe(children[1].track);
});
/**
* A sub-composition that declares BOTH a group and its members keeps those
* members out of the flat store entirely — the store holds only the host.
* So "inherit membership from the flat twin" had nothing to inherit from,
* and the group produced no timeline row at all, for exactly the case group
* support was extended to cover. Verified against a real studio session
* before this test was written: the flat store held three elements (the
* panel, the sub-comp host and an ungrouped bed) and neither voice.
*/
it("takes group membership from the DOM child when there is no flat store twin", () => {
const elements = [
el({ id: "voices-host", start: 0, duration: 12, compositionSrc: "voices.html" }),
];
const manifest = [
clip({ id: "voices-host", start: 0, duration: 12, compositionSrc: "voices.html" }),
];
const parentMap = new Map([
["voice-1", "voices-host"],
["voice-2", "voices-host"],
]);
const domClipChildren = [
{
id: "voice-1",
parentId: "voices-host",
hostId: "voices-host",
label: "voice-1",
stackingContextId: "css:0",
audioGroup: "voiceover",
audioGroupLabel: "Voiceover",
audioGroupVolume: 0.8,
audioGroupHidden: false,
},
{
id: "voice-2",
parentId: "voices-host",
hostId: "voices-host",
label: "voice-2",
stackingContextId: "css:0",
audioGroup: "voiceover",
audioGroupLabel: "Voiceover",
audioGroupVolume: 0.8,
audioGroupHidden: false,
},
];
const out = buildExpandedElements(
elements,
manifest,
parentMap,
"voices-host",
"voices-host",
domClipChildren,
);
const voices = out.filter((e) => e.domId?.startsWith("voice-"));
expect(voices).toHaveLength(2);
for (const voice of voices) {
expect(voice.audioGroup).toBe("voiceover");
expect(voice.audioGroupLabel).toBe("Voiceover");
expect(voice.audioGroupVolume).toBeCloseTo(0.8, 6);
}
});
});
@@ -146,6 +146,31 @@ interface DisplayBounds {
* could never be shown again (not even after a reload, since the attribute is in
* the source).
*/
/**
* Audio-group membership for an expanded child, from whichever source has it.
*
* The flat store twin when there is one; otherwise the `DomClipChild` record,
* which carried it off the live element during the DOM walk. That fallback is
* the ONLY source for a sub-composition that declares both a group and its
* members: those members never enter the flat store, so "inherit from the flat
* twin" silently produced no membership and therefore no group row — for
* exactly the case group support was extended to cover.
*/
function childGroupState(
flat: TimelineElement | undefined,
domChild: DomClipChild | undefined,
): Partial<TimelineElement> {
const source = flat?.audioGroup ? flat : domChild?.audioGroup ? domChild : null;
if (!source) return {};
return {
audioGroup: source.audioGroup,
audioGroupLabel: source.audioGroupLabel,
audioGroupVolume: source.audioGroupVolume,
audioGroupHidden: source.audioGroupHidden,
audioGroupFxChain: source.audioGroupFxChain,
};
}
function hostElementState(flat: TimelineElement | undefined): Partial<TimelineElement> {
if (!flat) return {};
return {
@@ -159,16 +184,6 @@ function hostElementState(flat: TimelineElement | undefined): Partial<TimelineEl
// still showed the chain and its toggles.
fxChain: flat.fxChain,
automation: flat.automation,
// And the same again for group membership. Without these an expanded
// sub-comp child had `audioGroup === undefined`, so `resolveGroupMembership`
// saw no members and emitted NO group row — for a group whose members are
// all sub-comp children, the group simply did not exist in the timeline,
// even though the carve would happily create one for exactly those clips.
audioGroup: flat.audioGroup,
audioGroupLabel: flat.audioGroupLabel,
audioGroupVolume: flat.audioGroupVolume,
audioGroupHidden: flat.audioGroupHidden,
audioGroupFxChain: flat.audioGroupFxChain,
};
}
@@ -182,6 +197,7 @@ function buildChildElements(
editBasis: { start: number; sourceFile: string | undefined },
expandedHostKey: string,
elements: readonly TimelineElement[],
domChildrenById: ReadonlyMap<string, DomClipChild>,
): TimelineElement[] {
const result: TimelineElement[] = [];
for (const child of siblings) {
@@ -210,6 +226,10 @@ function buildChildElements(
result.push({
...base,
...hostElementState(elements.find((element) => element.key === key)),
...childGroupState(
elements.find((element) => element.key === key),
domId ? domChildrenById.get(domId) : undefined,
),
key,
start: clamped.start,
duration: clamped.duration,
@@ -312,6 +332,7 @@ export function buildExpandedElements(
};
const parentKey = topLevelElement.key ?? topLevelElement.id;
const domChildrenById = new Map(domClipChildren.map((child) => [child.id, child]));
const expanded = buildChildElements(
siblings,
{
@@ -322,6 +343,7 @@ export function buildExpandedElements(
editBasis,
parentKey,
elements,
domChildrenById,
);
if (expanded.length === 0) return filterToTopLevel(elements, parentMap);
@@ -12,6 +12,8 @@ import { useCallback } from "react";
import { liveTime, usePlayerStore } from "../store/playerStore";
import type { TimelineElement, DomClipChild } from "../store/playerStore";
import { resolveCssStackingContextId } from "@hyperframes/core/runtime/stacking-context";
import { HF_AUDIO_GROUP_ATTR } from "@hyperframes/core/audio-groups";
import { groupInfoFor } from "../lib/timelineGroupInfo";
import type { PlaybackAdapter, ClipManifestClip, IframeWindow } from "../lib/playbackTypes";
import {
parseTimelineFromDOM,
@@ -86,6 +88,28 @@ export function resolveReloadSeekTime(input: {
return Math.min(target, input.duration);
}
/**
* A sub-comp child's audio-group membership, read off its live element.
*
* Captured during the DOM walk because that walk holds the only reference to
* the element. A sub-composition that declares both a group and its members
* keeps those members out of the flat store entirely, so an expanded child has
* no flat twin to inherit membership from later — without this, a group defined
* inside a sub-composition produced no group row at all.
*/
function readChildAudioGroupState(child: Element): Partial<DomClipChild> {
const audioGroup = child.getAttribute(HF_AUDIO_GROUP_ATTR);
if (!audioGroup) return {};
const info = groupInfoFor(child.ownerDocument, audioGroup);
return {
audioGroup,
audioGroupLabel: info.label,
audioGroupVolume: info.volume,
audioGroupHidden: info.hidden,
...(info.fxChain ? { audioGroupFxChain: info.fxChain } : {}),
};
}
/** Reject non-finite, non-positive, and absurdly large (loop-inflated) values. */
function sanitizeDurationSeconds(value: number): number {
return Number.isFinite(value) && value > 0 && value < 7200 ? value : 0;
@@ -201,6 +225,7 @@ export function useTimelineSyncCallbacks({
hostId,
label: isGroup ? child.getAttribute("data-hf-group") || child.id : child.id,
stackingContextId: resolveCssStackingContextId(child),
...readChildAudioGroupState(child),
});
parentMap.set(child.id, parentId);
if (isGroup) collect(child, child.id);
@@ -234,6 +234,17 @@ export interface DomClipChild {
hostId: string;
label: string;
stackingContextId: string;
/**
* The child's audio-group state, read off its live element during the DOM
* walk — the only place that sees it. A sub-composition can declare a group
* and its members entirely within itself, and those members never reach the
* flat store, so an expanded child has no twin to inherit membership from.
*/
audioGroup?: string;
audioGroupLabel?: string;
audioGroupVolume?: number;
audioGroupHidden?: boolean;
audioGroupFxChain?: string;
}
interface BeatHistoryEntry {