From 218cb7a24c51f0d4b8d176607c421cee2d0390df Mon Sep 17 00:00:00 2001 From: Aakash Kattelu Date: Mon, 3 Aug 2026 17:40:14 -0400 Subject: [PATCH] fix(llm): validate provider_params.timeout at config load Move the timeout coercion into src.config as coerce_provider_timeout and run it from a field validator on ModelOverrideSettings.provider_params, so a bad value in config.toml/env fails at startup with the exact config path instead of surfacing per-request as a retried 500. Good values normalize to float seconds at load. The per-request guard in src.llm.backend now delegates to the same coercion (wrapping ValueError in ValidationException) and continues to cover extra_params passed programmatically. Co-Authored-By: Claude Fable 5 --- src/config.py | 38 +++++++++++++++++++++++++++++++++++ src/llm/backend.py | 37 +++++++++++----------------------- tests/test_config.py | 47 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 26 deletions(-) diff --git a/src/config.py b/src/config.py index 84eedecd..82fa5716 100644 --- a/src/config.py +++ b/src/config.py @@ -1,4 +1,5 @@ import logging +import math import os from pathlib import Path from typing import Annotated, Any, ClassVar, Literal, cast @@ -66,6 +67,35 @@ ThinkingEffortLevel = Literal[ StructuredOutputMode = Literal["json_schema", "json_object"] +PROVIDER_TIMEOUT_ERROR = "provider_params.timeout must be a positive number of seconds" + + +def coerce_provider_timeout(value: Any) -> float: + """Coerce a `provider_params.timeout` value to positive, finite seconds. + + Canonical implementation shared by config-load validation (here) and + per-request validation (`src.llm.backend.request_timeout_from_extra_params`, + which translates the ValueError into a ValidationException). Lives in + config.py because src.exceptions imports src.config, so config validators + cannot raise Honcho exception types. + """ + if isinstance(value, bool): + raise ValueError(PROVIDER_TIMEOUT_ERROR) + if isinstance(value, int | float): + timeout = float(value) + elif isinstance(value, str): + try: + timeout = float(value.strip()) + except ValueError as exc: + raise ValueError(PROVIDER_TIMEOUT_ERROR) from exc + else: + raise ValueError(PROVIDER_TIMEOUT_ERROR) + + if not math.isfinite(timeout) or timeout <= 0: + raise ValueError(PROVIDER_TIMEOUT_ERROR) + return timeout + + class ModelOverrideSettings(BaseModel): """Advanced module-level transport overrides.""" @@ -91,6 +121,14 @@ class ModelOverrideSettings(BaseModel): ), ) + @field_validator("provider_params") + @classmethod + def _validate_provider_timeout(cls, v: dict[str, Any]) -> dict[str, Any]: + """Reject bad `timeout` values at config load; normalize good ones to float.""" + if "timeout" not in v: + return v + return {**v, "timeout": coerce_provider_timeout(v["timeout"])} + class PromptCachePolicy(BaseModel): """Per-call prompt-caching configuration. diff --git a/src/llm/backend.py b/src/llm/backend.py index 0211f161..0eca1fc9 100644 --- a/src/llm/backend.py +++ b/src/llm/backend.py @@ -1,12 +1,12 @@ from __future__ import annotations -import math from collections.abc import AsyncIterator from dataclasses import dataclass, field from typing import Any, Protocol, runtime_checkable from pydantic import BaseModel +from src.config import coerce_provider_timeout from src.exceptions import ValidationException @@ -51,34 +51,19 @@ class StreamChunk: def request_timeout_from_extra_params( extra_params: dict[str, Any] | None, ) -> float | None: - """Return a validated per-request provider timeout from extra params.""" + """Return a validated per-request provider timeout from extra params. + + Config-sourced timeouts are already validated and normalized at config + load (`coerce_provider_timeout` in src.config); this guards extra_params + passed programmatically at call time. + """ if not extra_params or "timeout" not in extra_params: return None - value = extra_params["timeout"] - if isinstance(value, bool): - raise ValidationException( - "provider_params.timeout must be a positive number of seconds" - ) - if isinstance(value, int | float): - timeout = float(value) - elif isinstance(value, str): - try: - timeout = float(value.strip()) - except ValueError as exc: - raise ValidationException( - "provider_params.timeout must be a positive number of seconds" - ) from exc - else: - raise ValidationException( - "provider_params.timeout must be a positive number of seconds" - ) - - if not math.isfinite(timeout) or timeout <= 0: - raise ValidationException( - "provider_params.timeout must be a positive number of seconds" - ) - return timeout + try: + return coerce_provider_timeout(extra_params["timeout"]) + except ValueError as exc: + raise ValidationException(str(exc)) from exc @runtime_checkable diff --git a/tests/test_config.py b/tests/test_config.py index 7730c751..1a714e55 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -81,3 +81,50 @@ def test_representation_batch_target_input_cannot_exceed_max_input_tokens() -> N MAX_INPUT_TOKENS=1000, REPRESENTATION_BATCH_TARGET_INPUT_TOKENS=2048, ) + + +def _configured_with_timeout(timeout: object) -> ConfiguredModelSettings: + return ConfiguredModelSettings.model_validate( + { + "model": "gpt-5.4-mini", + "transport": "openai", + "overrides": {"provider_params": {"timeout": timeout}}, + } + ) + + +@pytest.mark.parametrize("timeout", [30, 42.5, "42.5", " 60 "]) +def test_provider_timeout_is_normalized_at_config_load(timeout: object) -> None: + settings = _configured_with_timeout(timeout) + + normalized = settings.overrides.provider_params["timeout"] + assert isinstance(normalized, float) + assert normalized == float(str(timeout).strip()) + + +@pytest.mark.parametrize( + "timeout", + ["slow", "", 0, -1, True, float("nan"), float("inf"), "nan", "inf", None, [30]], +) +def test_provider_timeout_is_rejected_at_config_load(timeout: object) -> None: + with pytest.raises( + ValueError, match=r"provider_params\.timeout must be a positive number" + ): + _configured_with_timeout(timeout) + + +def test_provider_timeout_on_fallback_overrides_is_validated_at_config_load() -> None: + with pytest.raises( + ValueError, match=r"provider_params\.timeout must be a positive number" + ): + ConfiguredModelSettings.model_validate( + { + "model": "gpt-5.4-mini", + "transport": "openai", + "fallback": { + "model": "gpt-4.1", + "transport": "openai", + "overrides": {"provider_params": {"timeout": "slow"}}, + }, + } + )