From 114c0d1b69918e4845268a23585dc614e7ccf4e2 Mon Sep 17 00:00:00 2001
From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Date: Mon, 13 Apr 2026 15:23:08 -0700
Subject: [PATCH] feat: preserve scroll, cursor, and undo across tab switches
 (#421)

---
 .../src/components/editor/DiffViewer.tsx      | 51 ++++++++++++++++---
 .../src/components/editor/EditorContent.tsx   |  2 +
 .../src/components/editor/EditorPanel.tsx     | 51 ++++++++++++++++++-
 .../src/components/editor/MarkdownPreview.tsx |  8 ++-
 .../src/components/editor/MonacoEditor.tsx    | 29 +++++++++--
 .../editor/useEditorScrollRestore.ts          | 10 +++-
 src/renderer/src/lib/scroll-cache.test.ts     | 24 ++++++++-
 src/renderer/src/lib/scroll-cache.ts          | 12 +++++
 8 files changed, 171 insertions(+), 16 deletions(-)

diff --git a/src/renderer/src/components/editor/DiffViewer.tsx b/src/renderer/src/components/editor/DiffViewer.tsx
index 93c90840380..094a74c436a 100644
--- a/src/renderer/src/components/editor/DiffViewer.tsx
+++ b/src/renderer/src/components/editor/DiffViewer.tsx
@@ -1,11 +1,14 @@
-import React, { useCallback, useRef } from 'react'
+import React, { useCallback, useLayoutEffect, useRef } from 'react'
 import { DiffEditor, type DiffOnMount } from '@monaco-editor/react'
+import type { editor } from 'monaco-editor'
 import { useAppStore } from '@/store'
+import { diffViewStateCache, setWithLRU } from '@/lib/scroll-cache'
 import '@/lib/monaco-setup'
 import { computeEditorFontSize } from '@/lib/editor-font-zoom'
 import { useContextualCopySetup } from './useContextualCopySetup'
 
 type DiffViewerProps = {
+  modelKey: string
   originalContent: string
   modifiedContent: string
   language: string
@@ -18,6 +21,7 @@ type DiffViewerProps = {
 }
 
 export default function DiffViewer({
+  modelKey,
   originalContent,
   modifiedContent,
   language,
@@ -38,6 +42,8 @@ export default function DiffViewer({
     settings?.theme === 'dark' ||
     (settings?.theme === 'system' && window.matchMedia('(prefers-color-scheme: dark)').matches)
 
+  const diffEditorRef = useRef<editor.IStandaloneDiffEditor | null>(null)
+
   // Keep refs to latest callbacks so the mounted editor always calls current versions
   const onSaveRef = useRef(onSave)
   onSaveRef.current = onSave
@@ -50,13 +56,23 @@ export default function DiffViewer({
   propsRef.current = { relativePath, language, onSave }
 
   const handleMount: DiffOnMount = useCallback(
-    (editor, monaco) => {
-      const originalEditor = editor.getOriginalEditor()
-      const modifiedEditor = editor.getModifiedEditor()
+    (diffEditor, monaco) => {
+      diffEditorRef.current = diffEditor
+
+      const originalEditor = diffEditor.getOriginalEditor()
+      const modifiedEditor = diffEditor.getModifiedEditor()
 
       setupCopy(originalEditor, monaco, filePath, propsRef)
       setupCopy(modifiedEditor, monaco, filePath, propsRef)
 
+      // Why: restoring the full diff view state matches VS Code more closely
+      // than replaying scrollTop alone, and avoids divergent cursor/selection
+      // state between the original and modified panes.
+      const savedViewState = diffViewStateCache.get(modelKey)
+      if (savedViewState) {
+        requestAnimationFrame(() => diffEditor.restoreViewState(savedViewState))
+      }
+
       if (editable) {
         // Cmd/Ctrl+S to save
         modifiedEditor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => {
@@ -70,12 +86,27 @@ export default function DiffViewer({
 
         modifiedEditor.focus()
       } else {
-        editor.focus()
+        diffEditor.focus()
       }
     },
-    [editable, setupCopy, filePath]
+    [editable, setupCopy, modelKey, filePath]
   )
 
+  // Why: VS Code snapshots diff view state on deactivation, not on scroll events.
+  // The useLayoutEffect cleanup fires synchronously before React unmounts the
+  // component on tab switch, which is Orca's equivalent of VS Code's clearInput().
+  useLayoutEffect(() => {
+    return () => {
+      const de = diffEditorRef.current
+      if (de) {
+        const currentViewState = de.saveViewState()
+        if (currentViewState) {
+          setWithLRU(diffViewStateCache, modelKey, currentViewState)
+        }
+      }
+    }
+  }, [modelKey])
+
   return (
     <div className="flex flex-col flex-1 min-h-0">
       <div className="flex-1 min-h-0">
@@ -86,6 +117,14 @@ export default function DiffViewer({
           modified={modifiedContent}
           theme={isDark ? 'vs-dark' : 'vs'}
           onMount={handleMount}
+          // Why: A single file can have multiple live diff tabs at once
+          // (staged, unstaged, branch compare versions). The kept Monaco models
+          // must therefore key off the tab identity, not the raw file path, or
+          // one diff tab can incorrectly reuse another tab's model contents.
+          originalModelPath={`diff:original:${modelKey}`}
+          modifiedModelPath={`diff:modified:${modelKey}`}
+          keepCurrentOriginalModel
+          keepCurrentModifiedModel
           options={{
             readOnly: !editable,
             originalEditable: false,
diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx
index e5d9e2a620d..23b3c79fe91 100644
--- a/src/renderer/src/components/editor/EditorContent.tsx
+++ b/src/renderer/src/components/editor/EditorContent.tsx
@@ -306,6 +306,8 @@ export function EditorContent({
   }
   return (
     <DiffViewer
+      key={activeFile.id}
+      modelKey={activeFile.id}
       originalContent={dc.originalContent}
       modifiedContent={editBuffers[activeFile.id] ?? dc.modifiedContent}
       language={resolvedLanguage}
diff --git a/src/renderer/src/components/editor/EditorPanel.tsx b/src/renderer/src/components/editor/EditorPanel.tsx
index 2fcb5f68b1a..77616d92c74 100644
--- a/src/renderer/src/components/editor/EditorPanel.tsx
+++ b/src/renderer/src/components/editor/EditorPanel.tsx
@@ -3,7 +3,8 @@ save/load/render lifecycle for many modes (edit, diff, conflict review), and
 keeping that UI state together is easier to reason about than scattering it
 across multiple components. Autosave now lives in a smaller headless controller
 so hidden editor UI no longer participates in shutdown. */
-import React, { useCallback, useEffect, useState, Suspense } from 'react'
+import React, { useCallback, useEffect, useRef, useState, Suspense } from 'react'
+import * as monaco from 'monaco-editor'
 import { Columns2, FileText, Rows2 } from 'lucide-react'
 import { useAppStore } from '@/store'
 import { detectLanguage } from '@/lib/language-detect'
@@ -12,6 +13,7 @@ import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/comp
 import type { MarkdownViewMode, OpenFile } from '@/store/slices/editor'
 import MarkdownViewToggle from './MarkdownViewToggle'
 import { EditorContent } from './EditorContent'
+import { scrollTopCache, cursorPositionCache, diffViewStateCache } from '@/lib/scroll-cache'
 import type { GitDiffResult } from '../../../../shared/types'
 import {
   getOpenFilesForExternalFileChange,
@@ -69,9 +71,54 @@ export default function EditorPanel({
     }
   }
 
-  const openFilesRef = React.useRef(openFiles)
+  const openFilesRef = useRef(openFiles)
   openFilesRef.current = openFiles
 
+  // Why: keepCurrentModel / keepCurrent*Model retain Monaco models after unmount
+  // so undo history survives tab switches. When a tab is *closed*, the user has
+  // signalled they're done with the file — dispose the models to reclaim memory
+  // and delete cache entries so a reopened file starts fresh.
+  const prevOpenFilesRef = useRef<Map<string, OpenFile>>(new Map())
+
+  useEffect(() => {
+    const currentFilesById = new Map(openFiles.map((f) => [f.id, f]))
+    for (const [prevId, prevFile] of prevOpenFilesRef.current) {
+      if (!currentFilesById.has(prevId)) {
+        // Dispose only the kept-alive Monaco state that this tab mode owns.
+        // Why: edit and diff tabs use different retained-model keys, while the
+        // conflict-review surface does not create kept Monaco models today. An
+        // explicit switch makes that ownership boundary visible so future mode
+        // additions do not silently fall through without considering cleanup.
+        switch (prevFile.mode) {
+          case 'edit':
+            // Why: the edit model URI is constructed via monaco.Uri.parse(filePath)
+            // to match what @monaco-editor/react creates internally when the `path`
+            // prop is provided. This convention is version-dependent.
+            monaco.editor.getModel(monaco.Uri.parse(prevFile.filePath))?.dispose()
+            scrollTopCache.delete(prevFile.filePath)
+            // Why: markdown edit tabs cycle through three view modes (source, rich,
+            // preview), each caching scroll under a mode-scoped key. All must be
+            // evicted so a reopened file starts fresh regardless of which mode was
+            // last active.
+            scrollTopCache.delete(`${prevFile.filePath}:rich`)
+            scrollTopCache.delete(`${prevFile.filePath}:preview`)
+            cursorPositionCache.delete(prevFile.filePath)
+            break
+          case 'diff':
+            // Why: kept diff models are keyed by tab id, not file path, because the
+            // same file can appear in multiple diff tabs with different contents.
+            monaco.editor.getModel(monaco.Uri.parse(`diff:original:${prevId}`))?.dispose()
+            monaco.editor.getModel(monaco.Uri.parse(`diff:modified:${prevId}`))?.dispose()
+            diffViewStateCache.delete(prevId)
+            break
+          case 'conflict-review':
+            break
+        }
+      }
+    }
+    prevOpenFilesRef.current = currentFilesById
+  }, [openFiles])
+
   // Load file content when active file changes
   useEffect(() => {
     if (!activeFile) {
diff --git a/src/renderer/src/components/editor/MarkdownPreview.tsx b/src/renderer/src/components/editor/MarkdownPreview.tsx
index f0b1dca4554..7a0c212cd71 100644
--- a/src/renderer/src/components/editor/MarkdownPreview.tsx
+++ b/src/renderer/src/components/editor/MarkdownPreview.tsx
@@ -84,8 +84,12 @@ export default function MarkdownPreview({
 
     container.addEventListener('scroll', onScroll, { passive: true })
     return () => {
-      // Snapshot final position synchronously before detach.
-      setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
+      // Why: During React StrictMode double-mount (or rapid mount/unmount before
+      // react-markdown renders content), scrollHeight equals clientHeight and
+      // scrollTop is 0. Saving that would clobber a valid cached position.
+      if (container.scrollHeight > container.clientHeight || container.scrollTop > 0) {
+        setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
+      }
       if (throttleTimer !== null) {
         clearTimeout(throttleTimer)
       }
diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx
index c0a25d25587..8d3867449e8 100644
--- a/src/renderer/src/components/editor/MonacoEditor.tsx
+++ b/src/renderer/src/components/editor/MonacoEditor.tsx
@@ -9,7 +9,7 @@ import {
   DropdownMenuTrigger
 } from '@/components/ui/dropdown-menu'
 import { useAppStore } from '@/store'
-import { scrollTopCache, setWithLRU } from '@/lib/scroll-cache'
+import { scrollTopCache, cursorPositionCache, setWithLRU } from '@/lib/scroll-cache'
 import '@/lib/monaco-setup'
 import { computeEditorFontSize } from '@/lib/editor-font-zoom'
 
@@ -146,6 +146,10 @@ export default function MonacoEditor({
       }
       editorInstance.onDidChangeCursorPosition((e) => {
         setEditorCursorLine(filePath, e.position.lineNumber)
+        setWithLRU(cursorPositionCache, filePath, {
+          lineNumber: e.position.lineNumber,
+          column: e.position.column
+        })
       })
 
       // Why: Writing to the Map at 60fps (every scroll frame) is unnecessary since
@@ -189,15 +193,21 @@ export default function MonacoEditor({
           useAppStore.getState().setPendingEditorReveal(null)
         })
       } else {
+        const savedCursor = cursorPositionCache.get(filePath)
         const savedScrollTop = scrollTopCache.get(filePath)
-        if (savedScrollTop !== undefined) {
+        if (savedScrollTop !== undefined || savedCursor) {
           // Why: Monaco renders synchronously, so a single RAF is sufficient to
           // wait for the layout pass. Unlike react-markdown or Tiptap, there is
           // no async content loading that would require a retry loop.
           // Focus is deferred into the same RAF to avoid a one-frame flash where
           // the editor is focused at scroll position 0 before restoration.
           requestAnimationFrame(() => {
-            editorInstance.setScrollTop(savedScrollTop)
+            if (savedCursor) {
+              editorInstance.setPosition(savedCursor)
+            }
+            if (savedScrollTop !== undefined) {
+              editorInstance.setScrollTop(savedScrollTop)
+            }
             editorInstance.focus()
           })
         } else {
@@ -233,6 +243,13 @@ export default function MonacoEditor({
       const ed = editorRef.current
       if (ed) {
         setWithLRU(scrollTopCache, filePath, ed.getScrollTop())
+        const pos = ed.getPosition()
+        if (pos) {
+          setWithLRU(cursorPositionCache, filePath, {
+            lineNumber: pos.lineNumber,
+            column: pos.column
+          })
+        }
       }
       cancelScheduledReveal()
       clearTransientRevealHighlight()
@@ -316,6 +333,12 @@ export default function MonacoEditor({
           }
         }}
         path={filePath}
+        // Why: keepCurrentModel preserves the Monaco text model so undo/redo
+        // survives tab switches, but @monaco-editor/react's own view-state Map
+        // would become a second state owner. Orca restores cursor/scroll from
+        // its explicit caches so close/reopen semantics stay under app control.
+        saveViewState={false}
+        keepCurrentModel
       />
 
       {toastNode}
diff --git a/src/renderer/src/components/editor/useEditorScrollRestore.ts b/src/renderer/src/components/editor/useEditorScrollRestore.ts
index 1755f96d042..094d380d963 100644
--- a/src/renderer/src/components/editor/useEditorScrollRestore.ts
+++ b/src/renderer/src/components/editor/useEditorScrollRestore.ts
@@ -32,8 +32,14 @@ export function useEditorScrollRestore(
 
     container.addEventListener('scroll', onScroll, { passive: true })
     return () => {
-      // Snapshot final position synchronously before detach.
-      setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
+      // Why: During React StrictMode double-mount (or rapid mount/unmount before
+      // Tiptap renders content), the container has zero scrollable height and
+      // scrollTop is 0. Saving that would clobber a valid cached position from
+      // the previous session. Only save when the container was scrollable
+      // (content was rendered) or the user had scrolled.
+      if (container.scrollHeight > container.clientHeight || container.scrollTop > 0) {
+        setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
+      }
       if (throttleTimer !== null) {
         clearTimeout(throttleTimer)
       }
diff --git a/src/renderer/src/lib/scroll-cache.test.ts b/src/renderer/src/lib/scroll-cache.test.ts
index 9e66024c8df..14d5fb9d4ea 100644
--- a/src/renderer/src/lib/scroll-cache.test.ts
+++ b/src/renderer/src/lib/scroll-cache.test.ts
@@ -1,8 +1,10 @@
 import { describe, expect, it, beforeEach } from 'vitest'
-import { setWithLRU, scrollTopCache } from './scroll-cache'
+import { cursorPositionCache, diffViewStateCache, setWithLRU, scrollTopCache } from './scroll-cache'
 
 beforeEach(() => {
   scrollTopCache.clear()
+  cursorPositionCache.clear()
+  diffViewStateCache.clear()
 })
 
 describe('setWithLRU', () => {
@@ -110,3 +112,23 @@ describe('scrollTopCache', () => {
     expect(scrollTopCache.size).toBe(3)
   })
 })
+
+describe('diffViewStateCache', () => {
+  it('is an empty Map on import', () => {
+    expect(diffViewStateCache).toBeInstanceOf(Map)
+    expect(diffViewStateCache.size).toBe(0)
+  })
+
+  it('works with setWithLRU for diff-tab keys', () => {
+    const diffState = {
+      original: { cursorState: [], viewState: { scrollTop: 10, scrollTopWithoutViewZones: 10, scrollLeft: 0 } },
+      modified: { cursorState: [], viewState: { scrollTop: 20, scrollTopWithoutViewZones: 20, scrollLeft: 0 } },
+      modelState: { unchangedRegions: [] }
+    } as unknown as (typeof diffViewStateCache extends Map<string, infer T> ? T : never)
+
+    setWithLRU(diffViewStateCache, 'diff-tab', diffState)
+
+    expect(diffViewStateCache.get('diff-tab')).toBe(diffState)
+    expect(diffViewStateCache.size).toBe(1)
+  })
+})
diff --git a/src/renderer/src/lib/scroll-cache.ts b/src/renderer/src/lib/scroll-cache.ts
index 8f70ae00047..a00df35ce06 100644
--- a/src/renderer/src/lib/scroll-cache.ts
+++ b/src/renderer/src/lib/scroll-cache.ts
@@ -1,3 +1,5 @@
+import type { editor } from 'monaco-editor'
+
 // Why: 20 entries covers a typical working set of open/recently-viewed files.
 // Eviction only means losing a scroll position (user sees top of file), not a
 // correctness bug, so a conservative cap is fine.
@@ -33,3 +35,13 @@ export function setWithLRU<K, V>(
 // React re-renders (unlike Zustand, which would broadcast state changes on
 // every scroll event even though no component renders from scroll position).
 export const scrollTopCache = new Map<string, number>()
+
+// Why: Same rationale as scrollTopCache — module-scoped avoids Zustand
+// re-renders on every cursor move.
+export const cursorPositionCache = new Map<string, { lineNumber: number; column: number }>()
+
+// Why: Diff editors need more than a numeric scroll offset to restore the same
+// working context. Monaco's diff view state also carries cursor/selection state
+// for both sides plus diff model state, which matches VS Code's restore path
+// more closely than Orca's previous scroll-only cache.
+export const diffViewStateCache = new Map<string, editor.IDiffEditorViewState>()