Skip to content

perf(diff-viewer): virtualize diff body rendering for large diffs#773

Merged
matt2e merged 7 commits into
mainfrom
diffs-take-a-while
Jun 10, 2026
Merged

perf(diff-viewer): virtualize diff body rendering for large diffs#773
matt2e merged 7 commits into
mainfrom
diffs-take-a-while

Conversation

@matt2e

@matt2e matt2e commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Speeds up rendering of large diffs in the diff viewer by virtualizing the diff body — only a window of visible lines is rendered to the DOM at a time, rather than the entire diff.

Changes

  • Window-based virtualization (DiffViewer.svelte): render only a window of diff lines around the viewport instead of the full diff body.
  • Identity-indexed diff lines (diffViewerState.svelte.ts): index diff lines by identity so the rendered window can be tracked and mapped correctly.
  • Scroll → render → measure sequencing: sequence scroll, window render, and measurement so inline comments are positioned correctly against the virtualized window.
  • Timing instrumentation (diff_commands.rs, DiffModal.svelte, BranchCard.svelte): log diff load timing on both frontend and backend, plus click→open and post-render timing, to measure the performance improvements.

🤖 Generated with Claude Code

matt2e and others added 5 commits June 5, 2026 14:02
Add info-level logs along the diff-open path so slow clicks can be
attributed to the right stage. Frontend logs cover modal open,
loadFiles, loadFileDiff (including cache hits), and switchContext with
elapsed millis. Backend fills in the previously-silent remote cache hit
paths and the remote cache-miss collection step in get_diff_files and
get_file_diff.

Signed-off-by: Matt Toohey <contact@matttoohey.com>
The existing diff-open instrumentation brackets the wrong window: it
starts at "[diff] open" (DiffModal init) and stops at computeLineDiff,
leaving two uninstrumented gaps where the perceived latency actually
lives. Close both.

Click→open: markDiffOpenClick() stamps performance.now() at every
diff-open trigger in BranchCard (Diff button, review/commit/worktree
opens). createDiffViewerState logs the delta to "[diff] open" as
clickToOpen, capturing the DiffModal + DiffViewer mount cost.

Render tail: a guarded $effect in DiffModal times from a file's diff
becoming available through to pixels on screen via a double-rAF, so
syntax highlighting, DOM build, and layout/paint after computeLineDiff
are no longer invisible.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Phase 1 of virtualizing the diff body: decouple anchor positioning from
the assumption that every line is in the DOM, so it stays correct once
the body renders only a window of lines.

Stamp every rendered .line with data-line-index={i} (absolute index) in
all four pane {#each} blocks, and add a lineAt(pane, n) helper that
resolves a line by that attribute. Convert the positional NodeList
lookups (querySelectorAll('.line')[n]) in updateToolbarPosition,
decideCommentPosition, updateCommentEditorPosition,
updateLineSelectionToolbar, decideLineCommentPosition,
updateLineCommentEditorPosition, and the range-copy loop to identity
lookups via lineAt.

Replace handleSelectionDragMove's rect hit-test loop with arithmetic
(floor((clientY - paneTop + scrollY) / lineHeight), clamped) so the drag
focus line is derived from scroll math instead of iterating rendered
rows — correct and faster even when rows are unrendered.

No windowing yet: this is a correctness-preserving change against the
current full render. Comment open/jump, range-selection toolbar, and
drag-select behave identically. Typecheck passes.

Signed-off-by: Matt Toohey <contact@matttoohey.com>
Phase 2 of virtualizing the diff body: render only the lines near the
viewport instead of the entire file, building on Phase 1's index-by-
identity refactor.

Add per-pane window deriveds (beforeWindow/afterWindow) computed from the
scroll offset, line height, and viewport height the scroll controller
already owns: firstVisible = floor(scrollY / lineHeight), then a slice
[firstVisible - OVERSCAN, firstVisible + visibleCount + OVERSCAN) clamped
to the line count. computeWindow returns absolute indices, so iterating
beforeWindow.indices keeps every i-consumer (highlighting, boundaries,
class lookups, mouse handlers) unchanged — i is still the absolute line
index. A keyed {#each ... (i)} lets Svelte reuse rows across scrolls.

Position the slice under the existing -scrollY transform with a single
top spacer (.line-spacer) sized windowStart * lineHeight; no bottom
spacer is needed because the scrollbar height comes from the separate
contentHeight derived, not the node count.

To keep the window reactive on resize and font changes (not just scroll),
track lineHeight and viewportHeight per pane as $state, updated wherever
dimensions are already measured. OVERSCAN is 40 rows — generous enough to
cover the comment editor's vertical extent near a viewport edge.

Applied to all four line {#each} blocks: two-pane before/after, deleted-
file before, and new-file after. scrollToRow, markers, and connector
rendering are untouched (they read lengths/offsets, not nodes).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ents

Phase 3 of virtualizing the diff body: make the comment editor and line-
selection toolbar wait for the windowed body to render their anchor rows
before measuring, instead of assuming the rows exist on the next frame.

After Phase 2, scrolling to a comment sets a pane's scroll offset, which
recomputes the window and renders the target rows — but only on a later
frame. focusCommentInViewer's single requestAnimationFrame could fire
before the anchor mounted, so lineAt() returned null and the editor
silently never appeared. The same one-shot defer backed
handleCommentHighlightClick's fallback and handleLineMouseUp's toolbar.

Replace those one-shot rAFs with a bounded mount-then-measure retry:
measureAfterWindowRender ticks a measure callback across up to
WINDOW_RENDER_MAX_FRAMES (5) frames, stopping as soon as it reports
success (anchor resolved and positioned) or there is nothing to position.
A trackRaf callback lets a newer request cancel an in-flight wait, and
both pending frame ids are cancelled on unmount.

positionLineCommentEditor requires both the first and last range rows to
be mounted before running, since decideLineCommentPosition measures
space-above/space-below from both and the editor can render above the
range. This also moves the position decision off the synchronous path in
focusCommentInViewer and handleCommentHighlightClick, where it previously
measured against unmounted rows and fell back to 'below'.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e matt2e requested review from baxen and wesbillman as code owners June 9, 2026 06:53

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 673271b198

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +2281 to +2285
<div
class="line-spacer"
style="height: {afterWindow.start * afterLineHeight}px"
></div>
{#each afterWindow.indices as i (i)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve horizontal width across virtualized lines

When a file has its longest line outside the currently rendered window, measureContentWidth() now only sees the windowed DOM under .lines-wrapper, so contentWidth is set to the max width of the visible slice rather than the whole file. The scroll controller clamps horizontal scrolling to that smaller width, which means users cannot pan far enough to read long offscreen lines until they vertically scroll them into the window. This affects both panes because the body is now rendered from beforeWindow/afterWindow instead of all lines.

Useful? React with 👍 / 👎.

Comment on lines 1886 to 1888
for (let i = selectedLineRange.start; i <= selectedLineRange.end; i++) {
const lineEl = lineElements[i];
const lineEl = lineAt(pane, i);
if (lineEl) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Copy selected lines from data, not mounted rows

With virtualization, a selected range can include rows that are no longer mounted after the user scrolls before copying, so lineAt(pane, i) returns null and those selected lines are silently omitted from the clipboard. Previously every .line existed in the DOM, but after this change copying should read from beforeLines/afterLines (or another non-virtualized source) so offscreen selected lines are still copied.

Useful? React with 👍 / 👎.

matt2e and others added 2 commits June 9, 2026 17:24
The diff-body windowing (b45c465673271b) renders only the rows near
the viewport, but three spots still assumed every row was in the DOM.
Two were real bugs for ranges taller than the window (~OVERSCAN +
viewport); one was dead CSS.

Range copy: handleCopy's selectedLineRange branch read line text from
the DOM via lineAt, so any selected line outside the rendered slice
resolved to null and was silently dropped — a drag-selection spanning
more than the window copied only a partial range. Reconstruct the
copied text from the beforeLines/afterLines source arrays by index
instead (absolute line index maps 1:1 to the source-line array). The
native-selection branch stays DOM-bound, as a browser selection can't
extend to unrendered rows anyway.

Comment editor: positionLineCommentEditor required both the first and
last range rows mounted before measuring, because decideLineCommentPosition
reads space-above from the first row and space-below from the last. For
a range taller than the window the two ends can never co-mount, so it
spun out WINDOW_RENDER_MAX_FRAMES and the editor silently never appeared.
Now both-ends-mounted keeps the full space-based decision (short ranges),
and the tall-range case anchors to whichever end is rendered — callers
scroll to start first, so it lands in the window — passing the resolved
row to updateLineCommentEditorPosition so the anchor isn't re-derived
from the preference against an unmounted row.

CSS: drop the inert flex-shrink: 0 on .line-spacer; its wrapper is
display: block so it never applied, and the inline height already
reserves the unrendered-rows space.

Typecheck passes.

Signed-off-by: Matt Toohey <contact@matttoohey.com>
… window

The diff-body virtualization (b45c46526e6287) renders only the rows
near the viewport, but measureContentWidth still read the widest line
from the DOM via .lines-wrapper's scrollWidth. With most rows unmounted,
that saw only the widest *rendered* line, so a file whose longest line
sat outside the window clamped horizontal scrolling too narrow — long
offscreen lines couldn't be panned into view, and the horizontal
scrollbar thumb was mis-sized. Because updateContentWidths runs on lines
change / resize / style mutation but not on scroll, scrolling the long
line into the window didn't fix it either; the under-clamp persisted
until an unrelated resize or font change.

Measure from the source text instead. measureContentWidth now takes the
pane's source lines, picks the widest by display-column count (tabs
expanded to 8-column stops, iterated by code point), and measures just
that one line in a hidden offscreen probe span appended to the pane so
it inherits the live computed font and white-space: pre. Add the 24px
.line-content horizontal padding and floor at pane.clientWidth to keep
the .lines-wrapper min-width: 100% behavior. One probe node keeps the
virtualization intact while the result reflects the whole file at any
scroll position.

Applied to all four measurement sites (before/after dimension effects
and updateContentWidths) by passing beforeLines/afterLines. Empty panes
still return 0. Typecheck passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e matt2e merged commit 0e32032 into main Jun 10, 2026
6 checks passed
@matt2e matt2e deleted the diffs-take-a-while branch June 10, 2026 04:07
matt2e added a commit that referenced this pull request Jun 10, 2026
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant