fix: preserve <think> blocks in content for vLLM/custom provider replay - #20607
fix: preserve <think> blocks in content for vLLM/custom provider replay#20607vominh1919 wants to merge 1 commit into
Conversation
Fixes NousResearch#20577 When replaying multi-turn history to a vLLM-served thinking model over provider: custom, prior assistant turns had their <think> blocks stripped from content and routed to a separate reasoning_content field. vLLM's OpenAI-compat chat endpoint does NOT consume reasoning_content on incoming messages — MiniMax's official documentation explicitly requires the prior <think> block to be re-embedded inline in content. Net effect: vLLM sees stripped content with no chain-of-thought from prior turns, causing up to ~40% regression on multi-step benchmarks. Fix: in _copy_reasoning_content_for_api(), when the provider is 'custom', reconstruct <think>...</think> blocks in the content field from the stored reasoning field instead of setting reasoning_content.
liuhao1024
left a comment
There was a problem hiding this comment.
Note: PR #20609 (by @Bartok9) addresses the same issue (#20577) with a more complete approach — it adds a embeds_reasoning_in_content config option so users can opt-in per custom provider entry, rather than blanket-applying think-block reconstruction to all provider: custom calls.
The unconditional approach in this PR risks reconstructing blocks for custom providers that already handle reasoning_content correctly (e.g., providers that forward it properly to the upstream API). The config-gated approach in #20609 is safer.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the replay gap. The current main request path still drops reasoning for custom providers: agent/agent_runtime_helpers.py:2788-2830 strips reasoning_content when the active provider does not require echo-back, while agent/conversation_loop.py:809-816 removes the internal reasoning field.
Problems
- The changed method has moved: current
run_agent.py:5537-5540is only a forwarder, with the implementation now atagent/agent_runtime_helpers.py:2763. - The unconditional
provider == "custom"condition is too broad for heterogeneous custom OpenAI-compatible endpoints. The existing review discussion and issue #20577 describe endpoint-specific behavior and an opt-in capability as the safer shape. - Current assistant construction writes captured reasoning to
reasoning_content(agent/chat_completion_helpers.py:1016-1060), but the proposed branch is after the explicit-reasoning_contentreturn path. A current-main salvage must cover that path as well. - No regression tests accompany the change.
Suggested changes
- Move the implementation into
agent/agent_runtime_helpers.py::copy_reasoning_content_for_apiand gate it per custom endpoint/capability. - Cover both stored
reasoning_contentandreasoning, remove the outgoing structured field on the inline path, and add request-assembly tests for opted-in and ordinary custom endpoints.
Automated hermes-sweeper review.
| # model's chain-of-thought is preserved across turns. See #20577. | ||
| _provider = (getattr(self, "provider", None) or "").strip().lower() | ||
| if _provider == "custom": | ||
| _content = api_msg.get("content") or "" |
There was a problem hiding this comment.
provider == "custom" covers every custom OpenAI-compatible endpoint, not only vLLM endpoints that require inline thinking. Please gate this on an explicit endpoint capability/configuration so custom endpoints that accept reasoning_content are not silently changed.
Problem
When replaying multi-turn history to a vLLM-served thinking model (e.g. MiniMax-M2.7, DeepSeek-R1, GLM-4.x) over
provider: custom, prior assistant turns have their<think>blocks stripped fromcontentand routed to a separatereasoning_contentfield.vLLM's OpenAI-compat chat endpoint does not consume
reasoning_contenton incoming messages — MiniMax's official documentation explicitly requires the prior<think>block to be re-embedded inline incontent:Net effect: vLLM sees stripped content with no chain-of-thought from prior turns. MiniMax's interleaved-thinking docs claim up to ~40% regression on multi-step benchmarks when prior
<think>is omitted.Fix
In
_copy_reasoning_content_for_api(), when the provider is"custom", reconstruct<think>...</think>blocks in thecontentfield from the storedreasoningfield instead of settingreasoning_content.This is consistent with how the code already reconstructs think blocks for trajectory storage (line ~3953:
content = f"<think>\n{msg['reasoning']}\n</think>\n").Before vs After
contenthas no<think>,reasoning_contentset (ignored by vLLM)contenthas<think>blocks reconstructed fromreasoningfieldreasoning_contentset (unchanged)reasoning_contentset (unchanged)reasoning_contentset (unchanged)reasoning_contentset (unchanged)Related
Fixes #20577
Companion PR: #20594 (reasoning_config forwarding to custom providers)