fix(webui): keep a following reader pinned through streaming growth and SSE recovery

[allenliang2022]

Problem

Two related "jump back" reports where a reader who is following the live stream gets stranded mid-transcript instead of staying at the bottom. Both reproduced and fixed live (the A/B below was run against a local source build).

Class 1 — content grew beneath a pinned viewport (static/ui.js scroll listener)

While streaming on a tall transcript (most visible on mobile, where chunks land fast), new content increases scrollHeight under a stationary viewport. The reader never scrolled — top did not move up and _messageUserUnpinned is false — but bottomDistance crosses the nearBottom threshold, so the listener fell through to _scrollPinned=false, killing auto-follow mid-stream. The follow writer and the scroll listener then fought frame-by-frame: the viewport stalled while content kept growing and was progressively stranded mid-transcript.

Class 2 — SSE-recovery render restores a stale position (static/messages.js)

_handleStreamError (SSE drop), the Task-cancelled apply + fallback paths, and the reconnect-stream-dead cleanup all push/replace S.messages then call renderMessages({preserveScroll:true}). preserveScroll's restore path keys on the pre-render snapshot's bottom-distance, which during a live stream can read large (content was still growing under a followed viewport). So on a process restart / SSE drop / cancel it yanked a following reader up to a stale historical position — the "everything jumped back" report.

Fix

Class 1: in the !_messageUserUnpinned branch, when the viewport did not move up and auto-follow is on, keep the pin and re-snap to the true bottom via _setMessageScrollToBottom() instead of unpinning. The genuine scroll-away fallthrough (else { _scrollPinned=false }) is unchanged, so a real upward scroll still unpins.

Class 2: capture follow-intent (_isMessagePaneNearBottom(1200)) before mutating S.messages, and after the recovery render, scrollToBottom() if the reader was following — so they see the interruption/cancellation notice in place. Readers who had scrolled up to read history are left where they were (the near-bottom guard is false for them). Applied at all four recovery mutation points: SSE-drop notice, Task-cancelled embedded-payload apply, Task-cancelled fallback, and reconnect-stream-dead placeholder cleanup.

Verification

Behavioral A/B via Playwright, scenario = disconnect/cancel/recovery while the viewport is 400px from bottom (the >250px band that triggers preserveScroll's restore path):

Path	OLD (stranded)	FIX (at bottom)
Connection interrupted	582px from bottom	1px
Task cancelled	523px	1px
Reconnect-stream-dead	523px	1px
Content-grew pinned (200-line stream)	stalled, progressively stranded	stays pinned at bottom

Structural source-locks in tests/test_sse_recovery_scroll_stranding.py (4 follow-intent guards + the content-grew re-snap ordering). Local run green: 27 passed across the new file + test_issue4295_midstream_scroll_anchor.py + test_issue4856_android_scroll_regression.py.

Pure-additive: git diff origin/master is +53 lines, 0 deletions in the two JS files.

[greptile-apps]

Greptile Summary

This PR fixes two distinct "jump-back" scroll-stranding regressions that left following readers stranded mid-transcript during live streaming. The fixes are purely additive (+53 lines, 0 deletions) across two JS files, with a new regression test file and one window-widening adjustment to an existing source-lock test.

• Class 1 (static/ui.js): The scroll listener's !_messageUserUnpinned branch now detects when new content grew beneath a stationary pinned viewport (distinguishing it from a real upward scroll) and re-snaps to bottom via _setMessageScrollToBottom() instead of killing the pin. The real scroll-away unpin fallthrough is preserved unchanged.
• Class 2 (static/messages.js): All four SSE-recovery/cancel mutation sites now capture follow-intent (_isMessagePaneNearBottom(1200) && !unpinned) before mutating S.messages, then call scrollToBottom() after the recovery render to land following readers at the new-content bottom instead of at a stale historical position restored by preserveScroll's snapshot path.

Confidence Score: 5/5

Safe to merge — purely additive fix to two scroll-state paths with no mutations to shared data structures, no new network calls, and Playwright-verified A/B results.

The JS changes are well-scoped: each new code path captures a boolean before an existing mutation, then calls an idempotent scroll function after the existing render. The unpin/re-pin logic in ui.js is isolated to a single else-if branch that is only reached when the user has not manually scrolled. The regression tests cover the ordering invariant, the sticky-unpin guard, and the behavioral four-quadrant matrix.

No files require special attention. The four duplicated follow-intent expressions in messages.js are a maintenance note, not a correctness concern.

Important Files Changed

Filename	Overview
static/ui.js	Adds a new branch in the scroll listener to distinguish content-growth from a genuine user scroll-away, re-snapping to bottom via _setMessageScrollToBottom() instead of unpinning. Logic is correct.
static/messages.js	Adds follow-intent capture before S.messages mutation at all four SSE-recovery/cancel paths, then calls scrollToBottom() post-render for following readers. Guard expression is duplicated four times but correct at each site.
tests/test_sse_recovery_scroll_stranding.py	New regression-lock test file. Behavioral Node.js test and ordering test are well-structured. Per-guard assertions check global file presence rather than per-guard wiring (noted in existing thread).
tests/test_run_journal_frontend_static.py	Widens source-lock search window from 1200 to 2200 characters to accommodate new _wasFollowingAtReconnectDead code. Change is necessary and correctly motivated.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Scroll event fires] --> B{movedUp?}
    B -- yes --> C[_scrollPinned=false, _messageUserUnpinned=true]
    B -- no --> D{movedDown AND nearBottom?}
    D -- yes --> E[Increment counter, maybe re-pin]
    D -- no --> F{not _messageUserUnpinned?}
    F -- no --> G[_scrollPinned=false]
    F -- yes --> H{nearBottom?}
    H -- yes --> I[Increment counter, maybe pin]
    H -- no --> J{NEW: autoFollow AND _scrollPinned?}
    J -- yes --> K[Content grew beneath pinned viewport - re-snap via _setMessageScrollToBottom]
    J -- no --> L[_scrollPinned=false]
    M[SSE drop or cancel or reconnect-dead] --> N[Capture _wasFollowing before S.messages mutation]
    N --> O[renderMessages with preserveScroll true]
    O --> P{_wasFollowing?}
    P -- yes --> Q[scrollToBottom - reader lands at recovery notice]
    P -- no --> R[Reader stays at scrolled-away position]

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[Scroll event fires] --> B{movedUp?}
    B -- yes --> C[_scrollPinned=false, _messageUserUnpinned=true]
    B -- no --> D{movedDown AND nearBottom?}
    D -- yes --> E[Increment counter, maybe re-pin]
    D -- no --> F{not _messageUserUnpinned?}
    F -- no --> G[_scrollPinned=false]
    F -- yes --> H{nearBottom?}
    H -- yes --> I[Increment counter, maybe pin]
    H -- no --> J{NEW: autoFollow AND _scrollPinned?}
    J -- yes --> K[Content grew beneath pinned viewport - re-snap via _setMessageScrollToBottom]
    J -- no --> L[_scrollPinned=false]
    M[SSE drop or cancel or reconnect-dead] --> N[Capture _wasFollowing before S.messages mutation]
    N --> O[renderMessages with preserveScroll true]
    O --> P{_wasFollowing?}
    P -- yes --> Q[scrollToBottom - reader lands at recovery notice]
    P -- no --> R[Reader stays at scrolled-away position]

Loading

_{Reviews (5): Last reviewed commit: "test(#5217): scope follow-intent orderin..." | Re-trigger Greptile}

[nesquena-hermes]

Bouncing for one real CORE issue on the pin model — the fix is otherwise on the right track, and Codex reproduced this by reading the head.

The blocker (verified against the diff): the four recovery follow-guards infer "was the reader following?" from _isMessagePaneNearBottom(1200) only, ignoring the sticky manual-unpin state (_messageUserUnpinned) that static/ui.js maintains:

• static/messages.js:5581 / :5611 (cancel + cancel-fallback)
• static/messages.js:5784 / :5831 (SSE disconnect + reconnect-dead cleanup)

Each then calls scrollToBottom() (:5592/:5619/:5802/:5840), which clears _messageUserUnpinned. So a reader who scrolled up but is still within 1200px of the bottom gets force-scrolled to the bottom on cancel / SSE-drop / reconnect-dead recovery — clobbering the preserveScroll restore that currently works for them. (The code comment claims "readers who scrolled up are left where they were... the near-bottom guard is false for them" — but that's only true beyond 1200px; a manually-unpinned reader within 1200px is wrongly treated as following.) This conflicts with the sticky-unpin model just shipped in #5058 / #5118.

Fix-spec (Codex, sound): replace the four raw _isMessagePaneNearBottom(1200) recovery guards with a shared predicate that requires auto-follow not disabled AND the reader not manually unpinned, e.g.:

window._autoScrollFollow !== false
  && !(typeof _isMessageReaderUnpinned === 'function' && _isMessageReaderUnpinned())
  && ((typeof _shouldFollowMessagesOnDomReplace === 'function' && _shouldFollowMessagesOnDomReplace())
      || (typeof _isMessagePaneNearBottom === 'function' && _isMessagePaneNearBottom(1200)))

Then add a regression test: a reader unpinned at ~400px from bottom must NOT get scrollToBottom() called on cancel/SSE-drop/reconnect recovery (the case the current tests miss — they don't exercise unpinned-but-near-bottom).

What's good (keep it): the Class-1 streaming-growth pin preservation is the right idea, and the intent — keep a genuine follower at the bottom through recovery — is correct; it just needs to honor the sticky-unpin state so it doesn't yank a reader who deliberately scrolled up a little. Re-gate on re-push. Thanks @allenliang2022.

[nesquena-hermes]

Re-gated the re-push — appreciate the iteration, but the bounce concern is still present (Codex reproduced it, and I confirmed by reading the guards). The 4 SSE-recovery follow-guards still infer follow-intent from _isMessagePaneNearBottom(1200) only, ignoring the sticky _messageUserUnpinned state — so the exact case I bounced for is unfixed.

Reproduced: a reader scrolls UP but stays within 1200px of the bottom → _messageUserUnpinned===true (set on manual upward scroll, static/ui.js:4109/4398) AND _isMessagePaneNearBottom(1200)===true. On cancel / SSE-drop / disconnect / reconnect-dead, each _wasFollowing* guard calls scrollToBottom() and clobbers the reader's position.

Exact fix — make each of the 4 guards sticky-aware (static/messages.js:5615, :5645, :5818, :5865):

const nearBottom1200 = (typeof _isMessagePaneNearBottom==='function') ? _isMessagePaneNearBottom(1200) : true;
const readerUnpinned = (typeof _isMessageReaderUnpinned==='function')
  ? _isMessageReaderUnpinned()
  : (typeof _messageUserUnpinned!=='undefined' && _messageUserUnpinned);
const _wasFollowing = nearBottom1200 && !readerUnpinned;

(scrollToBottom() clears _messageUserUnpinned at static/ui.js:5050, so a genuine follower stays pinned; only a manually-unpinned reader is spared.)

Also — the new test ossifies the bug: tests/test_sse_recovery_scroll_stranding.py:78 asserts the proximity-only guard, so it would let this regression pass. Please update it to assert the sticky-follow invariant: a reader unpinned-but-within-1200px must NOT be re-followed on recovery, while a genuinely pinned follower still is.

Everything else (the streaming-growth pinning) is on the right track. Re-gate on re-push. Thanks @allenliang2022.

[allenliang2022]

Good catch — you're right, the proximity-only guards would clobber a reader who scrolled up but stayed within 1200px. Fixed exactly as you specified.

All 4 SSE-recovery follow-guards are now sticky-aware (_handleStreamError, the Task-cancelled embedded-payload + fallback paths, and reconnect-stream-dead cleanup). Each now ANDs the proximity check with the sticky-unpin state:

const _wasFollowingAtX = ((typeof _isMessagePaneNearBottom==='function')
    ? _isMessagePaneNearBottom(1200)
    : true)
  && !((typeof _isMessageReaderUnpinned==='function')
    ? _isMessageReaderUnpinned()
    : (typeof _messageUserUnpinned!=='undefined' && _messageUserUnpinned));

Since scrollToBottom() clears _messageUserUnpinned, a genuine follower stays pinned across recovery, while a manually-unpinned reader within 1200px is now spared.

Test updated to assert the sticky invariant (the old proximity-only assertion ossified the bug, as you noted). tests/test_sse_recovery_scroll_stranding.py now:

• structurally requires every guard to consult _isMessageReaderUnpinned (with the _messageUserUnpinned fallback), not just _isMessagePaneNearBottom(1200);
• adds a behavioral test that extracts the real _wasFollowingAtDisconnect expression and evaluates it in Node across the four (nearBottom × unpinned) states, asserting: genuine follower (near-bottom, pinned) → re-follow true; unpinned-but-within-1200px → false (not re-followed); scrolled-away → false.

Also rebased onto current master (was BEHIND). Local: 39 passed across the new test file + the adjacent test_run_journal_frontend_static.py / test_issue4295_midstream_scroll_anchor.py / test_issue4856_android_scroll_regression.py; ruff forward-gate clean. The streaming-growth pinning in ui.js is unchanged.

Ready for re-gate. Pushed as b117c5aa. Thanks for the precise repro + fix.

[nesquena-hermes]

Shipped in v0.51.782. Thanks @allenliang2022! 🎉 Converged from my earlier bounce — the SSE-recovery follow-guards are now sticky-aware (a scrolled-up reader isn't yanked to bottom on reconnect). I fixed 2 Codex test-quality nits on the branch (assertion scoping + EOF); behavioral test + live-drive confirm the fix; no regression to #5253/#5260. Full suite green.