fix(studio): group rows survive a missing provider, a collapse, an edit and a reload

Findings 12-15 from the review. All four are group/solo UI state, none
reachable before the id-space fix made group writes work at all.

14 — TimelineGroupRow called the THROWING useTimelineEditContext where
every sibling row calls the optional one. Timeline.test.ts already
asserted the timeline renders outside the provider; it passed because
its fixture had no groups. One grouped clip took the whole timeline
render down, not just the row. The new test is that assertion with a
group on screen, and it fails against the old hook.

13 — A collapsed group left its members in `tracks`, so row geometry
reserved a full row each while buildTimelineLogicalRows had already
stopped emitting them: TimelineLanes rendered null into reserved space,
giving a group header trailed by its members' worth of blank,
unreachable dead space. Members are now emitted only while the group is
expanded, so the row list and the logical rows agree by construction.
(Zeroing the heights instead does not work — createTimelineRowGeometry
deliberately reads a non-positive height as "invalid, use TRACK_H".)
Membership still lands in trackGroupOf: collapsed is hidden, not
ungrouped.

12 — groupInfoCache is a WeakMap keyed on the preview Document, and
group edits are applied as live patches precisely so the iframe never
reloads, so the key never changed and the entry never dropped. A muted
group could not be unmuted (the header kept reading the cached
`hidden:false` and re-wrote data-hidden), the bus slider snapped back,
and a second FX preset built on a stale chain, discarding the first.
Every live group write now drops the entry.

15 — Solo leaked two ways. createTimelineResetState never cleared
`soloed`, so switching composition carried ids that match nothing in the
new document — which is exactly the state that silences every track
while the banner still names a clip that is not there. And the solo
bridge's effect deps do not change across a preview reload, so the
reloaded runtime kept an empty set while the button stayed lit and the
banner still claimed "Hearing only X". Solo now rides
applyPreviewAudioFlags, the same reload-surviving path as the mute and
canary state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-08-20 16:39:36 -07:00
co-authored by Claude Opus 5
parent a8d27820ba
commit ce6426ddcb
13 changed files with 285 additions and 16 deletions
@@ -12,6 +12,7 @@ import { useExpandedTimelineElements } from "../player/hooks/useExpandedTimeline
import { saveProjectFilesWithHistory } from "../utils/studioFileHistory";
import { HF_AUDIO_GROUP_ATTR, HF_AUDIO_GROUP_TAG } from "@hyperframes/core/audio-groups";
import { runtimeAudioId } from "../player/lib/timelineElementHelpers";
import { invalidateGroupInfoCache } from "../player/lib/timelineDOM";
import { readTagSnippetByTarget, type PatchOperation } from "../utils/sourcePatcher";
import {
applyPatchByTarget,
@@ -39,6 +40,7 @@ function patchLiveAudioGroupState(
if (groupId) target.setAttribute(HF_AUDIO_GROUP_ATTR, groupId);
else target.removeAttribute(HF_AUDIO_GROUP_ATTR);
}
invalidateGroupInfoCache(iframe?.contentDocument);
}
/** Group ids are interpolated into markup and into a render-side filename, so
@@ -77,6 +79,7 @@ function patchLiveGroupElement(iframe: HTMLIFrameElement | null, groupId: string
const el = doc.createElement(HF_AUDIO_GROUP_TAG);
el.id = groupId;
doc.body.appendChild(el);
invalidateGroupInfoCache(doc);
return true;
}
@@ -1,4 +1,5 @@
import { useCallback } from "react";
import { invalidateGroupInfoCache } from "../player/lib/timelineDOM";
import {
buildPatchTarget,
persistElementAttribute,
@@ -21,6 +22,7 @@ function patchLiveGroupAttribute(
if (!target) return;
if (value === null) target.removeAttribute(attr);
else target.setAttribute(attr, value);
invalidateGroupInfoCache(iframe?.contentDocument);
}
interface SetAudioGroupAttributeInput {
@@ -46,10 +46,17 @@ vi.mock("./timelineRowVirtualizationFlag", () => ({
STUDIO_TIMELINE_ROW_VIRTUALIZATION_ENABLED: false,
}));
/** Enrolled in nothing by default, matching a user outside every canary. */
const enabledCanaries = new Set<string>();
vi.mock("../../telemetry/canary", () => ({
isCanaryEnabled: (name: string) => enabledCanaries.has(name),
}));
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
afterEach(() => {
document.body.innerHTML = "";
enabledCanaries.clear();
usePlayerStore.getState().reset();
});
@@ -299,6 +306,44 @@ describe("Timeline provider boundary", () => {
act(() => root.unmount());
});
// The same assertion, with a group present. It passed on an empty fixture
// while TimelineGroupRow called the THROWING context hook — one grouped clip
// and the whole timeline render died, not just the row.
it("renders without the provider even when a group row is on screen", () => {
enabledCanaries.add("audio-groups");
const host = createSizedTimelineHost(640);
usePlayerStore.setState({
duration: 4,
timelineReady: true,
elements: [
{
id: "voice-1",
domId: "voice-1",
tag: "audio",
start: 0,
duration: 2,
track: 0,
audioGroup: "voiceover",
},
{
id: "voice-2",
domId: "voice-2",
tag: "audio",
start: 2,
duration: 2,
track: 1,
audioGroup: "voiceover",
},
],
expandedGroupIds: new Set(["voiceover"]),
});
const root = createRoot(host);
act(() => root.render(React.createElement(Timeline)));
expect(host.querySelector('[role="treegrid"]')).not.toBeNull();
act(() => root.unmount());
});
it("renders the complete track list while row virtualization is gated off", () => {
const host = createSizedTimelineHost(640);
usePlayerStore.setState({
@@ -15,7 +15,7 @@ import { TimelineGroupHeader } from "./TimelineGroupHeader";
import { TimelineGroupBusStrip } from "./TimelineGroupBusStrip";
import { groupAutomationLanes } from "./automationLaneData";
import { LABEL_COL_W } from "./timelineLayout";
import { useTimelineEditContext } from "../../contexts/TimelineEditContext";
import { useTimelineEditContextOptional } from "../../contexts/TimelineEditContext";
import { useDomEditActionsContextOptional } from "../../contexts/DomEditContext";
interface TimelineGroupRowProps {
@@ -62,7 +62,12 @@ export function TimelineGroupRow({
return owner?.label ?? owner?.id ?? `track ${i + 1}`;
});
const isLaneOpen = expandedLaneOwnerIds.has(group.id);
const { onSetAudioGroupAttributeLive, onSetAudioGroupAttributeQuiet } = useTimelineEditContext();
// Optional, like every sibling row: Timeline renders outside the edit
// provider in read-only hosts (Timeline.test.ts asserts it), and the throwing
// hook took the whole timeline down with it the moment a group existed —
// not just this row.
const { onSetAudioGroupAttributeLive, onSetAudioGroupAttributeQuiet } =
useTimelineEditContextOptional();
const domEditActions = useDomEditActionsContextOptional();
const soloed = usePlayerStore((s) => s.soloed);
const toggleSolo = usePlayerStore((s) => s.toggleSolo);
@@ -1,5 +1,5 @@
import { useMemo } from "react";
import type { TimelineElement } from "../store/playerStore";
import { usePlayerStore, type TimelineElement } from "../store/playerStore";
import { isCanaryEnabled } from "../../telemetry/canary";
import { getTrackStyle, type TrackVisualStyle } from "./timelineIcons";
@@ -86,17 +86,27 @@ function buildGroupInfo(
};
}
/** Push a group's synthetic anchor row plus its members' rows, contiguously. */
/**
* Push a group's synthetic anchor row plus its members' rows, contiguously.
*
* A collapsed group emits its anchor and nothing else. `buildTimelineLogicalRows`
* already stops at the anchor when collapsed, so leaving the member rows in
* `tracks` left row geometry reserving a full row each for rows that then
* rendered as `null` a header trailed by its members' worth of blank,
* unreachable dead space. Membership still lands in `trackGroupOf`: a collapsed
* member is hidden, not ungrouped.
*/
function emitGroupRows(
info: TimelineTrackGroupInfo,
rawByTrack: ReadonlyMap<number, TimelineElement[]>,
trackGroupOf: Map<number, TimelineTrackGroupInfo>,
tracks: [number, TimelineElement[]][],
expanded: boolean,
): void {
tracks.push([info.anchorKey, []]);
for (const member of info.memberTracks) {
trackGroupOf.set(member, info);
tracks.push([member, rawByTrack.get(member) ?? []]);
if (expanded) tracks.push([member, rawByTrack.get(member) ?? []]);
}
}
@@ -106,7 +116,10 @@ function emitGroupRows(
* their position; a group's members move up to sit under its anchor even when
* other (ungrouped) tracks were interleaved between them.
*/
function groupTimelineTracks(rawTracks: [number, TimelineElement[]][]): {
function groupTimelineTracks(
rawTracks: [number, TimelineElement[]][],
expandedGroupIds: ReadonlySet<string>,
): {
tracks: [number, TimelineElement[]][];
groups: TimelineTrackGroupInfo[];
trackGroupOf: Map<number, TimelineTrackGroupInfo>;
@@ -128,7 +141,7 @@ function groupTimelineTracks(rawTracks: [number, TimelineElement[]][]): {
emitted.add(groupId);
const info = buildGroupInfo(groupId, trackNum, membership);
groups.push(info);
emitGroupRows(info, rawByTrack, trackGroupOf, tracks);
emitGroupRows(info, rawByTrack, trackGroupOf, tracks, expandedGroupIds.has(groupId));
}
return { tracks, groups, trackGroupOf };
}
@@ -157,6 +170,7 @@ export function useTimelineTrackDerivations(expandedElements: TimelineElement[])
return Array.from(map.entries()).sort(([a], [b]) => a - b);
}, [expandedElements]);
const expandedGroupIds = usePlayerStore((s) => s.expandedGroupIds);
const { tracks, groups, trackGroupOf } = useMemo(() => {
if (!isCanaryEnabled("audio-groups")) {
return {
@@ -165,8 +179,8 @@ export function useTimelineTrackDerivations(expandedElements: TimelineElement[])
trackGroupOf: new Map<number, TimelineTrackGroupInfo>(),
};
}
return groupTimelineTracks(rawTracks);
}, [rawTracks]);
return groupTimelineTracks(rawTracks, expandedGroupIds);
}, [rawTracks, expandedGroupIds]);
const trackStyles = useMemo(() => {
const map = new Map<number, TrackVisualStyle>();
@@ -3,7 +3,7 @@
import React, { act } from "react";
import { createRoot } from "react-dom/client";
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
import { afterEach, describe, expect, it } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import { usePlayerStore, type TimelineElement } from "../store/playerStore";
import { LANE_H, TRACK_H } from "./timelineLayout";
import { AUTOMATION_LANE_H } from "./automationLaneHeight";
@@ -12,7 +12,13 @@ import { resolveTrackKeyframeClip, useTimelineTrackLayout } from "./useTimelineT
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
const enabledCanaries = new Set<string>();
vi.mock("../../telemetry/canary", () => ({
isCanaryEnabled: (name: string) => enabledCanaries.has(name),
}));
afterEach(() => {
enabledCanaries.clear();
usePlayerStore.getState().reset();
});
@@ -38,6 +44,61 @@ function renderTrackLayout(
return { layout, unmount: () => act(() => root.unmount()) };
}
describe("collapsed audio groups", () => {
const member = (id: string, track: number): TimelineElement => ({
id,
domId: id,
tag: "audio",
start: 0,
duration: 5,
track,
audioGroup: "voiceover",
});
function renderGrouped(): {
layout: ReturnType<typeof useTimelineTrackLayout>;
unmount: () => void;
} {
enabledCanaries.add("audio-groups");
const elements = [member("voice-1", 0), member("voice-2", 1)];
let layout: ReturnType<typeof useTimelineTrackLayout> | undefined;
function Probe() {
layout = useTimelineTrackLayout(elements, new Map(), null, new Set());
return null;
}
const root = createRoot(document.createElement("div"));
act(() => root.render(React.createElement(Probe)));
if (!layout) throw new Error("Timeline track layout did not render");
return { layout, unmount: () => act(() => root.unmount()) };
}
// buildTimelineLogicalRows stops emitting member rows once a group is
// collapsed, so TimelineLanes renders null for them. Rows left in `tracks`
// still reserve height, turning that null into visible dead space — the row
// list and the logical rows have to agree.
it("emits only the anchor row while the group is collapsed", () => {
const { layout, unmount } = renderGrouped();
expect(layout.groups).toHaveLength(1);
expect(layout.groups[0]!.memberTracks).toEqual([0, 1]);
// The anchor (0 - 0.5) and nothing else.
expect(layout.trackOrder).toEqual([-0.5]);
expect(layout.rowGeometry.rowHeights).toHaveLength(1);
// Membership still resolves — collapsed is hidden, not ungrouped.
expect(layout.trackGroupOf.get(0)?.id).toBe("voiceover");
unmount();
});
it("emits the member rows once the group is expanded", () => {
usePlayerStore.setState({ expandedGroupIds: new Set(["voiceover"]) });
const { layout, unmount } = renderGrouped();
expect(layout.trackOrder).toEqual([-0.5, 0, 1]);
for (const track of layout.groups[0]!.memberTracks) {
expect(layout.rowGeometry.getRowHeight(layout.rowGeometry.getRowIndex(track))).toBe(TRACK_H);
}
unmount();
});
});
describe("useTimelineTrackLayout", () => {
it("counts a flat tween lane and reserves its expanded row height", () => {
const elements: TimelineElement[] = [
@@ -232,8 +232,8 @@ export function useTimelinePlayer() {
} catch {}
}, []);
const applyPreviewAudioState = useCallback(() => {
const { audioMuted, audioVolume } = usePlayerStore.getState();
applyPreviewAudioFlags(iframeRef.current, audioMuted, audioVolume);
const { audioMuted, audioVolume, soloed } = usePlayerStore.getState();
applyPreviewAudioFlags(iframeRef.current, audioMuted, audioVolume, soloed);
}, []);
const play = useCallback(() => {
stopRAFLoop();
@@ -4,6 +4,7 @@ import {
createTimelineElementFromManifestClip,
parseTimelineFromDOM,
createImplicitTimelineLayersFromDOM,
invalidateGroupInfoCache,
mergeTimelineElementsPreservingDowngrades,
} from "./timelineDOM";
import type { TimelineElement } from "../store/playerStore";
@@ -110,6 +111,54 @@ describe("parseTimelineFromDOM — hfId from data-hf-id", () => {
});
});
describe("group info cache", () => {
const parseMember = (doc: Document) =>
createTimelineElementFromManifestClip({
clip: {
id: "voice-1",
label: "voice-1",
kind: "element",
tagName: "audio",
start: 0,
duration: 5,
track: 0,
compositionId: null,
parentCompositionId: null,
compositionSrc: null,
assetUrl: null,
},
fallbackIndex: 0,
doc,
hostEl: doc.getElementById("voice-1"),
});
// Group edits are applied as LIVE patches so the preview iframe never
// reloads, which means the document identity this cache is keyed on never
// changes either. Without an explicit drop, a muted group could never be
// unmuted: the header kept reading the cached `hidden: false` and re-wrote
// `data-hidden` forever.
it("re-reads group state after an invalidation", () => {
const doc = makeDoc(`
<div data-composition-id="root">
<audio id="voice-1" data-start="0" data-duration="5" data-audio-group="voiceover"></audio>
<hf-audio-group id="voiceover" data-label="Voices"></hf-audio-group>
</div>
`);
expect(parseMember(doc).audioGroupHidden).toBe(false);
doc.getElementById("voiceover")?.setAttribute("data-hidden", "");
expect(parseMember(doc).audioGroupHidden).toBe(false); // still the cached scan
invalidateGroupInfoCache(doc);
expect(parseMember(doc).audioGroupHidden).toBe(true);
doc.getElementById("voiceover")?.removeAttribute("data-hidden");
invalidateGroupInfoCache(doc);
expect(parseMember(doc).audioGroupHidden).toBe(false);
});
});
describe("parseTimelineFromDOM — canonical playback rate", () => {
it.each([
["10", 5],
@@ -79,6 +79,20 @@ interface GroupInfo {
const groupInfoCache = new WeakMap<Document, Map<string, GroupInfo>>();
/**
* Drop the cached group scan for a document.
*
* MUST be called by every live write to group state. The cache is keyed on the
* document, and group edits are applied as live patches precisely so the iframe
* never reloads so the key never changes and the entry would otherwise live
* forever. Left stale, a muted group could never be unmuted (the header keeps
* reading `hidden:false` and re-writes `data-hidden`), the bus slider snapped
* back, and a second FX preset built on a stale chain, discarding the first.
*/
export function invalidateGroupInfoCache(doc: Document | null | undefined): void {
if (doc) groupInfoCache.delete(doc);
}
function groupInfoFor(doc: Document | null | undefined, groupId: string): GroupInfo {
if (!doc) return { label: groupId, volume: 1, hidden: false };
let info = groupInfoCache.get(doc);
@@ -1,6 +1,7 @@
// @vitest-environment jsdom
import { describe, expect, it, vi } from "vitest";
import {
applyPreviewAudioFlags,
buildMissingCompositionElements,
scrubPreviewAudio,
setPreviewMediaVolume,
@@ -87,3 +88,45 @@ describe("scrubPreviewAudio", () => {
stopScrubPreviewAudio();
});
});
describe("applyPreviewAudioFlags", () => {
function fakeIframe(): { iframe: HTMLIFrameElement; calls: Record<string, unknown[]> } {
const calls: Record<string, unknown[]> = {};
const win = {
__hf: {
setAudioSolo: (ids: readonly string[]) => {
calls.solo = [...ids];
},
setAudioMuteHidden: (enabled: boolean) => {
calls.muteHidden = [enabled];
},
},
};
const iframe = {
contentWindow: win,
contentDocument: null,
querySelector: () => null,
} as unknown as HTMLIFrameElement;
return { iframe, calls };
}
// Everything pushed here is state the runtime loses on reload and nothing
// else re-sends: the solo bridge's effect deps do not change across a
// reload, so the button stayed lit while every track played.
it("re-pushes the whole audio state, solo included", () => {
const { iframe, calls } = fakeIframe();
applyPreviewAudioFlags(iframe, false, 1, new Set(["voice-1"]));
expect(calls.solo).toEqual(["voice-1"]);
expect(calls.muteHidden).toEqual([false]);
});
it("pushes an empty solo set rather than skipping the call", () => {
const { iframe, calls } = fakeIframe();
applyPreviewAudioFlags(iframe, false, 1, new Set());
expect(calls.solo).toEqual([]);
});
});
@@ -157,23 +157,39 @@ function setPreviewMuteHidden(iframe: HTMLIFrameElement | null, enabled: boolean
} catch {}
}
/** Replace the runtime's soloed set. Same channel `useAudioSoloBridge` uses for
* live changes; repeated here because the bridge's effect deps do not change
* across a preview reload, so it never re-fires and the reloaded runtime would
* keep an empty set while the button stays lit. */
function setPreviewSolo(iframe: HTMLIFrameElement | null, ids: readonly string[]): void {
if (!iframe) return;
try {
const win = iframe.contentWindow as
| (Window & { __hf?: { setAudioSolo?: (ids: readonly string[]) => void } })
| null;
win?.__hf?.setAudioSolo?.(ids);
} catch {}
}
/**
* Everything the preview runtime has to be told about audio after it loads:
* the transport's mute, and the canary flags core cannot resolve for itself.
* Called from `applyPreviewAudioState`, which is the path that re-runs after a
* preview reload the runtime comes back with every flag at its default and
* nothing else pushes them again.
* the transport's mute, the session's solo set, and the canary flags core
* cannot resolve for itself. Called from `applyPreviewAudioState`, which is the
* path that re-runs after a preview reload the runtime comes back with every
* one of these at its default and nothing else pushes them again.
*/
export function applyPreviewAudioFlags(
iframe: HTMLIFrameElement | null,
muted: boolean,
volume: number,
soloed: ReadonlySet<string>,
): void {
setPreviewMediaMuted(iframe, muted);
// Volume too: the transport comes back at unity after a reload, so a preview
// the author had turned down came back loud.
setPreviewMediaVolume(iframe, volume);
setPreviewMuteHidden(iframe, isCanaryEnabled("audio-track-mute"));
setPreviewSolo(iframe, [...soloed]);
}
export function setPreviewPlaybackRate(
@@ -111,3 +111,18 @@ describe("isGroupHalfLitUnderSolo", () => {
expect(isGroupHalfLitUnderSolo(new Set(["other"]), "group-1", ["a", "b"])).toBe(false);
});
});
describe("solo does not outlive its composition", () => {
// Solo ids only mean anything against the document they were taken from.
// Carried into another composition they match nothing, and "match nothing"
// is exactly the state that silences every track while the banner still
// claims to be hearing one of them.
it("is cleared by the timeline reset that a composition switch runs", () => {
usePlayerStore.getState().toggleSolo("voice-1");
expect(usePlayerStore.getState().soloed.size).toBe(1);
usePlayerStore.getState().reset();
expect(usePlayerStore.getState().soloed).toEqual(new Set());
});
});
@@ -279,6 +279,8 @@ export function createTimelineResetState() {
// paste through `sel.elementKey === paste.elementKey` to a stale t0.
automationSelection: null,
expandedClipIds: new Set<string>(),
// Per-composition: ids from comp A match nothing in B, silencing all of it.
soloed: new Set<string>(),
expandedGroupIds: new Set<string>(),
expandedLaneOwnerIds: new Set<string>(),
focusedEaseSegment: null,