Skip to content

fix(agent): re-pad reasoning_content on cross-provider fallback to require-side providers - #33784

Closed
bisko wants to merge 1 commit into
NousResearch:mainfrom
bisko:fix/reasoning-echo-on-cross-provider-fallback
Closed

fix(agent): re-pad reasoning_content on cross-provider fallback to require-side providers#33784
bisko wants to merge 1 commit into
NousResearch:mainfrom
bisko:fix/reasoning-echo-on-cross-provider-fallback

Conversation

@bisko

@bisko bisko commented May 28, 2026

Copy link
Copy Markdown
Contributor

Note: This PR was drafted by an AI assistant — Claude, Opus 4.7 — under human review, while fixing a real failure case on a personal Hermes deployment.

Closes #33783

Problem

When the primary is openai-codex / gpt-5.5 and a mid-conversation fallback switches to a require-side thinking provider (deepseek / deepseek-v4-pro, also Kimi / MiMo), the first request on the new provider 400s:

The `reasoning_content` in the thinking mode must be passed back to the API.

api_messages is built once, before the retry loop (agent/conversation_loop.py:919), while the primary (Codex) is active — so _needs_thinking_reasoning_pad() is False and bare assistant tool-call turns get no reasoning_content. _try_activate_fallback() (L1159/1395/1465) switches the provider but continues without rebuilding, and _build_api_kwargs(api_messages) (L1186) re-sends the stale, primary-shaped history to DeepSeek, which requires the field on every assistant turn.

The pad logic in _copy_reasoning_content_for_api is correct — it just never re-runs after the switch.

Fix

Add reapply_reasoning_echo_for_provider(agent, api_messages) and call it once, immediately before _build_api_kwargs in the retry loop. It re-applies the echo-back pad against the current provider. It is:

  • Idempotent — only pads assistant turns that still lack reasoning_content; a second call is a no-op.
  • A no-op unless needed — returns immediately unless _needs_thinking_reasoning_pad() is True for the active provider, so non-failover and reject-side paths are unaffected.
  • Fallback-path agnostic — placed at the single kwargs-build site, so it covers all three current fallback continues and any added later.

+43 lines across 3 files.

Validation

End-to-end against the live DeepSeek API, replaying a captured real failing history (33 messages, 14 assistant tool-call turns, 5 bare):

Build provider Bare turns DeepSeek result
codex (pre-fix failover) 5 HTTP 400 reasoning_content
codex → fix applied on switch 0 200finish=tool_calls, real continuation, full context preserved (42,182 prompt tokens, 42,112 cached)
  • No-op confirmed under the Codex primary (0 padded; bare turns stay bare).
  • Idempotency confirmed (second call pads 0).
  • Context integrity confirmed (message count, tool_calls, tool results unchanged).

Tests

Adds TestReapplyReasoningEchoForProviderSwitch to tests/run_agent/test_deepseek_reasoning_content_echo.py:

  • pads bare turns on switch to DeepSeek; preserves existing summaries verbatim
  • no-op under a non-require provider (Codex)
  • idempotent
  • leaves non-assistant messages untouched

All 40 tests in the file pass.

Scope

This fixes the require-side failover (DeepSeek / Kimi / MiMo). The reject-side failovers (Codex→Anthropic #29205, Codex→xAI #32617) need stripping/sanitizing instead and are out of scope for this PR.

…quire-side providers

api_messages is built once before the retry loop while the primary provider
is active. When a mid-conversation fallback switches to a require-side thinking
provider (DeepSeek/Kimi/MiMo), assistant turns built under a non-require primary
(e.g. Codex) go out without reasoning_content and the new provider rejects the
request with HTTP 400 ("reasoning_content must be passed back").

Re-apply the echo-back pad against the current provider immediately before
building the request kwargs. Idempotent and a no-op unless the active provider
enforces echo-back, so it covers all fallback paths without affecting normal or
reject-side operation.

Drafted by Claude (Opus 4.7) under human review while fixing a personal deployment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/deepseek DeepSeek API labels May 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related: #21033 takes the broader approach of extracting _build_api_messages() and calling it at all 6 fallback sites. This PR takes a narrower approach — idempotent re-pad at the single _build_api_kwargs call site. Both fix the same root cause (stale api_messages after provider switch).

@teknium1

Copy link
Copy Markdown
Contributor

Salvaged and merged via PR #33795. Your commit was cherry-picked verbatim onto current main — authorship preserved (Biser Perchinkov <biser@bisko.be>) in b5495db70117b45320066f4e2768e66382062fce. Added you to scripts/release.py AUTHOR_MAP so release-changelog @-mentions resolve correctly.

Thanks for the tight diagnosis, the live DeepSeek validation, and the upfront AI-assistance disclosure — everything we needed to verify and ship.

Scope as described in your PR: closes the require-side variant. The reject-side siblings (#29205 Codex→Anthropic, #32617 Codex→xAI) and the broader rebuild-on-fallback structural fix (#21033, #13235) stay open as separate work — they need live repros we don't have yet, plus a different shape (strip vs. pad).

#33795

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 P1 High — major feature broken, no workaround provider/deepseek DeepSeek API type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Codex→DeepSeek fallback sends assistant turns without reasoning_content → HTTP 400 (require-side cross-provider failover)

3 participants