diff --git a/webber-api/src/ollama/provider.py b/webber-api/src/ollama/provider.py index a2de4a9..a863910 100644 --- a/webber-api/src/ollama/provider.py +++ b/webber-api/src/ollama/provider.py @@ -6,9 +6,20 @@ which PydanticAI sends for assistant messages that only contain tool calls. This provider sanitizes messages to use empty strings instead of null. Ported from tatlock project. + +It is also the one choke point where every completion is adapted to +the detected backend (workspace T-137): the agents author +`tool_choice: "required"` for Ollama, where it is a useful advisory +nudge — llama-server enforces it on every request in a run, which +through the boilerroom wrapper is an unbreakable tool loop (the +tatlock v2.6.0 cutover incident). The nudge therefore survives only +on Ollama, and through the wrapper every completion carries webber's +session name and eviction rank. """ +import asyncio from typing import Any +import httpx from openai import AsyncOpenAI from pydantic_ai.providers.ollama import OllamaProvider @@ -17,6 +28,85 @@ from src.shared.logging import get_logger logger = get_logger(__name__) +# Which server answers behind ollama_url: "boilerroom", "llama-server" +# or "ollama". Probed once, on the first completion: the wrapper names +# itself on /health, a bare llama-server serves /props, Ollama answers +# neither. A pure transport failure returns "ollama" WITHOUT caching, +# so a backend that was down at first call is re-probed rather than +# nudging llama-server forever. Ported from tatlock T-4. +_local_flavor: str | None = None +_flavor_lock = asyncio.Lock() + + +async def _detect_flavor() -> str: + global _local_flavor + if _local_flavor is not None: + return _local_flavor + async with _flavor_lock: + if _local_flavor is not None: + return _local_flavor + base = get_settings().ollama_url.rstrip("/") + connected = False + flavor = "ollama" + async with httpx.AsyncClient(timeout=5.0) as client: + try: + health = await client.get(f"{base}/health") + connected = True + if health.status_code == 200 and health.json().get("service") == "boilerroom": + flavor = "boilerroom" + except (httpx.HTTPError, ValueError): + pass + if flavor != "boilerroom": + try: + props = await client.get(f"{base}/props") + connected = True + if props.status_code == 200: + flavor = "llama-server" + except httpx.HTTPError: + pass + if connected: + _local_flavor = flavor + logger.info(f"local backend flavor detected: {flavor}") + return flavor + + +async def _adapt_to_backend(kwargs: dict[str, Any]) -> dict[str, Any]: + """Adapt one completion's extension fields to the detected backend.""" + flavor = await _detect_flavor() + extra = dict(kwargs.get("extra_body") or {}) + if flavor != "ollama": + # Advisory on Ollama, enforced by llama-server: an unbreakable + # tool loop through the wrapper. The nudge stays home. + extra.pop("tool_choice", None) + if flavor == "boilerroom": + settings = get_settings() + extra["session"] = settings.backend_session_name + extra["eviction_order"] = settings.backend_session_rank + if extra: + kwargs["extra_body"] = extra + else: + kwargs.pop("extra_body", None) + return kwargs + + +def _log_wrapper_signals(response: Any) -> None: + """Log the wrapper's per-response session signals (workspace T-137). + + Read from the parsed body's extra fields — the wrapper injects + `balancing` and `compaction_due` into non-streamed JSON answers, + and openai's pydantic models retain unknown fields in + `model_extra`. (An httpx event-hook variant was tried first and + never fired under the SDK; the body is the reliable channel, and + it is absent on any other backend, so this costs nothing there.) + Streams carry the signals in headers only and go unlogged here. + """ + extra = getattr(response, "model_extra", None) or {} + balancing = extra.get("balancing") + if balancing: + logger.info(f"backend balancing: {balancing}") + if extra.get("compaction_due"): + logger.warning("backend compaction due for webber's session") + class WebberOllamaProvider(OllamaProvider): """ @@ -38,10 +128,14 @@ class WebberOllamaProvider(OllamaProvider): clean_host = settings.ollama_url.rstrip("/") base_url = f"{clean_host}/v1" - super().__init__(base_url=base_url) - - # Override the client with our sanitized version - self._openai_client = _SanitizedAsyncOpenAI(base_url=base_url) + # The sanitized client goes through the official constructor + # parameter: the provider's `.client` property serves `_client`, + # and the previous pattern — poking `self._openai_client` after + # super().__init__ had built its own client — assigned an + # attribute nobody reads. Every completion bypassed the + # sanitizer and the backend adaptation until 2026-09-13, when + # the wrapper's 503 on a session-less request exposed it. + super().__init__(openai_client=_SanitizedAsyncOpenAI(base_url=base_url)) logger.debug(f"WebberOllamaProvider created with base_url={base_url}") @@ -125,7 +219,11 @@ class _SanitizedCompletions: if "messages" in kwargs: kwargs["messages"] = _sanitize_messages(kwargs["messages"]) - return await self._original.create(**kwargs) + kwargs = await _adapt_to_backend(kwargs) + + response = await self._original.create(**kwargs) + _log_wrapper_signals(response) + return response def _sanitize_messages(messages: list[dict[str, Any]]) -> list[dict[str, Any]]: diff --git a/webber-api/src/shared/config.py b/webber-api/src/shared/config.py index 83eea7a..d57d1f0 100644 --- a/webber-api/src/shared/config.py +++ b/webber-api/src/shared/config.py @@ -67,6 +67,13 @@ class Settings(BaseSettings): ollama_agent_model: str = "gemma4:e2b" ollama_embed_model: str = "nomic-embed-text:latest" + # Session identity through the boilerroom wrapper (workspace T-137): + # rank 20 in the decided ordering — tatlock phases 40, experts 35, + # librarian 30, webber 20; lower parks sooner. The fields only mean + # something to the wrapper; Ollama and a bare llama-server ignore them. + backend_session_name: str = "webber" + backend_session_rank: int = 20 + # Auth - Tatlock integration tatlock_api_url: str | None = "http://tatlock:8000" internal_api_key: str | None = None diff --git a/webber-api/tests/test_ollama_provider.py b/webber-api/tests/test_ollama_provider.py index fed1908..21587a8 100644 --- a/webber-api/tests/test_ollama_provider.py +++ b/webber-api/tests/test_ollama_provider.py @@ -28,9 +28,15 @@ class TestSanitizedClientIsReachable: assert chat.completions is not None def test_provider_reaches_completions(self): - """The full chain an agent request walks, short of the network call.""" + """The full chain an agent request walks, short of the network call. + + Through `.client` — the property pydantic_ai actually reads. The + old assertion walked `_openai_client`, a lookalike attribute + nobody read, which is exactly how the sanitizer sat bypassed in + production until 2026-09-13. + """ provider = get_ollama_provider() - assert provider._openai_client.chat.completions is not None + assert provider.client.chat.completions is not None def test_parent_lookup_survives_either_descriptor_shape(self): """openai has used both property and cached_property for `chat`. diff --git a/webber-api/tests/test_provider_backend.py b/webber-api/tests/test_provider_backend.py new file mode 100644 index 0000000..829433f --- /dev/null +++ b/webber-api/tests/test_provider_backend.py @@ -0,0 +1,117 @@ +""" +Backend adaptation at the provider choke point (workspace T-137). + +The flavor globals are set directly so the tests are deterministic +regardless of which backend is reachable; the wire facts the probe +relies on are the wrapper's contract, pinned in its own repo. +""" +import pytest + +from src.ollama import provider +from src.shared.config import get_settings + + +@pytest.fixture(autouse=True) +def reset_flavor(monkeypatch): + """Each test states its flavor; nothing leaks between them.""" + monkeypatch.setattr(provider, "_local_flavor", None) + + +@pytest.mark.anyio +async def test_ollama_keeps_the_advisory_nudge(monkeypatch): + monkeypatch.setattr(provider, "_local_flavor", "ollama") + kwargs = await provider._adapt_to_backend( + {"extra_body": {"tool_choice": "required"}} + ) + assert kwargs["extra_body"] == {"tool_choice": "required"} + + +@pytest.mark.anyio +async def test_wrapper_strips_the_nudge_and_names_the_session(monkeypatch): + # tool_choice "required" is enforced by llama-server — through the + # wrapper it is the unbreakable tool loop, so it must not pass. + monkeypatch.setattr(provider, "_local_flavor", "boilerroom") + kwargs = await provider._adapt_to_backend( + {"extra_body": {"tool_choice": "required"}} + ) + extra = kwargs["extra_body"] + assert "tool_choice" not in extra + assert extra["session"] == "webber" + assert extra["eviction_order"] == 20 + + +@pytest.mark.anyio +async def test_bare_llama_server_strips_without_naming(monkeypatch): + monkeypatch.setattr(provider, "_local_flavor", "llama-server") + kwargs = await provider._adapt_to_backend( + {"extra_body": {"tool_choice": "required"}} + ) + assert "extra_body" not in kwargs + + +@pytest.mark.anyio +async def test_rank_and_name_come_from_settings(monkeypatch): + monkeypatch.setattr(provider, "_local_flavor", "boilerroom") + settings = get_settings() + monkeypatch.setattr(settings, "backend_session_name", "webber-test") + monkeypatch.setattr(settings, "backend_session_rank", 7) + kwargs = await provider._adapt_to_backend({}) + assert kwargs["extra_body"] == {"session": "webber-test", "eviction_order": 7} + + +@pytest.mark.anyio +async def test_other_extension_fields_survive_adaptation(monkeypatch): + monkeypatch.setattr(provider, "_local_flavor", "boilerroom") + kwargs = await provider._adapt_to_backend({"extra_body": {"marker": 1}}) + assert kwargs["extra_body"]["marker"] == 1 + assert kwargs["extra_body"]["session"] == "webber" + + +def test_body_signals_are_logged(monkeypatch): + # The httpx event-hook variant never fired under the SDK; the body + # read must demonstrably log, or the signal passes silently again. + calls = [] + + class StubLogger: + def info(self, msg): + calls.append(("info", msg)) + + def warning(self, msg): + calls.append(("warning", msg)) + + monkeypatch.setattr(provider, "logger", StubLogger()) + + class Busy: + model_extra = {"balancing": [{"parked": "librarian"}], "compaction_due": True} + + provider._log_wrapper_signals(Busy()) + assert any(kind == "info" and "parked" in msg for kind, msg in calls) + assert any(kind == "warning" for kind, msg in calls) + + calls.clear() + + class Quiet: + model_extra = {"balancing": [], "compaction_due": False} + + provider._log_wrapper_signals(Quiet()) + assert calls == [] + + +def test_provider_serves_the_sanitized_client(): + # The whole mechanism rides on `.client` returning OUR instance: + # assigning a lookalike attribute after super().__init__ built its + # own client is how the sanitizer silently died until 2026-09-13. + p = provider.WebberOllamaProvider(base_url="http://127.0.0.1:9/v1") + assert isinstance(p.client, provider._SanitizedAsyncOpenAI) + + +@pytest.mark.anyio +async def test_probe_failure_answers_ollama_without_caching(monkeypatch): + # A backend that is down at first call must be re-probed later — + # caching "ollama" forever would nudge llama-server into the tool + # loop the moment the wrapper came back. + settings = get_settings() + monkeypatch.setattr(settings, "ollama_url", "http://127.0.0.1:9") + flavor = await provider._detect_flavor() + assert flavor == "ollama" + assert provider._local_flavor is None