From a254ff2220ba00ec30f5110c69a4841419917382 Mon Sep 17 00:00:00 2001 From: Fahad Date: Thu, 2 Oct 2025 11:08:56 +0400 Subject: [PATCH] refactor: removed method from provider, should use model capabilities instead refactor: cleanup temperature factory method --- docs/adding_providers.md | 6 +-- providers/base.py | 5 --- providers/custom.py | 18 --------- providers/dial.py | 25 +++++------- providers/gemini.py | 20 +++------- providers/openai_compatible.py | 7 ---- providers/openai_provider.py | 30 +++++--------- providers/openrouter.py | 14 ------- providers/openrouter_registry.py | 4 +- providers/shared/__init__.py | 2 - providers/shared/temperature.py | 35 +++++++--------- providers/xai.py | 21 ++-------- tests/mock_helpers.py | 1 - tests/test_collaboration.py | 5 --- tests/test_custom_openai_temperature_fix.py | 12 +++--- tests/test_custom_provider.py | 6 +-- tests/test_image_validation.py | 4 -- tests/test_issue_245_simple.py | 4 +- tests/test_large_prompt_handling.py | 6 +-- tests/test_openai_provider.py | 44 ++++++++++----------- tests/test_openrouter_registry.py | 4 +- tests/test_providers.py | 19 +++------ tests/test_workflow_utf8.py | 7 ++-- tests/test_xai_provider.py | 20 ++++------ tools/simple/base.py | 10 +++-- 25 files changed, 105 insertions(+), 224 deletions(-) diff --git a/docs/adding_providers.md b/docs/adding_providers.md index d0dec90..8c49647 100644 --- a/docs/adding_providers.md +++ b/docs/adding_providers.md @@ -15,7 +15,7 @@ Each provider: **Option A: Full Provider (`ModelProvider`)** - For APIs with unique features or custom authentication - Complete control over API calls and response handling -- Required methods: `generate_content()`, `count_tokens()`, `get_capabilities()`, `validate_model_name()`, `supports_thinking_mode()`, `get_provider_type()` +- Required methods: `generate_content()`, `count_tokens()`, `get_capabilities()`, `validate_model_name()`, `get_provider_type()` **Option B: OpenAI-Compatible (`OpenAICompatibleProvider`)** - For APIs that follow OpenAI's chat completion format @@ -130,10 +130,6 @@ class ExampleModelProvider(ModelProvider): def validate_model_name(self, model_name: str) -> bool: resolved_name = self._resolve_model_name(model_name) return resolved_name in self.MODEL_CAPABILITIES - - def supports_thinking_mode(self, model_name: str) -> bool: - capabilities = self.get_capabilities(model_name) - return capabilities.supports_extended_thinking ``` #### Option B: OpenAI-Compatible Provider (Simplified) diff --git a/providers/base.py b/providers/base.py index 93bbe62..8cd2091 100644 --- a/providers/base.py +++ b/providers/base.py @@ -109,11 +109,6 @@ class ModelProvider(ABC): constraint_desc = capabilities.temperature_constraint.get_description() raise ValueError(f"Temperature {temperature} is invalid for model {model_name}. {constraint_desc}") - @abstractmethod - def supports_thinking_mode(self, model_name: str) -> bool: - """Check if the model supports extended thinking mode.""" - pass - def get_model_configurations(self) -> dict[str, ModelCapabilities]: """Get model configurations for this provider. diff --git a/providers/custom.py b/providers/custom.py index 2a96f3a..6a06457 100644 --- a/providers/custom.py +++ b/providers/custom.py @@ -284,24 +284,6 @@ class CustomProvider(OpenAICompatibleProvider): **kwargs, ) - def supports_thinking_mode(self, model_name: str) -> bool: - """Check if the model supports extended thinking mode. - - Args: - model_name: Model to check - - Returns: - True if model supports thinking mode, False otherwise - """ - # Check if model is in registry - config = self._registry.resolve(model_name) if self._registry else None - if config and config.is_custom: - # Trust the config from custom_models.json - return config.supports_extended_thinking - - # Default to False for unknown models - return False - def get_model_configurations(self) -> dict[str, ModelCapabilities]: """Get model configurations from the registry. diff --git a/providers/dial.py b/providers/dial.py index 65f80fe..1c0b885 100644 --- a/providers/dial.py +++ b/providers/dial.py @@ -7,12 +7,7 @@ import time from typing import Optional from .openai_compatible import OpenAICompatibleProvider -from .shared import ( - ModelCapabilities, - ModelResponse, - ProviderType, - create_temperature_constraint, -) +from .shared import ModelCapabilities, ModelResponse, ProviderType, TemperatureConstraint logger = logging.getLogger(__name__) @@ -48,7 +43,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=20.0, supports_temperature=False, # O3 models don't accept temperature - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="OpenAI O3 via DIAL - Strong reasoning model", aliases=["o3"], ), @@ -66,7 +61,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=20.0, supports_temperature=False, # O4 models don't accept temperature - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="OpenAI O4-mini via DIAL - Fast reasoning model", aliases=["o4-mini"], ), @@ -84,7 +79,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=5.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Claude Sonnet 4.1 via DIAL - Balanced performance", aliases=["sonnet-4.1", "sonnet-4"], ), @@ -102,7 +97,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=5.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Claude Sonnet 4.1 with thinking mode via DIAL", aliases=["sonnet-4.1-thinking", "sonnet-4-thinking"], ), @@ -120,7 +115,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=5.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Claude Opus 4.1 via DIAL - Most capable Claude model", aliases=["opus-4.1", "opus-4"], ), @@ -138,7 +133,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=5.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Claude Opus 4.1 with thinking mode via DIAL", aliases=["opus-4.1-thinking", "opus-4-thinking"], ), @@ -156,7 +151,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=20.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Gemini 2.5 Pro with Google Search via DIAL", aliases=["gemini-2.5-pro-search"], ), @@ -174,7 +169,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=20.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Gemini 2.5 Pro via DIAL - Deep reasoning", aliases=["gemini-2.5-pro"], ), @@ -192,7 +187,7 @@ class DIALModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=20.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Gemini 2.5 Flash via DIAL - Ultra-fast", aliases=["gemini-2.5-flash"], ), diff --git a/providers/gemini.py b/providers/gemini.py index 2a79fce..d136f55 100644 --- a/providers/gemini.py +++ b/providers/gemini.py @@ -12,12 +12,7 @@ from google import genai from google.genai import types from .base import ModelProvider -from .shared import ( - ModelCapabilities, - ModelResponse, - ProviderType, - create_temperature_constraint, -) +from .shared import ModelCapabilities, ModelResponse, ProviderType, TemperatureConstraint logger = logging.getLogger(__name__) @@ -46,7 +41,7 @@ class GeminiModelProvider(ModelProvider): supports_images=True, # Vision capability max_image_size_mb=32.0, # Higher limit for Pro model supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), max_thinking_tokens=32768, # Max thinking tokens for Pro model description="Deep reasoning + thinking mode (1M context) - Complex problems, architecture, deep analysis", aliases=["pro", "gemini pro", "gemini-pro"], @@ -65,7 +60,7 @@ class GeminiModelProvider(ModelProvider): supports_images=True, # Vision capability max_image_size_mb=20.0, # Conservative 20MB limit for reliability supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), max_thinking_tokens=24576, # Same as 2.5 flash for consistency description="Gemini 2.0 Flash (1M context) - Latest fast model with experimental thinking, supports audio/video input", aliases=["flash-2.0", "flash2"], @@ -84,7 +79,7 @@ class GeminiModelProvider(ModelProvider): supports_images=False, # Does not support images max_image_size_mb=0.0, # No image support supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Gemini 2.0 Flash Lite (1M context) - Lightweight fast model, text-only", aliases=["flashlite", "flash-lite"], ), @@ -102,7 +97,7 @@ class GeminiModelProvider(ModelProvider): supports_images=True, # Vision capability max_image_size_mb=20.0, # Conservative 20MB limit for reliability supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), max_thinking_tokens=24576, # Flash 2.5 thinking budget limit description="Ultra-fast (1M context) - Quick analysis, simple queries, rapid iterations", aliases=["flash", "flash2.5"], @@ -397,11 +392,6 @@ class GeminiModelProvider(ModelProvider): return True - def supports_thinking_mode(self, model_name: str) -> bool: - """Check if the model supports extended thinking mode.""" - capabilities = self.get_capabilities(model_name) - return capabilities.supports_extended_thinking - def get_thinking_budget(self, model_name: str, thinking_mode: str) -> int: """Get actual thinking token budget for a model and thinking mode.""" resolved_name = self._resolve_model_name(model_name) diff --git a/providers/openai_compatible.py b/providers/openai_compatible.py index 2b5dfd2..6a0454f 100644 --- a/providers/openai_compatible.py +++ b/providers/openai_compatible.py @@ -734,13 +734,6 @@ class OpenAICompatibleProvider(ModelProvider): """ pass - def supports_thinking_mode(self, model_name: str) -> bool: - """Check if the model supports extended thinking mode. - - Default is False for OpenAI-compatible providers. - """ - return False - def _is_error_retryable(self, error: Exception) -> bool: """Determine if an error should be retried based on structured error codes. diff --git a/providers/openai_provider.py b/providers/openai_provider.py index 8040472..29071f0 100644 --- a/providers/openai_provider.py +++ b/providers/openai_provider.py @@ -7,12 +7,7 @@ if TYPE_CHECKING: from tools.models import ToolModelCategory from .openai_compatible import OpenAICompatibleProvider -from .shared import ( - ModelCapabilities, - ModelResponse, - ProviderType, - create_temperature_constraint, -) +from .shared import ModelCapabilities, ModelResponse, ProviderType, TemperatureConstraint logger = logging.getLogger(__name__) @@ -41,7 +36,7 @@ class OpenAIModelProvider(OpenAICompatibleProvider): supports_images=True, # GPT-5 supports vision max_image_size_mb=20.0, # 20MB per OpenAI docs supports_temperature=True, # Regular models accept temperature parameter - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="GPT-5 (400K context, 128K output) - Advanced model with reasoning support", aliases=["gpt5"], ), @@ -59,7 +54,7 @@ class OpenAIModelProvider(OpenAICompatibleProvider): supports_images=True, # GPT-5-mini supports vision max_image_size_mb=20.0, # 20MB per OpenAI docs supports_temperature=True, - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="GPT-5-mini (400K context, 128K output) - Efficient variant with reasoning support", aliases=["gpt5-mini", "gpt5mini", "mini"], ), @@ -77,7 +72,7 @@ class OpenAIModelProvider(OpenAICompatibleProvider): supports_images=True, max_image_size_mb=20.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="GPT-5 nano (400K context) - Fastest, cheapest version of GPT-5 for summarization and classification tasks", aliases=["gpt5nano", "gpt5-nano", "nano"], ), @@ -95,7 +90,7 @@ class OpenAIModelProvider(OpenAICompatibleProvider): supports_images=True, # O3 models support vision max_image_size_mb=20.0, # 20MB per OpenAI docs supports_temperature=False, # O3 models don't accept temperature parameter - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="Strong reasoning (200K context) - Logical problems, code generation, systematic analysis", aliases=[], ), @@ -113,7 +108,7 @@ class OpenAIModelProvider(OpenAICompatibleProvider): supports_images=True, # O3 models support vision max_image_size_mb=20.0, # 20MB per OpenAI docs supports_temperature=False, # O3 models don't accept temperature parameter - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="Fast O3 variant (200K context) - Balanced performance/speed, moderate complexity", aliases=["o3mini"], ), @@ -131,7 +126,7 @@ class OpenAIModelProvider(OpenAICompatibleProvider): supports_images=True, # O3 models support vision max_image_size_mb=20.0, # 20MB per OpenAI docs supports_temperature=False, # O3 models don't accept temperature parameter - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="Professional-grade reasoning (200K context) - EXTREMELY EXPENSIVE: Only for the most complex problems requiring universe-scale complexity analysis OR when the user explicitly asks for this model. Use sparingly for critical architectural decisions or exceptionally complex debugging that other models cannot handle.", aliases=["o3pro"], ), @@ -149,7 +144,7 @@ class OpenAIModelProvider(OpenAICompatibleProvider): supports_images=True, # O4 models support vision max_image_size_mb=20.0, # 20MB per OpenAI docs supports_temperature=False, # O4 models don't accept temperature parameter - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="Latest reasoning model (200K context) - Optimized for shorter contexts, rapid reasoning", aliases=["o4mini"], ), @@ -167,7 +162,7 @@ class OpenAIModelProvider(OpenAICompatibleProvider): supports_images=True, # GPT-4.1 supports vision max_image_size_mb=20.0, # 20MB per OpenAI docs supports_temperature=True, # Regular models accept temperature parameter - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="GPT-4.1 (1M context) - Advanced reasoning model with large context window", aliases=["gpt4.1"], ), @@ -303,13 +298,6 @@ class OpenAIModelProvider(OpenAICompatibleProvider): **kwargs, ) - def supports_thinking_mode(self, model_name: str) -> bool: - """Check if the model supports extended thinking mode.""" - try: - return self.get_capabilities(model_name).supports_extended_thinking - except ValueError: - return False - def get_preferred_model(self, category: "ToolModelCategory", allowed_models: list[str]) -> Optional[str]: """Get OpenAI's preferred model for a given category from allowed models. diff --git a/providers/openrouter.py b/providers/openrouter.py index 67f5990..1a60542 100644 --- a/providers/openrouter.py +++ b/providers/openrouter.py @@ -204,20 +204,6 @@ class OpenRouterProvider(OpenAICompatibleProvider): **kwargs, ) - def supports_thinking_mode(self, model_name: str) -> bool: - """Check if the model supports extended thinking mode. - - Currently, no models via OpenRouter support extended thinking. - This may change as new models become available. - - Args: - model_name: Model to check - - Returns: - False (no OpenRouter models currently support thinking mode) - """ - return False - def list_models( self, *, diff --git a/providers/openrouter_registry.py b/providers/openrouter_registry.py index a7cbfb2..e61ce7f 100644 --- a/providers/openrouter_registry.py +++ b/providers/openrouter_registry.py @@ -12,7 +12,7 @@ from utils.file_utils import read_json_file from .shared import ( ModelCapabilities, ProviderType, - create_temperature_constraint, + TemperatureConstraint, ) @@ -178,7 +178,7 @@ class OpenRouterModelRegistry: # Create ModelCapabilities directly from JSON data # Handle temperature_constraint conversion temp_constraint_str = model_data.get("temperature_constraint") - temp_constraint = create_temperature_constraint(temp_constraint_str or "range") + temp_constraint = TemperatureConstraint.create(temp_constraint_str or "range") # Set provider-specific defaults based on is_custom flag is_custom = model_data.get("is_custom", False) diff --git a/providers/shared/__init__.py b/providers/shared/__init__.py index 2ed6072..aa7c613 100644 --- a/providers/shared/__init__.py +++ b/providers/shared/__init__.py @@ -8,7 +8,6 @@ from .temperature import ( FixedTemperatureConstraint, RangeTemperatureConstraint, TemperatureConstraint, - create_temperature_constraint, ) __all__ = [ @@ -19,5 +18,4 @@ __all__ = [ "FixedTemperatureConstraint", "RangeTemperatureConstraint", "DiscreteTemperatureConstraint", - "create_temperature_constraint", ] diff --git a/providers/shared/temperature.py b/providers/shared/temperature.py index ec0adec..22a54a9 100644 --- a/providers/shared/temperature.py +++ b/providers/shared/temperature.py @@ -8,7 +8,6 @@ __all__ = [ "FixedTemperatureConstraint", "RangeTemperatureConstraint", "DiscreteTemperatureConstraint", - "create_temperature_constraint", ] # Common heuristics for determining temperature support when explicit @@ -102,7 +101,7 @@ class TemperatureConstraint(ABC): """ if constraint_hint: - constraint = create_temperature_constraint(constraint_hint) + constraint = TemperatureConstraint.create(constraint_hint) supports_temperature = constraint_hint != "fixed" reason = f"constraint hint '{constraint_hint}'" return supports_temperature, constraint, reason @@ -115,6 +114,19 @@ class TemperatureConstraint(ABC): return supports_temperature, constraint, reason + @staticmethod + def create(constraint_type: str) -> "TemperatureConstraint": + """Factory that yields the appropriate constraint for a configuration hint.""" + + if constraint_type == "fixed": + # Fixed temperature models (O3/O4) only support temperature=1.0 + return FixedTemperatureConstraint(1.0) + if constraint_type == "discrete": + # For models with specific allowed values - using common OpenAI values as default + return DiscreteTemperatureConstraint([0.0, 0.3, 0.7, 1.0, 1.5, 2.0], 0.3) + # Default range constraint (for "range" or None) + return RangeTemperatureConstraint(0.0, 2.0, 0.3) + class FixedTemperatureConstraint(TemperatureConstraint): """Constraint for models that enforce an exact temperature (for example O3).""" @@ -174,22 +186,3 @@ class DiscreteTemperatureConstraint(TemperatureConstraint): def get_default(self) -> float: return self.default_temp - - -def create_temperature_constraint(constraint_type: str) -> TemperatureConstraint: - """Factory that yields the appropriate constraint for a model configuration. - - The JSON configuration stored in ``conf/custom_models.json`` references this - helper via human-readable strings. Providers feed those values into this - function so that runtime logic can rely on strongly typed constraint - objects. - """ - - if constraint_type == "fixed": - # Fixed temperature models (O3/O4) only support temperature=1.0 - return FixedTemperatureConstraint(1.0) - if constraint_type == "discrete": - # For models with specific allowed values - using common OpenAI values as default - return DiscreteTemperatureConstraint([0.0, 0.3, 0.7, 1.0, 1.5, 2.0], 0.3) - # Default range constraint (for "range" or None) - return RangeTemperatureConstraint(0.0, 2.0, 0.3) diff --git a/providers/xai.py b/providers/xai.py index e0daf2f..a9b2387 100644 --- a/providers/xai.py +++ b/providers/xai.py @@ -7,12 +7,7 @@ if TYPE_CHECKING: from tools.models import ToolModelCategory from .openai_compatible import OpenAICompatibleProvider -from .shared import ( - ModelCapabilities, - ModelResponse, - ProviderType, - create_temperature_constraint, -) +from .shared import ModelCapabilities, ModelResponse, ProviderType, TemperatureConstraint logger = logging.getLogger(__name__) @@ -42,7 +37,7 @@ class XAIModelProvider(OpenAICompatibleProvider): supports_images=True, # Multimodal capabilities max_image_size_mb=20.0, # Standard image size limit supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="GROK-4 (256K context) - Frontier multimodal reasoning model with advanced capabilities", aliases=["grok", "grok4", "grok-4"], ), @@ -60,7 +55,7 @@ class XAIModelProvider(OpenAICompatibleProvider): supports_images=False, # Assuming GROK is text-only for now max_image_size_mb=0.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="GROK-3 (131K context) - Advanced reasoning model from X.AI, excellent for complex analysis", aliases=["grok3"], ), @@ -78,7 +73,7 @@ class XAIModelProvider(OpenAICompatibleProvider): supports_images=False, # Assuming GROK is text-only for now max_image_size_mb=0.0, supports_temperature=True, - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="GROK-3 Fast (131K context) - Higher performance variant, faster processing but more expensive", aliases=["grok3fast", "grokfast", "grok3-fast"], ), @@ -153,14 +148,6 @@ class XAIModelProvider(OpenAICompatibleProvider): **kwargs, ) - def supports_thinking_mode(self, model_name: str) -> bool: - """Check if the model supports extended thinking mode.""" - resolved_name = self._resolve_model_name(model_name) - capabilities = self.MODEL_CAPABILITIES.get(resolved_name) - if capabilities: - return capabilities.supports_extended_thinking - return False - def get_preferred_model(self, category: "ToolModelCategory", allowed_models: list[str]) -> Optional[str]: """Get XAI's preferred model for a given category from allowed models. diff --git a/tests/mock_helpers.py b/tests/mock_helpers.py index 6ecf90f..09766c3 100644 --- a/tests/mock_helpers.py +++ b/tests/mock_helpers.py @@ -25,7 +25,6 @@ def create_mock_provider(model_name="gemini-2.5-flash", context_window=1_048_576 mock_provider.get_capabilities.return_value = mock_capabilities mock_provider.get_provider_type.return_value = ProviderType.GOOGLE - mock_provider.supports_thinking_mode.return_value = False mock_provider.validate_model_name.return_value = True # Set up generate_content response diff --git a/tests/test_collaboration.py b/tests/test_collaboration.py index 367f081..399fe41 100644 --- a/tests/test_collaboration.py +++ b/tests/test_collaboration.py @@ -40,7 +40,6 @@ class TestDynamicContextRequests: mock_provider = create_mock_provider() mock_provider.get_provider_type.return_value = Mock(value="google") - mock_provider.supports_thinking_mode.return_value = False mock_provider.generate_content.return_value = Mock( content=clarification_json, usage={}, model_name="gemini-2.5-flash", metadata={} ) @@ -122,7 +121,6 @@ class TestDynamicContextRequests: mock_provider = create_mock_provider() mock_provider.get_provider_type.return_value = Mock(value="google") - mock_provider.supports_thinking_mode.return_value = False mock_provider.generate_content.return_value = Mock( content=malformed_json, usage={}, model_name="gemini-2.5-flash", metadata={} ) @@ -181,7 +179,6 @@ class TestDynamicContextRequests: mock_provider = create_mock_provider() mock_provider.get_provider_type.return_value = Mock(value="google") - mock_provider.supports_thinking_mode.return_value = False mock_provider.generate_content.return_value = Mock( content=clarification_json, usage={}, model_name="gemini-2.5-flash", metadata={} ) @@ -347,7 +344,6 @@ class TestCollaborationWorkflow: mock_provider = create_mock_provider() mock_provider.get_provider_type.return_value = Mock(value="google") - mock_provider.supports_thinking_mode.return_value = False mock_provider.generate_content.return_value = Mock( content=clarification_json, usage={}, model_name="gemini-2.5-flash", metadata={} ) @@ -414,7 +410,6 @@ class TestCollaborationWorkflow: mock_provider = create_mock_provider() mock_provider.get_provider_type.return_value = Mock(value="google") - mock_provider.supports_thinking_mode.return_value = False mock_provider.generate_content.return_value = Mock( content=clarification_json, usage={}, model_name="gemini-2.5-flash", metadata={} ) diff --git a/tests/test_custom_openai_temperature_fix.py b/tests/test_custom_openai_temperature_fix.py index fe168f6..b13441f 100644 --- a/tests/test_custom_openai_temperature_fix.py +++ b/tests/test_custom_openai_temperature_fix.py @@ -86,7 +86,7 @@ class TestCustomOpenAITemperatureParameterFix: mock_registry_class.return_value = mock_registry # Mock get_model_config to return our test model - from providers.shared import ModelCapabilities, ProviderType, create_temperature_constraint + from providers.shared import ModelCapabilities, ProviderType, TemperatureConstraint test_capabilities = ModelCapabilities( provider=ProviderType.OPENAI, @@ -102,7 +102,7 @@ class TestCustomOpenAITemperatureParameterFix: supports_images=True, max_image_size_mb=20.0, supports_temperature=False, # This is the key setting - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="Custom OpenAI GPT-5 test model", ) @@ -170,7 +170,7 @@ class TestCustomOpenAITemperatureParameterFix: mock_registry_class.return_value = mock_registry # Mock get_model_config to return a model that supports temperature - from providers.shared import ModelCapabilities, ProviderType, create_temperature_constraint + from providers.shared import ModelCapabilities, ProviderType, TemperatureConstraint test_capabilities = ModelCapabilities( provider=ProviderType.OPENAI, @@ -186,7 +186,7 @@ class TestCustomOpenAITemperatureParameterFix: supports_images=True, max_image_size_mb=20.0, supports_temperature=True, # This model DOES support temperature - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), description="Custom OpenAI GPT-4 test model", ) @@ -227,7 +227,7 @@ class TestCustomOpenAITemperatureParameterFix: mock_registry = Mock() mock_registry_class.return_value = mock_registry - from providers.shared import ModelCapabilities, ProviderType, create_temperature_constraint + from providers.shared import ModelCapabilities, ProviderType, TemperatureConstraint test_capabilities = ModelCapabilities( provider=ProviderType.OPENAI, @@ -243,7 +243,7 @@ class TestCustomOpenAITemperatureParameterFix: supports_images=True, max_image_size_mb=20.0, supports_temperature=False, - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), description="Custom OpenAI O3 test model", ) diff --git a/tests/test_custom_provider.py b/tests/test_custom_provider.py index 19b67aa..4c300f9 100644 --- a/tests/test_custom_provider.py +++ b/tests/test_custom_provider.py @@ -99,11 +99,11 @@ class TestCustomProvider: assert resolved_local == "llama3.2" def test_no_thinking_mode_support(self): - """Test CustomProvider doesn't support thinking mode.""" + """Custom provider generic capabilities default to no thinking mode.""" provider = CustomProvider(api_key="test-key", base_url="http://localhost:11434/v1") - assert not provider.supports_thinking_mode("llama3.2") - assert not provider.supports_thinking_mode("any-model") + assert not provider.get_capabilities("llama3.2").supports_extended_thinking + assert not provider.get_capabilities("any-model").supports_extended_thinking @patch("providers.custom.OpenAICompatibleProvider.generate_content") def test_generate_content_with_alias_resolution(self, mock_generate): diff --git a/tests/test_image_validation.py b/tests/test_image_validation.py index 12dbcd5..9734f6c 100644 --- a/tests/test_image_validation.py +++ b/tests/test_image_validation.py @@ -43,10 +43,6 @@ class MinimalTestProvider(ModelProvider): """Not needed for image validation tests.""" raise NotImplementedError("Not needed for image validation tests") - def supports_thinking_mode(self, model_name: str) -> bool: - """Not needed for image validation tests.""" - raise NotImplementedError("Not needed for image validation tests") - class TestImageValidation: """Test suite for image validation functionality.""" diff --git a/tests/test_issue_245_simple.py b/tests/test_issue_245_simple.py index 647de02..701fbda 100644 --- a/tests/test_issue_245_simple.py +++ b/tests/test_issue_245_simple.py @@ -41,7 +41,7 @@ def test_issue_245_custom_openai_temperature_ignored(): mock_registry = Mock() mock_registry_class.return_value = mock_registry - from providers.shared import ModelCapabilities, ProviderType, create_temperature_constraint + from providers.shared import ModelCapabilities, ProviderType, TemperatureConstraint # This is what the user configured in their custom_models.json custom_config = ModelCapabilities( @@ -56,7 +56,7 @@ def test_issue_245_custom_openai_temperature_ignored(): supports_streaming=True, supports_function_calling=True, supports_temperature=False, # User set this to false! - temperature_constraint=create_temperature_constraint("fixed"), + temperature_constraint=TemperatureConstraint.create("fixed"), supports_images=True, max_image_size_mb=20.0, description="Custom OpenAI GPT-5", diff --git a/tests/test_large_prompt_handling.py b/tests/test_large_prompt_handling.py index 20649f6..54d736b 100644 --- a/tests/test_large_prompt_handling.py +++ b/tests/test_large_prompt_handling.py @@ -244,7 +244,7 @@ class TestLargePromptHandling: with patch.object(tool, "get_model_provider") as mock_get_provider: mock_provider = MagicMock() mock_provider.get_provider_type.return_value = MagicMock(value="google") - mock_provider.supports_thinking_mode.return_value = False + mock_provider.get_capabilities.return_value = MagicMock(supports_extended_thinking=False) mock_provider.generate_content.return_value = MagicMock( content="Success", usage={"input_tokens": 10, "output_tokens": 20, "total_tokens": 30}, @@ -287,7 +287,7 @@ class TestLargePromptHandling: with patch.object(tool, "get_model_provider") as mock_get_provider: mock_provider = MagicMock() mock_provider.get_provider_type.return_value = MagicMock(value="google") - mock_provider.supports_thinking_mode.return_value = False + mock_provider.get_capabilities.return_value = MagicMock(supports_extended_thinking=False) mock_provider.generate_content.return_value = MagicMock( content="Response to the large prompt", usage={"input_tokens": 12000, "output_tokens": 10, "total_tokens": 12010}, @@ -319,7 +319,7 @@ class TestLargePromptHandling: with patch.object(tool, "get_model_provider") as mock_get_provider: mock_provider = MagicMock() mock_provider.get_provider_type.return_value = MagicMock(value="google") - mock_provider.supports_thinking_mode.return_value = False + mock_provider.get_capabilities.return_value = MagicMock(supports_extended_thinking=False) mock_provider.generate_content.return_value = MagicMock( content="Success", usage={"input_tokens": 10, "output_tokens": 20, "total_tokens": 30}, diff --git a/tests/test_openai_provider.py b/tests/test_openai_provider.py index 752935e..61cae0e 100644 --- a/tests/test_openai_provider.py +++ b/tests/test_openai_provider.py @@ -252,33 +252,31 @@ class TestOpenAIProvider: call_kwargs = mock_client.chat.completions.create.call_args[1] assert call_kwargs["model"] == "o3-mini" # Should be unchanged - def test_supports_thinking_mode(self): - """Test thinking mode support based on model capabilities.""" + def test_extended_thinking_capabilities(self): + """Thinking-mode support should be reflected via ModelCapabilities.""" provider = OpenAIModelProvider("test-key") - # GPT-5 models support thinking mode (reasoning tokens) - all variants - assert provider.supports_thinking_mode("gpt-5") is True - assert provider.supports_thinking_mode("gpt-5-mini") is True - assert provider.supports_thinking_mode("gpt-5-nano") is True # Now included + supported_aliases = [ + "gpt-5", + "gpt-5-mini", + "gpt-5-nano", + "gpt5", + "gpt5-mini", + "gpt5mini", + "gpt5-nano", + "gpt5nano", + "nano", + "mini", # resolves to gpt-5-mini + ] + for alias in supported_aliases: + assert provider.get_capabilities(alias).supports_extended_thinking is True - # Test GPT-5 aliases - assert provider.supports_thinking_mode("gpt5") is True - assert provider.supports_thinking_mode("gpt5-mini") is True - assert provider.supports_thinking_mode("gpt5mini") is True - assert provider.supports_thinking_mode("gpt5-nano") is True - assert provider.supports_thinking_mode("gpt5nano") is True - assert provider.supports_thinking_mode("nano") is True # New alias for gpt-5-nano + unsupported_aliases = ["o3", "o3-mini", "o4-mini"] + for alias in unsupported_aliases: + assert provider.get_capabilities(alias).supports_extended_thinking is False - # O3/O4 models don't support thinking mode - assert provider.supports_thinking_mode("o3") is False - assert provider.supports_thinking_mode("o3-mini") is False - assert provider.supports_thinking_mode("o4-mini") is False - assert ( - provider.supports_thinking_mode("mini") is True - ) # "mini" now resolves to gpt-5-mini which supports thinking - - # Test invalid model name - assert provider.supports_thinking_mode("invalid-model") is False + # Invalid models should not validate, treat as unsupported + assert not provider.validate_model_name("invalid-model") @patch("providers.openai_compatible.OpenAI") def test_o3_pro_routes_to_responses_endpoint(self, mock_openai_class): diff --git a/tests/test_openrouter_registry.py b/tests/test_openrouter_registry.py index 2a172b2..866cc3f 100644 --- a/tests/test_openrouter_registry.py +++ b/tests/test_openrouter_registry.py @@ -213,7 +213,7 @@ class TestOpenRouterModelRegistry: def test_model_with_all_capabilities(self): """Test model with all capability flags.""" - from providers.shared import create_temperature_constraint + from providers.shared import TemperatureConstraint caps = ModelCapabilities( provider=ProviderType.OPENROUTER, @@ -228,7 +228,7 @@ class TestOpenRouterModelRegistry: supports_function_calling=True, supports_json_mode=True, description="Fully featured test model", - temperature_constraint=create_temperature_constraint("range"), + temperature_constraint=TemperatureConstraint.create("range"), ) assert caps.context_window == 128000 assert caps.supports_extended_thinking diff --git a/tests/test_providers.py b/tests/test_providers.py index c9534b5..c3bdf9e 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -120,13 +120,6 @@ class TestGeminiProvider: capabilities = provider.get_capabilities("flash") assert capabilities.model_name == "gemini-2.5-flash" - def test_supports_thinking_mode(self): - """Test thinking mode support detection""" - provider = GeminiModelProvider(api_key="test-key") - - assert provider.supports_thinking_mode("gemini-2.5-flash") - assert provider.supports_thinking_mode("gemini-2.5-pro") - @patch("google.genai.Client") def test_generate_content(self, mock_client_class): """Test content generation""" @@ -219,12 +212,10 @@ class TestOpenAIProvider: assert not provider.validate_model_name("gpt-4o") assert not provider.validate_model_name("invalid-model") - def test_no_thinking_mode_support(self): - """Test that no OpenAI models support thinking mode""" + def test_openai_models_do_not_support_extended_thinking(self): + """OpenAI catalogue exposes extended thinking capability via ModelCapabilities.""" provider = OpenAIModelProvider(api_key="test-key") - assert not provider.supports_thinking_mode("o3") - assert not provider.supports_thinking_mode("o3mini") - assert not provider.supports_thinking_mode("o3-mini") - assert not provider.supports_thinking_mode("o4-mini") - assert not provider.supports_thinking_mode("o4-mini") + aliases = ["o3", "o3mini", "o3-mini", "o4-mini", "o4mini"] + for alias in aliases: + assert not provider.get_capabilities(alias).supports_extended_thinking diff --git a/tests/test_workflow_utf8.py b/tests/test_workflow_utf8.py index 506cc61..46eef3b 100644 --- a/tests/test_workflow_utf8.py +++ b/tests/test_workflow_utf8.py @@ -75,7 +75,7 @@ class TestWorkflowToolsUTF8(unittest.IsolatedAsyncioTestCase): # Mock provider with more complete setup (same as codereview test) mock_provider = Mock() mock_provider.get_provider_type.return_value = Mock(value="test") - mock_provider.supports_thinking_mode.return_value = False + mock_provider.get_capabilities.return_value = Mock(supports_extended_thinking=False) mock_provider.generate_content = AsyncMock( return_value=Mock( content=json.dumps( @@ -93,6 +93,7 @@ class TestWorkflowToolsUTF8(unittest.IsolatedAsyncioTestCase): # Use the same provider for both contexts mock_get_provider.return_value = mock_provider mock_context_instance.provider = mock_provider + mock_context_instance.capabilities = Mock(supports_extended_thinking=False) mock_model_context.return_value = mock_context_instance # Test the tool @@ -131,7 +132,7 @@ class TestWorkflowToolsUTF8(unittest.IsolatedAsyncioTestCase): # Mock with analysis in French mock_provider = Mock() mock_provider.get_provider_type.return_value = Mock(value="test") - mock_provider.supports_thinking_mode.return_value = False + mock_provider.get_capabilities.return_value = Mock(supports_extended_thinking=False) mock_provider.generate_content = AsyncMock( return_value=Mock( content=json.dumps( @@ -204,7 +205,7 @@ class TestWorkflowToolsUTF8(unittest.IsolatedAsyncioTestCase): # Mock provider mock_provider = Mock() mock_provider.get_provider_type.return_value = Mock(value="test") - mock_provider.supports_thinking_mode.return_value = False + mock_provider.get_capabilities.return_value = Mock(supports_extended_thinking=False) mock_provider.generate_content = AsyncMock( return_value=Mock( content=json.dumps( diff --git a/tests/test_xai_provider.py b/tests/test_xai_provider.py index bb8e97b..f3e3a76 100644 --- a/tests/test_xai_provider.py +++ b/tests/test_xai_provider.py @@ -144,21 +144,17 @@ class TestXAIProvider: with pytest.raises(ValueError, match="Unsupported X.AI model"): provider.get_capabilities("invalid-model") - def test_thinking_mode_support(self): - """Test thinking mode support for X.AI models.""" + def test_extended_thinking_flags(self): + """X.AI capabilities should expose extended thinking support correctly.""" provider = XAIModelProvider("test-key") - # Grok-4 supports thinking mode - assert provider.supports_thinking_mode("grok-4") is True - assert provider.supports_thinking_mode("grok") is True # Resolves to grok-4 + thinking_aliases = ["grok-4", "grok", "grok4"] + for alias in thinking_aliases: + assert provider.get_capabilities(alias).supports_extended_thinking is True - # Grok-3 models don't support thinking mode - assert not provider.supports_thinking_mode("grok-3") - assert not provider.supports_thinking_mode("grok-3-fast") - assert provider.supports_thinking_mode("grok-4") # grok-4 supports thinking mode - assert provider.supports_thinking_mode("grok") # resolves to grok-4 - assert provider.supports_thinking_mode("grok4") # resolves to grok-4 - assert not provider.supports_thinking_mode("grokfast") + non_thinking_aliases = ["grok-3", "grok-3-fast", "grokfast"] + for alias in non_thinking_aliases: + assert provider.get_capabilities(alias).supports_extended_thinking is False def test_provider_type(self): """Test provider type identification.""" diff --git a/tools/simple/base.py b/tools/simple/base.py index b3dc611..166b09a 100644 --- a/tools/simple/base.py +++ b/tools/simple/base.py @@ -422,13 +422,17 @@ class SimpleTool(BaseTool): estimated_tokens = estimate_tokens(prompt) logger.debug(f"Prompt length: {len(prompt)} characters (~{estimated_tokens:,} tokens)") + # Resolve model capabilities for feature gating + capabilities = self._model_context.capabilities + supports_thinking = capabilities.supports_extended_thinking + # Generate content with provider abstraction model_response = provider.generate_content( prompt=prompt, model_name=self._current_model_name, system_prompt=system_prompt, temperature=temperature, - thinking_mode=thinking_mode if provider.supports_thinking_mode(self._current_model_name) else None, + thinking_mode=thinking_mode if supports_thinking else None, images=images if images else None, ) @@ -485,9 +489,7 @@ class SimpleTool(BaseTool): model_name=self._current_model_name, system_prompt=system_prompt, temperature=temperature, - thinking_mode=( - thinking_mode if provider.supports_thinking_mode(self._current_model_name) else None - ), + thinking_mode=thinking_mode if supports_thinking else None, images=images if images else None, )