fix: re-pin auto-scroll in scrollIfPinned after user scrolls back to bottom

[luperrypf]

When a user scrolls up during streaming, _messageUserUnpinned is set true and scrollIfPinned() permanently stops auto-following — only scrollToBottom() clears the flag, but scrollIfPinned() is what runs on every streamed chunk.

Fix: Add a re-pin path in scrollIfPinned() that mirrors the scroll listener's _nearBottomCount debounce:

1. After 2 consecutive near-bottom (250px) streaming chunks
2. Without active scroll intent (_recentNonMessageScrollIntent, _recentMessageTouchScrollIntent)
3. Clear the unpinned flag and resume auto-follow

Also normalize the listener's re-pin threshold from 80px to 250px to match its own nearBottom gate, eliminating a dead zone (81-249px) where the counter could reach 2 but the re-pin would silently fail.

Before: one scroll-up → permanent auto-follow lockout
After: scroll back near bottom → 2 chunks confirm → auto-follow resumes

[greptile-apps]

Greptile Summary

This PR fixes a permanent auto-follow lockout where a single scroll-up during streaming would set _messageUserUnpinned=true and only scrollToBottom() (the explicit button) could clear it. The new re-pin path inside scrollIfPinned() mirrors the scroll listener's _nearBottomCount debounce: two consecutive near-bottom (≤250 px) streaming chunks without active scroll intent re-enable auto-follow.

• New scrollIfPinned() re-pin branch (lines 5519-5533): when _messageUserUnpinned is true, increments _nearBottomCount on each near-bottom chunk, resets it when the viewport is far away, and clears the unpinned flag after 2 consecutive qualifying chunks with no _recentNonMessageScrollIntent / _recentMessageTouchScrollIntent.
• Scroll-listener dead-zone fix (lines 4861-4865): removes the old inner if(!_messageUserUnpinned||bottomDistance<=80) guard, which silently blocked re-pin for scroll events that landed between 81–249 px from the bottom. nearBottom (requiring bottomDistance<250) already guarantees the threshold is met, so the guard was both dead and misleading.

Confidence Score: 5/5

Safe to merge — the change is a contained addition to scrollIfPinned() that correctly mirrors the scroll listener's existing debounce pattern, and the scroll listener's dead-zone removal is straightforwardly correct.

Both changed paths are tightly scoped to the scroll-pinning state machine. The new re-pin branch correctly guards all exit conditions and falls through to the existing auto-scroll call. No shared-state mutation outside the _nearBottomCount/_messageUserUnpinned/_scrollPinned trio, and the scroll listener itself serves as a safety net by re-setting _messageUserUnpinned if the user scrolls away again after an inadvertent re-pin.

static/ui.js — the new scrollIfPinned() re-pin block is the only area that warrants a careful read; the rest of the file is unchanged.

Important Files Changed

Filename	Overview
static/ui.js	Two-part fix: adds a re-pin path inside scrollIfPinned() for streaming chunks, and removes the vacuously-dead 80 px inner guard from the scroll listener. Logic is sound; the shared _nearBottomCount counter means the 2-chunk debounce can be triggered by 1 scroll event + 1 chunk (see existing thread). No new P1 issues identified.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["streaming chunk arrives\n→ scrollIfPinned()"] --> B{_autoScrollFollow?}
    B -- no --> Z[return — no-op]
    B -- yes --> C{_messageUserUnpinned?}

    C -- no --> D{_scrollPinned?}
    D -- no --> Z
    D -- yes --> E{_recentNonMessageScrollIntent?}
    E -- yes --> Z
    E -- no --> F{bottomDistance > 500?}
    F -- yes --> G[_setMessageScrollToBottom]
    F -- no --> H[_settleMessageScrollToBottom]
    G --> H

    C -- yes --> I{bottomDistance > 250?}
    I -- yes --> J["_nearBottomCount=0\nreturn"]
    I -- no --> K["_nearBottomCount++"]
    K --> L{count ≥ 2?}
    L -- no --> Z
    L -- yes --> M["_nearBottomCount=0"]
    M --> N{"_recentNonMessageScrollIntent\n|| _recentMessageTouchScrollIntent?"}
    N -- yes --> Z
    N -- no --> O["_messageUserUnpinned=false\n_scrollPinned=true\n→ re-pinned ✓"]
    O --> D

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["streaming chunk arrives\n→ scrollIfPinned()"] --> B{_autoScrollFollow?}
    B -- no --> Z[return — no-op]
    B -- yes --> C{_messageUserUnpinned?}

    C -- no --> D{_scrollPinned?}
    D -- no --> Z
    D -- yes --> E{_recentNonMessageScrollIntent?}
    E -- yes --> Z
    E -- no --> F{bottomDistance > 500?}
    F -- yes --> G[_setMessageScrollToBottom]
    F -- no --> H[_settleMessageScrollToBottom]
    G --> H

    C -- yes --> I{bottomDistance > 250?}
    I -- yes --> J["_nearBottomCount=0\nreturn"]
    I -- no --> K["_nearBottomCount++"]
    K --> L{count ≥ 2?}
    L -- no --> Z
    L -- yes --> M["_nearBottomCount=0"]
    M --> N{"_recentNonMessageScrollIntent\n|| _recentMessageTouchScrollIntent?"}
    N -- yes --> Z
    N -- no --> O["_messageUserUnpinned=false\n_scrollPinned=true\n→ re-pinned ✓"]
    O --> D

Loading

_{Reviews (2): Last reviewed commit: "fix: remove vacuous if-guard in listener..." | Re-trigger Greptile}

[greptile-apps]

_nearBottomCount is shared with the scroll listener's movedDown&&nearBottom path — the "2 chunks" debounce can be reached via mixed sources

The scroll listener (line 4859) also increments _nearBottomCount when the user physically scrolls down while near the bottom. This means the threshold of 2 can be reached by 1 scroll-down event + 1 streaming chunk, not strictly "2 consecutive near-bottom streaming chunks" as described. Concretely: user scrolls down once (listener → _nearBottomCount=1), next chunk arrives (scrollIfPinned() → _nearBottomCount=2) → re-pin fires after a single chunk. In the happy path (user is already near the bottom and not actively scrolling), this cooperative counting is fine, but the debounce semantics differ subtly from the PR description and could re-pin one chunk sooner than intended if a scroll event happens to coincide.

[luperrypf]

Good catch on both — addressed:

1. Shared counter: The code comment already says "the same _nearBottomCount debounce the scroll listener uses" — the shared counter is intentional. The PR description was slightly imprecise; the cooperative counting (scroll events + stream chunks sharing one counter) is the desired behavior.

2. Vacuous if: Removed in ccbfd9d. Now just the assignments with a comment explaining why it's unconditional.

[nesquena-hermes]

🔬 Gate certification — RED ⛔ (re-pins on mere near-bottom proximity → yanks a reading user back; breaks 3 scroll-pin regression tests — CORE)

Certified head: sha:ccbfd9d1 (rebased onto current master, git apply clean) · PR: #5544 · luperrypf, fix: re-pin auto-scroll in scrollIfPinned after user scrolls back to bottom
Verdict: The intent is good (one scroll-up shouldn't permanently kill auto-follow), but the re-pin trigger is too loose: it re-pins after 2 streamed chunks whenever within 250px of bottom, without requiring an actual return-to-bottom and without checking message-pane wheel/key intent. So a small scroll-up near the tail (while reading the last lines) gets yanked back to bottom mid-stream. Confirmed by 3 existing scroll-pin regression tests going red.

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

Gate	Result
Rebase onto current master	✅ git apply clean
Codex (reproduce)	SHIP-WITH-FIXES — 1 CORE (passive re-pin at 250px yanks reader); I confirmed + found backing test failures
Full pytest suite	5 failed / 11980 passed — 2 known env flakes (nous, issue4536) + 3 REAL scroll-pin regressions this PR causes

Findings

⛔ CORE (I + Codex CONFIRMED, + 3 regression tests) — streaming yanks a reader back to bottom after a small scroll-up near the tail (static/ui.js:5526): the new scrollIfPinned() branch clears _messageUserUnpinned after 2 streamed calls whenever _messageBottomDistance() <= 250, then _settleMessageScrollToBottom(false) writes bottom (ui.js:5441). It does NOT require an actual return-to-bottom, and it checks _recentMessageTouchScrollIntent()/_recentNonMessageScrollIntent() but OMITS the existing _recentMessageWheelIntent() (4504) and _recentMessageKeyScrollIntent() (4517) — so a wheel/keyboard scroll-up within 250px of the tail is overridden. Backing regression failures (this PR turns them red):

• test_issue4295_scroll_pin_reentry::test_near_bottom_proximity_alone_does_not_repin — asserts the EXACT invariant the PR violates (proximity alone must not re-pin).
• test_issue3250_upward_scroll_intent_window::test_scroll_if_pinned_respects_sticky_user_unpin — sticky-unpin broken.
• test_tars_scroll_reset_regressions::test_user_scroll_cancels_delayed_bottom_settling.
  Fix (Codex-exact): don't passively re-pin at the 250px near-bottom threshold — require an actual bottom return (strict <=80, matching the prior invariant), and bail/reset _nearBottomCount on recent message-pane wheel/key/scroll intent (_recentMessageWheelIntent(), _recentMessageKeyScrollIntent(), _recentMessageScrollIntent()) before incrementing.

Recommendation to the next agent / author

RED — gate-fail/changes-requested (1 CORE, breaks 3 scroll-pin regression tests): tighten the re-pin to require an actual return-to-bottom (<=80, not 250px near-bottom) and add the missing _recentMessageWheelIntent() + _recentMessageKeyScrollIntent() bails so a wheel/keyboard scroll-up near the tail isn't overridden. The goal (resume auto-follow when the user genuinely returns to bottom) is worth shipping — but it must not re-pin on mere proximity (the #4295 proximity-alone-does-not-repin invariant) or fight a wheel/key reader. Once tightened, the 3 red regression tests should pass and it's a real UX win. Then, since it's visible scroll behavior, Nathan's UX call on the exact resume threshold. concept 4/5 (real annoyance fix; the trigger is too eager). Author @luperrypf (T2). crit=3. (Gate value: the full suite caught it directly — 3 prior-scroll-fix regression tests (#3250/#4295/tars) encode the "proximity alone does not re-pin" invariant this PR breaks; always run the full suite on scroll-behavior changes, the regressions are pre-encoded.)

---

_Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; the passive-re-pin-at-250px CORE confirmed (ui.js:5526 clears _messageUserUnpinned on proximity without return-to-bottom, omits _recentMessageWheelIntent/recentMessageKeyScrollIntent), backed by 3 failing scroll-pin regression tests (#4295 proximity-alone-does-not-repin, #3250 sticky-unpin, tars delayed-settle); 2 other failures are known nous/issue4536 flakes. Cert valid for sha:ccbfd9d1.

[nesquena-hermes]

Shipped in v0.51.862 — thanks @luperrypf! 🎉

Your auto-scroll re-pin fix is live. The permanent auto-follow lockout is gone: once you've scrolled up during a streaming reply, returning to the true bottom of the transcript (within ~80px) now re-engages auto-scroll so the view follows the stream again — previously only starting a new turn re-enabled it.

The re-pin is deliberately conservative: it only fires at the true-bottom tail and bails on any active scroll intent (wheel / keyboard / touch / scrollbar drag), so reading back through history mid-stream never yanks you down — the #4295 invariant is preserved. The listener's <=80px true-bottom gate is restored too, so the two re-pin paths and the ↓-button all use one threshold.

Gate: rebuilt on current master and re-gated — Codex clean (regression), Fable SHIP-UX (matches ChatGPT's scroll-away-during-stream behavior; the 80px resume is the right call), 20/20 scroll-regression tests, full suite green, plus a live browser drive on a seeded 40-message session that confirmed both behaviors (no yank while reading, follow re-engages at bottom) with zero console errors. Merged via release PR #5580 with your authorship preserved.

Appreciate the sharp fix to a genuinely annoying regression.