release #5544: re-pin auto-scroll at true bottom after scroll-to-bottom

[nesquena-hermes]

Release: #5544 — re-pin auto-scroll at true bottom after scroll-to-bottom

Ships @luperrypf's fix for a crown-jewel chat auto-scroll regression, rebuilt on current master and re-gated.

The bug (#5544)

A single scroll-up while a response was streaming set _messageUserUnpinned=true, and scrollIfPinned() then permanently stopped auto-follow — only an explicit scrollToBottom() cleared it, so scrolling back to the bottom didn't re-engage following. Permanent auto-follow lockout for the rest of the turn.

The fix

scrollIfPinned() now re-pins auto-follow, but only when the reader has genuinely reached the true-bottom tail (<=80px) and shows no active scroll intent (wheel / keyboard / touch / non-message / scrollbar-drag), reusing the scroll listener's _nearBottomCount>=2 debounce. Proximity alone (the ~250px nearBottom band) never re-pins — that is the #4295 invariant (a reader scanning the last lines mid-stream must not be yanked down). The listener's <=80px true-bottom gate is also restored, so the two re-pin paths and the ↓-button show condition all use one threshold.

Gate (all green)

• Codex (regression): the only findings were stale-base artifacts (a prior stage was behind master); this branch is rebuilt on current master and the real diff is clean.
• Fable UX: SHIP-UX — reuses the existing intent-helper family + listener debounce, unifies three thresholds into one, matches ChatGPT's scroll-away-during-stream behavior; the 80px true-bottom resume is the right call.
• Live browser drive (seeded 40-msg session, real scroll events): scroll-up during stream does not yank the reader (bug(chat): viewport jumps upward while reading an in-progress streamed reply #4295 holds); returning to true-bottom re-engages follow (view followed content growth to bottom); zero JS console errors.
• Scroll-regression family: 20/20 (incl. test_near_bottom_proximity_alone_does_not_repin). Full suite: running on this PR.

Attribution: original author @luperrypf (Co-authored-by trailer preserved).

Closes #5544

[greptile-apps]

Greptile Summary

This release PR ships the #5544 fix: a single scroll-up during streaming permanently locked out auto-follow because scrollIfPinned() had an unconditional return on _messageUserUnpinned, and only an explicit scrollToBottom() click could clear it. The fix adds a re-pin path inside scrollIfPinned() gated by the true-bottom threshold (≤80px), a full set of scroll-intent guards (wheel/key/touch/scrollbar/non-message), and the existing _nearBottomCount≥2 debounce — preserving the #4295 invariant that proximity alone never re-pins.

• static/ui.js: scrollIfPinned() replaces the unconditional _messageUserUnpinned bail with a conditional re-pin block; the scroll listener's movedDown&&nearBottom branch gets the same ≤80px true-bottom gate added explicitly for the _messageUserUnpinned=true case; the scroll-to-bottom button's show condition was already at the same >80px threshold, so all three gates are now unified.
• Tests: Two existing tests are updated to assert the new per-guard invariants rather than the old "bail unconditionally" assertion, and a new near-bottom-proximity-alone-must-not-repin test was added (referenced in PR description as passing 20/20).

Confidence Score: 4/5

Safe to merge; the re-pin logic is correctly gated and the #4295 invariant is preserved end-to-end.

The fix is well-reasoned and well-tested. The one notable nuance is that _nearBottomCount is shared between the scroll listener's near-bottom debounce and the new re-pin accumulation in scrollIfPinned() — the scroll listener can reset the counter mid-accumulation, making the effective debounce occasionally cost more than 2 ticks. This doesn't break correctness (each reset corresponds to a genuine viewport-state change), but a dedicated counter would make the two uses independent and the semantics unambiguous.

static/ui.js — specifically the interaction between the scroll listener's _nearBottomCount reset path and the new scrollIfPinned() re-pin accumulation.

Important Files Changed

Filename	Overview
static/ui.js	Core fix: adds a guarded re-pin block in scrollIfPinned() and adds the ≤80px true-bottom gate to the scroll listener's movedDown+nearBottom branch. Logic is sound and well-guarded; minor concern that _nearBottomCount is shared between the scroll listener debounce path and the new scrollIfPinned() re-pin accumulation path.
tests/test_tars_scroll_reset_regressions.py	test_user_scroll_cancels_delayed_bottom_settling updated to assert the new per-guard invariants (_messageBottomDistance()>80, wheel/key intent checks) instead of the old unconditional bail; assertions are correct and complete.
tests/test_issue3250_upward_scroll_intent_window.py	test_scroll_if_pinned_respects_sticky_user_unpin updated from checking the old unconditional return to checking the new explicit block and its true-bottom + intent guards; structurally correct.
CHANGELOG.md	Release-authored CHANGELOG entry for #5544 — accurate description of the regression and fix, appropriate for this release PR.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["scrollIfPinned() called"] --> B{_autoScrollFollow?}
    B -- No --> Z[return]
    B -- Yes --> C{_messageUserUnpinned?}
    C -- No --> G{_scrollPinned?}
    C -- Yes --> D{Any recent scroll intent?\nwheel / key / touch /\nscrollbar / non-message}
    D -- Yes --> E["_nearBottomCount = 0\nreturn — re-pin suppressed"]
    D -- No --> F{_messageBottomDistance > 80px?}
    F -- Yes --> E2["_nearBottomCount = 0\nreturn — not at true bottom"]
    F -- No --> H["_nearBottomCount++"]
    H --> I{_nearBottomCount >= 2?}
    I -- No --> Z2["return — debounce accumulating"]
    I -- Yes --> J["_nearBottomCount = 0\n_messageUserUnpinned = false\n_scrollPinned = true\n✅ RE-PINNED"]
    J --> G
    G -- No --> Z3[return]
    G -- Yes --> K{_recentNonMessageScrollIntent?}
    K -- Yes --> Z4[return]
    K -- No --> L{_messageBottomDistance > 500px?}
    L -- Yes --> M[_setMessageScrollToBottom]
    M --> N[_settleMessageScrollToBottom]
    L -- No --> N

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["scrollIfPinned() called"] --> B{_autoScrollFollow?}
    B -- No --> Z[return]
    B -- Yes --> C{_messageUserUnpinned?}
    C -- No --> G{_scrollPinned?}
    C -- Yes --> D{Any recent scroll intent?\nwheel / key / touch /\nscrollbar / non-message}
    D -- Yes --> E["_nearBottomCount = 0\nreturn — re-pin suppressed"]
    D -- No --> F{_messageBottomDistance > 80px?}
    F -- Yes --> E2["_nearBottomCount = 0\nreturn — not at true bottom"]
    F -- No --> H["_nearBottomCount++"]
    H --> I{_nearBottomCount >= 2?}
    I -- No --> Z2["return — debounce accumulating"]
    I -- Yes --> J["_nearBottomCount = 0\n_messageUserUnpinned = false\n_scrollPinned = true\n✅ RE-PINNED"]
    J --> G
    G -- No --> Z3[return]
    G -- Yes --> K{_recentNonMessageScrollIntent?}
    K -- Yes --> Z4[return]
    K -- No --> L{_messageBottomDistance > 500px?}
    L -- Yes --> M[_setMessageScrollToBottom]
    M --> N[_settleMessageScrollToBottom]
    L -- No --> N

Loading

_{Reviews (1): Last reviewed commit: "release #5544: re-pin auto-scroll at tru..." | Re-trigger Greptile}

[greptile-apps]

_nearBottomCount is used as a debounce counter by both the scroll listener's movedDown&&nearBottom branch and by the new re-pin block in scrollIfPinned(). The scroll listener resets the counter to 0 whenever it fires in the movedDown&&nearBottom path and _nearBottomCount reaches 2 without re-pinning (i.e. _messageUserUnpinned=true and bottomDistance>80). This can reset the accumulation that scrollIfPinned() started — making the effective debounce cost more than 2 ticks whenever a scroll event fires between the first and second scrollIfPinned() call at true-bottom. This doesn't break correctness (the reset is sensible: if content grew enough to push the viewport above 80px between the two calls, delaying re-pin is right), but a dedicated counter for the scrollIfPinned() path would decouple the two uses and make the debounce semantics unambiguous.

Suggested change

	if(_recentNonMessageScrollIntent()||_recentMessageScrollIntent()||_recentMessageTouchScrollIntent()||_recentMessageWheelIntent()||_recentMessageKeyScrollIntent()){ _nearBottomCount=0; return; }
	if(_messageBottomDistance()>80){ _nearBottomCount=0; return; }
	_nearBottomCount=_nearBottomCount+1;
	if(_nearBottomCount<2) return;
	_nearBottomCount=0;
	if(_recentNonMessageScrollIntent()||_recentMessageScrollIntent()||_recentMessageTouchScrollIntent()||_recentMessageWheelIntent()||_recentMessageKeyScrollIntent()){ _repinCount=0; return; }
	if(_messageBottomDistance()>80){ _repinCount=0; return; }
	_repinCount=(_repinCount||0)+1;
	if(_repinCount<2) return;
	_repinCount=0;

[nesquena]

Review — end-to-end ✅ (clean approval, no fixes needed)

Independent review of the #5544 fix (@luperrypf's, rebuilt on master): a single scroll-up during a streaming response set _messageUserUnpinned=true, and since only an explicit scrollToBottom() ever cleared it, scrollIfPinned() permanently stopped auto-follow for the rest of the turn — scrolling back to the bottom didn't re-engage following.

What this ships

static/ui.js (+~28 — a re-pin path in scrollIfPinned() + the restored <=80px true-bottom gate in the scroll listener), tests/test_issue3250_upward_scroll_intent_window.py / tests/test_tars_scroll_reset_regressions.py (+25/-3), CHANGELOG.md. Agent-authored (nesquena-hermes), original author credited via Co-authored-by. MERGEABLE, CI fully green.

End-to-end trace — the re-pin is correctly conservative

This is crown-jewel scroll behavior, so the load-bearing question is whether the new re-pin can ever fire against the reader's will (the #4295 invariant: proximity alone must never yank a reader scanning the last lines mid-stream). Traced the gate order in scrollIfPinned():

1. Any recent scroll intent bails first — all five intent helpers (_recentNonMessageScrollIntent / _recentMessageScrollIntent / _recentMessageTouchScrollIntent / _recentMessageWheelIntent / _recentMessageKeyScrollIntent; I verified each is defined in ui.js) → reset debounce, return. An active scroll-up near the tail is never overridden. ✓
2. True bottom required: _messageBottomDistance() > 80 → reset debounce, return. The ~250px nearBottom band alone can never re-pin. ✓
3. Debounced: requires two consecutive at-bottom observations (_nearBottomCount >= 2), the same debounce the scroll listener uses — a single transient bottom touch doesn't re-engage. ✓
4. Only then: _messageUserUnpinned=false; _scrollPinned=true — and the pre-existing pinned-path guards (_recentNonMessageScrollIntent, >500px jump) still apply downstream. ✓

The listener-side gate is unified to the same threshold: if(!_messageUserUnpinned || bottomDistance<=80) — when unpinned, re-affirming the pin now also requires true bottom, so both re-pin paths and the ↓-button share one <=80px definition instead of three drifting thresholds. ✓

Sharing _nearBottomCount between the listener and scrollIfPinned() is semantically consistent (both count "consecutive at-bottom observations", and each resets it on any disqualifying state), so the debounce can't be satisfied by stale counts. ✓

#4295 invariant — structurally pinned

test_near_bottom_proximity_alone_does_not_repin is in the passing family, and the PR's stage ran a live browser drive (seeded 40-message session, real wheel/scroll events): scroll-up during stream does not yank (#4295 holds), returning to true bottom re-engages follow, zero console errors. The static gates I traced match that observed behavior.

Tests

• Targeted scroll-regression files — 17/17; the crown-jewel scroll family — 270 passed, 4 skipped.
• Fail-without-fix verified independently: against master's ui.js, 2 of the updated assertions fail — load-bearing.
• node --check static/ui.js clean.
• Full suite: 11820 passed / 0 failed in 311s (deselected the known darwin CRLF flake). CI fully green (lint + browser-smoke + all shards).

Minor observations (non-blocking)

• scrollIfPinned() increments the shared debounce on render ticks while the listener increments on scroll events — mixed sources can reach the threshold one tick sooner than two scroll events would. Both sources genuinely observe "still at true bottom with no intent", so this is within the debounce's meaning; flagging only for awareness.
• The CHANGELOG [Unreleased] entry follows convention.

Recommendation

✅ Approved clean. Parked at approval — ready for the release agent's merge/tag pipeline.

The fix restores bottom-re-engagement without touching the #4295 guarantee: re-pin requires true bottom (≤80px) and no scroll intent of any kind and a two-observation debounce — proximity alone still never re-pins, and the three thresholds are unified into one. Mutation-verified tests, 270-test family green, live-drive evidence consistent with the trace, full suite clean. Ship.