fix(virtualizer): positional splice anchor on total re-key (#8033)

[CrysisDeu]

Problem / Motivation

The mid-list splice capture (TRIGGERS 4/5/6) anchors the topmost visible mounted row whose key survives the commit, filtered through survivingKeys. When every visible row's key retires in one commit — a total re-identification of the on-screen rows, e.g. each streamed row replaced by its server copy under a different key, landing in the same commit as a splice — the capture returns null and silently stands down: no anchor, no scroll compensation, and the reader is displaced by the inserted rows' height.

The prepend path (TRIGGER 1) had the identical stand-down; PR #8001 fixed it by re-finding the row by POSITION (nearest-survivor displacement, net count as last resort). The splice half was knowingly deferred there as an accepted-and-deferred First Principles advisory. This PR closes it: the class is the same — a compensation path whose anchor-not-found branch is a silent no-op.

Why it matters

A reader scrolled up through history loses their place whenever a transcript refresh re-identifies the visible rows together with a mid-list insert/removal/swap: the viewport silently jumps by the spliced rows' height. Bounded harm, but it is the last silent no-op branch of this class in the virtualizer.

What changed (motivation → approach → change)

Symptom → root cause: the splice capture's keyAt filter admits only surviving keys, so a commit that retires every visible key gives captureTopAnchorFrom nothing to bind and the branch exits without arming the consumer.

Approach: reuse TRIGGER 1's positional re-identification rather than duplicating it.

• Factored PR fix(virtualizer): hold the reader by position when a prepend retires every key #8001's nearest-survivor displacement machinery into a shared module-level helper, nearestSurvivorShiftFrom(prevItems, prevGetKey, newIndexByKey, noSurvivorShift, boundary). TRIGGER 1's fallback now calls it (behavior unchanged, boundary defaults to 0 — a prepend renames index 0, so nothing sits before its boundary).
• Extended the splice capture: when the surviving-key capture returns null, re-run it with a positional keyAt — the row moved by as much as its nearest surviving old-index neighbour, and its new key is whatever now sits at old index + that displacement, priced by the CURRENT render's getKey paired with the current items (the pairing contract PR fix(virtualizer): hold the reader by position when a prepend retires every key #8001 pinned). The resolved anchor arms the existing splice consumer (shiftAnchorRef/shiftStageRef stage 'ready', plus the rowSwapped → spliceCommit bump), so part 2 performs the ordinary scrollTop correction.
• The boundary parameter is new relative to PR fix(virtualizer): hold the reader by position when a prepend retires every key #8001's machinery and carries the splice-specific correctness rule (surfaced by the pre-push review): survivors ABOVE the first changed index (movedIndex) did not move, and borrowing their zero displacement would anchor a splice landing exactly at the viewport top on the inserted row itself. Excluding them makes the displacement come from survivors at/after the change; when none remain, noSurvivorShift (the net count change) stands in, which for a contiguous splice above the reader IS the exact displacement.

Tests

Four regressions in website/src/test/useVirtualChat.prependAnchor.test.tsx, all on the existing mountScrolledUp harness, each proving non-vacuity (every previously visible key genuinely gone, ghost genuinely mounted) and that the hold is a real scrollTop write:

• holds the reader by POSITION when a splice retires every visible key — one row inserted above the viewport, index 0's key held, every visible row re-keyed; the topmost visible row keeps its screen offset (without the fix it moves down by the inserted row's height).
• holds the reader by POSITION across a splice-with-total-retirement when getKey is INDEX-ADDRESSED — pins the current-items/current-getKey pairing contract an identity getKey cannot distinguish.
• does not anchor the inserted row when the splice lands exactly at the viewport top — the boundary case: rows above the viewport keep their keys but did not move; without the boundary exclusion the anchor resolves to the ghost itself and no correction runs.
• does not anchor the inserted row at the viewport top when getKey is INDEX-ADDRESSED — same boundary case under the ChatPage-shaped getKey.

Manual verification

N/A — the deterministic fake-layout harness reproduces the displacement pixel-for-pixel (the same engine the existing prepend/splice regressions use), and the change is unreachable except on the total-retirement commit it targets.

Related Issues

Closes #8033

Pattern harvest

Not generalizable: this closes the second (and last) known instance of the "compensation path whose anchor-not-found branch is a silent no-op" class that PR #8001 named; both paths now share one positional-fallback helper, so a third instance would reuse it rather than re-derive it.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — N/A, hook-internal behavior documented in code comments
• No secrets, credentials, or internal references in the diff

[github-actions]

UX Review (Fable 5) — ✅ PASS

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

The diff is confined to website/src/hooks/virtualizer/useVirtualChat.ts plus a new test file — a scroll-anchoring fix inside the chat virtualizer with no user-facing strings, no layout or component changes, and no screenshots. The UX effect is purely corrective: the reading position no longer jumps when a mid-list splice coincides with a full row re-key.

UX-Verdict: PASS

Invisible-chrome fix: the reader's scroll position now holds through splice-plus-re-key commits; no strings, layout, or affordances change.

[UX-REVIEWED] 9f5271f

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

The diff is fully reviewed: a shared nearestSurvivorShiftFrom helper (TRIGGER 1's fallback extracted verbatim, boundary defaulting to 0 so its behavior is unchanged), a splice-path positional fallback that only runs when the surviving-key capture returns null, arming the existing consumer, and four non-vacuous regressions covering both getKey shapes and the boundary case.

Design gate outcome: real harm (reader displacement on total re-key + splice, the deferred half of #8033's class), solved with the approach already proven on the prepend path, factored rather than duplicated, reaching only the previously-silent branch — the common commit path gains no new conditional cost or failure mode, and a still-unresolvable anchor degrades to the pre-existing stand-down. Hook-internal, no contract change, fully reversible. The boundary addition encodes a genuine correctness rule and is pinned by two tests. No design-level findings survive the kill filter.

Design-Verdict: PASS

Closes the known-deferred half of an already-fixed defect class by reusing the proven mechanism; rare-path-only, degrades to prior behavior, fully reversible.

[DESIGN-REVIEWED] 9f5271f

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 9f5271fab945f2c9cadc4888c4d960e5ef26b8cb — 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.

I have everything I need. Let me verify the counts once more against what I read:

• captureTopAnchorFrom has 10 call sites in useVirtualChat.ts; exactly 2 use a survivorship-filtered resolver that can return null on total re-key (line 679, TRIGGER 1; line 826, splice) — both now carry the positional fallback, so the "last instance of the class" claim checks out.
• nearestSurvivorShiftFrom has 2 real consumers (lines 695, 844); its boundary parameter receives two distinct values (default 0 vs movedIndex), so it is not a one-consumer generalization.
• The extracted helper is byte-for-byte the machinery deleted from TRIGGER 1 (the diff removes the inline survivors/shiftAt block and replaces it with the call), so nothing was duplicated.

First-Principles-Verdict: PASS

Closes a reported, previously-deferred defect (#8033) at the cause level, reusing the existing mechanism instead of duplicating it; every item is declared.

What this change ships

Intent: a reader scrolled up in history no longer loses their place when a mid-list splice lands in the same commit that re-keys every visible row — a FIX.

1. Splice compensation now re-finds the reader's row by position when no visible key survives — justified (reported defect fix(virtualizer): splice capture still stands down when every visible key retires #8033, deferred from PR fix(virtualizer): hold the reader by position when a prepend retires every key #8001; cause-level, closes the class).
2. TRIGGER 1's inline nearest-survivor machinery moved into shared helper nearestSurvivorShiftFrom — declared; net subtraction (the alternative was a second copy of the binary-search block).
3. New boundary parameter on the helper — declared, derived (without it the fallback anchors the inserted row itself when the splice lands at the viewport top; pinned by two of the tests).
4. Four regression tests on the existing mountScrolledUp harness — declared.

Counted claims backing the verdict: 10 captureTopAnchorFrom call sites, of which 2 use a surviving-key filter that could stand down silently — both now have the positional fallback, so zero unfixed siblings remain (grepped captureTopAnchorFrom, survivingKeys.has|newIndexByKey.has). The helper has 2 consumers and its boundary parameter receives 2 distinct values, so neither is speculative surface.

[FIRST-PRINCIPLES-REVIEWED] 9f5271f

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 9f5271fab945f2c9cadc4888c4d960e5ef26b8cb — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 9f5271f

Verdict parsed from the review's SHA-scoped output markers for commit 9f5271fab945f2c9cadc4888c4d960e5ef26b8cb.

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 9f5271fab945f2c9cadc4888c4d960e5ef26b8cb and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 9f5271f

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

[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: fix with a clear root cause -- useVirtualChat anchored the prepend splice by identity, so a total re-key re-anchored to the wrong row; the fix anchors positionally and adds one regression test. Frontend virtualizer hook only, no auth/session/trust surface.

[chenmingwei23]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean (Semgrep 0 + CodeQL 0 alerts), security checklist all-NO, AI reviewers green. Category: fix(virtualizer) positional splice anchor on total re-key (#8033), 2 files, clear root cause.

[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: fix - restores the splice trigger's no-surviving-key positional fallback (#8033) by extracting the shared nearest-survivor displacement resolver; one hook source file plus its own test, no surface outside chat scroll anchoring.