Skip to content

fix(platform): stabilize chat scroll and layout during streaming drain#611

Merged
larryro merged 3 commits into
mainfrom
fix/chat-streaming-scroll-stability
Mar 1, 2026
Merged

fix(platform): stabilize chat scroll and layout during streaming drain#611
larryro merged 3 commits into
mainfrom
fix/chat-streaming-scroll-stability

Conversation

@larryro
Copy link
Copy Markdown
Collaborator

@larryro larryro commented Mar 1, 2026

Summary

  • Replace boolean programmatic-scroll guard with a timestamp-based window (50ms) to handle browsers that fire multiple scroll events per scrollTo call, preventing auto-scroll from permanently breaking mid-stream
  • Freeze the stream buffer anchor position once drain begins, stopping StableMarkdown re-parse layout shifts that caused cumulative upward scrollbar drift
  • Unify StructuredMessage to always render via the mapped-sections path so the TypewriterText at key="section-0" survives when new sections (e.g. [[NEXT_STEPS]]) appear mid-stream — eliminates content flash from unmount/remount

Test plan

  • Added tests for anchor freeze during drain phase (use-stream-buffer.test.ts)
  • Added tests for scrollTo helper and programmatic scroll guard window (use-auto-scroll.test.ts)
  • Added test for TypewriterText survival when [[NEXT_STEPS]] appears during streaming (structured-message.test.tsx)
  • Updated existing shrinkage test to verify scroll-follow during typewriter drain
  • Manual: verify chat auto-scrolls smoothly during streaming and drain
  • Manual: verify scrolling away pauses auto-scroll and returning resumes it

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added programmatic scroll-to functionality to the chat interface
    • Improved streaming message rendering to prevent visual instability
  • Bug Fixes

    • Fixed layout shift issues during message streaming completion
    • Enhanced auto-scroll behavior when handling rapid content changes
  • Tests

    • Expanded test coverage for programmatic scrolling behavior
    • Added tests for streaming message rendering stability
    • Added tests for anchor position handling during message completion

Replace boolean programmatic-scroll guard with a timestamp-based window
to handle browsers firing multiple scroll events per scrollTo call. Freeze
the stream buffer anchor position once drain begins to prevent StableMarkdown
re-parse layout shifts. Unify StructuredMessage to always render via the
mapped-sections path so TypewriterText survives when new sections (e.g.
[[NEXT_STEPS]]) appear mid-stream.
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 1, 2026

📝 Walkthrough

Walkthrough

This PR refactors chat streaming to prevent component remounts and layout instability. It introduces a scrollTo method to the useAutoScroll hook with timestamp-based guards to distinguish programmatic from user scroll events. The structured-message component is refactored to consistently map over sections rather than conditionally choosing render paths, preventing TypewriterText remounts when new sections arrive. The use-stream-buffer hook adds anchor freezing during the drain phase to prevent layout shifts while the buffer exhausts. Supporting tests validate programmatic scrolling behavior and drain-phase anchor stability.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

The changes span six files with heterogeneous modifications: public API surface expansion for useAutoScroll, state management additions in use-stream-buffer with anchor freezing logic and timestamp guards, component rendering refactoring in structured-message, and significant test additions with new test patterns. The logic density in the hook implementations (timestamp-based guards, anchor ref synchronization, condition inversions) and cross-module dependencies require careful reasoning for each section.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly summarizes the main objective: stabilizing chat scroll and layout during streaming drain, which is directly reflected in the changes across auto-scroll, stream buffer anchor freezing, and StructuredMessage rendering unification.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/chat-streaming-scroll-stability

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@services/platform/app/features/chat/hooks/__tests__/use-stream-buffer.test.ts`:
- Around line 430-451: The test "anchor still advances normally during active
streaming" currently uses a non-strict assertion that allows a stalled anchor;
update the assertion to require a strict increase by replacing
expect(anchor2).toBeGreaterThanOrEqual(anchor1) with
expect(anchor2).toBeGreaterThan(anchor1). If flakiness arises, increase the
simulated time window used with advanceFrames (e.g., the second advanceFrames
call) so the streaming has enough frames to progress; locate this logic around
the useStreamBuffer hook test referencing result.current.anchorPosition and
advanceFrames.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7fb13eb and e357fcd.

📒 Files selected for processing (7)
  • services/platform/app/features/chat/components/chat-interface.tsx
  • services/platform/app/features/chat/components/structured-message/__tests__/structured-message.test.tsx
  • services/platform/app/features/chat/components/structured-message/structured-message.tsx
  • services/platform/app/features/chat/hooks/__tests__/use-stream-buffer.test.ts
  • services/platform/app/features/chat/hooks/use-stream-buffer.ts
  • services/platform/app/hooks/__tests__/use-auto-scroll.test.ts
  • services/platform/app/hooks/use-auto-scroll.ts

…ng streaming

The ResizeObserver was resetting programmaticScrollAtRef on every height
change, making the 50ms guard window perpetually active during streaming.
This prevented users from scrolling away while content was growing.

Remove the timestamp reset from the ResizeObserver callback — the guard
now only activates for explicit scrollToBottom/scrollTo/useLayoutEffect
calls. Also fix the guard test which passed at the threshold boundary
regardless of whether the guard was active, mock performance.now()
globally for deterministic test timing, and correct a stale comment
referencing the removed enabledRef behavior.
@larryro larryro merged commit feefb82 into main Mar 1, 2026
16 checks passed
@larryro larryro deleted the fix/chat-streaming-scroll-stability branch March 1, 2026 11:00
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