diff --git a/.pql/changelog/ticket_history/2026-09.sql b/.pql/changelog/ticket_history/2026-09.sql index e95f541..dad01d6 100644 --- a/.pql/changelog/ticket_history/2026-09.sql +++ b/.pql/changelog/ticket_history/2026-09.sql @@ -1,2 +1,7 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G99D07EM5DZWTS0WD9V8PSKM', 'status', 'backlog', 'in_progress', NULL, '2026-09-12 13:50:48', '2026-09-12 13:50:48.289', '2026-09-12 13:50:48.289', NULL, '2e88d155325b3e704787f48b2aa97591', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G99D07EM5DZWTS0WD9V8PSKM', 'status', 'in_progress', 'done', NULL, '2026-09-12 14:13:29', '2026-09-12 14:13:29.027', '2026-09-12 14:13:29.027', NULL, '5b9dd9a62002db0fb4dc58f7f9ffcbb6', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G9KH87TRDGW45GMDWVDRPABR', 'description', 'src/ollama/provider.py sets self._openai_client after super().__init__, but OllamaProvider.client serves self._client (pydantic_ai 1.27''s official openai_client= constructor param is the right wiring — webber hit the identical bug and fixed it 2026-09-13, exposed by the wrapper 503ing a session-less request). Silently dead in production as a result: the content-null sanitizer (moot on llama-server, live hazard if Ollama ever returns), the T-4 x-boilerroom-* signal hook (never logs), and the OLLAMA_TIMEOUT=120 cap (SDK default ~600s applies). Sessions were unaffected — they ride ModelSettings extra_body. Fix like webber: pass _SanitizedAsyncOpenAI through openai_client=, add a wiring test asserting provider.client is the sanitized type, then verify balancing/compaction logs actually appear.', 'src/ollama/provider.py sets self._openai_client after super().__init__, but OllamaProvider.client serves self._client (pydantic_ai 1.27''s official openai_client= constructor param is the right wiring — webber hit the identical bug and fixed it 2026-09-13, exposed by the wrapper 503ing a session-less request). Silently dead in production as a result: the content-null sanitizer (moot on llama-server, live hazard if Ollama ever returns), the T-4 x-boilerroom-* signal hook (never logs), and the OLLAMA_TIMEOUT=120 cap (SDK default ~600s applies). Sessions were unaffected — they ride ModelSettings extra_body. Fix like webber: pass _SanitizedAsyncOpenAI through openai_client=, add a wiring test asserting provider.client is the sanitized type, then verify balancing/compaction logs actually appear. + +Second finding 2026-09-13, from webber''s twin fix: an httpx event-hook on the client (the T-4 signal-reading approach) never fires under the openai SDK even when correctly wired — webber verified headers arriving while the hook stayed silent. When rewiring the client, read the signals from the parsed body''s model_extra in the sanitized create() instead (the wrapper injects balancing/compaction_due into non-streamed JSON bodies; openai pydantic models retain unknown fields). The steward''s raw-call body read is unaffected and already correct.', NULL, '2026-09-13 08:00:17', '2026-09-13 08:00:17.002', '2026-09-13 08:00:17.002', NULL, '6fb4cc1d7d7222e3d570cd8c7fed4fd9', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G9KH87TRDGW45GMDWVDRPABR', 'status', 'backlog', 'in_progress', NULL, '2026-09-13 08:19:35', '2026-09-13 08:19:35.028', '2026-09-13 08:19:35.028', NULL, '5bcf19379361a2f766cd4787ff734611', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G9KH87TRDGW45GMDWVDRPABR', 'status', 'in_progress', 'done', NULL, '2026-09-13 08:27:21', '2026-09-13 08:27:21.779', '2026-09-13 08:27:21.779', NULL, '1c59422336b88a76c6f1c3720916d7d5', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/ticket_idmap/2026-09.sql b/.pql/changelog/ticket_idmap/2026-09.sql index cf883f1..67b69d4 100644 --- a/.pql/changelog/ticket_idmap/2026-09.sql +++ b/.pql/changelog/ticket_idmap/2026-09.sql @@ -1,3 +1,4 @@ INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G94KC9XM3PW91C6B7VJA6VRM', 'T-3', '2026-09-11 21:09:10.267', '2026-09-11 21:09:10.267', NULL, 'a1ccf4d118877c230dffb961a7531cb4', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at; INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G99D07EM5DZWTS0WD9V8PSKM', 'T-4', '2026-09-12 08:20:21.494', '2026-09-12 08:20:21.494', NULL, '5b28e07bdc3be1f9ed0ed71d2da60c72', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at; INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G99D07Z3FZY8EBT9PJ1RCGHW', 'T-5', '2026-09-12 08:20:21.625', '2026-09-12 08:20:21.625', NULL, '0a417012779f612a4dad1a7dbd0787e3', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at; +INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G9KH87TRDGW45GMDWVDRPABR', 'T-6', '2026-09-13 07:57:01.782', '2026-09-13 07:57:01.782', NULL, 'd6c184ba6d24e03c0dc77064bf71fead', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at; diff --git a/.pql/changelog/tickets/2026-09.sql b/.pql/changelog/tickets/2026-09.sql index e4cd4a1..8015b7b 100644 --- a/.pql/changelog/tickets/2026-09.sql +++ b/.pql/changelog/tickets/2026-09.sql @@ -3,3 +3,13 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G99D07Z3FZY8EBT9PJ1RCGHW', 'bug', NULL, 'The container logs nowhere: stdout empty, /app/logs mount untouched', 'docker logs tatlock is 0 lines and the mounted ~/docker-data/tatlock/logs has been empty since 2025-12 — the running app writes logs to neither, which made the 2026-09-12 wrapper-cutover verification blind (flavor detection had to be proven behaviorally). Dev mode logs to build/logs/server.log via make run; production should log to stdout for docker logs at minimum.', 'backlog', 'medium', NULL, NULL, NULL, '2026-09-12 08:20:21.624', '2026-09-12 08:20:21.624', NULL, '3fd294efc9afa857aef15810d9003298', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G99D07EM5DZWTS0WD9V8PSKM', 'story', NULL, 'Adopt boilerroom named sessions with the usage ranking', 'Replace raw id_slot pinning (BACKEND_SLOT_PINNING) with the wrapper''s session extension fields, now that tatlock routes through boilerroom:8602 (since 2026-09-12). Ranking by likelihood of use, decided 2026-09-12 — lower eviction_order parks sooner: tatlock pipeline phases highest (eviction_order 40; e.g. sessions tatlock-steward/tatlock-orchestrate/tatlock-synthesize), librarian-routed calls as session librarian (30), webber will sit at 20 (workspace T-137), Open WebUI stays session-less by design (idle slots only, can never evict — and it is being deprecated for an own-webui build). Read the x-boilerroom-balancing and compaction_due signals; the compaction endpoint expects the client transcript (tatlock''s Redis/Qdrant context is the natural producer — boilerroom D-5 names tatlock the first producer).', 'in_progress', 'high', NULL, NULL, NULL, '2026-09-12 08:20:21.493', '2026-09-12 13:50:48.289', NULL, '62abeb4da2f1f7aedb8d92884e7e148c', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G99D07EM5DZWTS0WD9V8PSKM', 'story', NULL, 'Adopt boilerroom named sessions with the usage ranking', 'Replace raw id_slot pinning (BACKEND_SLOT_PINNING) with the wrapper''s session extension fields, now that tatlock routes through boilerroom:8602 (since 2026-09-12). Ranking by likelihood of use, decided 2026-09-12 — lower eviction_order parks sooner: tatlock pipeline phases highest (eviction_order 40; e.g. sessions tatlock-steward/tatlock-orchestrate/tatlock-synthesize), librarian-routed calls as session librarian (30), webber will sit at 20 (workspace T-137), Open WebUI stays session-less by design (idle slots only, can never evict — and it is being deprecated for an own-webui build). Read the x-boilerroom-balancing and compaction_due signals; the compaction endpoint expects the client transcript (tatlock''s Redis/Qdrant context is the natural producer — boilerroom D-5 names tatlock the first producer).', 'done', 'high', NULL, NULL, NULL, '2026-09-12 08:20:21.493', '2026-09-12 14:13:29.027', NULL, '89ce1be323ebb26f00c63569f197cab1', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G9KH87TRDGW45GMDWVDRPABR', 'bug', NULL, 'The sanitized OpenAI client is never used: the override pokes an attribute nobody reads', 'src/ollama/provider.py sets self._openai_client after super().__init__, but OllamaProvider.client serves self._client (pydantic_ai 1.27''s official openai_client= constructor param is the right wiring — webber hit the identical bug and fixed it 2026-09-13, exposed by the wrapper 503ing a session-less request). Silently dead in production as a result: the content-null sanitizer (moot on llama-server, live hazard if Ollama ever returns), the T-4 x-boilerroom-* signal hook (never logs), and the OLLAMA_TIMEOUT=120 cap (SDK default ~600s applies). Sessions were unaffected — they ride ModelSettings extra_body. Fix like webber: pass _SanitizedAsyncOpenAI through openai_client=, add a wiring test asserting provider.client is the sanitized type, then verify balancing/compaction logs actually appear.', 'backlog', 'high', NULL, NULL, NULL, '2026-09-13 07:57:01.782', '2026-09-13 07:57:01.782', NULL, '5e8ea3bf2d18ade9d17f0044ae1ff96b', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G9KH87TRDGW45GMDWVDRPABR', 'bug', NULL, 'The sanitized OpenAI client is never used: the override pokes an attribute nobody reads', 'src/ollama/provider.py sets self._openai_client after super().__init__, but OllamaProvider.client serves self._client (pydantic_ai 1.27''s official openai_client= constructor param is the right wiring — webber hit the identical bug and fixed it 2026-09-13, exposed by the wrapper 503ing a session-less request). Silently dead in production as a result: the content-null sanitizer (moot on llama-server, live hazard if Ollama ever returns), the T-4 x-boilerroom-* signal hook (never logs), and the OLLAMA_TIMEOUT=120 cap (SDK default ~600s applies). Sessions were unaffected — they ride ModelSettings extra_body. Fix like webber: pass _SanitizedAsyncOpenAI through openai_client=, add a wiring test asserting provider.client is the sanitized type, then verify balancing/compaction logs actually appear. + +Second finding 2026-09-13, from webber''s twin fix: an httpx event-hook on the client (the T-4 signal-reading approach) never fires under the openai SDK even when correctly wired — webber verified headers arriving while the hook stayed silent. When rewiring the client, read the signals from the parsed body''s model_extra in the sanitized create() instead (the wrapper injects balancing/compaction_due into non-streamed JSON bodies; openai pydantic models retain unknown fields). The steward''s raw-call body read is unaffected and already correct.', 'backlog', 'high', NULL, NULL, NULL, '2026-09-13 07:57:01.782', '2026-09-13 08:00:17.002', NULL, '755abc3689e8ff4fc511deb164137fe5', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G9KH87TRDGW45GMDWVDRPABR', 'bug', NULL, 'The sanitized OpenAI client is never used: the override pokes an attribute nobody reads', 'src/ollama/provider.py sets self._openai_client after super().__init__, but OllamaProvider.client serves self._client (pydantic_ai 1.27''s official openai_client= constructor param is the right wiring — webber hit the identical bug and fixed it 2026-09-13, exposed by the wrapper 503ing a session-less request). Silently dead in production as a result: the content-null sanitizer (moot on llama-server, live hazard if Ollama ever returns), the T-4 x-boilerroom-* signal hook (never logs), and the OLLAMA_TIMEOUT=120 cap (SDK default ~600s applies). Sessions were unaffected — they ride ModelSettings extra_body. Fix like webber: pass _SanitizedAsyncOpenAI through openai_client=, add a wiring test asserting provider.client is the sanitized type, then verify balancing/compaction logs actually appear. + +Second finding 2026-09-13, from webber''s twin fix: an httpx event-hook on the client (the T-4 signal-reading approach) never fires under the openai SDK even when correctly wired — webber verified headers arriving while the hook stayed silent. When rewiring the client, read the signals from the parsed body''s model_extra in the sanitized create() instead (the wrapper injects balancing/compaction_due into non-streamed JSON bodies; openai pydantic models retain unknown fields). The steward''s raw-call body read is unaffected and already correct.', 'in_progress', 'high', NULL, NULL, NULL, '2026-09-13 07:57:01.782', '2026-09-13 08:19:35.028', NULL, '5af3c96930a30bbe4bb64d13ddbc509e', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G9KH87TRDGW45GMDWVDRPABR', 'bug', NULL, 'The sanitized OpenAI client is never used: the override pokes an attribute nobody reads', 'src/ollama/provider.py sets self._openai_client after super().__init__, but OllamaProvider.client serves self._client (pydantic_ai 1.27''s official openai_client= constructor param is the right wiring — webber hit the identical bug and fixed it 2026-09-13, exposed by the wrapper 503ing a session-less request). Silently dead in production as a result: the content-null sanitizer (moot on llama-server, live hazard if Ollama ever returns), the T-4 x-boilerroom-* signal hook (never logs), and the OLLAMA_TIMEOUT=120 cap (SDK default ~600s applies). Sessions were unaffected — they ride ModelSettings extra_body. Fix like webber: pass _SanitizedAsyncOpenAI through openai_client=, add a wiring test asserting provider.client is the sanitized type, then verify balancing/compaction logs actually appear. + +Second finding 2026-09-13, from webber''s twin fix: an httpx event-hook on the client (the T-4 signal-reading approach) never fires under the openai SDK even when correctly wired — webber verified headers arriving while the hook stayed silent. When rewiring the client, read the signals from the parsed body''s model_extra in the sanitized create() instead (the wrapper injects balancing/compaction_due into non-streamed JSON bodies; openai pydantic models retain unknown fields). The steward''s raw-call body read is unaffected and already correct.', 'done', 'high', NULL, NULL, NULL, '2026-09-13 07:57:01.782', '2026-09-13 08:27:21.778', NULL, 'ce3151ff27c70a1028ba7c03b07119a2', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; diff --git a/CHANGELOG.md b/CHANGELOG.md index cacd4f6..21616dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- The sanitized OpenAI client was never used (T-6): the provider + assigned `self._openai_client`, an attribute nobody reads — + `OllamaProvider.client` serves `self._client` — so the null-content + sanitizer, the wrapper signal reading and the `OLLAMA_TIMEOUT` cap + were all silently dead (sessions were unaffected; they ride + ModelSettings). Found via webber's identical bug. The client now + goes through the constructor's official `openai_client` parameter, + the parent-chat lookup survives openai's property/cached_property + descriptor change (ported from webber's 2026-08-11 fix — the old + `.fget` access would have crashed the first wired completion on + openai 2.11), and the wrapper's `balancing`/`compaction_due` + signals are read from the response body's `model_extra` in the + request path — the httpx event-hook approach demonstrably never + fires under the openai SDK. The provider tests now assert through + `provider.client`, the property pydantic_ai actually reads; the + old ones asserted on the dead attribute and stayed green + throughout. + ## [2.7.0] - 2026-09-13 ### Added diff --git a/src/ollama/provider.py b/src/ollama/provider.py index 93b4298..43217b3 100644 --- a/src/ollama/provider.py +++ b/src/ollama/provider.py @@ -4,11 +4,21 @@ PydanticAI provider for Ollama with message sanitization. Ollama's OpenAI-compatible API rejects messages with `content: null`, which PydanticAI sends for assistant messages that only contain tool calls. This provider sanitizes messages to use empty strings instead of null. + +History worth keeping (repo T-6): until 2026-09-13 the sanitized client +was never in the request path — the provider assigned +`self._openai_client`, an attribute nobody reads, while +`OllamaProvider.client` serves `self._client`. The sanitizer, the +wrapper-signal reading and the OLLAMA_TIMEOUT cap were all silently +dead (sessions were unaffected — they ride ModelSettings). webber hit +the identical bug the same day; both now wire the client through the +constructor's official `openai_client` parameter, and the signals are +read from the response body — an httpx event-hook variant demonstrably +never fires under the openai SDK. """ from typing import Any -import httpx from openai import AsyncOpenAI from pydantic_ai.providers.ollama import OllamaProvider @@ -18,21 +28,25 @@ from src.core.logging_config import get_logger logger = get_logger(__name__) -async def _log_wrapper_signals(response: httpx.Response) -> None: +def _log_wrapper_signals(response: Any) -> None: """ Surface the boilerroom wrapper's per-response signals (repo T-4). balancing names the sessions parked or restored to serve this request (wrapper D-4 — never silent); compaction-due says a session crossed its authored context budget (wrapper D-5 — a signal, not an - interrupt). Reading them is this ticket; producing the compacted - transcript is a later one. The headers are absent on any other - backend, so the hook costs nothing there. + interrupt). Read from the parsed body's extra fields: the wrapper + injects them into non-streamed JSON answers and openai's pydantic + models retain unknown fields in `model_extra`. Absent on any other + backend, so this costs nothing there. Streams carry the signals in + headers only and go unlogged here; the steward's raw call reads its + own body. """ - balancing = response.headers.get("x-boilerroom-balancing") - if balancing and balancing != "[]": + extra = getattr(response, "model_extra", None) or {} + balancing = extra.get("balancing") + if balancing: logger.info("backend_balancing", moves=balancing) - if response.headers.get("x-boilerroom-compaction-due") == "true": + if extra.get("compaction_due"): logger.warning( "backend_compaction_due", hint="GET /sessions on the wrapper names the session over budget", @@ -58,10 +72,12 @@ class TatlockOllamaProvider(OllamaProvider): clean_host = str(config.OLLAMA_HOST).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 poking a lookalike attribute after super().__init__ had + # built its own client is how the sanitizer sat bypassed in + # production (T-6). + super().__init__(openai_client=_SanitizedAsyncOpenAI(base_url=base_url)) logger.debug( "tatlock_ollama_provider_created", @@ -78,29 +94,49 @@ class _SanitizedAsyncOpenAI(AsyncOpenAI): # configured Ollama timeout instead of the SDK default (~600s), # so one stuck request cannot eat the whole delegation budget. kwargs.setdefault("timeout", float(config.OLLAMA_TIMEOUT)) - # The response hook reads the wrapper's balancing/compaction - # headers on every call this client makes, streams included. - kwargs.setdefault( - "http_client", - httpx.AsyncClient( - event_hooks={"response": [_log_wrapper_signals]}, - timeout=float(config.OLLAMA_TIMEOUT), - ), - ) super().__init__(api_key="ollama", **kwargs) @property - def chat(self) -> "_SanitizedChat": - """Return sanitized chat interface.""" + def chat(self) -> "_SanitizedChat": # type: ignore[override] + """Return sanitized chat interface. + + Deliberately incompatible with AsyncOpenAI.chat, which is a Chat + resource. Replacing it is the entire mechanism of this class; + typing it as the parent's Chat would describe an object this + class does not return. The suppression is on this member alone. + """ return _SanitizedChat(self) +def _parent_chat(client: AsyncOpenAI) -> Any: + """Get AsyncOpenAI's own `chat`, bypassing the subclass override. + + openai has shipped `chat` as both a property (getter `.fget`) and a + functools.cached_property (getter `.func`) — this venv's 2.11.0 is + the latter, so the old hardcoded `.fget` access raised + AttributeError the moment anything touched `.chat`. It went + unnoticed here for the same reason the whole class did: the client + was never wired (T-6). Reading whichever getter the descriptor + exposes keeps this working across both shapes, and raises something + legible if openai adopts a third. Ported from webber's 2026-08-11 + fix. + """ + descriptor = AsyncOpenAI.__dict__["chat"] + getter = getattr(descriptor, "func", None) or getattr(descriptor, "fget", None) + if getter is None: # pragma: no cover - defensive + raise TypeError( + f"AsyncOpenAI.chat is a {type(descriptor).__name__} with neither " + "'func' nor 'fget'; the sanitising wrapper needs updating" + ) + return getter(client) + + class _SanitizedChat: """Chat interface wrapper with sanitized completions.""" def __init__(self, client: _SanitizedAsyncOpenAI): self._client = client - self._original_chat = AsyncOpenAI.chat.fget(client) # type: ignore + self._original_chat = _parent_chat(client) @property def completions(self) -> "_SanitizedCompletions": @@ -124,7 +160,9 @@ class _SanitizedCompletions: if "messages" in kwargs: kwargs["messages"] = _sanitize_messages(kwargs["messages"]) - return await self._original.create(**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/tests/ollama/test_provider.py b/tests/ollama/test_provider.py index 641eb47..303259f 100644 --- a/tests/ollama/test_provider.py +++ b/tests/ollama/test_provider.py @@ -1,31 +1,73 @@ """ -Tests for TatlockOllamaProvider configuration. +Tests for TatlockOllamaProvider configuration and wiring. -The AsyncOpenAI client must carry an explicit timeout from -config.OLLAMA_TIMEOUT instead of the SDK default (~600s), so a stuck -LLM call cannot consume the whole delegation budget. +Every assertion here goes through `provider.client` — the property +pydantic_ai actually reads. The previous version asserted on +`_openai_client`, a lookalike attribute nobody read, and stayed green +for the whole period the sanitizer was bypassed in production (T-6): +a test whose subject is not the real thing cannot fail for the real +reason. """ import pytest from src.core.config import config -from src.ollama.provider import TatlockOllamaProvider, _sanitize_messages +from src.ollama import provider as provider_module +from src.ollama.provider import ( + TatlockOllamaProvider, + _parent_chat, + _sanitize_messages, + _SanitizedAsyncOpenAI, + _SanitizedCompletions, +) + + +@pytest.mark.unit +class TestClientWiring: + """The sanitized client must be the one the provider serves (T-6).""" + + def test_provider_serves_the_sanitized_client(self): + provider = TatlockOllamaProvider(base_url="http://localhost:11434/v1") + + assert isinstance(provider.client, _SanitizedAsyncOpenAI) + + def test_the_full_chain_reaches_completions(self): + # Walks provider.client -> sanitized chat -> parent lookup -> + # completions. Under openai's cached_property `chat`, the old + # hardcoded `.fget` access raised AttributeError right here. + provider = TatlockOllamaProvider(base_url="http://localhost:11434/v1") + + assert provider.client.chat.completions is not None + + def test_parent_lookup_survives_either_descriptor_shape(self): + from openai import AsyncOpenAI + + descriptor = AsyncOpenAI.__dict__["chat"] + assert hasattr(descriptor, "func") or hasattr( + descriptor, "fget" + ), "AsyncOpenAI.chat exposes neither getter; _parent_chat needs updating" + client = _SanitizedAsyncOpenAI(base_url="http://localhost:11434/v1") + parent = _parent_chat(client) + assert parent is not None + assert ( + type(parent).__name__ != "_SanitizedChat" + ), "parent lookup must not recurse into the override" @pytest.mark.unit class TestProviderTimeout: - """Timeout configuration on the underlying AsyncOpenAI client.""" + """Timeout configuration on the client the provider actually serves.""" def test_openai_client_timeout_from_config(self): provider = TatlockOllamaProvider(base_url="http://localhost:11434/v1") - assert provider._openai_client.timeout == float(config.OLLAMA_TIMEOUT) + assert provider.client.timeout == float(config.OLLAMA_TIMEOUT) def test_timeout_is_not_sdk_default(self): provider = TatlockOllamaProvider(base_url="http://localhost:11434/v1") # The OpenAI SDK defaults to 600s; the configured cap must win - assert provider._openai_client.timeout < 600 + assert provider.client.timeout < 600 @pytest.mark.unit @@ -52,3 +94,68 @@ class TestMessageSanitization: ] assert _sanitize_messages(messages) == messages + + +class _StubLogger: + def __init__(self): + self.calls = [] + + def info(self, event, **kwargs): + self.calls.append(("info", event, kwargs)) + + def warning(self, event, **kwargs): + self.calls.append(("warning", event, kwargs)) + + +class _BusyResponse: + def __init__(self): + self.model_extra = { + "balancing": [{"parked": "librarian"}], + "compaction_due": True, + } + + +class _QuietResponse: + def __init__(self): + self.model_extra = {"balancing": [], "compaction_due": False} + + +@pytest.mark.unit +class TestWrapperSignals: + """The wrapper's body signals must not pass silently (T-4/T-6). + + Read from the parsed body's model_extra: an httpx event-hook + variant demonstrably never fires under the openai SDK. + """ + + def test_busy_response_logs_balancing_and_compaction(self, monkeypatch): + stub = _StubLogger() + monkeypatch.setattr(provider_module, "logger", stub) + + provider_module._log_wrapper_signals(_BusyResponse()) + + assert any(kind == "info" and event == "backend_balancing" for kind, event, _ in stub.calls) + assert any(kind == "warning" for kind, _, _ in stub.calls) + + def test_quiet_response_logs_nothing(self, monkeypatch): + stub = _StubLogger() + monkeypatch.setattr(provider_module, "logger", stub) + + provider_module._log_wrapper_signals(_QuietResponse()) + + assert stub.calls == [] + + async def test_create_surfaces_the_signals(self, monkeypatch): + # The read must sit in the request path, not merely exist: + # removing the call from create() has to fail this test. + stub = _StubLogger() + monkeypatch.setattr(provider_module, "logger", stub) + + class FakeOriginal: + async def create(self, **kwargs): + return _BusyResponse() + + completions = _SanitizedCompletions(FakeOriginal()) + await completions.create(messages=[{"role": "user", "content": "hi"}]) + + assert any(event == "backend_balancing" for _, event, _ in stub.calls)