fix(diff): close large-diff deferral review findings from #17521

Deferral keyed "no line counts" off the untracked area, which both prompted
ordinary untracked binaries and silently auto-loaded every tracked row when a
status pass skipped counting (entry cap hit, numstat failed) — the freeze case
the deferral exists for. Decide from the path instead: rows that render as a
preview or a binary stub stay automatic, everything Monaco would open as text
defers.

Also give all three combined-diff virtualizers one shared row estimate, so the
PR-review viewers stop estimating a deferred/in-flight large row at 88px while
DiffSectionItem renders it at 188px, and drop the dead isLoadOnDemand
parameter that estimate covered.

src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts

@@ -23,25 +23,31 @@ describe('combined diff on-demand loading', () => {
     ).toBe(false)
   })
 
-  it('automatically loads tracked diffs when line counts are unavailable', () => {
-    expect(
-      shouldLoadCombinedDiffOnDemand({ added: undefined, removed: undefined, area: 'unstaged' })
-    ).toBe(false)
+  it('defers uncounted tracked text files, whose size is unknown', () => {
+    // A capped status listing or a failed numstat leaves every tracked row
+    // uncounted; auto-loading them is what froze Monaco before deferral.
+    expect(shouldLoadCombinedDiffOnDemand({ path: 'src/generated/schema.ts' })).toBe(true)
   })
 
   it('defers untracked files whose line counts were skipped as too large', () => {
-    expect(shouldLoadCombinedDiffOnDemand({ area: 'untracked', path: 'data/dump.json' })).toBe(true)
+    expect(shouldLoadCombinedDiffOnDemand({ path: 'data/dump.json' })).toBe(true)
   })
 
-  it('defers uncounted untracked svgs, which render as text rather than a preview', () => {
-    expect(shouldLoadCombinedDiffOnDemand({ area: 'untracked', path: 'assets/map.svg' })).toBe(true)
+  it('defers uncounted svgs, which render as text rather than a preview', () => {
+    expect(shouldLoadCombinedDiffOnDemand({ path: 'assets/map.svg' })).toBe(true)
   })
 
-  it('automatically loads untracked images that report no line counts', () => {
-    expect(shouldLoadCombinedDiffOnDemand({ area: 'untracked', path: 'docs/Shot.PNG' })).toBe(false)
+  it('automatically loads uncounted images, which render as a preview', () => {
+    expect(shouldLoadCombinedDiffOnDemand({ path: 'docs/Shot.PNG' })).toBe(false)
   })
 
-  it('defers untracked diffs when only additions are reported', () => {
+  it('automatically loads uncounted non-image binaries of any size', () => {
+    expect(shouldLoadCombinedDiffOnDemand({ path: 'fixtures/sample.zip' })).toBe(false)
+    expect(shouldLoadCombinedDiffOnDemand({ path: 'fonts/Inter.woff2' })).toBe(false)
+    expect(shouldLoadCombinedDiffOnDemand({ path: 'bun.lockb' })).toBe(false)
+  })
+
+  it('defers diffs when only additions are reported', () => {
     expect(shouldLoadCombinedDiffOnDemand({ added: MAX_AUTOMATIC_DIFF_CHANGED_LINES + 1 })).toBe(
       true
     )
@@ -52,4 +58,14 @@ describe('combined diff on-demand loading', () => {
       true
     )
   })
+
+  it('keeps counted binary-extension rows on the line-count rule', () => {
+    expect(shouldLoadCombinedDiffOnDemand({ added: 3, path: 'fixtures/sample.zip' })).toBe(false)
+    expect(
+      shouldLoadCombinedDiffOnDemand({
+        added: MAX_AUTOMATIC_DIFF_CHANGED_LINES + 1,
+        path: 'fixtures/sample.zip'
+      })
+    ).toBe(true)
+  })
 })

src/renderer/src/components/editor/combined-diff-on-demand-load.ts

@@ -1,36 +1,24 @@
-import type { GitStatusEntry } from '../../../../shared/git-status-types'
-import { IMAGE_FILE_EXTENSIONS } from '../../../../shared/image-file-extensions'
+import { hasBinaryFileExtension } from '../../../../shared/binary-file-extensions'
 
 export const MAX_AUTOMATIC_DIFF_CHANGED_LINES = 10_000
 
-// SVG is excluded: it reads as text, so the diff view renders its source in Monaco
-// rather than an image preview — deferral is exactly what an oversized one needs.
-const PREVIEWED_IMAGE_EXTENSIONS = IMAGE_FILE_EXTENSIONS.filter((ext) => ext !== '.svg')
-
-function isPreviewedImagePath(path: string | undefined): boolean {
-  const lowerPath = path?.toLowerCase()
-  return (
-    lowerPath !== undefined && PREVIEWED_IMAGE_EXTENSIONS.some((ext) => lowerPath.endsWith(ext))
-  )
-}
-
 export function shouldLoadCombinedDiffOnDemand({
   added,
   removed,
-  area,
   path
 }: {
   added?: number
   removed?: number
-  area?: GitStatusEntry['area']
   path?: string
 }): boolean {
   if (added === undefined && removed === undefined) {
-    // Untracked files lose their counts once they exceed the status-scan size cap
-    // (MAX_UNTRACKED_LINE_COUNT_BYTES), so an uncounted untracked file is either
-    // oversized text or binary — both too costly to auto-load. Images stay automatic
-    // because their preview is the point of the row.
-    return area === 'untracked' && !isPreviewedImagePath(path)
+    // A row with no counts is one of: a binary file (git numstat reports '-',
+    // and the untracked scan skips binaries), an untracked file past the scan's
+    // size cap (MAX_UNTRACKED_LINE_COUNT_BYTES), or any file in a status pass
+    // that skipped counting entirely (entry cap hit, numstat failed). Only the
+    // binary case is cheap to render, and only the path can tell us that here,
+    // so defer everything Monaco would open as unbounded text.
+    return !hasBinaryFileExtension(path)
   }
   return (added ?? 0) + (removed ?? 0) > MAX_AUTOMATIC_DIFF_CHANGED_LINES
 }

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

@@ -139,7 +139,6 @@ export function useCombinedDiffViewRestore({
         const loadOnDemand = shouldLoadCombinedDiffOnDemand({
           added: 'added' in entry ? entry.added : undefined,
           removed: 'removed' in entry ? entry.removed : undefined,
-          area: 'area' in entry ? entry.area : undefined,
           path: entry.path
         })
         return {

src/renderer/src/components/editor/combined-diff/scroll-viewport/use-combined-diff-virtualizer.ts

@@ -3,11 +3,7 @@ import type React from 'react'
 import { elementScroll, useVirtualizer, type Virtualizer } from '@tanstack/react-virtual'
 import type { ProgrammaticScrollMarks } from '@/hooks/programmatic-scroll-marks'
 import type { DiffSection } from '../../diff-section-types'
-import {
-  getDiffSectionEstimatedHeight,
-  isIntrinsicHeightImageDiff,
-  usesLargeDiffFallbackHeight
-} from '../../diff-section-layout'
+import { getDiffSectionRowEstimatedHeight } from '../../diff-section-layout'
 
 const COMBINED_DIFF_OVERSCAN = 5
 
@@ -39,19 +35,7 @@ export function useCombinedDiffVirtualizer({
         return 88
       }
 
-      return getDiffSectionEstimatedHeight({
-        collapsed: section.collapsed,
-        measuredContentHeight: sectionHeights[index],
-        originalContent: section.originalContent,
-        modifiedContent: section.modifiedContent,
-        changedLineCount:
-          section.added === undefined && section.removed === undefined
-            ? undefined
-            : (section.added ?? 0) + (section.removed ?? 0),
-        useIntrinsicImageHeight: isIntrinsicHeightImageDiff(section.diffResult),
-        isLargeDiffLimited: usesLargeDiffFallbackHeight(section),
-        lineCounts: section.largeDiffRenderLimit?.lineCounts ?? undefined
-      })
+      return getDiffSectionRowEstimatedHeight(section, sectionHeights[index])
     },
     overscan: COMBINED_DIFF_OVERSCAN,
     initialOffset: () => scrollOffsetRef.current,

src/renderer/src/components/editor/diff-section-layout.test.ts

@@ -3,11 +3,28 @@ import {
   getDiffSectionBodyHeight,
   getLargeDiffFallbackBodyHeight,
   getDiffSectionEstimatedHeight,
+  getDiffSectionRowEstimatedHeight,
   isIntrinsicHeightImageDiff,
   usesLargeDiffFallbackHeight
 } from './diff-section-layout'
+import type { DiffSection } from './diff-section-types'
 import type { GitDiffResult } from '../../../../shared/git-diff-compare-types'
 
+const largeTextSection: DiffSection = {
+  key: 'section',
+  path: 'big.txt',
+  status: 'modified',
+  added: 50_000,
+  removed: 0,
+  originalContent: '',
+  modifiedContent: '',
+  collapsed: false,
+  loading: true,
+  dirty: false,
+  diffResult: null,
+  largeDiffRenderLimit: null
+}
+
 describe('diff section layout', () => {
   it('uses Monaco measured content height for text diffs', () => {
     expect(
@@ -26,18 +43,30 @@ describe('diff section layout', () => {
 
   it('uses the bounded fallback height for an on-demand diff', () => {
     expect(
-      getDiffSectionEstimatedHeight({
-        collapsed: false,
-        measuredContentHeight: undefined,
-        originalContent: '',
-        modifiedContent: '',
-        changedLineCount: 60_000,
-        useIntrinsicImageHeight: false,
-        isLoadOnDemand: true
-      })
+      getDiffSectionRowEstimatedHeight(
+        { ...largeTextSection, loading: false, loadOnDemand: true },
+        undefined
+      )
     ).toBe(188)
   })
 
+  // Why: every viewer's virtualizer must estimate the same height DiffSectionItem
+  // renders for these rows, or each large row drifts ~100px per measure pass.
+  it('estimates deferred and in-flight large rows at the rendered fallback height', () => {
+    expect(getDiffSectionRowEstimatedHeight(largeTextSection, undefined)).toBe(188)
+    expect(getDiffSectionRowEstimatedHeight(largeTextSection, 3_800_000)).toBe(188)
+    expect(
+      getDiffSectionRowEstimatedHeight(
+        { ...largeTextSection, added: undefined, removed: undefined, path: 'vendor/blob.bin' },
+        undefined
+      )
+    ).toBe(88)
+  })
+
+  it('estimates a collapsed row as a header, whatever its size', () => {
+    expect(getDiffSectionRowEstimatedHeight({ ...largeTextSection, collapsed: true }, 500)).toBe(28)
+  })
+
   it('falls back to line-count height before Monaco has mounted', () => {
     expect(
       getDiffSectionBodyHeight({

src/renderer/src/components/editor/diff-section-layout.ts

@@ -39,7 +39,7 @@ export function getLargeDiffFallbackBodyHeight(): number {
 export function usesLargeDiffFallbackHeight(
   section: Pick<
     DiffSection,
-    'added' | 'area' | 'largeDiffRenderLimit' | 'loading' | 'loadOnDemand' | 'path' | 'removed'
+    'added' | 'largeDiffRenderLimit' | 'loading' | 'loadOnDemand' | 'path' | 'removed'
   >
 ): boolean {
   return (
@@ -93,18 +93,16 @@ export function getDiffSectionEstimatedHeight({
   changedLineCount,
   useIntrinsicImageHeight,
   lineCounts,
-  isLargeDiffLimited = false,
-  isLoadOnDemand = false
+  isLargeDiffLimited = false
 }: DiffSectionBodyHeightInput & {
   collapsed: boolean
   isLargeDiffLimited?: boolean
-  isLoadOnDemand?: boolean
 }): number {
   if (collapsed) {
     return DIFF_SECTION_HEADER_HEIGHT
   }
 
-  if (isLargeDiffLimited || isLoadOnDemand) {
+  if (isLargeDiffLimited) {
     return DIFF_SECTION_HEADER_HEIGHT + getLargeDiffFallbackBodyHeight()
   }
 
@@ -120,3 +118,39 @@ export function getDiffSectionEstimatedHeight({
     }) ?? MIN_DIFF_SECTION_BODY_HEIGHT)
   )
 }
+
+/**
+ * Single virtualizer estimate for a diff row, shared by the worktree and
+ * PR-review viewers so no viewer estimates a row the row's own layout metrics
+ * (useDiffSectionLayoutMetrics) would size from the bounded fallback instead.
+ */
+export function getDiffSectionRowEstimatedHeight(
+  section: Pick<
+    DiffSection,
+    | 'added'
+    | 'collapsed'
+    | 'diffResult'
+    | 'largeDiffRenderLimit'
+    | 'loading'
+    | 'loadOnDemand'
+    | 'modifiedContent'
+    | 'originalContent'
+    | 'path'
+    | 'removed'
+  >,
+  measuredContentHeight: number | undefined
+): number {
+  return getDiffSectionEstimatedHeight({
+    collapsed: section.collapsed,
+    measuredContentHeight,
+    originalContent: section.originalContent,
+    modifiedContent: section.modifiedContent,
+    changedLineCount:
+      section.added === undefined && section.removed === undefined
+        ? undefined
+        : (section.added ?? 0) + (section.removed ?? 0),
+    useIntrinsicImageHeight: isIntrinsicHeightImageDiff(section.diffResult),
+    isLargeDiffLimited: usesLargeDiffFallbackHeight(section),
+    lineCounts: section.largeDiffRenderLimit?.lineCounts ?? undefined
+  })
+}

src/renderer/src/components/github-item-dialog/inspect-pull-request/pr-files-combined-diff-viewer.tsx

@@ -4,10 +4,7 @@ import type { editor as monacoEditor } from 'monaco-editor'
 import type { DecoratedDiffComment } from '@/components/diff-comments/decorated-diff-comment'
 import { createCombinedDiffSectionIndexMap } from '../../editor/combined-diff/resolve-changes/combined-diff-section-identity'
 import { handleCombinedDiffFileTreeNavigation } from '../../editor/combined-diff/browse-files/combined-diff-file-tree-navigation'
-import {
-  getDiffSectionEstimatedHeight,
-  isIntrinsicHeightImageDiff
-} from '@/components/editor/diff-section-layout'
+import { getDiffSectionRowEstimatedHeight } from '@/components/editor/diff-section-layout'
 import type { DiffSection } from '@/components/editor/diff-section-types'
 import { getCombinedDiffBranchEntriesInTreeOrder } from '../../editor/combined-diff/browse-files/combined-diff-file-tree-filter'
 import type { CombinedDiffFileTreeEntry } from '../../editor/combined-diff/resolve-changes/combined-diff-section-identity'
@@ -239,19 +236,7 @@ function PRFilesCombinedDiffSections({
       if (!section) {
         return 88
       }
-      return getDiffSectionEstimatedHeight({
-        collapsed: section.collapsed,
-        measuredContentHeight: sectionHeights[index],
-        originalContent: section.originalContent,
-        modifiedContent: section.modifiedContent,
-        changedLineCount:
-          section.added === undefined && section.removed === undefined
-            ? undefined
-            : (section.added ?? 0) + (section.removed ?? 0),
-        useIntrinsicImageHeight: isIntrinsicHeightImageDiff(section.diffResult),
-        isLargeDiffLimited: section.largeDiffRenderLimit?.limited === true,
-        lineCounts: section.largeDiffRenderLimit?.lineCounts ?? undefined
-      })
+      return getDiffSectionRowEstimatedHeight(section, sectionHeights[index])
     },
     overscan: PR_DIFF_OVERSCAN,
     getItemKey: (index) => {

src/renderer/src/components/pull-request-page/files/combined-diff-viewer.tsx

@@ -6,10 +6,7 @@ import { DiffSectionItem } from '@/components/editor/DiffSectionItem'
 import { CombinedDiffFileTree } from '../../editor/combined-diff/browse-files/combined-diff-file-tree'
 import { createCombinedDiffSectionIndexMap } from '../../editor/combined-diff/resolve-changes/combined-diff-section-identity'
 import { handleCombinedDiffFileTreeNavigation } from '../../editor/combined-diff/browse-files/combined-diff-file-tree-navigation'
-import {
-  getDiffSectionEstimatedHeight,
-  isIntrinsicHeightImageDiff
-} from '@/components/editor/diff-section-layout'
+import { getDiffSectionRowEstimatedHeight } from '@/components/editor/diff-section-layout'
 import type { DiffSection } from '@/components/editor/diff-section-types'
 import { getCombinedDiffBranchEntriesInTreeOrder } from '../../editor/combined-diff/browse-files/combined-diff-file-tree-filter'
 import type { CombinedDiffFileTreeEntry } from '../../editor/combined-diff/resolve-changes/combined-diff-section-identity'
@@ -201,19 +198,7 @@ export function PRFilesCombinedDiffViewer({
       if (!section) {
         return 88
       }
-      return getDiffSectionEstimatedHeight({
-        collapsed: section.collapsed,
-        measuredContentHeight: sectionHeights[index],
-        originalContent: section.originalContent,
-        modifiedContent: section.modifiedContent,
-        changedLineCount:
-          section.added === undefined && section.removed === undefined
-            ? undefined
-            : (section.added ?? 0) + (section.removed ?? 0),
-        useIntrinsicImageHeight: isIntrinsicHeightImageDiff(section.diffResult),
-        isLargeDiffLimited: section.largeDiffRenderLimit?.limited === true,
-        lineCounts: section.largeDiffRenderLimit?.lineCounts ?? undefined
-      })
+      return getDiffSectionRowEstimatedHeight(section, sectionHeights[index])
     },
     overscan: PR_DIFF_OVERSCAN,
     getItemKey: (index) => {

src/shared/binary-file-extensions.test.ts

@@ -0,0 +1,26 @@
+import { describe, expect, it } from 'vitest'
+import { hasBinaryFileExtension } from './binary-file-extensions'
+
+describe('hasBinaryFileExtension', () => {
+  it('matches known binary extensions case-insensitively', () => {
+    expect(hasBinaryFileExtension('docs/Shot.PNG')).toBe(true)
+    expect(hasBinaryFileExtension('vendor/lib.tar.gz')).toBe(true)
+    expect(hasBinaryFileExtension('C:\\assets\\theme.woff2')).toBe(true)
+  })
+
+  it('treats svg as text because the diff view renders its source', () => {
+    expect(hasBinaryFileExtension('assets/map.svg')).toBe(false)
+  })
+
+  it('rejects text files, dotfiles, and extensionless paths', () => {
+    expect(hasBinaryFileExtension('src/index.ts')).toBe(false)
+    expect(hasBinaryFileExtension('.gitignore')).toBe(false)
+    expect(hasBinaryFileExtension('scripts/.eslintrc')).toBe(false)
+    expect(hasBinaryFileExtension('Makefile')).toBe(false)
+    expect(hasBinaryFileExtension(undefined)).toBe(false)
+  })
+
+  it('does not match an extension that only appears in a directory name', () => {
+    expect(hasBinaryFileExtension('build.zip/manifest')).toBe(false)
+  })
+})

src/shared/binary-file-extensions.ts

@@ -0,0 +1,90 @@
+import { IMAGE_FILE_EXTENSIONS } from './image-file-extensions'
+
+// SVG is an image format that editors open as source text, so it stays out of
+// the binary set even though it lives in IMAGE_FILE_EXTENSIONS.
+const TEXT_IMAGE_EXTENSIONS = new Set(['.svg'])
+
+const NON_IMAGE_BINARY_EXTENSIONS = [
+  // Archives
+  '.7z',
+  '.bz2',
+  '.gz',
+  '.jar',
+  '.rar',
+  '.tar',
+  '.tgz',
+  '.war',
+  '.xz',
+  '.zip',
+  '.zst',
+  // Audio and video
+  '.aac',
+  '.avi',
+  '.flac',
+  '.m4a',
+  '.mkv',
+  '.mov',
+  '.mp3',
+  '.mp4',
+  '.ogg',
+  '.wav',
+  '.webm',
+  // Documents
+  '.doc',
+  '.docx',
+  '.pdf',
+  '.ppt',
+  '.pptx',
+  '.xls',
+  '.xlsx',
+  // Fonts
+  '.eot',
+  '.otf',
+  '.ttc',
+  '.ttf',
+  '.woff',
+  '.woff2',
+  // Compiled artifacts and datastores
+  '.a',
+  '.bin',
+  '.class',
+  '.dll',
+  '.dylib',
+  '.exe',
+  '.idx',
+  '.lockb',
+  '.node',
+  '.o',
+  '.pack',
+  '.pyc',
+  '.pyd',
+  '.so',
+  '.sqlite',
+  '.sqlite3',
+  '.wasm'
+]
+
+export const BINARY_FILE_EXTENSIONS: readonly string[] = Object.freeze([
+  ...IMAGE_FILE_EXTENSIONS.filter((extension) => !TEXT_IMAGE_EXTENSIONS.has(extension)),
+  ...NON_IMAGE_BINARY_EXTENSIONS
+])
+
+const BINARY_FILE_EXTENSION_SET = new Set(BINARY_FILE_EXTENSIONS)
+
+/**
+ * Extension-only guess at "this file is not text". Content-based detection
+ * lives in `isBinaryBuffer`; use this only where the bytes are unavailable.
+ */
+export function hasBinaryFileExtension(filePath: string | undefined): boolean {
+  if (filePath === undefined) {
+    return false
+  }
+  const lowerPath = filePath.toLowerCase()
+  const dotIndex = lowerPath.lastIndexOf('.')
+  const separatorIndex = Math.max(lowerPath.lastIndexOf('/'), lowerPath.lastIndexOf('\\'))
+  // A leading dot is a dotfile (.gitignore), not an extension.
+  if (dotIndex <= separatorIndex + 1) {
+    return false
+  }
+  return BINARY_FILE_EXTENSION_SET.has(lowerPath.slice(dotIndex))
+}