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 <noreply@anthropic.com>
This commit is contained in:
@@ -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/`)
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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 "<root>")
|
||||
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"
|
||||
)
|
||||
Reference in New Issue
Block a user