perf(review): virtualize Guided Review file cards

[alexanderkreidich]

Summary

This PR fixes severe Guided Review performance degradation on large diffs while preserving the original two-column chapter layout and individual per-file diff cards.

All file cards remain in document order, but only the cards near the outer viewport mount an expensive Pierre CodeView. Pierre then virtualizes the lines inside each mounted file.

The shared viewport coordinator allows at most 8 file viewers to be mounted at once.

Performance results

Measurements were taken in Chrome using a representative large Guided Review workload.

Metric	Before	After
Mounted diff hosts	249–404	4–6 normally, hard limit of 8
Shadow DOM elements	~1.07–1.10 million	~2,400–2,700
Shadow DOM reduction	—	~99.8%
Guide opening rate	3.9–4.3 FPS	~55 FPS
Outer page scrolling	~27 FPS	59.5 FPS
Large-file inner scrolling	Degraded by accumulated viewers	60.1 FPS
Opening long-task time	~47 seconds total	118 ms total
Worst opening long task	~8.4 seconds	66 ms
JavaScript heap	~770 MB	~142 MB

During fast outer scrolling:

• 595 frames were rendered over 10 seconds.
• Average rendering rate was 59.5 FPS.
• The 95th-percentile frame time was 17.2 ms.
• The mounted viewer count remained bounded.
• No visible viewport placeholders remained after scrolling settled.

During inner scrolling of a large file:

• Average rendering rate was 60.1 FPS.
• The 95th-percentile frame time was 17.2 ms.
• No long tasks were recorded.
• The file remained fully navigable through Pierre's virtualized CodeView.

File-content requests are also bounded by the mounted window. The production path initially loads content only for nearby files. React StrictMode produced 8 requests for 4 initial mounts because of its expected development-only double mount, rather than requesting every guide file.

What changed

Outer file-card windowing

Guided Review now uses one shared GuideViewportManager for the entire page.

It:

• registers all lightweight file shells;
• observes them through one IntersectionObserver;
• mounts only nearby file viewers;
• enforces a maximum of 8 mounted CodeView instances;
• uses 1,200 px viewport overscan;
• delays window reconciliation during fast scrolling to avoid mount churn;
• slightly favors already-mounted viewers to provide hysteresis;
• force-mounts navigation targets before scrolling to them;
• pins the focused file inside the bounded window so an open annotation composer and its draft cannot be evicted;
• notifies only file cards whose mounted state changed.

This avoids creating one observer or one expensive viewer per guide reference.

Two-level virtualization

Each admitted file card renders a one-file AllFilesCodeView.

This provides two levels of virtualization:

1. The outer manager limits how many file viewers exist.
2. Pierre virtualizes the lines inside each mounted viewer.

No custom line pagination or Shadow DOM manipulation was introduced.

Persistent lightweight shells

Every resolved guide file keeps a lightweight shell in document flow, preserving:

• the original chapter layout;
• individual file cards;
• natural per-file descriptions;
• stable page height;
• predictable outer scroll position.

When a viewer leaves the mount window, its shell keeps the measured card height and shows a lightweight placeholder.

State preservation

File-card state survives viewer eviction and remounting:

• inner CodeView scroll position, restored once per viewer mount;
• collapsed/expanded state, without remounting the live CodeView on each toggle;
• viewed state;
• focused file ownership;
• tokenized navigation targets.

The focused card remains pinned inside the 8-viewer budget, preserving an open annotation composer and its in-progress draft while the outer page scrolls. Navigation from file chips, search results, AI citations, and annotations force-mounts and activates the destination before invoking Pierre navigation.

Preserved review behavior

The virtualized cards continue to support:

• line and file annotations;
• annotation navigation;
• Ask AI, AI history, and inline AI question markers;
• search matches, including offscreen result activation;
• viewed state;
• staging;
• collapse/expand;
• split and unified diff modes;
• code navigation;
• stale guide references;
• duplicate-reference first-placement behavior;
• the generated “Everything else” chapter.

Behavioral details

The virtualized guide intentionally keeps the branch's established placement rules explicit:

• duplicate file references render at their first resolved placement; later chapter chips navigate back to that card;
• stale references remain visible as outdated chapter chips rather than dashed inline diff rows;
• files in unplacedFiles render in a full “Everything else” chapter card;
• evicted diff bodies are absent from browser find-in-page until their viewer mounts again, while the lightweight shell and file path remain in the document.

Validation

Automated checks on ef75cc4:

• Review-editor DOM suite — 174 tests passed, 0 failures, 420 assertions
• bun run typecheck — passed
• bun run --cwd apps/review build — passed
• bun run build:hook — passed
• built CSS contains overscroll-behavior: auto for guide-card scroll chaining
• git diff --check — clean

Manual verification:

• Opened a persisted large Guided Review.
• Confirmed the original two-column chapter layout.
• Confirmed natural descriptions above independent file cards.
• Confirmed no more than 8 mounted file viewers.
• Tested fast outer scrolling.
• Tested inner scrolling of a large file.
• Tested navigation to an offscreen file.
• Confirmed the target mounts before scrolling.
• Confirmed no visible placeholder remains after scrolling settles.

Trade-off

During extremely fast outer scrolling, a distant file card may briefly display its height-preserving placeholder while the shared viewport window settles. The placeholder is replaced with the real virtualized viewer after scrolling becomes idle. Because evicted diff bodies are not present in the DOM, browser find-in-page covers mounted viewers plus every shell's file path, not all offscreen code at once.

This keeps DOM size and mount work bounded regardless of the total number of guide references.

[backnotprop]

Reviewed in depth (typecheck clean, full suite 2585 pass, review-editor DOM tests 166 pass). The core is solid: reviewed-state persistence, saved-guide resume, and progress counts are all owned above the virtualized layer and are untouched, and the AllFilesCodeView / ReviewStateContext changes are inert for non-guide surfaces. Verdict: merge after small fixes. Five things to address:

1. Open annotation composer is destroyed by eviction (GuideFileCard.tsx:59-62, GuideViewportManager.tsx:141-157). useAnnotationToolbar saves the draft on unmount but only restores on an isFocused false-to-true transition, and AllFilesCodeView hard-codes isFocused={true}, so a remounted card never restores. The toolbar is portaled to document.body, so it stays visible while the card behind it unmounts. Repro: select lines, start typing a comment, scroll ~1500px away, keep typing: toolbar and text vanish. Suggested fix: pin the focus-arbiter's file in the mounted set while it holds focus, or veto eviction while a draft/toolbar is open (the current FORCE_MOUNT_MS 1.5s window is not enough).

2. files={[file]} inline array defeats AllFilesCodeView's memos (GuideFileCard.tsx:96). Fresh array identity per render invalidates visualOrder/patchHashes/identity, so every mounted card re-runs hashString plus a full getSingularPatch reparse on every guide re-render, and the guide re-renders on every onPointerEnter and every ReviewState change. One line: const fileList = useMemo(() => [file], [file]).

3. Collapse and mark-viewed fully remount the CodeView (GuideFileCard.tsx:135). defaultCollapsed feeds fileSetKey, which is CodeView's React key, so toggling collapse flips the key and forces unmount/remount plus reparse and re-highlight, with a visible flash on two of the most common guide actions. Collapse state should survive eviction without routing every local toggle through the remount key.

4. initialScrollPosition is a live prop, not a mount seed (GuideFileCard.tsx:136, AllFilesCodeView.tsx:1707-1714). The restore effect depends on the prop value, so it can re-fire on any parent re-render (agent SSE, AI streaming) and snap an in-flight scroll to a stale offset. Make it one-shot per mount (hasRestoredRef) or pass a ref.

5. Inline AI markers are gone from guide diffs. The old GuideDiffSection passed aiMessages/onClickAIMarker to DiffViewer; AllFilesCodeView has no equivalent, so per-line AI markers no longer render in guides. If that is an accepted tradeoff, say so in the PR body (which currently claims AI history parity); otherwise it needs a wiring path.

Test note: both guide test files mock AllFilesCodeView, and happy-dom's IntersectionObserver never delivers entries, so the entire evict/remount band (reconcile, hysteresis, eviction, and findings 1-4) has zero coverage and the max-mounted assertions pass trivially at the eager-seed count. A small fake IntersectionObserver in packages/ui/test-setup/happy-dom.ts with a manual trigger would let you add one regression test each for reviewed-state, collapse-state, and draft survival across a forced evict/remount.

Smaller items, take or leave: chapter badges count the Everything else card (01/07 vs the header's 6 sections); GuideSectionCard cardRef is dead code; collapsed shell is 49px vs a 33px placeholder header; onPointerEnter runs getBoundingClientRect per mounted card inside the pointer handler; the aria-hidden placeholders widen the find-in-page gap to whole files (worth a line in the PR body); the fileSummaries assertion in GuideSectionCard.test.tsx checks a prop that no longer exists.

Also worth documenting in the PR description since they are real behavior changes: duplicate file refs now dedupe to first placement (later chapters show a jump-up chip instead of a card), stale refs lose the dashed inline row in favor of the outdated chip (this matters most for moved guides), and the unplaced bucket is now a full chapter card.

Nice work overall: the manager lifecycle is clean (no observer leaks, all timers cancelled), and reusing AllFilesCodeView removes a lot of duplicated logic.

[alexanderkreidich]

@backnotprop addressed the review feedback in ef75cc4:

• routed offscreen search results through the guide reveal/mount/activation flow;
• restored inline AI question markers and marker navigation;
• kept the no-IntersectionObserver fallback responsive to outer scrolling;
• enabled outer-page scroll chaining at nested guide diff boundaries;
• pinned the focused file within the bounded viewer window so an open annotation composer and draft survive scrolling;
• memoized each one-file array to avoid repeated patch parsing;
• preserved collapse state across true eviction/remount without remounting the live CodeView on every toggle;
• made initial inner-scroll restoration one-shot per viewer mount;
• added controlled-IntersectionObserver eviction coverage plus focused lifecycle regressions.

The PR description now also documents duplicate-reference placement, stale-reference presentation, the full “Everything else” chapter, and the find-in-page trade-off.

Validation:

• review-editor DOM suite: 174 tests passed, 0 failures, 420 assertions
• bun run typecheck — passed
• bun run --cwd apps/review build — passed
• bun run build:hook — passed
• built scroll-chaining CSS verified
• git diff --check — clean

Remote CI is currently running on ef75cc4. Could you please re-review?

[backnotprop]

Re-reviewed ef75cc4 finding by finding. All five items are properly fixed:

1. The pinned set in GuideViewportManager covers the composer case: the focus arbiter pins the card, scrolling never changes focus, cleanup un-pins symmetrically, and the set is bounded (verified it cannot grow past one entry given global dedupe). The eviction test asserting the focused card is the same DOM node across a window move is exactly the right regression test.
2. fileList memo confirmed stable (upstream identity comes from the memoized resolver); pinned by the referential-identity test.
3. mountCollapsed capture-once is the right shape: local toggles no longer touch the CodeView key, true eviction reseeds from the shell, and the ?? fallback keeps every non-guide consumer byte-identical to main.
4. One-shot restore latch confirmed, re-arms on genuine remount, pinned by the single-scrollTo test.
5. AI markers and marker navigation verified working in guide cards and provably inert elsewhere (EMPTY_AI_MESSAGES guard short-circuits the sync effect for consumers that pass nothing).

The three new paths also check out: search reveal has no re-fire loop, the no-IntersectionObserver fallback has no leak or double-registration, and scroll chaining is opt-in with every other consumer keeping overscroll-contain.

Approving. Follow-ups worth a note (none blocking, happy to file issues): search reveal steals focus and can un-pin a card with an open composer; type-ahead smooth-scrolls the page on every debounce tick (the all-files panel reveals in place instead); App.tsx handleRevealSearchMatch still routes guide-mode matches to the hidden dock (pre-existing); the useAnnotationToolbar restore path when a composing card IS evicted remains untested; plus the cosmetic nits from the first review (chapter badge count, dead cardRef, placeholder height, pointer-handler rect churn, vestigial fileSummaries assertion).

Nice turnaround, and the FakeIntersectionObserver harness is a real improvement to the test setup.

[backnotprop]

Verified fix commit ef75cc4 against the full review; details in comments.