From 0708c759fc7a6f377808486e9133e7570280411a Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 14 Jul 2026 10:26:55 +0200 Subject: [PATCH] fix(librarian): replace nullable tool params with sentinel defaults Ollama's OpenAI-compatible API mishandles anyOf[X, null] parameter schemas. update_wiki_page (content/title/tags/description) and smart_create_wiki_page (path) now use empty-string/empty-list sentinels translated to None inside the tool, following the biographer pattern from 9d7ce39. Adds a snapshot test that walks every registered librarian tool's emitted JSON schema and fails on any anyOf[..., null]. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 1 + src/agents/librarian/tools.py | 41 +++++++++-------- tests/agents/librarian/test_tool_schemas.py | 49 +++++++++++++++++++++ 3 files changed, 72 insertions(+), 19 deletions(-) create mode 100644 tests/agents/librarian/test_tool_schemas.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0658c4c..0442070 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Ollama-safe librarian tool schemas** - `update_wiki_page` and `smart_create_wiki_page` no longer use `X | None` parameters (Ollama's OpenAI-compatible API mishandles `anyOf[X, null]`); empty-string/empty-list sentinels are translated to `None` inside the tools, matching the biographer pattern. A snapshot test pins every librarian tool schema to contain no nullable `anyOf` - **Honest expert failures** - `run_librarian`/`run_librarian_stream` now raise a structured `AgentError` instead of returning error text as if it were research output, so delegation correctly reports `success=False` and the streaming error branch is reachable. Failures surface to the user as curated butler-toned sentences; exception detail (including internal URLs) stays in the logs only. Librarian tool errors no longer leak `str(e)` into synthesis - **HybridRAG response mapping** - The librarian client now parses the field names library-desk actually returns (`source_type`/`sources`, `rrf_score`, `context`, per-item `related_dossiers`, synonyms nested in the `keywords` dict); previously every result rendered as "unknown (score: 0.00)". Source icons now key off the per-item `sources` list. Requests no longer send zero limits (the service rejects them with 422); legs are disabled via `enable_*` flags. Pinned by a contract test against a recorded live response (`tests/agents/librarian/fixtures/`) diff --git a/src/agents/librarian/tools.py b/src/agents/librarian/tools.py index 095cb36..84a9a49 100644 --- a/src/agents/librarian/tools.py +++ b/src/agents/librarian/tools.py @@ -793,10 +793,10 @@ async def read_urls_batch( async def update_wiki_page( page_id: int, - content: str | None = None, - title: str | None = None, - tags: list[str] | None = None, - description: str | None = None, + content: str = "", + title: str = "", + tags: list[str] = [], # noqa: B006 - sentinel, never mutated + description: str = "", ) -> str: """ Update an existing wiki page. @@ -810,12 +810,15 @@ async def update_wiki_page( - Updating tags to organize pages into dossiers - Fixing descriptions or titles + Note: empty values are sentinels for "leave unchanged" (Ollama's + OpenAI-compatible API mishandles anyOf[X, null] parameter schemas). + Args: page_id: ID of the page to update (get from search_wiki results) - content: New markdown content (optional - only if changing content) - title: New title (optional - only if renaming) - tags: New tag list (optional - replaces existing tags) - description: New description (optional) + content: New markdown content (empty = leave unchanged) + title: New title (empty = leave unchanged) + tags: New tag list, replaces existing tags (empty = leave unchanged) + description: New description (empty = leave unchanged) Returns: Confirmation with updated page details @@ -829,21 +832,21 @@ async def update_wiki_page( async with LibraryDeskClient() as client: page = await client.update_wiki_page( page_id=page_id, - content=content, - title=title, - tags=tags, - description=description, + content=content if content else None, + title=title if title else None, + tags=tags if tags else None, + description=description if description else None, ) # Build update summary updated_fields = [] - if content is not None: + if content: updated_fields.append("content") - if title is not None: + if title: updated_fields.append("title") - if tags is not None: + if tags: updated_fields.append("tags") - if description is not None: + if description: updated_fields.append("description") output_parts = [ @@ -948,7 +951,7 @@ async def create_wiki_page( async def smart_create_wiki_page( topic: str, tags: list[str], - path: str | None = None, + path: str = "", include_web_research: bool = True, include_wiki_search: bool = True, ) -> str: @@ -970,7 +973,7 @@ async def smart_create_wiki_page( Args: topic: The topic to research and create a page about tags: List of tags/dossiers for categorization - path: Optional custom path (auto-generated from topic if not provided) + path: Optional custom path (empty = auto-generated from topic) include_web_research: Whether to search the web (default: True) include_wiki_search: Whether to search existing wiki (default: True) @@ -986,7 +989,7 @@ async def smart_create_wiki_page( response = await client.smart_create_wiki_page( topic=topic, tags=tags, - path=path, + path=path if path else None, include_web_research=include_web_research, include_wiki_search=include_wiki_search, ) diff --git a/tests/agents/librarian/test_tool_schemas.py b/tests/agents/librarian/test_tool_schemas.py new file mode 100644 index 0000000..8e13a70 --- /dev/null +++ b/tests/agents/librarian/test_tool_schemas.py @@ -0,0 +1,49 @@ +""" +Snapshot tests for the JSON schemas emitted for librarian tools. + +Ollama's OpenAI-compatible API mishandles anyOf[X, null] parameter +schemas, so tool parameters must use empty-string/empty-list sentinels +translated to None inside the tool (same pattern as the biographer +tools, commit 9d7ce39). This test fails if a X | None parameter ever +creeps back in. +""" + +import pytest +from pydantic_ai.tools import Tool + +from src.agents.librarian.tools import LIBRARIAN_TOOLS + + +def _nullable_anyof_paths(schema: object, path: str = "") -> list[str]: + """Recursively collect JSON-schema paths that are anyOf[..., null].""" + offenders: list[str] = [] + if isinstance(schema, dict): + any_of = schema.get("anyOf") + if isinstance(any_of, list) and any( + isinstance(sub, dict) and sub.get("type") == "null" for sub in any_of + ): + offenders.append(path or "") + for key, value in schema.items(): + offenders.extend(_nullable_anyof_paths(value, f"{path}/{key}")) + elif isinstance(schema, list): + for i, item in enumerate(schema): + offenders.extend(_nullable_anyof_paths(item, f"{path}[{i}]")) + return offenders + + +@pytest.mark.unit +class TestLibrarianToolSchemas: + """All registered librarian tools emit Ollama-safe parameter schemas.""" + + @pytest.mark.parametrize( + "tool_func", LIBRARIAN_TOOLS, ids=lambda f: f.__name__ + ) + def test_no_nullable_anyof_in_schema(self, tool_func): + schema = Tool(tool_func).function_schema.json_schema + + offenders = _nullable_anyof_paths(schema) + + assert offenders == [], ( + f"{tool_func.__name__} emits anyOf[..., null] at {offenders}; " + "use empty-string/empty-list sentinels instead of X | None" + )