From a641159a676b645e524948a5f49eb8ab6b261379 Mon Sep 17 00:00:00 2001 From: Fahad Date: Fri, 13 Jun 2025 09:28:33 +0400 Subject: [PATCH] Use consistent terminology Remove test folder from .gitignore for live simulation test to pass --- .gitignore | 1 - .../test_per_tool_deduplication.py | 20 +++++++++++++++--- tests/mock_helpers.py | 4 ++-- tests/test_conversation_field_mapping.py | 2 +- tests/test_openrouter_provider.py | 4 ++-- tests/test_openrouter_registry.py | 21 ++++++++----------- tests/test_providers.py | 4 ++-- tools/precommit.py | 5 +++-- 8 files changed, 36 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index e936c0a..ceb055a 100644 --- a/.gitignore +++ b/.gitignore @@ -165,4 +165,3 @@ test_simulation_files/.claude/ # Temporary test directories test-setup/ -/test_simulation_files/** diff --git a/simulator_tests/test_per_tool_deduplication.py b/simulator_tests/test_per_tool_deduplication.py index 4d6b55d..d5814a8 100644 --- a/simulator_tests/test_per_tool_deduplication.py +++ b/simulator_tests/test_per_tool_deduplication.py @@ -11,6 +11,7 @@ Validates that: 4. Docker logs show deduplication behavior """ +import os import subprocess from .base_test import BaseSimulatorTest @@ -98,14 +99,17 @@ class PerToolDeduplicationTest(BaseSimulatorTest): # Setup test files self.setup_test_files() - # Create a short dummy file for quick testing + # Create a short dummy file for quick testing in the current repo dummy_content = """def add(a, b): return a + b # Missing type hints def divide(x, y): return x / y # No zero check """ - dummy_file_path = self.create_additional_test_file("dummy_code.py", dummy_content) + # Create the file in the current git repo directory to make it show up in git status + dummy_file_path = os.path.join(os.getcwd(), "dummy_code.py") + with open(dummy_file_path, "w") as f: + f.write(dummy_content) # Get timestamp for log filtering import datetime @@ -162,7 +166,10 @@ def divide(x, y): def subtract(a, b): return a - b """ - new_file_path = self.create_additional_test_file("new_feature.py", new_file_content) + # Create another temp file in the current repo for git changes + new_file_path = os.path.join(os.getcwd(), "new_feature.py") + with open(new_file_path, "w") as f: + f.write(new_file_content) # Continue precommit with both files continue_params = { @@ -249,4 +256,11 @@ def subtract(a, b): self.logger.error(f"File deduplication workflow test failed: {e}") return False finally: + # Clean up temp files created in current repo + temp_files = ["dummy_code.py", "new_feature.py"] + for temp_file in temp_files: + temp_path = os.path.join(os.getcwd(), temp_file) + if os.path.exists(temp_path): + os.remove(temp_path) + self.logger.debug(f"Removed temp file: {temp_path}") self.cleanup_test_files() diff --git a/tests/mock_helpers.py b/tests/mock_helpers.py index 447dd5b..0ee4465 100644 --- a/tests/mock_helpers.py +++ b/tests/mock_helpers.py @@ -5,7 +5,7 @@ from unittest.mock import Mock from providers.base import ModelCapabilities, ProviderType, RangeTemperatureConstraint -def create_mock_provider(model_name="gemini-2.5-flash-preview-05-20", max_tokens=1_048_576): +def create_mock_provider(model_name="gemini-2.5-flash-preview-05-20", context_window=1_048_576): """Create a properly configured mock provider.""" mock_provider = Mock() @@ -14,7 +14,7 @@ def create_mock_provider(model_name="gemini-2.5-flash-preview-05-20", max_tokens provider=ProviderType.GOOGLE, model_name=model_name, friendly_name="Gemini", - max_tokens=max_tokens, + context_window=context_window, supports_extended_thinking=False, supports_system_prompts=True, supports_streaming=True, diff --git a/tests/test_conversation_field_mapping.py b/tests/test_conversation_field_mapping.py index 28cd82e..014d2e7 100644 --- a/tests/test_conversation_field_mapping.py +++ b/tests/test_conversation_field_mapping.py @@ -77,7 +77,7 @@ async def test_conversation_history_field_mapping(): provider=ProviderType.GOOGLE, model_name="gemini-2.5-flash-preview-05-20", friendly_name="Gemini", - max_tokens=200000, + context_window=200000, supports_extended_thinking=True, ) mock_get_provider.return_value = mock_provider diff --git a/tests/test_openrouter_provider.py b/tests/test_openrouter_provider.py index 600400d..34c9433 100644 --- a/tests/test_openrouter_provider.py +++ b/tests/test_openrouter_provider.py @@ -61,7 +61,7 @@ class TestOpenRouterProvider: caps = provider.get_capabilities("unknown-model") assert caps.provider == ProviderType.OPENROUTER assert caps.model_name == "unknown-model" - assert caps.max_tokens == 32_768 # Safe default + assert caps.context_window == 32_768 # Safe default assert hasattr(caps, "_is_generic") and caps._is_generic is True def test_model_alias_resolution(self): @@ -139,7 +139,7 @@ class TestOpenRouterRegistry: caps = registry.get_capabilities("opus") assert caps is not None assert caps.model_name == "anthropic/claude-3-opus" - assert caps.max_tokens == 200000 # Claude's context window + assert caps.context_window == 200000 # Claude's context window # Test using full model name caps = registry.get_capabilities("anthropic/claude-3-opus") diff --git a/tests/test_openrouter_registry.py b/tests/test_openrouter_registry.py index 830ca47..0f55449 100644 --- a/tests/test_openrouter_registry.py +++ b/tests/test_openrouter_registry.py @@ -120,7 +120,7 @@ class TestOpenRouterModelRegistry: assert caps.provider == ProviderType.OPENROUTER assert caps.model_name == "anthropic/claude-3-opus" assert caps.friendly_name == "OpenRouter" - assert caps.max_tokens == 200000 + assert caps.context_window == 200000 assert not caps.supports_extended_thinking def test_duplicate_alias_detection(self): @@ -147,13 +147,13 @@ class TestOpenRouterModelRegistry: os.unlink(temp_path) def test_backwards_compatibility_max_tokens(self): - """Test backwards compatibility with old max_tokens field.""" + """Test that old max_tokens field is no longer supported (should result in empty registry).""" config_data = { "models": [ { "model_name": "test/old-model", "aliases": ["old"], - "max_tokens": 16384, # Old field name + "max_tokens": 16384, # Old field name should cause error "supports_extended_thinking": False, } ] @@ -164,15 +164,12 @@ class TestOpenRouterModelRegistry: temp_path = f.name try: + # Should gracefully handle the error and result in empty registry registry = OpenRouterModelRegistry(config_path=temp_path) - config = registry.resolve("old") - - assert config is not None - assert config.context_window == 16384 # Should be converted - - # Check capabilities still work - caps = config.to_capabilities() - assert caps.max_tokens == 16384 + # Registry should be empty due to config error + assert len(registry.list_models()) == 0 + assert len(registry.list_aliases()) == 0 + assert registry.resolve("old") is None finally: os.unlink(temp_path) @@ -215,7 +212,7 @@ class TestOpenRouterModelRegistry: ) caps = config.to_capabilities() - assert caps.max_tokens == 128000 + assert caps.context_window == 128000 assert caps.supports_extended_thinking assert caps.supports_system_prompts assert caps.supports_streaming diff --git a/tests/test_providers.py b/tests/test_providers.py index f1827f5..5fa019c 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -84,7 +84,7 @@ class TestGeminiProvider: assert capabilities.provider == ProviderType.GOOGLE assert capabilities.model_name == "gemini-2.5-flash-preview-05-20" - assert capabilities.max_tokens == 1_048_576 + assert capabilities.context_window == 1_048_576 assert capabilities.supports_extended_thinking def test_get_capabilities_pro_model(self): @@ -165,7 +165,7 @@ class TestOpenAIProvider: assert capabilities.provider == ProviderType.OPENAI assert capabilities.model_name == "o3-mini" - assert capabilities.max_tokens == 200_000 + assert capabilities.context_window == 200_000 assert not capabilities.supports_extended_thinking def test_validate_model_names(self): diff --git a/tools/precommit.py b/tools/precommit.py index 23bdb99..23a5a2e 100644 --- a/tools/precommit.py +++ b/tools/precommit.py @@ -14,8 +14,6 @@ from typing import Any, Literal, Optional from mcp.types import TextContent from pydantic import Field -# Conservative fallback for token limits -DEFAULT_CONTEXT_WINDOW = 200_000 from prompts.tool_prompts import PRECOMMIT_PROMPT from utils.file_utils import translate_file_paths, translate_path_for_environment from utils.git_utils import find_git_repositories, get_git_status, run_git_command @@ -24,6 +22,9 @@ from utils.token_utils import estimate_tokens from .base import BaseTool, ToolRequest from .models import ToolOutput +# Conservative fallback for token limits +DEFAULT_CONTEXT_WINDOW = 200_000 + class PrecommitRequest(ToolRequest): """Request model for precommit tool"""