fix(ui): preserve visible chat stream text#89530
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 3, 2026, 4:25 AM ET / 08:25 UTC. Summary PR surface: Source +639, Tests +1148. Total +1787 across 13 files. Reproducibility: yes. for the narrowed WebChat state-retention bug: current main source clears visible stream state during history apply and terminal error cleanup, and the PR discussion includes before/after mocked browser proof. I did not rerun the broader Windows/LM Studio repro path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the controller-level stream-retention fix after current-head checks and maintainer review confirm the ordering/dedupe risk is acceptable; keep any broader Windows/LM Studio symptoms on the linked issue if they remain. Do we have a high-confidence way to reproduce the issue? Yes for the narrowed WebChat state-retention bug: current main source clears visible stream state during history apply and terminal error cleanup, and the PR discussion includes before/after mocked browser proof. I did not rerun the broader Windows/LM Studio repro path. Is this the best way to solve the issue? Yes, for the narrowed bug this is the right owner boundary: the fix lives in the WebChat controller/render reconciliation path rather than adding a loading-flag workaround. The remaining question is merge risk, not a better obvious implementation layer. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 34c3827290a7. Label changesLabel justifications:
Evidence reviewedPR surface: Source +639, Tests +1148. Total +1787 across 13 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3cefb8b05
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return messages.some((existing) => messageDisplaySignature(existing) === signature) | ||
| ? messages | ||
| : [...messages, message]; |
There was a problem hiding this comment.
Keep duplicate replies in later turns
This dedupes by display signature across the entire transcript, so a normal later turn whose assistant final has the same text as an earlier assistant message is dropped instead of appended (the final/aborted/error paths now call this helper). For example, if a user asks the same question twice and both final events contain OK, the second assistant bubble never appears even though it belongs after a new user message; limit this check to the current turn/recent preserved stream replacement rather than every prior message.
Useful? React with 👍 / 👎.
|
Implementation loop update for this PR. What changed:
Verification:
Known local note:
I did not merge this PR. |
|
Updated visual proof after the stream ordering fix. The mocked Gateway scenario forces a stale history refresh where the persisted user message has a newer timestamp than the live assistant stream. The DOM-order assertion passed for each capture: Before stale history refresh: After stale history refresh: Terminal error path: Upload verification: downloaded copies matched the local PNG SHA-256 hashes exactly. |
9999d6d to
890b86e
Compare
|
Land-ready proof for current head What changed:
Local proof run after rebase:
Live proof:
CI:
Known proof gap:
|
|
Merged.
Known remaining proof gap: true Windows IME and live LM Studio timing were not rerun for this merge; this lands the narrowed WebChat stream-retention fix. |



Opened on behalf of Onur Solmaz (
osolmaz).AI-assisted: yes.
Summary
WebChat could show assistant text, then make it disappear.
This keeps visible streamed assistant text on screen when stale history, tool-history catch-up, or terminal run events arrive.
The confirmed bug is in WebChat state reconciliation, not in local-model latency itself.
What Changed
The chat UI now has one place for deciding whether visible stream text should stay, be materialized into history, or be replaced by persisted messages.
That makes stale history reloads, terminal final/error/abort events, and tool-result catch-up follow the same rule.
ui/src/ui/chat/stream-reconciliation.tsfor stream/history/tool reconciliation.ui/src/ui/chat/stream-text.tsfor shared stream text normalization and matching.ui/src/ui/chat/tool-message-refs.tsso tool message ids/names are read through one typed helper instead of scattered field checks.chatStreamwhen a stale history reload does not contain replacement assistant text.build-chat-itemsso persisted tool-history rows and live tool-stream rows do not double-render.Linked Context
Closes #67035.
Related PR: #77418.
This follows the narrowed maintainer repro from
run_e7fc89b73ec0, where composer preservation passed but stale refresh and terminal-error stream retention failed.Real Behavior Proof
I tested the real WebChat path and the focused mocked browser paths.
The live smoke used the built Control UI from this branch with a real local OpenClaw gateway and the
testbobCodex/OpenAI agent config.127.0.0.1:18790, WebChat sessionmain, Codex/OpenAI agent runtime.pnpm ui:build.testbobconfig.http://127.0.0.1:18790/chat?session=main.Live smoke on PR 89530: reply with exactly "live smoke ok 89530".live smoke ok 89530appeared under the user prompt.SMOKE89530-LINE-* OKmarkers.provider=aws, leasecbx_126c45468a1a, runrun_e7fc89b73ec0failed 2 of 3 checks before this patch.Tests and Validation
The regression tests cover the state rules directly, and the browser tests cover the visible WebChat behavior.
Current-head CI is also green.
node scripts/run-vitest.mjs ui/src/ui/chat/tool-message-refs.test.ts ui/src/ui/controllers/chat.test.ts ui/src/ui/chat/build-chat-items.test.ts ui/src/ui/app-tool-stream.node.test.ts ui/src/ui/app-chat.test.ts --reporter=dotpnpm exec oxfmt --check ui/src/ui/chat/stream-reconciliation.ts ui/src/ui/chat/stream-text.ts ui/src/ui/chat/build-chat-items.ts ui/src/ui/controllers/chat.tspnpm lint:core -- ui/src/ui/chat/stream-reconciliation.ts ui/src/ui/chat/stream-text.ts ui/src/ui/chat/build-chat-items.ts ui/src/ui/controllers/chat.tsgit diff --checkpnpm ui:buildhttp://127.0.0.1:18790/chat?session=mainRegression coverage added or updated:
Risks
The main risk is message ordering or duplication in WebChat.
The refactor reduces that risk by centralizing the matching and materialization rules instead of keeping separate controller-only checks.
Current Review State
ClawSweeper has no concrete blocker on current head
9999d6dde242cdc4d9053fcfb0fd4ae211f7b707.GitHub reports the PR as mergeable and clean.
Current-head checks are green: 133 success, 25 skipped, 1 neutral, 0 failing, 0 pending.