Skip to content

fix(frontend): stop auto-scroll from hijacking scrollback during streaming - #152

Closed
lucastononro wants to merge 5 commits into
fix/98-memoize-chatfrom
fix/99-autoscroll
Closed

fix(frontend): stop auto-scroll from hijacking scrollback during streaming#152
lucastononro wants to merge 5 commits into
fix/98-memoize-chatfrom
fix/99-autoscroll

Conversation

@lucastononro

@lucastononro lucastononro commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • The chat pane's auto-scroll effect called bottomRef.current?.scrollIntoView({behavior:'smooth'}) on every chatItems change, including every single agent_token during streaming — many times per second. That yanked the view back to the bottom constantly and made it impossible to scroll up and read earlier output mid-run.

Changes

  • src/app/page.tsx — added chatScrollRef (ref to the actual overflow-y-auto scrollable chat pane) and pinnedToBottomRef, updated by a new onScroll handler (handleChatScroll) using a distance-from-bottom threshold (AUTO_SCROLL_PIN_THRESHOLD_PX = 96).
  • The auto-scroll effect now bails out entirely when the user isn't pinned near the bottom, and uses behavior: 'auto' (no animation) while a bubble is actively streaming (streamingItemIdRef.current set) vs. 'smooth' once streaming settles.
  • addItem re-pins to bottom whenever the item is a user message — if the user scrolled up to read history and then sends a new message, the view still follows them down, matching normal chat UX.
  • resetSessionState resets the pin ref to true on session switch, so a freshly loaded session starts pinned like before.

Test plan

Existing (must still work)

  • Streaming chat: send a message and watch the assistant reply stream in — while pinned to the bottom (the common case), the view still follows the stream smoothly/without jank.
  • Live metrics tab, file/image/PDF viewing, notebook updates — untouched by this change, quick smoke check since it's the same page component.
  • Switching sessions/experiments — chat pane starts pinned to bottom on load, same as before.

New

  • During a long streaming response, scroll up to read an earlier message — confirm the view stays put and does not get yanked back down on subsequent tokens.
  • While scrolled up mid-stream, send a new message (or wait for the stream to finish) — confirm the view returns to the bottom (re-pinned on send / still pinned once you scroll back down yourself).
  • Scroll back down near the bottom (within ~96px) during streaming — confirm auto-scroll resumes following new tokens.

Closes #99

🤖 Generated with Claude Code

Greptile Summary

This PR fixes the chat pane's auto-scroll behaviour during streaming by replacing the unconditional scrollIntoView({behavior:'smooth'}) on every chatItems change with a pin-to-bottom mechanism: a pinnedToBottomRef flag gated by a 96px distance threshold, updated by a handleChatScroll listener, and reset to true only when the user sends a message or switches sessions.

  • Auto-scroll guard (page.tsx): the useEffect now bails out immediately when pinnedToBottomRef.current is false, and uses behavior: 'auto' unconditionally to avoid the mid-smooth-animation un-pin race that the previous thread identified.
  • Re-pin on send (addItem): pinnedToBottomRef.current is set to true before setChatItems for user-type items, so the first streaming token always lands visible after a send.
  • Session reset (resetSessionState): resets the pin ref to true so a freshly loaded session starts scrolled to the bottom.
  • Manual test guide (docs/manual-tests/chat-autoscroll-pinning.md): comprehensive checklist covering the baseline, scrollback, re-pin-by-scroll, send-re-pin, and threshold scenarios.

Confidence Score: 5/5

Safe to merge — the change is well-scoped to the chat scroll behaviour and all edge cases (smooth-scroll race, session switch, send re-pin) are correctly handled.

The pin-to-bottom logic is implemented correctly: behavior:'auto' avoids the mid-animation un-pin race, pinnedToBottomRef is reset in all the right places (user send, session switch), and the handleChatScroll distance formula is standard and accurate. The two issues flagged in the previous review round are both resolved in this PR. No other defects found.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
frontend/src/app/page.tsx Implements pin-to-bottom scroll logic: chatScrollRef/pinnedToBottomRef refs, handleChatScroll callback, guarded useEffect, and re-pin on user send and session reset. Both previously flagged issues (module-level constant, behavior:'auto' to avoid mid-animation un-pin) are correctly resolved.
docs/manual-tests/chat-autoscroll-pinning.md New manual test tutorial covering five scenarios: pinned follow, scrollback not hijacked, re-pin by scrolling back down, send re-pin (with note about the smooth-scroll race fix), and threshold sanity. Well-structured and actionable.
STAGING-v0.0.5.md Staging log entry added for PR #150 (the parent branch memoize-chat fix). Metadata-only change; no logic affected.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["chatItems changes (new token / new item)"] --> B{pinnedToBottomRef.current?}
    B -- false --> C[bail out - view stays put]
    B -- true --> D["scrollIntoView behavior:'auto' instant jump to bottomRef"]
    D --> E[scroll event fires on chatScrollRef]
    E --> F["handleChatScroll: distanceFromBottom = scrollHeight - scrollTop - clientHeight"]
    F --> G{"distanceFromBottom <= 96px?"}
    G -- yes --> H[pinnedToBottomRef = true]
    G -- no --> I[pinnedToBottomRef = false]
    J[User scrolls up manually] --> F
    K["User sends message addItem type='user'"] --> L[pinnedToBottomRef = true then setChatItems]
    L --> A
    M[Session switch resetSessionState] --> N[pinnedToBottomRef = true chatItems cleared]
    N --> A
Loading

Reviews (3): Last reviewed commit: "merge: staging-v0.0.5 into fix/99-autosc..." | Re-trigger Greptile

…aming

bottomRef.scrollIntoView({behavior:'smooth'}) fired on every chatItems
change, including every agent_token during streaming, yanking the view
back to the bottom and making it impossible to read earlier output.
Track whether the user is pinned near the bottom via a scroll listener +
threshold and only auto-scroll while pinned; use behavior:'auto' while a
bubble is actively streaming, and re-pin when the user sends a message.

Closes #99

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Comment thread frontend/src/app/page.tsx
Comment thread frontend/src/app/page.tsx Outdated
- P1: use behavior 'auto' unconditionally in the auto-scroll effect. A
  smooth scroll after send re-pinned and then animated through
  intermediate positions whose scroll events un-pinned the view
  mid-animation, so fast first tokens could leave the stream unfollowed.
- P2: hoist AUTO_SCROLL_PIN_THRESHOLD_PX to module level so the constant
  is created once and is stable for the scroll-handler closure.
- Add a manual test tutorial for the scroll-pinning behavior (no frontend
  unit-test harness exists, and jsdom cannot reproduce smooth-scroll
  animation events anyway).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
@lucastononro

Copy link
Copy Markdown
Owner Author

Addressed Greptile review findings in 05cb539:

Fixed

  • [P1] page.tsx — smooth scroll during addItem('user') can briefly unpin the view: valid. The re-pin + scrollIntoView({behavior: 'smooth'}) animated through intermediate positions whose scroll events made handleChatScroll compute distanceFromBottom > 96 and un-pin mid-flight; if first streaming tokens arrived before the animation landed, auto-follow silently stopped. Fixed with the finding's simplest suggestion: behavior: 'auto' unconditionally — an instant jump fires a single scroll event already at the bottom, keeping pin state consistent. (The streamingItemIdRef-conditional smooth nudge is gone; the aesthetic cost is minor vs. the race.)
  • [P2] page.tsx — AUTO_SCROLL_PIN_THRESHOLD_PX defined in component body: valid. Hoisted to a module-level constant.

Dismissed: none — both findings were valid.

Tests

  • The frontend has no unit-test harness (no vitest/jest), and the P1 failure mode is a real-browser smooth-scroll-animation race that jsdom cannot reproduce, so instead of bolting on a harness that couldn't exercise the bug, I added a manual test tutorial: docs/manual-tests/chat-autoscroll-pinning.md (5 scenarios: pinned follow, scrollback not hijacked, re-pin on scroll-down, instant re-pin on send + follow under fast first tokens, ~96px threshold sanity).

Validation: npm ci, npx tsc --noEmit, npm run lint, npm run build — all pass.

Conflict in page.tsx: #152's handleChatScroll pin-tracking callback and
staging's #169 suggested-prompt effect appended at the same anchor — kept
both.
lucastononro added a commit that referenced this pull request Jul 29, 2026
lucastononro added a commit that referenced this pull request Jul 29, 2026
@lucastononro

Copy link
Copy Markdown
Owner Author

Merged into integration branch staging-v0.0.5 (see the STAGING-v0.0.5.md ledger on that branch for merge order, Greptile follow-ups and test results). These changes will land on main via the staging merge — closing to clear the queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant