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 <noreply@anthropic.com>
This commit is contained in:
parent
fec7941bb0
commit
218cb7a24c
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"}},
|
||||
},
|
||||
}
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in New Issue