Skip to content

Commit 57ba5b9

Browse files
committed
feat(v7-h03): undo/redo for spec mutations (Cmd/Ctrl+Z)
Closes V7-H03 (cppmega-mlx-cobb): bounded undo/redo for the canvas state (nodes + edges + spec triple) with TopBar buttons + keyboard shortcuts. New hook vbgui/src/hooks/useHistory.ts: - useHistory<T>(capacity=50) → { push, undo, redo, canUndo, canRedo, size, clear }. - Past stack capped at capacity; oldest entry drops on overflow. - Redo stack cleared on push (linear-undo semantics). - undo() returns the snapshot BEFORE the popped one; redo() returns the snapshot popped by undo. App.tsx integration: - History captures (nodes, edges, spec) on every verify-trigger key change (same set of user-meaningful mutations). - suppressNextHistoryPushRef avoids the "applying undo registers as a new mutation" feedback loop (otherwise redo stack would be cleared immediately after undo). - Cmd/Ctrl+Z = undo, Shift+Cmd/Ctrl+Z = redo. Skipped inside text inputs so typing isn't intercepted. TopBar: - top-bar-undo / top-bar-redo buttons render only when callbacks provided (default existing TopBar tests unaffected); disabled when !canUndo/!canRedo. Tests: - vbgui vitest useHistory.test.tsx: 7/7 — initial state, push grows, undo returns prev + enables redo, redo replays, push after undo clears redo, capacity bounds, clear resets. - vbgui vitest full suite: 220/220 passing. - vbgui e2e 78_undo_redo.spec.ts: 1/1 — preset drop → undo reduces canvas brick count → redo restores. - vbgui e2e regression (01_canvas_smoke + 55 + 63): 8/8 passing.
1 parent a5ff146 commit 57ba5b9

5 files changed

Lines changed: 271 additions & 0 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// V7-H03: undo/redo through TopBar buttons + canvas state.
2+
//
3+
// Drop a preset → verify canvas has bricks → click Undo → assert
4+
// fewer bricks (or none) → click Redo → assert bricks back.
5+
6+
import { test, expect } from "@playwright/test";
7+
import { gotoApp, selectPreset } from "../fixtures";
8+
9+
test("V7-H03: Undo after preset drop reduces canvas; Redo restores",
10+
async ({ page }) => {
11+
test.setTimeout(60_000);
12+
await gotoApp(page);
13+
14+
// Pre-state: canvas empty, both buttons disabled.
15+
await expect(page.getByTestId("top-bar-undo")).toBeDisabled();
16+
await expect(page.getByTestId("top-bar-redo")).toBeDisabled();
17+
18+
await selectPreset(page, "llama3_8b");
19+
// After preset drop, canvas has bricks.
20+
const before = await page.locator(
21+
"[data-testid^='brick-node-']").count();
22+
expect(before).toBeGreaterThan(0);
23+
24+
// Undo should now be enabled (we have >=2 history entries:
25+
// initial empty + post-preset).
26+
await expect.poll(async () =>
27+
await page.getByTestId("top-bar-undo").isEnabled(),
28+
{ timeout: 5_000 }).toBe(true);
29+
30+
await page.getByTestId("top-bar-undo").click();
31+
await expect.poll(async () =>
32+
await page.locator("[data-testid^='brick-node-']").count(),
33+
{ timeout: 5_000 }).toBeLessThan(before);
34+
35+
// Redo restores the brick count.
36+
await expect(page.getByTestId("top-bar-redo")).toBeEnabled();
37+
await page.getByTestId("top-bar-redo").click();
38+
await expect.poll(async () =>
39+
await page.locator("[data-testid^='brick-node-']").count(),
40+
{ timeout: 5_000 }).toBe(before);
41+
});

vbgui/src/App.tsx

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
INITIAL_SPEC, specReducer, type SpecState, type TopologyFactory,
2323
} from "@/state/spec";
2424
import { migrate } from "@/state/migrations";
25+
import { useHistory } from "@/hooks/useHistory";
2526
import type { ShardingProposalView } from "@/components/sidebar/ShardingTab";
2627

2728
// PRESETS list is now fetched dynamically from the backend via
@@ -90,6 +91,18 @@ export function App(): JSX.Element {
9091
const [projectName, setProjectName] = useState("untitled");
9192
const [proposals, setProposals] = useState<ShardingProposalView[]>([]);
9293
const [spec, dispatch] = useReducer(specReducer, INITIAL_SPEC);
94+
// V7-H03: bounded undo/redo. Snapshot is the (nodes, edges, spec)
95+
// triple captured on every Verify cycle (cheap proxy for any
96+
// user-meaningful mutation that landed). Undo/redo restore the
97+
// snapshot triple in one shot.
98+
const history = useHistory<{ nodes: Node[]; edges: Edge[];
99+
spec: SpecState }>(50);
100+
const lastHistoryKeyRef = useRef<string>("");
101+
// V7-H03: when undo/redo applies a snapshot back, the resulting
102+
// state-change effect would otherwise push that snapshot AGAIN
103+
// and clear the redo stack. This ref lets the apply-path skip
104+
// the next push.
105+
const suppressNextHistoryPushRef = useRef<boolean>(false);
93106
const [activeTab, setActiveTab] = useState<AppTab>("canvas");
94107
const [runReport, setRunReport] = useState<RunReport | null>(null);
95108
const [runError, setRunError] = useState<string | null>(null);
@@ -211,6 +224,57 @@ export function App(): JSX.Element {
211224
rewriterKey, sideChannelKey, availableSideChannelKey,
212225
scheduleVerify]);
213226

227+
// V7-H03: snapshot the (nodes, edges, spec) triple every time a
228+
// structural key changes — that's the same set of user-meaningful
229+
// mutations the verify debouncer reacts to.
230+
useEffect(() => {
231+
const key = `${nodesKey}|${edgesKey}|${lossKey}|${optimKey}|`
232+
+ `${shardingKey}|${rewriterKey}|${sideChannelKey}`;
233+
if (key === lastHistoryKeyRef.current) return;
234+
lastHistoryKeyRef.current = key;
235+
if (suppressNextHistoryPushRef.current) {
236+
suppressNextHistoryPushRef.current = false;
237+
return;
238+
}
239+
history.push({ nodes, edges, spec });
240+
}, [nodesKey, edgesKey, lossKey, optimKey, shardingKey,
241+
rewriterKey, sideChannelKey, history, nodes, edges, spec]);
242+
243+
const handleUndo = useCallback(() => {
244+
const prev = history.undo();
245+
if (prev) {
246+
suppressNextHistoryPushRef.current = true;
247+
setNodes(prev.nodes);
248+
setEdges(prev.edges);
249+
dispatch({ type: "spec.replace", spec: prev.spec });
250+
}
251+
}, [history]);
252+
const handleRedo = useCallback(() => {
253+
const nxt = history.redo();
254+
if (nxt) {
255+
suppressNextHistoryPushRef.current = true;
256+
setNodes(nxt.nodes);
257+
setEdges(nxt.edges);
258+
dispatch({ type: "spec.replace", spec: nxt.spec });
259+
}
260+
}, [history]);
261+
262+
// V7-H03: Cmd/Ctrl+Z = undo, Shift+Cmd/Ctrl+Z = redo.
263+
useEffect(() => {
264+
function onKey(ev: KeyboardEvent) {
265+
const meta = ev.metaKey || ev.ctrlKey;
266+
if (!meta || ev.key.toLowerCase() !== "z") return;
267+
// Don't intercept inside text inputs.
268+
const tag = (ev.target as HTMLElement | null)?.tagName ?? "";
269+
if (tag === "INPUT" || tag === "TEXTAREA") return;
270+
ev.preventDefault();
271+
if (ev.shiftKey) handleRedo();
272+
else handleUndo();
273+
}
274+
window.addEventListener("keydown", onKey);
275+
return () => window.removeEventListener("keydown", onKey);
276+
}, [handleUndo, handleRedo]);
277+
214278
// ----- Handlers ----------------------------------------------------------
215279

216280
const handleDropBrick = useCallback(
@@ -459,6 +523,10 @@ export function App(): JSX.Element {
459523
trainInFlight={trainInFlight}
460524
trainRunId={trainRunId}
461525
onCancelTrain={handleCancelTrain}
526+
onUndo={handleUndo}
527+
onRedo={handleRedo}
528+
canUndo={history.canUndo}
529+
canRedo={history.canRedo}
462530
onMixedPrecisionChange={(enabled) => dispatch({ type: "optim.set",
463531
optim: { ...spec.optim, mixed_precision: enabled } })}
464532
onFp8EnabledChange={(enabled) => dispatch({ type: "sharding.set",

vbgui/src/components/TopBar.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ export interface TopBarProps {
2323
/** H02: toggle callbacks. */
2424
onMixedPrecisionChange?: (enabled: boolean) => void;
2525
onFp8EnabledChange?: (enabled: boolean) => void;
26+
/** V7-H03 undo/redo controls. Buttons render only when callbacks
27+
* provided so unit tests for default TopBar are unaffected. */
28+
onUndo?: () => void;
29+
onRedo?: () => void;
30+
canUndo?: boolean;
31+
canRedo?: boolean;
2632
/** H03: cancel the currently in-flight Train run. */
2733
trainInFlight?: boolean;
2834
trainRunId?: string | null;
@@ -110,6 +116,18 @@ export function TopBar(p: TopBarProps): JSX.Element {
110116
</label>
111117
)}
112118

119+
{p.onUndo && (
120+
<button data-testid="top-bar-undo"
121+
onClick={p.onUndo}
122+
disabled={!p.canUndo}
123+
title="Undo (Cmd/Ctrl+Z)"></button>
124+
)}
125+
{p.onRedo && (
126+
<button data-testid="top-bar-redo"
127+
onClick={p.onRedo}
128+
disabled={!p.canRedo}
129+
title="Redo (Shift+Cmd/Ctrl+Z)"></button>
130+
)}
113131
{p.onSaveSpec && (
114132
<button data-testid="spec-save" onClick={p.onSaveSpec}>Save</button>
115133
)}

vbgui/src/hooks/useHistory.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// V7-H03: bounded undo/redo stack for spec mutations.
2+
//
3+
// Generic over the snapshot shape. The caller (App) is responsible for
4+
// pushing a snapshot every time a user-visible mutation lands and for
5+
// applying the snapshot returned by undo/redo back into its reducers.
6+
//
7+
// History capacity defaults to 50 entries. Pushing past the cap drops
8+
// the oldest snapshot. Redo stack is cleared on every push (the
9+
// classic linear-undo semantics — once the user branches off, the
10+
// forward history is gone).
11+
12+
import { useCallback, useRef, useState } from "react";
13+
14+
export interface HistoryAPI<T> {
15+
push: (snapshot: T) => void;
16+
undo: () => T | null;
17+
redo: () => T | null;
18+
canUndo: boolean;
19+
canRedo: boolean;
20+
size: number;
21+
clear: () => void;
22+
}
23+
24+
export function useHistory<T>(capacity = 50): HistoryAPI<T> {
25+
const past = useRef<T[]>([]);
26+
const future = useRef<T[]>([]);
27+
// setVersion bumps to force re-renders when canUndo/canRedo change.
28+
const [, setVersion] = useState(0);
29+
30+
const push = useCallback((snapshot: T) => {
31+
past.current.push(snapshot);
32+
if (past.current.length > capacity) past.current.shift();
33+
future.current = [];
34+
setVersion((v) => v + 1);
35+
}, [capacity]);
36+
37+
const undo = useCallback((): T | null => {
38+
const top = past.current.pop();
39+
if (top === undefined) return null;
40+
future.current.push(top);
41+
setVersion((v) => v + 1);
42+
// Caller should reset to the PREVIOUS snapshot — the one before
43+
// the popped state. If we want classic undo (restore previous),
44+
// peek the now-top after pop.
45+
const prev = past.current[past.current.length - 1];
46+
return prev ?? null;
47+
}, []);
48+
49+
const redo = useCallback((): T | null => {
50+
const next = future.current.pop();
51+
if (next === undefined) return null;
52+
past.current.push(next);
53+
setVersion((v) => v + 1);
54+
return next;
55+
}, []);
56+
57+
const clear = useCallback(() => {
58+
past.current = [];
59+
future.current = [];
60+
setVersion((v) => v + 1);
61+
}, []);
62+
63+
return {
64+
push,
65+
undo,
66+
redo,
67+
canUndo: past.current.length > 1, // need at least 2 entries to undo
68+
canRedo: future.current.length > 0,
69+
size: past.current.length,
70+
clear,
71+
};
72+
}

vbgui/tests/useHistory.test.tsx

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { describe, it, expect } from "vitest";
2+
import { renderHook, act } from "@testing-library/react";
3+
import { useHistory } from "@/hooks/useHistory";
4+
5+
describe("V7-H03 useHistory", () => {
6+
it("starts with no undo / no redo", () => {
7+
const { result } = renderHook(() => useHistory<number>());
8+
expect(result.current.canUndo).toBe(false);
9+
expect(result.current.canRedo).toBe(false);
10+
expect(result.current.size).toBe(0);
11+
});
12+
13+
it("push grows the size and enables undo after >=2 entries", () => {
14+
const { result } = renderHook(() => useHistory<number>());
15+
act(() => { result.current.push(1); });
16+
expect(result.current.canUndo).toBe(false);
17+
act(() => { result.current.push(2); });
18+
expect(result.current.canUndo).toBe(true);
19+
expect(result.current.size).toBe(2);
20+
});
21+
22+
it("undo returns the previous snapshot and enables redo", () => {
23+
const { result } = renderHook(() => useHistory<number>());
24+
act(() => { result.current.push(10); });
25+
act(() => { result.current.push(20); });
26+
act(() => { result.current.push(30); });
27+
let prev: number | null = null;
28+
act(() => { prev = result.current.undo(); });
29+
expect(prev).toBe(20);
30+
expect(result.current.canRedo).toBe(true);
31+
});
32+
33+
it("redo replays the snapshot popped by undo", () => {
34+
const { result } = renderHook(() => useHistory<number>());
35+
act(() => { result.current.push(1); });
36+
act(() => { result.current.push(2); });
37+
act(() => { result.current.push(3); });
38+
act(() => { result.current.undo(); }); // pop 3, prev=2
39+
let nxt: number | null = null;
40+
act(() => { nxt = result.current.redo(); });
41+
expect(nxt).toBe(3);
42+
});
43+
44+
it("new push after undo clears redo stack (linear semantics)", () => {
45+
const { result } = renderHook(() => useHistory<number>());
46+
act(() => { result.current.push(1); });
47+
act(() => { result.current.push(2); });
48+
act(() => { result.current.undo(); });
49+
expect(result.current.canRedo).toBe(true);
50+
act(() => { result.current.push(99); });
51+
expect(result.current.canRedo).toBe(false);
52+
});
53+
54+
it("capacity bounds the past stack", () => {
55+
const { result } = renderHook(() => useHistory<number>(3));
56+
act(() => { result.current.push(1); });
57+
act(() => { result.current.push(2); });
58+
act(() => { result.current.push(3); });
59+
act(() => { result.current.push(4); });
60+
expect(result.current.size).toBe(3); // 1 was dropped
61+
});
62+
63+
it("clear resets both stacks", () => {
64+
const { result } = renderHook(() => useHistory<number>());
65+
act(() => { result.current.push(1); });
66+
act(() => { result.current.push(2); });
67+
act(() => { result.current.clear(); });
68+
expect(result.current.canUndo).toBe(false);
69+
expect(result.current.canRedo).toBe(false);
70+
expect(result.current.size).toBe(0);
71+
});
72+
});

0 commit comments

Comments
 (0)