From ec35998b0ce31f7d51fb5bc93d4d80255328c683 Mon Sep 17 00:00:00 2001 From: Arham Wani Date: Sat, 8 Aug 2026 23:50:15 +0530 Subject: [PATCH 1/5] fix(editor): surface failed timeline saves --- src/components/ai-edition/NewEditorShell.tsx | 6 +- src/components/ai-edition/v4/V4Timeline.tsx | 1 + src/lib/ai-edition/store/timelineSave.ts | 24 ++++++++ .../store/useSequentialTimelineOps.test.ts | 19 ++++--- .../store/useSequentialTimelineOps.ts | 22 ++++--- src/lib/ai-edition/store/useTimeline.test.ts | 57 +++++++++++++++++++ src/lib/ai-edition/store/useTimeline.ts | 51 +++++++++++++---- 7 files changed, 149 insertions(+), 31 deletions(-) create mode 100644 src/lib/ai-edition/store/timelineSave.ts diff --git a/src/components/ai-edition/NewEditorShell.tsx b/src/components/ai-edition/NewEditorShell.tsx index 68931e66c..a3f09a9ab 100644 --- a/src/components/ai-edition/NewEditorShell.tsx +++ b/src/components/ai-edition/NewEditorShell.tsx @@ -14,6 +14,7 @@ import { } from "@/lib/ai-edition/document/timeline"; import { type AxcutClip, documentSchema } from "@/lib/ai-edition/schema"; import { useProjectStore } from "@/lib/ai-edition/store/projectStore"; +import { saveTimelineMutation } from "@/lib/ai-edition/store/timelineSave"; import { useAssetTranscriptions, useAutoTranscription, @@ -344,6 +345,7 @@ export function NewEditorShell() { // stale-closure bugs. const known = Number.isFinite(durationSec) && durationSec > 0 ? durationSec : 60; const state = useProjectStore.getState(); + const persist = state.saveDocument; setSourceDuration(known); const doc = state.document; if (!doc || doc.assets.length === 0) return; @@ -366,7 +368,7 @@ export function NewEditorShell() { [{ startSec: 0, endSec: known }], "Auto-created full-duration clip", ); - void state.saveDocument(next); + void saveTimelineMutation(persist, next); return; } // Hand the probed duration to the pure document layer: it patches only the @@ -377,7 +379,7 @@ export function NewEditorShell() { // nothing is waiting, so there is nothing to guard here. const next = applyProbedDuration(doc, assetId, known); if (next !== doc) { - void state.saveDocument(next); + void saveTimelineMutation(persist, next); } }, [setSourceDuration], diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index 22b79aa8d..96fe96f59 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -1059,6 +1059,7 @@ export function V4Timeline({ return; } const added = await tl.addZoomsBulk(suggestions); + if (added === 0) return; toast.success( t(added === 1 ? "toolbar.addedAutoZoom" : "toolbar.addedAutoZoomPlural", { count: added }), ); diff --git a/src/lib/ai-edition/store/timelineSave.ts b/src/lib/ai-edition/store/timelineSave.ts new file mode 100644 index 000000000..075dcaf29 --- /dev/null +++ b/src/lib/ai-edition/store/timelineSave.ts @@ -0,0 +1,24 @@ +import { toast } from "sonner"; +import type { AxcutDocument } from "../schema"; + +type SaveDocument = (document: AxcutDocument) => Promise; + +/** + * Persist a user-initiated timeline mutation without letting a detached caller + * turn a write failure into an unhandled rejection. + */ +export async function saveTimelineMutation( + saveDocument: SaveDocument, + document: AxcutDocument, +): Promise { + try { + await saveDocument(document); + return true; + } catch (error) { + console.error("[timeline] failed to save mutation:", error); + toast.error("Save failed", { + description: error instanceof Error ? error.message : String(error), + }); + return false; + } +} diff --git a/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts b/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts index f86088bcf..72d46d34d 100644 --- a/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts +++ b/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts @@ -18,6 +18,10 @@ import { type AxcutDocument, createEmptyDocument } from "@/lib/ai-edition/schema import { useProjectStore } from "./projectStore"; import { useSequentialTimelineOps } from "./useSequentialTimelineOps"; +const toastErrorMock = vi.hoisted(() => vi.fn()); + +vi.mock("sonner", () => ({ toast: { error: toastErrorMock } })); + function makeDocWithAsset(): AxcutDocument { const base = createEmptyDocument({ projectId: "proj_seq", title: "seq" }); return { @@ -55,6 +59,7 @@ function makeDocWithAsset(): AxcutDocument { beforeEach(() => { useProjectStore.getState().clear(); + toastErrorMock.mockReset(); }); afterEach(() => { @@ -112,7 +117,7 @@ describe("useSequentialTimelineOps", () => { expect(doc2.timeline.trimRanges.map((t) => t.startSec).sort()).toEqual([1, 5]); }); - it("swallows save errors so the next call can still proceed", async () => { + it("surfaces save errors without rejecting or poisoning the queue", async () => { const seed = makeDocWithAsset(); useProjectStore.setState({ document: seed }); @@ -140,22 +145,22 @@ describe("useSequentialTimelineOps", () => { reason: "second", }; - let firstSettled = false; + let firstResult: AxcutDocument | null | undefined; let secondSettled = false; await act(async () => { const p1 = result.current.apply(op1); const p2 = result.current.apply(op2); - await p1.catch((err: unknown) => { - firstSettled = true; - expect((err as Error).message).toBe("save failed"); - }); + firstResult = await p1; await p2.then(() => { secondSettled = true; }); }); - expect(firstSettled).toBe(true); + expect(firstResult).toBeNull(); expect(secondSettled).toBe(true); + expect(toastErrorMock).toHaveBeenCalledWith("Save failed", { + description: "save failed", + }); // The queue survived the first failure — both saves were attempted. expect(saveDocument).toHaveBeenCalledTimes(2); }); diff --git a/src/lib/ai-edition/store/useSequentialTimelineOps.ts b/src/lib/ai-edition/store/useSequentialTimelineOps.ts index 642c5a435..c9c25383f 100644 --- a/src/lib/ai-edition/store/useSequentialTimelineOps.ts +++ b/src/lib/ai-edition/store/useSequentialTimelineOps.ts @@ -7,15 +7,15 @@ // read the doc INSIDE the chain, after awaiting the previous save, so // every call sees the doc state the previous call committed. // -// Errors are swallowed when advancing the queue ref so a failed save -// doesn't poison the queue (the next call still has a resolved promise -// to chain off). The original promise returned to the caller is NOT -// swallowed — the caller can await it and observe the rejection. +// Save failures are surfaced once by the shared mutation boundary and resolve +// to null. This keeps detached UI calls from emitting unhandled rejections and +// also leaves the queue healthy for the next edit. import { useCallback, useRef } from "react"; import type { AxcutTimelineOperation } from "@/lib/ai-edition/document/operations"; import type { AxcutDocument } from "@/lib/ai-edition/schema"; import { useProjectStore } from "./projectStore"; +import { saveTimelineMutation } from "./timelineSave"; export interface SequentialTimelineOps { /** @@ -25,7 +25,7 @@ export interface SequentialTimelineOps { * saved. Calls are serialised — op N+1 reads the doc op N wrote. * * Returns the saved document, or `null` if no project document is - * loaded (store empty AND no fallback supplied). + * loaded (store empty AND no fallback supplied) or the save fails. */ apply: (op: AxcutTimelineOperation) => Promise; } @@ -43,7 +43,7 @@ export function useSequentialTimelineOps(options: { (op: AxcutTimelineOperation): Promise => { const queued = saveQueueRef.current .then(() => import("@/lib/ai-edition/document/operations")) - .then(({ applyTimelineOperation }) => { + .then(async ({ applyTimelineOperation }) => { // Read the doc inside the chain. The store holds the // latest committed state because the previous call's // save has already resolved by the time this .then @@ -51,13 +51,11 @@ export function useSequentialTimelineOps(options: { const doc = useProjectStore.getState().document ?? fallbackDocument; if (!doc) return null; const applied = applyTimelineOperation(doc, op); - return saveDocument(applied.document).then(() => applied.document); + const saved = await saveTimelineMutation(saveDocument, applied.document); + return saved ? applied.document : null; }); - // Swallow rejection when advancing the queue so a failed save - // doesn't poison the queue — the next call still has a - // resolved promise to chain off. The original `queued` is - // returned to the caller, who can await it and observe the - // rejection. + // Keep operation/import errors from poisoning the queue. Save + // failures already resolve to null after showing user feedback. saveQueueRef.current = queued.then( () => undefined, () => undefined, diff --git a/src/lib/ai-edition/store/useTimeline.test.ts b/src/lib/ai-edition/store/useTimeline.test.ts index 7f4541768..31cc075cf 100644 --- a/src/lib/ai-edition/store/useTimeline.test.ts +++ b/src/lib/ai-edition/store/useTimeline.test.ts @@ -18,6 +18,9 @@ const probeVideoDurationMock = vi.hoisted(() => vi.fn()); const probeVideoDimensionsMock = vi.hoisted(() => vi.fn().mockResolvedValue({ width: 1920, height: 1080 }), ); +const toastErrorMock = vi.hoisted(() => vi.fn()); + +vi.mock("sonner", () => ({ toast: { error: toastErrorMock } })); vi.mock("../timeline/duration", async (importOriginal) => { const actual = await importOriginal(); @@ -546,6 +549,30 @@ describe("useTimeline zoom modifiers (rotation + focus mode)", () => { focusMode: "auto", }); }); + + it("rolls a live focus edit back when its commit cannot be saved", async () => { + bridgeMocks.save.mockResolvedValueOnce({ success: false, error: "project file locked" }); + const { result } = renderTimeline(); + + act(() => result.current.updateZoomFocusLive("zoom_a", { cx: 0.8, cy: 0.2 })); + expect(useProjectStore.getState().document?.zoomRanges[0]?.focus).toEqual({ + cx: 0.8, + cy: 0.2, + }); + + await act(async () => { + await result.current.commitZoomFocus(); + }); + + expect(useProjectStore.getState().document?.zoomRanges[0]?.focus).toEqual({ + cx: 0.5, + cy: 0.5, + }); + expect(useProjectStore.getState().dirty).toBe(false); + expect(toastErrorMock).toHaveBeenCalledWith("Save failed", { + description: "project file locked", + }); + }); }); // Regression guard for the playhead-stutter fix. `currentTimeSec` is rewritten on @@ -669,3 +696,33 @@ describe("useTimeline selection", () => { expect(result.current.clipSelection).toBeNull(); }); }); + +describe("useTimeline save failures", () => { + beforeEach(() => { + useProjectStore.getState().clear(); + for (const mock of Object.values(bridgeMocks)) mock.mockReset(); + toastErrorMock.mockReset(); + bridgeMocks.save.mockResolvedValue({ success: false, error: "disk full" }); + useProjectStore.setState({ + projectId: "proj_test", + document: sampleDoc, + revision: 1, + status: "ready", + error: null, + }); + }); + + it("surfaces a rejected mutation without applying it or leaking the rejection", async () => { + const { result } = renderTimeline(); + + await act(async () => { + await expect(result.current.removeClip("clip_a")).resolves.toBeUndefined(); + }); + + expect(toastErrorMock).toHaveBeenCalledWith("Save failed", { + description: "disk full", + }); + expect(useProjectStore.getState().document?.timeline.clips).toHaveLength(1); + expect(useProjectStore.getState().document?.timeline.clips[0]?.id).toBe("clip_a"); + }); +}); diff --git a/src/lib/ai-edition/store/useTimeline.ts b/src/lib/ai-edition/store/useTimeline.ts index cd763bae1..98038eca2 100644 --- a/src/lib/ai-edition/store/useTimeline.ts +++ b/src/lib/ai-edition/store/useTimeline.ts @@ -30,6 +30,7 @@ import { import { dropTrimPillsByIds, resolveTimelineSpanToTrim } from "../timeline/trim-mapping"; import type { AutoZoomSuggestion } from "../timeline/zoom-suggestions"; import { useProjectStore } from "./projectStore"; +import { saveTimelineMutation } from "./timelineSave"; // How long a region lasts when the caller doesn't say. The timeline's toolbar // passes its own duration instead, derived from the current zoom so the new pill @@ -96,7 +97,11 @@ export function useTimeline() { const ts = useScopedT("settings"); const document = useProjectStore((s) => s.document); const projectId = useProjectStore((s) => s.projectId); - const saveDocument = useProjectStore((s) => s.saveDocument); + const saveProjectDocument = useProjectStore((s) => s.saveDocument); + const saveDocument = useCallback( + (document: AxcutDocument) => saveTimelineMutation(saveProjectDocument, document), + [saveProjectDocument], + ); const setDocument = useProjectStore((s) => s.setDocument); const [selection, setSelection] = useState(null); // F2.7 — shift-click multi-selection. `selection` stays the inspector's @@ -104,6 +109,10 @@ export function useTimeline() { // the Delete key operates on. const [multiSelection, setMultiSelection] = useState([]); const [clipSelection, setClipSelection] = useState(null); + const zoomFocusRollbackRef = useRef(null); + const zoomFocusLiveRef = useRef(null); + const annotationRollbackRef = useRef(null); + const annotationLiveRef = useRef(null); const hasDoc = document !== null && projectId !== null; @@ -217,7 +226,7 @@ export function useTimeline() { ...document, zoomRanges: [...document.zoomRanges, ...anchored] as AxcutDocument["zoomRanges"], }; - await saveDocument(next); + if (!(await saveDocument(next))) return 0; return suggestions.length; }, [document, saveDocument], @@ -305,7 +314,7 @@ export function useTimeline() { ...created, ] as unknown as AxcutDocument["annotations"], }; - await saveDocument(next); + if (!(await saveDocument(next))) return; // Select the freshly added annotation so its inspector opens and it shows a // selection box on the canvas, ready to be retyped over. const newId = created[0]?.id ?? ann.id; @@ -487,6 +496,7 @@ export function useTimeline() { (id: string, focus: { cx: number; cy: number }) => { const doc = useProjectStore.getState().document; if (!doc) return; + if (zoomFocusLiveRef.current !== doc) zoomFocusRollbackRef.current = doc; const next: AxcutDocument = { ...doc, zoomRanges: patchPillById(doc.zoomRanges, id, { @@ -494,6 +504,7 @@ export function useTimeline() { }) as AxcutDocument["zoomRanges"], }; setDocument(next); + zoomFocusLiveRef.current = next; }, [setDocument], ); @@ -501,7 +512,16 @@ export function useTimeline() { const commitZoomFocus = useCallback(async () => { const doc = useProjectStore.getState().document; if (!doc) return; - await saveDocument(doc); + const rollback = zoomFocusRollbackRef.current; + zoomFocusRollbackRef.current = null; + zoomFocusLiveRef.current = null; + if (!(await saveDocument(doc)) && rollback) { + useProjectStore.setState((state) => + state.document === doc + ? { document: rollback, revision: state.revision + 1, dirty: false } + : {}, + ); + } }, [saveDocument]); // Zoom-level control for the region-settings panel (1-6, matches @@ -590,11 +610,13 @@ export function useTimeline() { (id: string, patch: Partial) => { const doc = useProjectStore.getState().document; if (!doc) return; + if (annotationLiveRef.current !== doc) annotationRollbackRef.current = doc; const next: AxcutDocument = { ...doc, annotations: patchPillById(doc.annotations, id, patch), }; setDocument(next); + annotationLiveRef.current = next; }, [setDocument], ); @@ -602,7 +624,16 @@ export function useTimeline() { const commitAnnotationChange = useCallback(async () => { const doc = useProjectStore.getState().document; if (!doc) return; - await saveDocument(doc); + const rollback = annotationRollbackRef.current; + annotationRollbackRef.current = null; + annotationLiveRef.current = null; + if (!(await saveDocument(doc)) && rollback) { + useProjectStore.setState((state) => + state.document === doc + ? { document: rollback, revision: state.revision + 1, dirty: false } + : {}, + ); + } }, [saveDocument]); const updateSpeedSpan = useCallback( @@ -692,7 +723,7 @@ export function useTimeline() { async (kind: RegionKind, id: string) => { if (!document) return; // One shared mutator with the agent's removeTrim / removeModifier tools. - await saveDocument(removeRegionInDocument(document, kind, id)); + if (!(await saveDocument(removeRegionInDocument(document, kind, id)))) return; if (selection?.id === id) setSelection(null); setMultiSelection((prev) => prev.filter((h) => h.id !== id)); }, @@ -743,7 +774,7 @@ export function useTimeline() { ? { ...legacy, speedRegions: prevSpeed, cameraFullscreenRegions: prevCameraFullscreen } : document.legacyEditor, }; - await saveDocument(next); + if (!(await saveDocument(next))) return; setSelection(null); setMultiSelection([]); }, @@ -926,7 +957,7 @@ export function useTimeline() { timeline: { ...currentDoc.timeline, clips: newClips }, }; const finalDoc = rederiveRegionMs(next, newClips); - await saveDocument(finalDoc); + if (!(await saveDocument(finalDoc))) return; setClipSelection(newClip.id); // If we used the placeholder, kick off the probe in the background. @@ -971,7 +1002,7 @@ export function useTimeline() { // original, so its index in the result is the original's index + 1. const insertedIndex = document.timeline.clips.findIndex((c) => c.id === clipId) + 1; const next = duplicateClipInDocument(document, clipId, "user", "Duplicated clip"); - await saveDocument(next); + if (!(await saveDocument(next))) return; setClipSelection(next.timeline.clips[insertedIndex]?.id ?? null); }, [document, saveDocument], @@ -981,7 +1012,7 @@ export function useTimeline() { async (clipId: string) => { if (!document) return; // One shared mutator with the agent's removeClip tool: reflow survivors + rederive pills. - await saveDocument(removeClipInDocument(document, clipId)); + if (!(await saveDocument(removeClipInDocument(document, clipId)))) return; if (clipSelection === clipId) setClipSelection(null); }, [document, clipSelection, saveDocument], From 04f3be0062ac121963385cfd29de7454146e53b5 Mon Sep 17 00:00:00 2001 From: Arham Wani Date: Sun, 9 Aug 2026 00:14:11 +0530 Subject: [PATCH 2/5] test(editor): strengthen failed-save regressions --- .../ai-edition/store/useSequentialTimelineOps.test.ts | 9 ++++----- src/lib/ai-edition/store/useTimeline.test.ts | 4 ++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts b/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts index 72d46d34d..4b85a62d5 100644 --- a/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts +++ b/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts @@ -146,23 +146,22 @@ describe("useSequentialTimelineOps", () => { }; let firstResult: AxcutDocument | null | undefined; - let secondSettled = false; + let secondResult: AxcutDocument | null | undefined; await act(async () => { const p1 = result.current.apply(op1); const p2 = result.current.apply(op2); firstResult = await p1; - await p2.then(() => { - secondSettled = true; - }); + secondResult = await p2; }); expect(firstResult).toBeNull(); - expect(secondSettled).toBe(true); + expect(secondResult).not.toBeNull(); expect(toastErrorMock).toHaveBeenCalledWith("Save failed", { description: "save failed", }); // The queue survived the first failure — both saves were attempted. expect(saveDocument).toHaveBeenCalledTimes(2); + expect(saveDocument).toHaveBeenNthCalledWith(2, secondResult); }); it("returns null when the store has no document and no fallback is supplied", async () => { diff --git a/src/lib/ai-edition/store/useTimeline.test.ts b/src/lib/ai-edition/store/useTimeline.test.ts index 31cc075cf..52f39d5e0 100644 --- a/src/lib/ai-edition/store/useTimeline.test.ts +++ b/src/lib/ai-edition/store/useTimeline.test.ts @@ -569,6 +569,10 @@ describe("useTimeline zoom modifiers (rotation + focus mode)", () => { cy: 0.5, }); expect(useProjectStore.getState().dirty).toBe(false); + // The live edit advanced revision once; restoring a different document + // advances it again so async work cannot mistake the rollback for the + // optimistic document it replaced. + expect(useProjectStore.getState().revision).toBe(3); expect(toastErrorMock).toHaveBeenCalledWith("Save failed", { description: "project file locked", }); From 184bb5ab75200e1f3c643e1f2aa2d7bbad11d1fe Mon Sep 17 00:00:00 2001 From: Arham Wani Date: Mon, 10 Aug 2026 05:03:53 +0530 Subject: [PATCH 3/5] chore(editor): keep auto-zoom change scoped --- src/components/ai-edition/v4/V4Timeline.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index 96fe96f59..22b79aa8d 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -1059,7 +1059,6 @@ export function V4Timeline({ return; } const added = await tl.addZoomsBulk(suggestions); - if (added === 0) return; toast.success( t(added === 1 ? "toolbar.addedAutoZoom" : "toolbar.addedAutoZoomPlural", { count: added }), ); From 00bf80253b474b7b25d09f415380ec0e5adc0d1d Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Thu, 20 Aug 2026 12:11:40 +0200 Subject: [PATCH 4/5] refactor(i18n): one home for translating toasts fired outside React `transcriptionStore` had the only copy of "read the stored locale, validate it, translate" -- the thing any store needs to report to the user in their language. The project store is about to need it too, so it moves to `@/i18n/toastText` and takes the namespace as an argument instead of hardcoding `editor`. Co-Authored-By: Claude Opus 5 --- src/i18n/toastText.ts | 25 ++++++++ .../ai-edition/store/transcriptionStore.ts | 59 ++++++++----------- 2 files changed, 49 insertions(+), 35 deletions(-) create mode 100644 src/i18n/toastText.ts diff --git a/src/i18n/toastText.ts b/src/i18n/toastText.ts new file mode 100644 index 000000000..9b20d57e9 --- /dev/null +++ b/src/i18n/toastText.ts @@ -0,0 +1,25 @@ +// Translation for toasts fired OUTSIDE React -- stores, and anything else that reports +// to the user without a component around it to call `useScopedT`. + +import { DEFAULT_LOCALE, type I18nNamespace, LOCALE_STORAGE_KEY, type Locale } from "./config"; +import { getAvailableLocales, translate } from "./loader"; + +/** + * Toasts fired outside React still have to speak the user's language. Same source as + * `I18nProvider` (stored preference, else the default), validated so a stale value + * cannot push `translate` onto a locale it does not have. + */ +export function toastText( + namespace: I18nNamespace, + key: string, + vars?: Record, +): string { + let locale: Locale = DEFAULT_LOCALE; + try { + const stored = localStorage.getItem(LOCALE_STORAGE_KEY); + if (stored && getAvailableLocales().includes(stored as Locale)) locale = stored as Locale; + } catch { + // localStorage may be unavailable -- the default locale is a fine answer. + } + return translate(locale, namespace, key, vars); +} diff --git a/src/lib/ai-edition/store/transcriptionStore.ts b/src/lib/ai-edition/store/transcriptionStore.ts index 1cee92d11..3c3c1139f 100644 --- a/src/lib/ai-edition/store/transcriptionStore.ts +++ b/src/lib/ai-edition/store/transcriptionStore.ts @@ -25,8 +25,7 @@ import { useEffect, useMemo } from "react"; import { toast } from "sonner"; import { create } from "zustand"; -import { DEFAULT_LOCALE, LOCALE_STORAGE_KEY, type Locale } from "@/i18n/config"; -import { getAvailableLocales, translate } from "@/i18n/loader"; +import { toastText as translateToast } from "@/i18n/toastText"; import { transcribeAsset, withTranscript } from "../document/transcribe"; import type { AxcutDocument } from "../schema"; import { @@ -85,21 +84,9 @@ function hasLocalSttEngine(): boolean { return typeof window.electronAPI?.stt?.transcribe === "function"; } -/** - * Toasts fired outside React still have to speak the user's language. Same - * source as `I18nProvider` (stored preference, else the default), validated so - * a stale value can't push `translate` onto a locale it doesn't have. - */ -function toastText(key: string, vars?: Record): string { - let locale: Locale = DEFAULT_LOCALE; - try { - const stored = localStorage.getItem(LOCALE_STORAGE_KEY); - if (stored && getAvailableLocales().includes(stored as Locale)) locale = stored as Locale; - } catch { - // localStorage may be unavailable — the default locale is a fine answer. - } - return translate(locale, "editor", key, vars); -} +/** This store's toasts all live in the `editor` namespace. */ +const toastText = (key: string, vars?: Record) => + translateToast("editor", key, vars); export const useTranscriptionStore = create((set, get) => ({ projectId: null, @@ -321,24 +308,26 @@ async function persistPermanentFailure( const doc = project.document; if (!doc || doc.project.id !== projectId) return; if (!doc.assets.some((a) => a.id === assetId)) return; - try { - await project.saveDocument({ - ...doc, - assets: doc.assets.map((a) => - a.id === assetId - ? { - ...a, - transcriptionFailure: { - kind, - message: failure.message, - at: new Date().toISOString(), - }, - } - : a, - ), - }); - } catch (error) { - console.warn("[transcription] could not persist the failure on the asset:", error); + // Best-effort bookkeeping: `saveDocument` reports its own failures and resolves + // false rather than throwing, and a note on the asset is not worth a second + // message on top of the one the user already got. + const persisted = await project.saveDocument({ + ...doc, + assets: doc.assets.map((a) => + a.id === assetId + ? { + ...a, + transcriptionFailure: { + kind, + message: failure.message, + at: new Date().toISOString(), + }, + } + : a, + ), + }); + if (!persisted) { + console.warn("[transcription] could not persist the failure on the asset"); } } From 797755dd9d8459e86dbac908248beaa68f9e7559 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Thu, 20 Aug 2026 12:11:56 +0200 Subject: [PATCH 5/5] fix(editor): put the save-failure boundary where every save already goes Four findings from the review, all of them real. **The boundary was one rung too low.** `saveTimelineMutation` wrapped the timeline hook, so the timeline was covered while every sibling writer of the same document stayed silent -- and they are all reached by `void`-ed callers, so they emitted unhandled rejections with no toast, no log and no clue. Changing the wallpaper or a caption font on a read-only project lost the edit with zero feedback, through `useEditorSettings`, `useCaptions`, `CaptionsPane`, `LeftPanel` and `transcriptionStore`. Picking Save in the unsaved-changes dialog swallowed the error in a bare `catch {}` and resolved "cancel", so the window silently refused to close. `projectStore.saveDocument` is the one function all of them already call. The catch, the log and the toast live there now and it returns a boolean instead of throwing, which covers every path -- including the two direct store saves the hook could never see, one of which is the dimension backfill that fires on any project migrated from before dimensions were probed. `timelineSave.ts` is gone; this is a smaller diff than the wrapper it replaces. `dirty` is deliberately left set on failure: it is the only input to the `beforeunload` guard and to `setHasUnsavedChanges`, so a failed write is the last moment to claim there is nothing to save. The five callers that used the throw for control flow now read the boolean, and none of them lost behaviour: the close-request handler still answers false, the unsaved dialog still cancels, Ctrl+N and Ctrl+O still stay put. Their local "Save failed" / "Rename failed" toasts are gone -- a second, English-only copy of what the store now says in the user's language, which is the other half of this: the toast read `"Save failed"` hardcoded in a 13-locale app, and no locale file was touched, so `i18n:check` could not see it. It goes through `project.failedToSave` now. **Auto-enhance congratulated itself on a failed write.** `addZoomsBulk` was changed to toast and return 0 instead of throwing, but its caller kept toasting success unconditionally -- so a locked file produced "Added 0 automatic zooms" stacked on the failure, with no zoom anywhere. Before, the throw reached the caller's `catch` and produced one correct message. The caller guards on 0 again, and the producer's contract is now pinned by a test. **A rollback outlived its project.** `ZoomFocusOverlay` unmounts the instant `focusMode` flips to "auto", so `endDrag` never runs and the pre-drag snapshot stayed in the ref. Open another project, reset the focus, let that save fail, and project A's document was restored into project B -- the next successful save then wrote A over B on disk. The refs are cleared when the project changes, which also stops pinning two whole documents per hook instance when annotations can carry base64 image data URLs. **And the rollback claimed a cleanliness it could not prove.** It set `dirty: false`, but the rollback target is the document the drag started from, not the last SAVED one: with two commits in flight the first one's unsaved document is what gets restored. Saying "clean" there let the window close on real work without prompting. Co-Authored-By: Claude Opus 5 --- src/components/ai-edition/NewEditorShell.tsx | 66 +++++-------------- src/components/ai-edition/v4/V4Timeline.tsx | 4 ++ src/lib/ai-edition/store/projectStore.test.ts | 52 +++++++++++++++ src/lib/ai-edition/store/projectStore.ts | 55 ++++++++++++---- src/lib/ai-edition/store/timelineSave.ts | 24 ------- .../store/useSequentialTimelineOps.test.ts | 27 ++++---- .../store/useSequentialTimelineOps.ts | 28 ++++---- src/lib/ai-edition/store/useTimeline.test.ts | 57 +++++++++++++++- src/lib/ai-edition/store/useTimeline.ts | 42 ++++++++---- 9 files changed, 228 insertions(+), 127 deletions(-) delete mode 100644 src/lib/ai-edition/store/timelineSave.ts diff --git a/src/components/ai-edition/NewEditorShell.tsx b/src/components/ai-edition/NewEditorShell.tsx index a3f09a9ab..b28f8558e 100644 --- a/src/components/ai-edition/NewEditorShell.tsx +++ b/src/components/ai-edition/NewEditorShell.tsx @@ -14,7 +14,6 @@ import { } from "@/lib/ai-edition/document/timeline"; import { type AxcutClip, documentSchema } from "@/lib/ai-edition/schema"; import { useProjectStore } from "@/lib/ai-edition/store/projectStore"; -import { saveTimelineMutation } from "@/lib/ai-edition/store/timelineSave"; import { useAssetTranscriptions, useAutoTranscription, @@ -303,15 +302,9 @@ export function NewEditorShell() { // 3. Handle request-save-before-close from Electron const unsubSaveBeforeClose = window.electronAPI.onRequestSaveBeforeClose(async () => { const doc = useProjectStore.getState().document; - if (doc) { - try { - await saveDocument(doc); - return true; - } catch { - toast.error("Failed to save before closing"); - return false; - } - } + // The store already toasted the reason; answering false is what keeps the + // window open on top of it. + if (doc) return await saveDocument(doc); return true; }); @@ -345,7 +338,6 @@ export function NewEditorShell() { // stale-closure bugs. const known = Number.isFinite(durationSec) && durationSec > 0 ? durationSec : 60; const state = useProjectStore.getState(); - const persist = state.saveDocument; setSourceDuration(known); const doc = state.document; if (!doc || doc.assets.length === 0) return; @@ -368,7 +360,7 @@ export function NewEditorShell() { [{ startSec: 0, endSec: known }], "Auto-created full-duration clip", ); - void saveTimelineMutation(persist, next); + void state.saveDocument(next); return; } // Hand the probed duration to the pure document layer: it patches only the @@ -379,7 +371,7 @@ export function NewEditorShell() { // nothing is waiting, so there is nothing to guard here. const next = applyProbedDuration(doc, assetId, known); if (next !== doc) { - void saveTimelineMutation(persist, next); + void state.saveDocument(next); } }, [setSourceDuration], @@ -620,14 +612,7 @@ export function NewEditorShell() { const handleSave = useCallback(async () => { const doc = useProjectStore.getState().document; if (!doc) return; - try { - await saveDocument(doc); - toast.success("Project saved"); - } catch (err) { - toast.error("Save failed", { - description: err instanceof Error ? err.message : String(err), - }); - } + if (await saveDocument(doc)) toast.success("Project saved"); }, [saveDocument]); // Native File menu (electron/main.ts) → v4 actions. The menu is shown via @@ -655,13 +640,7 @@ export function NewEditorShell() { const doc = useProjectStore.getState().document; if (!doc) return; if (title === doc.project.title) return; - try { - await saveDocument({ ...doc, project: { ...doc.project, title } }); - } catch (err) { - toast.error("Rename failed", { - description: err instanceof Error ? err.message : String(err), - }); - } + await saveDocument({ ...doc, project: { ...doc.project, title } }); }, [saveDocument], ); @@ -682,13 +661,12 @@ export function NewEditorShell() { } if (choice === "save") { const doc = useProjectStore.getState().document; - if (doc) { - try { - await saveDocument(doc); - } catch { - resolve("cancel"); - return; - } + // A failed save cancels the action that prompted this dialog. The store has + // already said why -- which is what the bare `catch {}` here used to swallow, + // leaving the window refusing to close with nothing on screen explaining it. + if (doc && !(await saveDocument(doc))) { + resolve("cancel"); + return; } } if (action === "record") { @@ -847,13 +825,8 @@ export function NewEditorShell() { if (choice === "cancel") return; if (choice === "save") { const doc = useProjectStore.getState().document; - if (doc) { - try { - await saveDocument(doc); - } catch { - return; - } - } + // Stay put if the save did not land -- the store has already said why. + if (doc && !(await saveDocument(doc))) return; } setNewProjectOpen(true); })(); @@ -866,13 +839,8 @@ export function NewEditorShell() { if (choice === "cancel") return; if (choice === "save") { const doc = useProjectStore.getState().document; - if (doc) { - try { - await saveDocument(doc); - } catch { - return; - } - } + // Stay put if the save did not land -- the store has already said why. + if (doc && !(await saveDocument(doc))) return; } setOpenProjectOpen(true); })(); diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index 22b79aa8d..ba549180f 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -1059,6 +1059,10 @@ export function V4Timeline({ return; } const added = await tl.addZoomsBulk(suggestions); + // A failed write returns 0 and has already toasted why. Without this the user + // got "Added 0 automatic zooms" stacked on top of "Failed to save project", + // with no zoom anywhere -- a success message for something that did not happen. + if (added === 0) return; toast.success( t(added === 1 ? "toolbar.addedAutoZoom" : "toolbar.addedAutoZoomPlural", { count: added }), ); diff --git a/src/lib/ai-edition/store/projectStore.test.ts b/src/lib/ai-edition/store/projectStore.test.ts index 5483f2b88..7435eb733 100644 --- a/src/lib/ai-edition/store/projectStore.test.ts +++ b/src/lib/ai-edition/store/projectStore.test.ts @@ -297,6 +297,58 @@ describe("useProjectStore", () => { expect(toastMocks.error.mock.calls[0][0]).toContain("video.mp4"); }); + // The save boundary. Every write in the app funnels through `saveDocument`, and + // almost every caller `void`s it from a click handler, so what this function does + // with a failure IS what the user sees. + describe("saveDocument reports a failed write instead of rejecting", () => { + it("resolves false, tells the user, and leaves the document alone", async () => { + useProjectStore.setState({ + projectId: "proj_test", + document: sampleDoc, + revision: 3, + status: "ready", + dirty: true, + }); + bridgeMocks.save.mockResolvedValue({ success: false, error: "EACCES" }); + + const edited = { ...sampleDoc, project: { ...sampleDoc.project, title: "Edited" } }; + await expect(useProjectStore.getState().saveDocument(edited)).resolves.toBe(false); + + expect(toastMocks.error).toHaveBeenCalledWith("Failed to save project", { + description: "EACCES", + }); + const state = useProjectStore.getState(); + expect(state.document?.project.title).toBe("Test"); + expect(state.revision).toBe(3); + // Still dirty: `dirty` is the only input to the beforeunload guard and to + // `setHasUnsavedChanges`, so a failed write is the last moment to claim clean. + expect(state.dirty).toBe(true); + }); + + it("never rejects, so a detached caller cannot leak an unhandled rejection", async () => { + useProjectStore.setState({ projectId: "proj_test", document: sampleDoc, dirty: true }); + bridgeMocks.save.mockRejectedValue(new Error("bridge is gone")); + + await expect(useProjectStore.getState().saveDocument(sampleDoc)).resolves.toBe(false); + expect(toastMocks.error).toHaveBeenCalledWith("Failed to save project", { + description: "bridge is gone", + }); + }); + + it("resolves true and commits on success", async () => { + const saved = { ...sampleDoc, project: { ...sampleDoc.project, title: "Saved" } }; + useProjectStore.setState({ projectId: "proj_test", document: sampleDoc, dirty: true }); + bridgeMocks.save.mockResolvedValue({ success: true, document: saved }); + + await expect(useProjectStore.getState().saveDocument(saved)).resolves.toBe(true); + + expect(toastMocks.error).not.toHaveBeenCalled(); + const state = useProjectStore.getState(); + expect(state.document?.project.title).toBe("Saved"); + expect(state.dirty).toBe(false); + }); + }); + it("removeAsset requires a loaded project", async () => { await expect(useProjectStore.getState().removeAsset("asset_x")).rejects.toThrow( "No project loaded", diff --git a/src/lib/ai-edition/store/projectStore.ts b/src/lib/ai-edition/store/projectStore.ts index 539449dab..60b42e767 100644 --- a/src/lib/ai-edition/store/projectStore.ts +++ b/src/lib/ai-edition/store/projectStore.ts @@ -1,4 +1,6 @@ +import { toast } from "sonner"; import { create } from "zustand"; +import { toastText } from "@/i18n/toastText"; import { nativeBridgeClient } from "@/native/client"; import { type Interval, @@ -36,7 +38,17 @@ export interface ProjectState { refresh: () => Promise; addAsset: (path: string, label?: string) => Promise; removeAsset: (assetId: string) => Promise; - saveDocument: (document: AxcutDocument) => Promise; + /** + * Write the document to disk. Resolves `true` when it landed, `false` when it did + * not -- and a `false` has ALREADY been reported to the user and logged, so a + * caller that ignores it is choosing not to react, not choosing to stay silent. + * + * It never rejects, by design. Every save in the app funnels through here and + * almost all of them are `void`-ed from a click handler, so a rejection here was + * an unhandled rejection in the renderer with no toast, no log and no clue -- + * change a caption font on a read-only project and the edit was simply gone. + */ + saveDocument: (document: AxcutDocument) => Promise; setDocument: (document: AxcutDocument) => void; replaceTimeline: (intervals: Interval[], reason: string) => Promise; restoreFullTimeline: () => Promise; @@ -169,8 +181,10 @@ export const useProjectStore = create((set, get) => ({ a.id === addedAsset.id ? { ...a, cameraTrack: linked } : a, ), }; - await get().saveDocument(next); - document = parseDocument(next); + // Only adopt the linked document if it actually reached disk -- otherwise + // the caller is handed a document claiming a camera link the file does not + // have. The store has already told the user the write failed. + if (await get().saveDocument(next)) document = parseDocument(next); } // success:false just means no camera was found for this asset — // the normal case for a plain imported video. Nothing to surface. @@ -212,17 +226,32 @@ export const useProjectStore = create((set, get) => ({ }, async saveDocument(document) { - const result = await nativeBridgeClient.aiEdition.save(document); - if (!result.success || !result.document) { - throw new Error(result.error ?? "Failed to save project"); + try { + const result = await nativeBridgeClient.aiEdition.save(document); + if (!result.success || !result.document) { + throw new Error(result.error ?? "Failed to save project"); + } + const parsed = parseDocument(result.document); + set({ + document: parsed, + revision: get().revision + 1, + dirty: false, + lastSavedAt: new Date(), + }); + return true; + } catch (error) { + // Logged as well as toasted: a toast is gone in five seconds, and "my edit + // disappeared" gets reported much later than that. + console.error("[project] failed to save document:", error); + toast.error(toastText("editor", "project.failedToSave"), { + description: error instanceof Error ? error.message : String(error), + }); + // `dirty` is deliberately left alone. It is the only input to the + // `beforeunload` guard and to `setHasUnsavedChanges`, so clearing it here + // would let the window close without a prompt on the one path where there is + // definitely something unsaved. + return false; } - const parsed = parseDocument(result.document); - set({ - document: parsed, - revision: get().revision + 1, - dirty: false, - lastSavedAt: new Date(), - }); }, setDocument(document) { diff --git a/src/lib/ai-edition/store/timelineSave.ts b/src/lib/ai-edition/store/timelineSave.ts deleted file mode 100644 index 075dcaf29..000000000 --- a/src/lib/ai-edition/store/timelineSave.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { toast } from "sonner"; -import type { AxcutDocument } from "../schema"; - -type SaveDocument = (document: AxcutDocument) => Promise; - -/** - * Persist a user-initiated timeline mutation without letting a detached caller - * turn a write failure into an unhandled rejection. - */ -export async function saveTimelineMutation( - saveDocument: SaveDocument, - document: AxcutDocument, -): Promise { - try { - await saveDocument(document); - return true; - } catch (error) { - console.error("[timeline] failed to save mutation:", error); - toast.error("Save failed", { - description: error instanceof Error ? error.message : String(error), - }); - return false; - } -} diff --git a/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts b/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts index 4b85a62d5..91f69aac5 100644 --- a/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts +++ b/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts @@ -18,10 +18,6 @@ import { type AxcutDocument, createEmptyDocument } from "@/lib/ai-edition/schema import { useProjectStore } from "./projectStore"; import { useSequentialTimelineOps } from "./useSequentialTimelineOps"; -const toastErrorMock = vi.hoisted(() => vi.fn()); - -vi.mock("sonner", () => ({ toast: { error: toastErrorMock } })); - function makeDocWithAsset(): AxcutDocument { const base = createEmptyDocument({ projectId: "proj_seq", title: "seq" }); return { @@ -59,7 +55,6 @@ function makeDocWithAsset(): AxcutDocument { beforeEach(() => { useProjectStore.getState().clear(); - toastErrorMock.mockReset(); }); afterEach(() => { @@ -74,10 +69,11 @@ describe("useSequentialTimelineOps", () => { const callOrder: string[] = []; const saveDocument = vi.fn(async (doc: AxcutDocument) => { // Mirror the real store: write the saved doc back so the next - // call in the queue sees the latest committed state. + // call in the queue sees the latest committed state, and report + // success the way `projectStore.saveDocument` does. useProjectStore.getState().setDocument(doc); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion callOrder.push(doc.timeline.trimRanges[0]?.startSec.toString() ?? "empty"); + return true; }); const { result } = renderHook(() => @@ -117,15 +113,20 @@ describe("useSequentialTimelineOps", () => { expect(doc2.timeline.trimRanges.map((t) => t.startSec).sort()).toEqual([1, 5]); }); - it("surfaces save errors without rejecting or poisoning the queue", async () => { + it("keeps the queue healthy when a save reports failure", async () => { + // `projectStore.saveDocument` resolves false rather than rejecting -- it has + // already told the user why. What this hook owes is that the failed op resolves + // to null and the NEXT op still runs, off a document that never took the failed + // edit. const seed = makeDocWithAsset(); useProjectStore.setState({ document: seed }); const saveDocument = vi - .fn<(doc: AxcutDocument) => Promise>() - .mockRejectedValueOnce(new Error("save failed")) + .fn<(doc: AxcutDocument) => Promise>() + .mockResolvedValueOnce(false) .mockImplementationOnce(async (doc) => { useProjectStore.getState().setDocument(doc); + return true; }); const { result } = renderHook(() => @@ -156,16 +157,13 @@ describe("useSequentialTimelineOps", () => { expect(firstResult).toBeNull(); expect(secondResult).not.toBeNull(); - expect(toastErrorMock).toHaveBeenCalledWith("Save failed", { - description: "save failed", - }); // The queue survived the first failure — both saves were attempted. expect(saveDocument).toHaveBeenCalledTimes(2); expect(saveDocument).toHaveBeenNthCalledWith(2, secondResult); }); it("returns null when the store has no document and no fallback is supplied", async () => { - const saveDocument = vi.fn(async () => undefined); + const saveDocument = vi.fn(async () => true); const { result } = renderHook(() => useSequentialTimelineOps({ fallbackDocument: null, saveDocument }), ); @@ -194,6 +192,7 @@ describe("useSequentialTimelineOps", () => { const saveDocument = vi.fn(async (doc: AxcutDocument) => { useProjectStore.getState().setDocument(doc); + return true; }); const { result } = renderHook(() => diff --git a/src/lib/ai-edition/store/useSequentialTimelineOps.ts b/src/lib/ai-edition/store/useSequentialTimelineOps.ts index c9c25383f..0bcdd842f 100644 --- a/src/lib/ai-edition/store/useSequentialTimelineOps.ts +++ b/src/lib/ai-edition/store/useSequentialTimelineOps.ts @@ -7,15 +7,16 @@ // read the doc INSIDE the chain, after awaiting the previous save, so // every call sees the doc state the previous call committed. // -// Save failures are surfaced once by the shared mutation boundary and resolve -// to null. This keeps detached UI calls from emitting unhandled rejections and -// also leaves the queue healthy for the next edit. +// A failed save resolves to null rather than rejecting -- `projectStore.saveDocument` +// reports it to the user and returns false. Operation and dynamic-import errors still +// reject the promise handed to the caller; both call sites `void` it, so those remain +// unhandled rejections. They are also the two failures that mean the code is broken +// rather than the disk, so they belong in the console. import { useCallback, useRef } from "react"; import type { AxcutTimelineOperation } from "@/lib/ai-edition/document/operations"; import type { AxcutDocument } from "@/lib/ai-edition/schema"; import { useProjectStore } from "./projectStore"; -import { saveTimelineMutation } from "./timelineSave"; export interface SequentialTimelineOps { /** @@ -24,8 +25,11 @@ export interface SequentialTimelineOps { * previous op's save has resolved), and the resulting document is * saved. Calls are serialised — op N+1 reads the doc op N wrote. * - * Returns the saved document, or `null` if no project document is - * loaded (store empty AND no fallback supplied) or the save fails. + * Returns the saved document, or `null` for either of two different things: + * no project document is loaded (store empty AND no fallback supplied), which + * is a silent no-op, or the save failed, which the user has already been told + * about. Both call sites `void` the result, so they are not distinguished; a + * caller that needs to tell them apart has to widen this return type first. */ apply: (op: AxcutTimelineOperation) => Promise; } @@ -33,8 +37,9 @@ export interface SequentialTimelineOps { export function useSequentialTimelineOps(options: { /** Used only when the project store has no document yet. */ fallbackDocument: AxcutDocument | null; - /** Persist a document. The hook awaits this before unblocking the queue. */ - saveDocument: (doc: AxcutDocument) => Promise; + /** Persist a document, resolving false if the write failed (already reported). + * The hook awaits this before unblocking the queue. */ + saveDocument: (doc: AxcutDocument) => Promise; }): SequentialTimelineOps { const { fallbackDocument, saveDocument } = options; const saveQueueRef = useRef>(Promise.resolve()); @@ -51,11 +56,10 @@ export function useSequentialTimelineOps(options: { const doc = useProjectStore.getState().document ?? fallbackDocument; if (!doc) return null; const applied = applyTimelineOperation(doc, op); - const saved = await saveTimelineMutation(saveDocument, applied.document); - return saved ? applied.document : null; + return (await saveDocument(applied.document)) ? applied.document : null; }); - // Keep operation/import errors from poisoning the queue. Save - // failures already resolve to null after showing user feedback. + // Keep operation/import errors from poisoning the queue -- the next call still + // needs a resolved promise to chain off. Save failures already resolve to null. saveQueueRef.current = queued.then( () => undefined, () => undefined, diff --git a/src/lib/ai-edition/store/useTimeline.test.ts b/src/lib/ai-edition/store/useTimeline.test.ts index 52f39d5e0..cc51b3a01 100644 --- a/src/lib/ai-edition/store/useTimeline.test.ts +++ b/src/lib/ai-edition/store/useTimeline.test.ts @@ -568,15 +568,44 @@ describe("useTimeline zoom modifiers (rotation + focus mode)", () => { cx: 0.5, cy: 0.5, }); - expect(useProjectStore.getState().dirty).toBe(false); + // Still dirty, deliberately. The rollback target is the document this drag + // started from, not the last SAVED one, so claiming "clean" would let the window + // close on unsaved work without a prompt. + expect(useProjectStore.getState().dirty).toBe(true); // The live edit advanced revision once; restoring a different document // advances it again so async work cannot mistake the rollback for the // optimistic document it replaced. expect(useProjectStore.getState().revision).toBe(3); - expect(toastErrorMock).toHaveBeenCalledWith("Save failed", { + expect(toastErrorMock).toHaveBeenCalledWith("Failed to save project", { description: "project file locked", }); }); + + it("does not restore another project's document after the project changed", async () => { + // A drag does not always end in a commit: `ZoomFocusOverlay` unmounts the instant + // `focusMode` flips to "auto", so `endDrag` never runs and the snapshot outlives + // the project. Restoring it into the NEXT project put project A's document in + // project B, and the following successful save wrote A over B on disk. + const { result, rerender } = renderTimeline(); + + act(() => result.current.updateZoomFocusLive("zoom_a", { cx: 0.8, cy: 0.2 })); + + const otherProjectDoc: AxcutDocument = { + ...docWithZoom, + project: { ...docWithZoom.project, id: "proj_other", title: "Other" }, + }; + act(() => { + useProjectStore.setState({ projectId: "proj_other", document: otherProjectDoc }); + }); + rerender(); + + bridgeMocks.save.mockResolvedValueOnce({ success: false, error: "project file locked" }); + await act(async () => { + await result.current.commitZoomFocus(); + }); + + expect(useProjectStore.getState().document?.project.id).toBe("proj_other"); + }); }); // Regression guard for the playhead-stutter fix. `currentTimeSec` is rewritten on @@ -702,6 +731,10 @@ describe("useTimeline selection", () => { }); describe("useTimeline save failures", () => { + afterEach(() => { + vi.clearAllMocks(); + }); + beforeEach(() => { useProjectStore.getState().clear(); for (const mock of Object.values(bridgeMocks)) mock.mockReset(); @@ -723,10 +756,28 @@ describe("useTimeline save failures", () => { await expect(result.current.removeClip("clip_a")).resolves.toBeUndefined(); }); - expect(toastErrorMock).toHaveBeenCalledWith("Save failed", { + expect(toastErrorMock).toHaveBeenCalledWith("Failed to save project", { description: "disk full", }); expect(useProjectStore.getState().document?.timeline.clips).toHaveLength(1); expect(useProjectStore.getState().document?.timeline.clips[0]?.id).toBe("clip_a"); }); + + it("reports 0 zooms added when the bulk write fails", async () => { + // The count is what the Auto-enhance caller shows in its success toast, so a + // failed write returning `suggestions.length` produced "Added 3 automatic zooms" + // stacked on "Failed to save project", with no zoom anywhere. The caller guards + // on this 0 (`V4Timeline` runAutoZooms). + const { result } = renderTimeline(); + + let added: number | undefined; + await act(async () => { + added = await result.current.addZoomsBulk([ + { span: { start: 1000, end: 2000 }, focus: { cx: 0.5, cy: 0.5 } }, + ]); + }); + + expect(added).toBe(0); + expect(useProjectStore.getState().document?.zoomRanges).toHaveLength(0); + }); }); diff --git a/src/lib/ai-edition/store/useTimeline.ts b/src/lib/ai-edition/store/useTimeline.ts index 98038eca2..f921a9917 100644 --- a/src/lib/ai-edition/store/useTimeline.ts +++ b/src/lib/ai-edition/store/useTimeline.ts @@ -30,7 +30,6 @@ import { import { dropTrimPillsByIds, resolveTimelineSpanToTrim } from "../timeline/trim-mapping"; import type { AutoZoomSuggestion } from "../timeline/zoom-suggestions"; import { useProjectStore } from "./projectStore"; -import { saveTimelineMutation } from "./timelineSave"; // How long a region lasts when the caller doesn't say. The timeline's toolbar // passes its own duration instead, derived from the current zoom so the new pill @@ -97,11 +96,7 @@ export function useTimeline() { const ts = useScopedT("settings"); const document = useProjectStore((s) => s.document); const projectId = useProjectStore((s) => s.projectId); - const saveProjectDocument = useProjectStore((s) => s.saveDocument); - const saveDocument = useCallback( - (document: AxcutDocument) => saveTimelineMutation(saveProjectDocument, document), - [saveProjectDocument], - ); + const saveDocument = useProjectStore((s) => s.saveDocument); const setDocument = useProjectStore((s) => s.setDocument); const [selection, setSelection] = useState(null); // F2.7 — shift-click multi-selection. `selection` stays the inspector's @@ -109,11 +104,28 @@ export function useTimeline() { // the Delete key operates on. const [multiSelection, setMultiSelection] = useState([]); const [clipSelection, setClipSelection] = useState(null); + // Pre-drag snapshots for the two optimistic paths (zoom focus, annotations), so a + // failed commit can put the document back instead of leaving an edit on screen that + // was never written. const zoomFocusRollbackRef = useRef(null); const zoomFocusLiveRef = useRef(null); const annotationRollbackRef = useRef(null); const annotationLiveRef = useRef(null); + // A drag does not always end in a commit: `ZoomFocusOverlay` unmounts the moment + // `focusMode` flips to "auto", so `endDrag` never runs and the snapshot outlives the + // project. Left alone, resetting focus in project B and failing that save restored + // project A's document into B -- the next successful save then wrote A over B. It + // also pinned two whole documents per hook instance, and annotations can carry + // base64 image data URLs. + // biome-ignore lint/correctness/useExhaustiveDependencies: projectId is the trigger, not a read — the body only clears refs. + useEffect(() => { + zoomFocusRollbackRef.current = null; + zoomFocusLiveRef.current = null; + annotationRollbackRef.current = null; + annotationLiveRef.current = null; + }, [projectId]); + const hasDoc = document !== null && projectId !== null; // Backfill missing source dimensions for any USED asset whose `video` was never probed. @@ -517,9 +529,12 @@ export function useTimeline() { zoomFocusLiveRef.current = null; if (!(await saveDocument(doc)) && rollback) { useProjectStore.setState((state) => - state.document === doc - ? { document: rollback, revision: state.revision + 1, dirty: false } - : {}, + // `dirty` is deliberately NOT cleared. The rollback target is the last document + // this drag started from, which is not the same as the last SAVED one: with two + // commits in flight the first one's unsaved document is what we restore. Saying + // "clean" there tells `beforeunload` and `setHasUnsavedChanges` there is nothing + // to save, and the window closes on real work without prompting. + state.document === doc ? { document: rollback, revision: state.revision + 1 } : {}, ); } }, [saveDocument]); @@ -629,9 +644,12 @@ export function useTimeline() { annotationLiveRef.current = null; if (!(await saveDocument(doc)) && rollback) { useProjectStore.setState((state) => - state.document === doc - ? { document: rollback, revision: state.revision + 1, dirty: false } - : {}, + // `dirty` is deliberately NOT cleared. The rollback target is the last document + // this drag started from, which is not the same as the last SAVED one: with two + // commits in flight the first one's unsaved document is what we restore. Saying + // "clean" there tells `beforeunload` and `setHasUnsavedChanges` there is nothing + // to save, and the window closes on real work without prompting. + state.document === doc ? { document: rollback, revision: state.revision + 1 } : {}, ); } }, [saveDocument]);