Skip to content

Commit ae8d178

Browse files
fix(studio): undoing an agent's build takes its sections off the timeline too (#4757)
* fix(studio): undoing an agent's build takes its sections off the timeline too * fix(studio): a reloaded preview's timeline is read from the new document, sections included * fix(studio): a section still in the preview stays on the timeline, and trims are unchanged
1 parent 9a27b9f commit ae8d178

6 files changed

Lines changed: 106 additions & 3 deletions

File tree

‎packages/studio/src/player/hooks/useShadowPreviewReload.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,9 @@ export function useShadowPreviewReload({
118118
const liveTime = live?.getTime();
119119
const playing = usePlayerStore.getState().isPlaying;
120120
live?.pause();
121-
// The store takes the new document's timeline only now that it is the one on screen.
122-
pending.commit();
121+
// The store takes the new document's timeline only now that it is the one on screen, and reads it there.
123122
iframeRef.current = shadow;
123+
pending.commit();
124124
shadowIframeRef.current = null;
125125
attachIframeShortcutListeners();
126126
applyPreviewAudioState();

‎packages/studio/src/player/hooks/useTimelinePlayer.shadowReload.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,75 @@ describe("shadow reload store ownership and readiness", () => {
543543
unmount(root);
544544
});
545545

546+
it("takes a section the edit removed off the timeline, though the old document still has it", () => {
547+
const { getApi, gen, liveIframe, shadowIframe, root } = setup();
548+
const host = (id: string) =>
549+
`<div id="${id}" data-composition-id="${id}" data-composition-src="compositions/${id}.html" data-start="0" data-duration="3"></div>`;
550+
liveIframe.contentDocument!.body.innerHTML = `<div data-composition-id="main">${host("benefit")}<p id="a"></p></div>`;
551+
shadowIframe.contentDocument!.body.innerHTML = `<div data-composition-id="main"><p id="a"></p></div>`;
552+
const a = { id: "a", domId: "a", tag: "p", start: 0, duration: 3, track: 0 };
553+
const benefit = {
554+
id: "benefit",
555+
domId: "benefit",
556+
tag: "div",
557+
kind: "composition" as const,
558+
start: 0,
559+
duration: 3,
560+
track: 1,
561+
compositionSrc: "compositions/benefit.html",
562+
};
563+
act(() => usePlayerStore.getState().setElements([a, benefit]));
564+
(shadowIframe.contentWindow as Record<string, unknown>).__clipManifest = {
565+
source: "hf-preview",
566+
type: "timeline",
567+
durationInFrames: 1260,
568+
clips: [
569+
{ id: "a", label: "A", start: 0, duration: 3, track: 0, kind: "element", tagName: "p" },
570+
],
571+
};
572+
act(() => getApi().onShadowIframeLoad(gen));
573+
act(() => getApi().onShadowReadyChange(gen, true));
574+
expect(usePlayerStore.getState().elements.map((element) => element.id)).toEqual(["a"]);
575+
unmount(root);
576+
});
577+
578+
it("keeps a section the new document still hosts but its manifest misses, without dropping it once", () => {
579+
const { getApi, gen, liveIframe, shadowIframe, root } = setup();
580+
const film = `<div data-composition-id="main"><div id="benefit" data-composition-id="benefit"
581+
data-composition-src="compositions/benefit.html" data-start="0" data-duration="3"></div><p id="a"></p></div>`;
582+
liveIframe.contentDocument!.body.innerHTML = film;
583+
shadowIframe.contentDocument!.body.innerHTML = film;
584+
const a = { id: "a", domId: "a", tag: "p", start: 0, duration: 3, track: 0 };
585+
// As enrichment adds it: no kind.
586+
const benefit = {
587+
id: "benefit",
588+
domId: "benefit",
589+
tag: "div",
590+
start: 0,
591+
duration: 3,
592+
track: 1,
593+
compositionSrc: "compositions/benefit.html",
594+
};
595+
act(() => usePlayerStore.getState().setElements([a, benefit]));
596+
const seen: string[][] = [];
597+
const stop = usePlayerStore.subscribe((state) =>
598+
seen.push(state.elements.map((element) => element.id)),
599+
);
600+
(shadowIframe.contentWindow as Record<string, unknown>).__clipManifest = {
601+
source: "hf-preview",
602+
type: "timeline",
603+
durationInFrames: 1260,
604+
clips: [
605+
{ id: "a", label: "A", start: 0, duration: 3, track: 0, kind: "element", tagName: "p" },
606+
],
607+
};
608+
act(() => getApi().onShadowIframeLoad(gen));
609+
act(() => getApi().onShadowReadyChange(gen, true));
610+
stop();
611+
expect(seen.every((ids) => ids.includes("benefit"))).toBe(true);
612+
unmount(root);
613+
});
614+
546615
it("never writes a failed shadow's timeline into the store", () => {
547616
vi.spyOn(console, "error").mockImplementation(() => {});
548617
const { getApi, gen, root } = setup();

‎packages/studio/src/player/hooks/useTimelinePlayer.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export {
2828
import type { PlaybackAdapter, IframeWindow } from "../lib/playbackTypes";
2929
import { releaseStaticSeekCache, type StaticSeekCacheEntry } from "../lib/playbackAdapter";
3030
import { mergeTimelineElementsPreservingDowngrades } from "../lib/timelineDOM";
31+
import { findTimelineElementInIframe } from "../../hooks/timelineEditingHelpers";
3132
import { normalizeToZones } from "../components/timelineZones";
3233
import { applyPreviewAudioFlags, setPreviewPlaybackRate } from "../lib/timelineIframeHelpers";
3334
import { scrubMusicAtSeek, stopScrubPreviewAudio } from "../lib/playbackScrub";
@@ -92,6 +93,11 @@ export function useTimelinePlayer({
9293
elements,
9394
state.duration,
9495
resolvedDuration,
96+
(element) =>
97+
findTimelineElementInIframe(iframeRef.current, {
98+
...element,
99+
kind: "composition",
100+
}) !== null,
95101
),
96102
state.timelineProjectId,
97103
),

‎packages/studio/src/player/lib/timelineDOM.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,17 @@ describe("mergeTimelineElementsPreservingDowngrades — genuine removal vs trans
534534
expect(merged.map((e) => e.id).sort()).toEqual(["a", "sub-child"]);
535535
});
536536

537+
it("drops a section whose host left the preview (undo of an agent's build)", () => {
538+
const current = [
539+
el("a"),
540+
el("benefit-fresh", { compositionSrc: "compositions/benefit-fresh.html" }),
541+
];
542+
const next = [el("a")]; // the reverted film's manifest: the built section is gone
543+
const inPreview = (element: { id: string }) => element.id !== "benefit-fresh";
544+
const merged = mergeTimelineElementsPreservingDowngrades(current, next, 30, 30, inPreview);
545+
expect(merged.map((e) => e.id)).toEqual(["a"]);
546+
});
547+
537548
it("trusts the fresh scan fully when it is not shorter", () => {
538549
const current = [el("a"), el("b", { compositionSrc: "sub.html" })];
539550
const next = [el("a"), el("c")];

‎packages/studio/src/player/lib/timelineDOM.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ export function mergeTimelineElementsPreservingDowngrades(
382382
nextElements: TimelineElement[],
383383
currentDuration: number,
384384
nextDuration: number,
385+
stillInPreview: (element: TimelineElement) => boolean = () => true,
385386
): TimelineElement[] {
386387
const safeCurrentDuration = Number.isFinite(currentDuration) ? currentDuration : 0;
387388
const safeNextDuration = Number.isFinite(nextDuration) ? nextDuration : 0;
@@ -403,7 +404,8 @@ export function mergeTimelineElementsPreservingDowngrades(
403404
// re-adds. A TOP-LEVEL element missing from the fresh scan was genuinely
404405
// removed (undo of a split, a delete), so let it go — otherwise undoing a
405406
// split leaves a ghost clip in the timeline even though the file is reverted.
406-
element.compositionSrc != null,
407+
element.compositionSrc != null &&
408+
stillInPreview(element),
407409
);
408410
if (preserved.length === 0) return nextElements;
409411
return [...nextElements, ...preserved];

‎packages/studio/src/player/lib/timelineIframeHelpers.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
stopScrubPreviewAudio,
1010
} from "./timelineIframeHelpers";
1111
import type { IframeWindow } from "./playbackTypes";
12+
import { findTimelineElementInIframe } from "../../hooks/timelineEditingHelpers";
1213

1314
function makeDoc(html: string): Document {
1415
const d = document.implementation.createHTMLDocument();
@@ -42,6 +43,20 @@ describe("buildMissingCompositionElements — hfId (R7)", () => {
4243
expect([patched, updatedEls[0]?.compositionSrc]).toEqual([true, "compositions/scene.html"]);
4344
});
4445

46+
it("adds a host the lookup finds in its preview, as a composition", () => {
47+
const doc = makeDoc(`
48+
<div data-composition-id="root">
49+
<div id="benefit-fresh" data-composition-id="benefit-fresh" data-composition-src="compositions/benefit-fresh.html"
50+
data-start="0" data-duration="3"></div>
51+
</div>
52+
`);
53+
const { missing } = buildMissingCompositionElements(doc, window as IframeWindow, [], 10);
54+
const iframe = { contentDocument: doc } as unknown as HTMLIFrameElement;
55+
expect(findTimelineElementInIframe(iframe, { ...missing[0]!, kind: "composition" })).toBe(
56+
doc.getElementById("benefit-fresh"),
57+
);
58+
});
59+
4560
it("harvests hfId from data-hf-id on composition host elements", () => {
4661
const doc = makeDoc(`
4762
<div data-composition-id="root">

0 commit comments

Comments
 (0)