51 auto scroll during ai output prevents user from scrolling up to read older messages

[akemmanuel]

No description provided.

[github-actions]

Based on my review, here's my analysis:

---

Confidence Score: 4/5

The PR implements a significant refactoring to add virtual scrolling for message lists, which is a substantial improvement for handling long conversations. The core logic for detecting user scroll state (pinnedToBottomRef) and conditionally auto-scrolling appears correct.

---

Issues Found

1. Potential Stale Closure in handleScroll (Low Severity)

File: src/components/message-list/VirtualMessageScroller.tsx:131-136

const handleScroll = useCallback(() => {
  const el = scrollRef.current;
  if (!el || programmaticScrollRef.current) return;
  const nearBottom = isNearBottom(el);
  pinnedToBottomRef.current = nearBottom;
}, []);

The handleScroll callback has empty dependencies [], but references programmaticScrollRef.current. While useRef values are accessed at runtime (not captured), it's safer to include the ref in deps or use a ref callback pattern for clarity. This works but could be fragile if ref semantics change.

---

2. Missing cleanup in useLayoutEffect for restoreAnchor (Low Severity)

File: src/components/message-list/VirtualMessageScroller.tsx:145-147

useLayoutEffect(() => {
  restoreAnchor();
}, [restoreAnchor, messages.length]);

This effect runs restoreAnchor() when messages change, but has no cleanup function. If restoreAnchor triggers rapid state changes during message streaming, it could cause extra re-renders. Consider returning a no-op cleanup:

useLayoutEffect(() => {
  restoreAnchor();
  return () => {}; // or void if cleanup not needed
}, [restoreAnchor, messages.length]);

---

3. Race Condition in scrollToLatest - Multiple RAF Frames (Design Consideration)

File: src/components/message-list/VirtualMessageScroller.tsx:111-123

const scrollToLatest = useCallback(() => {
  if (messages.length === 0) return;
  pinnedToBottomRef.current = true;
  programmaticScrollRef.current = true;
  virtualizer.scrollToIndex(messages.length - 1, { align: "end" });
  requestAnimationFrame(() => {
    const el = scrollRef.current;
    if (el) el.scrollTop = el.scrollHeight;
    requestAnimationFrame(() => {
      programmaticScrollRef.current = false;
    });
  });
}, [messages.length, virtualizer]);

The double requestAnimationFrame is used to ensure the scroll position is set after the virtualizer layout completes. However, during active AI streaming (many rapid message updates), this could cause:

• Multiple pending scroll operations
• Brief visual jumps as messages are rendered

This is a known complexity with virtualized scrolling during streaming. The implementation is reasonable but may need fine-tuning for edge cases.

---

4. Type Safety - Unchecked Array Access (Low Severity)

File: src/components/message-list/VirtualMessageScroller.tsx:161-177

useEffect(() => {
  const firstIndex = virtualizer.getVirtualItems()[0]?.index;
  if (
    firstIndex == null ||
    firstIndex > LOAD_OLDER_THRESHOLD_INDEX ||
    // ...

The code accesses virtualizer.getVirtualItems()[0]?.index without null checking the array returned. While Tanstack Virtual typically returns an array, this could theoretically return undefined in edge cases during rapid state changes.

---

Positive Changes

The PR includes several quality improvements:

1. Virtualization - Moving to @tanstack/react-virtual enables efficient rendering of large message lists
2. Anchor-based scrolling - Preserving scroll position when loading older messages via captureAnchor/restoreAnchor
3. Improved touch handling - Added onTouchMove={() => detachFromBottom(true)} for mobile
4. Separation of concerns - MessageList now delegates scroll logic to VirtualMessageScroller

---

Summary

This is a well-structured PR that addresses performance concerns with large message lists. The auto-scroll prevention logic (pinnedToBottomRef) correctly checks whether the user has scrolled away from the bottom before auto-scrolling. The minor issues identified are low-severity and don't block the merge. The overall implementation is sound for the stated goal of preventing auto-scroll from interrupting user reading.

opencode session  |  github run