fix(desktop): defer large and generated diffs in the Changes pane

Closed
KI
2mo ago
+513-204

Opening a changeset fetched and parsed every file's diff up front, so a branch carrying a lockfile, a compiled locale catalog or any few-thousand-line file froze the renderer before the pane painted.

What changed

Files now render as a placeholder until their diff is asked for:

  • Auto-loads anything at or under 500 changed lines that isn't a generated artifact, when it scrolls into view. CodeView virtualizes with an IntersectionObserver, so this is per-visible-file rather than the whole changeset at once, and the common case looks unchanged.
  • "Load diff" button for larger files and generated artifacts (lockfiles, locales/*/messages.ts, dist/, vendor/, *.min.js, and friends).
  • "Open file" when the content would exceed the 500k-character render budget, since parsing it synchronously is what hangs the pane.
  • Retry per file on a failed fetch, replacing the pane-wide error state.

Because diffs are fetched per file again, on demand, git.getDiffBulk no longer has a caller in the renderer, and its seven invalidation sites are removed. The host-service procedure, its worker task and its scaling test stay as-is: an older desktop build talking to an updated host still calls it. Removing it is a separate call to make once those builds age out.

Notes for review

  • The guards (shouldAutoLoadDiff, isGeneratedDiffFile, isDiffContentTooLarge) live in their own module with no tRPC imports. Importing the hook pulls in the renderer tRPC client, which other test files partially mock, and mock.module is process-global in Bun, so a test importing through the hook passes alone and breaks the suite.
  • The single-line placeholder path is now shared by binary files and deferred diffs, which re-anchors existing review threads onto line 1 in both cases.
  • Thresholds worth arguing with: 500 changed lines, 500k characters.

Verification

  • apps/desktop: bun test → 2957 pass, 0 fail (317 files); bun run typecheck clean; biome check clean.
  • bun run --cwd packages/i18n check → exit 0. ja and zh-CN report 0 missing across all 6887 messages.
  • Mutation-checked the new guard test: breaking shouldAutoLoadDiff fails it.
  • Not yet verified visually in the running app; the four placeholder states (deferred, loading, error, too-large) have no CDP evidence behind them.

https://claude.ai/code/session_01Pkvq2izakEnyAXphgz176E

Summary by cubic

Fixes the Changes pane freezing on branches with a lockfile, compiled locale catalog, or other large generated files by fetching each diff on demand instead of parsing the whole changeset up front. Files now render a placeholder until their diff loads: diffs with 500 or fewer changed lines that aren't generated artifacts auto-load on scroll; larger or generated files get a "Load diff" button, content over the 500k-character render budget gets "Open file", and failed fetches show a per-file Retry instead of a pane-wide error. The diff cache key now relies on react-query's dataUpdatedAt rather than hashing both file bodies, dropping ~228ms of main-thread work when opening a large changeset.

Side effects

  • git.getDiffBulk is no longer called in the renderer, so its invalidations are removed; the host procedure stays in place so older desktop builds talking to an updated host still work.
  • Review threads on placeholder files re-anchor to line 1, matching the existing binary-file behavior.

Summary by CodeRabbit

  • New Features
    • Added on-demand diff loading with placeholders for deferred, loading, error, oversized, and generated-file states.
    • Added Load diff, Retry, and Open file actions where applicable.
  • Bug Fixes
    • Improved diff refresh behavior after discarding changes, saving edits, and refreshing workspace changes.
  • Tests
    • Added coverage for automatic loading rules and oversized or generated-file diffs.
  • Localization
    • Added translations for new diff loading and placeholder messages.