Skip to content

fix(ui): stabilize streaming chat scroll follow#375

Open
VooDisss wants to merge 1 commit intoNeuralNomadsAI:devfrom
VooDisss:upstream-dev-vfl-only-test
Open

fix(ui): stabilize streaming chat scroll follow#375
VooDisss wants to merge 1 commit intoNeuralNomadsAI:devfrom
VooDisss:upstream-dev-vfl-only-test

Conversation

@VooDisss
Copy link
Copy Markdown
Contributor

Summary

  • Fix sticky chat auto-scroll during streaming by making follow exit/re-entry more reliable in VirtualFollowList.
  • Prevent visible snapbacks by using DOM scroll state as the source of truth for immediate/manual scroll behavior.
  • Keep the upstream surface minimal: this change is scoped to packages/ui/src/components/virtual-follow-list.tsx only.

Testing

  • npm run typecheck --workspace @codenomad/ui
  • npm run build --workspace @codenomad/ui
  • Manually verified:
    • single-notch upward exit from auto-scroll
    • downward re-entry near bottom
    • normal browsing while escaped
    • no visible snapback during rapid streamed output

Review Notes

  • Maintainers may edit or adjust this PR as needed.
  • I will ignore gatekeeper bot reviews for this PR.

Stabilize the chat viewport scroll-follow behavior during streaming with a smaller, one-file fix centered on VirtualFollowList.

The original failure mode was that streaming output could keep follow mode sticky or trigger visible snapbacks when the user tried to inspect earlier parts of a long response. The important fixes are to treat upward wheel input as an immediate exit from follow, to allow native browsing while escaped, to rejoin follow only when the viewport deliberately returns near the bottom, and to rely on the DOM scroller as the source of truth when Virtua's internal scroll offset lags behind real manual movement.

This version keeps the fix narrowly scoped to virtual-follow-list.tsx. Immediate programmatic top/bottom moves use direct DOM scroll position writes, content-growth follow is coalesced through scrollHeight checks, and the state machine is trimmed down to the minimal follow/escape geometry that still preserves smooth exit, smooth re-entry, and no visible snapback during long streamed responses.
@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Apr 27, 2026

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented Apr 27, 2026

Gatekeeper review for latest PR state completed.

Request understood: review PR #375 ruthlessly for regressions and better implementation options. I treated this as review-only and did not make code changes.

Status: request changes

Findings:

  1. Blocking: the long-assistant-reply hold feature is now effectively dead code.

    message-section.tsx still computes and passes autoPinHoldTargetKey, autoPinHoldTopThresholdPx, and resolveAutoPinHoldElement into VirtualFollowList at packages/ui/src/components/message-section.tsx:638 and packages/ui/src/components/message-section.tsx:1089-1094. It also still renders the pause/hold toggle at packages/ui/src/components/message-section.tsx:1127-1144.

    But VirtualFollowList no longer reads those props. The old hold-target state and geometry logic were removed, while the public props remain in VirtualFollowListProps at packages/ui/src/components/virtual-follow-list.tsx:88-103.

    This is a behavior regression: users can toggle “hold long assistant replies”, the UI indicates it is active, but the list never honors the hold target. For long streaming assistant messages that exceed the viewport, the PR will keep forcing bottom follow instead of holding the growing message near the top edge. Either restore a minimal hold implementation or remove the preference/UI/props in the same PR. Leaving a live control backed by no behavior is not acceptable.

  2. Blocking: reset/session changes now reuse stale initial follow state.

    initialFollowEnabled is captured once at component initialization in packages/ui/src/components/virtual-follow-list.tsx:157, then reused on every reset in packages/ui/src/components/virtual-follow-list.tsx:471.

    Before this PR, reset used the current initialAutoScroll() value. That matters because message-section.tsx derives initialAutoScroll from the current session scroll snapshot at packages/ui/src/components/message-section.tsx:605-606, and VirtualFollowList explicitly supports resetKey for reusing the same list instance across different datasets.

    Regression scenario: switch from a session restored at bottom to a session restored away from bottom, or vice versa, while the same component instance survives. The reset path keeps the first session’s follow state instead of the new session snapshot, so subsequent streaming can unexpectedly auto-follow or fail to auto-follow.

    Suggested fix: do not cache initialAutoScroll() into a one-time initialFollowEnabled for reset logic. Compute the current initial follow state during reset, e.g. const followEnabled = Boolean(initialAutoScroll()), then sync both autoScroll and escaped/locked state from that current value.

  3. Major: setAutoScroll no longer maintains the internal escaped/follow-lock state.

    The public API implementation at packages/ui/src/components/virtual-follow-list.tsx:443 only calls setAutoScroll(Boolean(enabled)). The new follow model also depends on escapedFromLock, and several follow paths require !escapedFromLock() before scheduling bottom follow, e.g. packages/ui/src/components/virtual-follow-list.tsx:237 and packages/ui/src/components/virtual-follow-list.tsx:266.

    That means a consumer can call api.setAutoScroll(true) and observe getAutoScroll() === true, while the internal escaped state still prevents automatic bottom following on rendered content. This API is now misleading. It should route through the same state transitions as the rest of the component: enabling should rejoin follow, disabling should escape follow.

Implementation quality notes:

  • The PR is correctly scoped to one component, but the implementation is not actually minimal because it removes an existing feature path while preserving the external API and UI affordance.
  • cancelPendingContentGrowthCheck() at packages/ui/src/components/virtual-follow-list.tsx:200 is unused. Not itself a behavioral blocker, but it is a sign the new frame-cancellation model needs cleanup before merge.
  • scrollToBottom(true) now writes element.scrollTop = element.scrollHeight directly at packages/ui/src/components/virtual-follow-list.tsx:369. That may be intentional to prefer DOM state, but with a virtualizer in the loop this needs confidence/testing around large virtualized histories. The stronger blocker remains the removed hold behavior.

Verification:

  • git diff --check origin/dev...HEAD: passed.
  • npm run typecheck --workspace @codenomad/ui: failed before giving a useful PR-specific signal due broad existing/missing dependency/type resolution issues in this checkout, including missing solid-js, @opencode-ai/sdk, and many unrelated implicit-any errors outside the touched file.
  • npm run build --workspace @codenomad/ui: failed because vite could not be resolved from the local workspace dependencies.

Gatekeeper verdict: do not merge as-is. The PR likely fixes one scroll snapback symptom, but it regresses the long-stream hold control and introduces inconsistent follow-state semantics. Fix the state reset/API issues and either restore or intentionally remove the hold feature/UI before this should pass review.

--
Yours,
CodeNomadBot

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.

2 participants