fix(qwen): support reasoning-model params (max_completion_tokens, omit temperature) (#128)
The qwen_chat backend (the generic OpenAI-compatible client) hardcoded max_tokens and always sent temperature, so reasoning models behind OpenAI-compatible gateways (GPT-5.x, Claude Opus 4.8 via Azure/LiteLLM) would 400. - Add opt-in QWEN_CHAT_USE_MAX_COMPLETION_TOKENS (+ role variants) that swaps the payload key max_tokens -> max_completion_tokens. - Treat an explicit empty / none / off temperature as "omit" instead of collapsing to the 0.7 default (via _resolve_temperature). - Thread both through configure_qwen_chat / _update_config. - Defaults unchanged; fully backward compatible. Adds 6 tests. Fixes #127 Co-authored-by: Chirag Singhal <chirag127@users.noreply.github.com>
This commit is contained in:
@@ -13,13 +13,13 @@ from skillopt.model.backend_config import ( # noqa: F401
|
||||
configure_codex_exec,
|
||||
get_claude_code_exec_config,
|
||||
get_codex_exec_config,
|
||||
get_target_backend,
|
||||
get_optimizer_backend,
|
||||
get_target_backend,
|
||||
is_optimizer_chat_backend,
|
||||
is_target_chat_backend,
|
||||
is_target_exec_backend,
|
||||
is_optimizer_chat_backend,
|
||||
set_target_backend,
|
||||
set_optimizer_backend,
|
||||
set_target_backend,
|
||||
)
|
||||
|
||||
|
||||
@@ -440,18 +440,21 @@ def configure_qwen_chat(
|
||||
timeout_seconds: float | str | None = None,
|
||||
max_tokens: int | str | None = None,
|
||||
enable_thinking: bool | str | None = None,
|
||||
use_max_completion_tokens: bool | str | None = None,
|
||||
optimizer_base_url: str | None = None,
|
||||
optimizer_api_key: str | None = None,
|
||||
optimizer_temperature: float | str | None = None,
|
||||
optimizer_timeout_seconds: float | str | None = None,
|
||||
optimizer_max_tokens: int | str | None = None,
|
||||
optimizer_enable_thinking: bool | str | None = None,
|
||||
optimizer_use_max_completion_tokens: bool | str | None = None,
|
||||
target_base_url: str | None = None,
|
||||
target_api_key: str | None = None,
|
||||
target_temperature: float | str | None = None,
|
||||
target_timeout_seconds: float | str | None = None,
|
||||
target_max_tokens: int | str | None = None,
|
||||
target_enable_thinking: bool | str | None = None,
|
||||
target_use_max_completion_tokens: bool | str | None = None,
|
||||
) -> None:
|
||||
_qwen.configure_qwen_chat(
|
||||
base_url=base_url,
|
||||
@@ -460,18 +463,21 @@ def configure_qwen_chat(
|
||||
timeout_seconds=timeout_seconds,
|
||||
max_tokens=max_tokens,
|
||||
enable_thinking=enable_thinking,
|
||||
use_max_completion_tokens=use_max_completion_tokens,
|
||||
optimizer_base_url=optimizer_base_url,
|
||||
optimizer_api_key=optimizer_api_key,
|
||||
optimizer_temperature=optimizer_temperature,
|
||||
optimizer_timeout_seconds=optimizer_timeout_seconds,
|
||||
optimizer_max_tokens=optimizer_max_tokens,
|
||||
optimizer_enable_thinking=optimizer_enable_thinking,
|
||||
optimizer_use_max_completion_tokens=optimizer_use_max_completion_tokens,
|
||||
target_base_url=target_base_url,
|
||||
target_api_key=target_api_key,
|
||||
target_temperature=target_temperature,
|
||||
target_timeout_seconds=target_timeout_seconds,
|
||||
target_max_tokens=target_max_tokens,
|
||||
target_enable_thinking=target_enable_thinking,
|
||||
target_use_max_completion_tokens=target_use_max_completion_tokens,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
"""OpenAI-compatible Qwen chat backend for optimizer and target paths."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
import json
|
||||
import os
|
||||
import threading
|
||||
import time
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
from dataclasses import dataclass
|
||||
from typing import Any
|
||||
|
||||
from skillopt.model.common import (
|
||||
@@ -28,6 +29,7 @@ class QwenChatConfig:
|
||||
temperature: float | None
|
||||
enable_thinking: bool
|
||||
deployment: str
|
||||
use_max_completion_tokens: bool = False
|
||||
|
||||
|
||||
def _parse_bool(value: Any, default: bool = False) -> bool:
|
||||
@@ -56,6 +58,28 @@ def _role_env(role: str, key: str, default: str) -> str:
|
||||
return os.environ.get(role_key) or os.environ.get(generic_key) or default
|
||||
|
||||
|
||||
# Sentinels that mean "omit this optional parameter from the request payload".
|
||||
# Reasoning models (e.g. GPT-5.x, Claude Opus 4.8) reject an explicit
|
||||
# `temperature`, so allow it to be turned off via an empty string / none / off.
|
||||
_OMIT_SENTINELS = {"", "none", "off", "null"}
|
||||
|
||||
|
||||
def _resolve_temperature(role: str) -> float | None:
|
||||
"""Return the temperature, or None to omit it entirely.
|
||||
|
||||
Unlike ``_role_env`` an *explicitly set* empty (or ``none``/``off``) value is
|
||||
honored as "omit" instead of collapsing to the default. Precedence:
|
||||
role-specific env -> generic env -> 0.7 default.
|
||||
"""
|
||||
for key in (f"{role.upper()}_QWEN_CHAT_TEMPERATURE", "QWEN_CHAT_TEMPERATURE"):
|
||||
if key in os.environ:
|
||||
raw = os.environ[key].strip()
|
||||
if raw.lower() in _OMIT_SENTINELS:
|
||||
return None
|
||||
return float(raw)
|
||||
return 0.7
|
||||
|
||||
|
||||
def _initial_config(role: str) -> QwenChatConfig:
|
||||
role_upper = role.upper()
|
||||
deployment_env = "OPTIMIZER_DEPLOYMENT" if role == "optimizer" else "TARGET_DEPLOYMENT"
|
||||
@@ -64,8 +88,9 @@ def _initial_config(role: str) -> QwenChatConfig:
|
||||
api_key=_role_env(role, "API_KEY", ""),
|
||||
timeout_seconds=float(_role_env(role, "TIMEOUT_SECONDS", "300") or 300),
|
||||
max_tokens=_parse_int(_role_env(role, "MAX_TOKENS", "8000"), 8000),
|
||||
temperature=_parse_optional_float(_role_env(role, "TEMPERATURE", "0.7")),
|
||||
temperature=_resolve_temperature(role),
|
||||
enable_thinking=_parse_bool(_role_env(role, "ENABLE_THINKING", "false")),
|
||||
use_max_completion_tokens=_parse_bool(_role_env(role, "USE_MAX_COMPLETION_TOKENS", "false")),
|
||||
deployment=(
|
||||
os.environ.get(f"{role_upper}_QWEN_CHAT_MODEL")
|
||||
or os.environ.get("QWEN_CHAT_MODEL")
|
||||
@@ -186,10 +211,14 @@ def _chat_messages_impl(
|
||||
timeout: float | None = None,
|
||||
) -> tuple[Any, dict[str, int]]:
|
||||
config = OPTIMIZER_CONFIG if role == "optimizer" else TARGET_CONFIG
|
||||
token_limit = min(max_completion_tokens, config.max_tokens)
|
||||
# Reasoning models on some gateways (GPT-5.x, o-series) require
|
||||
# `max_completion_tokens` and reject the legacy `max_tokens`.
|
||||
token_key = "max_completion_tokens" if config.use_max_completion_tokens else "max_tokens"
|
||||
payload: dict[str, Any] = {
|
||||
"model": deployment or config.deployment,
|
||||
"messages": _json_safe(messages),
|
||||
"max_tokens": min(max_completion_tokens, config.max_tokens),
|
||||
token_key: token_limit,
|
||||
}
|
||||
if config.enable_thinking:
|
||||
payload["chat_template_kwargs"] = {"enable_thinking": True}
|
||||
@@ -231,18 +260,21 @@ def configure_qwen_chat(
|
||||
timeout_seconds: float | str | None = None,
|
||||
max_tokens: int | str | None = None,
|
||||
enable_thinking: bool | str | None = None,
|
||||
use_max_completion_tokens: bool | str | None = None,
|
||||
optimizer_base_url: str | None = None,
|
||||
optimizer_api_key: str | None = None,
|
||||
optimizer_temperature: float | str | None = None,
|
||||
optimizer_timeout_seconds: float | str | None = None,
|
||||
optimizer_max_tokens: int | str | None = None,
|
||||
optimizer_enable_thinking: bool | str | None = None,
|
||||
optimizer_use_max_completion_tokens: bool | str | None = None,
|
||||
target_base_url: str | None = None,
|
||||
target_api_key: str | None = None,
|
||||
target_temperature: float | str | None = None,
|
||||
target_timeout_seconds: float | str | None = None,
|
||||
target_max_tokens: int | str | None = None,
|
||||
target_enable_thinking: bool | str | None = None,
|
||||
target_use_max_completion_tokens: bool | str | None = None,
|
||||
) -> None:
|
||||
with _config_lock:
|
||||
if base_url is not None:
|
||||
@@ -256,29 +288,24 @@ def configure_qwen_chat(
|
||||
if max_tokens is not None:
|
||||
os.environ["QWEN_CHAT_MAX_TOKENS"] = str(max_tokens)
|
||||
if enable_thinking is not None:
|
||||
os.environ["QWEN_CHAT_ENABLE_THINKING"] = (
|
||||
"true" if _parse_bool(enable_thinking) else "false"
|
||||
os.environ["QWEN_CHAT_ENABLE_THINKING"] = "true" if _parse_bool(enable_thinking) else "false"
|
||||
if use_max_completion_tokens is not None:
|
||||
os.environ["QWEN_CHAT_USE_MAX_COMPLETION_TOKENS"] = (
|
||||
"true" if _parse_bool(use_max_completion_tokens) else "false"
|
||||
)
|
||||
_update_config(
|
||||
OPTIMIZER_CONFIG,
|
||||
"optimizer",
|
||||
base_url=optimizer_base_url if optimizer_base_url is not None else base_url,
|
||||
api_key=optimizer_api_key if optimizer_api_key is not None else api_key,
|
||||
temperature=(
|
||||
optimizer_temperature
|
||||
if optimizer_temperature is not None
|
||||
else temperature
|
||||
),
|
||||
timeout_seconds=(
|
||||
optimizer_timeout_seconds
|
||||
if optimizer_timeout_seconds is not None
|
||||
else timeout_seconds
|
||||
),
|
||||
temperature=(optimizer_temperature if optimizer_temperature is not None else temperature),
|
||||
timeout_seconds=(optimizer_timeout_seconds if optimizer_timeout_seconds is not None else timeout_seconds),
|
||||
max_tokens=optimizer_max_tokens if optimizer_max_tokens is not None else max_tokens,
|
||||
enable_thinking=(
|
||||
optimizer_enable_thinking
|
||||
if optimizer_enable_thinking is not None
|
||||
else enable_thinking
|
||||
enable_thinking=(optimizer_enable_thinking if optimizer_enable_thinking is not None else enable_thinking),
|
||||
use_max_completion_tokens=(
|
||||
optimizer_use_max_completion_tokens
|
||||
if optimizer_use_max_completion_tokens is not None
|
||||
else use_max_completion_tokens
|
||||
),
|
||||
)
|
||||
_update_config(
|
||||
@@ -287,16 +314,13 @@ def configure_qwen_chat(
|
||||
base_url=target_base_url if target_base_url is not None else base_url,
|
||||
api_key=target_api_key if target_api_key is not None else api_key,
|
||||
temperature=target_temperature if target_temperature is not None else temperature,
|
||||
timeout_seconds=(
|
||||
target_timeout_seconds
|
||||
if target_timeout_seconds is not None
|
||||
else timeout_seconds
|
||||
),
|
||||
timeout_seconds=(target_timeout_seconds if target_timeout_seconds is not None else timeout_seconds),
|
||||
max_tokens=target_max_tokens if target_max_tokens is not None else max_tokens,
|
||||
enable_thinking=(
|
||||
target_enable_thinking
|
||||
if target_enable_thinking is not None
|
||||
else enable_thinking
|
||||
enable_thinking=(target_enable_thinking if target_enable_thinking is not None else enable_thinking),
|
||||
use_max_completion_tokens=(
|
||||
target_use_max_completion_tokens
|
||||
if target_use_max_completion_tokens is not None
|
||||
else use_max_completion_tokens
|
||||
),
|
||||
)
|
||||
|
||||
@@ -311,6 +335,7 @@ def _update_config(
|
||||
timeout_seconds: float | str | None = None,
|
||||
max_tokens: int | str | None = None,
|
||||
enable_thinking: bool | str | None = None,
|
||||
use_max_completion_tokens: bool | str | None = None,
|
||||
) -> None:
|
||||
env_prefix = role.upper()
|
||||
if base_url is not None:
|
||||
@@ -321,7 +346,7 @@ def _update_config(
|
||||
os.environ[f"{env_prefix}_QWEN_CHAT_API_KEY"] = config.api_key
|
||||
if temperature is not None:
|
||||
raw = str(temperature).strip()
|
||||
config.temperature = float(raw) if raw else None
|
||||
config.temperature = None if raw.lower() in _OMIT_SENTINELS else float(raw)
|
||||
os.environ[f"{env_prefix}_QWEN_CHAT_TEMPERATURE"] = raw
|
||||
if timeout_seconds is not None:
|
||||
config.timeout_seconds = float(timeout_seconds)
|
||||
@@ -331,8 +356,11 @@ def _update_config(
|
||||
os.environ[f"{env_prefix}_QWEN_CHAT_MAX_TOKENS"] = str(max_tokens)
|
||||
if enable_thinking is not None:
|
||||
config.enable_thinking = _parse_bool(enable_thinking)
|
||||
os.environ[f"{env_prefix}_QWEN_CHAT_ENABLE_THINKING"] = (
|
||||
"true" if config.enable_thinking else "false"
|
||||
os.environ[f"{env_prefix}_QWEN_CHAT_ENABLE_THINKING"] = "true" if config.enable_thinking else "false"
|
||||
if use_max_completion_tokens is not None:
|
||||
config.use_max_completion_tokens = _parse_bool(use_max_completion_tokens)
|
||||
os.environ[f"{env_prefix}_QWEN_CHAT_USE_MAX_COMPLETION_TOKENS"] = (
|
||||
"true" if config.use_max_completion_tokens else "false"
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
"""Tests for the OpenAI-compatible Qwen chat backend."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
@@ -14,7 +15,6 @@ import pytest
|
||||
|
||||
from skillopt.envs.searchqa.evaluator import extract_answer
|
||||
|
||||
|
||||
_QWEN_CONFIG_ENV_KEYS = (
|
||||
"BASE_URL",
|
||||
"API_KEY",
|
||||
@@ -22,11 +22,10 @@ _QWEN_CONFIG_ENV_KEYS = (
|
||||
"TIMEOUT_SECONDS",
|
||||
"MAX_TOKENS",
|
||||
"ENABLE_THINKING",
|
||||
"USE_MAX_COMPLETION_TOKENS",
|
||||
)
|
||||
_ENV_KEYS = ("OPTIMIZER_BACKEND", "TARGET_BACKEND") + tuple(
|
||||
f"{prefix}QWEN_CHAT_{key}"
|
||||
for prefix in ("", "OPTIMIZER_", "TARGET_")
|
||||
for key in _QWEN_CONFIG_ENV_KEYS
|
||||
f"{prefix}QWEN_CHAT_{key}" for prefix in ("", "OPTIMIZER_", "TARGET_") for key in _QWEN_CONFIG_ENV_KEYS
|
||||
)
|
||||
|
||||
|
||||
@@ -225,3 +224,75 @@ def test_configure_qwen_chat_runtime_toggle_controls_payload(
|
||||
|
||||
assert recorder.calls[0]["payload"]["chat_template_kwargs"] == {"enable_thinking": True}
|
||||
assert "chat_template_kwargs" not in recorder.calls[1]["payload"]
|
||||
|
||||
|
||||
def test_chat_target_uses_max_tokens_by_default(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
isolate_qwen_state: tuple[Any, Any],
|
||||
) -> None:
|
||||
model_module, qwen_backend = isolate_qwen_state
|
||||
_use_qwen_target(model_module, qwen_backend, enable_thinking=False)
|
||||
recorder = _record_urlopen(monkeypatch, qwen_backend)
|
||||
|
||||
model_module.chat_target("system", "user", max_completion_tokens=128, retries=1)
|
||||
|
||||
payload = recorder.calls[0]["payload"]
|
||||
assert payload["max_tokens"] == 128
|
||||
assert "max_completion_tokens" not in payload
|
||||
|
||||
|
||||
def test_chat_target_uses_max_completion_tokens_when_enabled(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
isolate_qwen_state: tuple[Any, Any],
|
||||
) -> None:
|
||||
model_module, qwen_backend = isolate_qwen_state
|
||||
_use_qwen_target(model_module, qwen_backend, enable_thinking=False)
|
||||
qwen_backend.TARGET_CONFIG.use_max_completion_tokens = True
|
||||
recorder = _record_urlopen(monkeypatch, qwen_backend)
|
||||
|
||||
model_module.chat_target("system", "user", max_completion_tokens=128, retries=1)
|
||||
|
||||
payload = recorder.calls[0]["payload"]
|
||||
assert payload["max_completion_tokens"] == 128
|
||||
assert "max_tokens" not in payload
|
||||
|
||||
|
||||
def test_configure_qwen_chat_toggles_max_completion_tokens(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
isolate_qwen_state: tuple[Any, Any],
|
||||
) -> None:
|
||||
model_module, qwen_backend = isolate_qwen_state
|
||||
_use_qwen_target(model_module, qwen_backend, enable_thinking=False)
|
||||
recorder = _record_urlopen(monkeypatch, qwen_backend)
|
||||
|
||||
model_module.configure_qwen_chat(target_use_max_completion_tokens=True)
|
||||
model_module.chat_target("system", "user", max_completion_tokens=128, retries=1)
|
||||
model_module.configure_qwen_chat(target_use_max_completion_tokens=False)
|
||||
model_module.chat_target("system", "user", max_completion_tokens=128, retries=1)
|
||||
|
||||
assert "max_completion_tokens" in recorder.calls[0]["payload"]
|
||||
assert "max_tokens" in recorder.calls[1]["payload"]
|
||||
|
||||
|
||||
def test_temperature_omitted_when_env_is_blank(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
isolate_qwen_state: tuple[Any, Any],
|
||||
) -> None:
|
||||
_model_module, qwen_backend = isolate_qwen_state
|
||||
# Explicit blank means "omit", not "fall back to 0.7".
|
||||
monkeypatch.setenv("TARGET_QWEN_CHAT_TEMPERATURE", "")
|
||||
assert qwen_backend._resolve_temperature("target") is None
|
||||
monkeypatch.setenv("TARGET_QWEN_CHAT_TEMPERATURE", "off")
|
||||
assert qwen_backend._resolve_temperature("target") is None
|
||||
|
||||
|
||||
def test_temperature_resolves_float_and_default(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
isolate_qwen_state: tuple[Any, Any],
|
||||
) -> None:
|
||||
_model_module, qwen_backend = isolate_qwen_state
|
||||
monkeypatch.delenv("QWEN_CHAT_TEMPERATURE", raising=False)
|
||||
monkeypatch.delenv("TARGET_QWEN_CHAT_TEMPERATURE", raising=False)
|
||||
assert qwen_backend._resolve_temperature("target") == 0.7
|
||||
monkeypatch.setenv("TARGET_QWEN_CHAT_TEMPERATURE", "0.2")
|
||||
assert qwen_backend._resolve_temperature("target") == 0.2
|
||||
|
||||
Reference in New Issue
Block a user