fix(agent): ordering fix in _copy_reasoning_content_for_api — cross-provider reasoning isolation by Zjianru · Pull Request #15749 · NousResearch/hermes-agent

Zjianru · 2026-04-25T18:09:55Z

fix(agent): ordering fix in _copy_reasoning_content_for_api — cross-provider reasoning isolation

What this fixes

Fixes a logic-ordering bug in _copy_reasoning_content_for_api (run_agent.py) that allows reasoning content from a prior provider to be promoted to reasoning_content and sent to DeepSeek/Kimi thinking mode, causing HTTP 400.

The bug

The normalized_reasoning promotion path (reasoning → reasoning_content) returns before the DeepSeek/Kimi empty-string guard can run. For cross-provider histories, the reasoning field from MiniMax (or any other provider) gets forwarded to DeepSeek as reasoning_content, which DeepSeek rejects.

Current flow (buggy):

explicit_reasoning = source_msg.get("reasoning_content")  # None
→ first check returns False

normalized_reasoning = source_msg.get("reasoning")       # "MiniMax..."
→ promoted to reasoning_content
→ return   ← guard never reached

# unreachable:
if tool_calls and (kimi or deepseek):
    reasoning_content = ""

The fix

Reorder to 3-way branching:

# 1. reasoning_content already set — preserve verbatim
#    (covers DeepSeek's own "" placeholder + any valid content)
existing = source_msg.get("reasoning_content")
if isinstance(existing, str):
    api_msg["reasoning_content"] = existing
    return

# 2. Cross-provider poisoned history: neither reasoning_content nor reasoning.
#    Inject "" to satisfy DeepSeek/Kimi without forwarding stale content.
has_reasoning = isinstance(source_msg.get("reasoning"), str) and source_msg.get("reasoning")
needs_empty_reasoning = (
    not has_reasoning                              # ← key addition
    and source_msg.get("tool_calls")
    and (self._needs_kimi_tool_reasoning() or self._needs_deepseek_tool_reasoning())
)
if needs_empty_reasoning:
    api_msg["reasoning_content"] = ""
    return

# 3. Healthy same-provider path: promote reasoning to reasoning_content
normalized_reasoning = source_msg.get("reasoning")
if isinstance(normalized_reasoning, str) and normalized_reasoning:
    api_msg["reasoning_content"] = normalized_reasoning
    return

The not has_reasoning guard distinguishes "this provider's own reasoning to promote" from "a prior provider's reasoning that must not be forwarded".

Test results

tests/run_agent/test_deepseek_reasoning_content_echo.py
21 passed, 10 warnings

[Bug]: DeepSeek V4 Flash Discord session poisoned when tool-call assistant lacks reasoning_content #15250 (upstream issue for this bug)
[Bug]: HTTP 400 "reasoning_content must be passed back" with deepseek-v4-pro in cron/auxiliary path (thinking mode works in main loop, breaks elsewhere) #15213 (same root cause, auxiliary path variant)
PR fix: support DeepSeek v4-flash/v4-pro thinking mode toggle and fix reasoning_content 400 error #15228 (earlier fix that introduced this ordering issue)

…rovider reasoning isolation Fix logic-ordering bug where normalized_reasoning promotion returns before the DeepSeek/Kimi needs_empty_reasoning guard, causing cross-provider reasoning content (MiniMax → DeepSeek) to leak into reasoning_content and trigger HTTP 400. Changes: - Reorder branching: existing reasoning_content check first - Add 'not has_reasoning' guard so poisoned histories (no reasoning) still get '' injected for DeepSeek/Kimi - Healthy same-provider reasoning promotion path unchanged Refs: NousResearch#15250, NousResearch#15213

Zjianru · 2026-04-25T18:11:29Z

@teknium1 @OutThisLife @0xbyt4 @kshitijk4poor — this PR fixes a critical logic-ordering bug in _copy_reasoning_content_for_api that causes HTTP 400 errors on DeepSeek/Kimi when reasoning content from a prior provider (e.g. MiniMax) gets forwarded across provider switches. This is a high-impact production issue affecting any multi-provider agent workflow. All 21 regression tests pass. Please review when available. Related: #15250, #15213.

Copilot

Pull request overview

Fixes an ordering issue in run_agent.py::_copy_reasoning_content_for_api so DeepSeek/Kimi tool-call replay messages reliably include a provider-compatible reasoning_content placeholder (empty string) when the stored history lacks both reasoning_content and reasoning, avoiding DeepSeek thinking-mode 400s on replay.

Changes:

Reorders _copy_reasoning_content_for_api into explicit branches: preserve existing reasoning_content, inject "" for DeepSeek/Kimi poisoned tool-call history, otherwise promote internal reasoning to reasoning_content.
Ensures reasoning_content is removed from outgoing API messages when it would otherwise be non-string/null.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Zjianru · 2026-04-25T21:50:10Z

@OutThisLife Thanks for the approval! Both suggestions are addressed in the latest force-push: (1) now replaces the repeated call and whitespace-only strings are correctly treated as falsy via ; (2) comment wording updated from "rejects HTTP 400" to "returns HTTP 400". The diff is clean (+2 lines) — ready for merge whenever CI clears.

OutThisLife

The current ordering still does not fix the cross-provider reasoning leak described by the PR.

The new needs_empty_reasoning branch is gated by not has_reasoning, so if a prior provider left an internal reasoning string but no provider-native reasoning_content, DeepSeek/Kimi will still receive that foreign reasoning via the later promotion branch:

if needs_empty_reasoning:
    api_msg["reasoning_content"] = ""
    return

if isinstance(normalized_reasoning, str) and normalized_reasoning:
    api_msg["reasoning_content"] = normalized_reasoning
    return

That means the stated MiniMax -> DeepSeek leak still happens whenever source_msg["reasoning"] is non-empty. I reproduced the helper behavior locally with a DeepSeek agent and a source assistant message containing reasoning + tool_calls but no reasoning_content; the outgoing API message became {"reasoning_content": "minimax prior reasoning"}.

For this fix, provider-required placeholder handling should win when reasoning_content is absent:

if isinstance(existing, str):
    api_msg["reasoning_content"] = existing
    return

if source_msg.get("tool_calls") and (self._needs_kimi_tool_reasoning() or self._needs_deepseek_tool_reasoning()):
    api_msg["reasoning_content"] = ""
    return

if isinstance(normalized_reasoning, str) and normalized_reasoning:
    api_msg["reasoning_content"] = normalized_reasoning
    return

In short: trust explicit reasoning_content; otherwise inject the required placeholder for DeepSeek/Kimi tool-call replay; only promote generic reasoning for providers where that is safe.

…DeepSeek/Kimi tool_calls unconditionally

Zjianru · 2026-04-25T22:09:08Z

@OutThisLife You're right — I miscounted the promotion path as a desirable feature when it's actually the leak vector. The guard created a false positive: any non-empty reasoning (including foreign-provider content) would skip the empty-string injection and fall through to block 3.

The new logic is clean: DeepSeek/Kimi tool-call replay always gets the placeholder, block 3 promotion only fires for non-thinking-mode providers. Applied your suggestion verbatim (+6/-9 lines). Thanks for the precise reproduction — that made the root cause unmistakable.

… tool-call replay PR #15478 fixed missing reasoning_content for DeepSeek API but introduced a regression: tool-call messages with genuine 'reasoning' field were overwritten by empty-string fallback before promotion. Re-order _copy_reasoning_content_for_api steps: 1. Preserve explicit reasoning_content 2. Promote 'reasoning' field (MOVED UP) 3. DeepSeek/Kimi tool-call empty-string fallback (MOVED DOWN) 4. Non-thinking provider cleanup Fixes #15812, relates #15749, #15478.