Skip to content

fix(kosong): omit reasoning_effort instead of sending null when thinking is off#2476

Open
logicwu0 wants to merge 1 commit into
MoonshotAI:mainfrom
logicwu0:fix/2465-reasoning-effort-omit
Open

fix(kosong): omit reasoning_effort instead of sending null when thinking is off#2476
logicwu0 wants to merge 1 commit into
MoonshotAI:mainfrom
logicwu0:fix/2465-reasoning-effort-omit

Conversation

@logicwu0

@logicwu0 logicwu0 commented Jun 26, 2026

Copy link
Copy Markdown

Summary

OpenAILegacy.with_thinking("off") maps to a Python None (thinking_effort_to_reasoning_effort("off")), which was passed straight to client.chat.completions.create(reasoning_effort=...). The OpenAI SDK serializes an explicit None as "reasoning_effort": null — only the omit sentinel is dropped from the payload.

That null is wrong in two ways (per #2465):

  • It is not a valid value in the chat-completions schema — strict validators (e.g. Synthetic.new / TypeBox) reject it with HTTP 400, which drives a retry/rate-limit loop.
  • Lenient backends (e.g. Ollama Cloud) accept null but treat it as "reasoning on by default", so thinking is not actually disabled.

The existing auto-enable guard only handled Omit, so a None from with_thinking("off") flowed straight through.

Fix

In generate(), normalize a None reasoning_effort to the omit sentinel 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_effort asserting reasoning_effort is absent from the request body after with_thinking("off"). It fails before the fix (body contains reasoning_effort: null) and passes after. Full test_openai_legacy.py suite green; ruff clean.

Closes #2465


Open in Devin Review

…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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines +146 to +147
if reasoning_effort is None:
reasoning_effort = omit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 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 Noneomit 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kosong: OpenAILegacy emits reasoning_effort: null for thinking "off" — invalid for strict APIs and does not disable reasoning

1 participant