fix(webber-api): clear ruff, and two things it was pointing at
97 findings to zero. Most were mechanical — 52 unsorted import blocks, 10 unsorted __all__, assorted pyupgrade and simplify hints. Two were not, and both were visible only because the lint made me look. `webber version` did not exist. src/cli/commands/version.py defines show_version(), main.py imported it, and the registration line was never written — the CLI exposed chat and explore only. The import carried `# noqa: F401`, which is what kept the omission quiet: someone marked the symptom as intentional instead of asking why it was unused. show_version is not redundant with the --version flag; it prints the resolved Ollama URL, model and debug state, which is the form worth having when something is misconfigured. Registered, and the suppression dropped because the import is now genuinely used. test_spawn_explore_agent asserted nothing. It built a mock RunContext, patched get_agent, and stopped at the comment "For now, verify the explore agent would be called correctly". It had been counted as a passing test. An AST sweep of all 238 test functions found it was the only one, which is worth knowing — the problem was contained, not systemic. It is now skipped with a reason, so it reports as unfinished rather than as passing. Reducing it rather than deleting its imports was the point: tidying the imports would have made a hollow test look clean. Two findings were false positives, and both are recorded rather than silently worked around: B023 flagged run_agent closing over full_prompt and ctx. Traced: agent_task is awaited at line 326 before `continue` reaches the next iteration, so neither name can be rebound while the closure is pending, and the exception path cancels and awaits too. Not a bug. Bound as defaults anyway, because that stays true if the await ever moves. I had called it a live bug before tracing it, which is the mistake Rule 5 exists for. RUF012 flagged `rules: list[ApprovalRule] = []` on ApprovalRuleSet. Its suggested fix — annotate ClassVar — would remove the field from the model. ApprovalRuleSet is a pydantic model and pydantic deep-copies defaults per instance; verified by constructing two and confirming their lists are distinct objects. Suppressed with that evidence in the comment. Ruff cannot see the pydantic base because BaseSchema is a local subclass of BaseModel. Also moved a stray `from src.shared.logging import ...` that had drifted below a function definition, and merged a nested if in the ollama provider. 215 passed, 23 skipped, unchanged except for the new skip. `webber version` exercised end to end. mypy is NOT addressed here and the gate still fails on it — 55 errors in 14 files, 35 of them no-any-return from pydantic_ai's untyped returns. That was hidden behind ruff, because the gate stops at the first failing stage. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -21,7 +21,6 @@ from httpx import ASGITransport, AsyncClient
|
||||
|
||||
from src.main import app
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Command Line Options
|
||||
# =============================================================================
|
||||
|
||||
@@ -4,8 +4,9 @@ Tests for agent REST API endpoints.
|
||||
Includes integration tests that verify real code paths work correctly
|
||||
without over-mocking (only LLM calls are mocked).
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
|
||||
from src.domains.agents.schemas import PermissionMode
|
||||
|
||||
@@ -312,9 +313,10 @@ class TestAgentMethodSignatures:
|
||||
|
||||
def test_task_agent_run_accepts_mode(self):
|
||||
"""Verify TaskAgentImpl.run() accepts mode parameter."""
|
||||
from src.domains.agents.task.agent import TaskAgentImpl
|
||||
import inspect
|
||||
|
||||
from src.domains.agents.task.agent import TaskAgentImpl
|
||||
|
||||
sig = inspect.signature(TaskAgentImpl.run)
|
||||
params = list(sig.parameters.keys())
|
||||
|
||||
@@ -325,9 +327,10 @@ class TestAgentMethodSignatures:
|
||||
|
||||
def test_task_agent_run_stream_accepts_mode(self):
|
||||
"""Verify TaskAgentImpl.run_stream() accepts mode parameter."""
|
||||
from src.domains.agents.task.agent import TaskAgentImpl
|
||||
import inspect
|
||||
|
||||
from src.domains.agents.task.agent import TaskAgentImpl
|
||||
|
||||
sig = inspect.signature(TaskAgentImpl.run_stream)
|
||||
params = list(sig.parameters.keys())
|
||||
|
||||
@@ -337,9 +340,10 @@ class TestAgentMethodSignatures:
|
||||
|
||||
def test_trace_span_signature(self):
|
||||
"""Verify trace_span only accepts expected parameters."""
|
||||
from src.shared.logging import trace_span
|
||||
import inspect
|
||||
|
||||
from src.shared.logging import trace_span
|
||||
|
||||
sig = inspect.signature(trace_span.__init__)
|
||||
params = list(sig.parameters.keys())
|
||||
|
||||
|
||||
@@ -3,15 +3,14 @@ Tests for conversations domain.
|
||||
|
||||
Tests conversation CRUD, context building, and API endpoints.
|
||||
"""
|
||||
import pytest
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
|
||||
from src.domains.conversations.models import Conversation, Message
|
||||
from src.domains.conversations.schemas import (
|
||||
CreateConversationRequest,
|
||||
AddMessageRequest,
|
||||
ConversationResponse,
|
||||
MessageResponse,
|
||||
CreateConversationRequest,
|
||||
)
|
||||
|
||||
|
||||
@@ -173,9 +172,10 @@ class TestConversationService:
|
||||
@pytest.mark.anyio
|
||||
async def test_context_prompt_no_history(self):
|
||||
"""Test building context prompt with no history."""
|
||||
from src.domains.conversations.service import ConversationService
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from src.domains.conversations.service import ConversationService
|
||||
|
||||
# Create mock session
|
||||
mock_session = MagicMock()
|
||||
service = ConversationService(mock_session)
|
||||
@@ -190,10 +190,11 @@ class TestConversationService:
|
||||
@pytest.mark.anyio
|
||||
async def test_context_prompt_with_history(self):
|
||||
"""Test building context prompt with message history."""
|
||||
from src.domains.conversations.service import ConversationService
|
||||
from src.domains.conversations.models import Message
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from src.domains.conversations.models import Message
|
||||
from src.domains.conversations.service import ConversationService
|
||||
|
||||
mock_session = MagicMock()
|
||||
service = ConversationService(mock_session)
|
||||
|
||||
@@ -221,10 +222,11 @@ class TestConversationService:
|
||||
@pytest.mark.anyio
|
||||
async def test_context_prompt_with_summary(self):
|
||||
"""Test building context prompt with summary message."""
|
||||
from src.domains.conversations.service import ConversationService
|
||||
from src.domains.conversations.models import Message
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from src.domains.conversations.models import Message
|
||||
from src.domains.conversations.service import ConversationService
|
||||
|
||||
mock_session = MagicMock()
|
||||
service = ConversationService(mock_session)
|
||||
|
||||
@@ -253,8 +255,8 @@ class TestSummarization:
|
||||
|
||||
def test_format_messages_for_summary(self):
|
||||
"""Test formatting messages for summarization."""
|
||||
from src.domains.conversations.summarize import format_messages_for_summary
|
||||
from src.domains.conversations.models import Message
|
||||
from src.domains.conversations.summarize import format_messages_for_summary
|
||||
|
||||
messages = [
|
||||
Message(
|
||||
@@ -276,8 +278,8 @@ class TestSummarization:
|
||||
|
||||
def test_format_messages_with_summary(self):
|
||||
"""Test formatting messages that include a summary."""
|
||||
from src.domains.conversations.summarize import format_messages_for_summary
|
||||
from src.domains.conversations.models import Message
|
||||
from src.domains.conversations.summarize import format_messages_for_summary
|
||||
|
||||
messages = [
|
||||
Message(
|
||||
|
||||
@@ -6,8 +6,8 @@ from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from src.domains.tools.gitignore import GitignoreFilter, filter_gitignored
|
||||
from src.domains.tools.file.glob import GlobFilesTool
|
||||
from src.domains.tools.gitignore import GitignoreFilter, filter_gitignored
|
||||
from src.domains.tools.search.grep import GrepContentTool
|
||||
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ Tests registration, API endpoints, and tool restrictions.
|
||||
import pytest
|
||||
|
||||
from src.domains.agents.base import get_agent, list_agents
|
||||
from src.domains.agents.plan import plan_agent, PlanAgentImpl
|
||||
from src.domains.agents.plan import PlanAgentImpl, plan_agent
|
||||
|
||||
|
||||
class TestPlanAgentRegistration:
|
||||
|
||||
@@ -1,17 +1,16 @@
|
||||
"""
|
||||
Tests for retry utilities.
|
||||
"""
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from src.shared.retry import (
|
||||
with_retry,
|
||||
retry_async,
|
||||
calculate_backoff,
|
||||
is_retryable_exception,
|
||||
is_retryable_http_status,
|
||||
calculate_backoff,
|
||||
retry_async,
|
||||
with_retry,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -6,10 +6,10 @@ from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from src.domains.tools.file.read import ReadFileTool
|
||||
from src.domains.tools.file.write import WriteFileTool
|
||||
from src.domains.tools.file.edit import EditFileTool
|
||||
from src.domains.tools.file.glob import GlobFilesTool
|
||||
from src.domains.tools.file.read import ReadFileTool
|
||||
from src.domains.tools.file.write import WriteFileTool
|
||||
from src.domains.tools.shell.bash_full import BashTool
|
||||
|
||||
|
||||
@@ -275,5 +275,5 @@ class TestResourceLimits:
|
||||
|
||||
assert result.success
|
||||
# Should only return 5 files
|
||||
lines = [l for l in result.data.strip().split("\n") if l]
|
||||
lines = [line for line in result.data.strip().split("\n") if line]
|
||||
assert len(lines) <= 5
|
||||
|
||||
@@ -3,11 +3,11 @@ Tests for the Task agent.
|
||||
|
||||
Tests registration, API endpoints, tool access, and spawn_agent functionality.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
from src.domains.agents.base import get_agent, list_agents
|
||||
from src.domains.agents.task import task_agent, TaskAgentImpl
|
||||
from src.domains.agents.task import TaskAgentImpl, task_agent
|
||||
|
||||
|
||||
class TestTaskAgentRegistration:
|
||||
@@ -90,39 +90,28 @@ class TestTaskAgentTools:
|
||||
class TestSpawnAgentTool:
|
||||
"""Tests for spawn_agent orchestration functionality."""
|
||||
|
||||
@pytest.mark.skip(
|
||||
reason="never finished — the body built a mock context and then asserted "
|
||||
"nothing, so it counted as a passing test while verifying nothing"
|
||||
)
|
||||
@pytest.mark.anyio
|
||||
async def test_spawn_explore_agent(self):
|
||||
"""Test spawning an explore agent."""
|
||||
from src.domains.agents.task.tools import register_task_tools
|
||||
from src.domains.agents.base import AgentContext
|
||||
from pydantic_ai import Agent, RunContext
|
||||
from unittest.mock import MagicMock
|
||||
"""Spawning an explore agent should delegate to the explore agent.
|
||||
|
||||
# Create a mock context
|
||||
ctx = MagicMock(spec=RunContext)
|
||||
ctx.deps = AgentContext(
|
||||
working_dir="/tmp",
|
||||
allowed_paths=["/tmp"],
|
||||
timeout_seconds=30
|
||||
)
|
||||
The scaffolding that used to sit here — a MagicMock RunContext, an
|
||||
AgentContext with a /tmp working dir, and a patch of
|
||||
src.domains.agents.base.get_agent — ran and then stopped at the comment
|
||||
"For now, verify the explore agent would be called correctly". There was
|
||||
no assertion, so it passed unconditionally.
|
||||
|
||||
# Mock the explore agent
|
||||
with patch("src.domains.agents.base.get_agent") as mock_get_agent:
|
||||
mock_explore = AsyncMock()
|
||||
mock_explore.run = AsyncMock(return_value="Found 5 Python files")
|
||||
mock_get_agent.return_value = mock_explore
|
||||
|
||||
# Import and call spawn_agent directly
|
||||
from src.domains.agents.task import tools
|
||||
# We need to test the actual tool function
|
||||
# For now, verify the explore agent would be called correctly
|
||||
Removed rather than tidied: ruff flagged its imports as unused, and
|
||||
deleting those would have made the test look clean while leaving it
|
||||
hollow. git history has the setup for whoever finishes this.
|
||||
"""
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_spawn_unknown_agent_returns_error(self):
|
||||
"""Test that spawning unknown agent type returns error."""
|
||||
from src.domains.agents.base import AgentContext
|
||||
from unittest.mock import MagicMock
|
||||
from pydantic_ai import RunContext
|
||||
|
||||
# We can't easily test the tool directly, but we can verify
|
||||
# the agent type validation logic
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
"""
|
||||
Tests for token counting utilities.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from src.shared.tokens import count_tokens, count_message_tokens, estimate_tokens
|
||||
from src.shared.tokens import count_message_tokens, count_tokens, estimate_tokens
|
||||
|
||||
|
||||
class TestTokenCounting:
|
||||
|
||||
@@ -6,8 +6,8 @@ from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from src.domains.tools.file.read import ReadFileTool
|
||||
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
|
||||
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
"""
|
||||
Tests for WebSearchTool.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
|
||||
from src.domains.tools.search.web import WebSearchTool
|
||||
|
||||
|
||||
Reference in New Issue
Block a user