From b76ffb9c3ba2f1cd9697905a82a1906cf28059fb Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 24 Jun 2026 23:20:20 -0700 Subject: [PATCH] Validate entry metadata when matching cached combined diff sections Extract combined diff cache matching logic to a dedicated module and enhance it to verify status, area, old path, and added/removed counts. This prevents restoring cached diff content when the underlying files have changed or been renamed. --- .../components/editor/CombinedDiffViewer.tsx | 28 +----- .../combined-diff-section-cache-match.test.ts | 87 +++++++++++++++++++ .../combined-diff-section-cache-match.ts | 37 ++++++++ 3 files changed, 127 insertions(+), 25 deletions(-) create mode 100644 src/renderer/src/components/editor/combined-diff-section-cache-match.test.ts create mode 100644 src/renderer/src/components/editor/combined-diff-section-cache-match.ts diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index 380bda6b3bb..61dc1ffec0e 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -55,10 +55,7 @@ import { createCombinedDiffSectionIndexMap, handleCombinedDiffFileTreeNavigation } from './CombinedDiffFileTree' -import { - getCombinedDiffFileTreeSectionKey, - type CombinedDiffFileTreeMode -} from './combined-diff-file-tree-model' +import { getCombinedDiffFileTreeSectionKey } from './combined-diff-file-tree-model' import { ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, type EditorPathMutationTarget @@ -77,6 +74,7 @@ import type { DiffSection } from './diff-section-types' import { getInitialCombinedDiffSectionLoadIndices } from './combined-diff-initial-section-load' import { removeDiffSectionMeasuredHeight } from './diff-section-height-cache' import { createCombinedDiffLoadScheduler } from './combined-diff-load-scheduler' +import { combinedDiffSectionsMatchEntryMetadata } from './combined-diff-section-cache-match' import { beginCombinedDiffScrollbarDrag, type CombinedDiffScrollbarDragCleanup @@ -210,26 +208,6 @@ function getInitialCombinedDiffFileTreeCollapsed( return combinedDiffFileTreeCollapsedPreference ?? combinedDiffFileTreeVisibleByDefault !== true } -function cachedCombinedDiffSectionsMatchEntries({ - entries, - sections, - treeMode -}: { - entries: readonly (GitStatusEntry | GitBranchChangeEntry)[] - sections: readonly DiffSection[] - treeMode: CombinedDiffFileTreeMode -}): boolean { - return ( - sections.length === entries.length && - sections.every((section, index) => { - const entry = entries[index] - return ( - entry !== undefined && section.key === getCombinedDiffFileTreeSectionKey(treeMode, entry) - ) - }) - ) -} - export default function CombinedDiffViewer({ file, viewStateKey @@ -541,7 +519,7 @@ export default function CombinedDiffViewer({ const canRestoreSnapshotSectionsByKey = hasUncommittedEntriesSnapshot && cached !== undefined && - cachedCombinedDiffSectionsMatchEntries({ + combinedDiffSectionsMatchEntryMetadata({ entries, sections: cached.sections, treeMode diff --git a/src/renderer/src/components/editor/combined-diff-section-cache-match.test.ts b/src/renderer/src/components/editor/combined-diff-section-cache-match.test.ts new file mode 100644 index 00000000000..970a2155d00 --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff-section-cache-match.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from 'vitest' +import type { GitBranchChangeEntry, GitStatusEntry } from '../../../../shared/types' +import type { DiffSection } from './diff-section-types' +import { combinedDiffSectionsMatchEntryMetadata } from './combined-diff-section-cache-match' + +function section(overrides: Partial): DiffSection { + return { + key: 'unstaged:src/file.ts', + path: 'src/file.ts', + status: 'modified', + area: 'unstaged', + originalContent: '', + modifiedContent: '', + collapsed: false, + loading: false, + dirty: false, + diffResult: null, + largeDiffRenderLimit: null, + ...overrides + } +} + +describe('combinedDiffSectionsMatchEntryMetadata', () => { + it('matches sections when keys and entry metadata still match', () => { + const entry: GitStatusEntry = { + path: 'src/file.ts', + status: 'modified', + area: 'unstaged', + added: 2, + removed: 1 + } + + expect( + combinedDiffSectionsMatchEntryMetadata({ + entries: [entry], + sections: [section({ added: 2, removed: 1 })], + treeMode: 'uncommitted' + }) + ).toBe(true) + }) + + it('rejects cached sections when a same-path entry has new line counts', () => { + const entry: GitStatusEntry = { + path: 'src/file.ts', + status: 'modified', + area: 'unstaged', + added: 177, + removed: 175 + } + + expect( + combinedDiffSectionsMatchEntryMetadata({ + entries: [entry], + sections: [section({ added: 2, removed: 1 })], + treeMode: 'uncommitted' + }) + ).toBe(false) + }) + + it('keeps branch sections separate when old paths change for the same new path', () => { + const entry: GitBranchChangeEntry = { + path: 'src/new.ts', + oldPath: 'src/old.ts', + status: 'renamed', + added: 4, + removed: 3 + } + + expect( + combinedDiffSectionsMatchEntryMetadata({ + entries: [entry], + sections: [ + section({ + key: 'combined-branch:src/new.ts', + path: 'src/new.ts', + status: 'renamed', + area: undefined, + oldPath: 'src/older.ts', + added: 4, + removed: 3 + }) + ], + treeMode: 'branch' + }) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/components/editor/combined-diff-section-cache-match.ts b/src/renderer/src/components/editor/combined-diff-section-cache-match.ts new file mode 100644 index 00000000000..a53e3a1aa43 --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff-section-cache-match.ts @@ -0,0 +1,37 @@ +import type { GitBranchChangeEntry, GitStatusEntry } from '../../../../shared/types' +import type { CombinedDiffFileTreeMode } from './combined-diff-file-tree-model' +import { getCombinedDiffFileTreeSectionKey } from './combined-diff-file-tree-model' +import type { DiffSection } from './diff-section-types' + +export function combinedDiffSectionsMatchEntryMetadata({ + entries, + sections, + treeMode +}: { + entries: readonly (GitStatusEntry | GitBranchChangeEntry)[] + sections: readonly DiffSection[] + treeMode: CombinedDiffFileTreeMode +}): boolean { + // Why: same-path combined sections can survive status refreshes; metadata + // drift means restoring cached content would replay stale, partial diffs. + return ( + sections.length === entries.length && + sections.every((section, index) => { + const entry = entries[index] + if (!entry) { + return false + } + const entryArea = 'area' in entry ? entry.area : undefined + const entryAdded = 'added' in entry ? entry.added : undefined + const entryRemoved = 'removed' in entry ? entry.removed : undefined + return ( + section.key === getCombinedDiffFileTreeSectionKey(treeMode, entry) && + section.status === entry.status && + section.area === entryArea && + section.oldPath === entry.oldPath && + section.added === entryAdded && + section.removed === entryRemoved + ) + }) + ) +}