fix(chat): recover viewport anchor via sessionIdx when the content-derived key goes stale (desktop scroll jump-back)

[allenliang2022]

Problem

A residual scroll jump-back on desktop: while the reader is unpinned (scrolled up reading history) and a live assistant turn is streaming, the transcript occasionally yanks backward/downward on a stream chunk.

This is a distinct code path from the mobile jump-back addressed in #5338 / #5392. Those fixes target the browser's native overflow-anchor compensation, which only runs on mobile (touch devices rest at overflow-anchor:auto). Desktop rests at overflow-anchor:none, so that entire guard is a no-op here — this jump is produced by our own JS writing scrollTop, not the browser engine. (Confirmed the affected client was a landscape desktop viewport, not mobile.)

Root cause

_restoreMessageViewportAnchor resolves the captured viewport anchor row by its data-message-anchor-key. That key is content-derived — _messageViewportAnchorKeyForMessage builds it as role|ts|attachments|first-160-chars (see _compressionMessageAnchorKey, which does content...slice(0,160)).

While an assistant message is streaming, each chunk that changes the first 160 characters recomputes that row's data-message-anchor-key. So a scroll snapshot captured one chunk earlier now holds a stale key that no longer matches by key lookup.

The old code conceded immediately on a stale key:

if(!row&&anchorKey) return false;   // <- before sessionIdx was ever consulted

Because this returned false before the data-session-msg-idx fallback, a live-stream stale key could never reach the session-index recovery. The caller then fell back to an absolute scrollTop = snapshot.top, which does not compensate the above-viewport height change from that same chunk (worklog live→settled collapse, tool-card insert, etc.) — the visible jump.

Fix

Compute sessionIdx up front and try the stable data-session-msg-idx lookup before conceding. The anchored row is still in the DOM under its stable session-relative index even when its content key changed, so it recovers and the existing relative-offset realign (scrollTop += (rect.top - containerRect.top) - targetTop) compensates the height change correctly.

• A genuinely removed anchor (message compressed/deleted away) misses key and sessionIdx and still returns false (unchanged behavior).
• A missing sessionIdx is not degraded to the window-relative rawIdx (which could resolve to a different message), preserving the original per-tier guard.

Desktop-and-mobile safe: the realign path is shared; this only changes which row the realign targets when the content key is stale.

Verification

Isolated controlled repro (same DOM, same snapshot, only the impl swapped):

impl	anchor lookup	anchor row visual displacement
old (dead-ends on stale key → absolute fallback)	return false	-400px (jump)
new (recovers via sessionIdx → relative realign)	true	0px (stable)

Tests

• test_stale_content_key_recovers_via_session_index_and_compensates_height — behavior test driving the real _restoreMessageViewportAnchor with a stale key + present sessionIdx; asserts it recovers (true) and writes the exact relative-offset compensation instead of conceding. Base-fails on the old dead-end body, head-passes on the fix (mutation-verified: reverting the fix makes both new tests FAIL).
• test_stale_key_does_not_dead_end_before_session_index_fallback — guards the exact dead-code regression (old if(!row&&anchorKey) return false; must be gone; concede now requires both key and sessionIdx to have failed).
• Updated test_tars_scroll_reset_regressions structural assertions that pinned the old dead-end string.

static/ui.js + two test files only. @nesquena

[greptile-apps]

Greptile Summary

This PR fixes a desktop-specific scroll jump-back during live assistant message streaming by recovering the viewport anchor via its stable data-session-msg-idx attribute when the content-derived data-message-anchor-key goes stale mid-stream.

• Root cause eliminated: The old code had if(!row&&anchorKey) return false before sessionIdx was ever consulted, so a stale content key caused an immediate concede to an absolute scrollTop write that did not compensate above-viewport height growth — producing the visible jump. The fix moves sessionIdx computation above the key lookup and tries the session-index fallback before conceding.
• Concede guard tightened: The replaced if(!row&&hasSessionIdx) return false becomes if(!row&&(anchorKey||hasSessionIdx)) return false, which correctly concedes only after both identity channels have been exhausted while preserving the existing rawIdx fallback path for anchors that carried neither identifier.
• Two new behavioral tests cover the fix: one drives the real _restoreMessageViewportAnchor with a stub DOM and verifies the exact relative-offset scroll compensation (1050), and the other asserts the old dead-end pattern is absent and the new concede condition is present.

Confidence Score: 5/5

Safe to merge — the change is confined to a single early-exit guard inside the scroll restoration function, logic is correct across all identity-channel combinations, and the two new tests verify both the fixed behavior and the absence of the old dead-end path.

The reordering of sessionIdx computation and the replacement of the two separate concede guards with a single combined check are logically equivalent for every pre-existing case, and correctly add the sessionIdx recovery step for the newly reachable stale-key path. All edge cases (no key, no sessionIdx, stale key + valid sessionIdx, genuinely removed anchor) produce the same outcome as before or the intended fix. Tests drive the real function against a stub DOM and pin the exact scrollTop delta, providing strong mutation coverage.

No files require special attention.

Important Files Changed

Filename	Overview
static/ui.js	Moves sessionIdx extraction above the key lookup and removes the early-exit that prevented sessionIdx fallback on stale content keys; replaces the old per-identifier concede guards with a single combined check covering both channels.
tests/test_issue4295_midstream_scroll_anchor.py	Adds two tests: a Node.js behavioral test that runs the real function against a stub DOM and asserts the correct scrollTop compensation (1050), and a structural test asserting the old dead-end pattern is gone and the new concede condition is present.
tests/test_tars_scroll_reset_regressions.py	Updates structural assertions to expect the old dead-end pattern absent and the new combined concede condition present, keeping the regression suite consistent with the fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_restoreMessageViewportAnchor called] --> B{container && anchor?}
    B -- No --> Z[return false]
    B -- Yes --> C[Compute anchorKey, sessionIdx, hasSessionIdx]
    C --> D[Key lookup: querySelectorAll data-message-anchor-key]
    D --> E{row found?}
    E -- Yes --> F{getClientRects empty?}
    F -- Yes --> G[row = null]
    F -- No --> H[row is valid]
    E -- No --> G
    G --> I{hasSessionIdx?}
    I -- Yes --> J[querySelector data-session-msg-idx]
    J --> K{row found?}
    K -- Yes --> H
    K -- No --> L{anchorKey OR hasSessionIdx?}
    I -- No --> L
    L -- Yes --> Z
    L -- No --> M[rawIdx fallback lookup]
    H --> N[Write scrollTop relative-offset realign]
    M --> O{row found?}
    O -- No --> Z
    O -- Yes --> N
    N --> P[return true]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[_restoreMessageViewportAnchor called] --> B{container && anchor?}
    B -- No --> Z[return false]
    B -- Yes --> C[Compute anchorKey, sessionIdx, hasSessionIdx]
    C --> D[Key lookup: querySelectorAll data-message-anchor-key]
    D --> E{row found?}
    E -- Yes --> F{getClientRects empty?}
    F -- Yes --> G[row = null]
    F -- No --> H[row is valid]
    E -- No --> G
    G --> I{hasSessionIdx?}
    I -- Yes --> J[querySelector data-session-msg-idx]
    J --> K{row found?}
    K -- Yes --> H
    K -- No --> L{anchorKey OR hasSessionIdx?}
    I -- No --> L
    L -- Yes --> Z
    L -- No --> M[rawIdx fallback lookup]
    H --> N[Write scrollTop relative-offset realign]
    M --> O{row found?}
    O -- No --> Z
    O -- Yes --> N
    N --> P[return true]

Loading

_{Reviews (1): Last reviewed commit: "fix(chat): recover viewport anchor via s..." | Re-trigger Greptile}

[nesquena-hermes]

🔬 Gate certification — GREEN ✅

Certified head: sha:15fbcb4c (clean rebase, branch gate-rebase/5469-viewport-anchor-sessionidx) · PR: #5469 · allenliang2022, fix(chat): recover viewport anchor via sessionIdx when the content-derived key goes stale (desktop scroll jump-back)
Verdict: Full gate GREEN. A genuine dead-code fix: the old anchor-restore bailed (if(!row&&anchorKey) return false) before the sessionIdx fallback could run, so a stale content-derived key fell through to an absolute scrollTop that didn't compensate above-viewport height growth → desktop scroll jump-back. The fix makes the sessionIdx recovery reachable. Codex SAFE, suite green.

What I ran (rebased worktree /tmp/wt-rebase-5469)

Gate	Result
Rebase onto current master	✅ git apply clean
Codex (reproduce)	SAFE TO SHIP — gated the rebased worktree, 0 findings
Full pytest suite	2 failed / 11831 passed — both non-defects (nous env flake + test_issue4536 isolation flake)
PR's own tests	✅ 27/27 (test_issue4295_midstream_scroll_anchor + test_tars_scroll_reset_regressions)

Findings

✅ Sound multi-tier recovery, jump-back compensated: _restoreMessageViewportAnchor now: find by anchorKey → treat found-but-unrendered (getClientRects().length===0) as missing → if(!row&&hasSessionIdx) recover via [data-session-msg-idx="${sessionIdx}"] (stable session-relative index, not fragile nth-child) → if(!row&&(anchorKey||hasSessionIdx)) return false → data-msg-idx fallback for targetIdx → final if(!row) return false. The old premature if(!row&&anchorKey) return false (which made the sessionIdx branch dead code) is removed. Realignment uses a RELATIVE container.scrollTop += (rect.top - containerRect.top) - targetTop that compensates above-viewport height growth (the jump-back fix), with the mobile overflow-anchor double-shift awareness intact (no regression to #5392/#5338). Codex SAFE (0 findings), 27/27 tests.

Recommendation to the next agent

Ready to merge — use branch gate-rebase/5469-viewport-anchor-sessionidx (sha:15fbcb4c), NOT the PR's stale head d4043f76. Clean dead-code fix: the sessionIdx recovery tier is now reachable, finds the correct row by stable index, and the relative scrollTop adjustment compensates the above-viewport growth that caused the desktop jump-back; multi-tier fallback concedes correctly when genuinely unrecoverable. Codex SAFE + 27/27 + suite green bar 2 known flakes. Scroll-behavior fix (test-covered) — a quick desktop mid-stream scroll-position smoke confirms, but the recovery chain + compensation are verified. concept 4/5. Credit @allenliang2022 (co-authored). crit=3. (Same scroll-anchor family as the shipped #5392/#5338; no double-shift regression.)

---

Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; sessionIdx recovery reachability + correct-row-by-stable-index + relative jump-back compensation + no mobile-overflow-anchor regression verified, Codex SAFE + 27/27 + suite green bar 2 known flakes. Cert valid for sha:15fbcb4c.

[nesquena-hermes]

Shipped in v0.51.841 — thanks @allenliang2022! 🎉

The desktop scroll jump-back fix is live: _restoreMessageViewportAnchor now recovers via the stable data-session-msg-idx when a streaming chunk invalidates the content-derived anchor key (previously that fallback was unreachable dead code, so restore fell back to an absolute scroll position that didn't compensate for content growing above the viewport → the upward yank).

Full gate: Codex SAFE (sessionIdx fallback reachable, removed-message concede preserved, mobile overflow-anchor fix #5338/#5392 untouched, settled path unaffected), Opus ship-safe, suite green (2 unrelated pre-existing env flakes). Credited via Co-authored-by.

Fast-follow (non-blocking): Opus suggested a zero-rect hardening on the sessionIdx querySelector lookup — noted for a minor follow-up.