This repository was archived by the owner on Sep 23, 2026. It is now read-only.
Repository navigation
fix(kosong): preserve empty-string reasoning_content as ThinkPart - #2498
Merged
Merged
Conversation
Empty-string reasoning_content means "reasoned but empty", not "no reasoning" — but the truthy checks in the Kimi stream/non-stream converters dropped empty deltas, conflating the two. The stored assistant message then had no ThinkPart at all, so the next request omitted reasoning_content for that turn; preserved-thinking backends (thinking.keep=all, default-enforced on some models) reject such requests with 400 "assistant message is missing reasoning_content". Use 'is not None' in both converters so empty reasoning_content yields ThinkPart(think=""), which _convert_message round-trips as reasoning_content: "" via the existing has_reasoning path. Absent field still yields no ThinkPart — the distinction is preserved.
![devin-ai-integration[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Kimi provider message conversion in packages/kosong to correctly preserve the semantic difference between an absent reasoning_content field and a present-but-empty reasoning_content: "", which is required for “preserved thinking” backends that enforce thinking.keep=all and expect reasoning_content on every assistant message.
Changes:
- Update Kimi streaming and non-streaming response converters to treat
reasoning_content=""as meaningful (emitThinkPart(think="")) by switching from truthy checks tois not None. - Add regression tests covering: streaming empty delta, non-streaming empty field, and absent field (must not fabricate a
ThinkPart). - Document the fix in
packages/kosong/CHANGELOG.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/kosong/src/kosong/chat_provider/kimi.py | Preserves empty-string reasoning_content by emitting ThinkPart(think="") when the field is present (even if empty). |
| packages/kosong/tests/api_snapshot_tests/test_kimi.py | Adds tests ensuring empty vs absent reasoning_content is preserved for both streaming and non-streaming paths. |
| packages/kosong/CHANGELOG.md | Adds an Unreleased entry describing the preserved-thinking 400 fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Jul 15, 2026
NLPark-Cran
referenced
this pull request
in NLPark-Cran/cran-code
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
N/A — caught in a live session via request dump:
coding-model-okapi-0711-vibereturned 400preserved thinking (thinking.keep=all) requires reasoning_content on every assistant message, but assistant message at index 6 is missing reasoning_content.Description
Empty-string
reasoning_contentmeans "reasoned but empty", not "no reasoning". The truthy checks in the Kimi stream/non-stream converters dropped empty deltas, so a turn whose stream carriedreasoning_content: ""produced an assistant message with noThinkPartat all. On the next request_convert_messagethen omittedreasoning_contentfor that turn, and preserved-thinking backends (some always-thinking models enforcethinking.keep=allby default) reject the request with 400.Wire-log evidence from the incident session: the failing turn's stream contained only the ToolCall (output = 109 tokens ≈ the tool-call JSON), with no ContentPart at all — the model emitted empty reasoning for that turn.
Changes:
kimi.py: useis not Noneinstead of truthy checks in both_convert_stream_responseand_convert_non_stream_response, so empty-stringreasoning_contentyieldsThinkPart(think=""), which_convert_messageround-trips asreasoning_content: ""via the existinghas_reasoningpath.Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.