diff --git a/webber-api/requirements-dev.txt b/webber-api/requirements-dev.txt index 43c391e..bbf22d7 100644 --- a/webber-api/requirements-dev.txt +++ b/webber-api/requirements-dev.txt @@ -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 diff --git a/webber-api/src/domains/agents/base.py b/webber-api/src/domains/agents/base.py index 592a4fd..3e9dffc 100644 --- a/webber-api/src/domains/agents/base.py +++ b/webber-api/src/domains/agents/base.py @@ -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. diff --git a/webber-api/src/domains/agents/explore/agent.py b/webber-api/src/domains/agents/explore/agent.py index 991fc57..18669e3 100644 --- a/webber-api/src/domains/agents/explore/agent.py +++ b/webber-api/src/domains/agents/explore/agent.py @@ -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]: diff --git a/webber-api/src/domains/agents/explore/tools.py b/webber-api/src/domains/agents/explore/tools.py index 6a206c7..374d802 100644 --- a/webber-api/src/domains/agents/explore/tools.py +++ b/webber-api/src/domains/agents/explore/tools.py @@ -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 diff --git a/webber-api/src/domains/agents/plan/agent.py b/webber-api/src/domains/agents/plan/agent.py index a3d7249..4f07e08 100644 --- a/webber-api/src/domains/agents/plan/agent.py +++ b/webber-api/src/domains/agents/plan/agent.py @@ -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]: diff --git a/webber-api/src/domains/agents/plan/tools.py b/webber-api/src/domains/agents/plan/tools.py index e5fa074..49c83e2 100644 --- a/webber-api/src/domains/agents/plan/tools.py +++ b/webber-api/src/domains/agents/plan/tools.py @@ -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 diff --git a/webber-api/src/domains/agents/task/agent.py b/webber-api/src/domains/agents/task/agent.py index d234b8e..5791d77 100644 --- a/webber-api/src/domains/agents/task/agent.py +++ b/webber-api/src/domains/agents/task/agent.py @@ -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. diff --git a/webber-api/src/domains/agents/task/tools.py b/webber-api/src/domains/agents/task/tools.py index 516dc36..ece3459 100644 --- a/webber-api/src/domains/agents/task/tools.py +++ b/webber-api/src/domains/agents/task/tools.py @@ -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 diff --git a/webber-api/src/domains/tools/base.py b/webber-api/src/domains/tools/base.py index 0e513a2..4a9215c 100644 --- a/webber-api/src/domains/tools/base.py +++ b/webber-api/src/domains/tools/base.py @@ -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 """ diff --git a/webber-api/src/domains/tools/file/edit.py b/webber-api/src/domains/tools/file/edit.py index ca5a11c..e0366eb 100644 --- a/webber-api/src/domains/tools/file/edit.py +++ b/webber-api/src/domains/tools/file/edit.py @@ -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, diff --git a/webber-api/src/domains/tools/file/glob.py b/webber-api/src/domains/tools/file/glob.py index 09ad46a..f2d22e4 100644 --- a/webber-api/src/domains/tools/file/glob.py +++ b/webber-api/src/domains/tools/file/glob.py @@ -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, diff --git a/webber-api/src/domains/tools/file/read.py b/webber-api/src/domains/tools/file/read.py index 55247bd..c5a73cf 100644 --- a/webber-api/src/domains/tools/file/read.py +++ b/webber-api/src/domains/tools/file/read.py @@ -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, diff --git a/webber-api/src/domains/tools/file/write.py b/webber-api/src/domains/tools/file/write.py index bae773d..6f3fbf6 100644 --- a/webber-api/src/domains/tools/file/write.py +++ b/webber-api/src/domains/tools/file/write.py @@ -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 diff --git a/webber-api/src/domains/tools/search/grep.py b/webber-api/src/domains/tools/search/grep.py index c45494f..414cfe1 100644 --- a/webber-api/src/domains/tools/search/grep.py +++ b/webber-api/src/domains/tools/search/grep.py @@ -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, diff --git a/webber-api/src/domains/tools/search/web.py b/webber-api/src/domains/tools/search/web.py index 753b176..5ddaaaa 100644 --- a/webber-api/src/domains/tools/search/web.py +++ b/webber-api/src/domains/tools/search/web.py @@ -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, diff --git a/webber-api/src/domains/tools/shell/bash.py b/webber-api/src/domains/tools/shell/bash.py index ac73325..a90674e 100644 --- a/webber-api/src/domains/tools/shell/bash.py +++ b/webber-api/src/domains/tools/shell/bash.py @@ -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, diff --git a/webber-api/src/domains/tools/shell/bash_full.py b/webber-api/src/domains/tools/shell/bash_full.py index d3b7186..33a6995 100644 --- a/webber-api/src/domains/tools/shell/bash_full.py +++ b/webber-api/src/domains/tools/shell/bash_full.py @@ -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, diff --git a/webber-api/src/ollama/provider.py b/webber-api/src/ollama/provider.py index 4e86277..a2de4a9 100644 --- a/webber-api/src/ollama/provider.py +++ b/webber-api/src/ollama/provider.py @@ -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": diff --git a/webber-api/src/shared/logging.py b/webber-api/src/shared/logging.py index c28bf99..2e02553 100644 --- a/webber-api/src/shared/logging.py +++ b/webber-api/src/shared/logging.py @@ -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 diff --git a/webber-api/src/shared/retry.py b/webber-api/src/shared/retry.py index 481e633..0a78073 100644 --- a/webber-api/src/shared/retry.py +++ b/webber-api/src/shared/retry.py @@ -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( diff --git a/webber-api/tests/test_ollama_provider.py b/webber-api/tests/test_ollama_provider.py new file mode 100644 index 0000000..fed1908 --- /dev/null +++ b/webber-api/tests/test_ollama_provider.py @@ -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"