fix(editor): keep unsaved combined diff drafts beyond view history limit (#34791)

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

src/renderer/src/components/editor/combined-diff/remember-view/combined-diff-view-cache.ts

@@ -1,5 +1,4 @@
 import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor'
-import { setWithLRU } from '@/lib/scroll-cache'
 import type { DiffSection } from '../../diff-section-types'
 
 export type CachedCombinedDiffViewState = {
@@ -38,6 +37,8 @@ export type CombinedDiffViewLease = {
   revoked: boolean
 }
 
+// Drafts and mounted views are pinned, so the cache may exceed this while they are held.
+const MAX_CACHED_COMBINED_DIFF_VIEWS = 20
 const MAX_INACTIVE_COMBINED_DIFF_BODY_CHARACTERS = 16_000_000
 
 const bodyCharactersBySections = new WeakMap<DiffSection[], number>()
@@ -53,6 +54,31 @@ function isMountedViewKey(viewKey: string): boolean {
   return false
 }
 
+// Unsaved section edits live only here once their tab unmounts, so evicting them loses user work.
+function canEvictCombinedDiffView(viewKey: string, memory: CombinedDiffViewMemory): boolean {
+  if (memory.view?.sections.some(hasUnsavedEdit)) {
+    return false
+  }
+  return !isMountedViewKey(viewKey)
+}
+
+function evictOldestReloadableViews(): void {
+  for (const [viewKey, memory] of memoryByViewKey) {
+    if (memoryByViewKey.size <= MAX_CACHED_COMBINED_DIFF_VIEWS) {
+      return
+    }
+    if (canEvictCombinedDiffView(viewKey, memory)) {
+      memoryByViewKey.delete(viewKey)
+    }
+  }
+}
+
+function writeViewMemory(viewKey: string, memory: CombinedDiffViewMemory): void {
+  memoryByViewKey.delete(viewKey)
+  memoryByViewKey.set(viewKey, memory)
+  evictOldestReloadableViews()
+}
+
 // Fresh tab IDs inherit only closed predecessors; inactive open tabs are unmounted too and must retain their own views.
 function findInheritableViewKeys(
   viewKey: string,
@@ -99,6 +125,13 @@ export function claimCombinedDiffView(
     memoryByViewKey.delete(viewKey)
   }
   const tag = succession ? { contentKey: succession.contentKey, scopeId: succession.scopeId } : null
+  const lease: CombinedDiffViewLease = {
+    viewKey,
+    ...(tag ? { succession: tag } : {}),
+    worktreeId,
+    revoked: false
+  }
+  mountedLeases.add(lease)
   if (succession && tag && !memoryByViewKey.has(viewKey)) {
     const inheritableKeys = findInheritableViewKeys(viewKey, worktreeId, succession)
     const inherited = memoryByViewKey.get(inheritableKeys.at(-1) ?? '')
@@ -107,22 +140,16 @@ export function claimCombinedDiffView(
       memoryByViewKey.delete(key)
     }
     if (inherited) {
-      setWithLRU(memoryByViewKey, viewKey, { ...inherited, succession: tag })
+      writeViewMemory(viewKey, { ...inherited, succession: tag })
     }
   }
-  const lease: CombinedDiffViewLease = {
-    viewKey,
-    ...(tag ? { succession: tag } : {}),
-    worktreeId,
-    revoked: false
-  }
-  mountedLeases.add(lease)
   return lease
 }
 
 // The unmount snapshot still needs this lease to write after mount tracking releases.
 export function releaseCombinedDiffView(lease: CombinedDiffViewLease): void {
   mountedLeases.delete(lease)
+  evictOldestReloadableViews()
   trimInactiveDiffBodies()
 }
 
@@ -135,7 +162,7 @@ export function rememberCombinedDiffView(
   }
   const memory = memoryByViewKey.get(lease.viewKey)
   const owned = memory?.worktreeId === lease.worktreeId ? memory : { worktreeId: lease.worktreeId }
-  setWithLRU(memoryByViewKey, lease.viewKey, {
+  writeViewMemory(lease.viewKey, {
     ...owned,
     ...patch,
     ...(lease.succession ? { succession: lease.succession } : {})
@@ -143,11 +170,13 @@ export function rememberCombinedDiffView(
   trimInactiveDiffBodies()
 }
 
+function hasUnsavedEdit(section: DiffSection): boolean {
+  return Boolean(section.dirty || section.saveConflict)
+}
+
 // Dirty edits and save conflicts must survive invalidation; loading and on-demand sections hold no body to release.
 function isReloadableSection(section: DiffSection): boolean {
-  return (
-    !section.dirty && !section.saveConflict && !section.loading && section.loadOnDemand !== true
-  )
+  return !hasUnsavedEdit(section) && !section.loading && section.loadOnDemand !== true
 }
 
 function reloadableBodyCharacters(section: DiffSection): number {

src/renderer/src/components/editor/combined-diff/remember-view/combined-diff-view-draft-retention.test.ts

@@ -0,0 +1,74 @@
+import { afterEach, expect, it } from 'vitest'
+import type { DiffSection } from '../../diff-section-types'
+import {
+  claimCombinedDiffView,
+  clearCombinedDiffViewMemory,
+  readCombinedDiffViewMemory,
+  releaseCombinedDiffView,
+  rememberCombinedDiffView
+} from './combined-diff-view-cache'
+
+function rememberView(index: number, section: Partial<DiffSection> = {}) {
+  const lease = claimCombinedDiffView(`view-${index}`, 'workspace')
+  const sections: DiffSection[] = [
+    {
+      key: 'unstaged:file.ts',
+      path: 'file.ts',
+      status: 'modified',
+      area: 'unstaged',
+      originalContent: 'base',
+      modifiedContent: `draft-${index}`,
+      collapsed: false,
+      loading: false,
+      dirty: false,
+      diffResult: null,
+      largeDiffRenderLimit: null,
+      ...section
+    }
+  ]
+  rememberCombinedDiffView(lease, {
+    view: {
+      entrySignature: 'one',
+      gitStatusSignature: '',
+      sections,
+      sectionHeights: {},
+      loadedIndices: [0],
+      sideBySide: false
+    }
+  })
+  return lease
+}
+
+afterEach(clearCombinedDiffViewMemory)
+
+it.each([{ dirty: true }, { saveConflict: true as const }])(
+  'keeps unmounted drafts past the view history limit: %j',
+  (state) => {
+    releaseCombinedDiffView(rememberView(0, state))
+    for (let index = 1; index <= 21; index++) {
+      releaseCombinedDiffView(rememberView(index))
+    }
+    expect(readCombinedDiffViewMemory('view-0')?.view?.sections[0]?.modifiedContent).toBe('draft-0')
+    expect(readCombinedDiffViewMemory('view-1')).toBeUndefined()
+    expect(readCombinedDiffViewMemory('view-21')).toBeDefined()
+  }
+)
+
+it('makes a saved draft evictable again once its view unmounts', () => {
+  releaseCombinedDiffView(rememberView(0, { dirty: true }))
+  for (let index = 1; index <= 20; index++) {
+    releaseCombinedDiffView(rememberView(index, { dirty: true }))
+  }
+  const saved = claimCombinedDiffView('view-0', 'workspace')
+  const view = readCombinedDiffViewMemory('view-0')?.view
+  if (!view) {
+    throw new Error('missing edited view')
+  }
+  rememberCombinedDiffView(saved, {
+    view: { ...view, sections: view.sections.map((section) => ({ ...section, dirty: false })) }
+  })
+  expect(readCombinedDiffViewMemory('view-0')).toBeDefined()
+  releaseCombinedDiffView(saved)
+  expect(readCombinedDiffViewMemory('view-0')).toBeUndefined()
+  expect(readCombinedDiffViewMemory('view-1')?.view?.sections[0]?.dirty).toBe(true)
+})

src/renderer/src/store/slices/removed-worktree-diff-memory.test.ts

@@ -109,11 +109,18 @@ describe('removed workspace diff memory', () => {
     expect(readCombinedDiffViewMemory(key)).toEqual({ worktreeId: removed, scrollTop: 7 })
   })
 
-  it('lets a mounted viewer re-create its entry after LRU eviction', () => {
+  it('protects a mounted viewer from LRU eviction until release', () => {
     const { key, lease } = mountDiff(kept)
-    for (let index = 0; index < 20; index += 1) {
+    const memory = readCombinedDiffViewMemory(key)
+    expect(memory).toBeDefined()
+    const oldestInactiveKey = seedDiff('repo::/workspace/other-0')
+    for (let index = 1; index < 20; index += 1) {
       seedDiff(`repo::/workspace/other-${index}`)
     }
+    expect(readCombinedDiffViewMemory(key)).toBe(memory)
+    expect(readCombinedDiffViewMemory(oldestInactiveKey)).toBeUndefined()
+    releaseCombinedDiffView(lease)
+    seedDiff('repo::/workspace/other-20')
     expect(readCombinedDiffViewMemory(key)).toBeUndefined()
     rememberCombinedDiffView(lease, { scrollTop: 9 })
     expect(readCombinedDiffViewMemory(key)).toEqual({ worktreeId: kept, scrollTop: 9 })