Skip to content

fix: build a file's diff editor only when it nears the viewport#93

Merged
omartelo merged 4 commits into
mainfrom
fix/lazy-diff-editors
Jul 25, 2026
Merged

fix: build a file's diff editor only when it nears the viewport#93
omartelo merged 4 commits into
mainfrom
fix/lazy-diff-editors

Conversation

@omartelo

@omartelo omartelo commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Opening Files changed on a wide pull request locked the app (#92). Every file
in a diff panel starts expanded, and an expanded file builds its own CodeMirror
view — so #91's 191 files were 191 EditorViews constructed in a single commit,
each followed by a lazy language chunk re-configuring its document. The existing
LARGE_FILE_LINES guard is evaluated per file and never fired: not one of the
191 was large, the diff was wide.

What changed

  • FileDiff defers each editor to an IntersectionObserver. A card holds a
    height-estimated placeholder until it comes within 600px of the viewport. The
    placeholder has to carry the file's height, or every card would sit in the
    viewport at once and nothing would be deferred. Mount is one-way, so scrolling
    back keeps a view's selection and highlighting; collapsing the file still
    destroys it. This is the shared path, so Pulls' Files changed and the dock's
    Review tab both get it
    — the dock had the same root cause.
  • The Pulls diff stops refetching on window focus. HEAD moving already
    refetches it; replaying a wide diff on every alt-tab back into lich was the
    most surprising half of the bug.

Measured

The frontend suite runs in node with no jsdom, so none of this is reachable by
the gate. Measured by hand instead: #91's real diff (191 files, 10,020 lines)
rendered through the same FileDiff cards, headless Chromium at 1600x1000.

editors at rest main thread to idle
before 191 13,678 ms
after 8 465 ms

Scrolling to the bottom builds 11 editors in total, and every visible card
carries its own. LINE_HEIGHT_PX = 17 is calibrated against the diff theme's
measured 16.8px line box.

Not in this PR

From the issue's list, deliberately skipped — with 465 ms remounts they buy
nothing today:

  • caching the parsed diff by (path, head)
  • budgeting how many files start expanded per panel
  • the backend ceiling at internal/project/pr.go:325 (427 KB was never the
    bottleneck)

Test plan

  • Gate run from node_modules/.bin rather than through pnpm, which
    summarises biome's output and reported a false green here: biome check .
    clean, tsc --noEmit clean, 406 tests, vite build ok. After merging
    main: go test ./... green and gofmt -l . empty.
  • Editor count and time-to-idle measured before/after on refactor(frontend): adopt biome, dedupe the repeated code, group lib/ by domain #91's diff
  • Hand pass in the app: this pull request is reviewed and merged from lich's
    own Files changed tab — the screen the fix lands on.

Closes #92

omartelo and others added 2 commits July 25, 2026 18:03
Files changed on a wide pull request locked the app. Every file starts expanded
and an expanded file builds its own CodeMirror view, so #91's 191 files meant
191 EditorViews constructed in a single commit, each one then followed by a lazy
language chunk re-configuring its document. The existing LARGE_FILE_LINES guard
is per file and never fired: none of the 191 was large, the diff was wide.

FileDiff now defers each editor to an IntersectionObserver — a card holds a
height-estimated placeholder until it comes within 600px of the viewport. Mount
is one-way, so scrolling back keeps a view's selection and highlighting;
collapsing the file still destroys it. Both panels rendering FileDiff get it:
Pulls' Files changed and the dock's Review tab.

The Pulls diff also stops refetching on window focus. HEAD moving already
refetches it, and re-rendering a wide diff on every alt-tab back into lich was
the most surprising half of the freeze.

Measured on #91's real diff (191 files, 10k lines) rendered through the same
FileDiff cards at 1600x1000:

              editors at rest   main thread to idle
  before      191               13,678 ms
  after       8                 465 ms

Scrolling to the bottom builds 11 in total, and every visible card carries its
editor. The placeholder's per-line height is calibrated against the diff theme's
measured 16.8px line box.

Refs #92

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both waits on the Pulls screen are gh round-trips of about a second, and both
filled that second with one muted "Loading…" line — the pull request lookup in
the middle of an otherwise empty screen, the diff at the top-left of an empty
Files changed tab. On a surface that size a lone line reads as a failure, not as
work in progress.

Each now renders the shape of the answer instead: title, actions, status line
and tabs for the pull request; file tree, toolbar and file cards for Files
changed. The tree placeholder follows the panel's own hidden/shown preference,
so nothing jumps when the diff lands.

The skeleton primitive is the shadcn one, with its fill swapped from bg-muted —
which in this palette is the hover fill, invisible in light and near-invisible
in dark once the pulse halves it — to a foreground opacity step that reads in
both themes.

Refs #92

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@omartelo

Copy link
Copy Markdown
Owner Author

Second commit on the branch: the two Loading… lines on this screen are now skeletons.

Both waits are gh round-trips of about a second — the pull request lookup covers the whole screen, the diff covers the Files changed tab — and a single muted line on that much empty space read as a failure rather than as work in progress. Each now holds the shape of what follows: title, actions, status line and tabs for the pull request; file tree, toolbar and file cards for Files changed. The tree placeholder honours the panel's own hidden/shown preference, so nothing jumps when the diff lands.

The primitive is shadcn's skeleton, with one adaptation: stock fills with bg-muted, which in this palette is the same value as the hover fill — invisible in light, near-invisible in dark once the pulse halves it. It fills with a foreground opacity step instead, which reads in both themes. Verified by rendering both skeletons headless in light and dark.

omartelo and others added 2 commits July 25, 2026 18:38
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only CHANGELOG.md conflicted: both sides added a bullet under
[Unreleased] › Fixed. Kept all three, main's context-window fix first.
@omartelo
omartelo merged commit cdc3394 into main Jul 25, 2026
3 checks passed
@omartelo
omartelo deleted the fix/lazy-diff-editors branch July 25, 2026 21:51
@omartelo omartelo mentioned this pull request Jul 25, 2026
6 tasks
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.

Pulls › Files changed freezes the app on a wide PR: one CodeMirror view per file, mounted all at once

1 participant