fix(ui): deduplicate streaming chat segments to prevent growing duplicate bubbles#47399
fix(ui): deduplicate streaming chat segments to prevent growing duplicate bubbles#47399LittleBreak wants to merge 1 commit into
Conversation
…cate bubbles (openclaw#47188) The gateway sends the full accumulated assistant text in each streaming delta event. When a tool call interrupts the stream, the client saves the current chatStream as a segment. Subsequent deltas (still containing the full accumulated text) then show overlapping content: each segment contains all prior text, and the live stream bubble repeats it again. This was previously masked by a loadChatHistory() call after every tool result (removed in 0e8672a to fix a reload storm). Fix: track chatStreamSegmentOffset — the length of text already committed to segments. When saving a new segment, slice off the known prefix to store only the delta. When rendering the live stream bubble, strip the offset prefix so only new text appears. Adds test coverage for the segment dedup in chat.test.ts.
Greptile SummaryThis PR fixes a pre-existing segment duplication bug (#47188) where the gateway's full-accumulated-text streaming model caused each committed segment to contain incrementally more text from previous segments, producing growing duplicate bubbles after tool calls. The fix introduces a The core logic is sound: all paths that reset streaming state ( Key observations:
Confidence Score: 3/5
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 567141324a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (deltaText.trim().length > 0) { | ||
| host.chatStreamSegments = [...host.chatStreamSegments, { text: deltaText, ts: now }]; | ||
| } | ||
| host.chatStreamSegmentOffset = host.chatStream.length; |
There was a problem hiding this comment.
Preserve whitespace-only deltas when committing segments
When a tool starts, the new logic drops deltaText if it is whitespace-only (deltaText.trim().length === 0) but still advances chatStreamSegmentOffset to the full stream length. If the only text emitted between two tool calls is a space or newline, that separator is permanently discarded, so subsequent streamed text can be concatenated without intended spacing/formatting (for example "Hello" + " world" becomes "Helloworld"). This regression is specific to runs that interleave tool events with whitespace-only assistant increments.
Useful? React with 👍 / 👎.
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as superseded: the WebChat cumulative-stream bug is still real on current main, but open PR #46985 already tracks the same remaining fix with a cleaner current-main implementation, while this PR has concrete merge blockers and an unrelated binary artifact. So I’m closing this here and keeping the remaining discussion on the canonical linked item. Review detailsBest possible solution: Use #46985, or an equivalent maintainer-selected branch, as the canonical fix: it should dedupe both committed stream segments and the live preview on the current Do we have a high-confidence way to reproduce the issue? Yes. The source-level reproduction path is high-confidence: the gateway sends cumulative chat delta text, the UI stores that cumulative text as Is this the best way to solve the issue? No. The offset/suffix approach is directionally right, but this branch is not the best current solution because it drops whitespace-only suffixes, includes an unrelated binary artifact, and targets an outdated render path; #46985 is the cleaner remaining implementation to review. Security review: Security review needs attention: The TypeScript streaming changes do not broaden permissions or secrets, but the PR adds an unrelated binary workspace artifact that should be removed rather than merged.
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against c4a4c189f11e. |
Summary
Fixes #47188 — webchat streaming responses create multiple duplicate message bubbles with growing text instead of updating a single bubble in-place.
Root Cause
The gateway sends the full accumulated assistant text in each streaming delta event. When a tool call interrupts the stream,
app-tool-stream.tssaves the currentchatStream(full accumulated text) as a segment. After the tool completes, new delta events arrive containing the full text again (including text already in segments), causing each segment to show incrementally more duplicated text.This was previously masked by a
loadChatHistory()call after every tool result (removed in 0e8672a to fix a reload storm). Removing that call exposed this pre-existing segment duplication bug.Fix
Introduce
chatStreamSegmentOffsetto track the length of text already committed to segments:app-tool-stream.ts: When saving a segment on tool start, usechatStream.slice(offset)to store only the delta text (not the full accumulated text). Updateoffset = chatStream.lengthafter each save.views/chat.ts: When rendering the live stream bubble, usestream.slice(offset)to display only new text after the last segment.resetToolStream().Files Changed (8)
ui/src/ui/app-tool-stream.tsui/src/ui/views/chat.tsstreamSegmentOffsetpropui/src/ui/app.ts@state() chatStreamSegmentOffsetui/src/ui/app-view-state.tsui/src/ui/app-render.tsui/src/ui/views/chat.test.tsui/src/ui/app-tool-stream.node.test.tsui/src/ui/app-gateway.node.test.tsTesting