Skip to content

fix: forward Qwen target timeout and gate enable_thinking for vLLM targets - #40

Merged
Yifan Yang (Yif-Yang) merged 1 commit into
microsoft:mainfrom
mvanhorn:fix/28-qwen-chat-timeout-and-thinking-tag
Jun 7, 2026
Merged

Yifan Yang (Yif-Yang) merged 1 commit into
microsoft:mainfrom
mvanhorn:fix/28-qwen-chat-timeout-and-thinking-tag

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Summary

Fixes two interacting bugs that make SkillOpt report acc=0.000 across all rollouts when the target is a local OpenAI-compatible vLLM endpoint (the scenario in #28, which a collaborator diagnosed as an infra/format mismatch rather than a model capability gap).

Bugs

  1. Timeout not forwarded to the Qwen backend. chat_target() and chat_target_messages() in skillopt/model/__init__.py dispatched to the Qwen backend without passing timeout, so every Qwen rollout silently used the 300s module default regardless of the run's configured exec_timeout.
  2. enable_thinking injected unconditionally. qwen_backend.py always added chat_template_kwargs: {enable_thinking: <bool>} to the payload. Non-Qwen vLLM servers either reject the unknown field or emit <think>...</think> reasoning with no <answer>...</answer> tag, so the evaluator extracts a reasoning line instead of the answer and scores it wrong.

Fix

  • Forward timeout through both router functions to the Qwen backend.
  • Only set chat_template_kwargs.enable_thinking when config.enable_thinking is true, so the field is absent (and harmless to non-Qwen servers) in the default case.

Testing

  • pytest tests/test_qwen_backend.py — 4 passed
  • pytest (full suite) — 64 passed

Closes #28

AI was used for assistance.

Two bugs made local vLLM targets score acc=0.000: the router did not
forward 'timeout' to the Qwen backend (so runs used the 300s default),
and qwen_backend always injected chat_template_kwargs.enable_thinking,
which non-Qwen vLLM servers reject or answer with <think> output and no
<answer> tag. Forward timeout and only set the field when enabled.

Closes microsoft#28

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@Yif-Yang
Yifan Yang (Yif-Yang) merged commit b5328e8 into microsoft:main Jun 7, 2026
1 check passed
@mvanhorn

Copy link
Copy Markdown
Contributor Author

Appreciate the merge, Yifan Yang (@Yif-Yang). Forwarding the Qwen target timeout and gating enable_thinking keeps vLLM runs predictable.

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.

use local llm train ,acc is 0

2 participants