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.
This commit is contained in:
Jinjing
2026-06-24 23:20:20 -07:00
parent 7cde0f0b76
commit b76ffb9c3b
3 changed files with 127 additions and 25 deletions
@@ -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
@@ -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>): 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)
})
})
@@ -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
)
})
)
}