fix(webber-api): clear mypy, and the dead code it was covering for
55 errors to zero. Nearly all of them traced back to two causes rather than 55. THE DECORATOR. @logged wraps ~24 functions across this package and was declared `def decorator(func: Callable):` with no ParamSpec and no return annotation, so it erased the signature of everything it touched. ToolResult.execute() is annotated `-> ToolResult`; through the decorator it came back Any, and mypy reported 33 no-any-return errors spread across the tools and agents. Each looked like a local annotation slip. All of them were one decorator. Typed with ParamSpec/TypeVar; the async branch casts at the await rather than loosening R, because loosening R would put the Any straight back into every caller. THE MISSING TYPE PARAMETER. BaseAgent was not generic, so _create_agent returned a bare Agent — Agent[Any, Any] — and pydantic_ai then typed every run() result as Any. BaseAgent is now Generic[CtxT] bound to AgentContext, _agent is declared on the base instead of reached through hasattr, and the three tool-registration functions take their agent's real context type. tools_streaming.py already did this; the other three had not been updated. Eight `execute` overrides carry a targeted ignore rather than a package-wide disable_error_code. Every tool narrows the base's **kwargs to its own named parameters, which is a real LSP violation — but nothing anywhere is typed as BaseTool, and every call site constructs the concrete tool. The abstract method earns its place by making a tool without execute impossible to instantiate. The reasoning lives in BaseTool.execute's docstring; the per-site suppressions mean an override that IS unsound still gets caught. BaseAgent.run_stream widened to AsyncIterator[str | StreamEvent], which is what callers already receive: task streams structured events, explore and plan stream strings, and the router branches on isinstance with a comment calling the string path legacy. The annotation now says what the code does. AND THE PART THAT MATTERS MORE THAN THE TYPES. Chasing the last error found that the Ollama sanitiser has been broken. It fetched the parent's chat getter with `AsyncOpenAI.chat.fget`, and openai made `chat` a functools.cached_property, whose getter is `.func`. Touching `.chat` raised AttributeError — meaning the content: null workaround that CLAUDE.md documents as live would have failed on the first completion any agent attempted. Confirmed in the running container (openai 2.46.0) as well as locally (2.15.0). Two things hid it. The line carried a bare `# type: ignore`, which suppressed precisely the complaint that would have caught it. And /agents/run and /agents/stream have served zero requests in 30 days, so nothing exercised the path. A mitigation can rot completely while every check stays green, if no check actually runs it. The lookup now reads whichever getter the descriptor exposes and raises a legible TypeError if openai adopts a third shape. tests/test_ollama_provider.py walks the chain an agent request walks, short of the network call — mutation-checked: all four fail against the old lookup. 215 passed, 23 skipped, plus the four new. mypy clean over 90 files. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude
@@ -15,6 +15,9 @@ pip-audit~=2.9.0
|
||||
|
||||
# Type checking
|
||||
mypy~=1.19.1
|
||||
# Stubs for aiofiles, which ships none. Without them mypy reports
|
||||
# import-untyped on every module that reads or writes a file.
|
||||
types-aiofiles~=25.1
|
||||
|
||||
# Linting and formatting
|
||||
ruff~=0.9.4
|
||||
|
||||
@@ -6,10 +6,11 @@ All agents are built on PydanticAI and registered in a central registry.
|
||||
from abc import ABC, abstractmethod
|
||||
from collections.abc import AsyncIterator
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Any, Protocol, runtime_checkable
|
||||
from typing import Any, Generic, Protocol, TypeVar, runtime_checkable
|
||||
|
||||
from pydantic_ai import Agent
|
||||
|
||||
from src.domains.agents.schemas import StreamEvent
|
||||
from src.shared.logging import get_logger
|
||||
|
||||
logger = get_logger(__name__)
|
||||
@@ -27,6 +28,15 @@ class AgentContext:
|
||||
timeout_seconds: int = 120
|
||||
|
||||
|
||||
# Every agent narrows the context its tools receive — ExploreContext,
|
||||
# PlanContext, TaskContext. Without this parameter BaseAgent could only say
|
||||
# `Agent`, which is `Agent[Any, Any]`, and pydantic_ai then types every
|
||||
# `.run()` result as Any. That is where 35 of this package's mypy errors came
|
||||
# from: functions declared `-> str` returning Any, each looking like a local
|
||||
# annotation slip rather than one missing type parameter in the base class.
|
||||
CtxT = TypeVar("CtxT", bound=AgentContext)
|
||||
|
||||
|
||||
@runtime_checkable
|
||||
class AgentProtocol(Protocol):
|
||||
"""Protocol that all agents must implement."""
|
||||
@@ -60,7 +70,7 @@ class AgentProtocol(Protocol):
|
||||
...
|
||||
|
||||
|
||||
class BaseAgent(ABC):
|
||||
class BaseAgent(ABC, Generic[CtxT]):
|
||||
"""
|
||||
Abstract base class for agent implementations.
|
||||
|
||||
@@ -71,9 +81,10 @@ class BaseAgent(ABC):
|
||||
name = "explore"
|
||||
description = "Fast codebase exploration"
|
||||
|
||||
def _create_agent(self) -> Agent:
|
||||
# Create and configure PydanticAI agent
|
||||
...
|
||||
class ExploreAgent(BaseAgent[ExploreContext]):
|
||||
def _create_agent(self) -> Agent[ExploreContext, str]:
|
||||
# Create and configure PydanticAI agent
|
||||
...
|
||||
|
||||
async def run(self, prompt: str, **kwargs) -> str:
|
||||
# Execute agent
|
||||
@@ -92,15 +103,21 @@ class BaseAgent(ABC):
|
||||
"""Human-readable description."""
|
||||
pass
|
||||
|
||||
# Declared on the base rather than only in each subclass's __init__. The
|
||||
# base reached it through hasattr, so mypy could not determine its type at
|
||||
# all; the guard existed because nothing guaranteed the attribute existed.
|
||||
# Declaring it here makes the None check sufficient.
|
||||
_agent: "Agent[CtxT, str] | None" = None
|
||||
|
||||
@property
|
||||
def agent(self) -> Agent:
|
||||
def agent(self) -> "Agent[CtxT, str]":
|
||||
"""Lazy-loaded PydanticAI agent."""
|
||||
if not hasattr(self, '_agent') or self._agent is None:
|
||||
if self._agent is None:
|
||||
self._agent = self._create_agent()
|
||||
return self._agent
|
||||
|
||||
@abstractmethod
|
||||
def _create_agent(self) -> Agent:
|
||||
def _create_agent(self) -> "Agent[CtxT, str]":
|
||||
"""
|
||||
Create and configure the PydanticAI agent.
|
||||
|
||||
@@ -115,7 +132,7 @@ class BaseAgent(ABC):
|
||||
|
||||
async def run_stream(
|
||||
self, prompt: str, **kwargs: Any
|
||||
) -> AsyncIterator[str]:
|
||||
) -> AsyncIterator[str | StreamEvent]:
|
||||
"""
|
||||
Execute the agent with streaming output.
|
||||
|
||||
|
||||
@@ -31,7 +31,7 @@ class ExploreContext(AgentContext):
|
||||
pass
|
||||
|
||||
|
||||
class ExploreAgentImpl(BaseAgent):
|
||||
class ExploreAgentImpl(BaseAgent[ExploreContext]):
|
||||
"""
|
||||
Fast codebase exploration agent.
|
||||
|
||||
@@ -44,7 +44,7 @@ class ExploreAgentImpl(BaseAgent):
|
||||
|
||||
def __init__(self):
|
||||
"""Initialize the explore agent."""
|
||||
self._agent: Agent[ExploreContext, str] | None = None
|
||||
self._agent = None
|
||||
self._settings = get_settings()
|
||||
|
||||
def _create_agent(self) -> Agent[ExploreContext, str]:
|
||||
|
||||
@@ -5,7 +5,7 @@ Registers our tool implementations with the PydanticAI agent.
|
||||
"""
|
||||
from pydantic_ai import Agent, RunContext
|
||||
|
||||
from src.domains.agents.base import AgentContext
|
||||
from src.domains.agents.explore.agent import ExploreContext
|
||||
from src.domains.tools.file.edit import EditFileTool
|
||||
from src.domains.tools.file.glob import GlobFilesTool
|
||||
from src.domains.tools.file.read import ReadFileTool
|
||||
@@ -16,7 +16,7 @@ from src.domains.tools.shell.bash import BashReadOnlyTool
|
||||
from src.domains.tools.shell.bash_full import BashTool
|
||||
|
||||
|
||||
def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
def register_explore_tools(agent: Agent[ExploreContext, str]) -> None:
|
||||
"""
|
||||
Register all exploration tools with the agent.
|
||||
|
||||
@@ -25,7 +25,7 @@ def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def read_file(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[ExploreContext],
|
||||
file_path: str,
|
||||
offset: int = 0,
|
||||
limit: int = 2000
|
||||
@@ -52,7 +52,7 @@ def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def glob_files(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[ExploreContext],
|
||||
pattern: str,
|
||||
path: str | None = None,
|
||||
limit: int = 100
|
||||
@@ -85,7 +85,7 @@ def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def grep_content(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[ExploreContext],
|
||||
pattern: str,
|
||||
path: str | None = None,
|
||||
file_glob: str | None = None,
|
||||
@@ -125,7 +125,7 @@ def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def bash_readonly(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[ExploreContext],
|
||||
command: str,
|
||||
cwd: str | None = None,
|
||||
timeout: int = 30
|
||||
@@ -170,7 +170,7 @@ def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def edit_file(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[ExploreContext],
|
||||
file_path: str,
|
||||
old_string: str,
|
||||
new_string: str,
|
||||
@@ -204,7 +204,7 @@ def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def write_file(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[ExploreContext],
|
||||
file_path: str,
|
||||
content: str
|
||||
) -> str:
|
||||
@@ -231,7 +231,7 @@ def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def bash(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[ExploreContext],
|
||||
command: str,
|
||||
cwd: str | None = None,
|
||||
timeout: int = 60
|
||||
@@ -277,7 +277,7 @@ def register_explore_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def web_search(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[ExploreContext],
|
||||
query: str,
|
||||
num_results: int = 5,
|
||||
categories: str | None = None
|
||||
|
||||
@@ -32,7 +32,7 @@ class PlanContext(AgentContext):
|
||||
pass
|
||||
|
||||
|
||||
class PlanAgentImpl(BaseAgent):
|
||||
class PlanAgentImpl(BaseAgent[PlanContext]):
|
||||
"""
|
||||
Software architect agent for implementation planning.
|
||||
|
||||
@@ -47,7 +47,7 @@ class PlanAgentImpl(BaseAgent):
|
||||
|
||||
def __init__(self):
|
||||
"""Initialize the plan agent."""
|
||||
self._agent: Agent[PlanContext, str] | None = None
|
||||
self._agent = None
|
||||
self._settings = get_settings()
|
||||
|
||||
def _create_agent(self) -> Agent[PlanContext, str]:
|
||||
|
||||
@@ -6,14 +6,14 @@ It cannot modify files - only explore and analyze.
|
||||
"""
|
||||
from pydantic_ai import Agent, RunContext
|
||||
|
||||
from src.domains.agents.base import AgentContext
|
||||
from src.domains.agents.plan.agent import PlanContext
|
||||
from src.domains.tools.file.glob import GlobFilesTool
|
||||
from src.domains.tools.file.read import ReadFileTool
|
||||
from src.domains.tools.search.grep import GrepContentTool
|
||||
from src.domains.tools.shell.bash import BashReadOnlyTool
|
||||
|
||||
|
||||
def register_plan_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
def register_plan_tools(agent: Agent[PlanContext, str]) -> None:
|
||||
"""
|
||||
Register read-only exploration tools with the Plan agent.
|
||||
|
||||
@@ -28,7 +28,7 @@ def register_plan_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def read_file(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[PlanContext],
|
||||
file_path: str,
|
||||
offset: int = 0,
|
||||
limit: int = 2000
|
||||
@@ -55,7 +55,7 @@ def register_plan_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def glob_files(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[PlanContext],
|
||||
pattern: str,
|
||||
path: str | None = None,
|
||||
limit: int = 100
|
||||
@@ -88,7 +88,7 @@ def register_plan_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def grep_content(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[PlanContext],
|
||||
pattern: str,
|
||||
path: str | None = None,
|
||||
file_glob: str | None = None,
|
||||
@@ -128,7 +128,7 @@ def register_plan_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def bash_readonly(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[PlanContext],
|
||||
command: str,
|
||||
cwd: str | None = None,
|
||||
timeout: int = 30
|
||||
|
||||
@@ -62,7 +62,7 @@ def _summarize_result(result: str, max_len: int = 80) -> str:
|
||||
return result
|
||||
|
||||
|
||||
class TaskAgentImpl(BaseAgent):
|
||||
class TaskAgentImpl(BaseAgent[TaskContext]):
|
||||
"""
|
||||
Full orchestrator agent for autonomous task execution.
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ The Task agent has access to tools based on permission mode:
|
||||
"""
|
||||
from pydantic_ai import Agent, RunContext
|
||||
|
||||
from src.domains.agents.base import AgentContext
|
||||
from src.domains.agents.task.agent import TaskContext
|
||||
from src.domains.tools.file.edit import EditFileTool
|
||||
from src.domains.tools.file.glob import GlobFilesTool
|
||||
from src.domains.tools.file.read import ReadFileTool
|
||||
@@ -18,11 +18,11 @@ from src.domains.tools.shell.bash import BashReadOnlyTool
|
||||
from src.domains.tools.shell.bash_full import BashTool
|
||||
|
||||
|
||||
def _register_read_file(agent: Agent[AgentContext, str]) -> None:
|
||||
def _register_read_file(agent: Agent[TaskContext, str]) -> None:
|
||||
"""Register read_file tool."""
|
||||
@agent.tool
|
||||
async def read_file(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
file_path: str,
|
||||
offset: int = 0,
|
||||
limit: int = 2000
|
||||
@@ -48,11 +48,11 @@ def _register_read_file(agent: Agent[AgentContext, str]) -> None:
|
||||
return result.to_string()
|
||||
|
||||
|
||||
def _register_glob_files(agent: Agent[AgentContext, str]) -> None:
|
||||
def _register_glob_files(agent: Agent[TaskContext, str]) -> None:
|
||||
"""Register glob_files tool."""
|
||||
@agent.tool
|
||||
async def glob_files(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
pattern: str,
|
||||
path: str | None = None,
|
||||
limit: int = 100
|
||||
@@ -82,11 +82,11 @@ def _register_glob_files(agent: Agent[AgentContext, str]) -> None:
|
||||
return result.to_string()
|
||||
|
||||
|
||||
def _register_grep_content(agent: Agent[AgentContext, str]) -> None:
|
||||
def _register_grep_content(agent: Agent[TaskContext, str]) -> None:
|
||||
"""Register grep_content tool."""
|
||||
@agent.tool
|
||||
async def grep_content(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
pattern: str,
|
||||
path: str | None = None,
|
||||
file_glob: str | None = None,
|
||||
@@ -118,11 +118,11 @@ def _register_grep_content(agent: Agent[AgentContext, str]) -> None:
|
||||
return result.to_string()
|
||||
|
||||
|
||||
def _register_bash_readonly(agent: Agent[AgentContext, str]) -> None:
|
||||
def _register_bash_readonly(agent: Agent[TaskContext, str]) -> None:
|
||||
"""Register bash_readonly tool."""
|
||||
@agent.tool
|
||||
async def bash_readonly(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
command: str,
|
||||
cwd: str | None = None,
|
||||
timeout: int = 30
|
||||
@@ -156,11 +156,11 @@ def _register_bash_readonly(agent: Agent[AgentContext, str]) -> None:
|
||||
return result.to_string()
|
||||
|
||||
|
||||
def _register_spawn_agent(agent: Agent[AgentContext, str], readonly_only: bool = False) -> None:
|
||||
def _register_spawn_agent(agent: Agent[TaskContext, str], readonly_only: bool = False) -> None:
|
||||
"""Register spawn_agent tool."""
|
||||
@agent.tool
|
||||
async def spawn_agent(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
agent_type: str,
|
||||
prompt: str,
|
||||
working_dir: str | None = None
|
||||
@@ -212,7 +212,7 @@ def _register_spawn_agent(agent: Agent[AgentContext, str], readonly_only: bool =
|
||||
return f"Sub-agent error: {e}"
|
||||
|
||||
|
||||
def register_readonly_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
def register_readonly_tools(agent: Agent[TaskContext, str]) -> None:
|
||||
"""
|
||||
Register read-only tools with the agent.
|
||||
|
||||
@@ -227,7 +227,7 @@ def register_readonly_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
_register_spawn_agent(agent, readonly_only=True)
|
||||
|
||||
|
||||
def register_task_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
def register_task_tools(agent: Agent[TaskContext, str]) -> None:
|
||||
"""
|
||||
Register all tools with the Task agent.
|
||||
|
||||
@@ -247,7 +247,7 @@ def register_task_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def edit_file(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
file_path: str,
|
||||
old_string: str,
|
||||
new_string: str,
|
||||
@@ -281,7 +281,7 @@ def register_task_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def write_file(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
file_path: str,
|
||||
content: str
|
||||
) -> str:
|
||||
@@ -308,7 +308,7 @@ def register_task_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def bash(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
command: str,
|
||||
cwd: str | None = None,
|
||||
timeout: int = 60
|
||||
@@ -350,7 +350,7 @@ def register_task_tools(agent: Agent[AgentContext, str]) -> None:
|
||||
|
||||
@agent.tool
|
||||
async def web_search(
|
||||
ctx: RunContext[AgentContext],
|
||||
ctx: RunContext[TaskContext],
|
||||
query: str,
|
||||
num_results: int = 5,
|
||||
categories: str | None = None
|
||||
|
||||
@@ -87,6 +87,21 @@ class BaseTool(ABC):
|
||||
"""
|
||||
Execute the tool with given arguments.
|
||||
|
||||
Note on the `# type: ignore[override]` each implementation carries.
|
||||
Every tool narrows this to its own named parameters — read_file takes
|
||||
file_path/offset/limit, bash takes command/timeout — which mypy reports
|
||||
as an LSP violation, and strictly it is: a caller holding a BaseTool
|
||||
could call .execute(anything=1) and no implementation would accept it.
|
||||
|
||||
Nothing does. Checked: no reference anywhere in this package is typed as
|
||||
BaseTool, and every call site constructs the concrete tool and passes its
|
||||
specific arguments. What this abstract method buys is the runtime
|
||||
guarantee that a tool without an execute cannot be instantiated, and that
|
||||
is worth keeping.
|
||||
|
||||
The suppressions are per-site rather than a disable_error_code for the
|
||||
whole package, so a future override that IS unsound still gets caught.
|
||||
|
||||
Returns:
|
||||
ToolResult with success status and data or error
|
||||
"""
|
||||
|
||||
@@ -96,7 +96,7 @@ Examples:
|
||||
return "".join(diff)
|
||||
|
||||
@logged()
|
||||
async def execute(
|
||||
async def execute( # type: ignore[override] # see BaseTool.execute
|
||||
self,
|
||||
file_path: str,
|
||||
old_string: str,
|
||||
|
||||
@@ -63,7 +63,7 @@ IMPORTANT:
|
||||
self.honor_gitignore = honor_gitignore
|
||||
|
||||
@logged()
|
||||
async def execute(
|
||||
async def execute( # type: ignore[override] # see BaseTool.execute
|
||||
self,
|
||||
pattern: str,
|
||||
path: str | None = None,
|
||||
|
||||
@@ -56,7 +56,7 @@ IMPORTANT:
|
||||
self.max_line_length = max_line_length
|
||||
|
||||
@logged()
|
||||
async def execute(
|
||||
async def execute( # type: ignore[override] # see BaseTool.execute
|
||||
self,
|
||||
file_path: str,
|
||||
offset: int = 0,
|
||||
|
||||
@@ -58,7 +58,7 @@ Examples:
|
||||
self.max_content_size = max_content_size
|
||||
|
||||
@logged()
|
||||
async def execute(
|
||||
async def execute( # type: ignore[override] # see BaseTool.execute
|
||||
self,
|
||||
file_path: str,
|
||||
content: str
|
||||
|
||||
@@ -70,7 +70,7 @@ IMPORTANT:
|
||||
self.honor_gitignore = honor_gitignore
|
||||
|
||||
@logged()
|
||||
async def execute(
|
||||
async def execute( # type: ignore[override] # see BaseTool.execute
|
||||
self,
|
||||
pattern: str,
|
||||
path: str | None = None,
|
||||
|
||||
@@ -3,6 +3,7 @@ Web search tool using SearXNG.
|
||||
"""
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime
|
||||
from typing import Any
|
||||
|
||||
import httpx
|
||||
|
||||
@@ -91,10 +92,13 @@ IMPORTANT:
|
||||
params=params,
|
||||
)
|
||||
response.raise_for_status()
|
||||
return response.json()
|
||||
# httpx types .json() as Any. Naming the shape here keeps the Any from
|
||||
# travelling into every caller of this method.
|
||||
payload: dict[Any, Any] = response.json()
|
||||
return payload
|
||||
|
||||
@logged()
|
||||
async def execute(
|
||||
async def execute( # type: ignore[override] # see BaseTool.execute
|
||||
self,
|
||||
query: str,
|
||||
num_results: int = 5,
|
||||
|
||||
@@ -115,7 +115,7 @@ Examples:
|
||||
self.max_output_size = max_output_size
|
||||
|
||||
@logged()
|
||||
async def execute(
|
||||
async def execute( # type: ignore[override] # see BaseTool.execute
|
||||
self,
|
||||
command: str,
|
||||
cwd: str | None = None,
|
||||
|
||||
@@ -153,7 +153,7 @@ Examples:
|
||||
self.max_output_size = max_output_size
|
||||
|
||||
@logged()
|
||||
async def execute(
|
||||
async def execute( # type: ignore[override] # see BaseTool.execute
|
||||
self,
|
||||
command: str,
|
||||
cwd: str | None = None,
|
||||
|
||||
@@ -54,17 +54,54 @@ class _SanitizedAsyncOpenAI(AsyncOpenAI):
|
||||
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: Ollama
|
||||
rejects assistant messages carrying content: null alongside tool_calls,
|
||||
so every completion has to pass through the sanitiser. Typing it as the
|
||||
parent's Chat would describe an object this class does not return.
|
||||
|
||||
The suppression is on this member alone; the rest of the client keeps
|
||||
its inherited types.
|
||||
"""
|
||||
return _SanitizedChat(self)
|
||||
|
||||
|
||||
def _parent_chat(client: AsyncOpenAI) -> Any:
|
||||
"""Get AsyncOpenAI's own `chat`, bypassing the subclass override.
|
||||
|
||||
This read the descriptor's `.fget` until 2026-08-11, which is the property
|
||||
API. openai made `chat` a functools.cached_property, whose getter is `.func`,
|
||||
so the call raised AttributeError the moment anything touched `.chat` — that
|
||||
is, on the first completion any agent tried to make. Verified broken in the
|
||||
running container on openai 2.46.0 as well as locally on 2.15.0.
|
||||
|
||||
It went unnoticed because the endpoints that reach it had served no requests
|
||||
in 30 days, and because the line carried a bare `# type: ignore` that
|
||||
suppressed exactly the complaint that would have flagged it.
|
||||
|
||||
Reading whichever getter the descriptor actually exposes keeps this working
|
||||
across that change and the reverse of it, and raises something legible if
|
||||
openai adopts a third shape.
|
||||
"""
|
||||
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":
|
||||
|
||||
@@ -16,6 +16,7 @@ from collections.abc import Callable
|
||||
from contextvars import ContextVar, Token
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
from typing import Any, ParamSpec, TypeVar, cast
|
||||
from uuid import uuid4
|
||||
|
||||
# === Trace Context ===
|
||||
@@ -80,12 +81,21 @@ def get_logger(name: str) -> logging.Logger:
|
||||
|
||||
# === Decorator ===
|
||||
|
||||
# @logged wraps ~24 functions across this package. Untyped, its decorator
|
||||
# erased every one of their signatures, so mypy saw `Any` coming back from
|
||||
# annotated functions like `ToolResult.execute() -> ToolResult`. That surfaced
|
||||
# as 33 no-any-return errors scattered across the tools and agents — each
|
||||
# reading like a local annotation slip, all of them this one decorator.
|
||||
P = ParamSpec("P")
|
||||
R = TypeVar("R")
|
||||
|
||||
|
||||
def logged(
|
||||
logger: logging.Logger | None = None,
|
||||
slow_threshold_ms: float = 100.0,
|
||||
warn_threshold_ms: float = 500.0,
|
||||
include_args: bool = False,
|
||||
):
|
||||
) -> Callable[[Callable[P, R]], Callable[P, R]]:
|
||||
"""
|
||||
Decorator for automatic function logging with temporal benchmarking.
|
||||
|
||||
@@ -102,7 +112,7 @@ def logged(
|
||||
@logged(slow_threshold_ms=50, warn_threshold_ms=200)
|
||||
def critical_path(): ...
|
||||
"""
|
||||
def decorator(func: Callable):
|
||||
def decorator(func: Callable[P, R]) -> Callable[P, R]:
|
||||
nonlocal logger
|
||||
if logger is None:
|
||||
logger = logging.getLogger(func.__module__)
|
||||
@@ -140,7 +150,7 @@ def logged(
|
||||
logger.debug(f"[{tid}] {func_name} completed in {duration:.2f}ms")
|
||||
|
||||
@functools.wraps(func)
|
||||
async def async_wrapper(*args, **kwargs):
|
||||
async def async_wrapper(*args: P.args, **kwargs: P.kwargs) -> Any:
|
||||
span = _create_span()
|
||||
token = _current_span.set(span)
|
||||
|
||||
@@ -150,7 +160,7 @@ def logged(
|
||||
logger.debug(f"[{span.trace_id[:8]}] -> {func_name}")
|
||||
|
||||
try:
|
||||
result = await func(*args, **kwargs)
|
||||
result = await cast(Any, func(*args, **kwargs))
|
||||
_log_completion(span)
|
||||
return result
|
||||
except Exception as e:
|
||||
@@ -160,7 +170,7 @@ def logged(
|
||||
_current_span.reset(token)
|
||||
|
||||
@functools.wraps(func)
|
||||
def sync_wrapper(*args, **kwargs):
|
||||
def sync_wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
|
||||
span = _create_span()
|
||||
token = _current_span.set(span)
|
||||
|
||||
@@ -179,7 +189,11 @@ def logged(
|
||||
finally:
|
||||
_current_span.reset(token)
|
||||
|
||||
return async_wrapper if asyncio.iscoroutinefunction(func) else sync_wrapper
|
||||
# The branch is chosen at decoration time; mypy cannot narrow R to a
|
||||
# coroutine on the strength of iscoroutinefunction, so the union is
|
||||
# asserted here once instead of at every call site.
|
||||
chosen = async_wrapper if asyncio.iscoroutinefunction(func) else sync_wrapper
|
||||
return cast(Callable[P, R], chosen)
|
||||
return decorator
|
||||
|
||||
|
||||
|
||||
@@ -79,7 +79,7 @@ def calculate_backoff(
|
||||
Delay in seconds
|
||||
"""
|
||||
# Exponential backoff: base_delay * 2^attempt
|
||||
delay = min(base_delay * (2 ** attempt), max_delay)
|
||||
delay: float = min(base_delay * (2 ** attempt), max_delay)
|
||||
|
||||
if jitter:
|
||||
# Add up to 25% random jitter
|
||||
@@ -114,6 +114,11 @@ def with_retry(
|
||||
return response.json()
|
||||
"""
|
||||
extra_exceptions = retryable_exceptions or ()
|
||||
# Bound to a named, typed tuple: mypy cannot verify that a star-unpacked
|
||||
# tuple in an `except` clause holds exception classes, and reports it as
|
||||
# "exception type must be derived from BaseException" — which reads like a
|
||||
# real defect rather than an inference limit.
|
||||
retry_on: tuple[type[Exception], ...] = (*RETRYABLE_EXCEPTIONS, *extra_exceptions)
|
||||
|
||||
def decorator(func: Callable[..., Awaitable[T]]) -> Callable[..., Awaitable[T]]:
|
||||
@wraps(func)
|
||||
@@ -124,7 +129,7 @@ def with_retry(
|
||||
try:
|
||||
return await func(*args, **kwargs)
|
||||
|
||||
except (*RETRYABLE_EXCEPTIONS, *extra_exceptions) as e:
|
||||
except retry_on as e:
|
||||
last_exception = e
|
||||
should_retry = True
|
||||
|
||||
@@ -145,7 +150,7 @@ def with_retry(
|
||||
await asyncio.sleep(delay)
|
||||
elif not should_retry:
|
||||
# Non-retryable HTTP error
|
||||
raise last_exception # type: ignore
|
||||
raise last_exception
|
||||
|
||||
# All retries exhausted
|
||||
logger.error(
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
"""The sanitising Ollama client must actually be reachable.
|
||||
|
||||
src/ollama/provider.py exists to work around Ollama rejecting assistant
|
||||
messages that carry `content: null` alongside `tool_calls`. On 2026-08-11 it
|
||||
raised AttributeError the moment anything touched `.chat`: it fetched the
|
||||
parent's getter via `AsyncOpenAI.chat.fget`, and openai had made `chat` a
|
||||
functools.cached_property, whose getter is `.func`.
|
||||
|
||||
Nothing caught it. The line carried a bare `# type: ignore`, so mypy stayed
|
||||
quiet, and the endpoints that reach this code had served no requests in 30 days,
|
||||
so no user hit it either. The mitigation was dead and everything looked fine.
|
||||
|
||||
These tests exercise the path rather than the types, because the failure was a
|
||||
runtime attribute lookup that no annotation would have caught.
|
||||
"""
|
||||
from openai import AsyncOpenAI
|
||||
|
||||
from src.ollama.provider import _parent_chat, _SanitizedAsyncOpenAI, get_ollama_provider
|
||||
|
||||
|
||||
class TestSanitizedClientIsReachable:
|
||||
|
||||
def test_chat_can_be_accessed(self):
|
||||
"""The regression: this raised AttributeError, not a type error."""
|
||||
client = _SanitizedAsyncOpenAI(base_url="http://localhost:11434/v1")
|
||||
chat = client.chat
|
||||
assert chat is not None
|
||||
assert chat.completions is not None
|
||||
|
||||
def test_provider_reaches_completions(self):
|
||||
"""The full chain an agent request walks, short of the network call."""
|
||||
provider = get_ollama_provider()
|
||||
assert provider._openai_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`.
|
||||
|
||||
Whichever it is, the parent's own getter must be found — the previous
|
||||
code hardcoded `.fget` and broke on the switch to cached_property.
|
||||
"""
|
||||
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")
|
||||
assert _parent_chat(client) is not None
|
||||
|
||||
def test_parent_chat_is_not_the_override(self):
|
||||
"""It must return openai's Chat, not recurse into the subclass property.
|
||||
|
||||
Returning the subclass's own `chat` would be infinite recursion, and the
|
||||
sanitiser would wrap itself instead of the real completions resource.
|
||||
"""
|
||||
client = _SanitizedAsyncOpenAI(base_url="http://localhost:11434/v1")
|
||||
assert type(_parent_chat(client)).__name__ != "_SanitizedChat"
|
||||
Reference in New Issue
Block a user