Skip to content

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

Closed

Description

@omartelo

Opening Files changed on a PR with many files locks the whole app. Chromium
raises Page Unresponsive; once it comes back the screen is too heavy to use;
and leaving the tab and returning replays the whole thing, because nothing is
cached.

Found on #91 (191 files) — the first PR reviewed through the feature shipped in
v0.18.0.

Repro

  1. Open a PR with ~150+ changed files (refactor(frontend): adopt biome, dedupe the repeated code, group lib/ by domain #91 will do).
  2. Pulls → Files changed.
  3. The app stops responding. Chromium offers to kill the page.
  4. After it recovers, scrolling and switching sessions are unusable.
  5. Leave the tab, come back → same freeze, from scratch.

What it is not

Measured on #91's real diff (427 KB, 10,020 lines, 191 files), so nobody spends
a day on the wrong end:

gh pr diff (backend) 0.88 s
parseDiff 9 ms
buildFileDoc × 191 2 ms
RPC transport async fetch — the network wait never blocks the main thread

Parsing is not the problem, and neither is the backend. The documented ceiling
in internal/project/pr.go:325 (whole diff buffered as one JSON string) is real
but is not what is biting here.

What it is

1. One CodeMirror EditorView per file, all mounted in a single commit.

PullsFiles renders every file (PullsFiles.tsx:124), each as a FileDiff
that starts expanded, and an expanded FileDiff mounts an editor
(useDiffEditor.ts:32). On #91 that is 191 EditorViews constructed
synchronously
— each with its own DOM tree, gutter and line decorations.

The existing guard does not help. LARGE_FILE_LINES = 500 (FileDiff.tsx:21)
is evaluated per file, so it protects against one giant lockfile and does
nothing about many small files. On #91, 0 of 191 files exceed it — every
single one starts expanded. The failure mode is a wide PR, and nothing in the
panel is watching width.

2. 191 lazy language loads, each re-configuring its editor.

Every view then calls loadLanguage (useDiffEditor.ts:44), which resolves the
file's parser chunk and dispatches appendConfig
(codemirror.ts:84). Each dispatch re-parses that document. So the synchronous
mount storm is followed by an async storm of 191 reconfigurations — which is why
the screen stays unusable after the page recovers, not just during.

3. Nothing is cached, and window focus refetches everything.

usePullRequestDiff holds the parsed files in component state only
(use-pull-request-diff.ts:17). Unmounting the tab drops them, so coming back
refetches, reparses and remounts all 191 editors.

Worse, it subscribes to window focus (use-pull-request-diff.ts:43): every
alt-tab back into lich replays the whole freeze
, whether or not anything
changed. That is fine for the cheap detail lookup next door; it is not fine for
this one.

4. The dock's Review tab has the same root cause.

ReviewPanel renders the same FileDiff with the same defaults, so a working
tree with a few hundred changed files freezes the dock the same way. Any fix
should land in the shared path, not only in Pulls.

Fixes, in payoff order

  1. Mount the editor only when its file is on screen — an IntersectionObserver
    around DiffBody, or windowing the list. Kills causes 1 and 2 together and is
    the only one that scales with PR width rather than pushing the cliff further
    out. The collapsed header is cheap; it is the editor that is not.
  2. Cache the parsed diff keyed by (path, head), so returning to the tab is
    free. createKeyedStore already exists for exactly this shape.
  3. Stop refetching the diff on window focus, or gate it on HEAD having
    actually moved. Cheapest change here and it removes the most surprising part
    of the bug.
  4. Budget what starts expanded per panel, not per file — e.g. expand the
    first N and leave the rest collapsed. A stopgap if (1) is not immediate.
  5. Backend: revisit the pr.go:325 ceiling (cap or stream) once the frontend
    stops being the bottleneck. Not urgent — 427 KB is not what hurts.

The frontend suite runs in node with no jsdom, so none of this is reachable by
the existing gate; whatever lands needs a hand pass in task dev against a wide
PR, plus a measurement of first-paint editor count before and after.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething is broken

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions