mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-11-04 21:17:30 +00:00
fix(studio): a preview reload writes the composition as it is now, not as it was when it asked (#6998)
* fix(studio): a preview swap writes the composition as it is now, not as it was when it asked Claude-Session: https://claude.ai/code/session_01BzWPCkrNkiYrpRYeqVmKyK * fix(studio): a shadow reload that a write outran loads the file again Claude-Session: https://claude.ai/code/session_01BzWPCkrNkiYrpRYeqVmKyK * fix(studio): count the project's revision once per write, not again on its echo Claude-Session: https://claude.ai/code/session_01BzWPCkrNkiYrpRYeqVmKyK * refactor(studio): move the swap freshness check next to the swap it guards * refactor(studio): name the refetch bound instead of explaining it
This commit is contained in:
@@ -20,6 +20,7 @@ import {
|
||||
} from "./useTimelineSyncCallbacks";
|
||||
import type { IframeWindow, PlaybackAdapter } from "../lib/playbackTypes";
|
||||
import { setPreviewReloading } from "../previewReloading";
|
||||
import { studioFileRevision } from "../../utils/studioFileVersion";
|
||||
import { announcePreviewPromoted } from "../sceneSwap";
|
||||
import { afterStudioPendingEdits, isStudioEditSaving } from "../../utils/studioPendingEdits";
|
||||
import { usePlayerStore } from "../store/playerStore";
|
||||
@@ -117,6 +118,7 @@ export function useShadowPreviewReload({
|
||||
const stopHoldRef = useRef<() => void>(() => {});
|
||||
const shadowUrlRef = useRef("");
|
||||
const savesAtRequestRef = useRef(0);
|
||||
const revisionAtRequestRef = useRef(0);
|
||||
const beginShadowReloadRef = useRef<(url: string) => void>(() => {});
|
||||
const retry = useShadowReloadRetry(shadowGenRef, shadowUrlRef, beginShadowReloadRef);
|
||||
const promoteWhenReadyRef = useRef<(gen: number) => void>(() => {});
|
||||
@@ -179,8 +181,8 @@ export function useShadowPreviewReload({
|
||||
[],
|
||||
);
|
||||
|
||||
// Nothing replaces the preview under a gesture, and a shadow requested before a gesture's save
|
||||
// shows the file before it: once that save has landed, a fresh one loads instead.
|
||||
// Nothing replaces the preview under a gesture, and a shadow requested before a write
|
||||
// shows the file before it: once that write has landed, a fresh one loads instead.
|
||||
const heldOrStale = useCallback(
|
||||
(gen: number) => {
|
||||
const liveDoc = iframeRef.current?.contentDocument;
|
||||
@@ -188,7 +190,11 @@ export function useShadowPreviewReload({
|
||||
return waitThenRetry(gen, "gesture", (retry) =>
|
||||
afterStudioManualEditGestures(liveDoc, retry),
|
||||
);
|
||||
if (gestureSavesOnScreen() === savesAtRequestRef.current) return false;
|
||||
if (
|
||||
gestureSavesOnScreen() === savesAtRequestRef.current &&
|
||||
studioFileRevision() === revisionAtRequestRef.current
|
||||
)
|
||||
return false;
|
||||
if (isStudioEditSaving()) return waitThenRetry(gen, "save", afterStudioPendingEdits);
|
||||
logReload("shadow-reloaded-after-gesture-save", { gen });
|
||||
const url = new URL(shadowUrlRef.current, window.location.origin);
|
||||
@@ -335,6 +341,7 @@ export function useShadowPreviewReload({
|
||||
budgetsSpentRef.current = 0;
|
||||
shadowUrlRef.current = url;
|
||||
savesAtRequestRef.current = gestureSavesOnScreen();
|
||||
revisionAtRequestRef.current = studioFileRevision();
|
||||
armReadyTimer(gen);
|
||||
setPreviewSlots((prev) => planShadowReload(prev, gen, url));
|
||||
// Thumbnails of the edit wait for the new preview instead of competing with it.
|
||||
|
||||
@@ -8,6 +8,7 @@ import { SHADOW_READY_BUDGETS, SHADOW_READY_TIMEOUT_MS } from "./useShadowPrevie
|
||||
import { refusedPreviewMessage } from "@hyperframes/core/studio-preview-mark";
|
||||
import { usePlayerStore } from "../store/playerStore";
|
||||
import { isPreviewChanging } from "../previewReloading";
|
||||
import { studioWriteHeaders } from "../../utils/studioFileVersion";
|
||||
import { NLEProvider, useNLEContext, type NLEContextValue } from "../../components/nle/NLEContext";
|
||||
import {
|
||||
makeAdapterWindow,
|
||||
@@ -939,6 +940,32 @@ function stubVisibility(initial: DocumentVisibilityState) {
|
||||
};
|
||||
}
|
||||
|
||||
describe("a shadow reload loading while a write lands", () => {
|
||||
it("loads the file again instead of promoting the one it asked for", () => {
|
||||
const { getApi, root } = renderTimelinePlayerHarness();
|
||||
const liveIframe = makeLiveIframe();
|
||||
act(() => {
|
||||
getApi().iframeRef.current = liveIframe;
|
||||
getApi().onIframeLoad();
|
||||
});
|
||||
act(() => getApi().refreshPlayer());
|
||||
const asked = getApi().previewSlots.find((slot) => slot.role === "shadow")!;
|
||||
act(() => getApi().setShadowIframeNode(makeLiveIframe()));
|
||||
|
||||
// An undo writes while that shadow is still loading.
|
||||
studioWriteHeaders();
|
||||
act(() => {
|
||||
getApi().onShadowIframeLoad(asked.gen);
|
||||
getApi().onShadowReadyChange(asked.gen, true);
|
||||
});
|
||||
|
||||
expect(getApi().iframeRef.current).toBe(liveIframe);
|
||||
const loading = getApi().previewSlots.find((slot) => slot.role === "shadow")!;
|
||||
expect(loading.gen).not.toBe(asked.gen);
|
||||
unmount(root);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useTimelinePlayer scene swap", () => {
|
||||
function liveFilm(swap?: (html: string) => Promise<void>) {
|
||||
const { adapter, win } = makeAdapterWindow();
|
||||
@@ -985,6 +1012,46 @@ describe("useTimelinePlayer scene swap", () => {
|
||||
expect(isPreviewChanging()).toBe(false);
|
||||
});
|
||||
|
||||
it("swaps the composition as it is now when a write lands while the HTML is loading", async () => {
|
||||
const swap = vi.fn(async () => {});
|
||||
const replies: Array<(r: Response) => void> = [];
|
||||
const bodies = ["<html>undone-once</html>", "<html>undone-twice</html>"];
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(
|
||||
() => new Promise<Response>((resolve) => replies.push(resolve)),
|
||||
);
|
||||
const { getApi } = liveFilm(swap);
|
||||
|
||||
act(() => getApi().refreshPlayer());
|
||||
// A second undo writes while the first one's preview HTML is still loading.
|
||||
studioWriteHeaders();
|
||||
replies[0]!(new Response(bodies[0]));
|
||||
await settle();
|
||||
|
||||
expect(swap).not.toHaveBeenCalledWith(bodies[0], expect.anything());
|
||||
expect(replies).toHaveLength(2);
|
||||
replies[1]!(new Response(bodies[1]));
|
||||
await settle();
|
||||
expect(swap).toHaveBeenCalledWith(bodies[1], expect.anything());
|
||||
});
|
||||
|
||||
it("gives up refetching and swaps what it has when writes never stop", async () => {
|
||||
const swap = vi.fn(async () => {});
|
||||
let fetches = 0;
|
||||
// The project stops moving well past the bound, so a swap that never gave up still ends.
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
|
||||
fetches += 1;
|
||||
if (fetches < 20) studioWriteHeaders();
|
||||
return new Response("<html>vN</html>");
|
||||
});
|
||||
const { getApi } = liveFilm(swap);
|
||||
|
||||
act(() => getApi().refreshPlayer());
|
||||
await settle();
|
||||
|
||||
expect(fetches).toBe(6);
|
||||
expect(swap).toHaveBeenCalledWith("<html>vN</html>", expect.anything());
|
||||
});
|
||||
|
||||
it("falls back to the full reload when the preview refuses the swap", async () => {
|
||||
vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("<html>v2</html>"));
|
||||
const { getApi } = liveFilm(async () => {
|
||||
|
||||
@@ -43,8 +43,7 @@ import { createPreviewMessageHandler } from "./previewMessageRouter";
|
||||
import { usePreviewQualityForwarding } from "./usePreviewQualityForwarding";
|
||||
import { timelineElementsChanged } from "./timelinePlayerSync";
|
||||
import { safeContentDocument } from "./timelineSyncHydration";
|
||||
import { sceneSwapFor, supersededByNewerEdit } from "../sceneSwap";
|
||||
import { whileScriptWrites } from "../previewReloading";
|
||||
import { sceneSwapFor, supersededByNewerEdit, swapFreshly } from "../sceneSwap";
|
||||
|
||||
export interface UseTimelinePlayerOptions {
|
||||
/** Runs right after a reloaded preview becomes the live iframe. */
|
||||
@@ -506,13 +505,7 @@ export function useTimelinePlayer({
|
||||
!!iframe.contentDocument && isStudioManualEditGestureLiveIn(iframe.contentDocument);
|
||||
if (!swap || isRefreshingRef.current || swapWouldReplaceGestureNode)
|
||||
return reloadWholeFilm(url.toString());
|
||||
whileScriptWrites(() => swap(url.toString(), isCurrent, cancel.signal)).catch(
|
||||
(error: unknown) => {
|
||||
if (!isCurrent()) return;
|
||||
logReload("scene-swap-refused", { reason: String(error) });
|
||||
reloadWholeFilm(url.toString());
|
||||
},
|
||||
);
|
||||
swapFreshly(swap, url, isCurrent, cancel.signal, reloadWholeFilm);
|
||||
}, [reloadWholeFilm, previewGeneration]);
|
||||
const pauseRef = useRef(pause);
|
||||
pauseRef.current = pause;
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import { studioApiFetch } from "../utils/studioApiFetch";
|
||||
import { studioFileRevision } from "../utils/studioFileVersion";
|
||||
import { logReload } from "../utils/reloadDebug";
|
||||
import { whileScriptWrites } from "./previewReloading";
|
||||
|
||||
type SwapWindow = Window & {
|
||||
__hfSwapScenes?: (html: string, signal?: AbortSignal) => Promise<void>;
|
||||
};
|
||||
@@ -74,6 +78,38 @@ export function markScenesStale(iframe: HTMLIFrameElement | null, files: readonl
|
||||
export const supersededByNewerEdit = () =>
|
||||
new DOMException("superseded by a newer edit", "AbortError");
|
||||
|
||||
const SWAP_REFETCHES_BEFORE_THE_COPY_IN_HAND_WINS = 5;
|
||||
|
||||
type SceneSwap = (url: string, isCurrent: () => boolean, cancel?: AbortSignal) => Promise<void>;
|
||||
|
||||
export function swapFreshly(
|
||||
swap: SceneSwap,
|
||||
url: URL,
|
||||
isCurrent: () => boolean,
|
||||
cancel: AbortSignal,
|
||||
reloadWholeFilm: (url: string) => void,
|
||||
attempt = 0,
|
||||
): void {
|
||||
const at = studioFileRevision();
|
||||
const fresh = () => studioFileRevision() === at;
|
||||
const again = () => {
|
||||
url.searchParams.set("_t", String(Date.now()));
|
||||
swapFreshly(swap, url, isCurrent, cancel, reloadWholeFilm, attempt + 1);
|
||||
};
|
||||
const retries = attempt < SWAP_REFETCHES_BEFORE_THE_COPY_IN_HAND_WINS;
|
||||
const gate = retries ? () => isCurrent() && fresh() : isCurrent;
|
||||
whileScriptWrites(() => swap(url.toString(), gate, cancel))
|
||||
.then(() => {
|
||||
if (isCurrent() && !fresh() && retries) again();
|
||||
})
|
||||
.catch((error: unknown) => {
|
||||
if (!isCurrent()) return;
|
||||
if (!fresh() && retries) return again();
|
||||
logReload("scene-swap-refused", { reason: String(error) });
|
||||
reloadWholeFilm(url.toString());
|
||||
});
|
||||
}
|
||||
|
||||
/** Null when the preview cannot swap; rejects when superseded, cancelled or a full reload is needed. */
|
||||
export function sceneSwapFor(
|
||||
iframe: HTMLIFrameElement,
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
resetStudioWriteTokens,
|
||||
studioExpectedFileVersion,
|
||||
studioFileContentVersion,
|
||||
studioFileRevision,
|
||||
studioWriteHeaders,
|
||||
} from "./studioFileVersion";
|
||||
|
||||
@@ -96,4 +97,18 @@ describe("studio write-token echo identity", () => {
|
||||
markStudioWriteToken("abandoned-studio-write", 1_000);
|
||||
expect(consumeStudioWriteToken("abandoned-studio-write", 301_000)).toBe(false);
|
||||
});
|
||||
|
||||
it("moves the file revision once per write claimed, and not for its echo", () => {
|
||||
resetStudioWriteTokens();
|
||||
const start = studioFileRevision();
|
||||
markStudioWriteToken("revision-write");
|
||||
|
||||
expect(studioFileRevision()).toBe(start + 1);
|
||||
// The echo of a write already counted must not move it again: a reader asked for after the
|
||||
// write was claimed would read as stale on its own echo and fetch a second time for nothing.
|
||||
expect(consumeStudioWriteToken("revision-write")).toBe(true);
|
||||
expect(studioFileRevision()).toBe(start + 1);
|
||||
markStudioWriteToken("second-write");
|
||||
expect(studioFileRevision()).toBe(start + 2);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,12 @@
|
||||
// retries, and the subsequent file-change echo without retaining abandoned tokens forever.
|
||||
const WRITE_TOKEN_TTL_MS = 5 * 60_000;
|
||||
const studioWriteTokens = new Map<string, number>();
|
||||
let fileRevision = 0;
|
||||
|
||||
/** One step per write Studio claims: a copy of the project read at a lower revision is stale. */
|
||||
export function studioFileRevision(): number {
|
||||
return fileRevision;
|
||||
}
|
||||
|
||||
function pruneStudioWriteTokens(now: number): void {
|
||||
for (const [token, createdAt] of studioWriteTokens) {
|
||||
@@ -13,6 +19,7 @@ function pruneStudioWriteTokens(now: number): void {
|
||||
export function markStudioWriteToken(token: string, now: number = Date.now()): void {
|
||||
pruneStudioWriteTokens(now);
|
||||
studioWriteTokens.set(token, now);
|
||||
fileRevision += 1;
|
||||
}
|
||||
|
||||
/** Consumed by the external-change coordinator introduced in stack PR #2991. */
|
||||
|
||||
@@ -620,6 +620,15 @@ async function cropOutline(ctx, map) {
|
||||
return (await contentQuad(h)).map(map.toComp);
|
||||
}
|
||||
|
||||
/** Holds each history reply back, so the next undo is pressed while this one's preview update runs. */
|
||||
function slowHistory(ms) {
|
||||
const send = window.fetch;
|
||||
window.fetch = (url, init) =>
|
||||
/\/history\/step$/.test(String(url).split("?")[0])
|
||||
? send(url, init).then((r) => new Promise((ok) => setTimeout(() => ok(r), ms)))
|
||||
: send(url, init);
|
||||
}
|
||||
|
||||
/** Holds every save request `ms` before it is sent, so a later press lands while it is still in flight. */
|
||||
function slowSaves(ms) {
|
||||
const send = window.fetch;
|
||||
@@ -1035,6 +1044,8 @@ export async function inStudio({ browser, spec, dir, files, url, evidence, shots
|
||||
await page.evaluateOnNewDocument(installWebMcpHost, "__editBench");
|
||||
await page.evaluateOnNewDocument(instrumentPage);
|
||||
if (spec.slowSaves) await page.evaluateOnNewDocument(slowSaves, spec.slowSaves);
|
||||
if (process.env.HF_SLOW_HISTORY)
|
||||
await page.evaluateOnNewDocument(slowHistory, Number(process.env.HF_SLOW_HISTORY));
|
||||
await page.evaluateOnNewDocument(frameSamplerScript);
|
||||
await page.goto(url);
|
||||
let pre = await openStudio(ctx);
|
||||
|
||||
Reference in New Issue
Block a user