diff --git a/src/components/ai-edition/v4/FloatingInspector.tsx b/src/components/ai-edition/v4/FloatingInspector.tsx index 42ae170cd..c94b4dc95 100644 --- a/src/components/ai-edition/v4/FloatingInspector.tsx +++ b/src/components/ai-edition/v4/FloatingInspector.tsx @@ -24,6 +24,7 @@ import { type Rotation3DPreset, SPEED_OPTIONS, ZOOM_DEPTH_SCALES, + type ZoomDepth, } from "@/components/video-editor/types"; import { useScopedT } from "@/contexts/I18nContext"; import { @@ -413,7 +414,142 @@ function convertAnnotationKind( return { ...parked, type: next, content: restored }; } -const ZOOM_DEPTHS = [1, 2, 3, 4, 5, 6] as const; +const ZOOM_DEPTHS: readonly ZoomDepth[] = [1, 2, 3, 4, 5, 6]; + +/** + * The six zoom levels as one row of buttons, so a level is one click away instead of two + * (open the select, then pick). Six short labels fit the 300px pane on their own line, which + * is why this is a stacked label/row rather than a `paneRow`. + * + * `aria-pressed` buttons inside a labelled `role="group"` is `TranscriptLaneSwitch`'s pattern + * (the facet rail is the same buttons without the wrapper, since its own label carries), so + * every level stays in the Tab order and reads like its neighbours. Arrow keys step through + * the levels, which is what the ` - void tl.updateZoomDepth(region.id, Number(e.target.value) as 1 | 2 | 3 | 4 | 5 | 6) - } - style={selectStyle} - > - {ZOOM_DEPTHS.map((d) => ( - - ))} - , - )} +
{paneRow( ts("zoom.camera.title"), @@ -1131,6 +1250,30 @@ const selectStyle: React.CSSProperties = { font: "500 12.5px var(--font-display)", }; +// Six of these share the pane's 266px of content width, so each gets ~41px: enough for +// "1.25×" at 12px with room either side, and no horizontal padding to lose. +const zoomLevelBtnStyle: React.CSSProperties = { + flex: "1 1 0", + minWidth: 0, + height: 28, + padding: 0, + borderRadius: 8, + border: "1px solid var(--border)", + background: "var(--surface)", + color: "var(--fg-2)", + font: "500 12px var(--font-display)", + cursor: "pointer", +}; + +// Same signal as the pressed facet-rail button: accent text on the soft accent fill. +const zoomLevelPressedStyle: React.CSSProperties = { + ...zoomLevelBtnStyle, + border: "1px solid var(--accent)", + background: "var(--accent-soft)", + color: "var(--accent)", + fontWeight: 600, +}; + const secondaryBtnStyle: React.CSSProperties = { padding: "9px 14px", borderRadius: 10, diff --git a/src/components/ai-edition/v4/ZoomLevelControl.test.tsx b/src/components/ai-edition/v4/ZoomLevelControl.test.tsx new file mode 100644 index 000000000..eb18eba1d --- /dev/null +++ b/src/components/ai-edition/v4/ZoomLevelControl.test.tsx @@ -0,0 +1,391 @@ +// @vitest-environment jsdom +import "@testing-library/jest-dom"; +import { act, fireEvent, render, screen } from "@testing-library/react"; +import { useState } from "react"; +import { describe, expect, it, vi } from "vitest"; +import { ZOOM_DEPTH_SCALES, type ZoomDepth } from "@/components/video-editor/types"; + +// The pane is only reachable with a project open and a zoom region selected, so drive the +// control directly. The translator echoes keys, as in `SpeedControl.test.tsx`. +vi.mock("@/contexts/I18nContext", () => ({ + useScopedT: () => (key: string) => key, +})); + +import { ZoomLevelControl } from "./FloatingInspector"; + +function renderControl(depth: ZoomDepth) { + const updateZoomDepth = vi.fn(async () => true); + render(); + const group = screen.getByRole("group", { name: "zoom.level" }); + const buttons = screen.getAllByRole("button"); + return { updateZoomDepth, group, buttons }; +} + +/** + * The same control with the pane's half of the loop in place: in the editor `updateZoomDepth` + * writes the region and the pane re-renders with the new `depth`, so the pressed button moves + * under the keyboard. Stepping is only coherent if that feedback exists — with a frozen prop + * every arrow would keep counting from the level the control opened on. + */ +function renderControlled(initial: ZoomDepth) { + const updateZoomDepth = vi.fn(async (_id: string, _depth: ZoomDepth) => true); + function Harness() { + const [depth, setDepth] = useState(initial); + return ( + { + // Synchronously, so the re-render lands inside the `fireEvent` that caused it: + // the next keystroke in a test then sees the same DOM a user's would. + updateZoomDepth(id, next); + setDepth(next); + return Promise.resolve(true); + }, + }} + /> + ); + } + render(); + const group = screen.getByRole("group", { name: "zoom.level" }); + const buttons = screen.getAllByRole("button"); + const focusLevel = (depth: ZoomDepth) => (buttons[depth - 1] as HTMLButtonElement).focus(); + return { updateZoomDepth, group, buttons, focusLevel }; +} + +describe("ZoomLevelControl", () => { + it("renders one button per depth, labelled with the table value, current one pressed", () => { + const { buttons } = renderControl(3); + expect(buttons).toHaveLength(6); + expect(buttons.map((b) => b.textContent)).toEqual( + ([1, 2, 3, 4, 5, 6] as const).map((d) => `${ZOOM_DEPTH_SCALES[d]}×`), + ); + expect(buttons.map((b) => b.getAttribute("aria-pressed"))).toEqual([ + "false", + "false", + "true", + "false", + "false", + "false", + ]); + }); + + it("commits a level in one click", () => { + const { updateZoomDepth, buttons } = renderControl(3); + fireEvent.click(buttons[4] as HTMLButtonElement); + expect(updateZoomDepth).toHaveBeenCalledTimes(1); + expect(updateZoomDepth).toHaveBeenCalledWith("z1", 5); + }); + + it("does not write when the current level is clicked again", () => { + // A no-op edit would still land a save and an undo entry. + const { updateZoomDepth, buttons } = renderControl(3); + fireEvent.click(buttons[2] as HTMLButtonElement); + expect(updateZoomDepth).not.toHaveBeenCalled(); + }); + + // The other half of that guard: undo/redo and the agent write the region without going + // through this control, so a request of ours must never outlive the prop. The moment the + // region says something else, that is the level to compare against. + it("follows the region when the level is changed from elsewhere", async () => { + const updateZoomDepth = vi.fn(async (_id: string, _depth: ZoomDepth) => true); + const { rerender } = render( + , + ); + fireEvent.click(screen.getAllByRole("button")[4] as HTMLButtonElement); + expect(updateZoomDepth).toHaveBeenCalledWith("z1", 5); + await act(async () => { + // Let the request settle so the follow-effect is allowed to copy the prop. + }); + + // An undo lands on 2 instead of the 5 this control asked for. + rerender(); + fireEvent.click(screen.getAllByRole("button")[1] as HTMLButtonElement); + expect(updateZoomDepth).toHaveBeenCalledTimes(1); + + fireEvent.click(screen.getAllByRole("button")[4] as HTMLButtonElement); + expect(updateZoomDepth).toHaveBeenCalledTimes(2); + expect(updateZoomDepth).toHaveBeenLastCalledWith("z1", 5); + }); + + it("steps to the neighbouring level with the arrow keys and moves focus with it", () => { + const { updateZoomDepth, group, buttons, focusLevel } = renderControlled(3); + focusLevel(3); + fireEvent.keyDown(group, { key: "ArrowRight" }); + expect(updateZoomDepth).toHaveBeenLastCalledWith("z1", 4); + expect(buttons[3]).toHaveFocus(); + fireEvent.keyDown(group, { key: "ArrowDown" }); + expect(updateZoomDepth).toHaveBeenLastCalledWith("z1", 5); + expect(buttons[4]).toHaveFocus(); + fireEvent.keyDown(group, { key: "ArrowLeft" }); + expect(updateZoomDepth).toHaveBeenLastCalledWith("z1", 4); + fireEvent.keyDown(group, { key: "ArrowUp" }); + expect(updateZoomDepth).toHaveBeenLastCalledWith("z1", 3); + expect(buttons[2]).toHaveFocus(); + expect(updateZoomDepth).toHaveBeenCalledTimes(4); + expect(buttons[2]).toHaveAttribute("aria-pressed", "true"); + }); + + // Every level is a Tab stop, so focus can sit on a level that is not the selected one. + // Counting from the selection there moved focus the wrong way across the row. + it("steps from the button that has focus, not from the selected level", () => { + const { updateZoomDepth, group, buttons, focusLevel } = renderControlled(2); + focusLevel(5); + fireEvent.keyDown(group, { key: "ArrowRight" }); + expect(buttons[5]).toHaveFocus(); + expect(updateZoomDepth).toHaveBeenCalledWith("z1", 6); + }); + + // `updateZoomDepth` writes the document, so the pressed state only catches up a tick later. + // Focus moves in the keystroke itself, which is why holding an arrow down keeps advancing + // instead of re-applying the same step against a `depth` prop that has not landed yet. + it("keeps stepping while the write is still in flight", () => { + const updateZoomDepth = vi.fn(async (_id: string, _depth: ZoomDepth) => true); + function Harness() { + const [depth, setDepth] = useState(3); + return ( + { + await updateZoomDepth(id, next); + setDepth(next); + return true; + }, + }} + /> + ); + } + render(); + const group = screen.getByRole("group", { name: "zoom.level" }); + const buttons = screen.getAllByRole("button"); + (buttons[2] as HTMLButtonElement).focus(); + fireEvent.keyDown(group, { key: "ArrowRight" }); + fireEvent.keyDown(group, { key: "ArrowRight" }); + expect(updateZoomDepth.mock.calls.map(([, depth]) => depth)).toEqual([4, 5]); + expect(buttons[4]).toHaveFocus(); + }); + + // The mirror image of the test above, and the one that catches a stale read: stepping + // BACK to where the region started. The guard that makes re-pressing the current level a + // no-op has to compare against what was last asked for, not against a `depth` prop that + // still says 3 because the first write has not landed -- or the user's second keystroke + // is dropped and the level stays on 4. + it("does not drop a step back while the first write is still in flight", () => { + const updateZoomDepth = vi.fn(async (_id: string, _depth: ZoomDepth) => true); + function Harness() { + const [depth, setDepth] = useState(3); + return ( + { + await updateZoomDepth(id, next); + setDepth(next); + return true; + }, + }} + /> + ); + } + render(); + const group = screen.getByRole("group", { name: "zoom.level" }); + const buttons = screen.getAllByRole("button"); + (buttons[2] as HTMLButtonElement).focus(); + fireEvent.keyDown(group, { key: "ArrowRight" }); + fireEvent.keyDown(group, { key: "ArrowLeft" }); + expect(updateZoomDepth.mock.calls.map(([, depth]) => depth)).toEqual([4, 3]); + expect(buttons[2]).toHaveFocus(); + }); + + // `saveDocument` writes the returned document into the store and only then + // resolves, so an earlier request can echo back while a later one is still in + // flight. Copying that echo into the no-op guard made ArrowLeft look like a + // re-press of the current level: 3 → 4 → 5, 4 lands, ArrowLeft dropped, and + // the level stayed on 5 with focus on 4. + it("does not treat an earlier in-flight write as the latest request", async () => { + const resolvers: Array<() => void> = []; + const updateZoomDepth = vi.fn((_id: string, _depth: ZoomDepth) => { + return new Promise((resolve) => { + resolvers.push(() => resolve(true)); + }); + }); + function Harness() { + const [depth, setDepth] = useState(3); + return ( + { + const pending = updateZoomDepth(id, next); + void pending.then(() => setDepth(next)); + return pending; + }, + }} + /> + ); + } + render(); + const group = screen.getByRole("group", { name: "zoom.level" }); + const buttons = screen.getAllByRole("button"); + (buttons[2] as HTMLButtonElement).focus(); + fireEvent.keyDown(group, { key: "ArrowRight" }); + fireEvent.keyDown(group, { key: "ArrowRight" }); + expect(updateZoomDepth.mock.calls.map(([, depth]) => depth)).toEqual([4, 5]); + expect(resolvers).toHaveLength(2); + + await act(async () => { + resolvers[0]!(); + }); + expect(buttons[3]).toHaveAttribute("aria-pressed", "true"); + expect(buttons[4]).toHaveFocus(); + + fireEvent.keyDown(group, { key: "ArrowLeft" }); + expect(updateZoomDepth.mock.calls.map(([, depth]) => depth)).toEqual([4, 5, 4]); + expect(buttons[3]).toHaveFocus(); + }); + + // Depth values repeat, so an older request landing on 4 must not look like + // the later request for 4 has confirmed, or the 5 in between is treated as + // an external write and the latest 4 is lost. + it("does not treat an older request for the same depth as the latest one", async () => { + const resolvers: Array<() => void> = []; + const updateZoomDepth = vi.fn((_id: string, _depth: ZoomDepth) => { + return new Promise((resolve) => { + resolvers.push(() => resolve(true)); + }); + }); + function Harness() { + const [depth, setDepth] = useState(3); + return ( + { + const pending = updateZoomDepth(id, next); + void pending.then(() => setDepth(next)); + return pending; + }, + }} + /> + ); + } + render(); + const group = screen.getByRole("group", { name: "zoom.level" }); + const buttons = screen.getAllByRole("button"); + (buttons[2] as HTMLButtonElement).focus(); + fireEvent.keyDown(group, { key: "ArrowRight" }); + fireEvent.keyDown(group, { key: "ArrowRight" }); + fireEvent.keyDown(group, { key: "ArrowLeft" }); + expect(updateZoomDepth.mock.calls.map(([, depth]) => depth)).toEqual([4, 5, 4]); + + await act(async () => { + resolvers[0]!(); + }); + await act(async () => { + resolvers[1]!(); + }); + fireEvent.click(buttons[4] as HTMLButtonElement); + expect(updateZoomDepth.mock.calls.map(([, depth]) => depth)).toEqual([4, 5, 4, 5]); + }); + + it("does not leak a pending request onto a different zoom region", async () => { + const updateZoomDepth = vi.fn(async (_id: string, _depth: ZoomDepth) => true); + const { rerender } = render( + , + ); + fireEvent.click(screen.getAllByRole("button")[4] as HTMLButtonElement); + expect(updateZoomDepth).toHaveBeenCalledWith("A", 5); + + rerender(); + fireEvent.click(screen.getAllByRole("button")[4] as HTMLButtonElement); + expect(updateZoomDepth).toHaveBeenLastCalledWith("B", 5); + expect(updateZoomDepth).toHaveBeenCalledTimes(2); + }); + + it("follows an undo after rapid steps have all settled", async () => { + const updateZoomDepth = vi.fn(async (_id: string, _depth: ZoomDepth) => true); + const { rerender } = render( + , + ); + const buttons = screen.getAllByRole("button"); + fireEvent.click(buttons[3] as HTMLButtonElement); + fireEvent.click(buttons[4] as HTMLButtonElement); + expect(updateZoomDepth.mock.calls.map(([, depth]) => depth)).toEqual([4, 5]); + await act(async () => { + // both generations must drain, or the follow-effect stays blocked + }); + + rerender(); + fireEvent.click(buttons[4] as HTMLButtonElement); + expect(updateZoomDepth).toHaveBeenCalledTimes(3); + expect(updateZoomDepth).toHaveBeenLastCalledWith("z1", 5); + }); + + it("retries the same level after a failed save", async () => { + const updateZoomDepth = vi.fn(async (_id: string, _depth: ZoomDepth) => false); + render(); + const buttons = screen.getAllByRole("button"); + fireEvent.click(buttons[4] as HTMLButtonElement); + await act(async () => { + // settle the failed write so the same target is not stuck as current + }); + fireEvent.click(buttons[4] as HTMLButtonElement); + expect(updateZoomDepth).toHaveBeenCalledTimes(2); + expect(updateZoomDepth).toHaveBeenLastCalledWith("z1", 5); + }); + + it("clamps at the lowest level instead of wrapping", () => { + const { updateZoomDepth, group, buttons, focusLevel } = renderControlled(1); + focusLevel(1); + fireEvent.keyDown(group, { key: "ArrowLeft" }); + expect(updateZoomDepth).not.toHaveBeenCalled(); + expect(buttons[0]).toHaveFocus(); + }); + + it("clamps at the highest level instead of wrapping", () => { + const { updateZoomDepth, group, buttons, focusLevel } = renderControlled(6); + focusLevel(6); + fireEvent.keyDown(group, { key: "ArrowRight" }); + expect(updateZoomDepth).not.toHaveBeenCalled(); + expect(buttons[5]).toHaveFocus(); + }); + + it("keeps its own keys off the window listener, and lets every other key through", () => { + // The editor shell listens on WINDOW, above React's root container: ArrowLeft/ArrowRight + // seek the playhead there and Space is play/pause. Space matters most — the shell + // `preventDefault()`s it, which cancels the button's own activation, so an unstopped + // Space changed no level and started playback instead. + // + // What this pins is the propagation rule, which is where the bug was. jsdom does not + // dispatch a button's native activation for Space at all, so "Space commits the focused + // level" is only provable in a browser, where it was checked against the running editor. + const onWindowKey = vi.fn(); + window.addEventListener("keydown", onWindowKey); + try { + const { group } = renderControlled(3); + for (const key of ["ArrowLeft", "ArrowRight", "ArrowUp", "ArrowDown", " ", "Enter"]) { + fireEvent.keyDown(group, { key }); + } + expect(onWindowKey).not.toHaveBeenCalled(); + + // Keys the group ignores still get there, or the editor shortcuts would be dead. + fireEvent.keyDown(group, { key: "z" }); + fireEvent.keyDown(group, { key: "Tab" }); + expect(onWindowKey).toHaveBeenCalledTimes(2); + } finally { + window.removeEventListener("keydown", onWindowKey); + } + }); + + // `preventDefault()` on the activation keys would cancel the button's own click, which is + // exactly how the shell broke Space in the first place. + it("does not cancel the default action of the activation keys", () => { + const { group } = renderControlled(3); + for (const key of ["Enter", " "]) { + const event = new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true }); + group.dispatchEvent(event); + expect(event.defaultPrevented).toBe(false); + } + }); +}); diff --git a/src/lib/ai-edition/store/documentWriteAudit.test.ts b/src/lib/ai-edition/store/documentWriteAudit.test.ts index 0d054eb83..263c1ab6a 100644 --- a/src/lib/ai-edition/store/documentWriteAudit.test.ts +++ b/src/lib/ai-edition/store/documentWriteAudit.test.ts @@ -285,6 +285,7 @@ const DECLARED: WritePath[] = [ w("src/lib/ai-edition/store/useTimeline.ts", "removeClip", "save", "gesture"), w("src/lib/ai-edition/store/useTimeline.ts", "removeRegion", "save", "gesture"), w("src/lib/ai-edition/store/useTimeline.ts", "removeRegions", "save", "gesture"), + w("src/lib/ai-edition/store/useTimeline.ts", "saveZoomPatch", "save", "gesture"), w("src/lib/ai-edition/store/useTimeline.ts", "setTrimEntries", "save", "gesture"), // The live halves of the two drags. w("src/lib/ai-edition/store/useTimeline.ts", "updateAnnotationLive", "set", "automatic"), @@ -293,12 +294,7 @@ const DECLARED: WritePath[] = [ w("src/lib/ai-edition/store/useTimeline.ts", "updateSpeedSpan", "save", "gesture"), w("src/lib/ai-edition/store/useTimeline.ts", "updateSpeedValue", "save", "gesture"), w("src/lib/ai-edition/store/useTimeline.ts", "updateTrim", "save", "gesture"), - w("src/lib/ai-edition/store/useTimeline.ts", "updateZoomClickImpact", "save", "gesture"), - w("src/lib/ai-edition/store/useTimeline.ts", "updateZoomDepth", "save", "gesture"), w("src/lib/ai-edition/store/useTimeline.ts", "updateZoomFocusLive", "set", "automatic"), - w("src/lib/ai-edition/store/useTimeline.ts", "updateZoomFocusMode", "save", "gesture"), - w("src/lib/ai-edition/store/useTimeline.ts", "updateZoomHideCursor", "save", "gesture"), - w("src/lib/ai-edition/store/useTimeline.ts", "updateZoomRotation", "save", "gesture"), w("src/lib/ai-edition/store/useTimeline.ts", "updateZoomSpan", "save", "gesture"), // Source-dimension backfill for assets a migration left unprobed. On load, for // every project, whether or not the user touches anything. diff --git a/src/lib/ai-edition/store/useTimeline.test.ts b/src/lib/ai-edition/store/useTimeline.test.ts index 797f39eda..5fd041c42 100644 --- a/src/lib/ai-edition/store/useTimeline.test.ts +++ b/src/lib/ai-edition/store/useTimeline.test.ts @@ -1197,6 +1197,304 @@ describe("useTimeline undo history", () => { expect(useProjectStore.getState().document?.timeline.clips).toHaveLength(2); }); + // Holds the first document save until the test releases it; every later one lands at once. + const gateFirstSave = () => { + const gate: { release?: () => void } = {}; + let calls = 0; + bridgeMocks.save.mockImplementation(async (doc: AxcutDocument) => { + calls += 1; + if (calls === 1) { + await new Promise((resolve) => { + gate.release = resolve; + }); + } + return { success: true, document: doc }; + }); + return gate; + }; + + it("lands rapid zoom-level steps in order, one undo step each", async () => { + seed(docWithZoom); + const gate = gateFirstSave(); + const { result } = renderTimeline(); + + const p4 = result.current.updateZoomDepth("zoom_a", 4); + const p5 = result.current.updateZoomDepth("zoom_a", 5); + await waitFor(() => expect(gate.release).toEqual(expect.any(Function))); + await act(async () => { + gate.release?.(); + await Promise.all([p4, p5]); + }); + + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(5); + act(() => { + expect(undo()).toBe(true); + }); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(4); + }); + + it("keeps a pending zoom level when the 3D tilt is changed before it lands", async () => { + seed(docWithZoom); + const gate = gateFirstSave(); + const { result } = renderTimeline(); + + const pDepth = result.current.updateZoomDepth("zoom_a", 4); + const pRotation = result.current.updateZoomRotation("zoom_a", "iso"); + await waitFor(() => expect(gate.release).toEqual(expect.any(Function))); + await act(async () => { + gate.release?.(); + await Promise.all([pDepth, pRotation]); + }); + + expect(useProjectStore.getState().document?.zoomRanges[0]).toMatchObject({ + depth: 4, + rotationPreset: "iso", + }); + }); + + // Rebase compatibility (#694 × current main): `updateZoomClickImpact` joined the zoom pane + // after this PR was authored, as one more one-field whole-document writer. While a level + // write is still pending, a click-impact toggle built from the render's document would + // rebuild the pill from the stale pre-level document — and whichever save landed last won, + // so the pending level could come back off. Click impact must share the zoom chain so both + // values survive. + it("keeps a pending zoom level when click impact toggles before it lands", async () => { + seed(docWithZoom); + const gate = gateFirstSave(); + const { result } = renderTimeline(); + + const pDepth = result.current.updateZoomDepth("zoom_a", 4); + const pImpact = result.current.updateZoomClickImpact("zoom_a", true); + await waitFor(() => expect(gate.release).toEqual(expect.any(Function))); + await act(async () => { + gate.release?.(); + await Promise.all([pDepth, pImpact]); + }); + + expect(useProjectStore.getState().document?.zoomRanges[0]).toMatchObject({ + depth: 4, + clickImpact: true, + }); + }); + + // Rebase-review finding (queued zoom writes vs. document replacement): a zoom write + // queued behind a still-pending one starts AFTER an undo has restored the document, + // and must not apply its stale patch to the replacement. The in-flight write itself + // is dropped by `saveDocument`'s epoch check; the queued one is the hole. + it("drops a queued zoom write that starts after an undo replaces the document", async () => { + seed(docWithZoom); + const { result } = renderTimeline(); + // One settled write so the undo has a recorded state to restore. + await act(async () => { + await result.current.updateZoomDepth("zoom_a", 4); + }); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(4); + + const gate = gateFirstSave(); + const pRotation = result.current.updateZoomRotation("zoom_a", "iso"); + const pCursor = result.current.updateZoomHideCursor("zoom_a", true); + await waitFor(() => expect(gate.release).toEqual(expect.any(Function))); + let undid = false; + act(() => { + undid = undo(); + }); + expect(undid).toBe(true); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(3); + + await act(async () => { + gate.release?.(); + await Promise.allSettled([pRotation, pCursor]); + }); + + // The undo's result stands; neither queued write landed on the restored document. + expect(useProjectStore.getState().document?.zoomRanges[0]).toMatchObject({ depth: 3 }); + expect(useProjectStore.getState().document?.zoomRanges[0]?.rotationPreset).toBeUndefined(); + expect(useProjectStore.getState().document?.zoomRanges[0]?.hideCursor).toBeUndefined(); + }); + + // Same finding through the project-switch path: `loadProject` replaces projectId and + // document without superseding the queue, and project B deliberately contains the same + // region id, so a stale patch must not escape detection by id coincidence. + it("drops queued zoom writes when a project switch replaces the document", async () => { + seed(docWithZoom); + const { result } = renderTimeline(); + const projectB: AxcutDocument = { + ...docWithZoom, + project: { ...docWithZoom.project, id: "proj_b", title: "Project B" }, + zoomRanges: [{ ...docWithZoom.zoomRanges[0]!, depth: 2 }], + }; + + const gate = gateFirstSave(); + const pRotation = result.current.updateZoomRotation("zoom_a", "iso"); + const pCursor = result.current.updateZoomHideCursor("zoom_a", true); + await waitFor(() => expect(gate.release).toEqual(expect.any(Function))); + bridgeMocks.get.mockResolvedValue({ success: true, document: projectB }); + await act(async () => { + await useProjectStore.getState().loadProject("proj_b"); + }); + expect(useProjectStore.getState().projectId).toBe("proj_b"); + + await act(async () => { + gate.release?.(); + await Promise.allSettled([pRotation, pCursor]); + }); + + // Project B's own zoom_a is untouched by the stale project-A queue. + expect(useProjectStore.getState().document?.zoomRanges[0]).toMatchObject({ + id: "zoom_a", + depth: 2, + }); + expect(useProjectStore.getState().document?.zoomRanges[0]?.rotationPreset).toBeUndefined(); + expect(useProjectStore.getState().document?.zoomRanges[0]?.hideCursor).toBeUndefined(); + }); + + // Rebase-review finding (stalled save blocks the zoom queue): while one zoom save's + // answer is unknown (bridge never settles), later zoom-pane writes must not queue + // behind it forever — they are refused until the unknown save settles, then work + // again. The refusal is the safe half: the unknown save may still land, so racing + // it would recreate the stale-document overwrite this chain exists to prevent. + it("refuses zoom writes while a save is unknown and recovers when it settles", async () => { + seed(docWithZoom); + vi.useFakeTimers(); + try { + let hungDoc: AxcutDocument | undefined; + let releaseHungSave: (result: { success: boolean; document: AxcutDocument }) => void; + bridgeMocks.save.mockImplementation((doc: AxcutDocument) => { + hungDoc = doc; + return new Promise((resolve) => { + releaseHungSave = (result) => { + // Settle this one call only; later saves answer immediately. + bridgeMocks.save.mockImplementation(async (next: AxcutDocument) => ({ + success: true, + document: next, + })); + resolve(result); + }; + }); + }); + const { result } = renderTimeline(); + + let depthOk: boolean | undefined; + act(() => { + void result.current.updateZoomDepth("zoom_a", 4).then((ok) => { + depthOk = ok; + }); + }); + // Deadline passes with the bridge still silent: the write's result is unknown, + // reported to the caller as not-taken, and the document is left alone. + await act(async () => { + await vi.advanceTimersByTimeAsync(10_000); + }); + expect(depthOk).toBe(false); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(3); + + // A later zoom write is refused while that save is still unknown. + let rotationOk: boolean | undefined; + await act(async () => { + rotationOk = await result.current.updateZoomRotation("zoom_a", "iso"); + }); + expect(rotationOk).toBe(false); + expect(useProjectStore.getState().document?.zoomRanges[0]?.rotationPreset).toBeUndefined(); + + // The unknown save settles late — it may land — and the refusal clears. + await act(async () => { + releaseHungSave({ success: true, document: hungDoc! }); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + await act(async () => { + await result.current.updateZoomDepth("zoom_a", 5); + }); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(5); + } finally { + vi.useRealTimers(); + } + }); + + // Rebase-review follow-up (unknown save × replacement): the refusal must not outlive + // its reason. Once an undo bumps the epoch, the stuck save can no longer install + // anything (`saveDocument` drops it), so it must stop blocking zoom writes — even if + // the bridge never answers. A project switch does NOT bump the epoch, so there the + // stuck save can still land and the refusal correctly stays. + it("stops refusing zoom writes once a replacement makes the unknown save unable to land", async () => { + seed(docWithZoom); + vi.useFakeTimers(); + try { + let hungDoc: AxcutDocument | undefined; + let releaseHungSave: (result: { success: boolean; document: AxcutDocument }) => void = () => { + // replaced once the hung save registers + }; + let saveCalls = 0; + bridgeMocks.save.mockImplementation((doc: AxcutDocument) => { + saveCalls += 1; + if (saveCalls === 2) { + // The write whose answer never comes; the test never releases it until + // the very end, and then only to prove the epoch guard drops it. + hungDoc = doc; + return new Promise((resolve) => { + releaseHungSave = resolve; + }); + } + return Promise.resolve({ success: true, document: doc }); + }); + const { result } = renderTimeline(); + + // One settled write so the undo has a recorded state to restore. + await act(async () => { + await result.current.updateZoomDepth("zoom_a", 4); + }); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(4); + + let depthOk: boolean | undefined; + act(() => { + void result.current.updateZoomDepth("zoom_a", 5).then((ok) => { + depthOk = ok; + }); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(10_000); + }); + expect(depthOk).toBe(false); + + // The undo replaces the document; the stuck save can no longer install it. + let undid = false; + act(() => { + undid = undo(); + }); + expect(undid).toBe(true); + + // Zoom writes must work again on the restored document. + await act(async () => { + const ok = await result.current.updateZoomDepth("zoom_a", 5); + expect(ok).toBe(true); + }); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(5); + + // When the stuck save finally settles, the epoch guard drops it — the restored + // (and since re-edited) document stands. + await act(async () => { + releaseHungSave({ success: true, document: hungDoc! }); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(5); + } finally { + vi.useRealTimers(); + } + }); + + it("resolves a zoom-level write with whether the save took effect", async () => { + seed(docWithZoom); + const { result } = renderTimeline(); + bridgeMocks.save.mockResolvedValueOnce({ success: false, error: "read-only" }); + + let ok: boolean | undefined; + await act(async () => { + ok = await result.current.updateZoomDepth("zoom_a", 4); + }); + + expect(ok).toBe(false); + expect(useProjectStore.getState().document?.zoomRanges[0]?.depth).toBe(3); + }); + it("leaves no undo step behind a focus drag whose commit failed", async () => { // The drag used to push its pre-drag document from the FIRST `setDocument`. When // the commit then failed, `commitZoomFocus` restored that same document through diff --git a/src/lib/ai-edition/store/useTimeline.ts b/src/lib/ai-edition/store/useTimeline.ts index 72a074f53..37796941c 100644 --- a/src/lib/ai-edition/store/useTimeline.ts +++ b/src/lib/ai-edition/store/useTimeline.ts @@ -43,7 +43,9 @@ import { } from "../timeline/timelineMap"; import { dropTrimPillsByIds, resolveTimelineSpanToTrim } from "../timeline/trim-mapping"; import type { AutoZoomSuggestion } from "../timeline/zoom-suggestions"; -import { useProjectStore, waitForDocumentSaves } from "./projectStore"; +import { saveWithDeadline, useProjectStore, waitForDocumentSaves } from "./projectStore"; +import { currentWriteEpoch } from "./undoStack"; +import { useSequentialTimelineOps } from "./useSequentialTimelineOps"; // 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 @@ -705,21 +707,80 @@ export function useTimeline() { } }, [saveDocument]); + // The zoom pane's own write chain -- see `saveZoomPatch`. Only `enqueue` is used, so + // there is no fallback document to hand it. + const { enqueue: enqueueZoomWrite } = useSequentialTimelineOps({ + fallbackDocument: null, + saveDocument, + }); + + // The zoom pane's one-field writes: level, 3D tilt, focus mode, cursor, click impact. Each + // is a whole-document save, so they share one chain and read the document INSIDE it. The + // level buttons step while the previous save is still out, and 3 -> 4 -> 5 built both saves + // from the render's depth-3 document: the main process does not order them, so the 4 could + // land last, and even in order one Ctrl+Z skipped a level. A neighbouring select changed + // while a level was pending rebuilt from that same document and put 3 back. Resolves + // `saveDocument`'s answer, so the level buttons can retry a failed write. + // + // Each request is bound to the project and write epoch it was asked against — the same + // pair `addAsset` samples: an undo bumps the epoch, a project switch swaps both, and a + // queued patch that only STARTS after such a replacement must not apply to the document + // that replaced its target. A save whose answer is unknown (`saveWithDeadline` timed out + // with the bridge still silent) may still land, so later zoom writes are refused until it + // settles instead of racing it — the same "a queued write racing a stuck one" the + // `waitForDocumentSaves` header calls out. The block is keyed to the save's own epoch: + // once a replacement moves the epoch, that save can no longer install anything + // (`saveDocument` drops it) and must stop blocking; a project switch does not move the + // epoch, so there the stuck save can still land and the block correctly stays. + const unknownZoomSavesRef = useRef>([]); + const saveZoomPatch = useCallback( + (id: string, patch: Partial) => { + const epoch = currentWriteEpoch(); + const projectId = useProjectStore.getState().projectId; + return enqueueZoomWrite(async () => { + if (useProjectStore.getState().projectId !== projectId || currentWriteEpoch() !== epoch) { + return false; + } + if (unknownZoomSavesRef.current.filter((stuck) => stuck === epoch).length > 0) { + return false; + } + const doc = useProjectStore.getState().document; + if (!doc) return false; + const save = saveDocument( + { + ...doc, + zoomRanges: patchPillById(doc.zoomRanges, id, patch) as AxcutDocument["zoomRanges"], + }, + { history: true }, + ); + const outcome = await saveWithDeadline(save); + if (outcome !== "timeout") return outcome === true; + // Unknown, not failed: the write may still land. Report not-taken (the + // buttons retry) and refuse later writes into this same document generation + // until the save settles or the epoch moves past it. + unknownZoomSavesRef.current.push(epoch); + void save + .then( + () => undefined, + () => undefined, + ) + .finally(() => { + unknownZoomSavesRef.current = unknownZoomSavesRef.current.filter( + (stuck) => stuck !== epoch, + ); + }); + return false; + }); + }, + [enqueueZoomWrite, saveDocument], + ); + // Zoom-level control for the region-settings panel (1-6, matches // zoomRegionSchema's depth literal union — 1.0x..3.5x in 0.5x steps per // the `depth/2 + 0.5` label formula used throughout the timeline UI). const updateZoomDepth = useCallback( - async (id: string, depth: 1 | 2 | 3 | 4 | 5 | 6) => { - if (!document) return; - const next: AxcutDocument = { - ...document, - zoomRanges: patchPillById(document.zoomRanges, id, { - depth, - }) as AxcutDocument["zoomRanges"], - }; - await saveDocument(next, { history: true }); - }, - [document, saveDocument], + (id: string, depth: 1 | 2 | 3 | 4 | 5 | 6) => saveZoomPatch(id, { depth }), + [saveZoomPatch], ); // Same story as `focusMode` below: the 3D tilt was implemented end to end — schema @@ -728,17 +789,9 @@ export function useTimeline() { // `undefined` clears the preset back to a flat frame; `migrate.ts` already drops the field // when it is falsy, so absent and "no rotation" are the same state. const updateZoomRotation = useCallback( - async (id: string, rotationPreset: Rotation3DPreset | undefined) => { - if (!document) return; - const next: AxcutDocument = { - ...document, - zoomRanges: patchPillById(document.zoomRanges, id, { - rotationPreset, - }) as AxcutDocument["zoomRanges"], - }; - await saveDocument(next, { history: true }); - }, - [document, saveDocument], + (id: string, rotationPreset: Rotation3DPreset | undefined) => + saveZoomPatch(id, { rotationPreset }), + [saveZoomPatch], ); // Nothing could set `focusMode`: "auto" only ever arrived from the automatic suggestion pass @@ -750,47 +803,24 @@ export function useTimeline() { // Writing "manual" explicitly is safe even though `migrate.ts` only persists "auto": an absent // field MEANS manual, so both forms resolve identically. const updateZoomFocusMode = useCallback( - async (id: string, focusMode: "manual" | "auto") => { - if (!document) return; - const next: AxcutDocument = { - ...document, - zoomRanges: patchPillById(document.zoomRanges, id, { - focusMode, - }) as AxcutDocument["zoomRanges"], - }; - await saveDocument(next, { history: true }); - }, - [document, saveDocument], + (id: string, focusMode: "manual" | "auto") => saveZoomPatch(id, { focusMode }), + [saveZoomPatch], ); const updateZoomHideCursor = useCallback( - async (id: string, hideCursor: boolean | undefined) => { - if (!document) return; - const next: AxcutDocument = { - ...document, - zoomRanges: patchPillById(document.zoomRanges, id, { - hideCursor: hideCursor ? true : undefined, - }) as AxcutDocument["zoomRanges"], - }; - await saveDocument(next, { history: true }); - }, - [document, saveDocument], + (id: string, hideCursor: boolean | undefined) => + saveZoomPatch(id, { hideCursor: hideCursor ? true : undefined }), + [saveZoomPatch], ); // Per-region, like the preset it animates. `undefined` rather than `false` so the document - // keeps omitting the key when the option is off. + // keeps omitting the key when the option is off. Shares `saveZoomPatch` with the pane's + // other one-field writes: a toggle arriving while a level write is still pending must not + // rebuild the pill from the stale pre-level document and drop the level on the floor. const updateZoomClickImpact = useCallback( - async (id: string, clickImpact: boolean) => { - if (!document) return; - const next: AxcutDocument = { - ...document, - zoomRanges: patchPillById(document.zoomRanges, id, { - clickImpact: clickImpact ? true : undefined, - }) as AxcutDocument["zoomRanges"], - }; - await saveDocument(next, { history: true }); - }, - [document, saveDocument], + (id: string, clickImpact: boolean) => + saveZoomPatch(id, { clickImpact: clickImpact ? true : undefined }), + [saveZoomPatch], ); const updateAnnotationSpan = useCallback( diff --git a/tests/e2e/v4-shell.spec.ts b/tests/e2e/v4-shell.spec.ts index 1df64a83d..79c1dd4b4 100644 --- a/tests/e2e/v4-shell.spec.ts +++ b/tests/e2e/v4-shell.spec.ts @@ -14,6 +14,18 @@ const EDITOR_URL = `${BASE_URL}/?windowType=editor`; // 300 MB exactly, so MediaStage's formatSize renders "300 MB". const SIZED_BYTES = 314_572_800; +// Only what a zoom region needs to survive `documentSchema` and reach the timeline. +interface ZoomFixture { + id: string; + startMs: number; + endMs: number; + clipId: string; + sourceStartSec: number; + sourceEndSec: number; + depth: 1 | 2 | 3 | 4 | 5 | 6; + focus: { cx: number; cy: number }; +} + function makeAsset(id: string, label: string, sizeBytes?: number) { return { id, @@ -64,7 +76,7 @@ function makeDoc() { captionRanges: [], }, annotations: [], - zoomRanges: [], + zoomRanges: [] as ZoomFixture[], legacyEditor: null, agent: { pendingQuestions: [], suggestions: [], lastAppliedOperations: [] }, preview: { strategy: "seek" as const, revision: 0 }, @@ -73,6 +85,26 @@ function makeDoc() { }; } +// Same fixture with one zoom region on the only clip, so the inspector's zoom pane — +// and the level row inside it — has something to select. Depth 3 is the editor's default, +// and `ZOOM_DEPTH_SCALES` renders it as the "1.80×" the pill is addressed by below. +function makeZoomDoc(): ReturnType { + const doc = makeDoc(); + doc.zoomRanges = [ + { + id: "zoom_e2e", + startMs: 60_000, + endMs: 180_000, + clipId: "clip_e2e", + sourceStartSec: 60, + sourceEndSec: 180, + depth: 3, + focus: { cx: 0.5, cy: 0.5 }, + }, + ]; + return doc; +} + // Same fixture, split into two clips: FloatingInspector's "Edit clip" button // only renders its picker popover past one clip (clips.length === 1 jumps // straight to onEditClip instead), so testing the popover needs a second clip. @@ -243,6 +275,60 @@ test.describe("v4 editor shell", () => { expect(await storeTimeSec()).toBeGreaterThan(400); }); + // The zoom levels are buttons rather than a `