Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions backend/app/services/chat_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

from app.config.config_models import RAGConfig
from app.config.llm_settings import GUARDRAIL_ROLE, LLMSettings
from app.services.llm_providers import PROVIDERS
from app.services.llm_providers import PROVIDERS, apply_timeout

logger = logging.getLogger(__name__)

Expand All @@ -43,11 +43,6 @@
# Cap on prior turns included as context, to avoid overflowing the model's context window.
MAX_HISTORY_MESSAGES = 10

# ChatOllama has no top-level `timeout` field — a bare `timeout=` kwarg is silently
# dropped. Its request timeout instead goes through `client_kwargs`, forwarded verbatim
# to the underlying httpx client. Every other provider accepts `timeout` directly.
_OLLAMA_PROVIDER = "ollama"

# Answer generation wants some variety in wording; safety classification wants the same
# message to get the same verdict every time (#207), so the guardrail stage samples greedily.
GENERATION_TEMPERATURE: Final = 0.7
Expand Down Expand Up @@ -104,10 +99,7 @@ def build_chat_model(
max_tokens_kwarg: config.max_tokens,
}
if timeout is not None:
if provider_name == _OLLAMA_PROVIDER:
kwargs["client_kwargs"] = {"timeout": timeout}
else:
kwargs["timeout"] = timeout
apply_timeout(provider, kwargs, timeout)
return init_chat_model(model_name, **kwargs)


Expand Down
43 changes: 41 additions & 2 deletions backend/app/services/llm_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
files: the max-tokens kwarg, base-URL fallback env vars, and credential env var in
``app.services.chat_model``, plus the infra-error exception types in
``app.guardrails.llm_classifier``. This module is the single per-provider table both
read from instead.
read from instead — including the ways a provider's constructor kwargs differ from
the common case, such as how the request timeout is wired (#164).
"""

from dataclasses import dataclass
from typing import Final
from typing import Any, Final

import anthropic
import ollama
Expand All @@ -18,6 +19,10 @@
from langchain_ollama import ChatOllama
from langchain_openai import ChatOpenAI

# The constructor kwarg carrying the request timeout for a provider that takes it
# top-level, which is what LangChain's own provider integrations expect.
DEFAULT_TIMEOUT_KWARG: Final = "timeout"


@dataclass(frozen=True, slots=True)
class ProviderConfig:
Expand All @@ -26,6 +31,9 @@ class ProviderConfig:
Args:
model_type: The LangChain ``BaseChatModel`` subclass this provider constructs.
max_tokens_kwarg: The constructor kwarg name for the output-length cap.
timeout_kwarg: The constructor kwarg name carrying the request timeout, or
``None`` for a provider whose chat model has no top-level timeout field
and takes the timeout nested under ``client_kwargs`` instead.
base_url_env_vars: Env vars this provider's own SDK falls back to for its API
base URL, in fallback order.
credential_env_var: The env var this provider's own SDK reads its credential
Expand All @@ -40,6 +48,12 @@ class ProviderConfig:
# field isn't named max_tokens (e.g. ChatOllama's num_predict) has to be mapped.
max_tokens_kwarg: str = "max_tokens"

# The constructor kwarg carrying the request timeout, or None for a provider whose
# chat model has no top-level timeout field: ChatOllama's request timeout belongs
# to its underlying httpx client, so it goes through `client_kwargs`, forwarded
# verbatim — a bare `timeout=` kwarg is silently dropped.
timeout_kwarg: str | None = DEFAULT_TIMEOUT_KWARG

# Each provider's own SDK falls back to these env vars for its API base URL when
# neither an explicit kwarg nor TAPIO_LLM_API_BASE is set. The cleartext-transport
# guard in chat_model.py has to check the URL that will actually be used, not just
Expand All @@ -64,6 +78,9 @@ class ProviderConfig:
"ollama": ProviderConfig(
model_type=ChatOllama,
max_tokens_kwarg="num_predict",
# ChatOllama has no top-level timeout field — its request timeout goes to the
# underlying httpx client through client_kwargs.
timeout_kwarg=None,
# ollama.Client reads OLLAMA_HOST as its API base fallback.
base_url_env_vars=("OLLAMA_HOST",),
infra_error_types=(ollama.ResponseError,),
Expand All @@ -88,3 +105,25 @@ class ProviderConfig:
infra_error_types=(anthropic.APIConnectionError, anthropic.APIStatusError),
),
}


def apply_timeout(provider: ProviderConfig | None, kwargs: dict[str, Any], timeout: float) -> None:
"""Apply a request timeout to constructor kwargs the way the provider takes it.

A provider whose chat model has no top-level timeout field has its timeout nested
under ``client_kwargs`` instead, which LangChain forwards verbatim to the
provider's underlying HTTP client. Which of the two applies is a per-provider
table entry, so no caller needs a provider-specific branch (#164).

Args:
provider: The provider's table entry, or ``None`` for a provider LangChain
supports but Tapio has no entry for — those take the plain top-level
``timeout`` kwarg.
kwargs: The constructor kwargs to add the timeout to, in place.
timeout: The request timeout in seconds.
"""
timeout_kwarg = provider.timeout_kwarg if provider else DEFAULT_TIMEOUT_KWARG
if timeout_kwarg is None:
kwargs["client_kwargs"] = {"timeout": timeout}
else:
kwargs[timeout_kwarg] = timeout
51 changes: 51 additions & 0 deletions backend/tests/services/test_llm_providers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
"""Tests for the per-provider configuration table (#146, #164)."""

from typing import Any

import pytest

from app.services.llm_providers import PROVIDERS, ProviderConfig, apply_timeout


class TestApplyTimeout:
"""Tests for wiring a request timeout into a provider's constructor kwargs."""

def test_a_provider_with_a_top_level_timeout_kwarg_takes_it_directly(self) -> None:
kwargs: dict[str, Any] = {}

apply_timeout(PROVIDERS["openai"], kwargs, 30.0)

assert kwargs == {"timeout": 30.0}

def test_a_provider_without_one_nests_it_under_client_kwargs(self) -> None:
"""ChatOllama has no top-level timeout field — its httpx client takes one."""
kwargs: dict[str, Any] = {}

apply_timeout(PROVIDERS["ollama"], kwargs, 30.0)

assert kwargs == {"client_kwargs": {"timeout": 30.0}}

def test_a_provider_without_a_table_entry_takes_a_top_level_timeout(self) -> None:
"""A provider LangChain supports but Tapio doesn't configure gets the common case."""
kwargs: dict[str, Any] = {}

apply_timeout(None, kwargs, 30.0)

assert kwargs == {"timeout": 30.0}

def test_the_wiring_comes_from_the_table_entry(self) -> None:
"""The nesting is the provider's own quirk, not a hardcoded provider name."""
provider = ProviderConfig(model_type=PROVIDERS["ollama"].model_type, timeout_kwarg=None)
kwargs: dict[str, Any] = {}

apply_timeout(provider, kwargs, 30.0)

assert kwargs == {"client_kwargs": {"timeout": 30.0}}

@pytest.mark.parametrize("provider_name", sorted(PROVIDERS))
def test_every_configured_provider_wires_the_timeout_one_way(self, provider_name: str) -> None:
kwargs: dict[str, Any] = {}

apply_timeout(PROVIDERS[provider_name], kwargs, 30.0)

assert kwargs in ({"timeout": 30.0}, {"client_kwargs": {"timeout": 30.0}})
Loading