Fix pinned streaming scroll restoration

[santastabber]

Summary

• preserve tail-relative scroll position for pinned/following transcript snapshots before semantic viewport-anchor restoration
• keep semantic viewport-anchor restoration for explicitly unpinned/manual reader positions
• add a focused regression lock for pinned streaming restore ordering

Why

Live activity/worklog DOM updates can rebuild rows while a response is streaming. When the reader is pinned to the live tail, restoring the first visible viewport anchor can remount an older row and yank long transcripts upward. Pinned snapshots should instead preserve bottom distance; manual reading positions should keep the existing semantic-anchor behavior.

Verification

• node -c static/ui.js
• git diff --check
• ./scripts/test.sh tests/test_tars_scroll_reset_regressions.py tests/test_issue1690_scroll_completion.py tests/test_issue4295_scroll_pin_reentry.py tests/test_streaming_sidebar_scroll.py tests/test_issue4006_auto_scroll_follow_default.py tests/test_live_to_final_anchor_visible_order.py -q

[greptile-apps]

Greptile Summary

This PR fixes a streaming scroll regression where live DOM rebuilds (Worklog/activity rows) could restore the semantic viewport anchor and yank a pinned/following transcript upward. It introduces _restorePinnedMessageScrollSnapshot, a dedicated helper that short-circuits both _restoreMessageScrollSnapshot and _restoreMessageScrollSnapshotSameFrame for pinned, non-user-unpinned snapshots, using tail-relative bottom distance instead of the semantic anchor.

• Adds _restorePinnedMessageScrollSnapshot that computes maxTop - bottom and sets all pin-tracking globals (_scrollPinned, _messageUserUnpinned, _nearBottomCount), then early-returns from both restore paths before the anchor logic runs.
• Updates three test files to inject the new function into Node.js eval harnesses, and adds a targeted ordering regression test in test_tars_scroll_reset_regressions.py.

Confidence Score: 4/5

Safe to merge; the fix is logically sound and the test coverage directly validates the ordering invariant. One stale code block in _restoreMessageScrollSnapshotSameFrame could mislead future contributors but doesn't change runtime behavior for the cases this PR touches.

The new helper correctly uses tail-relative bottom distance for pinned snapshots and is consistently injected into both restore paths. The stale if(snapshot.pinned===true) block left in _restoreMessageScrollSnapshotSameFrame is dead for the common pinned case and could confuse future edits, but produces no incorrect behavior now.

static/ui.js around lines 11443–11451 in _restoreMessageScrollSnapshotSameFrame — the pin-state block is effectively dead for ordinary pinned snapshots after the early-return guard and warrants a cleanup or clarifying comment.

Important Files Changed

Filename	Overview
static/ui.js	Adds _restorePinnedMessageScrollSnapshot and inserts it as an early-return guard in both restore functions; the pinned-state block inside _restoreMessageScrollSnapshotSameFrame is now dead code for the main pinned case.
tests/test_tars_scroll_reset_regressions.py	Adds test_pinned_preserve_scroll_uses_bottom_distance_before_viewport_anchor which checks guard string presence and call-ordering invariants; looks correct.
tests/test_issue4295_midstream_scroll_anchor.py	Injects _restorePinnedMessageScrollSnapshot into two test harnesses so _restoreMessageScrollSnapshot calls resolve correctly; straightforward dependency update.
tests/test_issue500_message_list_virtualization.py	Adds _restorePinnedMessageScrollSnapshot eval before _restoreMessageScrollSnapshotSameFrame; test snapshot uses pinned: false so the new guard path is not exercised, but the dependency is correctly threaded.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[DOM rebuild triggers restore] --> B{_restoreMessageScrollSnapshot\nor SameFrame variant}
    B --> C[_restorePinnedMessageScrollSnapshot]
    C --> D{snapshot.pinned === true\nAND userUnpinned !== true?}
    D -- YES --> E[Compute target = maxTop - bottom\nSet scrollTop tail-relative\nSync _lastScrollTop\nSet _scrollPinned=true\n_nearBottomCount=2\n_deferClearProgrammaticScroll]
    E --> F[return true → caller returns early]
    D -- NO --> G[Semantic viewport anchor restore\n_restoreMessageViewportAnchor]
    G --> H{Anchor found in DOM?}
    H -- YES --> I[Anchor-based scrollTop\nSet pin state from snapshot]
    H -- NO --> J[Try _remountMessageViewportAnchor\nfallback to snapshot.top]
    J --> I

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[DOM rebuild triggers restore] --> B{_restoreMessageScrollSnapshot\nor SameFrame variant}
    B --> C[_restorePinnedMessageScrollSnapshot]
    C --> D{snapshot.pinned === true\nAND userUnpinned !== true?}
    D -- YES --> E[Compute target = maxTop - bottom\nSet scrollTop tail-relative\nSync _lastScrollTop\nSet _scrollPinned=true\n_nearBottomCount=2\n_deferClearProgrammaticScroll]
    E --> F[return true → caller returns early]
    D -- NO --> G[Semantic viewport anchor restore\n_restoreMessageViewportAnchor]
    G --> H{Anchor found in DOM?}
    H -- YES --> I[Anchor-based scrollTop\nSet pin state from snapshot]
    H -- NO --> J[Try _remountMessageViewportAnchor\nfallback to snapshot.top]
    J --> I

Loading

Comments Outside Diff (1)

1. static/ui.js, line 11443-11451 (link)

   Dead pin-state block after the new early return

   The if(snapshot.pinned===true) branch at line 11443 is now unreachable for the normal pinned case (pinned===true && userUnpinned!==true) because _restorePinnedMessageScrollSnapshot handles that path and returns early. The only way this block executes today is the unusual combination pinned===true && userUnpinned===true, in which case it takes the if(snapshot.pinned===true) arm and sets _messageUserUnpinned=false, silently overriding the user's explicit unpin signal. A future developer modifying this branch expecting it to run for ordinary pinned snapshots will be surprised. Consider either removing the now-dead pinned===true arm or adding a comment documenting that only the userUnpinned===true else-if is live for callers that reach this point.

_{Reviews (1): Last reviewed commit: "Fix pinned stream scroll restoration" | Re-trigger Greptile}

[nesquena-hermes]

Shipped in v0.51.725 🎉 — thanks @santastabber. Pinned/following readers now keep their tail-relative scroll position when a streaming turn rebuilds its rows, instead of getting bounced to an older row; unpinned/manual positions still use the semantic anchor restore. Full gate clean (Codex regression-safe + Opus ship-able + full suite + 49 scroll/virtualization regression tests). A small follow-up to remove the now-dead pinned fallback branches is noted for later cleanup.