refactor(llm): address review nits on timeout plumbing
Apply eisene's review feedback: - Rename PROVIDER_TIMEOUT_ERROR → PROVIDER_TIMEOUT_ERROR_TEXT - Move request_timeout_from_extra_params from backend.py (pure dataclasses) to request_builder.py (request assembly) - Add comment explaining Gemini's ms timeout conversion - Generalize _normalize_extra_params with _strip_none_params helper Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
ae1d6e375e
commit
7f39872073
|
|
@ -67,32 +67,34 @@ ThinkingEffortLevel = Literal[
|
|||
StructuredOutputMode = Literal["json_schema", "json_object"]
|
||||
|
||||
|
||||
PROVIDER_TIMEOUT_ERROR = "provider_params.timeout must be a positive number of seconds"
|
||||
PROVIDER_TIMEOUT_ERROR_TEXT = (
|
||||
"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`,
|
||||
per-request validation (`src.llm.request_builder.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)
|
||||
raise ValueError(PROVIDER_TIMEOUT_ERROR_TEXT)
|
||||
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
|
||||
raise ValueError(PROVIDER_TIMEOUT_ERROR_TEXT) from exc
|
||||
else:
|
||||
raise ValueError(PROVIDER_TIMEOUT_ERROR)
|
||||
raise ValueError(PROVIDER_TIMEOUT_ERROR_TEXT)
|
||||
|
||||
if not math.isfinite(timeout) or timeout <= 0:
|
||||
raise ValueError(PROVIDER_TIMEOUT_ERROR)
|
||||
raise ValueError(PROVIDER_TIMEOUT_ERROR_TEXT)
|
||||
return timeout
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -6,9 +6,6 @@ from typing import Any, Protocol, runtime_checkable
|
|||
|
||||
from pydantic import BaseModel
|
||||
|
||||
from src.config import coerce_provider_timeout
|
||||
from src.exceptions import ValidationException
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class ToolCallResult:
|
||||
|
|
@ -48,24 +45,6 @@ class StreamChunk:
|
|||
output_tokens: int | None = None
|
||||
|
||||
|
||||
def request_timeout_from_extra_params(
|
||||
extra_params: dict[str, Any] | None,
|
||||
) -> float | None:
|
||||
"""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
|
||||
|
||||
try:
|
||||
return coerce_provider_timeout(extra_params["timeout"])
|
||||
except ValueError as exc:
|
||||
raise ValidationException(str(exc)) from exc
|
||||
|
||||
|
||||
@runtime_checkable
|
||||
class ProviderBackend(Protocol):
|
||||
"""Transport-agnostic interface for LLM providers.
|
||||
|
|
|
|||
|
|
@ -8,13 +8,11 @@ from typing import Any
|
|||
from anthropic.types import TextBlock, ThinkingBlock, ToolUseBlock
|
||||
from pydantic import BaseModel, ValidationError
|
||||
|
||||
from src.llm.backend import (
|
||||
CompletionResult,
|
||||
StreamChunk,
|
||||
ToolCallResult,
|
||||
from src.llm.backend import CompletionResult, StreamChunk, ToolCallResult
|
||||
from src.llm.request_builder import (
|
||||
apply_sdk_passthroughs,
|
||||
request_timeout_from_extra_params,
|
||||
)
|
||||
from src.llm.request_builder import apply_sdk_passthroughs
|
||||
from src.llm.structured_output import repair_response_model_json, schema_instruction
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -8,19 +8,17 @@ from google.genai import types as genai_types
|
|||
from pydantic import BaseModel
|
||||
|
||||
from src.exceptions import LLMError, ValidationException
|
||||
from src.llm.backend import (
|
||||
CompletionResult,
|
||||
StreamChunk,
|
||||
ToolCallResult,
|
||||
request_timeout_from_extra_params,
|
||||
)
|
||||
from src.llm.backend import CompletionResult, StreamChunk, ToolCallResult
|
||||
from src.llm.caching import (
|
||||
GeminiCacheHandle,
|
||||
PromptCachePolicy,
|
||||
build_cache_key,
|
||||
gemini_cache_store,
|
||||
)
|
||||
from src.llm.request_builder import coerce_passthrough_mapping
|
||||
from src.llm.request_builder import (
|
||||
coerce_passthrough_mapping,
|
||||
request_timeout_from_extra_params,
|
||||
)
|
||||
from src.llm.structured_output import repair_response_model_json, schema_instruction
|
||||
|
||||
GEMINI_BLOCKED_FINISH_REASONS = {
|
||||
|
|
@ -323,6 +321,7 @@ class GeminiBackend:
|
|||
if timeout is not None:
|
||||
if http_options is None:
|
||||
http_options = genai_types.HttpOptions()
|
||||
# Gemini has no native timeout kwarg; set the httpx-level value in ms.
|
||||
http_options.timeout = int(timeout * 1000)
|
||||
if http_options is not None:
|
||||
config["http_options"] = http_options
|
||||
|
|
|
|||
|
|
@ -10,13 +10,11 @@ from openai import BadRequestError, LengthFinishReasonError
|
|||
from pydantic import BaseModel, ValidationError
|
||||
|
||||
from src.exceptions import ValidationException
|
||||
from src.llm.backend import (
|
||||
CompletionResult,
|
||||
StreamChunk,
|
||||
ToolCallResult,
|
||||
from src.llm.backend import CompletionResult, StreamChunk, ToolCallResult
|
||||
from src.llm.request_builder import (
|
||||
apply_sdk_passthroughs,
|
||||
request_timeout_from_extra_params,
|
||||
)
|
||||
from src.llm.request_builder import apply_sdk_passthroughs
|
||||
from src.llm.structured_output import (
|
||||
StructuredOutputError,
|
||||
empty_structured_output,
|
||||
|
|
|
|||
|
|
@ -11,14 +11,13 @@ from typing import Any, cast
|
|||
|
||||
from pydantic import BaseModel
|
||||
|
||||
from src.config import ModelConfig, PromptCachePolicy
|
||||
from src.config import ModelConfig, PromptCachePolicy, coerce_provider_timeout
|
||||
from src.exceptions import ValidationException
|
||||
|
||||
from .backend import (
|
||||
CompletionResult,
|
||||
ProviderBackend,
|
||||
StreamChunk,
|
||||
request_timeout_from_extra_params,
|
||||
)
|
||||
|
||||
# Operator escape-hatch keys recognized inside ModelConfig.provider_params.
|
||||
|
|
@ -104,12 +103,43 @@ def build_config_extra_params(config: ModelConfig) -> dict[str, Any]:
|
|||
return extra_params
|
||||
|
||||
|
||||
def request_timeout_from_extra_params(
|
||||
extra_params: dict[str, Any] | None,
|
||||
) -> float | None:
|
||||
"""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
|
||||
|
||||
try:
|
||||
return coerce_provider_timeout(extra_params["timeout"])
|
||||
except ValueError as exc:
|
||||
raise ValidationException(str(exc)) from exc
|
||||
|
||||
|
||||
def _strip_none_params(
|
||||
params: dict[str, Any],
|
||||
keys: tuple[str, ...],
|
||||
) -> dict[str, Any]:
|
||||
"""Remove specified keys from extra params when their values are None."""
|
||||
return {k: v for k, v in params.items() if not (k in keys and v is None)}
|
||||
|
||||
|
||||
def _normalize_extra_params(extra_params: dict[str, Any]) -> dict[str, Any]:
|
||||
"""Normalize shared extra params before they reach provider backends."""
|
||||
timeout = request_timeout_from_extra_params(extra_params)
|
||||
if timeout is None:
|
||||
return extra_params
|
||||
return {**extra_params, "timeout": timeout}
|
||||
"""Normalize and clean shared extra params before they reach backends.
|
||||
|
||||
Centralizes per-key coercion and null-stripping so new keys are added
|
||||
here rather than spawning one-off normalizers.
|
||||
"""
|
||||
result = dict(extra_params)
|
||||
timeout = request_timeout_from_extra_params(result)
|
||||
if timeout is not None:
|
||||
result["timeout"] = timeout
|
||||
return _strip_none_params(result, ("timeout",))
|
||||
|
||||
|
||||
async def execute_completion(
|
||||
|
|
|
|||
Loading…
Reference in New Issue