diff --git a/packages/studio/src/player/hooks/useShadowPreviewReload.ts b/packages/studio/src/player/hooks/useShadowPreviewReload.ts index 8cd41c9ff9..8868dc4617 100644 --- a/packages/studio/src/player/hooks/useShadowPreviewReload.ts +++ b/packages/studio/src/player/hooks/useShadowPreviewReload.ts @@ -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. diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.shadowReload.test.ts b/packages/studio/src/player/hooks/useTimelinePlayer.shadowReload.test.ts index dde074101f..f1423905b4 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.shadowReload.test.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.shadowReload.test.ts @@ -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) { 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 = ["undone-once", "undone-twice"]; + vi.spyOn(globalThis, "fetch").mockImplementation( + () => new Promise((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("vN"); + }); + const { getApi } = liveFilm(swap); + + act(() => getApi().refreshPlayer()); + await settle(); + + expect(fetches).toBe(6); + expect(swap).toHaveBeenCalledWith("vN", expect.anything()); + }); + it("falls back to the full reload when the preview refuses the swap", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("v2")); const { getApi } = liveFilm(async () => { diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index 35352cd139..0465d78873 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -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; diff --git a/packages/studio/src/player/sceneSwap.ts b/packages/studio/src/player/sceneSwap.ts index 6c0d24fff7..dcc003217d 100644 --- a/packages/studio/src/player/sceneSwap.ts +++ b/packages/studio/src/player/sceneSwap.ts @@ -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; }; @@ -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; + +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, diff --git a/packages/studio/src/utils/studioFileVersion.test.ts b/packages/studio/src/utils/studioFileVersion.test.ts index e33b342b1c..426268d10e 100644 --- a/packages/studio/src/utils/studioFileVersion.test.ts +++ b/packages/studio/src/utils/studioFileVersion.test.ts @@ -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); + }); }); diff --git a/packages/studio/src/utils/studioFileVersion.ts b/packages/studio/src/utils/studioFileVersion.ts index 46b3bee8f1..29f88282a8 100644 --- a/packages/studio/src/utils/studioFileVersion.ts +++ b/packages/studio/src/utils/studioFileVersion.ts @@ -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(); +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. */ diff --git a/packages/studio/tests/e2e/edit-accuracy/case.mjs b/packages/studio/tests/e2e/edit-accuracy/case.mjs index a745c71518..4d3d64af1a 100644 --- a/packages/studio/tests/e2e/edit-accuracy/case.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/case.mjs @@ -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);