Skip to content

fix(web): coordinate streaming reasoning scroll - #1398

Merged
tiann merged 1 commit into
tiann:mainfrom
KorenKrita:agent/fix-reasoning-scroll-follow
Aug 7, 2026
Merged

fix(web): coordinate streaming reasoning scroll#1398
tiann merged 1 commit into
tiann:mainfrom
KorenKrita:agent/fix-reasoning-scroll-follow

Conversation

@KorenKrita

Copy link
Copy Markdown
Contributor

Summary

  • open an active reasoning panel at its latest streamed content and keep following while its inner viewport remains at the bottom
  • stop following when the user scrolls away, and resume when they return to the bottom
  • coordinate nested-scroll ownership with the outer chat viewport and contain overscroll inside the reasoning panel
  • add keyboard focus and regression coverage for pointer, wheel, and follow-tail behavior

Root cause

The reasoning body was an independent scroll container with no follow-tail state. Meanwhile, user interaction inside it did not pause HappyThread's outer follow-tail, so the outer ResizeObserver kept moving the chat viewport while the user manipulated the inner scrollbar.

Validation

  • bun run typecheck
  • bun run test:web (241 files, 2157 tests)
  • focused reasoning and HappyThread regression coverage

Closes #1397

@KorenKrita
KorenKrita force-pushed the agent/fix-reasoning-scroll-follow branch from 8c311dd to 874ef30 Compare August 6, 2026 23:56

@github-actions github-actions 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.

Findings

  • [Major] No-op nested interactions can leave outer follow-tail disabled - the new pointer/wheel/touch/key handlers claim nested ownership before movement, while only an inner scroll event releases it. A click/tap, a wheel/key gesture at the boundary, or any interaction in a reasoning block shorter than 60vh produces no scroll event, so HappyThread keeps autoScrollEnabledRef false and stops following later streamed growth. Evidence web/src/components/assistant-ui/reasoning.tsx:100, web/src/components/assistant-ui/reasoning.tsx:139, web/src/components/AssistantChat/HappyThread.tsx:972.
    Suggested fix:
    const syncNestedFollow = () => {
        const scroll = scrollRef.current
        if (!scroll) return
        const followLatest = scroll.scrollHeight - scroll.scrollTop - scroll.clientHeight <= 8
        followLatestRef.current = followLatest
        chatContext?.onNestedScrollFollowChange?.(followLatest)
    }
    
    const claimNestedPointerScroll = () => {
        claimNestedScroll()
        window.addEventListener('pointerup', syncNestedFollow, { once: true })
        window.addEventListener('pointercancel', syncNestedFollow, { once: true })
    }
    
    const claimNestedWheelOrKeyScroll = () => {
        claimNestedScroll()
        requestAnimationFrame(syncNestedFollow)
    }

Summary

  • Review mode: initial
  • One major follow-tail state restoration bug found.

Testing

  • Not run (automation); repository CI is pending. Add coverage proving click/tap and boundary wheel/key input at the inner bottom leave outer follow enabled, including a non-scrollable reasoning block.

HAPI Bot

Comment thread web/src/components/assistant-ui/reasoning.tsx Outdated
@KorenKrita
KorenKrita force-pushed the agent/fix-reasoning-scroll-follow branch from 874ef30 to e585def Compare August 7, 2026 00:05

@github-actions github-actions 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.

Findings

  • [Major] Collapsing a reasoning panel after scrolling it away from the bottom does not release nested follow ownership. The outer thread can remain with follow-tail disabled after the inner viewport is hidden. Evidence web/src/components/assistant-ui/reasoning.tsx:87, web/src/components/AssistantChat/HappyThread.tsx:972.
    Suggested fix:
    useEffect(() => {
        if (isOpen || followLatestRef.current) return
        followLatestRef.current = true
        chatContext?.onNestedScrollFollowChange?.(true)
    }, [isOpen, chatContext])
  • [Minor] The new reasoning scroll viewport remains keyboard-focusable while its parent is collapsed with max-h-0 opacity-0, creating an invisible tab stop for every collapsed reasoning block. Evidence web/src/components/assistant-ui/reasoning.tsx:185.
    Suggested fix:
    tabIndex={isOpen ? 0 : -1}

Summary
Review mode: follow-up after new commits

  • The prior no-op pointer/wheel ownership issue is addressed. Two added-line regressions remain.

Testing

  • Not run (automation); static review only. CI is pending. Add coverage for collapse-after-scroll-away and collapsed/expanded tab order.

HAPI Bot

Comment thread web/src/components/assistant-ui/reasoning.tsx
Comment thread web/src/components/assistant-ui/reasoning.tsx Outdated
@KorenKrita
KorenKrita force-pushed the agent/fix-reasoning-scroll-follow branch from e585def to 9a8974e Compare August 7, 2026 00:15

@github-actions github-actions 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.

Findings

  • [Major] Preference-driven collapse can still leave outer follow-tail disabled. The streaming preference effect resets followLatestRef before the close-release effect can observe the prior scrolled-away state, so a cross-tab/global collapse after an inner scroll never sends true back to HappyThread. Evidence web/src/components/assistant-ui/reasoning.tsx:77, web/src/components/assistant-ui/reasoning.tsx:82, web/src/components/AssistantChat/HappyThread.tsx:974.
    Suggested fix:
    useEffect(() => {
        if (!isStreaming) return
        const nextOpen = !reasoningCollapsed
        if (nextOpen) followLatestRef.current = true
        setIsOpen(nextOpen)
    }, [isStreaming, reasoningCollapsed])
  • [Major] Programmatic inner auto-follow cancels the outer initial-scroll recovery timers. useLayoutEffect moves the inner viewport during streaming; its resulting scroll event reports followLatest=true, but the outer callback clears settling timers for both user claims and programmatic follow updates. A tall reasoning block during initial load can therefore remove the timers that protect against assistant-ui resetting the chat viewport. Evidence web/src/components/AssistantChat/HappyThread.tsx:973, related flow web/src/components/assistant-ui/reasoning.tsx:90 and web/src/components/assistant-ui/reasoning.tsx:192.
    Suggested fix:
    const handleNestedScrollFollowChange = useCallback((followLatest: boolean) => {
        if (!followLatest) clearInitialScrollTimers()
        autoScrollEnabledRef.current = followLatest && atBottomRef.current
    }, [clearInitialScrollTimers])

Summary
Review mode: follow-up after new commits

  • The prior collapse/tab-order fixes are present, but two follow-tail regressions remain.

Testing

  • Not run (automation); static review only. CI checks were pending at review time.
  • Add coverage for storage/preference collapse after scrolling the inner viewport away, and for programmatic inner auto-follow preserving outer initial-settling recovery.

HAPI Bot

Comment thread web/src/components/assistant-ui/reasoning.tsx Outdated
Comment thread web/src/components/AssistantChat/HappyThread.tsx Outdated
@KorenKrita
KorenKrita force-pushed the agent/fix-reasoning-scroll-follow branch from 9a8974e to 260cec3 Compare August 7, 2026 00:27

@github-actions github-actions 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.

Findings

  • [Major] Pointer ownership can be released before the gesture ends - claimNestedPointerScroll pauses only the outer follower, while followLatestRef remains true. If reasoning streams before pointer-up, the layout effect advances the inner viewport and its scroll event reports true, re-enabling HappyThread during the drag and recreating the nested-scroll fight. Evidence web/src/components/assistant-ui/reasoning.tsx:117, related flow web/src/components/assistant-ui/reasoning.tsx:90 and web/src/components/assistant-ui/reasoning.tsx:112.
    Suggested fix:
    const pointerActiveRef = useRef(false)
    
    const syncNestedFollow = useCallback(() => {
        const scroll = scrollRef.current
        if (!scroll) return
        const atBottom = scroll.scrollHeight - scroll.scrollTop - scroll.clientHeight <= 8
        const followLatest = atBottom && !pointerActiveRef.current
        followLatestRef.current = followLatest
        chatContext?.onNestedScrollFollowChange?.(followLatest)
    }, [chatContext])
    
    const claimNestedPointerScroll = () => {
        pointerActiveRef.current = true
        followLatestRef.current = false
        claimNestedScroll()
        // existing listener setup; clear pointerActiveRef before
        // syncNestedFollow() in finish
    }

Summary

  • Review mode: follow-up after new commits
  • The two prior Major findings are no longer present at the latest head; one pointer-gesture race remains.

Testing

  • Not run: bun is unavailable in the review runner (bun: command not found); GitHub test check was pending.
  • Add a regression test that starts streaming, fires pointer-down, rerenders with taller content, and asserts no true ownership release or inner auto-follow until pointer-up.

HAPI Bot

Comment thread web/src/components/assistant-ui/reasoning.tsx
@KorenKrita
KorenKrita force-pushed the agent/fix-reasoning-scroll-follow branch from 260cec3 to a87fbf1 Compare August 7, 2026 00:36

@github-actions github-actions 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.

Findings

  • [Major] Nested input still reaches the outer viewport before ownership is claimed — the new onWheel/onKeyDown/pointer handlers are React delegated handlers, but HappyThread registers native listeners directly on the ancestor viewport. The ancestor therefore handles an upward wheel/key/touch first and can call requestOlderRef.current('user'), so scrolling the reasoning panel near the chat preload boundary can unexpectedly load/trim outer history. Evidence web/src/components/assistant-ui/reasoning.tsx:202, related context web/src/components/AssistantChat/HappyThread.tsx:788, web/src/components/AssistantChat/HappyThread.tsx:866, and web/src/components/AssistantChat/HappyThread.tsx:888.
    Suggested fix:
    // reasoning.tsx
    <div data-hapi-nested-scroll="true" ... />
    
    // HappyThread.tsx, before every native key/pointer/wheel/touch handler
    const isNestedScrollEvent = (event: Event) =>
        event.target instanceof Element
        && event.target.closest('[data-hapi-nested-scroll="true"]') !== null
    
    const handleWheel = (event: WheelEvent) => {
        if (isNestedScrollEvent(event)) return
        // existing outer-scroll handling
    }
    Apply the same guard to keyboard, viewport/window pointer/mouse, and touch handlers; add an integrated nested-scroll test that asserts no outer history request.

Summary
Review mode: follow-up after new commits

The prior pointer-gesture race is fixed at the latest head. One Major integration regression remains because nested events are observed by the outer native listeners before the new React handlers run.

Testing

  • Not run (automation): bun is unavailable in this runner; the GitHub test check is currently pending.
  • Add an integrated HappyThread + reasoning-panel regression test for upward wheel, keyboard, and touch input while outer history coverage is eligible.

HAPI Bot

Comment thread web/src/components/assistant-ui/reasoning.tsx
@KorenKrita
KorenKrita force-pushed the agent/fix-reasoning-scroll-follow branch from a87fbf1 to 6a9d4d0 Compare August 7, 2026 00:45

@github-actions github-actions 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.

Findings

  • None.

Summary
Review mode: follow-up after new commits

No issues found in the latest full diff. The prior Major about nested input reaching outer native listeners is resolved by the new ancestor guards. Residual risk: the added HappyThread coverage unit-tests only the event-target predicate; it does not exercise the integrated outer history/follow-tail handlers.

Testing

  • Not run (static automation review; PR code was not executed).
  • GitHub test check was pending at review time.
  • Suggested: add an integrated HappyThread + ReasoningGroup regression asserting wheel, keyboard, pointer, and touch input inside the reasoning viewport cannot trigger outer history loading or follow-tail state changes.

HAPI Bot

@tiann
tiann merged commit 0a05fad into tiann:main Aug 7, 2026
2 checks passed
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.

Expanded streaming reasoning panel fights nested scrolling

2 participants