Skip to content

fix(qwen): make thinking mode explicit - #74845

Open
jtstothard wants to merge 1 commit into
NousResearch:mainfrom
jtstothard:fix/qwen-explicit-thinking-contract
Open

fix(qwen): make thinking mode explicit#74845
jtstothard wants to merge 1 commit into
NousResearch:mainfrom
jtstothard:fix/qwen-explicit-thinking-contract

Conversation

@jtstothard

Copy link
Copy Markdown
Contributor

Summary

Qwen-compatible endpoints can default to server-side auto reasoning. For short utility or coding requests, hidden reasoning can consume the entire completion budget and leave empty visible output.

This PR makes Qwen thinking explicit at the provider boundary:

  • absent, malformed, false, or non-boolean reasoning config → enable_thinking: false
  • explicit {"enabled": true}enable_thinking: true plus a bounded thinking_budget: 32768
  • existing Qwen session metadata remains top-level metadata

Why this is provider-specific

Qwen uses enable_thinking in the request body. This is separate from the custom-provider / llama.cpp/vLLM work in #12427 and #64153, which uses chat_template_kwargs.enable_thinking for a different provider path. This PR does not attempt to redesign generic extra_body configuration or model-specific field mapping.

Reasoning contract

Hermes' reasoning parser produces {"enabled": true, "effort": ...} for explicit reasoning levels and {"enabled": false} for none. Only the genuine boolean enabled=True is authoritative here; effort is intentionally non-authoritative so explicit enablement is not accidentally suppressed by effort="none" or malformed effort values.

Unset reasoning is deliberately sent as enable_thinking=false rather than relying on a server-side auto default. This prevents hidden reasoning from silently consuming utility-request output budgets. The enabled 32K thinking budget is below the profile's 64K completion default, preserving visible-output headroom.

Validation

  • python3 -m pytest tests/providers/test_provider_profiles.py -q — 16 passed
  • python3 -m pytest tests/providers/test_profile_wiring.py tests/providers/test_transport_parity.py -q — 22 passed
  • Independent adversarial review — APPROVE
  • Review covered malformed/absent/false/true/effort edge cases, actual ChatCompletionsTransport forwarding, metadata preservation, diff scope, and secret safety

Validation exercised the Qwen-compatible llama.cpp/Lemonade transport path. No live Qwen Portal credential or production service is required or included.

Related upstream work

Scope

Only the Qwen provider profile and its focused tests are changed. No routing, Hindsight, LiteLLM, Lemonade, credentials, or deployment changes are included.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins provider/qwen Qwen / Alibaba Cloud (OAuth) P3 Low — cosmetic, nice to have labels Jul 30, 2026
@jtstothard
jtstothard marked this pull request as ready for review July 30, 2026 14:20
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused provider-boundary fix. Current main still has the relevant gap: QwenProfile.build_api_kwargs_extras() returns {} at plugins/model-providers/qwen-oauth/__init__.py:85-96, while the active transport forwards profile additions into extra_body at agent/transports/chat_completions.py:580-628.

Problems

  • The new tests call the profile hook directly. They do not verify the outgoing transport payload; the existing Qwen transport coverage only asserts image-resolution and metadata behavior in tests/providers/test_transport_parity.py:124-148.

Suggested changes

  • Add transport-level assertions for disabled and enabled Qwen reasoning. Verify enable_thinking, conditional thinking_budget, vl_high_resolution_images, and top-level metadata together in ChatCompletionsTransport.build_kwargs().

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have provider/qwen Qwen / Alibaba Cloud (OAuth) sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants