Skip to content

fix(ui): preserve visible chat stream text#89530

Merged
osolmaz merged 24 commits into
mainfrom
codex/67035-webchat-stream-retention
Jun 3, 2026
Merged

fix(ui): preserve visible chat stream text#89530
osolmaz merged 24 commits into
mainfrom
codex/67035-webchat-stream-retention

Conversation

@osolmaz

@osolmaz osolmaz commented Jun 2, 2026

Copy link
Copy Markdown
Member

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.

  • Added ui/src/ui/chat/stream-reconciliation.ts for stream/history/tool reconciliation.
  • Added ui/src/ui/chat/stream-text.ts for shared stream text normalization and matching.
  • Added ui/src/ui/chat/tool-message-refs.ts so tool message ids/names are read through one typed helper instead of scattered field checks.
  • Kept visible chatStream when a stale history reload does not contain replacement assistant text.
  • Materialized visible streamed assistant text before terminal final, aborted, or error cleanup.
  • Pruned live tool cards only after persisted history includes the same tool ids.
  • Reworked build-chat-items so persisted tool-history rows and live tool-stream rows do not double-render.
  • Replaced the earlier controller-local patch with smaller helpers and focused tests around each rule.

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 testbob Codex/OpenAI agent config.

  • Behavior addressed: visible WebChat assistant stream text disappearing after stale history refreshes, tool-history catch-up, or terminal chat events.
  • Real environment tested: macOS local OpenClaw checkout, built Control UI, real local gateway on 127.0.0.1:18790, WebChat session main, Codex/OpenAI agent runtime.
  • Live smoke after the refactors:
    • Built current UI with pnpm ui:build.
    • Started gateway from this checkout with the testbob config.
    • Opened http://127.0.0.1:18790/chat?session=main.
    • Sent Live smoke on PR 89530: reply with exactly "live smoke ok 89530".
    • Verified the assistant reply live smoke ok 89530 appeared under the user prompt.
    • Reloaded the page and verified the user prompt and assistant reply rehydrated correctly.
    • Sent a longer 12-line response using SMOKE89530-LINE-* OK markers.
    • Reloaded again and verified all 12 assistant lines rehydrated correctly.
  • Before evidence: Crabbox AWS browser repro provider=aws, lease cbx_126c45468a1a, run run_e7fc89b73ec0 failed 2 of 3 checks before this patch.
  • What was not tested: true Windows IME input and live LM Studio/local-model streaming timing.
  • Proof limitation: the live model completed quickly, so the live smoke proves completed-output/history rehydration. The mid-token stream interruption case is covered by the mocked browser/controller tests.

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=dot
  • pnpm 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.ts
  • pnpm 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.ts
  • git diff --check
  • pnpm ui:build
  • Live WebChat smoke through http://127.0.0.1:18790/chat?session=main

Regression coverage added or updated:

  • Error events preserve partial streamed assistant text before clearing stream state.
  • Error events avoid duplicating streamed text when the server already supplied the same assistant message.
  • Stale history reloads keep active visible streamed text.
  • Stale history reloads materialize orphaned streamed text when no active run remains.
  • Caught-up history clears streamed text when it contains the replacement assistant message.
  • Tool-history catch-up clears live tool cards only when matching persisted tool ids exist.
  • Mocked browser E2E verifies visible stream text survives stale startup history and terminal error.

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.

  • User-visible behavior changed: Yes.
  • Config, environment, or migration behavior changed: No.
  • Security, auth, secrets, network, or tool execution behavior changed: No.
  • Highest-risk area: WebChat message reconciliation around stream/history/tool ordering.
  • Risk mitigation: focused controller tests, tool-message tests, build-chat-items tests, mocked browser E2E, green CI, and live WebChat smoke.

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.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: M maintainer Maintainer-authored PR labels Jun 2, 2026
@clawsweeper

clawsweeper Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 3, 2026, 4:25 AM ET / 08:25 UTC.

Summary
The PR adds WebChat stream-reconciliation helpers and tests so streamed assistant text and tool-adjacent stream segments survive stale history reloads and terminal error/final/abort cleanup.

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: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Verify current-head checks on 9999d6d.
  • Optionally run the broader Windows/LM Studio timing path before closing the linked issue.

Mantis proof suggestion
A short browser visual proof would be useful because the PR changes visible WebChat message retention after stale history and terminal errors. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify WebChat keeps streamed assistant text visible after stale history refresh and after a terminal chat error.

Risk before merge

  • [P1] This changes WebChat stream/history/tool reconciliation, so an edge-case mistake could duplicate, reorder, or suppress assistant text or tool cards.
  • [P1] The proof covers the narrowed mocked Gateway/browser scenarios, not the original Windows 10 plus LM Studio timing path from the linked report.
  • [P1] The latest head is newer than the green-check comment in the discussion, so current-head checks should be verified before merge.

Maintainer options:

  1. Merge after current-head proof (recommended)
    If checks are green at head 9999d6d and maintainers accept the focused mocked browser proof, land this as the narrowed stream-retention fix.
  2. Request broader live timing proof
    If maintainers want confidence against the original broader report, ask for a live Windows/LM Studio or Crabbox timing run before merge.
  3. Pause for related PR ownership
    If maintainers prefer a smaller or alternate path, pause this until the overlap with fix(web-chat): prevent blank chat pane after assistant response (#67035) #77418 and fix(ui): deduplicate assistant messages on chat.status final event #86646 is resolved.

Next step before merge

  • [P2] Protected maintainer PR with no narrow mechanical repair finding; the next action is maintainer merge judgment plus current-head verification.

Security
Cleared: The diff touches UI/controller/tests and a shared tool-content helper only; I found no dependency, CI, secret, permission, or package-resolution change.

Review details

Best 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 changes

Label justifications:

  • P1: The PR addresses a user-visible WebChat regression where assistant stream text can disappear during normal chat use.
  • merge-risk: 🚨 message-delivery: The diff changes message reconciliation around stale history, terminal events, and tool-stream catch-up, which can affect whether assistant text is delivered, duplicated, or ordered correctly.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body and follow-up screenshots show after-fix mocked browser behavior for stale-history and terminal-error stream retention, with no contributor action needed for proof.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up screenshots show after-fix mocked browser behavior for stale-history and terminal-error stream retention, with no contributor action needed for proof.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body and follow-up screenshots show after-fix mocked browser behavior for stale-history and terminal-error stream retention, with no contributor action needed for proof.
Evidence reviewed

PR surface:

Source +639, Tests +1148. Total +1787 across 13 files.

View PR surface stats
Area Files Added Removed Net
Source 7 713 74 +639
Tests 6 1157 9 +1148
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 13 1870 83 +1787

What I checked:

  • Current main reload behavior: Current main applies fetched history, then clears chatStream/chatStreamStartedAt whenever the active run is considered reset, which matches the stale-history disappearance path this PR changes. (ui/src/ui/controllers/chat.ts:710, 34c3827290a7)
  • Current main terminal error behavior: Current main's error branch appends only the error assistant message and does not materialize the partial visible stream before reconciling the terminal run. (ui/src/ui/controllers/chat.ts:1171, 34c3827290a7)
  • PR history reconciliation path: At PR head, loadChatHistory checks whether history replaced visible stream/tool state and preserves or materializes visible stream text instead of always clearing it. (ui/src/ui/controllers/chat.ts:730, 9999d6dde242)
  • PR terminal reconciliation path: At PR head, final/aborted/error events route visible stream text through materializeVisibleAssistantStreamMessages and appendTerminalAssistantMessage before run cleanup. (ui/src/ui/controllers/chat.ts:1203, 9999d6dde242)
  • PR shared helper behavior: The new helper inserts visible stream fallback messages near matching tool rows, timestamps them between neighboring messages when needed, and prunes live tool stream state once persisted tool ids arrive. (ui/src/ui/chat/stream-reconciliation.ts:375, 9999d6dde242)
  • Visual proof inspected: The linked PNG proof downloaded successfully; the after-refresh screenshot shows the user prompt above retained assistant partial output, and the terminal-error screenshot shows retained assistant partial output above the error bubble.

Likely related people:

  • @vincentkoc: Current main blame for the loadChatHistory, handleChatEvent, buildChatItems, app-tool-stream, and tool-content surfaces under review points to recent Control UI consolidation and a follow-up startup metadata commit. (role: recent area contributor; confidence: high; commits: ac8338bb026d, c0b05a2100d3; files: ui/src/ui/controllers/chat.ts, ui/src/ui/chat/build-chat-items.ts, ui/src/ui/app-tool-stream.ts)
  • @steipete: Git history shows earlier WebChat tool-run stabilization and live tool-stream work on the same controller/tool-stream boundary. (role: feature history owner; confidence: medium; commits: de2ccffec166, b7e708c764b0, e3ff8c4d288b; files: ui/src/ui/controllers/chat.ts, ui/src/ui/app-tool-stream.ts, ui/src/ui/chat/build-chat-items.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread ui/src/ui/controllers/chat.ts Outdated
Comment on lines +205 to +207
return messages.some((existing) => messageDisplaySignature(existing) === signature)
? messages
: [...messages, message];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 2, 2026
@osolmaz

osolmaz commented Jun 2, 2026

Copy link
Copy Markdown
Member Author

Implementation loop update for this PR.

What changed:

  • WebChat now keeps visible streamed assistant text through stale history reloads, terminal error/abort/final events, and tool-result history catch-up.
  • Recovered stream segments are materialized before the matching persisted tool rows, keep their original segment timestamps, and carry the tool id they originally preceded so multi-tool turns stay ordered.
  • Live tool cards stay visible until history includes the current tool IDs, and accumulated live stream text is trimmed to the visible tail when caught-up tools are cleared.
  • Terminal assistant payloads replace marked stream fallback messages instead of duplicating them, while normal repeated assistant replies are preserved.

Verification:

  • node scripts/run-vitest.mjs ui/src/ui/controllers/chat.test.ts ui/src/ui/app-tool-stream.node.test.ts --reporter=dot - 23 tool-stream tests passed, 113 controller tests passed.
  • OPENCLAW_VITEST_MAX_WORKERS=2 OPENCLAW_VITEST_SHARD_NAME=core-unit-ui OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=300000 OPENCLAW_VITEST_NO_OUTPUT_RETRY=1 OPENCLAW_TEST_PROJECTS_PARALLEL=2 pnpm exec node scripts/test-projects.mjs test/vitest/vitest.unit-ui.config.ts - 626 passed, 20 skipped.
  • PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" node scripts/run-vitest.mjs ui/src/ui/e2e/chat-flow.e2e.test.ts --reporter=dot - 12 passed.
  • node scripts/run-vitest.mjs ui/src/ui/controllers/chat.test.ts ui/src/ui/app-gateway-chat-load.node.test.ts ui/src/ui/app-gateway.node.test.ts ui/src/ui/app-tool-stream.node.test.ts --reporter=dot - 95 UI tests passed, 113 controller tests passed.
  • Direct touched-file style checks passed: pnpm exec oxlint ..., git diff --check, and pnpm exec oxfmt --check --threads=1 ....
  • codex review --base origin/main on head 38fdc4c934 - no actionable regressions found.
  • PR CI is green on head 38fdc4c934, including checks-node-core-ui, check-lint, check-prod-types, check-test-types, checks-windows-node-test, and Real behavior proof.

Known local note:

  • The wrapper node scripts/run-oxlint.mjs ... hit an unrelated local plugin-sdk boundary dts prep failure before checking these files, while direct touched-file style checks and PR CI lint passed.

I did not merge this PR.

@osolmaz

osolmaz commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

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: user,assistant.

Before stale history refresh:

Before stale history refresh: user prompt above live assistant stream

After stale history refresh:

After stale history refresh: user prompt remains above live assistant stream

Terminal error path:

Terminal error path: user prompt remains above partial assistant output and error

Upload verification: downloaded copies matched the local PNG SHA-256 hashes exactly.

@osolmaz osolmaz assigned BunsDev and unassigned BunsDev Jun 3, 2026
@osolmaz
osolmaz requested a review from BunsDev June 3, 2026 06:07
@clawsweeper clawsweeper Bot added the proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. label Jun 3, 2026
@osolmaz
osolmaz force-pushed the codex/67035-webchat-stream-retention branch from 9999d6d to 890b86e Compare June 3, 2026 08:51
@osolmaz

osolmaz commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

Land-ready proof for current head 890b86e565d53ea6fb338bee0bbb12216bf62131.

What changed:

  • WebChat keeps visible streamed assistant text through stale history reloads, terminal final/error/abort events, and tool-history catch-up.
  • The final implementation is split into stream-reconciliation.ts, stream-text.ts, and tool-message-refs.ts, with build-chat-items using the same typed tool-message references.
  • Rebased onto current origin/main and resolved the terminal-event overlap from main.

Local proof run after rebase:

  • 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=dot — passed, 2 shards, 257 tests total.
  • pnpm exec oxfmt --check ui/src/ui/controllers/chat.test.ts 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.ts — passed.
  • git diff --check — passed.

Live proof:

  • Built the current Control UI with pnpm ui:build.
  • Started a real local gateway from this checkout with the testbob config.
  • Opened http://127.0.0.1:18790/chat?session=main.
  • Sent a short live message and verified live smoke ok 89530 rendered under the user prompt.
  • Reloaded and verified that prompt and assistant reply rehydrated correctly.
  • Sent a longer 12-line marker response and verified all 12 assistant lines survived reload.

CI:

  • Current-head GitHub checks are green enough to land: 133 success, 27 skipped, 1 neutral, 0 failing, 0 pending.
  • The transient Scan changed paths (precise) failure was an OpenGrep install fetch failure; rerun passed.

Known proof gap:

  • I did not run true Windows IME or live LM Studio timing. The live smoke covers real WebChat/gateway completed-output rehydration; the mocked/controller tests cover the mid-stream interruption paths.

@osolmaz
osolmaz merged commit 17a285f into main Jun 3, 2026
161 of 162 checks passed
@osolmaz
osolmaz deleted the codex/67035-webchat-stream-retention branch June 3, 2026 08:56
@osolmaz

osolmaz commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

Merged.

  • Squash commit: 17a285f
  • Head merged: 890b86e
  • Behavior landed: WebChat keeps visible assistant stream text through stale history reloads, tool-history catch-up, and terminal final/error/abort events.
  • Proof before merge: local focused UI/chat tests passed, formatting/diff checks passed, live WebChat smoke passed, and current-head GitHub checks were green with 133 success, 27 skipped, 1 neutral, 0 failing, 0 pending.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui maintainer Maintainer-authored PR merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 2026.4.14 Windows chat UI regression: input text swallowed, streamed replies often invisible until refresh, typing indicator flashes then blanks

2 participants