Skip to content

feat: Implement smart auto-scroll for chat messages#15

Merged
zvadaadam merged 36 commits into
mainfrom
zvadaadam/chat-autoscroll
Oct 22, 2025
Merged

feat: Implement smart auto-scroll for chat messages#15
zvadaadam merged 36 commits into
mainfrom
zvadaadam/chat-autoscroll

Conversation

@zvadaadam

@zvadaadam zvadaadam commented Oct 22, 2025

Copy link
Copy Markdown
Owner

Summary

Implements intelligent auto-scroll behavior for chat messages with distinct handling for user and assistant messages:

  • User messages: Scroll to top of viewport (push previous messages up, focus on new user message)
  • Assistant messages: Smart overflow detection - only scrolls if content would be hidden
  • Streaming: Auto-scrolls during response streaming when content grows beyond viewport
  • User control: Stops auto-scroll if user manually scrolls up, with "scroll to bottom" button

Changes

New Features

  • Smart auto-scroll hook with configurable thresholds
  • User messages scroll to top of viewport for better focus
  • Assistant responses only scroll when content would overflow
  • ResizeObserver for smooth streaming auto-scroll
  • Respects manual user scrolling with state tracking

UI Improvements

  • Fixed MessageInput height (consistent 40px, auto-expands with content)
  • Invisible marker div for reliable scroll positioning
  • Smooth scroll animations with proper easing

Technical Implementation

  • useAutoScroll hook: Centralized scroll logic with dual ref system
    • lastMessageRef: Tracks last message for user message scroll-to-top
    • messagesEndRef: Empty div marker for assistant message scroll-to-bottom
  • Auto-scroll prevention: isAutoScrollingRef prevents scroll event handler from interfering
  • Smart overflow detection: Only scrolls if content bottom > viewport bottom + buffer
  • Configurable: scrollThreshold, inputHeightBuffer, smoothScrollUser options

Test Plan

  • User sends message → scrolls to top of viewport
  • Assistant responds with short message → no scroll if fits in view
  • Assistant responds with long message → auto-scrolls when content hidden
  • Streaming response → continuous auto-scroll as content grows
  • User scrolls up manually → auto-scroll stops, "scroll to bottom" button appears
  • MessageInput maintains consistent height (40px)
  • Works with empty user messages (tool-only messages)

Files Changed

  • src/features/session/hooks/useAutoScroll.ts - Complete rewrite with smart scroll logic
  • src/features/session/ui/Chat.tsx - Added dual ref system and invisible marker
  • src/features/session/ui/MessageInput.tsx - Fixed height inconsistency
  • src/features/session/ui/SessionPanel.tsx - Updated to pass both refs

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Smarter auto-scroll distinguishing user vs assistant messages, honoring reduced-motion and optional smooth scrolling
    • Auto-scroll during streaming content growth to keep new content visible
    • Per-message anchoring so the last message can be focused precisely
    • Scroll-to-bottom control moved to the session container and shows/hides based on user scroll state
  • Bug Fixes / Improvements

    • More consistent message input resizing with fixed initial height and reliable max limit
    • Chat API simplified: last-message anchoring added and in-component scroll button removed; motion-reduce behavior improved

Greptile Overview

Updated On: 2025-10-22 11:52:39 UTC

Greptile Summary

This PR implements an intelligent auto-scroll system for the chat interface that distinguishes between user and assistant messages to improve UX during conversations. When users send messages, the chat scrolls their message to the top of the viewport (keeping it visible), while assistant responses only trigger scrolling when content would overflow the visible area. The implementation uses a dual-ref architecture: lastMessageRef points to the actual last message element (enabling scroll-to-top positioning), while messagesEndRef remains an empty marker div at the container's end (for consistent scroll-to-bottom behavior). A ResizeObserver monitors content growth during streaming responses to trigger smooth auto-scroll when new content appears below the viewport. The hook respects manual user scrolling by detecting scroll-up gestures and disabling auto-scroll until the user clicks "scroll to bottom." Additionally, MessageInput now enforces a fixed 40px base height to ensure predictable scroll calculations. This fits into the codebase's chat-first philosophy (from CLAUDE.md) by treating chat interactions as first-class UX concerns with polished, context-aware scrolling behavior.

PR Description Notes:

  • The PR description lists Chat.tsx as "Added dual ref system and invisible marker" but the critical change is wrapping each message in a wrapper <div> and conditionally rendering the marker—this could impact the existing gap-6 spacing since gap applies to direct children.
Changed Files
Filename Score Overview
src/features/session/ui/SessionPanel.tsx 5/5 Added lastMessageRef declaration and passed both refs to useAutoScroll hook and Chat component
src/features/session/ui/MessageInput.tsx 4/5 Fixed height inconsistency by enforcing40px base height with auto-expand; redundant mount useEffect and potential flashing
src/features/session/hooks/useAutoScroll.ts 4/5 Complete rewrite implementing dual-ref scroll logic with role-based behavior, ResizeObserver, and scroll conflict guards
src/features/session/ui/Chat.tsx 4/5 Wrapped messages in divs to attach lastMessageRef marker to last message; uses inline styles instead of Tailwind classes

Confidence score: 3/5

  • This PR introduces complex scroll logic with subtle race conditions and layout side effects that require careful testing before merging.
  • Score reflects three concerns: (1) isAutoScrollingRef guard in useAutoScroll.ts may not prevent all scroll event conflicts during rapid scrolling or streaming, (2) wrapping messages in Chat.tsx divs alters the gap-6 spacing behavior and could break existing message layout, (3) MessageInput.tsx reset-then-expand pattern may cause brief visual flashing during typing.
  • Pay close attention to src/features/session/hooks/useAutoScroll.ts (scroll conflict guards and overflow detection logic), src/features/session/ui/Chat.tsx (wrapper div impact on spacing), and src/features/session/ui/MessageInput.tsx (height reset flashing during typing).

Sequence Diagram

sequenceDiagram
    participant User
    participant SessionPanel
    participant Chat
    participant MessageInput
    participant useAutoScroll
    participant DOM
    participant TanStackQuery

    User->>MessageInput: "Types message"
    User->>MessageInput: "Clicks Send (or ⌘+Enter)"
    MessageInput->>SessionPanel: "onSend()"
    SessionPanel->>TanStackQuery: "sendMessage(content)"
    TanStackQuery-->>SessionPanel: "Message sent"
    SessionPanel->>SessionPanel: "Clear messageInput"
    
    TanStackQuery->>SessionPanel: "New message arrives (role: user)"
    SessionPanel->>Chat: "Render new message"
    Chat->>DOM: "Add message to DOM"
    Chat->>DOM: "Render lastMessageRef marker"
    
    useAutoScroll->>useAutoScroll: "Detect new message (useEffect)"
    useAutoScroll->>useAutoScroll: "Check isUserScrolledUp = false"
    useAutoScroll->>useAutoScroll: "Check role === 'user'"
    useAutoScroll->>DOM: "requestAnimationFrame(() => scrollTop = markerTop)"
    DOM-->>Chat: "Scroll to top of viewport"
    
    TanStackQuery->>SessionPanel: "Assistant response streaming"
    SessionPanel->>Chat: "Update message content"
    Chat->>DOM: "Content grows"
    
    useAutoScroll->>DOM: "ResizeObserver detects growth"
    useAutoScroll->>useAutoScroll: "Check sessionStatus === 'working'"
    useAutoScroll->>useAutoScroll: "Check isContentHidden"
    useAutoScroll->>DOM: "scrollToBottom() via messagesEndRef"
    DOM-->>Chat: "Auto-scroll to reveal new content"
    
    User->>Chat: "Manually scrolls up"
    Chat->>useAutoScroll: "handleScroll event"
    useAutoScroll->>useAutoScroll: "isNearBottom() = false"
    useAutoScroll->>useAutoScroll: "Set isUserScrolledUp = true"
    useAutoScroll->>Chat: "showScrollButton = true"
    Chat->>DOM: "Show scroll-to-bottom button"
    
    User->>Chat: "Clicks scroll-to-bottom button"
    Chat->>useAutoScroll: "handleScrollToBottomClick()"
    useAutoScroll->>useAutoScroll: "Reset isUserScrolledUp = false"
    useAutoScroll->>DOM: "scrollToBottom(smooth: true)"
    DOM-->>Chat: "Smooth scroll to bottom"
    Chat->>DOM: "Hide scroll button"
Loading

Context used:

  • Context from dashboard - CLAUDE.md (source)

@coderabbitai

coderabbitai Bot commented Oct 22, 2025

Copy link
Copy Markdown

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a per-message lastMessageRef, expands UseAutoScrollOptions (threshold, buffer, smooth-scroll), rewrites auto-scroll logic (near-bottom detection, user-scroll tracking, ResizeObserver for streaming), threads lastMessageRef through Chat and SessionPanel, moves scroll-to-bottom UI into SessionPanel, and adjusts textarea resize behavior.

Changes

Cohort / File(s) Summary
Auto-scroll Hook Enhancement
src/features/session/hooks/useAutoScroll.ts
Adds lastMessageRef, scrollThreshold, inputHeightBuffer, smoothScrollUser to UseAutoScrollOptions; introduces AUTO_SCROLL_RESET_DELAY, isNearBottom, scrollToBottom, user-scroll vs auto-scroll tracking, lastMessageCountRef; skips initial auto-scroll; per-message scroll behavior (user vs assistant); adds ResizeObserver for streaming growth; reworks scroll event handling and manual scroll-to-bottom.
Chat UI
src/features/session/ui/Chat.tsx
ChatProps gains lastMessageRef; messages are wrapped so the last message wrapper receives lastMessageRef; motion-reduce variants added to container and spinners; internal scroll-to-bottom prop/button removed.
Message Input Resizing
src/features/session/ui/MessageInput.tsx
Textarea resize resets height to 40px before measuring, caps at 200px; removed mount-time resize effect; resize runs on input changes and on focus; inline initial height set; removed min-rows/min-h constraints. Public API unchanged.
Session Panel Integration
src/features/session/ui/SessionPanel.tsx
Introduces local lastMessageRef, passes it into useAutoScroll(...) and forwards to Chat at call sites; reintroduces panel-level scroll-to-bottom button (controlled by hook state) and positions it across embedded/timeline/embedded-block layouts; minor layout wrappers added for scrolling UI.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SessionPanel
    participant Chat
    participant UseAutoScroll
    participant ResizeObserver
    participant MessagesEnd

    User->>SessionPanel: send message / interact
    SessionPanel->>Chat: render messages (attach lastMessageRef)
    Chat->>UseAutoScroll: notify messages changed

    alt Last message from User
        UseAutoScroll->>Chat: scroll lastMessageRef into view (respect reduced-motion / smooth)
    else Last message from Assistant
        UseAutoScroll->>MessagesEnd: scroll to messagesEndRef (auto)
    end

    ResizeObserver->>UseAutoScroll: content growth observed
    UseAutoScroll->>MessagesEnd: if working && not scrolled up && content hidden -> auto-scroll

    User->>Chat: manual scroll
    Chat->>UseAutoScroll: update isUserScrolledUp, show/hide panel scroll button

    User->>SessionPanel: click panel scroll-to-bottom
    UseAutoScroll->>MessagesEnd: scroll to bottom, reset user-scroll state
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A hop, a ref, a gentle scroll,
I nudge the view when answers roll,
If humans climb the message hill,
I pause, then glide the bottom still,
Tiny paws, content and whole.


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

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

4 files reviewed, 7 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +46 to +50
// Reset to minimum first
el.style.height = '40px';
// Then expand to fit content, max 200px
const newHeight = Math.min(Math.max(el.scrollHeight, 40), 200);
el.style.height = newHeight + 'px';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Potential flashing issue: resetting to 40px then immediately expanding may cause visible height jump during typing. Consider setting height only when scrollHeight differs from current height. Have you tested this with rapid typing or paste operations to ensure smooth height transitions?

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/session/ui/MessageInput.tsx
Line: 46:50

Comment:
**style:** Potential flashing issue: resetting to 40px then immediately expanding may cause visible height jump during typing. Consider setting height only when scrollHeight differs from current height. Have you tested this with rapid typing or paste operations to ensure smooth height transitions?

How can I resolve this? If you propose a fix, please make it concise.

resizeTextarea();
}}
onBlur={() => setIsFocused(false)}
style={{ height: '40px' }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Inline style conflicts with CSS class: setting height='40px' inline means the resizeTextarea() function must override it every time. The inline style takes precedence, so resizeTextarea() correctly overwrites it, but this creates redundant DOM operations.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/session/ui/MessageInput.tsx
Line: 96:96

Comment:
**style:** Inline style conflicts with CSS class: setting height='40px' inline means the resizeTextarea() function must override it every time. The inline style takes precedence, so resizeTextarea() correctly overwrites it, but this creates redundant DOM operations.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 59 to 64
useEffect(() => {
resizeTextarea();
const el = textareaRef.current;
if (el) {
el.style.height = '40px';
}
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Duplicate initialization: inline style on line 96 already sets height to 40px, making this useEffect redundant.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/session/ui/MessageInput.tsx
Line: 59:64

Comment:
**style:** Duplicate initialization: inline style on line 96 already sets height to 40px, making this useEffect redundant.

How can I resolve this? If you propose a fix, please make it concise.


container.addEventListener('scroll', handleScroll);
return () => container.removeEventListener('scroll', handleScroll);
}, [messagesContainerRef]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: Missing isNearBottom dependency in the cleanup

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/session/hooks/useAutoScroll.ts
Line: 131:131

Comment:
**logic:** Missing `isNearBottom` dependency in the cleanup

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +138 to 155
const resizeObserver = new ResizeObserver(() => {
const { scrollTop, scrollHeight, clientHeight } = container;
const viewportBottom = scrollTop + clientHeight;
const contentBottom = scrollHeight;
const isContentHidden = contentBottom > viewportBottom + inputHeightBuffer;

// Only auto-scroll if:
// 1. In working session (streaming)
// 2. User hasn't scrolled up
// 3. Content would be hidden below viewport
if (sessionStatus === 'working' && !isUserScrolledUp && isContentHidden) {
isAutoScrollingRef.current = true;
scrollToBottom(false);
setTimeout(() => {
isAutoScrollingRef.current = false;
}, 50);
}
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: ResizeObserver may fire many times during streaming, potentially causing performance issues. Consider throttling or debouncing the scroll checks.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/session/hooks/useAutoScroll.ts
Line: 138:155

Comment:
**style:** ResizeObserver may fire many times during streaming, potentially causing performance issues. Consider throttling or debouncing the scroll checks.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread src/features/session/ui/Chat.tsx Outdated
/>
{/* Invisible ref marker - always renders even if MessageItem returns null */}
{index === messages.length - 1 && (
<div ref={lastMessageRef} style={{ height: 0, width: 0 }} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Inline styles bypass Tailwind and break consistency. Use Tailwind classes instead:

Suggested change
<div ref={lastMessageRef} style={{ height: 0, width: 0 }} />
<div ref={lastMessageRef} className="h-0 w-0" />

Context Used: Context from dashboard - CLAUDE.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/session/ui/Chat.tsx
Line: 69:69

Comment:
**style:** Inline styles bypass Tailwind and break consistency. Use Tailwind classes instead:

```suggestion
                  <div ref={lastMessageRef} className="h-0 w-0" />
```

**Context Used:** Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=5f29a80f-5c70-41a4-8bfe-6ea6c6a3911c))

How can I resolve this? If you propose a fix, please make it concise.

Comment thread src/features/session/ui/Chat.tsx Outdated
Comment on lines +61 to +71
<div key={message.id}>
<MessageItem
message={message}
parseContent={parseContent}
toolResultMap={toolResultMap}
/>
{/* Invisible ref marker - always renders even if MessageItem returns null */}
{index === messages.length - 1 && (
<div ref={lastMessageRef} style={{ height: 0, width: 0 }} />
)}
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: Wrapping each MessageItem in a div breaks the original gap-6 spacing (line 59). The gap applies between direct children, so the wrapper divs will have 6-spacing, but MessageItem's internal content won't benefit from the original layout logic. Is the gap-6 spacing still correct with the wrapper divs, or does this change the visual spacing between messages?

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/session/ui/Chat.tsx
Line: 61:71

Comment:
**logic:** Wrapping each MessageItem in a div breaks the original gap-6 spacing (line 59). The gap applies between direct children, so the wrapper divs will have 6-spacing, but MessageItem's internal content won't benefit from the original layout logic. Is the gap-6 spacing still correct with the wrapper divs, or does this change the visual spacing between messages?

How can I resolve this? If you propose a fix, please make it concise.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (6)
src/features/session/ui/MessageInput.tsx (1)

58-64: Consider removing redundant mount effect.

This effect might be unnecessary since:

  • Line 96 already sets the inline style to height: '40px'
  • The resize effect (lines 54-56) runs on mount regardless of messageInput value

The redundancy doesn't cause issues but could be simplified.

src/features/session/ui/Chat.tsx (2)

36-42: Respect reduced motion for programmatic scrolling.

Add a reduced‑motion fallback so the container doesn’t force smooth scrolling for users who prefer reduced motion.

Apply this diff:

-      className="relative flex-1 overflow-y-auto overflow-x-hidden scroll-smooth min-h-0 px-6 pt-6"
+      className="relative flex-1 overflow-y-auto overflow-x-hidden scroll-smooth motion-reduce:scroll-auto min-h-0 px-6 pt-6"

74-82: Pulse/spin animations: prefer ease and disable on reduced motion.

Guidelines recommend avoiding built-in easings other than ease/linear and honoring reduced‑motion.

Apply this diff:

-          {sessionStatus === 'working' && (
+          {sessionStatus === 'working' && (
             <div
               role="status"
               aria-live="polite"
-              className="flex items-center gap-2 p-2.5 px-3.5 mt-2 mr-auto max-w-[85%] bg-success/10 backdrop-blur-sm border border-success/30 rounded-xl text-success font-medium text-[0.85rem] shadow-sm animate-[pulse_0.6s_ease-in-out_infinite]"
+              className="flex items-center gap-2 p-2.5 px-3.5 mt-2 mr-auto max-w-[85%] bg-success/10 backdrop-blur-sm border border-success/30 rounded-xl text-success font-medium text-[0.85rem] shadow-sm motion-reduce:animate-none animate-[pulse_0.6s_ease_infinite]"
             >
-              <div className="w-4 h-4 border-2 border-success/20 border-t-success rounded-full animate-spin flex-shrink-0" aria-hidden="true"></div>
+              <div className="w-4 h-4 border-2 border-success/20 border-t-success rounded-full animate-spin motion-reduce:animate-none flex-shrink-0" aria-hidden="true"></div>
               <span>Claude is working...</span>
             </div>
           )}

As per coding guidelines.

src/features/session/hooks/useAutoScroll.ts (3)

76-100: Use smooth scrolling option and honor reduced motion for user messages.

smoothScrollUser is unused. Use scrollTo with behavior, and avoid console noise in production.

Apply this diff:

-      if (lastMessage.role === 'user') {
+      if (lastMessage.role === 'user') {
         // USER message: Scroll marker to TOP of viewport
         requestAnimationFrame(() => {
           const marker = lastMessageRef.current;
           if (marker) {
             // Get marker position relative to container
             const markerTop = marker.offsetTop;
             // Scroll container so marker is at the top
-            container.scrollTop = markerTop;
+            const prefersReduced = window.matchMedia('(prefers-reduced-motion: reduce)').matches;
+            container.scrollTo({
+              top: Math.max(0, markerTop),
+              behavior: smoothScrollUser && !prefersReduced ? 'smooth' : 'auto',
+            });
 
-            if (import.meta.env.DEV) {
+            if (import.meta.env.DEV) {
               console.log('[useAutoScroll] User message scrolled to top:', markerTop);
             }
           } else {
-            console.error('[useAutoScroll] lastMessageRef is null!');
+            if (import.meta.env.DEV) {
+              console.warn('[useAutoScroll] lastMessageRef is null!');
+            }
           }
 
           setTimeout(() => {
             isAutoScrollingRef.current = false;
           }, 100);
         });

161-165: Respect reduced motion for “scroll to bottom” button.

Avoid forced smooth scrolling when user prefers reduced motion.

Apply this diff:

-  const handleScrollToBottomClick = () => {
-    setIsUserScrolledUp(false);
-    scrollToBottom(true);
-  };
+  const handleScrollToBottomClick = () => {
+    setIsUserScrolledUp(false);
+    const prefersReduced = window.matchMedia('(prefers-reduced-motion: reduce)').matches;
+    scrollToBottom(!prefersReduced);
+  };

112-114: Edge case: same length, different last message.

Using only messages.length misses replacements where length stays constant (e.g., last item swapped). Track the last message id to detect true additions.

Suggested change:

const lastMessageIdRef = useRef<string | null>(null);
// ...
if (!messages.length || messages[messages.length - 1]?.id === lastMessageIdRef.current) {
  lastMessageIdRef.current = messages[messages.length - 1]?.id ?? null;
  return;
}
lastMessageIdRef.current = messages[messages.length - 1]?.id ?? null;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Lite

📥 Commits

Reviewing files that changed from the base of the PR and between d9ac849 and ab40ee7.

📒 Files selected for processing (4)
  • src/features/session/hooks/useAutoScroll.ts (1 hunks)
  • src/features/session/ui/Chat.tsx (3 hunks)
  • src/features/session/ui/MessageInput.tsx (2 hunks)
  • src/features/session/ui/SessionPanel.tsx (4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{tsx,jsx,css}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,jsx,css}: Avoid hardcoding colors; always use color tokens from Tailwind config
Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Use consistent paddings with a default of 16px unless there’s a strong reason otherwise

Files:

  • src/features/session/ui/Chat.tsx
  • src/features/session/ui/MessageInput.tsx
  • src/features/session/ui/SessionPanel.tsx
**/*.{css,tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{css,tsx,jsx}: Default to ease-out for most animations
Animations should be shorter than 1s; most between 0.2s and 0.3s
Do not use built-in CSS easings except ease or linear
Prefer ease-out for entering/user-initiated interactions; use ease-in-out for in-place movements; generally avoid ease-in
Use ease with 200ms duration for simple hover transitions (color, background-color, opacity)
Disable hover transitions on touch devices using @media (hover: hover) and (pointer: fine)
Respect prefers-reduced-motion: disable transform-based animations when reduced motion is requested
Animate elements from their trigger by setting an appropriate transform-origin
Prefer animating opacity and transform; avoid animating positional properties like top/left
Do not animate drag gestures using CSS variables
Do not animate blur values higher than 20px
Use will-change sparingly and only for transform, opacity, clipPath, or filter

Files:

  • src/features/session/ui/Chat.tsx
  • src/features/session/ui/MessageInput.tsx
  • src/features/session/ui/SessionPanel.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,jsx}: When using Framer Motion, use transform rather than x/y for hardware-accelerated animations
Default to spring animations with Framer Motion
Avoid bouncy spring animations unless working with drag gestures

Files:

  • src/features/session/ui/Chat.tsx
  • src/features/session/ui/MessageInput.tsx
  • src/features/session/ui/SessionPanel.tsx
🧬 Code graph analysis (2)
src/features/session/ui/Chat.tsx (1)
src/features/session/ui/MessageItem.tsx (1)
  • MessageItem (26-117)
src/features/session/hooks/useAutoScroll.ts (1)
tests/e2e-flow.test.cjs (1)
  • lastMessage (334-334)
🔇 Additional comments (2)
src/features/session/ui/MessageInput.tsx (1)

43-51: LGTM! Solid auto-resize implementation.

The reset-to-minimum pattern is correct and necessary to allow the textarea to shrink when text is deleted. The logic properly maintains a 40px minimum and 200px maximum height.

src/features/session/ui/SessionPanel.tsx (1)

41-44: Ref wiring looks correct across all render paths.

messagesEndRef, lastMessageRef, and messagesContainerRef are consistently created here, passed into useAutoScroll, and threaded to Chat in both embedded and dialog flows. No issues spotted.

Also applies to: 170-176, 260-261, 333-334

Comment thread src/features/session/hooks/useAutoScroll.ts
Comment thread src/features/session/ui/Chat.tsx
zvadaadam and others added 3 commits October 22, 2025 14:01
Critical fixes:
- Move ref to wrapper div (fixes user message scroll bug)
- Change ResizeObserver to observe content instead of container

Accessibility improvements:
- Add reduced motion support for scroll animations
- Add motion-reduce classes for pulse/spin animations
- Check prefers-reduced-motion in scroll behaviors

Code cleanup:
- Remove redundant mount effect in MessageInput
- Improve console logging (error → warn for nulls)
- Simplify textarea height management

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/features/session/hooks/useAutoScroll.ts (2)

145-162: Consider throttling the ResizeObserver callback.

During streaming, the ResizeObserver can fire many times per second as content grows, triggering frequent scroll operations and timeout scheduling. This may impact performance, especially on slower devices.

Consider throttling the callback:

+  // Throttle helper
+  const throttle = (fn: Function, delay: number) => {
+    let lastCall = 0;
+    return (...args: any[]) => {
+      const now = Date.now();
+      if (now - lastCall >= delay) {
+        lastCall = now;
+        fn(...args);
+      }
+    };
+  };
+
   const resizeObserver = new ResizeObserver(
+    throttle(() => {
       const { scrollTop, scrollHeight, clientHeight } = container;
       // ... rest of logic
+    }, 100)
   );

Alternatively, use a library like lodash.throttle or implement a useThrottle hook.


122-137: Add scrollThreshold to effect dependencies.

The scroll event handler closes over isNearBottom, which uses scrollThreshold from props. If scrollThreshold changes at runtime, the event handler won't see the updated value.

Apply this diff:

     container.addEventListener('scroll', handleScroll);
     return () => container.removeEventListener('scroll', handleScroll);
-  }, [messagesContainerRef]);
+  }, [messagesContainerRef, scrollThreshold]);
🧹 Nitpick comments (1)
src/features/session/hooks/useAutoScroll.ts (1)

102-104: Inconsistent timeout durations for resetting scroll flag.

The timeouts to reset isAutoScrollingRef.current vary: 100ms for user/assistant messages (lines 103, 112) but 50ms for streaming (line 159). This inconsistency could lead to subtle race conditions where scroll events fire before the flag is properly reset.

Consider using a consistent timeout or extracting it as a constant:

+const AUTO_SCROLL_RESET_DELAY = 100; // ms
+
 export function useAutoScroll({
   // ...
 }) {
   // ... 
           setTimeout(() => {
             isAutoScrollingRef.current = false;
-          }, 100);
+          }, AUTO_SCROLL_RESET_DELAY);

Also applies to: 111-113, 158-160

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Lite

📥 Commits

Reviewing files that changed from the base of the PR and between ab40ee7 and fedaaca.

📒 Files selected for processing (3)
  • src/features/session/hooks/useAutoScroll.ts (1 hunks)
  • src/features/session/ui/Chat.tsx (4 hunks)
  • src/features/session/ui/MessageInput.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/features/session/ui/MessageInput.tsx
  • src/features/session/ui/Chat.tsx
🔇 Additional comments (3)
src/features/session/hooks/useAutoScroll.ts (3)

1-14: Interface additions support the new features well.

The extended UseAutoScrollOptions with lastMessageRef, configurable thresholds, and smooth-scroll control provides the flexibility needed for the smart auto-scroll behavior.


48-63: Helper functions are well-implemented.

Both isNearBottom and scrollToBottom are clear, correct, and handle null refs safely.


168-179: Manual scroll handler works correctly.

The handleScrollToBottomClick properly resets user scroll state and respects motion preferences. The return values expose the necessary API.

Comment thread src/features/session/hooks/useAutoScroll.ts Outdated
zvadaadam and others added 3 commits October 22, 2025 15:19
Fixes:
- Scroll button flicker: Changed from sticky+padding to absolute positioning
  - Removes layout shift when button shows/hides
  - Button no longer affects document flow
- Consistent timeouts: Extract AUTO_SCROLL_RESET_DELAY constant (100ms)
  - All auto-scroll flag resets now use same delay
  - Previously mixed 50ms and 100ms delays

Changes:
- Chat.tsx: Button uses absolute positioning (no pb-6)
- useAutoScroll.ts: Single constant for all setTimeout delays

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
src/features/session/hooks/useAutoScroll.ts (3)

69-122: Add missing dependencies to prevent stale closures.

The effect accesses messages (line 76) and scrollToBottom (line 112), but only includes messages.length in the dependency array. While refs are stable, the messages array and scrollToBottom function should be included to prevent stale closures.

Apply this diff:

-  }, [messages.length, isUserScrolledUp, smoothScrollUser]);
+  }, [messages, isUserScrolledUp, smoothScrollUser, scrollToBottom]);

Note: This assumes scrollToBottom is wrapped in useCallback (see previous comment). If not memoized, this will cause the effect to re-run on every render.


125-140: Add isNearBottom to effect dependencies.

The effect calls isNearBottom (line 133) but doesn't include it in the dependency array. While this might work due to closure, it's better practice to either:

  1. Include isNearBottom in dependencies, or
  2. Wrap isNearBottom in useCallback with proper dependencies

Option 1 - Add to dependencies:

     container.addEventListener('scroll', handleScroll);
     return () => container.removeEventListener('scroll', handleScroll);
-  }, [messagesContainerRef]);
+  }, [messagesContainerRef, isNearBottom]);

Option 2 (preferred) - Memoize isNearBottom:

+  const isNearBottom = useCallback((threshold = scrollThreshold) => {
+    const container = messagesContainerRef.current;
+    if (!container) return false;
+
+    const { scrollTop, scrollHeight, clientHeight } = container;
+    return scrollHeight - scrollTop - clientHeight < threshold;
+  }, [scrollThreshold, messagesContainerRef]);

61-66: Wrap scrollToBottom in useCallback to prevent unnecessary effect re-runs.

The scrollToBottom function is recreated on every render and used in the auto-scroll effect's dependencies (line 122). This causes the effect to re-run unnecessarily, even when the scroll behavior hasn't changed.

Apply this diff:

-  // Scroll to bottom function
-  const scrollToBottom = (smooth = false) => {
+  // Scroll to bottom function
+  const scrollToBottom = useCallback((smooth = false) => {
     messagesEndRef.current?.scrollIntoView({
       behavior: smooth ? 'smooth' : 'auto',
       block: 'end'
     });
-  };
+  }, [messagesEndRef]);
🧹 Nitpick comments (1)
src/features/session/hooks/useAutoScroll.ts (1)

148-165: Consider throttling ResizeObserver for streaming performance.

The ResizeObserver fires on every content size change during streaming, which could cause performance issues with rapid updates. While the guards (sessionStatus, isUserScrolledUp, isContentHidden) prevent most unnecessary scrolls, throttling the callback could further optimize performance.

Example throttled implementation:

const resizeObserver = new ResizeObserver(() => {
  // Skip if we just scrolled
  if (isAutoScrollingRef.current) return;
  
  const { scrollTop, scrollHeight, clientHeight } = container;
  const viewportBottom = scrollTop + clientHeight;
  const contentBottom = scrollHeight;
  const isContentHidden = contentBottom > viewportBottom + inputHeightBuffer;

  if (sessionStatus === 'working' && !isUserScrolledUp && isContentHidden) {
    isAutoScrollingRef.current = true;
    scrollToBottom(false);
    setTimeout(() => {
      isAutoScrollingRef.current = false;
    }, AUTO_SCROLL_RESET_DELAY);
  }
});

Note: The early return for isAutoScrollingRef.current provides natural throttling by skipping updates during the 100ms reset window.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Lite

📥 Commits

Reviewing files that changed from the base of the PR and between fedaaca and 5ab0521.

📒 Files selected for processing (2)
  • src/features/session/hooks/useAutoScroll.ts (1 hunks)
  • src/features/session/ui/Chat.tsx (4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{tsx,jsx,css}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,jsx,css}: Avoid hardcoding colors; always use color tokens from Tailwind config
Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Use consistent paddings with a default of 16px unless there’s a strong reason otherwise

Files:

  • src/features/session/ui/Chat.tsx
**/*.{css,tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{css,tsx,jsx}: Default to ease-out for most animations
Animations should be shorter than 1s; most between 0.2s and 0.3s
Do not use built-in CSS easings except ease or linear
Prefer ease-out for entering/user-initiated interactions; use ease-in-out for in-place movements; generally avoid ease-in
Use ease with 200ms duration for simple hover transitions (color, background-color, opacity)
Disable hover transitions on touch devices using @media (hover: hover) and (pointer: fine)
Respect prefers-reduced-motion: disable transform-based animations when reduced motion is requested
Animate elements from their trigger by setting an appropriate transform-origin
Prefer animating opacity and transform; avoid animating positional properties like top/left
Do not animate drag gestures using CSS variables
Do not animate blur values higher than 20px
Use will-change sparingly and only for transform, opacity, clipPath, or filter

Files:

  • src/features/session/ui/Chat.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,jsx}: When using Framer Motion, use transform rather than x/y for hardware-accelerated animations
Default to spring animations with Framer Motion
Avoid bouncy spring animations unless working with drag gestures

Files:

  • src/features/session/ui/Chat.tsx
🧬 Code graph analysis (1)
src/features/session/ui/Chat.tsx (1)
src/features/session/ui/MessageItem.tsx (1)
  • MessageItem (26-117)
🔇 Additional comments (1)
src/features/session/ui/Chat.tsx (1)

40-40: Excellent accessibility: motion-reduce variants properly applied.

The motion-reduce:scroll-auto, motion-reduce:animate-none classes properly respect user preferences for reduced motion, aligning with WCAG accessibility guidelines and the coding standards.

As per coding guidelines.

Also applies to: 77-77, 79-79

Comment on lines +60 to 71
{messages.map((message, index) => (
<div
key={message.id}
message={message}
parseContent={parseContent}
toolResultMap={toolResultMap}
/>
ref={index === messages.length - 1 ? lastMessageRef : undefined}
>
<MessageItem
message={message}
parseContent={parseContent}
toolResultMap={toolResultMap}
/>
</div>
))}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Wrapper divs may cause extra spacing when MessageItem returns null.

Each message is now wrapped in a div, but MessageItem can return null for messages containing only tool_result blocks (see MessageItem.tsx:25-116). When MessageItem returns null, the wrapper div still exists and participates in the gap-6 spacing, potentially creating unexpected gaps.

Consider filtering messages before rendering or conditionally rendering the wrapper:

           <div className="flex flex-col gap-6 pb-8 min-h-0">
-            {messages.map((message, index) => (
+            {messages.map((message, index) => {
+              const isLast = index === messages.length - 1;
+              return (
               <div
                 key={message.id}
-                ref={index === messages.length - 1 ? lastMessageRef : undefined}
+                ref={isLast ? lastMessageRef : undefined}
               >
                 <MessageItem
                   message={message}
                   parseContent={parseContent}
                   toolResultMap={toolResultMap}
                 />
               </div>
-            ))}
+              );
+            })}
           </div>

Or better, conditionally render the wrapper only when MessageItem would render:

{messages.map((message, index) => {
  const isLast = index === messages.length - 1;
  const messageItem = (
    <MessageItem
      message={message}
      parseContent={parseContent}
      toolResultMap={toolResultMap}
    />
  );
  
  return isLast ? (
    <div key={message.id} ref={lastMessageRef}>
      {messageItem}
    </div>
  ) : (
    messageItem
  );
})}

However, this second approach defeats the purpose of always having a ref target for scrolling.

🤖 Prompt for AI Agents
In src/features/session/ui/Chat.tsx around lines 60-71, the current wrapper div
around each message creates unwanted spacing when MessageItem returns null;
update rendering to avoid rendering empty wrapper elements: compute/render the
MessageItem first, skip rendering any wrapper/div when the MessageItem is null,
and only render a container when the MessageItem exists; ensure the
lastMessageRef still attaches to the last rendered message by either forwarding
a ref into MessageItem (implement React.forwardRef in MessageItem and pass
lastMessageRef when that message is last) or by using a callback ref on the
rendered element so scrolling still targets the final visible message.

zvadaadam and others added 3 commits October 22, 2025 15:29
Problem: Scroll button was inside the scrollable Chat component,
so it scrolled with content and became invisible.

Solution:
- Remove button from Chat component
- Render button in SessionPanel parent (outside scrollable area)
- Position absolute relative to parent wrapper
- Button stays fixed at bottom-right, always visible
- Use bottom-20 to account for MessageInput height

Changes:
- Chat.tsx: Remove button rendering and props
- SessionPanel.tsx: Add button rendering in both embedded/modal views

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Changed bottom-20 to bottom-28 for more clearance above message input.
This prevents the button from being cropped by the input field.
Adds elegant gradient overlay that fades messages as they approach
the input field, creating a polished visual effect.

Implementation:
- 128px height gradient (medium length)
- Fades from transparent to background color
- Uses CSS variable for theme compatibility
- pointer-events: none (doesn't block interactions)
- z-index: 5 (below button, above content)

Design principles:
- Signals scrollability
- Softens content cutoff
- Creates visual depth
- Non-intrusive and subtle
Problem: Fade was between Chat and MessageInput, hidden by input background.

Solution: Wrap Chat and fade together, position fade absolutely at
bottom of Chat container to overlay scrollable content.

This creates the proper effect where messages fade as they approach
the bottom edge of the scroll area.
The wrapper div broke the flex layout, making chat unscrollable
and hiding the message input.

Reverted to stable layout:
- Chat component (flex-1, scrollable)
- Scroll button (absolute positioning)
- MessageInput (sticky at bottom)

Note: Scroll fade effect removed temporarily - can be added
back using CSS mask-image on Chat component if needed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/features/session/ui/SessionPanel.tsx (3)

254-291: Consider extracting the fade overlay and scroll button into a reusable component.

The fade overlay (lines 268-273) and scroll-to-bottom button (lines 277-290) are duplicated in both the embedded render path (here) and the timeline render path (lines 367-372 and 377-389). Extracting this UI pattern into a shared component would improve maintainability.

Example refactor:

Create a new component:

function ChatWithScrollControls({
  messages,
  loading,
  sessionStatus,
  parseContent,
  messagesEndRef,
  lastMessageRef,
  messagesContainerRef,
  toolResultMap,
  showScrollButton,
  onScrollToBottom,
}: ChatProps & {
  showScrollButton: boolean;
  onScrollToBottom: () => void;
}) {
  return (
    <div className="relative flex-1 min-h-0">
      <Chat
        messages={messages}
        loading={loading}
        sessionStatus={sessionStatus}
        parseContent={parseContent}
        messagesEndRef={messagesEndRef}
        lastMessageRef={lastMessageRef}
        messagesContainerRef={messagesContainerRef}
        toolResultMap={toolResultMap}
      />

      {/* Scroll fade overlay */}
      <div
        className="absolute bottom-0 left-0 right-0 h-32 pointer-events-none z-10"
        style={{
          background: 'linear-gradient(to bottom, transparent 0%, hsl(var(--background)) 100%)'
        }}
      />

      {/* Scroll to bottom button */}
      {showScrollButton && (
        <div className="absolute bottom-28 right-6 pointer-events-auto z-20">
          <Button
            variant="secondary"
            size="icon"
            className="rounded-full shadow-lg"
            onClick={onScrollToBottom}
            title="Scroll to bottom"
            aria-label="Scroll to bottom"
            aria-controls="chat-messages"
          >
            <ChevronDown className="h-4 w-4" aria-hidden="true" />
          </Button>
        </div>
      )}
    </div>
  );
}

Then use it in both render paths:

-        <div className="relative flex-1 min-h-0">
-          <Chat ... />
-          <div className="absolute bottom-0 ..." />
-        </div>
-        {showScrollButton && <div className="absolute bottom-28 ..." />}
+        <ChatWithScrollControls
+          messages={messages}
+          loading={loading}
+          sessionStatus={sessionStatus}
+          parseContent={parseContent}
+          messagesEndRef={messagesEndRef}
+          lastMessageRef={lastMessageRef}
+          messagesContainerRef={messagesContainerRef}
+          toolResultMap={toolResultMap}
+          showScrollButton={showScrollButton}
+          onScrollToBottom={handleScrollToBottomClick}
+        />

277-290: Consider animating the scroll button appearance.

The button appears and disappears abruptly when showScrollButton changes. Adding a fade-in/fade-out animation would create a smoother user experience.

Example using Framer Motion:

+import { motion, AnimatePresence } from "framer-motion";

-        {showScrollButton && (
-          <div className="absolute bottom-28 right-6 pointer-events-auto z-10">
+        <AnimatePresence>
+          {showScrollButton && (
+            <motion.div
+              initial={{ opacity: 0, scale: 0.8 }}
+              animate={{ opacity: 1, scale: 1 }}
+              exit={{ opacity: 0, scale: 0.8 }}
+              transition={{ type: "spring", stiffness: 400, damping: 25 }}
+              className="absolute bottom-28 right-6 pointer-events-auto z-20"
+            >
              <Button
                variant="secondary"
                size="icon"
                className="rounded-full shadow-lg"
                onClick={handleScrollToBottomClick}
                title="Scroll to bottom"
                aria-label="Scroll to bottom"
                aria-controls="chat-messages"
              >
                <ChevronDown className="h-4 w-4" aria-hidden="true" />
              </Button>
-          </div>
-        )}
+            </motion.div>
+          )}
+        </AnimatePresence>

269-269: Consider using explicit z-index layering for the scroll button.

Both the fade overlay and scroll button use z-10. While the button appears on top due to DOM order, using a higher z-index for the button (e.g., z-20) would make the layering intent explicit and more maintainable.

Apply this diff:

-          <div className="absolute bottom-28 right-6 pointer-events-auto z-10">
+          <div className="absolute bottom-28 right-6 pointer-events-auto z-20">

Apply to both occurrences (lines 278 and 377).

Also applies to: 278-278, 368-368, 377-377

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Lite

📥 Commits

Reviewing files that changed from the base of the PR and between 7b67619 and 41b9e3f.

📒 Files selected for processing (1)
  • src/features/session/ui/SessionPanel.tsx (6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{tsx,jsx,css}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,jsx,css}: Avoid hardcoding colors; always use color tokens from Tailwind config
Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Use consistent paddings with a default of 16px unless there’s a strong reason otherwise

Files:

  • src/features/session/ui/SessionPanel.tsx
**/*.{css,tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{css,tsx,jsx}: Default to ease-out for most animations
Animations should be shorter than 1s; most between 0.2s and 0.3s
Do not use built-in CSS easings except ease or linear
Prefer ease-out for entering/user-initiated interactions; use ease-in-out for in-place movements; generally avoid ease-in
Use ease with 200ms duration for simple hover transitions (color, background-color, opacity)
Disable hover transitions on touch devices using @media (hover: hover) and (pointer: fine)
Respect prefers-reduced-motion: disable transform-based animations when reduced motion is requested
Animate elements from their trigger by setting an appropriate transform-origin
Prefer animating opacity and transform; avoid animating positional properties like top/left
Do not animate drag gestures using CSS variables
Do not animate blur values higher than 20px
Use will-change sparingly and only for transform, opacity, clipPath, or filter

Files:

  • src/features/session/ui/SessionPanel.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,jsx}: When using Framer Motion, use transform rather than x/y for hardware-accelerated animations
Default to spring animations with Framer Motion
Avoid bouncy spring animations unless working with drag gestures

Files:

  • src/features/session/ui/SessionPanel.tsx
🧬 Code graph analysis (1)
src/features/session/ui/SessionPanel.tsx (1)
src/features/session/ui/Chat.tsx (1)
  • Chat (19-82)
🔇 Additional comments (5)
src/features/session/ui/SessionPanel.tsx (5)

23-23: LGTM!

The ChevronDown import is correctly added for the scroll-to-bottom button icon.


41-42: LGTM!

The dual-ref system is well-documented with clear comments explaining each ref's purpose.


167-176: LGTM!

The lastMessageRef is correctly integrated into the useAutoScroll hook, completing the dual-ref system.


352-405: LGTM!

The timeline render path correctly implements the same fade overlay and scroll button pattern as the embedded mode. The code duplication has been noted in the previous comment.


278-278: Button positioning is functionally safe but visually static as MessageInput expands.

The scroll button (bottom-28) is positioned within the messages container, separate from the MessageInput. Since MessageInput is a sibling flex-shrink-0 container below, there's no risk of overlap. However, the button's position is fixed at 112px from the bottom of the messages area, while MessageInput can expand from 40px to 200px (max). This creates an inconsistent visual gap—the button appears to drift further from the input as it expands. The current implementation works without overlap, but consider whether the spacing should feel more responsive to input growth for better UX.

zvadaadam and others added 2 commits October 22, 2025 15:44
Positioned relative to MessageInput, extends upward 128px to fade
messages approaching the input field.

Implementation:
- absolute bottom-full (extends upward from input)
- 128px height gradient overlay
- pointer-events: none (doesn't block interactions)
- Uses CSS variable for theme compatibility

This approach doesn't break flex layout since it's positioned
absolutely relative to the input component.
- Wrap scrollToBottom and isNearBottom in useCallback to prevent recreation
- Fix missing dependencies in all useEffect hooks
- Add throttling (100ms) to ResizeObserver to reduce scroll operations during streaming
- Improve performance and prevent stale closures

Addresses code review feedback from CodeRabbit and Greptile AI.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/features/session/hooks/useAutoScroll.ts (1)

148-155: Consider using useRef for persistent throttle state.

The throttle uses a closure variable lastCall (line 149) that resets to 0 whenever the effect re-runs. While the effect dependencies are relatively stable, rapid changes to isUserScrolledUp or sessionStatus during streaming would reset the throttle counter, potentially allowing bursts of scroll operations.

Refactor to use useRef for more robust throttling:

+  const lastCallRef = useRef(0);
+  const throttleDelay = 100; // ms
+
   const resizeObserver = new ResizeObserver(() => {
     const now = Date.now();
-    if (now - lastCall < throttleDelay) return;
-    lastCall = now;
+    if (now - lastCallRef.current < throttleDelay) return;
+    lastCallRef.current = now;

Then move lastCallRef and throttleDelay outside the effect.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Lite

📥 Commits

Reviewing files that changed from the base of the PR and between eae79be and abd512f.

📒 Files selected for processing (1)
  • src/features/session/hooks/useAutoScroll.ts (1 hunks)
🔇 Additional comments (7)
src/features/session/hooks/useAutoScroll.ts (7)

1-18: LGTM! Clean interface design with sensible defaults.

The expanded interface with configurable options and dual-ref system is well-structured. The 100ms reset delay aligns with the ResizeObserver throttle interval.


51-66: LGTM! Helper functions are correctly memoized.

Both isNearBottom and scrollToBottom use useCallback with proper dependency arrays and handle null refs safely.


69-122: Past review comment addressed: dependencies are now complete.

The effect now correctly includes messages, scrollToBottom, messagesContainerRef, lastMessageRef in its dependency array (line 122), resolving the stale-closure concern from the previous review.


125-140: Past review comment addressed: isNearBottom is now in the dependency array.

The effect correctly includes isNearBottom on line 140, ensuring the scroll handler uses the latest threshold. The guard on line 131 properly prevents state updates during programmatic scrolls.


142-177: Past review comments addressed: throttling added and correct target observed.

The ResizeObserver now:

  • Observes lastMessageRef ?? messagesEndRef (line 145), ensuring content changes trigger the callback
  • Includes 100ms throttling (lines 148-155) to prevent excessive scroll operations during streaming

Both concerns from previous reviews are resolved.


179-184: LGTM! Clean manual scroll handler with accessibility support.

Correctly resets the user-scroll state and respects prefers-reduced-motion for smooth scrolling.


82-94: No changes required. The offsetTop calculation is correct for your DOM structure.

The review concern assumes positioned elements between marker and container could break the calculation, but your DOM structure has position: relative only on the container itself. The marker is nested within a static-positioned flex div, making the container its offsetParent. Therefore, marker.offsetTop correctly returns the distance from the container's top edge—exactly what scrollTo needs.

The suggested scrollIntoView alternative would sacrifice the pixel-level control your current implementation provides. Your approach is more precise for this use case.

Likely an incorrect or invalid review comment.

@zvadaadam zvadaadam merged commit 6e7cc50 into main Oct 22, 2025
1 check passed
zvadaadam added a commit that referenced this pull request Apr 14, 2026
…n-exported 2 dead Zod schemas from shared/enums.ts, removed the dead useWindowFocus hook and its useSyncExternalStore infrastructure (50 lines) from useWindowFocus.ts, and removed dead BaseToolRendererProps barrel re-export — net reduction of 62 lines across 4 files with all 825 tests passing and clean tsc.
zvadaadam added a commit that referenced this pull request Apr 14, 2026
* gnhf #1: Extracted two helper methods in claude-adapter.ts (closeActiveParts, accumulateStreamDelta) to eliminate 5 repeated close-text/close-thinking call sites and 2 duplicate 25-line streaming delta handlers; also replaced a duplicate 15-line formatTime function in WorkspaceItem.tsx with the existing shared formatTimeAgo utility — net reduction of 49 lines with all 375 agent-server tests passing.

* gnhf #2: Extracted duplicate workspace-grouping logic (85 lines across query-engine.ts and workspaces.ts route) into shared lib/workspace-grouping.ts, and consolidated duplicate parameter-reading helpers (readString/readNumber across query-engine.ts and commands.ts) into shared lib/query-params.ts — net reduction of 41 lines with all 825 tests passing.

* gnhf #3: Removed unused  parameter from cancellation functions across both agent handlers, extracted  helper replacing 5 identical repo-lookup patterns in repos.ts, unified duplicate git progress push functions, and removed dead  export — net reduction of 23 lines across 7 files with all 825 tests passing.

* gnhf #4: Removed 5 dead exports/functions and un-exported 2 internal-only functions across 6 files — net reduction of 112 lines with all 825 tests passing.

* gnhf #5: Removed 2 dead exported functions (getRepoInitials, getRepoColor), un-exported 3 internal-only symbols (RECENT_PROJECT_LIMIT, resolveGitProjectRoot, setLastOpenInAppId), and replaced duplicate timeAgo in AccessSection.tsx with shared formatTimeAgo — net reduction of 41 lines across 5 files with all 825 tests passing.

* gnhf #6: Un-exported 14 dead Zod schema validators (6 from shared/events.ts, 8 from shared/agent-events.ts), un-exported 6 dead type aliases from shared/events.ts, and consolidated duplicate parseGitHubRepo function from gh.service.ts and deus-import.ts into shared/lib/github.ts — reducing public API surface by 20 exports and eliminating 1 duplicate function across 5 files with all 825 tests passing.

* gnhf #7: Deleted 2 dead component files (OpenInDropdown 214 lines, EmptyState 39 lines), removed 3 dead API type definitions (ApiResponse, PaginatedResponse, WorkspaceQueryParams) from shared/types/api.ts, removed 4 dead session type aliases (SessionMessageEvent, SessionErrorEvent, SessionEnterPlanModeEvent, SessionStatusEvent) from shared/types/session.ts, and cleaned up 3 barrel re-export files — net reduction of 330 lines across 7 files with all 825 tests passing.

* gnhf #8: Deleted 2 dead platform files (updates.ts 31 lines, listenerGroup.ts 38 lines), removed dead  function from dialog.ts, removed the entire dead StatusChanged notification pipeline across 5 files (method, schema, type, constant, test builder, union), un-exported  from electron barrel, and cleaned up 3 barrel re-export files — net reduction of 121 lines across 11 files with all 825 tests passing.

* gnhf #9: Removed 3 dead query hooks (useStats, useUncommittedFiles, useLastTurnFiles) with their stub service methods and query keys, removed 4 dead type definitions (ChangedFilesResult, BranchInfo, PaginationParams, DevServer) and their barrel re-exports, un-exported internal-only connectToRelay function, and cleaned up 2 dead barrel re-exports (clearToken, ConnectionIllustration) — net reduction of 121 lines across 12 files with all 825 tests passing and clean tsc.

* gnhf #10: Removed 6 dead visual effect builder functions (98 lines) from visual-effects.ts, un-exported 5 internal-only symbols (resolveClaudeDir, getAgentConfig, StatusPriority, StatusConfig, WorkflowStatusConfig), and removed dead barrel re-exports (createAgentEventHandler, AgentEventHandler) from agent/index.ts — net reduction of 98 lines across 5 files with all 825 tests passing and clean tsc.

* gnhf #11: Consolidated inline path validation in files.ts to use shared resolveWorkspaceRelativePath from git.service.ts, and replaced 10 hand-rolled readString+throw param validation patterns in commands.ts with the existing requireParam utility — net reduction of 17 lines across 2 files with all 825 tests passing and clean tsc.

* gnhf #12: Removed 211 lines of dead CSS from global.css: the entire glitch-swap effect system (6 classes, 6 @Keyframes, 2 media queries) and the empty tool-use-enter class — none referenced by any component, with all 825 tests passing.

* gnhf #13: Deleted 2 dead component files (RepoGroup.tsx 102 lines, WorkspaceItem.tsx 123 lines) from repository/ui that were never rendered anywhere, removed 14 dead barrel re-exports across 4 barrel files (repository/ui, sidebar/ui, sidebar feature, session/ui/blocks), and cleaned up knip.json — net reduction of 242 lines across 7 files with all 825 tests passing and clean tsc.

* gnhf #14: Deleted the dead agent-server/messages/index.ts barrel (30 lines), removed 4 dead type exports from shared/messages/types.ts (ToolLocation, ToolOutputContent, PartType, PartTypeSchema), removed dead ToolResultMap type from chat-types.ts, and cleaned up 15 dead barrel re-exports across 4 barrel files (session/hooks, session/ui, workspace/ui, shared/hooks) — net reduction of 51 lines across 8 files with all 825 tests passing and clean tsc.

* gnhf #15: Removed 10 dead type aliases from shared/agent-events.ts, un-exported 2 dead Zod schemas from shared/enums.ts, removed the dead useWindowFocus hook and its useSyncExternalStore infrastructure (50 lines) from useWindowFocus.ts, and removed dead BaseToolRendererProps barrel re-export — net reduction of 62 lines across 4 files with all 825 tests passing and clean tsc.

* refactor: extract backend helpers and fix typecheck

* Address CodeRabbit review feedback

* Fix Claude adapter message handoff
zvadaadam added a commit that referenced this pull request May 31, 2026
…n-exported 2 dead Zod schemas from shared/enums.ts, removed the dead useWindowFocus hook and its useSyncExternalStore infrastructure (50 lines) from useWindowFocus.ts, and removed dead BaseToolRendererProps barrel re-export — net reduction of 62 lines across 4 files with all 825 tests passing and clean tsc.
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