fix(studio): address PR review — restore rollback, fix probe race, harden edit suppression

Restore rollback path: enqueueEdit now returns the queued promise so
Promise.resolve(handler(...)).catch(rollback) in useTimelineClipDrag
fires correctly on save failure. Handlers return the promise chain.

Fix lost-update race in probe enrichment: use zustand's functional
setState so concurrent probe completions each read the latest state
atomically instead of all reading the same stale snapshot.

Harden file-change suppression: pendingTimelineEditPathRef is now a
Set<string> with exact-match lookup instead of single-slot + endsWith.
Multiple concurrent edits on different files are all suppressed correctly.

Remove dead canOffsetTrimClipStart function and its tests — no longer
called after the capability gate simplification.

Document runtime sync mechanism: added comment explaining that the
runtime re-reads data attributes on each sync tick (init.ts:1324-1368).

Fix comment wording in patchIframeDomTiming catch block.
This commit is contained in:
Miguel Ángel
2026-05-19 20:59:45 -04:00
parent beb807493c
commit 366fcc2a64
6 changed files with 39 additions and 85 deletions
+1 -1
View File
@@ -117,7 +117,7 @@ export function StudioApp() {
}); });
const editHistory = usePersistentEditHistory({ projectId }); const editHistory = usePersistentEditHistory({ projectId });
const domEditSaveTimestampRef = useRef(0); const domEditSaveTimestampRef = useRef(0);
const pendingTimelineEditPathRef = useRef<string | null>(null); const pendingTimelineEditPathRef = useRef(new Set<string>());
const reloadPreview = useCallback(() => { const reloadPreview = useCallback(() => {
setRefreshKey((k) => k + 1); setRefreshKey((k) => k + 1);
}, []); }, []);
@@ -30,7 +30,7 @@ interface UsePreviewPersistenceParams {
domEditSaveTimestampRef: React.MutableRefObject<number>; domEditSaveTimestampRef: React.MutableRefObject<number>;
/** Tracks in-flight timeline edits that patch the iframe DOM directly. File-change /** Tracks in-flight timeline edits that patch the iframe DOM directly. File-change
* events for these paths are always suppressed since the preview is already up-to-date. */ * events for these paths are always suppressed since the preview is already up-to-date. */
pendingTimelineEditPathRef?: React.MutableRefObject<string | null>; pendingTimelineEditPathRef?: React.MutableRefObject<Set<string>>;
/** Called to reload the preview after undo/redo or external file changes. */ /** Called to reload the preview after undo/redo or external file changes. */
reloadPreview: () => void; reloadPreview: () => void;
} }
@@ -167,9 +167,8 @@ export function usePreviewPersistence({
const changedPath = readStudioFileChangePath(payload); const changedPath = readStudioFileChangePath(payload);
if (!changedPath) return; if (!changedPath) return;
const recentDomEditSave = Date.now() - domEditSaveTimestampRef.current < 4000; const recentDomEditSave = Date.now() - domEditSaveTimestampRef.current < 4000;
const pendingPath = pendingTimelineEditPathRef?.current; if (pendingTimelineEditPathRef?.current.has(changedPath)) {
if (pendingPath && changedPath.endsWith(pendingPath)) { pendingTimelineEditPathRef.current.delete(changedPath);
pendingTimelineEditPathRef!.current = null;
return; return;
} }
if (!recentDomEditSave) { if (!recentDomEditSave) {
+28 -25
View File
@@ -39,7 +39,7 @@ interface UseTimelineEditingOptions {
domEditSaveTimestampRef: React.MutableRefObject<number>; domEditSaveTimestampRef: React.MutableRefObject<number>;
reloadPreview: () => void; reloadPreview: () => void;
previewIframeRef: React.RefObject<HTMLIFrameElement | null>; previewIframeRef: React.RefObject<HTMLIFrameElement | null>;
pendingTimelineEditPathRef: React.MutableRefObject<string | null>; pendingTimelineEditPathRef: React.MutableRefObject<Set<string>>;
uploadProjectFiles: (files: Iterable<File>, dir?: string) => Promise<string[]>; uploadProjectFiles: (files: Iterable<File>, dir?: string) => Promise<string[]>;
} }
@@ -55,6 +55,9 @@ function buildPatchTarget(element: { domId?: string; selector?: string; selector
return null; return null;
} }
// The runtime re-reads data-start/data-duration from the DOM on each sync tick
// (packages/core/src/runtime/init.ts:1324-1368), so attribute mutations here are
// picked up automatically on the next frame without a rebind call.
function patchIframeDomTiming( function patchIframeDomTiming(
iframe: HTMLIFrameElement | null, iframe: HTMLIFrameElement | null,
element: TimelineElement, element: TimelineElement,
@@ -71,7 +74,7 @@ function patchIframeDomTiming(
if (!el) return; if (!el) return;
for (const [name, value] of attrs) el.setAttribute(name, value); for (const [name, value] of attrs) el.setAttribute(name, value);
} catch { } catch {
// Cross-origin or mid-navigation — safe to ignore, file is already saved. // Cross-origin or mid-navigation — file save is enqueued; iframe patch is best-effort.
} }
} }
@@ -112,7 +115,7 @@ interface PersistTimelineEditInput {
writeProjectFile: (path: string, content: string) => Promise<void>; writeProjectFile: (path: string, content: string) => Promise<void>;
recordEdit: (input: RecordEditInput) => Promise<void>; recordEdit: (input: RecordEditInput) => Promise<void>;
domEditSaveTimestampRef: React.MutableRefObject<number>; domEditSaveTimestampRef: React.MutableRefObject<number>;
pendingTimelineEditPathRef: React.MutableRefObject<string | null>; pendingTimelineEditPathRef: React.MutableRefObject<Set<string>>;
} }
async function persistTimelineEdit(input: PersistTimelineEditInput): Promise<void> { async function persistTimelineEdit(input: PersistTimelineEditInput): Promise<void> {
@@ -129,7 +132,7 @@ async function persistTimelineEdit(input: PersistTimelineEditInput): Promise<voi
throw new Error(`Unable to patch timeline element ${input.element.id} in ${targetPath}`); throw new Error(`Unable to patch timeline element ${input.element.id} in ${targetPath}`);
} }
input.pendingTimelineEditPathRef.current = targetPath; input.pendingTimelineEditPathRef.current.add(targetPath);
input.domEditSaveTimestampRef.current = Date.now(); input.domEditSaveTimestampRef.current = Date.now();
await saveProjectFilesWithHistory({ await saveProjectFilesWithHistory({
projectId: input.projectId, projectId: input.projectId,
@@ -183,26 +186,26 @@ export function useTimelineEditing({
element: TimelineElement, element: TimelineElement,
label: string, label: string,
buildPatches: PersistTimelineEditInput["buildPatches"], buildPatches: PersistTimelineEditInput["buildPatches"],
) => { ): Promise<void> => {
const pid = projectIdRef.current; const pid = projectIdRef.current;
if (!pid) return; if (!pid) return Promise.resolve();
editQueueRef.current = editQueueRef.current const queued = editQueueRef.current.then(() =>
.then(() => persistTimelineEdit({
persistTimelineEdit({ projectId: pid,
projectId: pid, element,
element, activeCompPath,
activeCompPath, label,
label, buildPatches,
buildPatches, writeProjectFile,
writeProjectFile, recordEdit,
recordEdit, domEditSaveTimestampRef,
domEditSaveTimestampRef, pendingTimelineEditPathRef,
pendingTimelineEditPathRef, }),
}), );
) editQueueRef.current = queued.catch((error) => {
.catch((error) => { console.error(`[Timeline] Failed to persist: ${label}`, error);
console.error(`[Timeline] Failed to persist: ${label}`, error); });
}); return queued;
}, },
[ [
activeCompPath, activeCompPath,
@@ -219,7 +222,7 @@ export function useTimelineEditing({
["data-start", formatTimelineAttributeNumber(updates.start)], ["data-start", formatTimelineAttributeNumber(updates.start)],
["data-track-index", String(updates.track)], ["data-track-index", String(updates.track)],
]); ]);
enqueueEdit(element, "Move timeline clip", (original, target) => { return enqueueEdit(element, "Move timeline clip", (original, target) => {
let patched = applyPatchByTarget(original, target, { let patched = applyPatchByTarget(original, target, {
type: "attribute", type: "attribute",
property: "start", property: "start",
@@ -244,7 +247,7 @@ export function useTimelineEditing({
["data-start", formatTimelineAttributeNumber(updates.start)], ["data-start", formatTimelineAttributeNumber(updates.start)],
["data-duration", formatTimelineAttributeNumber(updates.duration)], ["data-duration", formatTimelineAttributeNumber(updates.duration)],
]); ]);
enqueueEdit(element, "Resize timeline clip", (original, target) => { return enqueueEdit(element, "Resize timeline clip", (original, target) => {
const pbs = resolveResizePlaybackStart(original, target, element, updates); const pbs = resolveResizePlaybackStart(original, target, element, updates);
let patched = applyPatchByTarget(original, target, { let patched = applyPatchByTarget(original, target, {
type: "attribute", type: "attribute",
@@ -4,7 +4,6 @@ import {
buildPromptCopyText, buildPromptCopyText,
buildTimelineElementAgentPrompt, buildTimelineElementAgentPrompt,
buildTimelineAgentPrompt, buildTimelineAgentPrompt,
canOffsetTrimClipStart,
getTimelineEditCapabilities, getTimelineEditCapabilities,
hasPatchableTimelineTarget, hasPatchableTimelineTarget,
resolveBlockedTimelineEditIntent, resolveBlockedTimelineEditIntent,
@@ -158,42 +157,6 @@ describe("resolveTimelineMove", () => {
}); });
}); });
describe("canOffsetTrimClipStart", () => {
it("allows front trim for clips that carry playback offset metadata", () => {
expect(
canOffsetTrimClipStart({
tag: "div",
playbackStartAttr: "media-start",
}),
).toBe(true);
});
it("allows front trim for media clips with source duration metadata", () => {
expect(
canOffsetTrimClipStart({
tag: "video",
sourceDuration: 12,
}),
).toBe(true);
});
it("allows front trim for plain audio clips even before media-start exists", () => {
expect(
canOffsetTrimClipStart({
tag: "audio",
}),
).toBe(true);
});
it("blocks front trim for generic motion clips", () => {
expect(
canOffsetTrimClipStart({
tag: "section",
}),
).toBe(false);
});
});
describe("hasPatchableTimelineTarget", () => { describe("hasPatchableTimelineTarget", () => {
it("returns true when the clip has a DOM id", () => { it("returns true when the clip has a DOM id", () => {
expect(hasPatchableTimelineTarget({ domId: "hero-card" })).toBe(true); expect(hasPatchableTimelineTarget({ domId: "hero-card" })).toBe(true);
@@ -201,18 +201,6 @@ export function hasPatchableTimelineTarget(input: { domId?: string; selector?: s
return Boolean(input.domId || input.selector); return Boolean(input.domId || input.selector);
} }
export function canOffsetTrimClipStart(input: {
tag: string;
playbackStart?: number;
playbackStartAttr?: "media-start" | "playback-start";
sourceDuration?: number;
}): boolean {
if (input.playbackStartAttr != null) return true;
if (input.playbackStart != null) return true;
const normalizedTag = input.tag.toLowerCase();
return ["video", "audio"].includes(normalizedTag);
}
export function getTimelineEditCapabilities(input: { export function getTimelineEditCapabilities(input: {
tag: string; tag: string;
duration: number; duration: number;
@@ -122,13 +122,14 @@ export function useTimelinePlayer() {
needsProbe.map(async (el) => { needsProbe.map(async (el) => {
const result = await probeMediaUrl(el.src!); const result = await probeMediaUrl(el.src!);
if (!result) return; if (!result) return;
const current = usePlayerStore.getState().elements;
const key = el.key ?? el.id; const key = el.key ?? el.id;
const idx = current.findIndex((e) => (e.key ?? e.id) === key); usePlayerStore.setState((state) => {
if (idx === -1 || current[idx].sourceDuration != null) return; const idx = state.elements.findIndex((e) => (e.key ?? e.id) === key);
const patched = current.slice(); if (idx === -1 || state.elements[idx].sourceDuration != null) return {};
patched[idx] = { ...current[idx], sourceDuration: result.duration }; const patched = state.elements.slice();
setElements(patched); patched[idx] = { ...state.elements[idx], sourceDuration: result.duration };
return { elements: patched };
});
}), }),
); );
} }