fix(deepseek): inject empty reasoning_content on replay for OpenRouter DeepSeek#15325
Closed
ukint-vs wants to merge 3 commits into
Closed
fix(deepseek): inject empty reasoning_content on replay for OpenRouter DeepSeek#15325ukint-vs wants to merge 3 commits into
ukint-vs wants to merge 3 commits into
Conversation
…r DeepSeek
DeepSeek v4/v4-flash/v4-pro in thinking mode rejects multi-turn conversations
when replayed assistant messages lack `reasoning_content`, failing with:
HTTP 400: The reasoning_content in the thinking mode must be passed back.
`_copy_reasoning_content_for_api` already injects an empty placeholder for
Kimi/Moonshot on tool-call turns. Extend the same fallback to DeepSeek models,
matching the direction of NousResearch#15228 but detecting by model-name substring so it
covers OpenRouter-routed DeepSeek (`deepseek/deepseek-v4-flash`) as well as
direct `api.deepseek.com` usage.
The `tool_calls` gate from the Kimi branch is dropped here on purpose — per
NousResearch#15228, DeepSeek requires `reasoning_content` on every assistant message in
history, not only tool-call turns. Real reasoning is still preserved by the
earlier `explicit_reasoning` / `normalized_reasoning` branches; the empty
string only fills in when no reasoning was captured upstream (compressed
summaries and replays of sessions persisted before this fix).
Why not patch `normalize_response` instead (c.f. NousResearch#14973): on current openai
SDK (2.30.0) + pydantic 2.x with `extra='allow'`, `getattr(msg, 'reasoning_content')`
already reaches `model_extra` via `__getattr__`, so the capture path works on
modern SDKs. The real leak is the round-trip — covered here.
Refs:
- Fixes symptom described in NousResearch#15213 for OpenRouter-routed DeepSeek
- Complements NousResearch#15228 (direct api.deepseek.com path)
- Supersedes NousResearch#14973 for modern SDK versions
Tests: tests/run_agent/test_deepseek_reasoning_content.py covers six cases —
enabled-injects, disabled-skips, non-deepseek-skips, preserves-real-reasoning,
reasoning_config-None-defaults-enabled, preserves-explicit-empty.
…path
`_handle_max_iterations` builds its own `api_messages` list when requesting
the "please summarize" final turn after max_iterations. It stripped internal
fields (`reasoning`, `finish_reason`, `_thinking_prefill`) but never called
`_copy_reasoning_content_for_api`. That call is the remap step: it copies
`source_msg["reasoning"]` (trajectory-only field) into `api_msg["reasoning_content"]`
(the field DeepSeek requires on every assistant message in thinking mode).
Without the call, DeepSeek 400s on the summary request, surfacing as:
WARNING root: Failed to get summary response: Error code: 400 ...
The `reasoning_content` in the thinking mode must be passed back to the API.
The main loop (~line 9395) and flush_memories path (~line 7628) already made
this call. `_handle_max_iterations` was the missing third site.
Refs:
- Same bug class as NousResearch#15213 flagged for auxiliary paths
- Extends the fix introduced in the prior commit on this branch
Review feedback (multi-specialist + adversarial):
1. **Tighten DeepSeek detection.** Broad `"deepseek" in model.lower()` match
wrongly fires on Bedrock (`deepseek.v3.2`), NVIDIA NIM
(`deepseek-ai/deepseek-v3.2`), HuggingFace, and third-party fine-tunes.
These providers have NOT been validated to accept the `reasoning_content`
field — injecting empty placeholders there risks regressions on paths
that were previously fine. Switch to provider+host detection mirroring
the Kimi branch shape: OpenRouter-routed `deepseek/*` models OR direct
`api.deepseek.com`. No behavior change for OpenRouter or direct users;
untested hosts are left alone.
2. **Harden reasoning_config predicate.** Adjacent code at line 7342 uses
`isinstance(self.reasoning_config, dict)` before `.get()`. The new branch
was missing that guard — would have AttributeErrored on non-dict shapes
(string, Mock). Invert to an explicit "is False" disable check and guard
the isinstance.
3. **Add trailing `return` after the DeepSeek injection.** Matches the
pattern every prior branch in this function uses. Prevents a future
branch inserted below from unconditionally firing after DeepSeek.
4. **Expand test coverage** from 6 → 12 cases:
- `test_deepseek_injects_on_tool_call_message` — the entire point of
dropping the tool_calls gate per NousResearch#15228, was untested.
- `test_deepseek_reasoning_config_without_enabled_key` — common shape
`{"effort": "high"}` with no `enabled` field.
- `test_deepseek_reasoning_config_non_dict_is_safe` — pins the
defensive isinstance guard.
- `test_direct_deepseek_api_injects` — pins `api.deepseek.com` path.
- `test_bedrock_deepseek_skips_injection` — regression: Bedrock
`deepseek.v3.2` must NOT get injection.
- `test_nvidia_nim_deepseek_skips_injection` — same for NIM.
Full suite: 994 passed, 0 failed.
Collaborator
6 tasks
Author
|
Closing — superseded by #15446, which unifies this fix with related DeepSeek V4 work (thinking-mode toggle, context windows, normalize_response empty-string preservation) into a single coherent PR. Same detection scope (OpenRouter |
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.
DeepSeek v4 in thinking mode 400s on multi-turn replay when any assistant message in history lacks
reasoning_content:Hits three paths:
_handle_max_iterationssummary turn, which builds its ownapi_messagesand was missing the_copy_reasoning_content_for_apicall.This PR covers OpenRouter-routed DeepSeek (
deepseek/*) — #15228 handles directapi.deepseek.com; they co-exist.Fix
run_agent.py::_copy_reasoning_content_for_api— add a DeepSeek branch that injectsreasoning_content=""as a placeholder. Detection mirrors the Kimi branch's shape: host isopenrouter.aiwith adeepseek/model slug, OR host isapi.deepseek.com. Deliberately scoped — a bare"deepseek" in modelsubstring would wrongly fire on Bedrock, NIM, and third-party hosts we haven't validated.run_agent.py::_handle_max_iterations— missing_copy_reasoning_content_for_apicall in the summary-turnapi_messagesbuilder.isinstance(dict)guard onreasoning_configmirrors line 7342.Tests
New file
tests/run_agent/test_deepseek_reasoning_content.py, 12 cases:reasoning_configvariants (None, missingenabled, non-dict)tool_callsturn (key divergence from Kimi branch — fix: support DeepSeek v4-flash/v4-pro thinking mode toggle and fix reasoning_content 400 error #15228)api.deepseek.compathdeepseek.v3.2+ NIMdeepseek-ai/deepseek-v3.2must NOT be injectedFull suite: 994 passed.
Refs
extra='allow'already exposesmodel_extravia__getattr__)Tested on
macOS 15 · Python 3.11.15 · openai 2.30.0 · pydantic 2.12.5 · live OpenRouter DeepSeek V4 Flash traffic, no 400s after patch.
Three commits on the branch for reviewability — squash at merge is fine.