fix(kosong): skip empty reasoning fields in OpenAI-compatible streams - #2227
fix(kosong): skip empty reasoning fields in OpenAI-compatible streams#2227Liewzheng wants to merge 3 commits into
Conversation
Some OpenAI-compatible gateways pad every content-phase stream chunk with an empty reasoning field (e.g. "reasoning_content":""). The provider yielded an empty think part for each of those chunks, interleaving them between text deltas; sequential part merging then could not merge the text, so the assembled message ended up with one text part per token and the TUI rendered a line break per token. Skip empty reasoning strings at both yield sites (stream and non-stream) so text deltas stay adjacent and merge into a single text part.
🦋 Changeset detectedLatest commit: af5fa73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d6c0a483e
ℹ️ 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".
| // parts between text deltas and defeat sequential merging downstream. | ||
| const reasoning = reasoningKeyDialect.observe(message); | ||
| if (reasoning !== undefined) { | ||
| if (reasoning !== undefined && reasoning.length > 0) { |
There was a problem hiding this comment.
When a non-stream response contains tool_calls together with reasoning_content: "", this condition now discards the empty ThinkPart even though the existing serialization contract explicitly preserves that marker for assistant tool-call messages (openai-legacy.test.ts's serializes an explicitly empty ThinkPart to reasoning_content). As a result, the assembled assistant history has no ThinkPart, so convertMessage omits reasoning_content from the subsequent tool-result request; OpenAI-compatible reasoning endpoints that require the reasoning field to be replayed can reject or mishandle the continuation. The padding workaround should be limited to content-phase stream chunks rather than dropping an empty reasoning field from tool-call responses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — addressed in ad702be. The empty-reasoning skip now applies only to content-only messages/chunks; tool-call responses (stream and non-stream) keep the empty think marker so the reasoning field is still replayed on continuation. Adjacent empty think parts merge into one, so at most a single marker survives. Added regression tests for both paths.
Addresses review feedback: the empty-reasoning skip now applies only to content-only messages/chunks. Tool-call responses keep the empty think marker so the reasoning field is replayed on continuation, matching the existing serialization contract for reasoning endpoints.
End-to-end regression coverage for the per-token-line-breaks symptom: after draining and merging a stream whose content chunks pad empty reasoning fields, the assembled message must contain one text part.
|
Hardening pass after the Codex review:
All 1338 kosong tests pass; |
|
Hi @RealKai42 — gentle ping for a review when you have a moment. This touches the empty-reasoning preservation contract from your #1676: the fix skips empty reasoning strings only on content-only messages/chunks, while tool-call turns (the case #1676 protects) keep the marker, with regression tests on both paths. Happy to adjust the approach if you'd rather handle the padding at a different layer. |
1 participant
Related Issue
Resolve #2226
Problem
See linked issue. In short: some OpenAI-compatible gateways pad every content-phase stream chunk with an empty reasoning field (
"reasoning_content":""). The provider yielded an emptythinkpart per chunk, which interleaved between text deltas and defeated sequential part merging — so streamed replies rendered one token per line in the TUI.What changed
Fix — In the OpenAI chat-completions provider (
openai-legacy), skip empty reasoning strings at both yield sites (stream and non-stream), except when the message/chunk carriestool_calls. Rationale for the exception: #1676 established that reasoning backends can reject continuations when the reasoning field is omitted from assistant tool-call messages, so the empty marker on tool-call turns is load-bearing and is preserved exactly. Adjacent empty think parts merge into one, so at most a single marker survives per turn. Content-only turns — the case that produced the per-token rendering bug — no longer emit empty think parts at all.Why at the provider layer — dropping the padding at the yield site protects every downstream consumer (message assembly, persisted history, transcript projection, TUI). Fixing it in
mergeInPlaceinstead would still persist semantically empty think parts into session history.Scope audit — the other kosong adapters were checked for the same hazard:
anthropicandopenai-responsesempty think parts carry protocol significance (thinking-block signatures, reasoning-item boundaries) and are not produced by per-chunk padding;kimi's first-party API does not pad empty reasoning fields;google-genaiyields think parts only from real thought text.agent-core-v2now builds its model wire layer on kosong, so this single fix covers both engines.Tests — updated the two tests that asserted the old empty-ThinkPart behavior, and added regression coverage for: skipping empty streaming reasoning fields, no interleaving between text deltas (GLM-style chunks), preserving the empty marker on streaming tool-call chunks, preserving it on non-stream tool-call responses, and an end-to-end
generate()assertion that the assembled message contains exactly one merged text part.Changeset —
@moonshot-ai/kimi-code, patch.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.