diff --git a/strix/config/models.py b/strix/config/models.py index 858d7dd7..df26deac 100644 --- a/strix/config/models.py +++ b/strix/config/models.py @@ -5,7 +5,6 @@ from __future__ import annotations import os from typing import TYPE_CHECKING -import httpx from agents import set_default_openai_api, set_default_openai_key, set_tracing_disabled from agents.models.multi_provider import MultiProvider from agents.retry import ( @@ -22,19 +21,23 @@ if TYPE_CHECKING: from strix.config.settings import Settings -def request_timeout_extra_args(timeout_s: float | None) -> dict[str, httpx.Timeout] | None: +def request_timeout_extra_args(timeout_s: float | None) -> dict[str, float] | None: """Per-request model timeout as ``extra_args``, forwarded to the provider call. - Uses ``read`` for inactivity (matching pre-v1's per-chunk ``wait_for``): a stalled - stream trips ``read`` and is retried by ``DEFAULT_MODEL_RETRY``, while a healthy - long stream that keeps emitting tokens never trips it. An explicit ``httpx.Timeout`` - (rather than a scalar) keeps this a read-inactivity timeout instead of a - total-duration deadline on every httpx-based backend (``responses.create`` / - ``chat.completions.create`` / ``litellm.acompletion``). + A stalled stream trips the timeout and is retried by ``DEFAULT_MODEL_RETRY``, + restoring pre-v1's per-turn inactivity guard. + + The value MUST be a plain ``float``, not an ``httpx.Timeout``. The Chat + Completions and LiteLLM model paths build their tracing generation span from + ``ModelSettings.to_json_dict()``, which pydantic-serializes ``extra_args`` in + JSON mode; an ``httpx.Timeout`` is not JSON-serializable and raises + ``PydanticSerializationError`` there, failing every turn on those paths. A + scalar serializes cleanly and, on httpx-based clients, is applied as the read + (inactivity) timeout — not a total-duration deadline. """ if not timeout_s or timeout_s <= 0: return None - return {"timeout": httpx.Timeout(timeout_s, connect=min(timeout_s, 30.0))} + return {"timeout": timeout_s} def _retry_statusless_provider_errors(context: RetryPolicyContext) -> bool: diff --git a/tests/test_models.py b/tests/test_models.py index df1231c4..7965df56 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -3,6 +3,7 @@ from __future__ import annotations import pytest +from agents.model_settings import ModelSettings from strix.config.models import ( RECOMMENDED_MODEL_NAMES, @@ -17,18 +18,15 @@ def test_recommended_models_are_accepted(model_name: str) -> None: def test_request_timeout_extra_args_positive() -> None: - args = request_timeout_extra_args(300) - assert args is not None - timeout = args["timeout"] - # read (inactivity) carries the configured value; connect is capped so a dead - # endpoint fails fast rather than waiting the full read window. - assert timeout.read == 300.0 - assert timeout.connect == 30.0 + assert request_timeout_extra_args(300) == {"timeout": 300} + assert request_timeout_extra_args(10) == {"timeout": 10} - short = request_timeout_extra_args(10) - assert short is not None - assert short["timeout"].read == 10.0 - assert short["timeout"].connect == 10.0 + +def test_request_timeout_extra_args_survives_model_settings_json_dump() -> None: + """The Chat Completions and LiteLLM paths pydantic-serialize ModelSettings for + their tracing span; a non-JSON-serializable timeout fails every turn there.""" + settings = ModelSettings(extra_args=request_timeout_extra_args(300)) + assert settings.to_json_dict()["extra_args"] == {"timeout": 300} @pytest.mark.parametrize("value", [None, 0, -1])