fix(virtualizer): hold the reader across an equal-count row swap

[chenmingwei23]

Why no screenshot: scroll-position fix with no static visual delta -- the change is where the viewport sits across one commit, which a before/after still cannot show. Reproduced numerically instead (700px of drift in the harness).

Problem / Motivation

The render-phase scroll-anchor capture has one consumer, and it is invalidated by windowRange and itemCount. An equal-count commit moves neither, so the consumer does not run in that commit.

That left one shape of the transient-row jitter uncompensated after #7198: the "thinking" placeholder leaving and its replacement arriving in one React-batched update. The net count never moves, so no count-delta trigger fires -- and when the replacement renders taller than the row it replaced, everything above a reader scrolled up moves, and their row walks off under them.

Measured in the prependAnchor harness with a 3x-taller replacement: 700px. Two hundred of that is the row itself growing; the rest is the mean re-pricing every unmeasured row above the reader, because getHeight prices an unmeasured row from the running mean of the measured ones, so one new measurement re-prices the whole region.

Why it matters

It is the ordinary streaming shape, not an edge case: React batches the placeholder's removal and the output's insertion into a single update, so this fires on every placeholder-to-output transition of every turn. A reader scrolled back through a transcript while an agent is still working gets the #6076 jitter for each one, even with #7198 landed.

What changed (motivation -> approach -> change)

The capture was never the missing piece -- the invalidation key was. #7198 could see the swap (it already retires the departed row's height there) and deliberately declined to capture an anchor: with no key that moves on an equal-count commit, the anchor would have sat in the shared slot until some later commit changed windowRange or itemCount, and then been spent on unrelated geometry -- the stranded-anchor hazard the render-phase capture exists to remove. Capturing without the key would have traded a 700px drift for a nondeterministic yank.

So the key comes first, and the capture follows it:

1. spliceCommit, a counter bumped in the render that captures a swap anchor, added to part 2's deps. Every other trigger already rides a key that consumer watches -- a prepend, splice or append moves itemCount, a window shift moves windowRange. Only the swap has neither, so it brings its own, and the anchor is now spent in the very commit it describes.

It is real state rather than a ref token, for the reason heightCommit in this same file is: a counter the effect does not subscribe to is invisible to tooling and needs an exhaustive-deps exemption to sit in a dep array at all. The bump is the render-time state-update pattern the session and height-cache sentinels above it already use, and it terminates for the same reason they do -- prependPrevRef has advanced by the time React re-invokes the render, so the second pass detects no swap, captures nothing and bumps nothing.

2. TRIGGER 6 joins the existing splice gate, through the same capture point, the same slot, the same consumer and the same key mapping (previous items at the mounted node's previous index, filtered to keys that survive the commit, so the replaced row's key falls forward to the next survivor). No parallel anchor path -- a second path would fight this one for scrollTop.

Cost is one extra render pass, and only on a commit that actually captures. The bump sits INSIDE the capture, which is gated on !stickRef.current, so the primary reading mode (pinned to the bottom) never pays it; and a token append is not a swap, so the commit that lands per streamed chunk never reaches it.

What this deliberately does NOT change: the offset memo's deps. The issue proposed adding the key there too, and on main that is already covered by a different mechanism: #7198's retirement block calls heightIndex.sync(itemCount) in the render phase (useVirtualChat.ts:1050), precisely because the memo is keyed on itemCount. A swap that departs a key -- which is every swap -- therefore already re-syncs the tree in its own commit, and adding the key to the memo would buy a second O(N) walk over the same heights in the same render. Confirmed empirically: the cases below hold at <=1px with the memo untouched.

Diff is confined to website/src/hooks/virtualizer/useVirtualChat.ts and its prependAnchor harness. No reformatting.

Tests

useVirtualChat.prependAnchor.test.tsx grows a per-key height override (rowHeightByKey), because an equal-height swap displaces nothing and would pass with no fix at all. Three cases, each red before:

New case	Reverted change	On the revert	On this branch
row SWAPPED at equal count above the reader	TRIGGER 6 capture	drifted 700px	holds (<=1px)
same, with an INDEX-ADDRESSED getKey (ChatPage's shape)	TRIGGER 6 capture	drifted 700px	holds (<=1px)
row SWAPPED at equal count above the reader	spliceCommit dep only	drifted 700px	holds (<=1px)
same, INDEX-ADDRESSED	spliceCommit dep only	drifted 700px	holds (<=1px)

Both halves are load-bearing, proven separately: reverting only the gate widening (anchor never captured) and reverting only the dep (anchor captured, consumer never runs) each reproduce the full 700px.

x holds the reading position when a row is SWAPPED at equal count above the reader
  AssertionError: expected 700 to be less than or equal to 1
x holds the reading position across an equal-count SWAP when getKey is INDEX-ADDRESSED
  AssertionError: expected 700 to be less than or equal to 1

Plus a pinned counterpart, does not hold position for a PINNED reader across an equal-count SWAP, which pins the gate: a pinned reader is never pulled back up to where a row used to sit, and stick survives (the next streamed message still lands at the bottom). Non-vacuity is asserted in each case -- the replaced row really is gone and the taller replacement really mounted at the same index.

No existing assertion weakened. The virtualizer family is 223/223 green (anchorRestore, followDisengage, heightSyncAnchor, integration, layoutShrink, observerBackfill, postStreamLurch, prependAnchor, railCollapse, spacerLurch, viewportResize, zeroHeightGuard, useVirtualChat, UseVirtualChatCoverage, HeightCache, HeightCache.multiInstance, virtualizerHeightOwner, ScrollAnchorCache, ChatPage.queueBandReanchor) -- the family-wide pass the issue asked for. The consumer side is 155/155 green across the files that assert scroll behaviour through this hook (ChatPage.navFarJump, ChatPane.scrollChrome, useChatScrollFollow, useScrollManager, useScrollManagerCov80, FollowController, WindowCalculator, McpAppFrame, ArtifactsPage.singleScroller, ThinkingBlock), which is where an extra render pass would show up if it were visible at all. tsc -b, eslint and jscpd clean.

Manual verification

N/A -- the jitter is a scroll-position delta, and the harness reproduces it numerically: its deterministic layout engine makes the rows genuinely move, so every number above is measured rather than asserted against source text.

Related Issues

Closes #7234
Refs #7198, #6076

Pattern harvest

Rule candidate: review-prompt

Pattern: when a shared consumer is invalidated by a proxy for "something changed", any producer whose change the proxy cannot express is silently unserved -- and the fix belongs in the invalidation contract, not in the producer. Here the proxy was a pair of count/range values standing in for "the geometry above the reader moved", and the one commit shape that moves geometry without moving either value had no way to reach the consumer. The tell is a comment explaining that a detection is correct but deliberately not acted on: #7198 identified the swap, retired its height, and had to write down that it could not anchor it. That asymmetry -- half a commit shape handled, the other half documented as out of reach -- is the reviewable signal that an invalidation key, not a trigger, is missing. The general question to ask of any effect whose deps are values rather than events: enumerate the commit shapes that change none of them, and check that none of them is a shape the effect exists to serve.

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of 3e2f516b9f503bb1d275025ffe227cd76f127746 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff is entirely internal virtualizer logic (useVirtualChat.ts) plus tests — no user-facing strings, components, screenshots, or visual surfaces. The change fixes a scroll-position jump when a streaming placeholder row is swapped for its taller replacement above the reader's viewport, and keeps the pinned-to-bottom reading mode untouched. That's a pure UX improvement with no new surface to evaluate against the lenses.

UX-Verdict: PASS

No user-facing surface changes; the fix removes a scroll jump mid-read and preserves bottom-pinned following, verified for both directions in tests.

[UX-REVIEWED] 3e2f516

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 3e2f516b9f503bb1d275025ffe227cd76f127746 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

The missing invalidation key was the actual root cause; the fix lands it at the contract, keeps one anchor path, and proves both halves load-bearing independently.

[DESIGN-REVIEWED] 3e2f516

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 3e2f516b9f503bb1d275025ffe227cd76f127746 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified against the repo: spliceCommit has exactly one bump site (useVirtualChat.ts:744) and one consumer (the dep array at useVirtualChat.ts:1902); the "memo deps untouched" claim is backed by the existing render-phase heightIndex.sync(itemCount) at useVirtualChat.ts:1050; and the removed comment on main explicitly deferred this exact fix to #7234, which this PR closes. Sibling scan of proxy-keyed consumers in the file (dep arrays on itemCount/windowRange): part 1 (line 1845) only serves prepends, which always move the count; the height-anchor consumer (line 1929) subscribes to the owner's real version, not a proxy — zero unfixed siblings.

First-Principles-Verdict: PASS

A reported, measured 700px reader drift (#7234) is removed at the invalidation contract — the actual cause — through the existing capture point and consumer, adding no parallel path.

What this change ships

Intent: keep a scrolled-up reader's row steady when the thinking placeholder is swapped for taller output in one commit — a FIX.

1. Reader scrolled up no longer drifts 700px on every placeholder→output swap — justified (defect fix(virtualizer): the anchor consumer has no invalidation key for an equal-count row swap #7234, deliberately deferred by fix(virtualizer): hold the reader across a mid-list row splice #7198's own comment).
2. Internal spliceCommit counter keying the existing anchor consumer — justified; 1 producer (line 744), 1 consumer (line 1902), minimal form given the file's documented no-ref-token-in-deps rule (heightCommit precedent).
3. One extra render pass, only on an unpinned swap commit — declared, gated on !stickRef.current, pinned by the new PINNED test.
4. Test harness gains per-key row heights (rowHeightByKey) — declared; defaults preserve every existing case.
5. Trigger docs updated five→six in the same commit — mandated by repo convention.

No riders, no undeclared items, no new public surface: the hook's signature and every config/API boundary are unchanged, and the fix reuses the existing capture point, anchor slot, and consumer rather than adding a second correction path.

[FIRST-PRINCIPLES-REVIEWED] 3e2f516

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 3e2f516b9f503bb1d275025ffe227cd76f127746 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 3e2f516

Verdict parsed from the review's SHA-scoped output markers for commit 3e2f516b9f503bb1d275025ffe227cd76f127746.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 3e2f516b9f503bb1d275025ffe227cd76f127746: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 3e2f516b9f503bb1d275025ffe227cd76f127746 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 3e2f516

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 3e2f516b9f503bb1d275025ffe227cd76f127746: <one-sentence reason>

[NicholasRBowers]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix with clear root cause — equal-count row swap stranded the scroll anchor because no invalidation key moved; adds a spliceCommit key so the anchor is consumed in its own commit, with a red-before regression test.

[dwu96]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: frontend-only virtualizer scroll-anchor fix with a clear root cause — an equal-count row swap moves neither invalidation key the compensation consumer watches, so the reader lost position; the fix adds a dedicated spliceCommit key so the anchor is spent in the commit it describes, plus four tests including the pinned-reader negative case.

[iamwhatever]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: adds a sixth compensation trigger with its own spliceCommit invalidation key so the virtual-chat reader anchor survives an equal-count row swap; frontend virtualizer hook plus its test, no runtime surface beyond scroll anchoring.