fix: reasoning_content Error (Related to Deepseek and Kimi Models) - #10341
fix: reasoning_content Error (Related to Deepseek and Kimi Models)#10341bytrangle wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50b27c0798
ℹ️ 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".
| if options.preserve_thinking_context | ||
| && reasoning_text.is_empty() | ||
| && converted.get("tool_calls").is_some() | ||
| { | ||
| converted["reasoning_content"] = json!(null); |
There was a problem hiding this comment.
Gate null reasoning_content to reasoning providers
This now adds reasoning_content: null to every assistant tool-call message whenever preserve_thinking_context is enabled, even when the upstream model never returned reasoning. Declarative OpenAI-compatible providers default that option on, so ordinary non-reasoning tool-call histories for strict OpenAI-compatible endpoints can start including a provider-specific null field that the endpoint did not emit and may reject; this should be limited to providers/models that require a placeholder, or only emitted when real reasoning was present and needs to be replayed.
Useful? React with 👍 / 👎.
|
Thanks for the fix- sorry this sat for a while. Closing this one because the underlying bug was fixed by #10366. I re-tested on current main against the live Moonshot API- tool-call round trips now carry reasoning_content and complete without the 400, so the null-injection here never triggers. Meanwhile preserves_thinking defaults to true for every OpenAI-engine provider, and some reject reasoning_content entirely (Cerebras, see 10774), so sending null everywhere risks breaking currently-working providers. If you can still reproduce the problem, please open a fresh issue |
Summary
I haven't been able to use any Deepseek model in Goose. API calls always fail no matter what the prompt is:
This happens because of how Deepseek API works in thinking mode. When the assistant thinks before answering, it sends back
reasoning_contentfield alongside the regular response. Goose doesn't includereasoning_contentfield if it is empty. But Deepseek's API requires this field to be present in every subsequent request.The fixes
reasoning_contentfield tonull. This signals "no reasoning" without triggering Deepseek's rejection.null reasoning_contentto all tool-call messages, themerge_split_tool_call_messagesfunction may try to merge unrelated tool-call messages because they both have the samenullvalue forreasoning_content. So I've added a guard to skip merging when reasoning_content is null.Testing
These changes have been tested with unit/integration tests.
Related Issues
solves #9632 and #10012