diff --git a/src/renderer/src/components/editor/CombinedDiffFileTree.test.ts b/src/renderer/src/components/editor/CombinedDiffFileTree.test.ts index 7a21281213d..f84d8abd823 100644 --- a/src/renderer/src/components/editor/CombinedDiffFileTree.test.ts +++ b/src/renderer/src/components/editor/CombinedDiffFileTree.test.ts @@ -9,12 +9,20 @@ import { COMBINED_DIFF_FILE_TREE_QUERY_MAX_BYTES, getCombinedDiffBranchEntriesInTreeOrder, getFilteredCombinedDiffFileTreeEntries, - isCombinedDiffFileTreeQueryTooLarge + isCombinedDiffFileTreeQueryTooLarge, + isCombinedDiffSectionViewed } from './combined-diff-file-tree-model' import type { GitBranchChangeEntry } from '../../../../shared/git-diff-compare-types' import type { GitStatusEntry } from '../../../../shared/git-status-types' describe('CombinedDiffFileTree navigation mapping', () => { + it('does not count deferred sections as viewed', () => { + expect(isCombinedDiffSectionViewed({ loading: false, loadOnDemand: true })).toBe(false) + expect(isCombinedDiffSectionViewed({ loading: false, loadOnDemand: false })).toBe(true) + expect(isCombinedDiffSectionViewed({ loading: false })).toBe(true) + expect(isCombinedDiffSectionViewed({ loading: true, loadOnDemand: false })).toBe(false) + }) + it('disambiguates uncommitted entries with the same path by area', () => { const staged: GitStatusEntry = { path: 'src/App.tsx', status: 'modified', area: 'staged' } const unstaged: GitStatusEntry = { path: 'src/App.tsx', status: 'modified', area: 'unstaged' } diff --git a/src/renderer/src/components/editor/CombinedDiffFileTree.tsx b/src/renderer/src/components/editor/CombinedDiffFileTree.tsx index 99a53af8795..e9305f03746 100644 --- a/src/renderer/src/components/editor/CombinedDiffFileTree.tsx +++ b/src/renderer/src/components/editor/CombinedDiffFileTree.tsx @@ -28,7 +28,8 @@ export { createCombinedDiffSectionIndexMap, getCombinedDiffFileTreeNavigationIndex, getCombinedDiffFileTreeSectionKey, - handleCombinedDiffFileTreeNavigation + handleCombinedDiffFileTreeNavigation, + isCombinedDiffSectionViewed } from './combined-diff-file-tree-model' const UNCOMMITTED_AREA_ORDER: readonly GitStagingArea[] = ['unstaged', 'staged', 'untracked'] diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index e8cd899c76f..b029dc82ae4 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -52,7 +52,8 @@ import { DiffNotesSendMenu } from './DiffNotesSendMenu' import { CombinedDiffFileTree, createCombinedDiffSectionIndexMap, - handleCombinedDiffFileTreeNavigation + handleCombinedDiffFileTreeNavigation, + isCombinedDiffSectionViewed } from './CombinedDiffFileTree' import { getCombinedDiffFileTreeSectionKey } from './combined-diff-file-tree-model' import { @@ -310,6 +311,13 @@ export default function CombinedDiffViewer({ const deferredLoadRequestsRef = useRef>(new Set()) const sectionsRef = useRef([]) const generationRef = useRef(0) + // Why: status polling can replace the entries array without changing its + // metadata; don't reinitialize and cancel an in-flight explicit diff load. + const initializedEntryStateRef = useRef<{ + viewStateKey: string + entrySignature: string + hasUncommittedEntriesSnapshot: boolean + } | null>(null) // Why: per-section reload token, so a sibling's reload can't discard this section's in-flight load. const sectionLoadTokensRef = useRef>(new Map()) const renderedIndicesRef = useRef>(new Set()) @@ -520,6 +528,19 @@ export default function CombinedDiffViewer({ // Why: tab/worktree switches unmount this viewer; cache by pane key so remount restores sections+scroll before repaint. useLayoutEffect(() => { + const initializedEntryState = initializedEntryStateRef.current + if ( + initializedEntryState?.viewStateKey === viewStateKey && + initializedEntryState.entrySignature === entrySignature && + initializedEntryState.hasUncommittedEntriesSnapshot === hasUncommittedEntriesSnapshot + ) { + return + } + initializedEntryStateRef.current = { + viewStateKey, + entrySignature, + hasUncommittedEntriesSnapshot + } deferredLoadRequestsRef.current.clear() const cached = combinedDiffViewStateCache.get(viewStateKey) const canRestoreSnapshotSectionsByKey = @@ -1131,7 +1152,12 @@ export default function CombinedDiffViewer({ setActiveTreeSectionState({ entrySignature, key: null }) } const viewedSectionKeys = React.useMemo( - () => new Set(sections.filter((section) => !section.loading).map((section) => section.key)), + () => + new Set( + sections + .filter((section) => isCombinedDiffSectionViewed(section)) + .map((section) => section.key) + ), [sections] ) const handleTreeNavigate = useCallback( diff --git a/src/renderer/src/components/editor/combined-diff-file-tree-model.ts b/src/renderer/src/components/editor/combined-diff-file-tree-model.ts index fc3c440fc95..80849fef5ae 100644 --- a/src/renderer/src/components/editor/combined-diff-file-tree-model.ts +++ b/src/renderer/src/components/editor/combined-diff-file-tree-model.ts @@ -1,6 +1,7 @@ import { basename } from '@/lib/path' import type { GitBranchChangeEntry } from '../../../../shared/git-diff-compare-types' import type { GitStatusEntry } from '../../../../shared/git-status-types' +import type { DiffSection } from './diff-section-types' import { isClipboardTextByteLengthOverLimit } from '../../../../shared/clipboard-text' import { buildSourceControlTree, @@ -38,6 +39,12 @@ export function createCombinedDiffSectionIndexMap( return new Map(sections.map((section, index) => [section.key, index])) } +export function isCombinedDiffSectionViewed( + section: Pick +): boolean { + return !section.loading && section.loadOnDemand !== true +} + export function getCombinedDiffFileTreeNavigationIndex({ mode, entry, diff --git a/src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts b/src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts index baddd2a041a..8931f4baea3 100644 --- a/src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts +++ b/src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts @@ -26,4 +26,16 @@ describe('combined diff on-demand loading', () => { it('automatically loads diffs when line counts are unavailable', () => { expect(shouldLoadCombinedDiffOnDemand({ added: undefined, removed: undefined })).toBe(false) }) + + it('defers untracked diffs when only additions are reported', () => { + expect(shouldLoadCombinedDiffOnDemand({ added: MAX_AUTOMATIC_DIFF_CHANGED_LINES + 1 })).toBe( + true + ) + }) + + it('defers diffs when only removals are reported', () => { + expect(shouldLoadCombinedDiffOnDemand({ removed: MAX_AUTOMATIC_DIFF_CHANGED_LINES + 1 })).toBe( + true + ) + }) }) diff --git a/src/renderer/src/components/editor/combined-diff-on-demand-load.ts b/src/renderer/src/components/editor/combined-diff-on-demand-load.ts index 51221106abe..1b1dc9bcb42 100644 --- a/src/renderer/src/components/editor/combined-diff-on-demand-load.ts +++ b/src/renderer/src/components/editor/combined-diff-on-demand-load.ts @@ -7,8 +7,10 @@ export function shouldLoadCombinedDiffOnDemand({ added?: number removed?: number }): boolean { - if (added === undefined || removed === undefined) { + // Untracked text files report additions only; both fields are absent when + // line counts are unavailable (for example, binary or oversized files). + if (added === undefined && removed === undefined) { return false } - return added + removed > MAX_AUTOMATIC_DIFF_CHANGED_LINES + return (added ?? 0) + (removed ?? 0) > MAX_AUTOMATIC_DIFF_CHANGED_LINES } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index ba61e7f8c03..2c1e5af4e06 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -14869,6 +14869,10 @@ "directoryAccessRequestMultiple": "This preview wants to read files in {{folders}}.", "allowDirectories": "Allow {{count}} folders", "editAddressControl": "Edit address" + }, + "LargeDiffLoadPrompt": { + "a0af0198aa": "Large diffs are not rendered by default.", + "f7fa7a40d0": "Load diff" } }, "diff": { diff --git a/tests/e2e/large-diff-freeze-repro.spec.ts b/tests/e2e/large-diff-freeze-repro.spec.ts index 1dd0a12dc5d..025e6177bbd 100644 --- a/tests/e2e/large-diff-freeze-repro.spec.ts +++ b/tests/e2e/large-diff-freeze-repro.spec.ts @@ -87,14 +87,23 @@ test.describe('Large diff freeze repro', () => { if (!store) { throw new Error('window.__store is not available') } - const status = await window.api.git.status({ worktreePath: repoPath }) - store.getState().setGitStatus(wId, status) - const entry = status.entries.find( + let status = await window.api.git.status({ worktreePath: repoPath }) + let entry = status.entries.find( (candidate) => candidate.path === relativePath && candidate.area === 'unstaged' ) + // Why: the app may still be settling the just-added worktree's first status read. + const statusDeadline = performance.now() + 5_000 + while (!entry && performance.now() < statusDeadline) { + await new Promise((resolve) => window.setTimeout(resolve, 100)) + status = await window.api.git.status({ worktreePath: repoPath }) + entry = status.entries.find( + (candidate) => candidate.path === relativePath && candidate.area === 'unstaged' + ) + } if (!entry) { throw new Error(`large diff status entry not found: ${relativePath}`) } + store.getState().setGitStatus(wId, status) store.getState().openAllDiffs(wId, repoPath, undefined, 'unstaged', [entry]) }, { wId: worktreeId, repoPath: fixture.repoPath, relativePath: fixture.relativePath }