fix(transport): emit deepseek-v4 thinking.type and reasoning_effort on non-OpenRouter routes - #16614
Conversation
There was a problem hiding this comment.
Pull request overview
Extends the agent → transport plumbing so DeepSeek V4 “thinking mode” parameters (reasoning_effort and extra_body.thinking.type) are emitted based on the model name (e.g., deepseek-v4-*), enabling non-OpenRouter routes (direct DeepSeek API, OpenAI-compatible relays) to receive the correct request shape.
Changes:
- Update
run_agent.pyreasoning-extra-body gating to treatdeepseek-v4*as reasoning-capable. - Update
ChatCompletionsTransport.build_kwargs()to emit DeepSeek V4’s nativereasoning_effort+extra_body.thinkingand to avoid the genericextra_body["reasoning"]path for V4. - Add transport-level unit tests covering DeepSeek V4 request shaping and guarding legacy
deepseek-chatbehavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/agent/transports/test_chat_completions.py | Adds DeepSeek V4-focused unit tests for reasoning_effort and extra_body.thinking behavior. |
| run_agent.py | Adjusts _supports_reasoning_extra_body() to return true for deepseek-v4* models. |
| agent/transports/chat_completions.py | Adds DeepSeek V4 request-shaping branch and excludes V4 from generic extra_body.reasoning. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # DeepSeek V4 series: supports reasoning_effort + thinking via any route | ||
| if self.model and self.model.lower().startswith("deepseek-v4"): | ||
| return True | ||
|
|
There was a problem hiding this comment.
_supports_reasoning_extra_body() is also used in _handle_max_iterations() to decide whether to send extra_body["reasoning"] (OpenRouter-style). Returning True for deepseek-v4* will make iteration-limit summary requests include extra_body.reasoning, which conflicts with the DeepSeek V4 contract (should use top-level reasoning_effort + extra_body.thinking). Suggest keeping this method scoped to the generic extra_body.reasoning format and instead handle DeepSeek V4 summary kwargs via the chat_completions transport (or add an explicit DeepSeek V4 branch in the summary path).
| @@ -188,6 +188,7 @@ def build_kwargs( | |||
| anthropic_max_out = params.get("anthropic_max_output") | |||
| is_nvidia_nim = params.get("is_nvidia_nim", False) | |||
| is_kimi = params.get("is_kimi", False) | |||
| is_deepseek_v4 = params.get("model_lower", "").startswith("deepseek-v4") | |||
There was a problem hiding this comment.
DeepSeek V4 detection uses params.get("model_lower", ""), which means the DeepSeek V4 branch silently won’t run if a caller relies on the existing fallback model_lower = params.get("model_lower", (model or "").lower()) (i.e., doesn’t pass model_lower). Use the already-computed local model_lower (or (model or "").lower()) to keep detection consistent across call sites.
| is_deepseek_v4 = params.get("model_lower", "").startswith("deepseek-v4") | |
| is_deepseek_v4 = model_lower.startswith("deepseek-v4") |
| @@ -240,8 +256,19 @@ def build_kwargs( | |||
| "type": "enabled" if _kimi_thinking_enabled else "disabled", | |||
| } | |||
|
|
|||
| # DeepSeek V4: extra_body.thinking | |||
| if is_deepseek_v4: | |||
| _ds_thinking_off = bool( | |||
| reasoning_config | |||
| and isinstance(reasoning_config, dict) | |||
| and reasoning_config.get("enabled") is False | |||
| ) | |||
| extra_body["thinking"] = { | |||
| "type": "enabled" if not _ds_thinking_off else "disabled", | |||
| } | |||
There was a problem hiding this comment.
The DeepSeek V4 block repeats the same _ds_thinking_off computation in two places (reasoning_effort + extra_body.thinking). Consider computing it once and reusing it to reduce duplication and the chance of future drift between the two branches.
427d9c9 to
feeb834
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _ds_thinking_off = bool( | ||
| reasoning_config | ||
| and isinstance(reasoning_config, dict) | ||
| and reasoning_config.get("enabled") is False |
There was a problem hiding this comment.
_ds_thinking_off only checks reasoning_config['enabled'] is False, but other parts of the codebase pass reasoning_config={"effort": "none"} to disable thinking (e.g., batch_runner's --reasoning_disabled). For DeepSeek V4 this currently still sends reasoning_effort and sets extra_body.thinking.type to enabled. Consider treating effort == "none" (case/whitespace-normalized) as thinking disabled here as well, and omit reasoning_effort in that case.
| and reasoning_config.get("enabled") is False | |
| and ( | |
| reasoning_config.get("enabled") is False | |
| or (reasoning_config.get("effort") or "").strip().lower() == "none" | |
| ) |
| _ds_thinking_off = ( | ||
| self.reasoning_config is not None | ||
| and isinstance(self.reasoning_config, dict) | ||
| and self.reasoning_config.get("enabled") is False | ||
| ) | ||
| summary_extra_body["thinking"] = { | ||
| "type": "enabled" if not _ds_thinking_off else "disabled", | ||
| } |
There was a problem hiding this comment.
DeepSeek V4 summary requests treat thinking as disabled only when reasoning_config.enabled is False, but callers may also disable thinking via reasoning_config={"effort": "none"} (used elsewhere in the codebase). With the current check, V4 summary calls will still set extra_body.thinking.type to enabled even when the user requested no thinking. Update _ds_thinking_off to also consider effort == "none" as disabled (normalize case/whitespace).
| def test_deepseek_v4_reasoning_effort_omitted_when_thinking_disabled(self, transport): | ||
| kw = transport.build_kwargs( | ||
| model="deepseek-v4-pro", messages=[{"role": "user", "content": "Hi"}], | ||
| model_lower="deepseek-v4-pro", | ||
| reasoning_config={"enabled": False}, | ||
| max_tokens_param_fn=lambda n: {"max_tokens": n}, | ||
| ) | ||
| assert "reasoning_effort" not in kw | ||
|
|
||
| def test_deepseek_v4_thinking_enabled_extra_body(self, transport): | ||
| kw = transport.build_kwargs( | ||
| model="deepseek-v4-pro", messages=[{"role": "user", "content": "Hi"}], | ||
| model_lower="deepseek-v4-pro", | ||
| max_tokens_param_fn=lambda n: {"max_tokens": n}, | ||
| ) | ||
| assert kw["extra_body"]["thinking"] == {"type": "enabled"} | ||
|
|
||
| def test_deepseek_v4_thinking_disabled_extra_body(self, transport): | ||
| kw = transport.build_kwargs( | ||
| model="deepseek-v4-pro", messages=[{"role": "user", "content": "Hi"}], | ||
| model_lower="deepseek-v4-pro", | ||
| reasoning_config={"enabled": False}, | ||
| max_tokens_param_fn=lambda n: {"max_tokens": n}, | ||
| ) | ||
| assert kw["extra_body"]["thinking"] == {"type": "disabled"} |
There was a problem hiding this comment.
The new DeepSeek V4 tests cover reasoning_config={"enabled": False} but not the existing disable shape reasoning_config={"effort": "none"} (used by CLI/batch runner paths). Add a regression test asserting that effort=none results in extra_body.thinking.type == "disabled" and that reasoning_effort is omitted for V4.
- _supports_reasoning_extra_body(): detect deepseek-v4 models by name - build_kwargs(): emit reasoning_effort top-level (xhigh→max, others→high) and extra_body.thinking.type=enabled/disabled for DeepSeek V4 - Exclude DeepSeek V4 from generic extra_body.reasoning path - Add 7 transport-level tests covering enabled/disabled/xhigh/chat-clean
feeb834 to
7d329ae
Compare
What does this PR do?
Add DeepSeek V4 thinking mode support that works through any route (direct
api.deepseek.com, opencode-go, or other OpenAI-compatible relays), not just OpenRouter.Currently
_supports_reasoning_extra_body()gates on base_url containing"openrouter", which silently dropsreasoning_configfor direct DeepSeek connections and opencode-go. This PR makesreasoning_effortandthinking.typeactually reach the API.Related Issue
Fixes #15717
Related to #14958, #15251 — those PRs correct the detection gate but stop at
run_agent.py. This PR completes the pipeline end-to-end.Type of Change
Changes Made
run_agent.py—_supports_reasoning_extra_body()(1 line)deepseek-v4), so routes like opencode-go (opencode.ai/zen/go/v1) and any future relay can enable thinking.agent/transports/chat_completions.py—build_kwargs()(28 lines)is_deepseek_v4branch, modelled after the existingis_kimihandlingreasoning_effortwith effort mapping:xhigh→max, others →highextra_body["thinking"] = {"type": "enabled"/"disabled"}— DeepSeek-native formatextra_body["reasoning"]path to avoid format conflicttests/agent/transports/test_chat_completions.py— 7 new testsreasoning_efforttop-level (high, xhigh→max, disabled→omitted)extra_body.thinkingenabled/disabledextra_body.reasoningfor V4deepseek-chat(V2/V3) untouched — regression guardHow to Test
provider: opencode-go,base_url: https://opencode.ai/zen/go/v1,reasoning_effort: xhigh)reasoning_effort: "max"andextra_body: {"thinking": {"type": "enabled"}}Or run the transport-level unit tests:
python -m pytest tests/agent/transports/test_chat_completions.py -k "deepseek" -vChecklist
Code
pytest tests/agent/transports/test_chat_completions.py— 48/48 passing (41 existing + 7 new)Documentation & Housekeeping
cli-config.yaml.example— or N/AWhy this approach
Existing PRs (#14958, #15251) add
is_deepseekdetection inrun_agent.pybut don't touchbuild_kwargs()— so the transport still emits OpenRouter-styleextra_body["reasoning"]format. #15577 introduces a unifiedthinking_modeparameter framework.This PR takes the minimal path:
reasoning_configflowdeepseek-v4*prefix match;deepseek-chat/deepseek-reasoner(legacy) are untouchedis_kimihandling, keeping the transport predictable