Skip to content

fix(gateway): accept delta-only assistant events for live chat projection#83038

Closed
jackjin1997 wants to merge 1 commit into
openclaw:mainfrom
jackjin1997:fix/82988-delta-only-assistant
Closed

fix(gateway): accept delta-only assistant events for live chat projection#83038
jackjin1997 wants to merge 1 commit into
openclaw:mainfrom
jackjin1997:fix/82988-delta-only-assistant

Conversation

@jackjin1997
Copy link
Copy Markdown
Contributor

@jackjin1997 jackjin1997 commented May 17, 2026

Summary

src/gateway/server-chat.ts and src/tui/embedded-backend.ts both gated live chat projection on a cumulative data.text string, dropping canonical assistant events that only carry an incremental data.delta. Live UIs lost working/streaming state between run start and final response.

Add a small hasLiveAssistantContent helper in src/gateway/live-chat-projector.ts so the gate accepts either text or delta. emitChatDelta and resolveMergedAssistantText already merge delta-only inputs correctly, so the change is limited to the two gates plus their import.

Fixes #82988

Real behavior proof

Behavior addressed: Gateway and embedded TUI live chat projection drop assistant events that only carry data.delta. After this patch, both gates accept either data.text or data.delta, and the existing resolveMergedAssistantText buffer keeps streaming deltas appended into the live chat surface.

Real environment tested: Local macOS checkout of openclaw/openclaw in a git worktree based on origin/main 2c9f68f42b, Node v25.5.0, tsx loader from the repo's node_modules.

Exact steps or command run after this patch:

node --import tsx .artifacts/probe-82988.mjs

The probe imports the patched src/gateway/live-chat-projector.ts directly and exercises (a) the new hasLiveAssistantContent predicate that both gates now consult, and (b) the existing resolveMergedAssistantText buffer with a delta-only stream that simulates the exact event shape from the issue.

Evidence after fix: Live stdout from the probe (real node runtime, no test framework):

PASS text-only event accepted -> true (expected true)
PASS delta-only event accepted (the #82988 case) -> true (expected true)
PASS text+delta event accepted -> true (expected true)
PASS event without text or delta rejected -> false (expected false)
PASS null data rejected -> false (expected false)
PROBE buffered live chat from delta-only stream: {"firstChunk":"hello","secondChunk":"hello world"}

Result: 5 passed, 0 failed

Pre-patch, the delta-only event would have failed the typeof evt.data?.text === "string" gate in both server-chat.ts and embedded-backend.ts, so the chat projection layer would never reach emitChatDelta and the buffered firstChunk / secondChunk would stay empty.

Observed result after fix: Assistant events that only carry data.delta now pass the live chat gate and are appended to the per-run buffer through the existing resolveMergedAssistantText merge path. A delta-only stream of \"hello\" followed by \" world\" accumulates to \"hello world\" in the live chat surface instead of being dropped.

What was not tested: End-to-end live run through a real provider that emits delta-only assistant events; the unaffected throttle/coalesce path (isAgentTextThrottleEvent) intentionally still requires data.text because the issue is scoped to live chat projection.

Verification

  • node scripts/run-vitest.mjs src/gateway/live-chat-projector.test.ts src/tui/embedded-backend.test.ts -> 4 files, 38/38 pass (covers the new hasLiveAssistantContent cases, the resolveMergedAssistantText delta-only path, and a new TUI test emits a chat delta for assistant events that only carry delta with no text).
  • node --import tsx .artifacts/probe-82988.mjs -> 5/5 pass (output above).

The probe script lives at .artifacts/probe-82988.mjs in the contributor environment only; it is not part of the committed diff.

…tion

Both gateway/server-chat and tui/embedded-backend gated chat projection
on a cumulative data.text string, dropping canonical assistant events that
only carry an incremental data.delta. Live UIs lost working/streaming state
between run start and final response.

Add hasLiveAssistantContent helper so the gate accepts either text or
delta. emitChatDelta already handles delta-only inputs via
resolveMergedAssistantText.

Fixes openclaw#82988
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 17, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 17, 2026

Codex review: needs maintainer review before merge.

Summary
The PR adds a shared live assistant content predicate, uses it in Gateway and embedded TUI chat projection gates, and adds focused delta-only regression coverage.

Reproducibility: yes. source-level reproduction is high confidence: current main requires data.text in both projection gates while the SDK accepts string data.delta as an assistant delta. I did not execute the runtime path in this read-only review.

Real behavior proof
Sufficient (terminal): The PR body includes after-fix terminal output from a real Node probe that exercises the changed projection predicate and delta merge path; it does not include full live-provider E2E proof.

Next step before merge
No automated repair is needed; maintainers should review CI/Testbox results and decide whether to land the focused PR.

Security
Cleared: The diff only changes internal event projection logic and tests, with no dependency, workflow, secret-handling, permission, or external code execution changes.

Review details

Best possible solution:

Land the focused gate normalization with regression coverage once the normal CI and maintainer merge checks pass, preserving the existing SDK contract for delta-only assistant chunks.

Do we have a high-confidence way to reproduce the issue?

Yes, source-level reproduction is high confidence: current main requires data.text in both projection gates while the SDK accepts string data.delta as an assistant delta. I did not execute the runtime path in this read-only review.

Is this the best way to solve the issue?

Yes, the proposed approach is the narrowest maintainable fix I found: admit text-or-delta assistant content at the two gates and reuse the existing normalization and merge helpers. A broader provider or SDK change is not needed for this bug.

Acceptance criteria:

  • node scripts/run-vitest.mjs src/gateway/live-chat-projector.test.ts src/tui/embedded-backend.test.ts
  • Relevant CI or changed checks before merge

What I checked:

  • Current Gateway gate drops delta-only assistant events: Current main only calls live chat projection when assistant events include string data.text, so a supported event with only data.delta does not reach emitChatDelta. (src/gateway/server-chat.ts:953, 8dd91b14d3c0)
  • Current embedded TUI gate has the same contract mismatch: The embedded TUI path also requires string data.text before merging and emitting live chat deltas. (src/tui/embedded-backend.ts:531, 8dd91b14d3c0)
  • Delta-only assistant events are a supported SDK shape: The SDK normalizer classifies assistant stream events with string data.delta as assistant.delta, so the PR aligns chat projection with the existing event contract. (packages/sdk/src/normalize.ts:58, 8dd91b14d3c0)
  • Existing merge helper already handles delta-only text: resolveMergedAssistantText appends nextDelta when nextText is empty, so the patch can reuse the existing buffering path once the gates admit the event. (src/gateway/live-chat-projector.ts:37, 8dd91b14d3c0)
  • PR changes the two gates through a shared predicate: The PR commit adds hasLiveAssistantContent and uses it in both Gateway and embedded TUI, passing an empty text string plus the original delta when cumulative text is absent. (src/gateway/server-chat.ts:957, 69dcd56abbaa)
  • PR adds focused regression coverage: The branch tests delta-only helper behavior and an embedded TUI assistant event with { delta: "hello" } producing a chat delta payload. (src/tui/embedded-backend.test.ts:243, 69dcd56abbaa)

Likely related people:

  • steipete: Blame and recent history tie the current Gateway/TUI projection gate and shared projector helper to Peter Steinberger, and shortlog shows the heaviest recent activity in these files. (role: feature-history owner; confidence: high; commits: e4ec1b3de8af, 42f9737e592f, 43b36bfe8cc7; files: src/gateway/server-chat.ts, src/tui/embedded-backend.ts, src/gateway/live-chat-projector.ts)
  • vignesh07: Recent history shows Vignesh Natarajan worked on preserving streamed prefixes and filtering heartbeat chat noise in the same live chat merge/projection area. (role: adjacent stream-merge contributor; confidence: medium; commits: d326861eb470, 2227840989c1, fc8f59261a4c; files: src/gateway/server-chat.ts, src/tui/embedded-backend.ts, src/gateway/live-chat-projector.ts)

Remaining risk / open question:

  • I did not run the reported focused tests because this was a read-only sweep; merge should still rely on CI or maintainer-run validation.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 8dd91b14d3c0.

@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. impact:message-loss Channel message delivery can be lost, duplicated, or misrouted. impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. labels May 17, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 17, 2026
@jackjin1997
Copy link
Copy Markdown
Contributor Author

Updated the PR body with a Real behavior proof section: ran an inline node --import tsx .artifacts/probe-82988.mjs runtime probe against the patched src/gateway/live-chat-projector.ts and captured the live stdout (5/5 cases pass, plus a delta-only stream buffering to "hello world"). Real behavior proof check is green on the new evaluation.

@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 17, 2026
@jackjin1997
Copy link
Copy Markdown
Contributor Author

Closing this as stale — 17 days with 0 maintainer activity, well past the typical review window for this repo. The delta-only event projection fix in live-chat-projection.ts may still be relevant for the gateway live chat path; happy to reopen or resubmit if it surfaces again. Closing to keep the queue clean.

@jackjin1997 jackjin1997 closed this Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime impact:message-loss Channel message delivery can be lost, duplicated, or misrouted. impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Gateway and embedded TUI ignore delta-only assistant events for live chat projection

1 participant