diff --git a/strix/config/models.py b/strix/config/models.py index 31bef982..01598054 100644 --- a/strix/config/models.py +++ b/strix/config/models.py @@ -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 diff --git a/strix/core/execution.py b/strix/core/execution.py index 69a5cf87..5d4f0a32 100644 --- a/strix/core/execution.py +++ b/strix/core/execution.py @@ -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 diff --git a/strix/core/inputs.py b/strix/core/inputs.py index bb33c5b9..f99e7a28 100644 --- a/strix/core/inputs.py +++ b/strix/core/inputs.py @@ -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( diff --git a/tests/test_execution_transient_retry.py b/tests/test_execution_transient_retry.py index d81e4e70..d53a25d2 100644 --- a/tests/test_execution_transient_retry.py +++ b/tests/test_execution_transient_retry.py @@ -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 diff --git a/tests/test_inputs.py b/tests/test_inputs.py index e4cdf5aa..01d7f89a 100644 --- a/tests/test_inputs.py +++ b/tests/test_inputs.py @@ -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