Skip to content

fix(ai): assistant text blocks are run together on replay - #115743

Merged
steipete merged 2 commits into
openclaw:mainfrom
Yigtwxx:fix/completions-text-block-separator
Aug 1, 2026
Merged

fix(ai): assistant text blocks are run together on replay#115743
steipete merged 2 commits into
openclaw:mainfrom
Yigtwxx:fix/completions-text-block-separator

Conversation

@Yigtwxx

@Yigtwxx Yigtwxx commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where users on any OpenAI-compatible provider would have the assistant's own prior turn replayed back to the model with sentences fused together, losing the boundary between them.

An assistant turn holding two text blocks, "Let me check the file." and "The file contains X.", was replayed as:

Let me check the file.The file contains X.

Nothing errors and nothing is logged. The corrupted text is what the model reads as its own previous turn on every subsequent request in the conversation, so the damage compounds as the transcript grows.

This affects the Chat Completions lane generally: OpenRouter, Groq, DeepSeek, Together, LM Studio, Ollama and other OpenAI-compatible endpoints.

Why This Change Was Made

Two text blocks in one assistant turn is a routine shape, not a corner case:

  • Streaming opens a new text block whenever the current block is not text, so a text → tool call → text turn ends with two separate text blocks.
  • Cross-model replay converts a thinking block into a text block, which is then adjacent to the real answer block.

convertMessages flattened them with join(""). Every neighbouring path disagrees with that choice:

  • the thinking blocks a few lines below in the same function join with "\n\n", and the thinking signature joins with "\n"
  • flattenCompletionMessagesToStringContent, the helper that performs this very operation for strict OpenAI-compatible servers, joins with "\n"
  • the Anthropic, Responses and Mistral lanes never flatten at all; they emit one content block per block

This change joins with "\n", matching the string-content flattener, which is the closest sibling: same input shape, same output shape, same purpose. "\n\n" was the alternative, but it is used for thinking blocks rather than for flattening text parts, so the narrower precedent seemed the safer one to follow. Happy to switch if maintainers prefer the paragraph break.

Nothing else changes: block filtering, surrogate sanitization, the thinking paths, and the content-part array shape are all untouched.

User Impact

Assistant turns that contain more than one text block are replayed with their boundaries intact, so the model sees what it actually said instead of two sentences fused mid-word. Turns with a single text block, which is the common case, serialize byte-for-byte as before.

Evidence

Real runtime proof: what an OpenAI-compatible server actually receives

Requested by review — this goes past Vitest output and captures the bytes on the wire.

A local node:http server stands in for the provider endpoint: model.baseUrl points at http://127.0.0.1:<port>/v1, the server parses the real request body, records messages[], and answers with a real SSE stream that the transport parses normally. No fetch stub, no mocked client — a real socket, the real streamOpenAICompletions path, the real serializer. It needs no provider account.

The conversation contains one assistant turn with two text blocks. Only openai-completions-messages.ts differs between the two runs.

source assistant.content received by the server
current main "Let me check the file.The file contains X."
this PR "Let me check the file.\nThe file contains X."

Terminal output, unfixed source first:

--- source reverted to main ---
=== what the OpenAI-compatible server received on the wire ===
assistant content: "Let me check the file.The file contains X."
contains newline between blocks: false
fused (bug): true
RESULT: fused

and with the patch applied:

=== what the OpenAI-compatible server received on the wire ===
assistant content: "Let me check the file.\nThe file contains X."
contains newline between blocks: true
fused (bug): false
RESULT: separated

The script exits on a boolean computed from the captured payload rather than printing a fixed "PASS", so a regression cannot pass silently.

Harness (run with tsx, not committed)
const server = createServer((req, res) => {
  const chunks: Buffer[] = [];
  req.on("data", (chunk: Buffer) => chunks.push(chunk));
  req.on("end", () => {
    const payload = JSON.parse(Buffer.concat(chunks).toString("utf8"));
    for (const message of payload.messages) {
      captured.push({ content: message.content, role: message.role });
    }
    res.writeHead(200, { "content-type": "text/event-stream" });
    // ... real SSE chunks + [DONE]
  });
});
await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));

const model = { /* ...,  baseUrl: `http://127.0.0.1:${port}/v1` */ };
const assistant = {
  role: "assistant",
  content: [
    { type: "text", text: "Let me check the file." },
    { type: "text", text: "The file contains X." },
  ],
  // ...
};

await streamOpenAICompletions(model, context, { apiKey: "proof-key" }).result();

const replayed = captured.find((m) => m.role === "assistant");
const pass = replayed.content.includes("file.\nThe file");
process.exit(pass ? 0 : 1);

Regression test

Added packages/ai/src/openai-completions-messages.test.ts, which drives convertMessages with a two-text-block assistant turn and asserts the replayed content. It is load-bearing — against the unfixed source it reproduces the exact corruption rather than an incidental error:

FAIL packages/ai/src/openai-completions-messages.test.ts
  > convertMessages assistant text replay > keeps separate assistant text blocks apart
- Expected
+ Received
- Let me check the file.
- The file contains X.
+ Let me check the file.The file contains X.

Test Files  1 failed (1)
     Tests  1 failed (1)

The whole packages/ai suite is green, including the compatibility suite that pins Chat Completions payload shapes across provider variants:

node scripts/run-vitest.mjs run packages/ai/src/

Test Files  51 passed (51)
     Tests  906 passed (906)

oxfmt --check is clean on both touched files. The branch is rebased on current main.

AI-assisted: written and verified with an AI coding agent; the runtime proof above, the failing-test-first measurement, and the regression run were reviewed by me.

@Yigtwxx
Yigtwxx force-pushed the fix/completions-text-block-separator branch 3 times, most recently from 53dc9a7 to f1bff00 Compare July 29, 2026 12:43
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 29, 2026
@clawsweeper

clawsweeper Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 1, 2026, 3:57 AM ET / 07:57 UTC.

ClawSweeper review

What this changes

The PR preserves boundaries between multiple assistant text blocks during OpenAI-compatible Chat Completions transcript replay by joining them with a newline and testing the resulting request content.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

This PR is still necessary: current main concatenates separate assistant text blocks with no separator before sending an OpenAI Chat Completions replay. The branch changes that one conversion boundary to a newline, aligns it with the strict string-content compatibility helper, adds a focused regression test, and has no supported correctness or security finding.

Priority: P2
Reviewed head: 725ed96ebe5a8beda5b03b37ece3c93f9ec222a8

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, source-proven repair with strong direct runtime evidence and no actionable correctness finding.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body provides an after-fix loopback HTTP/SSE capture of the real Chat Completions serializer receiving newline-separated assistant content, supplemented by focused regression coverage and successful supplied exact-head checks.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body provides an after-fix loopback HTTP/SSE capture of the real Chat Completions serializer receiving newline-separated assistant content, supplemented by focused regression coverage and successful supplied exact-head checks.
Evidence reviewed 7 items Current-main defect: Current main filters assistant text content blocks and joins their sanitized text with an empty string, so two non-empty blocks replay without a boundary.
Sibling serialization contract: The strict OpenAI-compatible string-content helper flattens an array of text parts with a newline, providing a direct same-protocol precedent for the proposed delimiter.
Production entry point: The OpenAI Chat Completions provider calls this converter while building the streaming request parameters, making the changed value part of the provider request transcript.
Findings None None.
Security None None.

How this fits together

The AI package converts stored conversation messages into OpenAI Chat Completions request messages. That serialized transcript becomes the prior context sent to OpenAI-compatible providers, so a flattened assistant turn affects later model responses.

flowchart LR
  A[Conversation history] --> B[Assistant content blocks]
  B --> C[Chat Completions converter]
  C --> D[Serialized request transcript]
  D --> E[OpenAI-compatible provider]
  E --> F[Next model response]
Loading

Before merge

  • Resolve merge risk (P1) - The supplied GitHub state marks the cleanly mergeable head as behind current main. Refresh the GitHub merge result and required checks against the exact merge head if main advances again before landing.
  • Complete next step (P2) - No repair lane is needed because this PR already contains the narrow source fix and regression coverage; the remaining action is normal maintainer landing after confirming the current merge result.
Agent review details

Security

None.

PR surface

Source +2, Tests +52. Total +54 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 3 1 +2
Tests 1 52 0 +52
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 55 1 +54

Review metrics

None.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused newline repair and its regression test once the current GitHub merge result remains clean; retain the existing special handling for thinking content and tool calls.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the focused newline repair and its regression test once the current GitHub merge result remains clean; retain the existing special handling for thinking content and tool calls.

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

Yes—current main directly joins two non-empty assistant text blocks with "" at packages/ai/src/openai-completions-messages.ts:144; the PR also supplies a loopback request-capture proof for that exact conversion path.

Is this the best way to solve the issue?

Yes—the newline is the narrowest repair because it changes only string flattening for adjacent assistant text blocks and matches packages/ai/src/transports/openai-completions-string-content.ts:21 without altering thinking, tool-call, or content-array behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8317e23c1c2b.

Labels

Label justifications:

  • P2: The defect can corrupt model-visible replay text for OpenAI-compatible conversations, but the repair and blast radius are confined to one transcript conversion path.
  • 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 (terminal): The PR body provides an after-fix loopback HTTP/SSE capture of the real Chat Completions serializer receiving newline-separated assistant content, supplemented by focused regression coverage and successful supplied exact-head checks.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides an after-fix loopback HTTP/SSE capture of the real Chat Completions serializer receiving newline-separated assistant content, supplemented by focused regression coverage and successful supplied exact-head checks.

Evidence

What I checked:

Likely related people:

  • steipete: Blame attributes both the empty-string assistant replay line and the sibling newline flattener to the same current-main refactor commit by Peter Steinberger. (role: current converter and compatibility-helper history owner; confidence: high; commits: 89394978e0a6; files: packages/ai/src/openai-completions-messages.ts, packages/ai/src/transports/openai-completions-string-content.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Before landing, refresh the GitHub merge result if current main advances so required checks remain tied to the exact merge head.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (7 earlier review cycles)
  • reviewed 2026-07-29T12:59:43.188Z sha f1bff00 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-29T13:47:08.575Z sha f1bff00 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-29T15:49:04.756Z sha b97f49b :: needs maintainer review before merge. :: none
  • reviewed 2026-07-29T16:51:53.357Z sha 81bc4c5 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T23:48:30.915Z sha 81bc4c5 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T00:08:19.566Z sha 81bc4c5 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T07:38:40.472Z sha 725ed96 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 29, 2026
@Yigtwxx
Yigtwxx force-pushed the fix/completions-text-block-separator branch from f1bff00 to b97f49b Compare July 29, 2026 15:44
Chat Completions replay flattened every assistant text block with an empty
separator, so two distinct blocks came back as one word-joined sentence. The
same message shape survives distinctly on the Anthropic, Responses and Mistral
lanes, and the string-content flattener for strict OpenAI-compatible servers
already joins with a newline.

Two blocks arise routinely: streaming opens a new text block after a tool call,
and cross-model replay converts a thinking block into a text block.
@Yigtwxx
Yigtwxx force-pushed the fix/completions-text-block-separator branch from b97f49b to 81bc4c5 Compare July 29, 2026 16:44
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 1, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Aug 1, 2026
@Yigtwxx

Yigtwxx commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed onto current main (7aacd1314d7), which clears the P1 "branch is behind" item from the last review. Merged rather than rebased, per this repo's convention: no conflicts, and neither file this PR touches has been modified on main since the branch's base, so the diff itself is unchanged.

Exact-head CI on 725ed96ebe5: 82 passed / 0 failed / 0 pending (25 skipped). The two cancelled entries are superseded runs rather than failures — both were re-dispatched on the same head seconds later and completed green:

check started 07:39:17 started 07:39:26–33
Real behavior proof cancelled success
auto-response cancelled success

Locally on the merged tree, the changed module's full test surface passes: openai-completions-messages.test.ts plus openai-completions-structured-content-parity.test.ts, the only other suite in the repo that consumes it — 55 passed (55).

@Yigtwxx

Yigtwxx commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

The review above was taken while the exact-head checks were still running — they have since completed green (82 passed / 0 failed / 0 pending, detail in the comment above), so the "checks still in progress" half of the remaining item is resolved on this head.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@steipete

steipete commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Land-ready maintainer verification for the unchanged original contributor head 725ed96ebe5a8beda5b03b37ece3c93f9ec222a8:

  • Independently reproduced current main with the real installed OpenAI SDK v6.49.0, an actual localhost HTTP server, and real streamed SSE responses on both production entry points: streamOpenAICompletions and createOpenAICompletionsTransportStreamFn. Both currently send fused assistant transcript text: "Let me check the file.The file contains X." and "Before tool.After tool.".
  • Repeated the identical owner-path proof on this exact contributor head: 6/6 actual HTTP requests passed across the two transports. Adjacent text becomes "Let me check the file.\nThe file contains X."; text surrounding a real serialized tool call becomes "Before tool.\nAfter tool." while tool-call ID call_owner_42 remains intact; a single assistant block remains byte-identical on both paths. Both SDK clients consumed real SSE completion streams successfully.
  • Focused regression and sibling-path proof: node scripts/run-vitest.mjs packages/ai/src/openai-completions-messages.test.ts packages/ai/src/providers/openai-completions-structured-content-parity.test.ts55/55 tests passing.
  • Personally inspected the actual installed dependency contract: openai/resources/chat/completions/completions.d.ts accepts assistant string content and tool calls; completions.js posts that exact request body to /chat/completions.
  • Fresh structured review: .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --engine codex --thinking xhigh — clean, no accepted/actionable findings.
  • ClawSweeper's optional rank-up move was applied: refreshed GitHub merge computation now reports MERGEABLE; an independent git merge-tree --write-tree origin/main 725ed96ebe5a8beda5b03b37ece3c93f9ec222a8 is clean, and neither touched file changed on main after the PR base.
  • Exact-head full hosted CI is green and fresh: https://github.com/openclaw/openclaw/actions/runs/30689974207

Best fix is the existing shared conversion owner: its newline matches flattenCompletionMessagesToStringContent, repairs both production transports at once, and adds no configuration, dependency, API, or provider-specific workaround. Original contributor commits/head are preserved; no rebase or fixup needed.

@steipete
steipete merged commit afe024e into openclaw:main Aug 1, 2026
108 of 110 checks passed
@steipete

steipete commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

vincentkoc added a commit to Alix-007/openclaw that referenced this pull request Aug 1, 2026
* origin/main:
  fix(test/ui): prevent shared history pollution (openclaw#117348)
  fix(openrouter): apply image request transport policy (openclaw#117336)
  perf(gateway): skip clean transcript workers (openclaw#117342)
  test(testing): assert relay smoke readiness
  fix(testing): activate relay live smoke transport
  feat(channels): record account lifecycle facts and retire inferred health vocabularies (openclaw#117300)
  fix(canvas): remove unsupported snapshot delay hint (openclaw#117326)
  refactor(agents): fold cache TTL pruning into prompt projection (openclaw#117313)
  fix(canvas): honor paired-node invocation deadlines (openclaw#117316)
  fix(system-agent): make exit-and-run-a-terminal guidance surface-correct (openclaw#114633)
  fix(ai): assistant text blocks are run together on replay (openclaw#115743)
  refactor(agents): canonicalize subagent execution state (openclaw#117267)
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Aug 2, 2026
…15743)

Chat Completions replay flattened every assistant text block with an empty
separator, so two distinct blocks came back as one word-joined sentence. The
same message shape survives distinctly on the Anthropic, Responses and Mistral
lanes, and the string-content flattener for strict OpenAI-compatible servers
already joins with a newline.

Two blocks arise routinely: streaming opens a new text block after a tool call,
and cross-model replay converts a thinking block into a text block.
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Aug 2, 2026
* origin/main:
  fix(test/ui): prevent shared history pollution (openclaw#117348)
  fix(openrouter): apply image request transport policy (openclaw#117336)
  perf(gateway): skip clean transcript workers (openclaw#117342)
  test(testing): assert relay smoke readiness
  fix(testing): activate relay live smoke transport
  feat(channels): record account lifecycle facts and retire inferred health vocabularies (openclaw#117300)
  fix(canvas): remove unsupported snapshot delay hint (openclaw#117326)
  refactor(agents): fold cache TTL pruning into prompt projection (openclaw#117313)
  fix(canvas): honor paired-node invocation deadlines (openclaw#117316)
  fix(system-agent): make exit-and-run-a-terminal guidance surface-correct (openclaw#114633)
  fix(ai): assistant text blocks are run together on replay (openclaw#115743)
  refactor(agents): canonicalize subagent execution state (openclaw#117267)
@Yigtwxx
Yigtwxx deleted the fix/completions-text-block-separator branch August 2, 2026 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S 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.

2 participants