From ce481fd2c19a44c0187d19836576a54346dedd93 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 18 Apr 2026 17:33:26 +0000 Subject: [PATCH] fix: undo/redo history index divergence when exceeding 50 entries Fixes two bugs identified by Devin Review on PR #1: 1. History index goes out of bounds (50) when entries exceed the 50-entry cap, causing the first undo to be a silent no-op. The shift() removed an entry but the index still incremented past the array bounds. 2. pushHistory uses stale historyIndex closure value inside setHistory's functional updater, causing entries to be silently dropped when multiple pushHistory calls are batched by React. Fix: Combine history entries and index into a single useReducer state so both are always updated atomically. Add 'reset' action for new transaction tab creation. Co-Authored-By: Nakamoto, S --- src/App.tsx | 77 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 27 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index e10384d..e0aa2b7 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback, useRef } from 'react'; +import { useState, useEffect, useCallback, useRef, useReducer } from 'react'; import { addEdge, applyNodeChanges, applyEdgeChanges, type Node, type Edge, type Connection, type NodeChange, type EdgeChange } from '@xyflow/react'; import TitleBar from './components/TitleBar'; import ActivityBar from './components/ActivityBar'; @@ -66,44 +66,68 @@ export default function App() { })); }, [activeTransactionId]); - // Undo/redo - const [history, setHistory] = useState([{ nodes: [], edges: [] }]); - const [historyIndex, setHistoryIndex] = useState(0); + // Undo/redo — combined into single state to prevent index/entries divergence + type HistoryState = { entries: HistoryEntry[]; index: number }; + type HistoryAction = + | { type: 'push'; nodes: Node[]; edges: Edge[] } + | { type: 'undo' } + | { type: 'redo' } + | { type: 'reset' }; + + const historyReducer = useCallback((state: HistoryState, action: HistoryAction): HistoryState => { + switch (action.type) { + case 'push': { + const trimmed = state.entries.slice(0, state.index + 1); + const entry = { nodes: JSON.parse(JSON.stringify(action.nodes)), edges: JSON.parse(JSON.stringify(action.edges)) }; + const next = [...trimmed, entry]; + let newIndex = trimmed.length; + if (next.length > 50) { + next.shift(); + newIndex = next.length - 1; + } + return { entries: next, index: newIndex }; + } + case 'undo': { + if (state.index <= 0) return state; + return { ...state, index: state.index - 1 }; + } + case 'redo': { + if (state.index >= state.entries.length - 1) return state; + return { ...state, index: state.index + 1 }; + } + case 'reset': { + return { entries: [{ nodes: [], edges: [] }], index: 0 }; + } + } + }, []); + + const [historyState, dispatchHistory] = useReducer(historyReducer, { entries: [{ nodes: [], edges: [] }], index: 0 }); + const history = historyState.entries; + const historyIndex = historyState.index; const skipHistoryRef = useRef(false); const pushHistory = useCallback((n: Node[], e: Edge[]) => { if (skipHistoryRef.current) { skipHistoryRef.current = false; return; } - setHistory(prev => { - const trimmed = prev.slice(0, historyIndex + 1); - const entry = { nodes: JSON.parse(JSON.stringify(n)), edges: JSON.parse(JSON.stringify(e)) }; - const next = [...trimmed, entry]; - if (next.length > 50) next.shift(); - return next; - }); - setHistoryIndex(prev => Math.min(prev + 1, 50)); - }, [historyIndex]); + dispatchHistory({ type: 'push', nodes: n, edges: e }); + }, []); const undo = useCallback(() => { - if (historyIndex <= 0) return; - const newIndex = historyIndex - 1; - const entry = history[newIndex]; - if (!entry) return; + const entry = historyState.entries[historyState.index - 1]; + if (!entry || historyState.index <= 0) return; skipHistoryRef.current = true; - setHistoryIndex(newIndex); + dispatchHistory({ type: 'undo' }); setNodes(JSON.parse(JSON.stringify(entry.nodes))); setEdges(JSON.parse(JSON.stringify(entry.edges))); - }, [historyIndex, history, setNodes, setEdges]); + }, [historyState, setNodes, setEdges]); const redo = useCallback(() => { - if (historyIndex >= history.length - 1) return; - const newIndex = historyIndex + 1; - const entry = history[newIndex]; - if (!entry) return; + const entry = historyState.entries[historyState.index + 1]; + if (!entry || historyState.index >= historyState.entries.length - 1) return; skipHistoryRef.current = true; - setHistoryIndex(newIndex); + dispatchHistory({ type: 'redo' }); setNodes(JSON.parse(JSON.stringify(entry.nodes))); setEdges(JSON.parse(JSON.stringify(entry.edges))); - }, [historyIndex, history, setNodes, setEdges]); + }, [historyState, setNodes, setEdges]); // Selected nodes const [selectedNodeIds, setSelectedNodeIds] = useState>(new Set()); @@ -359,8 +383,7 @@ export default function App() { const id = `tx-${Date.now()}`; setTransactionTabs(prev => [...prev, { id, name: `Transaction ${prev.length + 1}`, nodes: [], edges: [] }]); setActiveTransactionId(id); - setHistory([{ nodes: [], edges: [] }]); - setHistoryIndex(0); + dispatchHistory({ type: 'reset' }); addTerminalEntry('info', 'system', 'New transaction tab created'); }, [addTerminalEntry]);