fix(kosong): omit reasoning_effort instead of sending null when thinking is off#2476
Open
logicwu0 wants to merge 1 commit into
Open
fix(kosong): omit reasoning_effort instead of sending null when thinking is off#2476logicwu0 wants to merge 1 commit into
logicwu0 wants to merge 1 commit into
Conversation
…ing is off
OpenAILegacy.with_thinking("off") maps to a Python None, which was passed straight
to client.chat.completions.create(). The OpenAI SDK serializes an explicit None as
"reasoning_effort": null, which is invalid in the chat-completions schema: strict
validators reject it with HTTP 400 (driving a retry/rate-limit loop) and lenient
backends treat null as "reasoning on by default", so thinking is not actually disabled.
Normalize a None reasoning_effort to the SDK omit sentinel in generate() so the field
is dropped from the payload. Adds a snapshot test asserting reasoning_effort is absent
from the request body when thinking is off.
Closes MoonshotAI#2465
Comment on lines
+146
to
+147
| if reasoning_effort is None: | ||
| reasoning_effort = omit |
Contributor
There was a problem hiding this comment.
🚩 Kimi provider may have the same null reasoning_effort serialization issue
The Kimi provider's with_thinking("off") also sets reasoning_effort=None (kimi.py:199), and this value is passed via **generation_kwargs to self.client.chat.completions.create() at kimi.py:170-177. Unlike OpenAILegacy, there is no None → omit guard, so "reasoning_effort": null will appear in the Kimi API request payload. This may be acceptable because the Kimi API endpoint handles it differently (and the extra_body.thinking.type: "disabled" already signals the intent), but it is an inconsistency with the fix applied here.
Was this helpful? React with 👍 or 👎 to provide feedback.
This was referenced Jun 27, 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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
OpenAILegacy.with_thinking("off")maps to a PythonNone(thinking_effort_to_reasoning_effort("off")), which was passed straight toclient.chat.completions.create(reasoning_effort=...). The OpenAI SDK serializes an explicitNoneas"reasoning_effort": null— only theomitsentinel is dropped from the payload.That
nullis wrong in two ways (per #2465):nullbut treat it as "reasoning on by default", so thinking is not actually disabled.The existing auto-enable guard only handled
Omit, so aNonefromwith_thinking("off")flowed straight through.Fix
In
generate(), normalize aNonereasoning_effortto theomitsentinel before the API call, so the field is dropped from the payload entirely when thinking is off.Test
Added
test_openai_legacy_with_thinking_off_omits_reasoning_effortassertingreasoning_effortis absent from the request body afterwith_thinking("off"). It fails before the fix (body containsreasoning_effort: null) and passes after. Fulltest_openai_legacy.pysuite green;ruffclean.Closes #2465