diff --git a/AGENTS.md b/AGENTS.md index 436b2e7..6605c0d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,6 +22,11 @@ This document contains instructions and documentation references for AI assistan * **Test REST endpoints** against `http://localhost:8777` using curl or similar tools * **Only deploy** when a phase or feature is complete and tested locally * **Environment**: Copy `.env.example` to `.env` and configure for your local setup (Ollama, Redis, Qdrant hosts) +* **Running tests**: Always use the venv explicitly to avoid environment mismatches: + ```bash + .venv/bin/python -m pytest tests/ # All tests + .venv/bin/python -m pytest tests/core/ -v # Core tests only + ``` ### 🌐 Internal Service Access * **git.schweitz.net**: Access via `http://localhost:3002` (direct Gitea) to bypass Authentik SSO diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c68d16..3d75e14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.8.5] - 2025-12-16 + +### Fixed + +- **Redis benchmark boolean storage** - Convert booleans to strings for Redis `hset` (Redis doesn't accept bool type directly) +- **Tool tracking capability matching** - `delegate_to_librarian` now correctly recognized as using "librarian" capability when checking Steward recommendations +- **E2E test fixture scope** - Fixed pytest-asyncio ScopeMismatch error by using `loop_scope="module"` for module-scoped async fixtures + ## [1.8.4] - 2025-12-16 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index 99ed950..5d6c014 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "tatlock" -version = "1.8.4" +version = "1.8.5" description = "OpenAI-compatible API with Ollama backend" requires-python = ">=3.12" dependencies = [] diff --git a/src/core/benchmarks.py b/src/core/benchmarks.py index ed45ac2..d833680 100644 --- a/src/core/benchmarks.py +++ b/src/core/benchmarks.py @@ -47,6 +47,10 @@ class PerformanceBenchmark(BaseModel): data = self.model_dump() data["timestamp"] = self.timestamp.isoformat() data["metadata"] = json.dumps(self.metadata) + # Convert booleans to strings (Redis doesn't accept bool type) + for key, value in data.items(): + if isinstance(value, bool): + data[key] = str(value) return data @classmethod @@ -54,6 +58,10 @@ class PerformanceBenchmark(BaseModel): """Reconstruct from Redis dict.""" data["timestamp"] = datetime.fromisoformat(data["timestamp"]) data["metadata"] = json.loads(data.get("metadata", "{}")) + # Convert string booleans back to bool + for key in ["success", "was_recommended", "was_actually_used"]: + if key in data and isinstance(data[key], str): + data[key] = data[key] == "True" return cls(**data) diff --git a/src/core/tool_tracking.py b/src/core/tool_tracking.py index eb8a7e2..beb42c2 100644 --- a/src/core/tool_tracking.py +++ b/src/core/tool_tracking.py @@ -43,6 +43,16 @@ class ToolCallTracker: conversation_id=conversation_id, ) + def _extract_capability(self, tool_name: str) -> str: + """ + Extract capability name from tool name. + + Tool names like 'delegate_to_librarian' map to capability 'librarian'. + """ + if tool_name.startswith("delegate_to_"): + return tool_name.replace("delegate_to_", "") + return tool_name + async def track_call(self, tool_name: str, duration: float): """ Record a tool call with timing. @@ -56,8 +66,9 @@ class ToolCallTracker: self.actual_calls[tool_name] = [] self.actual_calls[tool_name].append(duration) - # Check if tool was recommended - was_recommended = tool_name in self.recommended_capabilities + # Check if tool was recommended (normalize tool name to capability) + capability = self._extract_capability(tool_name) + was_recommended = capability in self.recommended_capabilities if not was_recommended: logger.warning( @@ -98,8 +109,12 @@ class ToolCallTracker: Called after Tatlock completes its response to identify tools that were recommended but never used. """ + # Normalize actual tool names to capabilities for comparison + used_capabilities = { + self._extract_capability(tool) for tool in self.actual_calls.keys() + } # Find tools that were recommended but not used - unused_tools = self.recommended_capabilities - set(self.actual_calls.keys()) + unused_tools = self.recommended_capabilities - used_capabilities if unused_tools: logger.info( @@ -145,7 +160,11 @@ class ToolCallTracker: Dict with tracking statistics """ total_calls = sum(len(durations) for durations in self.actual_calls.values()) - unused = self.recommended_capabilities - set(self.actual_calls.keys()) + # Normalize actual tool names to capabilities for comparison + used_capabilities = { + self._extract_capability(tool) for tool in self.actual_calls.keys() + } + unused = self.recommended_capabilities - used_capabilities return { "recommended_capabilities": list(self.recommended_capabilities), @@ -154,11 +173,11 @@ class ToolCallTracker: "total_calls": total_calls, "accuracy": { "recommended_and_used": len( - self.recommended_capabilities & set(self.actual_calls.keys()) + self.recommended_capabilities & used_capabilities ), "recommended_but_unused": len(unused), "not_recommended_but_used": len( - set(self.actual_calls.keys()) - self.recommended_capabilities + used_capabilities - self.recommended_capabilities ), }, } diff --git a/tests/core/test_benchmarks.py b/tests/core/test_benchmarks.py index 32bba2f..def6946 100644 --- a/tests/core/test_benchmarks.py +++ b/tests/core/test_benchmarks.py @@ -61,7 +61,7 @@ class TestPerformanceBenchmark: redis_dict = benchmark.to_redis_dict() assert redis_dict["operation"] == "test_op" assert redis_dict["duration_seconds"] == 1.0 - assert redis_dict["success"] is True + assert redis_dict["success"] == "True" # Booleans stored as strings in Redis assert isinstance(redis_dict["timestamp"], str) assert isinstance(redis_dict["metadata"], str) @@ -72,7 +72,7 @@ class TestPerformanceBenchmark: "timestamp": now.isoformat(), "operation": "test_op", "duration_seconds": 1.5, - "success": True, + "success": "True", # Booleans stored as strings in Redis "metadata": json.dumps({"test": "data"}), "recommendation_count": None, "confidence": None, @@ -85,6 +85,7 @@ class TestPerformanceBenchmark: benchmark = PerformanceBenchmark.from_redis_dict(redis_dict) assert benchmark.operation == "test_op" assert benchmark.duration_seconds == 1.5 + assert benchmark.success is True # Converted back to bool assert benchmark.metadata == {"test": "data"} @@ -162,12 +163,12 @@ class TestBenchmarkStore: mock_key = f"benchmark:test_op:{int(now.timestamp() * 1000)}" mock_redis.zrevrangebyscore.return_value = [mock_key] - # Mock hgetall to return proper data + # Mock hgetall to return proper data (booleans as strings, like Redis) mock_redis.hgetall.return_value = { "timestamp": now.isoformat(), "operation": "test_op", "duration_seconds": 1.5, # Numeric, not string - "success": True, + "success": "True", # Booleans stored as strings in Redis "metadata": "{}", "recommendation_count": None, "confidence": None, @@ -237,7 +238,7 @@ class TestBenchmarkStore: "timestamp": now.isoformat(), "operation": "test_op", "duration_seconds": float(data["duration_seconds"]), - "success": data["success"] == "True", + "success": data["success"], # Pass string through, from_redis_dict converts "metadata": "{}", "recommendation_count": None, "confidence": None, @@ -296,14 +297,14 @@ class TestBenchmarkStore: "timestamp": now.isoformat(), "operation": "tool_call", "duration_seconds": 1.0, - "success": True, + "success": "True", # Booleans stored as strings in Redis "metadata": "{}", "recommendation_count": None, "confidence": None, "tool_name": "test_tool", "conversation_id": None, - "was_recommended": data["was_recommended"] == "True", - "was_actually_used": data["was_actually_used"] == "True", + "was_recommended": data["was_recommended"], # Already strings + "was_actually_used": data["was_actually_used"], # Already strings } mock_redis.hgetall.side_effect = mock_hgetall diff --git a/tests/core/test_tool_tracking.py b/tests/core/test_tool_tracking.py new file mode 100644 index 0000000..ab83ca4 --- /dev/null +++ b/tests/core/test_tool_tracking.py @@ -0,0 +1,101 @@ +""" +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 + + +class TestToolCallTracker: + """Test ToolCallTracker functionality.""" + + def test_extract_capability_delegation_tool(self): + """Test extracting capability from delegation tool name.""" + tracker = ToolCallTracker(recommended_capabilities=["librarian"]) + + assert tracker._extract_capability("delegate_to_librarian") == "librarian" + assert tracker._extract_capability("delegate_to_biographer") == "biographer" + assert tracker._extract_capability("delegate_to_housekeeper") == "housekeeper" + + def test_extract_capability_non_delegation_tool(self): + """Test that non-delegation tools return unchanged.""" + tracker = ToolCallTracker(recommended_capabilities=[]) + + assert tracker._extract_capability("calculate") == "calculate" + assert tracker._extract_capability("search_web") == "search_web" + + @pytest.mark.asyncio + async def test_track_call_recognizes_delegation_as_recommended(self): + """Test that delegate_to_X is recognized when X is recommended.""" + tracker = ToolCallTracker( + 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) + + # 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 + + @pytest.mark.asyncio + async def test_track_call_detects_not_recommended(self): + """Test that unrecommended tools are flagged.""" + tracker = ToolCallTracker( + 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) + + call_args = mock_store.return_value.record.call_args + benchmark = call_args[0][0] + assert benchmark.was_recommended is False + + def test_get_summary_with_delegation_tools(self): + """Test summary correctly maps delegation tools to capabilities.""" + tracker = ToolCallTracker( + recommended_capabilities=["librarian", "biographer"] + ) + tracker.actual_calls = { + "delegate_to_librarian": [1.0, 2.0], + "delegate_to_housekeeper": [0.5], # Not recommended + } + + summary = tracker.get_summary() + + assert summary["accuracy"]["recommended_and_used"] == 1 # librarian + assert summary["accuracy"]["recommended_but_unused"] == 1 # biographer + assert summary["accuracy"]["not_recommended_but_used"] == 1 # housekeeper + + @pytest.mark.asyncio + async def test_finalize_with_delegation_tools(self): + """Test finalize correctly identifies unused recommendations.""" + tracker = ToolCallTracker( + recommended_capabilities=["librarian", "biographer"] + ) + tracker.actual_calls = { + "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() + + # 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 diff --git a/tests/e2e/test_api_endpoints.py b/tests/e2e/test_api_endpoints.py index 7a429b1..1f9cc72 100644 --- a/tests/e2e/test_api_endpoints.py +++ b/tests/e2e/test_api_endpoints.py @@ -8,8 +8,8 @@ These tests hit the actual running server and test the full stack: - Response formatting """ import pytest +import pytest_asyncio import httpx -import asyncio from typing import AsyncGenerator # Test server base URL (assumes server is running on localhost:8777 via ./wakeup.sh) @@ -17,15 +17,7 @@ BASE_URL = "http://localhost:8777" API_TIMEOUT = 120.0 # 120 second timeout for LLM calls -@pytest.fixture(scope="module") -def event_loop(): - """Create event loop for async tests.""" - loop = asyncio.get_event_loop_policy().new_event_loop() - yield loop - loop.close() - - -@pytest.fixture(scope="module") +@pytest_asyncio.fixture(loop_scope="module", scope="module") async def client() -> AsyncGenerator[httpx.AsyncClient, None]: """HTTP client for making requests.""" async with httpx.AsyncClient(base_url=BASE_URL, timeout=API_TIMEOUT) as client: