Fix chat scroll jitter and reduce viewport slack space - #363
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 22 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a CSS rule enforcing auto scroll behavior on Changes
Sequence DiagramsequenceDiagram
participant Thread as Thread Component
participant State as thread.isRunning
participant ScrollOnRunEnd
participant RAF as requestAnimationFrame
participant Viewport as ThreadPrimitive.Viewport
Thread->>ScrollOnRunEnd: Render with viewportRef
ScrollOnRunEnd->>State: useAuiState() monitors isRunning
Note over ScrollOnRunEnd: Tracks prior running state with useRef
State->>ScrollOnRunEnd: isRunning: true → false (transition)
ScrollOnRunEnd->>ScrollOnRunEnd: Detect state change (running to idle)
ScrollOnRunEnd->>RAF: Schedule scroll callback
RAF->>Viewport: scrollTo(scrollHeight, behavior: "instant")
Viewport-->>ScrollOnRunEnd: Scroll complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR addresses two issues: CSS smooth-scroll jitter during assistant-ui streaming (fixed by overriding
Confidence Score: 4/5Safe to merge for the scroll-jitter fix, but the slack-reduction feature is broken and needs correction before the intended behavior is achieved. One P1 finding: the fillClamp props are on the wrong component (MessagePrimitive.Root instead of ThreadPrimitive.ViewportSlack), so the primary slack-reduction goal of the PR won't take effect. The CSS fix is correct and independent, but since half the stated intent of this PR is silently a no-op, the developer should address it before merging. src/components/assistant-ui/thread.tsx — AssistantMessage component needs ThreadPrimitive.ViewportSlack rendered inside it instead of fillClamp props on MessagePrimitive.Root. Important Files Changed
Reviews (1): Last reviewed commit: "Fix chat viewport scroll jitter and redu..." | Re-trigger Greptile |
| const AssistantMessage: FC = () => { | ||
| return ( | ||
| <MessagePrimitive.Root asChild> | ||
| <MessagePrimitive.Root asChild fillClampThreshold="0px" fillClampOffset="20em"> |
There was a problem hiding this comment.
Props on wrong component — slack reduction has no effect
fillClampThreshold and fillClampOffset are props of ThreadPrimitive.ViewportSlack, not MessagePrimitive.Root. Placing them here means the library never sees them in the right context: since asChild is used, they'll be forwarded as unknown attributes to the underlying <div> and silently ignored. The viewport slack will remain at its default value.
To actually reduce the slack space, ThreadPrimitive.ViewportSlack needs to be rendered as a child inside the assistant message (the library docs say "ViewportSlack is needed on the last assistant message"):
// Inside AssistantMessage, e.g. after the footer div:
<ThreadPrimitive.ViewportSlack fillClampThreshold="0px" fillClampOffset="20em">
<div /> {/* children is required */}
</ThreadPrimitive.ViewportSlack>And MessagePrimitive.Root should revert to its original signature:
| <MessagePrimitive.Root asChild fillClampThreshold="0px" fillClampOffset="20em"> | |
| <MessagePrimitive.Root asChild> |
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/assistant-ui/thread.tsx">
<violation number="1">
P3: This increases the last user message's bottom margin and adds extra blank space below the thread.</violation>
<violation number="2" location="src/components/assistant-ui/thread.tsx:1149">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
The PR description claims these edits reduce slack space, but the changed margins increase it instead, so the stated behavior is not implemented.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| </MessagePrimitive.Error> | ||
| ); | ||
| }; | ||
|
|
There was a problem hiding this comment.
P3: This increases the last user message's bottom margin and adds extra blank space below the thread.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/assistant-ui/thread.tsx, line 1275:
<comment>This increases the last user message's bottom margin and adds extra blank space below the thread.</comment>
<file context>
@@ -1272,7 +1272,7 @@ const UserMessage: FC = () => {
<MessagePrimitive.Root asChild>
<div
- className="aui-user-message-root mx-auto grid w-full max-w-[var(--thread-max-width)] animate-breathe-in auto-rows-auto grid-cols-[minmax(72px,1fr)_auto] gap-y-2 px-2 pt-4 pb-1 last:mb-2 [&:where(>*)]:col-start-2"
+ className="aui-user-message-root mx-auto grid w-full max-w-[var(--thread-max-width)] animate-breathe-in auto-rows-auto grid-cols-[minmax(72px,1fr)_auto] gap-y-2 px-2 pt-4 pb-1 last:mb-5 [&:where(>*)]:col-start-2"
data-role="user"
>
</file context>
| className="aui-user-message-root mx-auto grid w-full max-w-[var(--thread-max-width)] animate-breathe-in auto-rows-auto grid-cols-[minmax(72px,1fr)_auto] gap-y-2 px-2 pt-4 pb-1 last:mb-2 [&:where(>*)]:col-start-2" |
There was a problem hiding this comment.
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 `@src/components/assistant-ui/thread.tsx`:
- Around line 134-141: The effect currently forces a scroll-to-bottom on every
wasRunningRef.current && !isRunning transition which pulls users back when they
manually scrolled up; modify the useEffect that references wasRunningRef,
isRunning, and viewportRef so it computes the current distance from bottom (use
viewportRef.current.scrollHeight, scrollTop, clientHeight) and only call
scrollTo when that distance is within a small threshold (e.g., 40–100px)
indicating the user was already at/near the bottom; keep the
requestAnimationFrame logic and the same scrollTo options but gate it behind the
"near bottom" check so manual scroll position is preserved if the user scrolled
up.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b4070a5-9185-4317-a0aa-648100469a27
📒 Files selected for processing (2)
src/app/globals.csssrc/components/assistant-ui/thread.tsx
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/assistant-ui/thread.tsx">
<violation number="1" location="src/components/assistant-ui/thread.tsx:135">
P2: This unconditionally snaps to the bottom when a run finishes, so a user who scrolled up during streaming gets yanked back down. Guard this with a proximity check — only snap if the viewport is already near the bottom (e.g. within ~64px).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This PR fixes janky scrolling when sending messages and reduces excessive empty space below the last message in the chat viewport.
src/app/globals.css: Added
.aui-thread-viewport { scroll-behavior: auto; }outside@layer baseto overridehtml { scroll-behavior: smooth; }. Prevents CSS smooth scrolling from interfering with assistant-ui's programmaticscrollTocalls during streaming, which was causing missed/janky scroll positions.src/components/assistant-ui/thread.tsx:
fillClampThreshold="0px"andfillClampOffset="20em"toAssistantMessage'sMessagePrimitive.Rootto reduce slack space from ~700px to ~480px on typical viewports (about 60% instead of ~90%)AssistantMessagebottom margin fromlast:mb-4tolast:mb-2UserMessagebottom margin fromlast:mb-5tolast:mb-2Summary by CodeRabbit