Skip to content

fix(agent): preserve Qwen3.6 / vLLM preserve_thinking history (#56004) - #112

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56019
Open

hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56019

Conversation

@hashbender

Copy link
Copy Markdown
Owner

What was fixed

Qwen3.6 (and similar self-hosted models served via vLLM with the preserve_thinking chat-template flag) loses its prior-turn chain-of-thought on every replay. The agent loop strips both reasoning_content (strict-provider gate in copy_reasoning_content_for_api) and reasoning (unconditional pop in conversation_loop.py), so vLLM ≥0.20 has nothing to render under the preserve_thinking template.

Root cause

Two client-side strip paths in agent/agent_runtime_helpers.py and agent/conversation_loop.py unconditionally remove the reasoning fields that vLLM/Qwen3.6 need to re-render prior thinking. vLLM ≥0.20 also drops reasoning_content on incoming assistant messages (vllm-project/vllm#38488), so the existing DeepSeek/Kimi/MiMo reasoning_content echo-back path is incompatible regardless.

Fix

Added a _preserves_thinking_history() predicate on AIAgent that detects Qwen3.6-style endpoints by model name (qwen3.6 / qwen-3.6, conservative word-boundary regex — does NOT match plain qwen3 per the model card). Result cached on the AIAgent instance keyed by (provider, model, base_url) — same invalidation contract as the adjacent _thinking_pad_cache, so all switch_model / _try_activate_fallback paths automatically invalidate it.

Three touch points now gated on the predicate:

  1. copy_reasoning_content_for_api — skip the strict-provider reasoning_content strip on preserves-thinking-history providers.
  2. conversation_loop.py — skip the unconditional reasoning pop (keep the field on the outgoing message for vLLM to render).
  3. reapply_reasoning_echo_for_provider — strip reasoning on the way out ONLY for strict providers (handles the strict-fallback case where Qwen3.6 → Mistral would 422 on the unknown field).

Verification

  • 11 new regression tests, all green on this commit, 9/11 RED on upstream/main with AttributeError: 'AIAgent' object has no attribute '_preserves_thinking_history' (real production-path coverage).
  • 5 detection tests (qwen3.6 positive, qwen-3.6 hyphen positive, qwen3 negative — no false positives on older Qwen, deepseek/mistral negative — distinct path through _needs_thinking_reasoning_pad).
  • 3 strip-path integration tests covering both copy_reasoning_content_for_api and reapply_reasoning_echo_for_provider in both directions (preserve-history provider AND strict-fallback).
  • 1 cache-invalidation test (keyed on model/provider/base_url).
  • 1 integration sanity test asserting the predicate is exposed on AIAgent.
  • No-regression sweep over existing reasoning/conversation/fallback tests (279 + 152 passing).
python3 -m pytest tests/run_agent/test_qwen_preserve_thinking_56004.py -v -o "addopts=" --tb=short
============================= 11 passed in 0.52s ==============================

Files changed (1 focused commit, 4 files, +289/-19)

  • agent/agent_runtime_helpers.py — gated reasoning_content strip + new reasoning strip branch in reapply_reasoning_echo_for_provider (handles strict-fallback).
  • agent/conversation_loop.py — gated the unconditional reasoning pop.
  • run_agent.py — new _preserves_thinking_history() predicate on AIAgent.
  • tests/run_agent/test_qwen_preserve_thinking_56004.py — 11 new regression tests.

Notes

  • Detection is intentionally model-name based per the issue author's "reasonable default if a config flag is undesirable" note. A future config-driven flag (custom_providers[*].preserve_thinking: true) is documented in the docstring as a more robust follow-up — today this covers the reported Qwen3.6 / vLLM case correctly.
  • extra_body: chat_template_kwargs: {preserve_thinking: true} already works server-side today; this PR completes the client-side loop.

Auto-published by Moonsong via Path B automated pipeline.


Mirror-of: NousResearch#56019
NousResearch#56019

@tenki-reviewer

tenki-reviewer Bot commented Jul 1, 2026 •

Copy link
Copy Markdown

Review Complete

Risk: 🟡 Medium (48/100) — 1 medium finding · 308 LOC across 4 files

Files Reviewed: 4
Findings: 1

By Severity:

  • 🟡 Medium: 1

PR adds preserve_thinking guards for Qwen3.6/vLLM reasoning content across three message-building paths, but misses the handle_max_iterations() path in chat_completion_helpers.py.

Files Reviewed (4 files)
agent/agent_runtime_helpers.py
agent/conversation_loop.py
run_agent.py
tests/run_agent/test_qwen_preserve_thinking_56004.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant