fix(vscode): eliminate streaming transcript flicker - #13408
Conversation
5298d4f to
c516947
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous Review Summaries (3 snapshots, latest commit 2aab3fb)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 2aab3fb)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit e23312b)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous review (commit 378c187)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (7 files)
Reviewed by grok-4.6 · Input: 33.4K · Output: 5.2K · Cached: 214.9K Review guidance: REVIEW.md from base branch |
2aab3fb to
9bc84d2
Compare
iscekic
left a comment
iscekic
left a comment
There was a problem hiding this comment.
Approved, bot remarks below, feel free to ignore
| // because the final content size is unchanged no resize entry follows, so | ||
| // the correction has to happen here or the transcript stays parked below | ||
| // its bottom until the next content update. | ||
| if (active()) bottom() |
There was a problem hiding this comment.
medium: The new re-pin calls bottom() on every non-input scroll event while the session is active (working or settling), which includes user scrolls that are never marked as input — a native scrollbar drag dispatches no pointer events, so mark() in scroll-user-activity.ts never fires, and the search-highlight scrollIntoView at webview-ui/src/components/chat/MessageList.tsx:879 never calls pause(). On classic-scrollbar platforms the view is therefore yanked back to the bottom on every scroll event, so a user cannot scroll up to read earlier output during a stream or within the 300ms settle window, and in-stream search match centering is defeated (the old code left these non-input scrolls in place and re-pinned only on content resize). Fix: re-pin only when the scroll follows content growth (compare scrollHeight to the last pinned value) and treat an unmarked scroll away from the bottom as a pause.
| // lays out and paints. A ResizeObserver entry arrives after that layout, so | ||
| // waiting for it lets the browser paint one frame with the new content hanging | ||
| // below the viewport, which reads as the transcript twitching as it streams. | ||
| const onContentMutate = () => { |
There was a problem hiding this comment.
low: The new MutationObserver invokes follow() for every childList/characterData mutation of the whole transcript while the session is active, and each invocation forces a synchronous layout through the distanceFromBottom() read (plus a second one through bottom()'s scrollHeight read when the pin fires). During streaming this runs once per streamed chunk on top of the virtualizer's own re-measurement work, so a large transcript can jank at the very moment the change is meant to make it feel smooth. Fix: coalesce the pins per frame, e.g. schedule follow() once via requestAnimationFrame or a microtask flag rather than per mutation.
2 participants
Streaming session transcripts could visibly twitch while reasoning and tool output arrived. The viewport was usually at the bottom, but the browser briefly painted newly mounted content below the visible area before auto-follow corrected
scrollTop. Tool cards and reasoning blocks also changed their DOM shape during the same interval, which caused short height pulses, reasoning overflow during collapse, cursor changes under a parked pointer, and copy controls to mount and unmount between streamed text parts.The key timing problem was the existing
ResizeObserverpath. It observes the result of layout, so it runs after a DOM mutation has already been laid out. By then, one frame could paint with a non-zero distance from the bottom. The previous deferred-follow experiment made this worse by intentionally waiting another frame for every growth event. A transient shrink could also make the browser clampscrollTopupward, then recover before a resize notification was delivered, leaving the transcript temporarily off the bottom.This change observes transcript DOM mutations before layout is painted and follows immediately when the user has not paused auto-follow. It retains the resize path for non-DOM size changes and restores the bottom when a layout correction moves the viewport without changing the final content size. User wheel, pointer, touch, keyboard, scrollbar, and nested-scroll handling remain separate, so reading earlier output is not interrupted.
The rendering fixes address the other sources of visible churn:
This follows the earlier scroll fixes rather than replacing them:
fix(ui): prevent snap-to-bottom and flickering during upward session scroll, stopped inactive resize handling from fighting upward user scrolling and established input-aware auto-follow behavior.fix(ui): preserve streaming chat scroll position, separated streaming/layout reflow from user movement and covered native scrollbar and touch interactions.fix(vscode): keep session scroll pinned during layout corrections, stopped stable-height layout corrections from being misclassified as user scrolling.Those PRs fixed incorrect intent classification. This change fixes the remaining browser timing gap between DOM mutation, layout, and auto-follow correction.