Skip to content

Fix DeepSeek reasoning_content propagation - #15760

Closed
LMTRADES wants to merge 3 commits into
NousResearch:mainfrom
LMTRADES:fix-deepseek-reasoning-upstream
Closed

Fix DeepSeek reasoning_content propagation#15760
LMTRADES wants to merge 3 commits into
NousResearch:mainfrom
LMTRADES:fix-deepseek-reasoning-upstream

Conversation

@LMTRADES

Copy link
Copy Markdown

This PR fixes two issues with DeepSeek reasoning_content. 1. Extracts reasoning_content from model_extra for ZenMux responses. 2. Adds empty string fallback for DeepSeek to non-tool-call messages.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/deepseek DeepSeek API labels Apr 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Part of the reasoning_content propagation cluster. Related open items: #15748, #15749. See also closed #15250, #15353 for prior fixes in this area.

@LMTRADES

Copy link
Copy Markdown
Author

Hi @alt-glitch! Thanks for linking those.

I reviewed #15748 and #15749, and it looks like there might be a boolean logic bug in the #15749 proposed fix for the cross-provider reasoning leak.

In #15749, if source_msg.get("reasoning") is present (e.g., from MiniMax), has_reasoning evaluates to True. That makes not has_reasoning False, which skips the needs_empty_reasoning block and falls through to the promotion block. This means MiniMax's reasoning still gets promoted to reasoning_content and leaks to DeepSeek.

My PR (#15760) naturally avoids this by evaluating the DeepSeek requirements before the normalized_reasoning promotion block. It injects "" for DeepSeek (for ALL assistant messages, which is another requirement I found) and returns early. This properly stops any stale cross-provider reasoning from leaking into DeepSeek's reasoning_content, while ensuring DeepSeek's own native reasoning is still preserved (since it's caught by the very first explicit_reasoning check).

I've tested this end-to-end and it successfully resolves the 400s without leaking!

@LMTRADES

Copy link
Copy Markdown
Author

Whoops, self-correction! I just took a closer look at the code I pushed originally in this PR and realized it was actually suffering from the exact same ordering issue I just criticized #15749 for.

In my initial push, I had evaluated deepseek_requires_reasoning after the normalized_reasoning check, which meant cross-provider (e.g. MiniMax) reasoning would still have leaked to DeepSeek.

I just force-pushed a fix to this PR. Now the logic works as I described above: deepseek_requires_reasoning and kimi_requires_reasoning are evaluated before the normalized_reasoning check.

This completely strips MiniMax's reasoning and safely injects "" for DeepSeek (or preserves DeepSeek's native reasoning via the very first explicit_reasoning check).

I have confirmed this with test coverage locally. The logic here is now fully water-tight for cross-provider isolation! Thanks for linking the issues and sorry for the initial confusion!

@teknium1

Copy link
Copy Markdown
Contributor

Closing as redundant — the DeepSeek reasoning_content thinking-mode 400 and cross-provider leak chain of issues is now fully covered on main:

21 regression tests in tests/run_agent/test_deepseek_reasoning_content_echo.py + 2 new tests for the cross-provider scenario exercise every known path. Thanks for the submission — appreciate the digging on this area.

@teknium1 teknium1 closed this Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/deepseek DeepSeek API type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants