diff --git a/src/renderer/src/components/editor/combined-diff/remember-view/combined-diff-view-cache.ts b/src/renderer/src/components/editor/combined-diff/remember-view/combined-diff-view-cache.ts index e945aa164f8..0e1d20e6510 100644 --- a/src/renderer/src/components/editor/combined-diff/remember-view/combined-diff-view-cache.ts +++ b/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() @@ -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 { diff --git a/src/renderer/src/components/editor/combined-diff/remember-view/combined-diff-view-draft-retention.test.ts b/src/renderer/src/components/editor/combined-diff/remember-view/combined-diff-view-draft-retention.test.ts new file mode 100644 index 00000000000..5d2ecbc0649 --- /dev/null +++ b/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 = {}) { + 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) +}) diff --git a/src/renderer/src/store/slices/removed-worktree-diff-memory.test.ts b/src/renderer/src/store/slices/removed-worktree-diff-memory.test.ts index 7357f8d3a59..2afb46b6eee 100644 --- a/src/renderer/src/store/slices/removed-worktree-diff-memory.test.ts +++ b/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 })