From 334d313d17010a73d72fe1414cad792d61b61a43 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Thu, 5 Feb 2026 20:15:59 +0100 Subject: [PATCH] fix: repair broken tests and ensure Claude backend is used in integration tests - Remove references to unimplemented get_benchmark_store from steward and tool tracking tests - Fix steward test fixture calling async initialize_application synchronously by using sync register_household_members instead - Rewrite tool tracking tests to assert actual logging behavior - Change unit test fixture model from Tatlock to lorem-tester so unit tests don't require external services - Add session-scoped _initialize_app fixture to run Claude health check, ensuring integration tests use Claude instead of falling back to Ollama - Increase integration test timeouts from 30s to 120s to match OLLAMA_TIMEOUT - Add Steward reasoning as ReasoningOutputItem in create_response_with_steward so tags appear in chat completion responses - Add test_tatlock_ollama_fallback to verify Ollama fallback path works Co-Authored-By: Claude Opus 4.6 --- src/responses/service.py | 8 ++ tests/agents/steward/test_steward_service.py | 93 ++++++++------------ tests/agents/test_tatlock_agent.py | 72 ++++++++++++--- tests/conftest.py | 19 +++- tests/core/test_tool_tracking.py | 42 +++------ 5 files changed, 139 insertions(+), 95 deletions(-) diff --git a/src/responses/service.py b/src/responses/service.py index c37acb9..8a44940 100644 --- a/src/responses/service.py +++ b/src/responses/service.py @@ -651,6 +651,14 @@ async def create_response_with_steward(request: ResponseRequest) -> Response: # Build response output items output_items = [] + # Add Steward reasoning as reasoning output + if enriched.steward_reasoning: + output_items.append(ReasoningOutputItem( + id=f"rs_{generate_id()}", + summary=[enriched.steward_reasoning], + status="completed" + )) + # Add Tatlock's message output_items.append(MessageOutputItem( id=f"msg_{generate_id()}", diff --git a/tests/agents/steward/test_steward_service.py b/tests/agents/steward/test_steward_service.py index 59f5e25..5cec948 100644 --- a/tests/agents/steward/test_steward_service.py +++ b/tests/agents/steward/test_steward_service.py @@ -9,13 +9,13 @@ import pytest from src.agents.steward.schemas import ConversationContext, StewardRecommendation from src.agents.steward.service import analyze_request, format_steward_note, _build_enriched_query -from src.core.startup import initialize_application +from src.core.startup import register_household_members @pytest.fixture(scope="module", autouse=True) def setup_household_registry(): """Initialize household registry before running tests.""" - initialize_application() + register_household_members() class TestAnalyzeRequest: @@ -29,17 +29,14 @@ class TestAnalyzeRequest: mock_agent.analyze = AsyncMock(return_value="Simple greeting requires no tools. This is a simple request.") with patch("src.agents.steward.service.get_steward_agent", return_value=mock_agent): - with patch("src.agents.steward.service.get_benchmark_store") as mock_store: - mock_store.return_value.record = AsyncMock() + result = await analyze_request( + "Hello!", + conversation_history=[], + ) - result = await analyze_request( - "Hello!", - conversation_history=[], - ) - - assert result.recommended_capabilities == [] - assert result.estimated_complexity == "simple" - assert mock_agent.analyze.called + assert result.recommended_capabilities == [] + assert result.estimated_complexity == "simple" + assert mock_agent.analyze.called @pytest.mark.asyncio async def test_analyze_math_request(self): @@ -50,16 +47,13 @@ class TestAnalyzeRequest: ) with patch("src.agents.steward.service.get_steward_agent", return_value=mock_agent): - with patch("src.agents.steward.service.get_benchmark_store") as mock_store: - mock_store.return_value.record = AsyncMock() + result = await analyze_request( + "What's sqrt(144)?", + conversation_history=[], + ) - result = await analyze_request( - "What's sqrt(144)?", - conversation_history=[], - ) - - assert "tatlock_core" in result.recommended_capabilities - assert result.estimated_complexity == "simple" + assert "tatlock_core" in result.recommended_capabilities + assert result.estimated_complexity == "simple" @pytest.mark.asyncio async def test_analyze_with_conversation_history(self): @@ -75,21 +69,18 @@ class TestAnalyzeRequest: ] with patch("src.agents.steward.service.get_steward_agent", return_value=mock_agent): - with patch("src.agents.steward.service.get_benchmark_store") as mock_store: - mock_store.return_value.record = AsyncMock() + result = await analyze_request( + "And what's that times 5?", + conversation_history=conversation_history, + ) - result = await analyze_request( - "And what's that times 5?", - conversation_history=conversation_history, - ) + assert result.conversation_context.has_previous_context is True + assert 0 in result.conversation_context.relevant_turns - assert result.conversation_context.has_previous_context is True - assert 0 in result.conversation_context.relevant_turns - - # Verify conversation history was passed - call_kwargs = mock_agent.analyze.call_args.kwargs - assert "conversation_history" in call_kwargs - assert len(call_kwargs["conversation_history"]) == 2 + # Verify conversation history was passed + call_kwargs = mock_agent.analyze.call_args.kwargs + assert "conversation_history" in call_kwargs + assert len(call_kwargs["conversation_history"]) == 2 @pytest.mark.asyncio async def test_analyze_with_missing_capabilities(self): @@ -100,16 +91,13 @@ class TestAnalyzeRequest: ) with patch("src.agents.steward.service.get_steward_agent", return_value=mock_agent): - with patch("src.agents.steward.service.get_benchmark_store") as mock_store: - mock_store.return_value.record = AsyncMock() + result = await analyze_request( + "Generate an image of a sunset", + conversation_history=[], + ) - result = await analyze_request( - "Generate an image of a sunset", - conversation_history=[], - ) - - assert result.missing_capabilities is not None - assert "not available" in result.missing_capabilities + assert result.missing_capabilities is not None + assert "not available" in result.missing_capabilities @pytest.mark.asyncio async def test_analyze_with_conversation_id(self): @@ -120,18 +108,15 @@ class TestAnalyzeRequest: ) with patch("src.agents.steward.service.get_steward_agent", return_value=mock_agent): - with patch("src.agents.steward.service.get_benchmark_store") as mock_store: - mock_store.return_value.record = AsyncMock() + result = await analyze_request( + "Test request", + conversation_history=[], + conversation_id="test_conv_123", + ) - result = await analyze_request( - "Test request", - conversation_history=[], - conversation_id="test_conv_123", - ) - - # Verify analysis completed successfully - assert result.recommended_capabilities == ["tatlock_core"] - assert result.estimated_complexity == "simple" + # Verify analysis completed successfully + assert result.recommended_capabilities == ["tatlock_core"] + assert result.estimated_complexity == "simple" @pytest.mark.asyncio async def test_analyze_handles_errors(self): diff --git a/tests/agents/test_tatlock_agent.py b/tests/agents/test_tatlock_agent.py index 4999e61..08ef498 100644 --- a/tests/agents/test_tatlock_agent.py +++ b/tests/agents/test_tatlock_agent.py @@ -34,7 +34,7 @@ async def test_tatlock_conversation_history_memory(async_client: AsyncClient): response_1 = await async_client.post( "/v1/chat/completions", json=request_data_1, - timeout=30.0 + timeout=120.0 ) assert response_1.status_code == 200 @@ -56,7 +56,7 @@ async def test_tatlock_conversation_history_memory(async_client: AsyncClient): response_2 = await async_client.post( "/v1/chat/completions", json=request_data_2, - timeout=30.0 + timeout=120.0 ) assert response_2.status_code == 200 @@ -95,7 +95,7 @@ async def test_tatlock_multi_turn_context(async_client: AsyncClient): response_1 = await async_client.post( "/v1/chat/completions", json=request_1, - timeout=30.0 + timeout=120.0 ) assert response_1.status_code == 200 @@ -117,7 +117,7 @@ async def test_tatlock_multi_turn_context(async_client: AsyncClient): response_2 = await async_client.post( "/v1/chat/completions", json=request_2, - timeout=30.0 + timeout=120.0 ) assert response_2.status_code == 200 @@ -150,7 +150,7 @@ async def test_tatlock_tool_call_logging_search(async_client: AsyncClient): response = await async_client.post( "/v1/chat/completions", json=request_data, - timeout=60.0 + timeout=120.0 ) assert response.status_code == 200 @@ -192,7 +192,7 @@ async def test_tatlock_tool_call_logging_calculator(async_client: AsyncClient): response = await async_client.post( "/v1/chat/completions", json=request_data, - timeout=30.0 + timeout=120.0 ) assert response.status_code == 200 @@ -243,7 +243,7 @@ async def test_tatlock_tool_call_logging_datetime(async_client: AsyncClient): response = await async_client.post( "/v1/chat/completions", json=request_data, - timeout=30.0 + timeout=120.0 ) assert response.status_code == 200 @@ -293,7 +293,7 @@ async def test_tatlock_no_tool_calls_no_logging(async_client: AsyncClient): response = await async_client.post( "/v1/chat/completions", json=request_data, - timeout=30.0 + timeout=120.0 ) assert response.status_code == 200 @@ -336,7 +336,7 @@ async def test_tatlock_conversation_history_with_tools(async_client: AsyncClient response_1 = await async_client.post( "/v1/chat/completions", json=request_1, - timeout=30.0 + timeout=120.0 ) assert response_1.status_code == 200 @@ -362,7 +362,7 @@ async def test_tatlock_conversation_history_with_tools(async_client: AsyncClient response_2 = await async_client.post( "/v1/chat/completions", json=request_2, - timeout=30.0 + timeout=120.0 ) assert response_2.status_code == 200 @@ -378,3 +378,55 @@ async def test_tatlock_conversation_history_with_tools(async_client: AsyncClient ) if not has_calculation: pytest.xfail(f"LLM did not remember calculation (non-deterministic): {second_response[:200]}") + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_tatlock_ollama_fallback(async_client: AsyncClient): + """ + Test that Tatlock falls back to Ollama when Claude is unavailable. + + Patches _claude_available to False to force the Ollama path, + then verifies the system still produces a valid response. + """ + import src.anthropic.model_selector as model_selector + + # Save original value + original = model_selector._claude_available + + try: + # Force Ollama fallback + model_selector._claude_available = False + + # Verify we're actually using Ollama + info = model_selector.get_model_info() + assert info["backend"] == "ollama", f"Expected ollama backend, got {info['backend']}" + + request_data = { + "model": "Tatlock", + "messages": [ + {"role": "user", "content": "Say hello to me."} + ], + "stream": False + } + + response = await async_client.post( + "/v1/chat/completions", + json=request_data, + timeout=120.0 + ) + + assert response.status_code == 200 + data = response.json() + + # Verify response structure is valid + assert "choices" in data + assert len(data["choices"]) == 1 + full_response = data["choices"][0]["message"]["content"] + assert len(full_response) > 0, "Ollama should produce a non-empty response" + + print(f"\nOllama fallback response: {full_response[:200]}") + + finally: + # Restore original value + model_selector._claude_available = original diff --git a/tests/conftest.py b/tests/conftest.py index 11d292b..5bfa42f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,8 @@ Shared test fixtures for all tests. Following FastAPI testing best practices. """ +import asyncio + import pytest from fastapi.testclient import TestClient from httpx import AsyncClient, ASGITransport @@ -9,11 +11,22 @@ from httpx import AsyncClient, ASGITransport from src.main import app +@pytest.fixture(scope="session", autouse=True) +def _initialize_app(): + """ + Run application lifespan (Claude health check, household registration, etc.) + once per test session. ASGITransport doesn't trigger lifespan events, + so we call it explicitly. + """ + from src.core.startup import initialize_application + asyncio.run(initialize_application()) + + @pytest.fixture def client() -> TestClient: """ Synchronous test client for FastAPI. - + Use for simple tests that don't require async. """ return TestClient(app) @@ -23,7 +36,7 @@ def client() -> TestClient: async def async_client() -> AsyncClient: """ Async test client for FastAPI. - + Use for testing async endpoints and streaming. """ async with AsyncClient( @@ -37,7 +50,7 @@ async def async_client() -> AsyncClient: def mock_chat_request() -> dict: """Standard chat completion request fixture.""" return { - "model": "Tatlock", + "model": "lorem-tester", "messages": [ {"role": "user", "content": "Hello, world!"} ], diff --git a/tests/core/test_tool_tracking.py b/tests/core/test_tool_tracking.py index ab83ca4..947b5cc 100644 --- a/tests/core/test_tool_tracking.py +++ b/tests/core/test_tool_tracking.py @@ -3,8 +3,6 @@ Tests for tool call tracking. Tests capability extraction and recommendation matching. """ -from unittest.mock import AsyncMock, patch - import pytest from src.core.tool_tracking import ToolCallTracker @@ -35,15 +33,11 @@ class TestToolCallTracker: recommended_capabilities=["librarian", "biographer"] ) - with patch("src.core.tool_tracking.get_benchmark_store") as mock_store: - mock_store.return_value.record = AsyncMock() + await tracker.track_call("delegate_to_librarian", 1.0) - await tracker.track_call("delegate_to_librarian", 1.0) - - # Should NOT log warning since librarian was recommended - call_args = mock_store.return_value.record.call_args - benchmark = call_args[0][0] - assert benchmark.was_recommended is True + # Should record the call + assert "delegate_to_librarian" in tracker.actual_calls + assert tracker.actual_calls["delegate_to_librarian"] == [1.0] @pytest.mark.asyncio async def test_track_call_detects_not_recommended(self): @@ -52,14 +46,12 @@ class TestToolCallTracker: recommended_capabilities=["librarian"] ) - with patch("src.core.tool_tracking.get_benchmark_store") as mock_store: - mock_store.return_value.record = AsyncMock() + await tracker.track_call("delegate_to_housekeeper", 1.0) - await tracker.track_call("delegate_to_housekeeper", 1.0) - - call_args = mock_store.return_value.record.call_args - benchmark = call_args[0][0] - assert benchmark.was_recommended is False + # Should record the call even though not recommended + assert "delegate_to_housekeeper" in tracker.actual_calls + summary = tracker.get_summary() + assert summary["accuracy"]["not_recommended_but_used"] == 1 def test_get_summary_with_delegation_tools(self): """Test summary correctly maps delegation tools to capabilities.""" @@ -87,15 +79,9 @@ class TestToolCallTracker: "delegate_to_librarian": [1.0], } - with patch("src.core.tool_tracking.get_benchmark_store") as mock_store: - mock_store.return_value.record = AsyncMock() + await tracker.finalize() - await tracker.finalize() - - # Should record benchmark for unused biographer - assert mock_store.return_value.record.called - call_args = mock_store.return_value.record.call_args - benchmark = call_args[0][0] - assert benchmark.tool_name == "biographer" - assert benchmark.was_recommended is True - assert benchmark.was_actually_used is False + # Summary should show biographer as recommended but unused + summary = tracker.get_summary() + assert summary["accuracy"]["recommended_and_used"] == 1 # librarian + assert summary["accuracy"]["recommended_but_unused"] == 1 # biographer