fix(qwen): always send explicit enable_thinking in chat_template_kwargs

[dylan (LESdylan)]

Problem

skillopt/model/qwen_backend.py only attached chat_template_kwargs when enable_thinking was true:

if config.enable_thinking:
    payload["chat_template_kwargs"] = {"enable_thinking": True}

With enable_thinking=false the key was omitted entirely, so whether Qwen thinking mode ran was decided by the vLLM server / chat-template default — which varies across deployments and makes results hard to reproduce. This is exactly point 3 of #90, where Zisu Huang (@Huangzisu) confirmed the reported results used client-side enable_thinking=false and that the code would be adjusted.

Change

Always send an explicit boolean:

payload["chat_template_kwargs"] = {"enable_thinking": bool(config.enable_thinking)}

This matches the MiniMax backend, which already sends the flag unconditionally (minimax_backend.py).

Tests

Updated the two tests that pinned the omit-when-disabled behavior to assert an explicit {"enable_thinking": False} instead. Full suite: 165 passed, 5 skipped.

Ref #90

[Yifan Yang (Yif-Yang)]

Thanks for this fix — the behavior is valuable and we would like to land it through your original PR so your contribution remains fully attributed.

Current main has since merged #128, so this branch now conflicts in qwen_backend.py. Could you please rebase onto the latest main and resolve it by preserving both behaviors?

• Keep fix(qwen): support reasoning-model params (max_completion_tokens, omit temperature) #128 handling for reasoning models: max_completion_tokens and omission of temperature where required.
• Always send chat_template_kwargs={"enable_thinking": bool(config.enable_thinking)}, including the explicit false case from this PR.
• Keep the existing optimizer/target role-specific configuration.

We tested that combined resolution locally: the Qwen tests and the full suite pass. Suggested verification:

pytest tests/test_qwen_backend.py -q
pytest tests/ -q

The CLA check is also still waiting for your response to the policy bot. Once rebased, CLA-complete, and green, this should be a good merge candidate. Please ping us after updating; we are happy to re-review promptly.

[Yifan Yang (Yif-Yang)]

Hi dylan (@LESdylan) — thanks again for this. Of the related open PRs, this is the closest merge candidate. The explicit false behavior is genuinely valuable, and we have already validated it locally in a combined resolution alongside #128.

Two things are still needed:

1. Please sign the CLA, which is still pending.
2. Please rebase onto the latest main to clear the conflicts, preserving the fix(qwen): support reasoning-model params (max_completion_tokens, omit temperature) #128 changes in the resolution.

Once those are in, please ping us and we will re-review promptly.

[Yifan Yang (Yif-Yang)]

Thanks dylan (@LESdylan) — you identified a real gap: after c31c50b the client had no supported way to send an explicit enable_thinking: false, only to omit the field.

We can't land the unconditional emit as written, though, because omitting the field is itself a fix: chat_template_kwargs is a vLLM/SGLang extension that OpenAI, Azure, and strict OpenAI-compatible gateways reject with HTTP 400, and non-Qwen vLLM models served with it can emit <think> output with no <answer> tag. That's #28, and always sending the field would reopen it.

The protocol really has three states, so #240 makes the setting three-state — server_default (omit, unchanged default), enabled, disabled — which fixes what you reported via disabled without regressing #28. It also warns once per role under server_default and records the resolved mode in each run's config.json, so the thinking policy is never silently server-dependent.

Closing in favor of #240, which credits you in the PR. This branch is also CONFLICTING against current main and the CLA is unsigned, so there's nothing lost by closing it here. Thanks again for flagging it.