Repository navigation
fix(gateway): support thinking:disabled for DeepSeek and GLM - #3152
Merged
steebchen merged 2 commits intoJul 24, 2026
Merged
Conversation
vicovaro
marked this pull request as draft
July 20, 2026 16:32
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDeepSeek model mappings now allow ChangesDeepSeek reasoning support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/models/src/models/deepseek.ts (1)
267-280: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winAdd
"reasoning_effort"tosupportedParameters.Both the
deepseek-v4-proanddeepseek-v4-flashprovider mappings add"none"toreasoningEfforts, but they omit"reasoning_effort"from theirsupportedParametersarrays. Based on learnings, the gateway uses this array as the source of truth and maps any parameters not listed toundefinedbefore building the request. Without it, thereasoning_effortparameter will be silently stripped from incoming requests and the translation logic will never trigger.
packages/models/src/models/deepseek.ts#L267-L280: add"reasoning_effort"to thedeepseek-v4-proprovider's supported parameters array.packages/models/src/models/deepseek.ts#L382-L395: add"reasoning_effort"to thedeepseek-v4-flashprovider's supported parameters array.🐛 Proposed fix (apply to both sites)
supportedParameters: [ "temperature", "max_tokens", "top_p", "frequency_penalty", "presence_penalty", "stop", "stream", "response_format", "tools", + "reasoning_effort", ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/models/src/models/deepseek.ts` around lines 267 - 280, Add "reasoning_effort" to the supportedParameters arrays for both the deepseek-v4-pro mapping at packages/models/src/models/deepseek.ts:267-280 and the deepseek-v4-flash mapping at packages/models/src/models/deepseek.ts:382-395, preserving the existing parameter entries.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/actions/src/prepare-request-body.ts`:
- Around line 3597-3606: Add "deepseek" to the provider checks composing
handlesNoneNatively in the upstream reasoning-effort normalization logic. Keep
the existing providers and API-format fallback unchanged so reasoning_effort ===
"none" reaches the translation logic for DeepSeek.
---
Outside diff comments:
In `@packages/models/src/models/deepseek.ts`:
- Around line 267-280: Add "reasoning_effort" to the supportedParameters arrays
for both the deepseek-v4-pro mapping at
packages/models/src/models/deepseek.ts:267-280 and the deepseek-v4-flash mapping
at packages/models/src/models/deepseek.ts:382-395, preserving the existing
parameter entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6a7401a7-14e6-45f5-bc18-a1b9887cf470
📒 Files selected for processing (2)
packages/actions/src/prepare-request-body.tspackages/models/src/models/deepseek.ts
vicovaro
marked this pull request as ready for review
July 20, 2026 16:46
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/actions/src/prepare-request-body.spec.ts (1)
1290-1516: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply DRY principles to the test suite and eliminate
as any.The current test suite duplicates the lengthy
prepareRequestBodycall signature multiple times, repeatedly casting the result toany. As per coding guidelines, developers should "never useanyoras anyunless absolutely necessary" and must "apply DRY principles for code reuse".By moving the existing
preparehelper outside thedescribeblocks and extending it to accept overrides forprovider,externalId,messages, andsupportsReasoning, you can significantly reduce boilerplate and restrict the type assertions to a single, safe interface cast.♻️ Proposed refactor
-describe("prepareRequestBody - DeepSeek thinking", () => { - async function prepare(options: { - model: string; - reasoningEffort?: - | "none" - | "minimal" - | "low" - | "medium" - | "high" - | "xhigh" - | "max"; - }) { - return (await prepareRequestBody( - "deepseek", - options.model, - null, - options.model, - [{ role: "user", content: "Hello!" }], - false, // stream - undefined, // temperature - undefined, // max_tokens - undefined, // top_p - undefined, // frequency_penalty - undefined, // presence_penalty - undefined, // response_format - undefined, // tools - undefined, // tool_choice - options.reasoningEffort, // reasoning_effort - true, // supportsReasoning - )) as any; - } +const prepareDeepSeek = async (options: { + provider?: Parameters<typeof prepareRequestBody>[0]; + model: string; + externalId?: string; + messages?: Parameters<typeof prepareRequestBody>[4]; + reasoningEffort?: Parameters<typeof prepareRequestBody>[14]; + supportsReasoning?: boolean; +}) => { + return (await prepareRequestBody( + options.provider ?? "deepseek", + options.model, + null, + options.externalId ?? options.model, + options.messages ?? [{ role: "user", content: "Hello!" }], + false, // stream + undefined, // temperature + undefined, // max_tokens + undefined, // top_p + undefined, // frequency_penalty + undefined, // presence_penalty + undefined, // response_format + undefined, // tools + undefined, // tool_choice + options.reasoningEffort, + options.supportsReasoning ?? true, + )) as { + thinking?: { type: string }; + reasoning_effort?: string; + messages: { content: string }[]; + }; +}; + +describe("prepareRequestBody - DeepSeek thinking", () => { test("maps none to thinking disabled and never forwards reasoning_effort", async () => { - const requestBody = await prepare({ + const requestBody = await prepareDeepSeek({ model: "deepseek-v4-pro", reasoningEffort: "none", }); expect(requestBody.thinking).toEqual({ type: "disabled" }); expect(requestBody.reasoning_effort).toBeUndefined(); }); test("forwards native effort tiers verbatim without a thinking parameter", async () => { - const requestBody = await prepare({ + const requestBody = await prepareDeepSeek({ model: "deepseek-v4-pro", reasoningEffort: "high", }); expect(requestBody.reasoning_effort).toBe("high"); expect(requestBody.thinking).toBeUndefined(); }); test("forwards max natively", async () => { - const requestBody = await prepare({ + const requestBody = await prepareDeepSeek({ model: "deepseek-v4-pro", reasoningEffort: "max", }); expect(requestBody.reasoning_effort).toBe("max"); expect(requestBody.thinking).toBeUndefined(); }); test("keeps the provider default when no reasoning_effort is requested", async () => { - const requestBody = await prepare({ model: "deepseek-v4-pro" }); + const requestBody = await prepareDeepSeek({ model: "deepseek-v4-pro" }); expect(requestBody.thinking).toBeUndefined(); expect(requestBody.reasoning_effort).toBeUndefined(); }); test("forwards unsupported tiers verbatim so the provider rejects them", async () => { - const requestBody = await prepare({ + const requestBody = await prepareDeepSeek({ model: "deepseek-v4-pro", reasoningEffort: "xhigh", }); expect(requestBody.reasoning_effort).toBe("xhigh"); expect(requestBody.thinking).toBeUndefined(); }); test("drops none for models that do not declare it in reasoningEfforts", async () => { // deepseek-v3.2 on the deepseek provider has reasoning: false // and no reasoningEfforts, so supportsReasoning is false - const requestBody = (await prepareRequestBody( - "deepseek", - "deepseek-v3.2", - null, - "deepseek-chat", - [{ role: "user", content: "Hello!" }], - false, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - "none", - false, // supportsReasoning — model has reasoning: false - false, - )) as any; + const requestBody = await prepareDeepSeek({ + model: "deepseek-v3.2", + externalId: "deepseek-chat", + reasoningEffort: "none", + supportsReasoning: false, + }); expect(requestBody.thinking).toBeUndefined(); expect(requestBody.reasoning_effort).toBeUndefined(); }); test("flattens tool message array content to plain strings", async () => { - const requestBody = (await prepareRequestBody( - "deepseek", - "deepseek-v4-pro", - null, - "deepseek-v4-pro", - [ - { role: "user", content: "Run the analysis" }, - { - role: "tool", - tool_call_id: "call_1", - content: [ - { type: "text", text: "Part A. " }, - { type: "text", text: "Part B." }, - ], - }, - ], - false, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - true, // supportsReasoning - false, - )) as any; + const requestBody = await prepareDeepSeek({ + model: "deepseek-v4-pro", + messages: [ + { role: "user", content: "Run the analysis" }, + { + role: "tool", + tool_call_id: "call_1", + content: [ + { type: "text", text: "Part A. " }, + { type: "text", text: "Part B." }, + ], + }, + ], + }); expect(requestBody.messages[1].content).toBe("Part A. \nPart B."); }); test("drops image blocks from tool messages when flattening", async () => { - const requestBody = (await prepareRequestBody( - "deepseek", - "deepseek-v4-pro", - null, - "deepseek-v4-pro", - [ - { role: "user", content: "What is in this image?" }, - { - role: "tool", - tool_call_id: "call_1", - content: [ - { type: "text", text: "The image shows: " }, - { - type: "image_url", - image_url: { - url: "https://example.com/result.png", - }, - }, - ], - }, - ], - false, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - true, - false, - )) as any; + const requestBody = await prepareDeepSeek({ + model: "deepseek-v4-pro", + messages: [ + { role: "user", content: "What is in this image?" }, + { + role: "tool", + tool_call_id: "call_1", + content: [ + { type: "text", text: "The image shows: " }, + { + type: "image_url", + image_url: { + url: "https://example.com/result.png", + }, + }, + ], + }, + ], + }); expect(requestBody.messages[1].content).toBe("The image shows: "); }); }); describe("prepareRequestBody - default case thinking disabled", () => { test("maps none to thinking disabled for non-native providers that declare it", async () => { // deepinfra goes through the default case. Its deepseek-v4-pro // mapping declares reasoningEfforts: ["none", "low", "medium", "high", "xhigh"] - const requestBody = (await prepareRequestBody( - "deepinfra", - "deepseek-v4-pro", - null, - "deepseek-ai/DeepSeek-V4-Pro", - [{ role: "user", content: "Hello!" }], - false, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - "none", - true, // supportsReasoning - false, - )) as any; + const requestBody = await prepareDeepSeek({ + provider: "deepinfra", + model: "deepseek-v4-pro", + externalId: "deepseek-ai/DeepSeek-V4-Pro", + reasoningEffort: "none", + }); expect(requestBody.thinking).toEqual({ type: "disabled" }); expect(requestBody.reasoning_effort).toBeUndefined(); }); test("does not send thinking disabled for providers without none in reasoningEfforts", async () => { // together-ai deepseek-v4-pro has reasoningEfforts: ["high", "max"] // — no "none", so it should keep the provider default - const requestBody = (await prepareRequestBody( - "together-ai", - "deepseek-v4-pro", - null, - "deepseek-ai/DeepSeek-V4-Pro", - [{ role: "user", content: "Hello!" }], - false, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - "none", - true, // supportsReasoning - false, - )) as any; + const requestBody = await prepareDeepSeek({ + provider: "together-ai", + model: "deepseek-v4-pro", + externalId: "deepseek-ai/DeepSeek-V4-Pro", + reasoningEffort: "none", + }); expect(requestBody.thinking).toBeUndefined(); // Without "none" in reasoningEfforts, none is not forwarded either — // provider default is kept expect(requestBody.reasoning_effort).toBeUndefined(); }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/actions/src/prepare-request-body.spec.ts` around lines 1290 - 1516, Refactor the duplicated test setup around prepareRequestBody by moving the existing prepare helper outside both describe blocks and extending it with overrides for provider, externalId, messages, and supportsReasoning while preserving current defaults. Replace every repeated lengthy invocation with this helper, and eliminate all repeated as any casts by using one narrowly scoped, typed interface assertion inside the helper.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/actions/src/prepare-request-body.spec.ts`:
- Around line 1290-1516: Refactor the duplicated test setup around
prepareRequestBody by moving the existing prepare helper outside both describe
blocks and extending it with overrides for provider, externalId, messages, and
supportsReasoning while preserving current defaults. Replace every repeated
lengthy invocation with this helper, and eliminate all repeated as any casts by
using one narrowly scoped, typed interface assertion inside the helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c1160895-7e19-45d7-8139-6ff07fb93a11
📒 Files selected for processing (2)
packages/actions/src/prepare-request-body.spec.tspackages/actions/src/prepare-request-body.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/actions/src/prepare-request-body.ts
vicovaro
force-pushed
the
fix/deepseek-xiaomi-thinking-disabled
branch
7 times, most recently
from
July 20, 2026 17:26
b507448 to
a51c5fb
Compare
vicovaro
changed the title
vicovaro
force-pushed
the
fix/deepseek-xiaomi-thinking-disabled
branch
from
July 20, 2026 17:39
a51c5fb to
8945a72
Compare
Member
|
Verified this live against both provider APIs and end-to-end through a locally built gateway on this branch: DeepSeek (
Z.ai GLM (
Pushed a small follow-up (7f2caed): spec coverage for the guard-bypass cases ( |
vicovaro
force-pushed
the
fix/deepseek-xiaomi-thinking-disabled
branch
from
July 22, 2026 17:00
35679e5 to
7f2caed
Compare
vicovaro
changed the title
Contributor
Author
|
Did more cleanup of MR description and rebased the merge from main as part of the base, instead of a separate commit. Should be ready for review and merge. |
vicovaro
force-pushed
the
fix/deepseek-xiaomi-thinking-disabled
branch
from
July 22, 2026 17:50
e67ab6d to
628bf98
Compare
Translate reasoning_effort="none" to thinking: { type: "disabled" } for
providers that think by default and expose a binary thinking toggle:
- DeepSeek V4 (new case "deepseek"): "none" was stripped upstream, burning
reasoning tokens on every request. The new case translates "none" to
thinking: { type: "disabled" } and keeps the provider default for unset.
Native effort tiers (high, max) are forwarded verbatim via reasoning_effort.
- GLM / Z.ai (fix case "zai"): "none" was incorrectly mapping to
thinking: { type: "enabled" }. Now "none" explicitly disables thinking
regardless of tools or response_format (caller intent wins over the
structured-output safety guard).
MiMo / Xiaomi already handled "none" correctly and is unchanged.
Add "none" to reasoningEfforts for deepseek-v4-pro, deepseek-v4-flash,
and zai GLM models, add deepseek and zai to the upstream handlesNoneNatively
guard, and restore reasoning_effort to deepseek supportedParameters
(accidentally dropped from the native provider mappings).
Reference:
- https://api-docs.deepseek.com/guides/thinking_mode
- https://docs.z.ai/guides/capabilities/thinking-mode
- https://mimo.mi.com/docs/en-US/quick-start/usage-guide/text-generation/deep-thinking?target=thinking-disabled
Co-Authored-By: Claude <noreply@anthropic.com>
Cover the guard-bypass cases (none + tools / response_format) in the Z.ai spec, advertise the none effort on the zai glm-5.2 mapping so /v1/models reflects the new capability, and mention Z.ai in the handlesNoneNatively comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vicovaro
force-pushed
the
fix/deepseek-xiaomi-thinking-disabled
branch
from
July 22, 2026 17:57
628bf98 to
1dd87fc
Compare
Contributor
Author
|
I have tried to keep @steebchen commit alive but made some git mistakes, you can redo the commit so it doesn't have the |
steebchen
added this pull request to the merge queue
Jul 24, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 24, 2026
Contributor
Author
|
i confirm it works properly with |
This was referenced Aug 2, 2026
This was referenced Aug 10, 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
2 participants
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
case "deepseek"translatesreasoning_effort="none"→thinking: { type: "disabled" }, matching the DeepSeek API. Native effort tiers (high,max) are forwarded verbatim.case "zai"—"none"was incorrectly mapping tothinking: { type: "enabled" }; now correctly sets"disabled", matching the Z.ai docs. Caller-explicit"none"bypasses the structured-output safety guard.Why
DeepSeek V4 and GLM 4.5+ models think by default. Without an explicit disable, every request burns reasoning tokens — even when the caller passes
reasoning_effort="none". The gateway was either stripping the parameter upstream (DeepSeek) or mapping it to the wrong toggle value (GLM). These providers expose a binarythinkingtoggle rather than the OpenAI-style effort enum, so"none"needs a provider-specific translation.Per-provider
deepseeknoneignored (stripped upstream)thinking: { type: "disabled" }zainonemapped to"enabled"(bug)thinking: { type: "disabled" }MiMo (Xiaomi) already handled
none→thinking: { type: "disabled" }correctly in its existing switch case and is unchanged.Reference
thinking: { type: "disabled" }, do not sendreasoning_effortalongsidethinking: { type: "disabled" }, supported on GLM 4.5+🤖 Generated with Claude Code