fix(run_agent): gate reasoning_content re-feed to DeepSeek/Kimi only - #18446
Closed
leon7609 wants to merge 1 commit into
Closed
fix(run_agent): gate reasoning_content re-feed to DeepSeek/Kimi only#18446leon7609 wants to merge 1 commit into
leon7609 wants to merge 1 commit into
Conversation
`_copy_reasoning_content_for_api` step 3 promotes msg["reasoning"]
to api_msg["reasoning_content"] for any provider that has reasoning
text in the unified `reasoning` field.
For Qwen models served via vLLM and any other custom OpenAI-compatible
provider, this re-feed violates the multi-turn spec (Qwen explicitly
forbids re-feeding reasoning_content from prior turns) and produces:
- context balloon across multi-turn conversations
- thinking-mode death loops where the model keeps re-thinking
its own prior reasoning back at itself
Add `needs_thinking_pad and ` to step 3's condition so the promotion
runs only for DeepSeek/Kimi (which require it for tool-call replay,
guarded by `_needs_kimi_tool_reasoning` / `_needs_deepseek_tool_reasoning`).
OpenRouter, Anthropic, and OpenAI carry reasoning continuity via the
separate `reasoning_details` field (untouched by this helper), so
dropping the promotion for them is safe — they were not relying on
this code path.
For Qwen and other custom providers, the helper now falls through to
step 5 and pops reasoning_content from the API replay message.
This was referenced May 3, 2026
Contributor
Author
|
Closing as superseded: upstream implemented the equivalent gate natively in 2b3a4f0 ("fix(agent): strip stale reasoning_content when falling back to a strict provider", #50480), first shipped in v0.18.0 (v2026.7.1). Step 3 of Thanks! |
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.
What does this PR do?
Gates step 3 of
_copy_reasoning_content_for_apito providers that actually need thereasoning→reasoning_contentpromotion (DeepSeek / Kimi).Today step 3 promotes
msg["reasoning"]toapi_msg["reasoning_content"]for any assistant message that carries reasoning text in the unifiedreasoningfield — regardless of provider. This is harmful for at least two cases:reasoning_contentfrom prior turns. In practice this causes (a) context balloon across turns and (b) thinking-mode death loops where the model re-thinks its own prior reasoning back at itself. It's the dominant failure mode on long Qwen-on-vLLM sessions.reasoning_detailsfield (unaffected by this helper). They never relied on step 3's promotion path; dropping it for them is a no-op.The fix is a one-condition addition: gate step 3 on
needs_thinking_pad(already computed at line 8782 via_needs_thinking_reasoning_pad()). DeepSeek / Kimi keep their existing behaviour because they're true under that flag; everyone else falls through to step 5 and hasreasoning_contentpopped from the API replay message.Related Issue
No existing issue. Diagnosed from a Qwen3.6-27B-FP8 vLLM session that exhibited progressive context inflation and thinking loops; tracing showed the prior turn's
<think>text was being re-fed viareasoning_content.Type of Change
Changes Made
run_agent.py—_copy_reasoning_content_for_api, step 3:tests/run_agent/test_reasoning_content_provider_gate.py— 6 new unit tests:reasoningis not promoted.reasoningis still promoted.reasoning_content) is unaffected by the gate.The existing tests in
tests/run_agent/test_deepseek_reasoning_content_echo.pycontinue to pass — DeepSeek/Kimi paths are unchanged.How to Test
reasoning_contentre-appearing in the next request payload; on long sessions you'll see context inflate and the model re-reasoning over its own thoughts.reasoning_contentis only present on DeepSeek/Kimi turns; Qwen multi-turn behaviour matches the upstream Qwen spec.Or run the unit tests:
Checklist
Code
fix(run_agent): ...)pytest tests/ -qand the changes don't introduce new failures (vs. main baseline)Documentation & Housekeeping