feat(backend): adapt every completion to the detected backend (workspace T-137)
One choke point in the sanitized client: the flavor is probed once (the boilerroom wrapper names itself on /health, a bare llama-server serves /props, Ollama answers neither) and every completion adapts. The agents' tool_choice "required" survives only on Ollama — advisory there, enforced by llama-server, an unbreakable tool loop through the wrapper. Through the wrapper every completion carries webber's session identity: session webber, eviction_order 20 in the decided ranking, configurable via settings. The wrapper's balancing and compaction signals are read from the response body's extra fields — an httpx event-hook variant was tried and never fires under the openai SDK. The enabling fix: the sanitized client was never in the request path. The provider assigned self._openai_client, an attribute nobody reads — OllamaProvider.client serves self._client — so every completion has bypassed the null-content sanitizer since the class was introduced. Exposed when the wrapper 503'd a session-less request the choke point should have named; the client now goes through the constructor's official openai_client parameter, and a wiring test pins provider.client to the sanitized type. The same bug exists in tatlock (its T-6, filed). Verified against the live wrapper from the dev server: flavor boilerroom detected, a tool-using explore run answered in 4.8 s with no tool loop, webber resident at rank 20, and the wrapper parked librarian and tatlock-experts to seat it — the ranking doing exactly its job. Six new tests (227 green), five mutation-checked: the rank default, the strip condition, the session-add condition, the no-cache-on-failure rule, and the client wiring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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]]:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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`.
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user