mirror of
https://github.com/usestrix/strix.git
synced 2026-10-04 02:33:47 +00:00
fix(inputs): send reasoning_effort=none explicitly on chat completions; hint at the Responses API when tools+effort are rejected
This commit is contained in:
parent
066bd60a03
commit
e1ec259ac2
5 changed files with 71 additions and 71 deletions
|
|
@ -697,27 +697,6 @@ def _catalog_supported_endpoints(model_name: str) -> list[str]:
|
|||
return [str(e) for e in endpoints] if isinstance(endpoints, list) else []
|
||||
|
||||
|
||||
def chat_completions_accept_reasoning_effort(model_name: str) -> bool:
|
||||
"""Whether OpenAI's chat completions take ``reasoning_effort`` for ``model_name``.
|
||||
|
||||
Some reasoning models accept the parameter on the Responses API only and
|
||||
reject a chat completion carrying it (with function tools). LiteLLM's
|
||||
parameter map records which; a model it does not know is given the benefit
|
||||
of the doubt.
|
||||
"""
|
||||
if not _catalog_entry(model_name):
|
||||
return True
|
||||
import litellm
|
||||
|
||||
try:
|
||||
params = litellm.get_supported_openai_params(
|
||||
_bare_openai_name(model_name), custom_llm_provider="openai"
|
||||
)
|
||||
except Exception: # noqa: BLE001 - an unmapped model keeps the parameter
|
||||
return True
|
||||
return "reasoning_effort" in (params or [])
|
||||
|
||||
|
||||
def _mirror_api_key_to_provider_env(model_name: str | None, api_key: str) -> None:
|
||||
if not model_name:
|
||||
return
|
||||
|
|
|
|||
|
|
@ -125,6 +125,20 @@ _TRANSIENT_MODEL_RETRY_BASE_DELAY_S = 2.0
|
|||
_TRANSIENT_MODEL_RETRY_MAX_DELAY_S = 90.0
|
||||
|
||||
|
||||
_TOOLS_WITH_REASONING_EFFORT = "Function tools with reasoning_effort are not supported"
|
||||
_TOOLS_WITH_REASONING_EFFORT_HINT = (
|
||||
"This model takes reasoning_effort together with tools on the Responses API only: "
|
||||
"set STRIX_API_TYPE=responses, or STRIX_REASONING_EFFORT=none to stay on chat completions."
|
||||
)
|
||||
|
||||
|
||||
def _failure_text(exc: BaseException) -> str:
|
||||
text = request_log.failure_text(exc)
|
||||
if _model_error_status_code(exc) == 400 and _TOOLS_WITH_REASONING_EFFORT in text:
|
||||
text = f"{text} {_TOOLS_WITH_REASONING_EFFORT_HINT}"
|
||||
return text
|
||||
|
||||
|
||||
def _model_error_status_code(exc: BaseException) -> int | None:
|
||||
code = getattr(exc, "status_code", None)
|
||||
return code if isinstance(code, int) else None
|
||||
|
|
@ -693,7 +707,7 @@ async def _run_cycle_parked(
|
|||
raise
|
||||
except Exception as exc:
|
||||
logger.exception("error escaped the run cycle for %s; parking as failed", agent_id)
|
||||
await coordinator.set_status(agent_id, "failed", error=request_log.failure_text(exc))
|
||||
await coordinator.set_status(agent_id, "failed", error=_failure_text(exc))
|
||||
await notify_parent_on_terminal(coordinator, agent_id, "failed")
|
||||
return None
|
||||
|
||||
|
|
@ -854,9 +868,7 @@ async def _run_cycle( # noqa: PLR0912, PLR0915
|
|||
await _salvage_stream_to_session(session, pre_run_items, stream, agent_id)
|
||||
if isinstance(exc, ProviderRefusalError):
|
||||
logger.warning("agent %s refused by the model provider: %s", agent_id, exc)
|
||||
await coordinator.set_status(
|
||||
agent_id, "failed", error=request_log.failure_text(exc)
|
||||
)
|
||||
await coordinator.set_status(agent_id, "failed", error=_failure_text(exc))
|
||||
await notify_parent_on_terminal(coordinator, agent_id, "failed")
|
||||
return None
|
||||
if isinstance(exc, MaxTurnsExceeded):
|
||||
|
|
@ -870,7 +882,7 @@ async def _run_cycle( # noqa: PLR0912, PLR0915
|
|||
# non-interactive agent's task: a child that dies still owes its parent a
|
||||
# report, and the parent would otherwise wait out its timeout on a message
|
||||
# the dead child can no longer send.
|
||||
await coordinator.set_status(agent_id, status, error=request_log.failure_text(exc))
|
||||
await coordinator.set_status(agent_id, status, error=_failure_text(exc))
|
||||
await notify_parent_on_terminal(coordinator, agent_id, status)
|
||||
if not interactive:
|
||||
raise
|
||||
|
|
|
|||
|
|
@ -13,7 +13,6 @@ from strix.config.models import (
|
|||
DEFAULT_MODEL_RETRY,
|
||||
OPENROUTER_ATTRIBUTION_HEADERS,
|
||||
bedrock_route_supports_prompt_caching,
|
||||
chat_completions_accept_reasoning_effort,
|
||||
is_bedrock_route,
|
||||
is_claude_model,
|
||||
is_known_openai_bare_model,
|
||||
|
|
@ -260,8 +259,7 @@ def make_model_settings(
|
|||
has_tools: bool = True,
|
||||
api_type: ApiType | None = None,
|
||||
) -> ModelSettings:
|
||||
"""``api_type`` is the resolved SDK-native OpenAI route, when known; it decides
|
||||
whether a reasoning model can be sent ``reasoning_effort`` at all."""
|
||||
"""``api_type`` is the resolved SDK-native OpenAI route, when known."""
|
||||
headers = _request_headers(model_name, extra_headers)
|
||||
model_settings = ModelSettings(
|
||||
parallel_tool_calls=False if has_tools else None,
|
||||
|
|
@ -270,20 +268,12 @@ def make_model_settings(
|
|||
extra_args=request_timeout_extra_args(request_timeout),
|
||||
extra_headers=headers,
|
||||
)
|
||||
if (
|
||||
reasoning_effort is not None
|
||||
and reasoning_effort != "none"
|
||||
and model_supports_reasoning(model_name)
|
||||
):
|
||||
if _route_rejects_reasoning_effort(model_name, api_type):
|
||||
logger.info(
|
||||
"Omitting reasoning_effort=%s: %s does not accept it on chat completions",
|
||||
reasoning_effort,
|
||||
model_name,
|
||||
)
|
||||
else:
|
||||
if reasoning_effort is not None and model_supports_reasoning(model_name):
|
||||
if reasoning_effort != "none":
|
||||
model_settings = model_settings.resolve(_reasoning_settings(reasoning_effort))
|
||||
elif _explicit_none_required(model_name, api_type):
|
||||
model_settings = model_settings.resolve(
|
||||
_reasoning_settings(reasoning_effort),
|
||||
ModelSettings(reasoning=Reasoning(effort="none"))
|
||||
)
|
||||
if force_required_tool_choice and _accepts_required_tool_choice(model_name):
|
||||
model_settings = model_settings.resolve(ModelSettings(tool_choice="required"))
|
||||
|
|
@ -298,12 +288,12 @@ def make_model_settings(
|
|||
return model_settings
|
||||
|
||||
|
||||
def _route_rejects_reasoning_effort(model_name: str, api_type: ApiType | None) -> bool:
|
||||
"""LiteLLM drops unsupported parameters itself on its own route; the
|
||||
SDK-native chat completions route sends whatever it is given."""
|
||||
if api_type != "chat_completions" or routes_through_litellm(model_name):
|
||||
return False
|
||||
return not chat_completions_accept_reasoning_effort(model_name)
|
||||
def _explicit_none_required(model_name: str, api_type: ApiType | None) -> bool:
|
||||
"""OpenAI's chat completions reason at a default effort when the field is
|
||||
absent, and the newer reasoning models reject function tools at any effort
|
||||
but ``none``, so ``none`` has to be sent, not left out. LiteLLM's own route
|
||||
maps the field per provider and is left alone."""
|
||||
return api_type == "chat_completions" and not routes_through_litellm(model_name)
|
||||
|
||||
|
||||
def _request_headers(
|
||||
|
|
|
|||
|
|
@ -88,6 +88,23 @@ def test_client_errors_are_not_transient() -> None:
|
|||
assert execution._is_transient_model_error(ValueError("nope")) is False
|
||||
|
||||
|
||||
def test_tools_with_reasoning_effort_rejection_carries_a_route_hint() -> None:
|
||||
rejected = BadRequestError(
|
||||
"Error code: 400 - {'error': {'message': \"Function tools with reasoning_effort are "
|
||||
"not supported for gpt-5.6-sol in /v1/chat/completions. To use function tools, use "
|
||||
"/v1/responses or set reasoning_effort to 'none'.\", 'param': 'reasoning_effort'}}",
|
||||
response=httpx.Response(400, request=_request()),
|
||||
body=None,
|
||||
)
|
||||
text = execution._failure_text(rejected)
|
||||
assert text.startswith("Error code: 400")
|
||||
assert "STRIX_API_TYPE=responses" in text
|
||||
assert "STRIX_REASONING_EFFORT=none" in text
|
||||
|
||||
other = BadRequestError("bad", response=httpx.Response(400, request=_request()), body=None)
|
||||
assert execution._failure_text(other) == "bad"
|
||||
|
||||
|
||||
class _FakeStream:
|
||||
def __init__(self, exc: BaseException | None = None) -> None:
|
||||
self._exc = exc
|
||||
|
|
|
|||
|
|
@ -432,29 +432,31 @@ def test_user_headers_override_openrouter_attribution() -> None:
|
|||
assert headers["HTTP-Referer"] == "https://strix.ai"
|
||||
|
||||
|
||||
def test_reasoning_effort_omitted_where_chat_completions_reject_it() -> None:
|
||||
# OpenAI serves some reasoning models' reasoning_effort on the Responses API
|
||||
# only; sending it on chat completions fails the request outright.
|
||||
chat = make_model_settings(
|
||||
"high", model_name="gpt-daybreak-blue-latest", api_type="chat_completions"
|
||||
)
|
||||
assert chat.reasoning is None
|
||||
responses = make_model_settings(
|
||||
"high", model_name="gpt-daybreak-blue-latest", api_type="responses"
|
||||
)
|
||||
assert responses.reasoning is not None
|
||||
assert responses.reasoning.effort == "high"
|
||||
def test_reasoning_effort_none_sent_explicitly_on_chat_completions() -> None:
|
||||
# Absent the field, OpenAI's chat completions reason at a default effort,
|
||||
# which the newer models reject together with function tools.
|
||||
for model in ("gpt-daybreak-blue-latest", "gpt-5.6-sol", "openai/gpt-5.4"):
|
||||
chat = make_model_settings("none", model_name=model, api_type="chat_completions")
|
||||
assert chat.reasoning is not None, model
|
||||
assert chat.reasoning.effort == "none"
|
||||
|
||||
|
||||
def test_reasoning_effort_kept_on_chat_completions_where_supported() -> None:
|
||||
for model in ("gpt-5.6-sol", "gpt-5", "openai/gpt-5.4"):
|
||||
settings = make_model_settings("high", model_name=model, api_type="chat_completions")
|
||||
assert settings.reasoning is not None, model
|
||||
def test_reasoning_effort_none_omitted_off_chat_completions() -> None:
|
||||
assert (
|
||||
make_model_settings("none", model_name="gpt-5.6-sol", api_type="responses").reasoning
|
||||
is None
|
||||
)
|
||||
assert make_model_settings("none", model_name="gpt-5.6-sol", api_type=None).reasoning is None
|
||||
litellm_route = make_model_settings(
|
||||
"none", model_name="litellm/gpt-5.6-sol", api_type="chat_completions"
|
||||
)
|
||||
assert litellm_route.reasoning is None
|
||||
|
||||
|
||||
def test_reasoning_effort_sent_as_configured_on_every_route() -> None:
|
||||
for api_type in ("chat_completions", "responses", None):
|
||||
settings = make_model_settings(
|
||||
"high", model_name="gpt-daybreak-blue-latest", api_type=api_type
|
||||
)
|
||||
assert settings.reasoning is not None, api_type
|
||||
assert settings.reasoning.effort == "high"
|
||||
|
||||
|
||||
def test_reasoning_effort_left_to_litellm_on_its_route() -> None:
|
||||
settings = make_model_settings(
|
||||
"high", model_name="litellm/gpt-daybreak-blue-latest", api_type="chat_completions"
|
||||
)
|
||||
assert settings.reasoning is not None
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue