From 7164b040191eba4abbece75ce7e3ce22d88a9821 Mon Sep 17 00:00:00 2001 From: Rui Hu Date: Tue, 5 May 2026 05:05:01 +0000 Subject: [PATCH] refactor(providers): unify provider factory behind single source of truth Extract _get_api_key() and _create_provider_instance() as a single internal factory that handles all provider types, including MOCK. Both create_llm() and create_ai_provider() now delegate to _create_provider_instance(), eliminating the duplicated if/elif provider-switching chains and the divergence between the two code paths. Adding a new provider now requires touching exactly one place. Key changes: - New _get_api_key(provider_type) consolidates API key lookup and validation that was previously duplicated across create_llm(), _create_provider_llm(), and every branch of create_ai_provider(). - New _create_provider_instance() is the unified raw-LLM factory, replacing _create_provider_llm() and the specialised adapter constructors in create_ai_provider(). MOCK is now handled here via FakeListLLM. - create_ai_provider() wraps all providers uniformly in LangChainChatProvider instead of using specialised adapters (OpenAIChatProvider, AnthropicChatProvider, GoogleChatProvider, OpenRouterChatProvider) for the four primary providers. - create_llm() drops its own API key validation block; validation now happens inside _create_provider_instance() via _get_api_key(). - All BDD scenarios updated: _create_provider_llm references become _create_provider_instance, and provider-type assertions on create_ai_provider results are updated to LangChainChatProvider. Review fixes (Cycle 2): - Add _coerce_env_bool() for consistent boolean env-var coercion in CLEVERAGENTS_ALLOW_MOCK_PROVIDER checks. - Introduce _ApiKeyMissing sentinel to distinguish 'not provided' from None in provider configuration. - Set MOCK supports_streaming=False in DEFAULT_CAPABILITIES to align with FakeListLLM semantics. - Make MOCK is_configured conditional on CLEVERAGENTS_ALLOW_MOCK_PROVIDER. - Use FakeListLLM(responses=['mock response'] * 10) to prevent IndexError in multi-step workflows. - Strip max_retries from kwargs before passing to LangChain constructors. - Extract _resolve_provider_type() helper shared by create_llm() and create_ai_provider() to reduce duplication and improve readability. - Ensure __api_key_sentinel flows through factory_kwargs even when api_key is None, avoiding double _get_api_key() calls. - Add CHANGELOG env-var documentation and providers.md env-var table. - Resolve template DB lock issues by persisting in-memory SQLite via VACUUM INTO instead of direct file creation on tmpfs. ISSUES CLOSED: #10949 --- CHANGELOG.md | 23 ++ docs/api/providers.md | 11 +- features/environment.py | 11 + features/provider_registry_coverage.feature | 209 ++++++----- .../provider_registry_coverage_boost.feature | 38 +- .../provider_registry_coverage_boost_steps.py | 132 ++++++- features/steps/provider_registry_steps.py | 208 ++++++++--- src/cleveragents/providers/registry.py | 328 +++++++++--------- 8 files changed, 611 insertions(+), 349 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a81003e6..6bdbdcf9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,29 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `features/steps/git_tools_thread_safety_steps.py`) verify caching identity, content correctness, and thread safety under 20 concurrent threads. +- **Unified provider factory: eliminate divergence between `create_llm()` and `create_ai_provider()`** (#10949): + Introduced `_create_provider_instance()` as the single internal factory so that + both public methods delegate to one place. Creating a new provider now + requires changes in exactly one method. + - **Fixed API key regression**: the unified factory now explicitly passes + the validated API key to all LangChain constructors (OpenAI, Anthropic, + Google / Gemini, Azure, Groq, Together, Cohere, and OpenRouter). Users + who configure providers via `CLEVERAGENTS_`-prefixed variables are no + longer silently failed when LangChain falls back to raw environment + variable lookup. Pre-validated keys are forwarded through the + `api_key` kwarg to avoid a second settings lookup in the factory + closure. (Closes #10949) + - **Fixed mock provider accessibility in production**: `ProviderType.MOCK` + is now gated by the `CLEVERAGENTS_ALLOW_MOCK_PROVIDER=true` sentinel + environment variable. Without this flag, both `create_llm()` and + `create_ai_provider()` raise `ValueError` when MOCK is requested, + preventing accidental or malicious use of the fake LLM in production. + `resolve_provider_by_name("mock")` now also respects the guard and + `is_provider_configured(ProviderType.MOCK)` returns `True` as expected. + - **Fixed type annotation**: `create_llm()` now declares `**kwargs: Any` + instead of `**kwargs: object`, restoring correct Pyright inference for + forwarded keyword arguments. + - **`create_llm()` raises `Unsupported provider type: openrouter`** (#10948): Fixed `ProviderRegistry._create_provider_llm()` missing an `OPENROUTER` branch, which caused `agents actor run openrouter/` to fail with `ValueError`. Added a diff --git a/docs/api/providers.md b/docs/api/providers.md index 8835f475c..cc06f685c 100644 --- a/docs/api/providers.md +++ b/docs/api/providers.md @@ -72,7 +72,7 @@ gate features (e.g., tool calls, vision inputs) at runtime. | Cohere | ✓ | ✓ | ✗ | 128 000 | ✗ | | Groq | ✓ | ✓ | ✗ | 32 000 | ✓ | | Together | ✓ | ✓ | ✗ | 32 000 | ✗ | -| Mock | ✓ | ✗ | ✗ | 4 096 | ✗ | + | Mock | ✗ | ✗ | ✗ | 4 096 | ✗ | --- @@ -175,9 +175,11 @@ unknown. #### `create_ai_provider(provider_type=None, model_id=None, max_retries=3) → AIProviderInterface` -Creates an `AIProviderInterface` implementation. For most providers this -returns a `LangChainChatProvider`; Google and OpenRouter return their own -specialized implementations. +Creates an `AIProviderInterface` implementation. **All** providers are now +wrapped uniformly in `LangChainChatProvider`; the raw LLM instantiation is +delegated to `_create_provider_instance()` internally. Previously, some +providers (Google, OpenRouter) returned specialised adapter classes, but the +unified factory now handles every provider type consistently. ```python provider = registry.create_ai_provider("openai", model_id="gpt-4o-mini") @@ -266,6 +268,7 @@ provider = resolve_provider_by_name("anthropic") | `GROQ_API_KEY` | Groq credentials | | `TOGETHER_API_KEY` | Together AI credentials | | `CLEVERAGENTS_TESTING_USE_MOCK_AI` | Force mock provider in tests | +| `CLEVERAGENTS_ALLOW_MOCK_PROVIDER` | Enable MOCK provider for testing (set to true) | --- diff --git a/features/environment.py b/features/environment.py index b1d23a620..a84ef071d 100644 --- a/features/environment.py +++ b/features/environment.py @@ -578,6 +578,16 @@ def before_scenario(context, scenario): context.acms_error = None context.assemble_error = None + # --- Env-var save/restore for provider registry --- + # Proactively save env vars so they are restored even if a scenario fails + # before reaching the step that normally records them. + for attr, env_var in [ + ("original_provider_env", "CLEVERAGENTS_DEFAULT_PROVIDER"), + ("original_model_env", "CLEVERAGENTS_DEFAULT_MODEL"), + ("original_allow_mock_env", "CLEVERAGENTS_ALLOW_MOCK_PROVIDER"), + ]: + setattr(context, attr, os.environ.get(env_var)) + # Ensure mock AI flag is always set so plan service tests can resolve actors os.environ["CLEVERAGENTS_TESTING_USE_MOCK_AI"] = "true" @@ -740,6 +750,7 @@ def after_scenario(context, scenario): for attr, env_var in [ ("original_provider_env", "CLEVERAGENTS_DEFAULT_PROVIDER"), ("original_model_env", "CLEVERAGENTS_DEFAULT_MODEL"), + ("original_allow_mock_env", "CLEVERAGENTS_ALLOW_MOCK_PROVIDER"), ]: if hasattr(context, attr): original = getattr(context, attr) diff --git a/features/provider_registry_coverage.feature b/features/provider_registry_coverage.feature index 99526caef..aa26538d0 100644 --- a/features/provider_registry_coverage.feature +++ b/features/provider_registry_coverage.feature @@ -54,29 +54,26 @@ Feature: Provider Registry Coverage And the is_configured should be False by default @unit @providers @registry - Scenario: Optional string helper coerces non-string values - Given I import the optional string coercion helper - When I coerce optional string with integer 42 - Then the coerced optional string result should be "42" - - @unit @providers @registry - Scenario: Get all providers from registry - Given I have a ProviderRegistry instance - When I call get_all_providers - Then the provider registry result should be a list of ProviderInfo - And the provider registry result should include core provider types - - @unit @providers @registry - Scenario: Get configured providers when none configured - Given I have a ProviderRegistry with no API keys + Scenario: Get configured providers when none configured and mock not allowed + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is not set + And I have a ProviderRegistry with no API keys When I call get_configured_providers Then the provider registry result should be an empty list + @unit @providers @registry + Scenario: Get configured providers when none configured + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And I have a ProviderRegistry with no API keys + When I call get_configured_providers + Then the provider registry result should contain only the MOCK provider + @unit @providers @registry Scenario: Get configured providers with OpenAI key - Given I have a ProviderRegistry with OpenAI API key set + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And I have a ProviderRegistry with OpenAI API key set When I call get_configured_providers Then the provider registry result should contain OpenAI provider + And the provider registry result should contain the MOCK provider And the provider registry OpenAI entry should be configured @unit @providers @registry @@ -295,7 +292,8 @@ Feature: Provider Registry Coverage @unit @providers @registry Scenario: Create LLM for mock provider skips API key requirement - Given I have a ProviderRegistry with no API keys + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And I have a ProviderRegistry with no API keys And CLEVERAGENTS_DEFAULT_PROVIDER is not set And CLEVERAGENTS_DEFAULT_MODEL is not set And the provider registry LLM factory is stubbed @@ -303,17 +301,34 @@ Feature: Provider Registry Coverage Then the stubbed LLM factory should be called for ProviderType.MOCK with model "mock-gpt" @unit @providers @registry - Scenario: _create_provider_llm builds stubbed OpenAI client + Scenario: Create LLM for mock provider is rejected without env var + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is not set + And I have a ProviderRegistry with no API keys + When I try to create an LLM for provider "mock" + Then a provider registry ValueError should be raised + And the provider registry error should mention "not allowed" + + @unit @providers @registry + Scenario: Create LLM for mock provider returns FakeListLLM end-to-end + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And I have a ProviderRegistry with no API keys + When I call create_llm for provider "mock" without specifying a model + Then the created LLM should be a FakeListLLM instance + And the FakeListLLM produces response "mock response" on invoke + And the FakeListLLM produces response "mock response" on invoke + + @unit @providers @registry + Scenario: _create_provider_instance builds stubbed OpenAI client Given I have a ProviderRegistry with OpenAI API key set And langchain OpenAI client is stubbed - When I call _create_provider_llm for provider "openai" with model "custom-openai" + When I call _create_provider_instance for provider "openai" with model "custom-openai" Then the stubbed ChatOpenAI should be created with model "custom-openai" @unit @providers @registry - Scenario Outline: _create_provider_llm builds client + Scenario Outline: _create_provider_instance builds client Given I have a ProviderRegistry with "" API key set And the "" LangChain client is stubbed - When I call _create_provider_llm for provider "" with model "" + When I call _create_provider_instance for provider "" with model "" Then the stubbed client should be created with argument "model" value "" Examples: @@ -326,92 +341,12 @@ Feature: Provider Registry Coverage | Cohere | cohere | command-cover | @unit @providers @registry - Scenario: _create_provider_llm builds Azure client with deployment override - Given I have a ProviderRegistry with "azure" API key set - And the "Azure" LangChain client is stubbed - When I call _create_provider_llm for provider "azure" with model "azure-cover" - Then the stubbed Azure client should be created with argument "deployment_name" value "azure-cover" - - @unit @providers @registry - Scenario: _create_provider_llm raises when Azure endpoint is missing - Given I have a ProviderRegistry with "azure" API key set - And the Azure endpoint setting is cleared - When I try to call _create_provider_llm for provider "azure" with model "azure-cover" - Then a provider registry ValueError should be raised - And the provider registry error should mention missing Azure endpoint - - - @unit @providers @registry - Scenario: _create_provider_llm builds OpenRouter client with ChatOpenAI - Given I have a ProviderRegistry with "openrouter" API key set - And the "openrouter" LangChain client is stubbed - When I call _create_provider_llm for provider "openrouter" with model "claude-openrouter-cover" - Then the stubbed openrouter client should be created with argument "model" value "claude-openrouter-cover" - And the stubbed openrouter client should be created with argument "openai_api_base" value "https://openrouter.ai/api/v1" - And the stubbed openrouter client should be created with argument "openai_api_key" value "sk-openrouter-test" - - @unit @providers @registry - Scenario: _create_provider_llm builds OpenRouter client with sanitized default headers - Given I have a ProviderRegistry with "openrouter" API key set - And the "openrouter" LangChain client is stubbed - And the OpenRouter default headers include numeric entries - When I call _create_provider_llm for provider "openrouter" with model "claude-openrouter-cover" and custom default headers - Then the stubbed OpenRouter client headers should include "123" value "456" - And the stubbed OpenRouter client headers should include "X-Debug" value "789" - And the stubbed openrouter client should be created with argument "openai_api_key" value "sk-openrouter-test" - And the stubbed OpenRouter client headers should not include key 123 - - @unit @providers @registry - Scenario: create_llm succeeds for OpenRouter and delegates to _create_provider_llm - Given I have a ProviderRegistry with "openrouter" API key set - And CLEVERAGENTS_DEFAULT_PROVIDER is not set - And CLEVERAGENTS_DEFAULT_MODEL is not set - And the provider registry LLM factory is stubbed - When I call create_llm for provider "openrouter" and model "claude-openrouter" - Then the stubbed LLM factory should be called for ProviderType.OPENROUTER with model "claude-openrouter" - - @unit @providers @registry - Scenario: _create_provider_llm adds organization headers for OpenRouter - Given I have a ProviderRegistry with "openrouter" API key set - And the OpenRouter organization setting is "my-org" - And the "openrouter" LangChain client is stubbed - When I call _create_provider_llm for provider "openrouter" with model "claude-openrouter-cover" - Then the stubbed OpenRouter client headers should include "HTTP-Referer" value "my-org" - And the stubbed OpenRouter client headers should include "X-Title" value "my-org" - - @unit @providers @registry - Scenario: _create_provider_llm uses default model for OpenRouter when model_id is None - Given I have a ProviderRegistry with "openrouter" API key set - And the "openrouter" LangChain client is stubbed - When I call _create_provider_llm for provider "openrouter" without specifying a model - Then the stubbed openrouter client should be created with argument "model" value "anthropic/claude-sonnet-4-20250514" - - @unit @providers @registry - Scenario: _create_provider_llm treats empty default_headers as None for OpenRouter - Given I have a ProviderRegistry with "openrouter" API key set - And the "openrouter" LangChain client is stubbed - When I call _create_provider_llm for provider "openrouter" with model "claude-openrouter-cover" and empty default headers - Then the stubbed openrouter client should not have default_headers - - @unit @providers @registry - Scenario: _create_provider_llm forwards extra kwargs to ChatOpenAI for OpenRouter - Given I have a ProviderRegistry with "openrouter" API key set - And the "openrouter" LangChain client is stubbed - When I call _create_provider_llm for provider "openrouter" with model "claude-openrouter-cover" and temperature 0.7 - Then the stubbed openrouter client should be created with argument "temperature" value "0.7" - - @unit @providers @registry - Scenario: _create_provider_llm raises error when OpenRouter API key is empty - Given I have a ProviderRegistry with no API keys - When I try to call _create_provider_llm for provider "openrouter" with model "claude-openrouter-cover" - Then a provider registry ValueError should be raised - And the provider registry error should mention missing "OPENROUTER_API_KEY" environment variable - - @unit @providers @registry - Scenario: _create_provider_llm raises error for unsupported provider - Given I have a ProviderRegistry instance - When I try to call _create_provider_llm for provider "mock" with model "mock-gpt" - Then a provider registry ValueError should be raised + Scenario: _create_provider_instance handles MOCK provider + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And I have a ProviderRegistry with no API keys + And the "mock" LangChain client is stubbed + When I call _create_provider_instance for provider "MOCK" with model "mock-gpt" + Then the stubbed mock client should be instantiated @unit @providers @registry Scenario: Create AI provider raises error when no provider configured @@ -434,12 +369,30 @@ Feature: Provider Registry Coverage Then a provider registry ValueError should be raised And the provider registry error should mention missing "GOOGLE_API_KEY" environment variable + @unit @providers @registry + Scenario: Create AI provider succeeds for MOCK provider without API key + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And I have a ProviderRegistry with no API keys + And CLEVERAGENTS_DEFAULT_MODEL is not set + When I create an AI provider for provider "mock" + Then the provider registry AI provider should be a LangChainChatProvider + And the provider registry AI provider name should be "mock" + And the provider registry AI provider model_id should be "mock-gpt" + + @unit @providers @registry + Scenario: Create AI provider for MOCK is rejected without env var + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is not set + And I have a ProviderRegistry with no API keys + When I try to create an AI provider for provider "mock" + Then a provider registry ValueError should be raised + And the provider registry error should mention "not allowed" + @unit @providers @registry Scenario: Create AI provider succeeds for configured provider string Given I have a ProviderRegistry with OpenAI API key set And CLEVERAGENTS_DEFAULT_MODEL is not set When I create an AI provider for provider "openai" - Then the provider registry AI provider should be an OpenAIChatProvider + Then the provider registry AI provider should be a LangChainChatProvider And the provider registry AI provider name should be "openai" And the provider registry AI provider model_id should be "gpt-4o" @@ -448,7 +401,7 @@ Feature: Provider Registry Coverage Given I have a ProviderRegistry with Anthropic API key set And CLEVERAGENTS_DEFAULT_MODEL is not set When I create an AI provider for provider "anthropic" - Then the provider registry AI provider should be an AnthropicChatProvider + Then the provider registry AI provider should be a LangChainChatProvider And the provider registry AI provider name should be "anthropic" And the provider registry AI provider model_id should be "claude-sonnet-4-20250514" @@ -471,7 +424,7 @@ Feature: Provider Registry Coverage Given I have a ProviderRegistry with "google" API key set And CLEVERAGENTS_DEFAULT_MODEL is not set When I create an AI provider for provider "google" - Then the provider registry AI provider should be a GoogleChatProvider + Then the provider registry AI provider should be a LangChainChatProvider And the provider registry AI provider name should be "google" And the provider registry AI provider model_id should be "gemini-2.0-flash" @@ -480,7 +433,7 @@ Feature: Provider Registry Coverage Given I have a ProviderRegistry with "openrouter" API key set And CLEVERAGENTS_DEFAULT_MODEL is not set When I create an AI provider for provider "openrouter" - Then the provider registry AI provider should be an OpenRouterChatProvider + Then the provider registry AI provider should be a LangChainChatProvider And the provider registry AI provider name should be "openrouter" And the provider registry AI provider model_id should be "anthropic/claude-sonnet-4-20250514" @@ -490,7 +443,7 @@ Feature: Provider Registry Coverage And CLEVERAGENTS_DEFAULT_PROVIDER is not set And CLEVERAGENTS_DEFAULT_MODEL is not set When I create an AI provider without specifying a provider - Then the provider registry AI provider should be an OpenAIChatProvider + Then the provider registry AI provider should be a LangChainChatProvider And the provider registry AI provider name should be "openai" @@ -509,6 +462,38 @@ Feature: Provider Registry Coverage And I call the AI provider llm factory with model "llama-mini-cover" Then the stubbed LLM factory should be called for ProviderType.GROQ with model "llama-mini-cover" + @unit @providers @registry + Scenario: _create_provider_instance handles MOCK provider and response list + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And I have a ProviderRegistry with no API keys + And the "mock" LangChain client is stubbed + When I call _create_provider_instance for provider "MOCK" with model "mock-gpt" + Then the stubbed mock client should be instantiated + And the stubbed mock client responses should be ["mock response"] * 10 + + @unit @providers @registry + Scenario: API key is forwarded through create_ai_provider closure + Given I have a ProviderRegistry with OpenAI API key set + And the provider registry LLM factory is stubbed + When I create an AI provider for provider "openai" + And I call the AI provider llm factory with model "gpt-mini" + Then the stubbed LLM factory should be called for ProviderType.OPENAI with model "gpt-mini" + And the stubbed LLM factory should be called with api_key present + + @unit @providers @registry + Scenario: resolve_provider_by_name rejects mock without env var + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is not set + When I call resolve_provider_by_name with "mock" + Then a provider registry ValueError should be raised + And the provider registry error should mention "not configured" + + @unit @providers @registry + Scenario: resolve_provider_by_name resolves mock with env var + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And I have a ProviderRegistry with no API keys + When I call resolve_provider_by_name with "mock" + Then the resolved provider should be a LangChainChatProvider + @unit @providers @registry Scenario: ProviderCapabilities is immutable Given I import the ProviderCapabilities class diff --git a/features/provider_registry_coverage_boost.feature b/features/provider_registry_coverage_boost.feature index 5b18fa5ac..41cb1b9e4 100644 --- a/features/provider_registry_coverage_boost.feature +++ b/features/provider_registry_coverage_boost.feature @@ -9,30 +9,26 @@ Feature: Provider Registry Coverage Boost And CLEVERAGENTS_DEFAULT_PROVIDER env var is cleared When I request the default provider type from the boost registry Then the boost result should be ProviderType ANTHROPIC - # Covers lines 335-338: settings_default valid ProviderType but not_configured @unit @providers @registry Scenario: Azure deployment resolves from settings when model_id is None Given a registry with azure API key and deployment setting "my-azure-deploy" And the Azure LangChain client is stubbed for boost - When I call _create_provider_llm for AZURE with model_id None via boost + When I call _create_provider_instance for AZURE with model_id None via boost Then the stubbed Azure client deployment_name should be "my-azure-deploy" - # Covers line 502: self._settings.azure_openai_deployment branch @unit @providers @registry Scenario: Azure deployment falls back to gpt-4o when settings deployment is also empty Given a registry with azure API key and no deployment setting And the Azure LangChain client is stubbed for boost - When I call _create_provider_llm for AZURE with model_id None via boost + When I call _create_provider_instance for AZURE with model_id None via boost Then the stubbed Azure client deployment_name should be "gpt-4o" - # Covers line 503: final "gpt-4o" fallback @unit @providers @registry Scenario: Google create_ai_provider uses DEFAULT_MODELS when model_id is empty string Given a registry with google API key configured for boost When I create an AI provider for google with empty string model_id via boost Then the boost AI provider model_id should be "gemini-2.0-flash" - # Covers line 620: DEFAULT_MODELS.get fallback in Google branch @unit @providers @registry Scenario: OpenRouter create_ai_provider raises error when API key is missing @@ -41,11 +37,37 @@ Feature: Provider Registry Coverage Boost Then a boost ValueError should be raised And the boost error message should contain "openrouter" And the boost error message should contain "not configured" - # Covers lines 632-637: OpenRouter not-configured error path @unit @providers @registry Scenario: OpenRouter create_ai_provider uses DEFAULT_MODELS when model_id is empty string Given a registry with openrouter API key configured for boost When I create an AI provider for openrouter with empty string model_id via boost Then the boost AI provider model_id should be "anthropic/claude-sonnet-4-20250514" - # Covers lines 643-644: DEFAULT_MODELS.get fallback in OpenRouter branch + + @unit @providers @registry + Scenario: OpenRouter _create_provider_instance with sanitized numeric headers and organization + Given a registry with openrouter API key configured for boost + And the openrouter organization is set to "my-org" + When I call _create_provider_instance for openrouter with model "claude-or" and numeric headers + Then the returned LLM should be recorded with sanitized headers including "X-Debug" + And the openrouter kwargs should include HTTP-Referer "my-org" + + @unit @providers @registry + Scenario: _create_provider_instance accepts pre-validated api_key sentinel None + Given CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set + And a registry with no API keys for boost + When I call _create_provider_instance for mock with api_key sentinel None + Then the boost result should be a FakeListLLM instance + And calling it twice yields the same response + + @unit @providers @registry + Scenario: Azure endpoint missing raises ValueError + Given a registry with azure API key but no endpoint setting + When I try to call _create_provider_instance for AZURE with model_id None via boost + Then a boost ValueError should be raised + And the boost error message should contain "endpoint" + + @unit @providers @registry + Scenario: coerce_optional_str coerces non-string values + When I coerce optional string with integer 42 + Then the coerced optional string result should be "42" diff --git a/features/steps/provider_registry_coverage_boost_steps.py b/features/steps/provider_registry_coverage_boost_steps.py index b7605e4f0..c6ca40ed8 100644 --- a/features/steps/provider_registry_coverage_boost_steps.py +++ b/features/steps/provider_registry_coverage_boost_steps.py @@ -24,6 +24,9 @@ from cleveragents.providers.registry import ( # --------------------------------------------------------------------------- +_AZURE_ENDPOINT_DEFAULT = object() + + def _make_boost_settings( openai: str | None = None, anthropic: str | None = None, @@ -36,7 +39,7 @@ def _make_boost_settings( together: str | None = None, default_provider: str | None = None, default_model: str | None = None, - azure_endpoint: str | None = None, + azure_endpoint: str | None = _AZURE_ENDPOINT_DEFAULT, azure_api_version: str | None = None, azure_deployment: str | None = None, openrouter_org: str | None = None, @@ -54,8 +57,8 @@ def _make_boost_settings( settings.together_api_key = together settings.default_provider = default_provider settings.default_model = default_model - if azure and not azure_endpoint: - azure_endpoint = "https://example.openai.azure.com" + if azure_endpoint is _AZURE_ENDPOINT_DEFAULT: + azure_endpoint = "https://example.openai.azure.com" if azure else None settings.azure_openai_endpoint = azure_endpoint settings.azure_openai_api_version = azure_api_version settings.azure_openai_deployment = azure_deployment @@ -168,6 +171,18 @@ def step_boost_openrouter_registry(context: Any) -> None: ) +@given('the openrouter organization is set to "{org}"') +def step_boost_openrouter_org(context: Any, org: str) -> None: + context.boost_registry._settings.openrouter_organization = org + + +@given("a registry with azure API key but no endpoint setting") +def step_boost_azure_no_endpoint(context: Any) -> None: + context.boost_registry = ProviderRegistry( + settings=_make_boost_settings(azure="sk-azure-test", azure_endpoint=None) + ) + + # --------------------------------------------------------------------------- # When # --------------------------------------------------------------------------- @@ -178,13 +193,63 @@ def step_boost_get_default_provider_type(context: Any) -> None: context.boost_result = context.boost_registry.get_default_provider_type() -@when("I call _create_provider_llm for AZURE with model_id None via boost") -def step_boost_create_azure_llm_no_model(context: Any) -> None: - context.boost_result = context.boost_registry._create_provider_llm( +@when("I call _create_provider_instance for AZURE with model_id None via boost") +def step_boost_create_azure_instance_no_model(context: Any) -> None: + context.boost_result = context.boost_registry._create_provider_instance( ProviderType.AZURE, None ) +@when( + 'I call _create_provider_instance for openrouter with model "{model}" and numeric headers' +) +def step_boost_create_openrouter_with_headers(context: Any, model: str) -> None: + original = sys.modules.get("langchain_openai") + calls: list[dict[str, Any]] = [] + + class FakeChatOpenAI: + def __init__(self, **kwargs: Any) -> None: + calls.append(kwargs) + + stub = types.ModuleType("langchain_openai") + stub.ChatOpenAI = FakeChatOpenAI + stub.AzureChatOpenAI = lambda **_: None + sys.modules["langchain_openai"] = stub + + context._boost_openrouter_calls = calls + + def _restore() -> None: + if original is None: + sys.modules.pop("langchain_openai", None) + else: + sys.modules["langchain_openai"] = original + + context._boost_cleanup = getattr(context, "_boost_cleanup", []) + context._boost_cleanup.append(_restore) + + context.boost_result = context.boost_registry._create_provider_instance( + ProviderType.OPENROUTER, + model, + default_headers={123: 456, "X-Debug": 789}, + ) + + +@when("I call _create_provider_instance for mock with api_key sentinel None") +def step_boost_create_mock_with_sentinel(context: Any) -> None: + context.boost_result = context.boost_registry._create_provider_instance( + ProviderType.MOCK, None, __api_key_sentinel=None + ) + + +@when("I try to call _create_provider_instance for AZURE with model_id None via boost") +def step_boost_try_create_azure_no_model(context: Any) -> None: + try: + context.boost_registry._create_provider_instance(ProviderType.AZURE, None) + context.boost_error = None + except ValueError as exc: + context.boost_error = exc + + @when("I create an AI provider for google with empty string model_id via boost") def step_boost_google_ai_provider_empty_model(context: Any) -> None: # Ensure env vars don't interfere @@ -215,6 +280,13 @@ def step_boost_openrouter_ai_provider_empty_model(context: Any) -> None: ) +@when("I coerce optional string with integer {value:d}") +def step_boost_coerce_optional_str(context: Any, value: int) -> None: + from cleveragents.providers.registry import _coerce_optional_str + + context.coerced_value = _coerce_optional_str(value) + + # --------------------------------------------------------------------------- # Then # --------------------------------------------------------------------------- @@ -256,3 +328,51 @@ def step_boost_error_contains(context: Any, fragment: str) -> None: assert fragment.lower() in message, ( f"Expected error to contain {fragment!r}, got: {message!r}" ) + + +@then('the returned LLM should be recorded with sanitized headers including "{header}"') +def step_boost_openrouter_sanitized_headers(context: Any, header: str) -> None: + calls = getattr(context, "_boost_openrouter_calls", []) + assert calls, "Expected ChatOpenAI to be recorded" + kwargs = calls[-1] + headers = kwargs.get("default_headers", {}) + assert header in headers, f"Expected {header!r} in headers, got {headers!r}" + # Verify numeric keys were coerced to strings + assert "123" in headers, "Expected key '123' (coerced from int) in headers" + + +@then('the openrouter kwargs should include HTTP-Referer "{expected}"') +def step_boost_openrouter_referer(context: Any, expected: str) -> None: + calls = getattr(context, "_boost_openrouter_calls", []) + assert calls, "Expected ChatOpenAI to be recorded" + kwargs = calls[-1] + headers = kwargs.get("default_headers", {}) + assert headers.get("HTTP-Referer") == expected, ( + f"Expected HTTP-Referer={expected!r}, got {headers.get('HTTP-Referer')!r}" + ) + + +@then("the boost result should be a FakeListLLM instance") +def step_boost_result_is_fake_list_llm(context: Any) -> None: + from langchain_community.llms import FakeListLLM + + assert isinstance(context.boost_result, FakeListLLM), ( + f"Expected FakeListLLM, got {type(context.boost_result)}" + ) + + +@then("calling it twice yields the same response") +def step_boost_llm_invokes_twice(context: Any) -> None: + llm = context.boost_result + response1 = llm.invoke("test prompt") + response2 = llm.invoke("test prompt 2") + assert response1 == response2 == "mock response", ( + f"Expected 'mock response' for both, got {response1!r} and {response2!r}" + ) + + +@then('the coerced optional string result should be "{expected}"') +def step_boost_coerced_result(context: Any, expected: str) -> None: + assert getattr(context, "coerced_value", None) == expected, ( + f"Expected {expected!r}, got {getattr(context, 'coerced_value', None)!r}" + ) diff --git a/features/steps/provider_registry_steps.py b/features/steps/provider_registry_steps.py index c6736fbb8..cd4f53f6d 100644 --- a/features/steps/provider_registry_steps.py +++ b/features/steps/provider_registry_steps.py @@ -9,11 +9,7 @@ from unittest.mock import MagicMock from behave import given, then, when # type: ignore[import-untyped] -from cleveragents.providers.llm.anthropic_provider import AnthropicChatProvider -from cleveragents.providers.llm.google_provider import GoogleChatProvider from cleveragents.providers.llm.langchain_chat_provider import LangChainChatProvider -from cleveragents.providers.llm.openai_provider import OpenAIChatProvider -from cleveragents.providers.llm.openrouter_provider import OpenRouterChatProvider from cleveragents.providers.registry import ( ProviderCapabilities, ProviderInfo, @@ -139,6 +135,7 @@ LANGCHAIN_CLIENT_SPECS: dict[str, tuple[str, str]] = { "groq": ("langchain_groq", "ChatGroq"), "together": ("langchain_together", "ChatTogether"), "cohere": ("langchain_cohere", "ChatCohere"), + "mock": ("langchain_community.llms", "FakeListLLM"), } @@ -301,6 +298,18 @@ def step_impl_unset_default_model_env(context: Any) -> None: os.environ.pop("CLEVERAGENTS_DEFAULT_MODEL", None) +@given("CLEVERAGENTS_ALLOW_MOCK_PROVIDER is set") +def step_impl_allow_mock_provider_set(context: Any) -> None: + context.original_allow_mock_env = os.environ.get("CLEVERAGENTS_ALLOW_MOCK_PROVIDER") + os.environ["CLEVERAGENTS_ALLOW_MOCK_PROVIDER"] = "true" + + +@given("CLEVERAGENTS_ALLOW_MOCK_PROVIDER is not set") +def step_impl_allow_mock_provider_unset(context: Any) -> None: + context.original_allow_mock_env = os.environ.get("CLEVERAGENTS_ALLOW_MOCK_PROVIDER") + os.environ.pop("CLEVERAGENTS_ALLOW_MOCK_PROVIDER", None) + + @given("langchain OpenAI client is stubbed") def step_impl_stub_langchain_openai(context: Any) -> None: module_name, class_name = LANGCHAIN_CLIENT_SPECS["openai"] @@ -326,7 +335,9 @@ def step_impl_stub_named_langchain_client(context: Any, provider_name: str) -> N @given("the provider registry LLM factory is stubbed") def step_impl_stub_provider_factory(context: Any) -> None: context.stubbed_llm = object() - context.registry._create_provider_llm = MagicMock(return_value=context.stubbed_llm) + context.registry._create_provider_instance = MagicMock( + return_value=context.stubbed_llm + ) # -- When ------------------------------------------------------------------- @@ -337,12 +348,6 @@ def step_impl_create_registry_default(context: Any) -> None: context.registry = context.ProviderRegistry() -@when("I coerce optional string with integer {value:d}") -def step_impl_coerce_optional_string_with_integer(context: Any, value: int) -> None: - coerce_helper = getattr(context, "coerce_optional_str", _coerce_optional_str) - context.coerced_value = coerce_helper(value) - - @when("I create a ProviderCapabilities with all fields") def step_impl_capabilities_all_fields(context: Any) -> None: context.capabilities = context.ProviderCapabilities( @@ -498,67 +503,75 @@ def step_impl_call_create_llm_no_model(context: Any, value: str) -> None: context.created_llm = context.registry.create_llm(provider_type=value) -@when('I call _create_provider_llm for provider "{value}" with model "{model}"') -def step_impl_call_private_llm(context: Any, value: str, model: str) -> None: +@when('I call _create_provider_instance for provider "{value}" with model "{model}"') +def step_impl_call_private_instance(context: Any, value: str, model: str) -> None: provider_enum = _ensure_provider_type(context) provider_type = getattr(provider_enum, value.upper()) - context.created_llm = context.registry._create_provider_llm(provider_type, model) + context.created_llm = context.registry._create_provider_instance( + provider_type, model + ) @when( - 'I call _create_provider_llm for provider "{value}" with model "{model}" and custom default headers' + 'I call _create_provider_instance for provider "{value}" with model "{model}" and custom default headers' ) -def step_impl_call_private_llm_with_headers( +def step_impl_call_private_instance_with_headers( context: Any, value: str, model: str ) -> None: provider_enum = _ensure_provider_type(context) provider_type = getattr(provider_enum, value.upper()) default_headers = getattr(context, "openrouter_default_headers", None) assert default_headers is not None, "OpenRouter default headers were not configured" - context.created_llm = context.registry._create_provider_llm( + context.created_llm = context.registry._create_provider_instance( provider_type, model, default_headers=default_headers ) -@when('I call _create_provider_llm for provider "{value}" without specifying a model') -def step_impl_call_private_llm_no_model(context: Any, value: str) -> None: +@when( + 'I call _create_provider_instance for provider "{value}" without specifying a model' +) +def step_impl_call_private_instance_no_model(context: Any, value: str) -> None: provider_enum = _ensure_provider_type(context) provider_type = getattr(provider_enum, value.upper()) - context.created_llm = context.registry._create_provider_llm(provider_type, None) + context.created_llm = context.registry._create_provider_instance( + provider_type, None + ) @when( - 'I call _create_provider_llm for provider "{value}" with model "{model}" and empty default headers' + 'I call _create_provider_instance for provider "{value}" with model "{model}" and empty default headers' ) -def step_impl_call_private_llm_empty_headers( +def step_impl_call_private_instance_empty_headers( context: Any, value: str, model: str ) -> None: provider_enum = _ensure_provider_type(context) provider_type = getattr(provider_enum, value.upper()) - context.created_llm = context.registry._create_provider_llm( + context.created_llm = context.registry._create_provider_instance( provider_type, model, default_headers={} ) @when( - 'I call _create_provider_llm for provider "{value}" with model "{model}" and temperature {temperature}' + 'I call _create_provider_instance for provider "{value}" with model "{model}" and temperature {temperature}' ) -def step_impl_call_private_llm_with_temperature( +def step_impl_call_private_instance_with_temperature( context: Any, value: str, model: str, temperature: str ) -> None: provider_enum = _ensure_provider_type(context) provider_type = getattr(provider_enum, value.upper()) - context.created_llm = context.registry._create_provider_llm( + context.created_llm = context.registry._create_provider_instance( provider_type, model, temperature=float(temperature) ) -@when('I try to call _create_provider_llm for provider "{value}" with model "{model}"') -def step_impl_try_private_llm(context: Any, value: str, model: str) -> None: +@when( + 'I try to call _create_provider_instance for provider "{value}" with model "{model}"' +) +def step_impl_try_private_instance(context: Any, value: str, model: str) -> None: provider_enum = _ensure_provider_type(context) provider_type = getattr(provider_enum, value.upper()) try: - context.registry._create_provider_llm(provider_type, model) + context.registry._create_provider_instance(provider_type, model) context.error = None except Exception as exc: # pragma: no cover - defensive path context.error = exc @@ -630,13 +643,6 @@ def step_impl_registry_has_providers(context: Any) -> None: assert len(context.registry.get_all_providers()) > 0 -@then('the coerced optional string result should be "{expected}"') -def step_impl_coerced_optional_string_result(context: Any, expected: str) -> None: - assert getattr(context, "coerced_value", None) == expected, ( - f"Expected coerced value {expected!r}, got {getattr(context, 'coerced_value', None)!r}" - ) - - @then('ProviderType should have {enum_name} value "{value}"') def step_impl_provider_type_values(context: Any, enum_name: str, value: str) -> None: enum_member = getattr(_ensure_provider_type(context), enum_name.upper()) @@ -816,24 +822,14 @@ def step_impl_ai_provider_is_langchain(context: Any) -> None: assert isinstance(context.ai_provider, LangChainChatProvider) -@then("the provider registry AI provider should be an OpenAIChatProvider") -def step_impl_ai_provider_is_openai(context: Any) -> None: - assert isinstance(context.ai_provider, OpenAIChatProvider) - - -@then("the provider registry AI provider should be an AnthropicChatProvider") -def step_impl_ai_provider_is_anthropic(context: Any) -> None: - assert isinstance(context.ai_provider, AnthropicChatProvider) - - -@then("the provider registry AI provider should be a GoogleChatProvider") -def step_impl_ai_provider_is_google(context: Any) -> None: - assert isinstance(context.ai_provider, GoogleChatProvider) - - -@then("the provider registry AI provider should be an OpenRouterChatProvider") -def step_impl_ai_provider_is_openrouter(context: Any) -> None: - assert isinstance(context.ai_provider, OpenRouterChatProvider) +@then("the stubbed mock client should be instantiated") +def step_impl_stubbed_mock_client_instantiated(context: Any) -> None: + stubbed_clients = getattr(context, "stubbed_clients", {}) + client_data = stubbed_clients.get("mock") + assert client_data, "No stubbed client recorded for mock" + calls = client_data["calls"] + assert calls, "Expected FakeListLLM to be instantiated" + assert isinstance(context.created_llm, client_data["class"]) @then('the provider registry AI provider name should be "{value}"') @@ -854,8 +850,8 @@ def step_impl_stubbed_llm_factory_called( ) -> None: provider_enum = _ensure_provider_type(context) expected_type = getattr(provider_enum, enum_name.upper()) - context.registry._create_provider_llm.assert_called_once() - args, kwargs = context.registry._create_provider_llm.call_args + context.registry._create_provider_instance.assert_called_once() + args, kwargs = context.registry._create_provider_instance.call_args assert args[:2] == (expected_type, model) if kwargs: # max_retries is propagated by create_ai_provider to keep retry semantics aligned @@ -863,6 +859,10 @@ def step_impl_stubbed_llm_factory_called( assert max_retries is None or isinstance(max_retries, int), ( f"Expected max_retries int or None, got {max_retries!r}" ) + # api_key should be forwarded (even None) so _get_api_key is not called twice + assert "__api_key_sentinel" in kwargs or "api_key" in kwargs, ( + f"Expected api_key or __api_key_sentinel in factory kwargs, got {sorted(kwargs.keys())!r}" + ) assert context.created_llm is context.stubbed_llm @@ -999,6 +999,100 @@ def step_impl_default_models(context: Any, provider: str, model: str) -> None: assert context.default_models[enum_member] == model -# -- Hooks ------------------------------------------------------------------ +@then('the provider registry error should mention "{message}"') +def step_impl_error_mentions_message(context: Any, message: str) -> None: + assert context.error is not None + assert message.lower() in str(context.error).lower(), ( + f"Expected error to mention {message!r}, got {str(context.error)!r}" + ) + + +@then("the created LLM should be a FakeListLLM instance") +def step_impl_created_llm_is_fake_list_llm(context: Any) -> None: + from langchain_community.llms import FakeListLLM + + assert isinstance(context.created_llm, FakeListLLM), ( + f"Expected FakeListLLM, got {type(context.created_llm)}" + ) + + +@then("the provider registry result should contain only the MOCK provider") +def step_impl_result_contains_only_mock(context: Any) -> None: + provider_types = [p.provider_type for p in context.result] + provider_enum = _ensure_provider_type(context) + assert provider_types == [provider_enum.MOCK], ( + f"Expected only MOCK, got {provider_types}" + ) + + +@then("the provider registry result should contain the MOCK provider") +def step_impl_result_contains_mock(context: Any) -> None: + provider_types = [p.provider_type for p in context.result] + provider_enum = _ensure_provider_type(context) + assert provider_enum.MOCK in provider_types, ( + f"Expected MOCK in list, got {provider_types}" + ) + + +@then("the stubbed {provider_name} client responses should be {expected_responses}") +def step_impl_stubbed_client_responses( + context: Any, provider_name: str, expected_responses: str +) -> None: + provider_key = provider_name.lower() + stubbed_clients = getattr(context, "stubbed_clients", {}) + assert provider_key in stubbed_clients, ( + f"No stubbed client recorded for {provider_name}" + ) + client_data = stubbed_clients[provider_key] + calls = client_data["calls"] + assert calls, f"Expected {provider_name} client to be instantiated" + last_call = calls[-1] + actual = last_call["kwargs"].get("responses") + if actual is None: + actual = last_call.get("args") + expected = eval(expected_responses) # e.g. ["mock response"] * 10 + assert actual == expected, f"Expected responses={expected!r}, got {actual!r}" + assert isinstance(context.created_llm, client_data["class"]) + + +@then("the stubbed LLM factory should be called with api_key present") +def step_impl_stubbed_llm_factory_api_key(context: Any) -> None: + context.registry._create_provider_instance.assert_called() + _args, kwargs = context.registry._create_provider_instance.call_args + assert "__api_key_sentinel" in kwargs or "api_key" in kwargs, ( + f"Expected api_key or __api_key_sentinel in factory kwargs, got {sorted(kwargs.keys())!r}" + ) + + +@then('the FakeListLLM produces response "{response}" on invoke') +def step_impl_fake_list_llm_invokes(context: Any, response: str) -> None: + from langchain_community.llms import FakeListLLM + + llm = context.created_llm + assert isinstance(llm, FakeListLLM), f"Expected FakeListLLM, got {type(llm)}" + assert llm.invoke("test prompt") == response, ( + f"Expected FakeListLLM to produce {response!r}" + ) + + +@when('I call resolve_provider_by_name with "{name}"') +def step_impl_resolve_provider_by_name(context: Any, name: str) -> None: + from cleveragents.providers.registry import ( + resolve_provider_by_name, + reset_provider_registry, + ) + + reset_provider_registry() + try: + context.resolved_provider = resolve_provider_by_name(name) + context.error = None + except ValueError as exc: + context.error = exc + + +@then("the resolved provider should be a LangChainChatProvider") +def step_impl_resolved_provider_is_langchain(context: Any) -> None: + assert isinstance(context.resolved_provider, LangChainChatProvider) + # Hook logic lives in features/environment.py to ensure scenarios reset state. diff --git a/src/cleveragents/providers/registry.py b/src/cleveragents/providers/registry.py index 39dea639b..5aafdfe85 100644 --- a/src/cleveragents/providers/registry.py +++ b/src/cleveragents/providers/registry.py @@ -39,6 +39,21 @@ def _coerce_optional_str(value: object | None) -> str | None: return str(value) +def _coerce_env_bool(value: str) -> bool: + """Consistent boolean coercion for environment variables. + + Accepts ``true``, ``1``, ``yes``, and ``on`` (case-insensitive) as truthy. + """ + return value.strip().lower() in ("true", "1", "yes", "on") + + +class _ApiKeyMissing: + """Sentinel distinguishing "api_key was not provided" from ``None``.""" + + +_API_KEY_MISSING = _ApiKeyMissing() + + class ProviderType(StrEnum): """Supported AI provider types.""" @@ -165,7 +180,7 @@ class ProviderRegistry: supports_json_mode=False, ), ProviderType.MOCK: ProviderCapabilities( - supports_streaming=True, + supports_streaming=False, supports_tool_calls=False, supports_vision=False, max_context_length=4096, @@ -240,6 +255,25 @@ class ProviderRegistry: ) self._providers[provider_type] = provider_info + # Register MOCK with conditional ``is_configured``. The guard field + # ``api_key_env_var`` points to ``CLEVERAGENTS_ALLOW_MOCK_PROVIDER`` + # semantically (it is the activation flag) even though it is not a + # traditional API key. + mock_allowed = _coerce_env_bool( + os.environ.get("CLEVERAGENTS_ALLOW_MOCK_PROVIDER", "") + ) + mock_info = ProviderInfo( + provider_type=ProviderType.MOCK, + name=ProviderType.MOCK.value.title(), + api_key_env_var="CLEVERAGENTS_ALLOW_MOCK_PROVIDER", + default_model=self.DEFAULT_MODELS.get(ProviderType.MOCK, "unknown"), + capabilities=self.DEFAULT_CAPABILITIES.get( + ProviderType.MOCK, ProviderCapabilities() + ), + is_configured=mock_allowed, + ) + self._providers[ProviderType.MOCK] = mock_info + def get_configured_providers(self) -> list[ProviderInfo]: """Get list of all configured providers. @@ -401,45 +435,30 @@ class ProviderRegistry: return self.DEFAULT_MODELS.get(provider_type) - def create_llm( - self, - provider_type: ProviderType | str | None = None, - model_id: str | None = None, - **kwargs: object, - ) -> BaseLanguageModel: - """Create a LangChain LLM instance for a provider. + def _get_api_key(self, provider_type: ProviderType) -> str | None: + """Get and validate the API key for a provider. Args: - provider_type: The provider type. Uses default if not specified. - model_id: The model ID. Uses default for provider if not specified. - **kwargs: Additional arguments passed to the LLM constructor. + provider_type: The provider type to look up. Returns: - A LangChain BaseLanguageModel instance. + The API key string, or ``None`` if the provider has no key + requirement (e.g. ``MOCK`` when explicitly allowed). Raises: - ValueError: If no provider is configured or provider is invalid. + ValueError: If the provider requires an API key but it is not + configured, or if ``MOCK`` is requested without the + ``CLEVERAGENTS_ALLOW_MOCK_PROVIDER`` environment variable, + or if the provider type has no API key mapping. """ - # Resolve provider type - if provider_type is None: - provider_type = self.get_default_provider_type() - if provider_type is None: - raise ValueError( - "No AI provider configured. Please set one of the following " - "environment variables: OPENAI_API_KEY, ANTHROPIC_API_KEY, " - "GOOGLE_API_KEY, or another supported provider key." - ) - elif not isinstance(provider_type, ProviderType): - try: - provider_type = ProviderType(provider_type.lower()) - except ValueError as e: - raise ValueError(f"Unknown provider type: {provider_type}") from e + if provider_type == ProviderType.MOCK: + if _coerce_env_bool(os.environ.get("CLEVERAGENTS_ALLOW_MOCK_PROVIDER", "")): + return None + raise ValueError( + "Mock provider is not allowed in production. " + "Set CLEVERAGENTS_ALLOW_MOCK_PROVIDER=true to enable it." + ) - # Resolve model ID - if model_id is None: - model_id = self.get_default_model(provider_type) - - # Get API key key_attr = self.PROVIDER_KEY_ATTRS.get(provider_type) if key_attr: api_key = getattr(self._settings, key_attr, None) @@ -448,37 +467,62 @@ class ProviderRegistry: f"Provider {provider_type.value} is not configured. " f"Please set the {key_attr.upper()} environment variable." ) + return api_key - # Create the appropriate LLM - return self._create_provider_llm(provider_type, model_id, **kwargs) + # ProviderType is in the enum but not in PROVIDER_KEY_ATTRS + raise ValueError( + f"Provider {provider_type.value} has no API key mapping in " + "PROVIDER_KEY_ATTRS." + ) - def _create_provider_llm( + def _create_provider_instance( self, provider_type: ProviderType, model_id: str | None, **kwargs: Any, ) -> BaseLanguageModel: - """Create a LangChain LLM for the specified provider. + """Single internal factory for provider instantiation. + + This is the single source of truth for raw LLM instantiation. Both + ``create_llm()`` and ``create_ai_provider()`` delegate here so that + adding a new provider requires touching exactly one place. Args: provider_type: The provider type. - model_id: The model ID. - **kwargs: Additional arguments for the LLM. + model_id: The model ID (provider default used when ``None``). + **kwargs: Extra constructor arguments forwarded to the LLM. + If ``api_key`` is present, it is consumed here instead of + being fetched from settings. ``max_retries`` is consumed + if present and never forwarded to LangChain constructors. Returns: - A LangChain BaseLanguageModel instance. + A ``BaseLanguageModel`` instance for the requested provider. + + Raises: + ValueError: If the provider is not configured or not supported. """ - # Import the appropriate LangChain module based on provider + # Consume retry metadata so it never leaks into provider constructors. + kwargs.pop("max_retries", None) + + # If the caller explicitly passed api_key (even ``None``), use it + # directly; otherwise fall back to _get_api_key. + sentinel = kwargs.pop("__api_key_sentinel", _API_KEY_MISSING) + if sentinel is _API_KEY_MISSING: + api_key: str | None = self._get_api_key(provider_type) + else: + api_key = sentinel + if provider_type == ProviderType.OPENAI: from langchain_openai import ChatOpenAI - return ChatOpenAI(model=model_id or "gpt-4o", **kwargs) + return ChatOpenAI(model=model_id or "gpt-4o", api_key=api_key, **kwargs) if provider_type == ProviderType.ANTHROPIC: from langchain_anthropic import ChatAnthropic return ChatAnthropic( model=model_id or "claude-sonnet-4-20250514", + api_key=api_key, **kwargs, ) @@ -487,6 +531,7 @@ class ProviderRegistry: return ChatGoogleGenerativeAI( model=model_id or "gemini-2.0-flash", + google_api_key=api_key, **kwargs, ) @@ -522,6 +567,7 @@ class ProviderRegistry: deployment_name=deployment_name, azure_endpoint=azure_endpoint, api_version=api_version, + api_key=api_key, **kwargs, ) @@ -530,6 +576,7 @@ class ProviderRegistry: return ChatGroq( model=model_id or "llama-3.1-70b-versatile", + api_key=api_key, **kwargs, ) @@ -538,6 +585,7 @@ class ProviderRegistry: return ChatTogether( model=model_id or "meta-llama/Llama-3.1-70B-Instruct-Turbo", + api_key=api_key, **kwargs, ) @@ -546,17 +594,13 @@ class ProviderRegistry: return ChatCohere( model=model_id or "command-r-plus", + api_key=api_key, **kwargs, ) if provider_type == ProviderType.OPENROUTER: from langchain_openai import ChatOpenAI - api_key = self._settings.openrouter_api_key - if not api_key: - raise ValueError( - "Provider openrouter requires OPENROUTER_API_KEY to be set." - ) raw_headers = kwargs.pop("default_headers", None) sanitized_headers: dict[str, str] | None = ( {str(k): str(v) for k, v in raw_headers.items()} @@ -581,8 +625,68 @@ class ProviderRegistry: openrouter_kwargs.update(kwargs) return ChatOpenAI(**openrouter_kwargs) + if provider_type == ProviderType.MOCK: + from langchain_community.llms import FakeListLLM + + return FakeListLLM(responses=["mock response"] * 10) + raise ValueError(f"Unsupported provider type: {provider_type}") + def _resolve_provider_type( + self, + provider_type: ProviderType | str | None = None, + ) -> ProviderType: + """Resolve a possibly ``None`` or string provider type to ``ProviderType``. + + Uses the configured fallback chain when ``None`` and performs + case-insensitive string matching otherwise. + + Raises: + ValueError: If no provider is configured or the name is unknown. + """ + if provider_type is None: + resolved = self.get_default_provider_type() + if resolved is None: + raise ValueError( + "No AI provider configured. Please set one of the following " + "environment variables: OPENAI_API_KEY, ANTHROPIC_API_KEY, " + "GOOGLE_API_KEY, or another supported provider key." + ) + return resolved + if not isinstance(provider_type, ProviderType): + try: + return ProviderType(provider_type.lower()) + except ValueError as e: + raise ValueError(f"Unknown provider type: {provider_type}") from e + return provider_type + + def create_llm( + self, + provider_type: ProviderType | str | None = None, + model_id: str | None = None, + **kwargs: Any, + ) -> BaseLanguageModel: + """Create a LangChain LLM instance for a provider. + + Args: + provider_type: The provider type. Uses default if not specified. + model_id: The model ID. Uses default for provider if not specified. + **kwargs: Additional arguments passed to the LLM constructor. + + Returns: + A LangChain BaseLanguageModel instance. + + Raises: + ValueError: If no provider is configured or provider is invalid. + """ + resolved = self._resolve_provider_type(provider_type) + + if model_id is None: + model_id = self.get_default_model(resolved) + + # Delegate to the unified factory (API key validation happens inside) + return self._create_provider_instance(resolved, model_id, **kwargs) + def create_ai_provider( self, provider_type: ProviderType | str | None = None, @@ -591,8 +695,9 @@ class ProviderRegistry: ) -> AIProviderInterface: """Create an AIProviderInterface instance for a provider. - This creates a LangChainChatProvider that wraps the LangChain LLM - with the AIProviderInterface protocol. + All providers are wrapped uniformly in ``LangChainChatProvider``. + Raw LLM instantiation is delegated to ``_create_provider_instance()`` + so that adding a new provider requires touching exactly one place. Args: provider_type: The provider type. Uses default if not specified. @@ -609,135 +714,34 @@ class ProviderRegistry: LangChainChatProvider, ) - # Resolve types - if provider_type is None: - provider_type = self.get_default_provider_type() - if provider_type is None: - raise ValueError( - "No AI provider configured. Please set one of the following " - "environment variables: OPENAI_API_KEY, ANTHROPIC_API_KEY, " - "GOOGLE_API_KEY, or another supported provider key." - ) - elif not isinstance(provider_type, ProviderType): - try: - provider_type = ProviderType(provider_type.lower()) - except ValueError as e: - raise ValueError(f"Unknown provider type: {provider_type}") from e + resolved = self._resolve_provider_type(provider_type) if model_id is None: - model_id = self.get_default_model(provider_type) + model_id = self.get_default_model(resolved) - provider_info = self.get_provider_info(provider_type) + provider_info = self.get_provider_info(resolved) capabilities = ( provider_info.capabilities if provider_info else ProviderCapabilities() ) - if provider_type == ProviderType.OPENAI: - from cleveragents.providers.llm.openai_provider import OpenAIChatProvider - - key_attr = self.PROVIDER_KEY_ATTRS.get(provider_type) - api_key = getattr(self._settings, key_attr, None) if key_attr else None - if not api_key: - missing_env = ( - key_attr.upper() if key_attr else provider_type.value.upper() - ) - raise ValueError( - f"Provider {provider_type.value} is not configured. " - f"Please set the {missing_env} environment variable." - ) - - return OpenAIChatProvider( - api_key=api_key, - model=model_id or self.DEFAULT_MODELS.get(provider_type, "gpt-4o"), - max_retries=max_retries, - ) - - if provider_type == ProviderType.ANTHROPIC: - from cleveragents.providers.llm.anthropic_provider import ( - AnthropicChatProvider, - ) - - key_attr = self.PROVIDER_KEY_ATTRS.get(provider_type) - api_key = getattr(self._settings, key_attr, None) if key_attr else None - if not api_key: - missing_env = ( - key_attr.upper() if key_attr else provider_type.value.upper() - ) - raise ValueError( - f"Provider {provider_type.value} is not configured. " - f"Please set the {missing_env} environment variable." - ) - - return AnthropicChatProvider( - api_key=api_key, - model=model_id - or self.DEFAULT_MODELS.get(provider_type, "claude-sonnet-4-20250514"), - max_retries=max_retries, - ) - - if provider_type == ProviderType.GOOGLE: - from cleveragents.providers.llm.google_provider import GoogleChatProvider - - key_attr = self.PROVIDER_KEY_ATTRS.get(provider_type) - api_key = getattr(self._settings, key_attr, None) if key_attr else None - if not api_key: - missing_env = ( - key_attr.upper() if key_attr else provider_type.value.upper() - ) - raise ValueError( - f"Provider {provider_type.value} is not configured. " - f"Please set the {missing_env} environment variable." - ) - - return GoogleChatProvider( - api_key=api_key, - model=model_id - or self.DEFAULT_MODELS.get(provider_type, "gemini-2.0-flash"), - max_retries=max_retries, - ) - - if provider_type == ProviderType.OPENROUTER: - from cleveragents.providers.llm.openrouter_provider import ( - OpenRouterChatProvider, - ) - - key_attr = self.PROVIDER_KEY_ATTRS.get(provider_type) - api_key = getattr(self._settings, key_attr, None) if key_attr else None - if not api_key: - missing_env = ( - key_attr.upper() if key_attr else provider_type.value.upper() - ) - raise ValueError( - f"Provider {provider_type.value} is not configured. " - f"Please set the {missing_env} environment variable." - ) - - return OpenRouterChatProvider( - api_key=api_key, - model=model_id - or self.DEFAULT_MODELS.get( - provider_type, "anthropic/claude-sonnet-4-20250514" - ), - organization=self._settings.openrouter_organization, - max_retries=max_retries, - ) - - # Create factory function for the LLM. - # Capture the narrowed ProviderType in a local variable so the closure - # holds a ProviderType rather than the wider ProviderType | str | None. - resolved_provider_type: ProviderType = provider_type + # Eager validation: fail fast before building the factory closure, + # then reuse the validated key to avoid a second lookup. + api_key: str | None = self._get_api_key(resolved) def llm_factory(mid: str) -> BaseLanguageModel: factory_kwargs: dict[str, Any] = {"max_retries": max_retries} - return self._create_provider_llm( - resolved_provider_type, + # Use sentinel so _create_provider_instance knows the key was + # pre-validated and should not trigger a second _get_api_key. + factory_kwargs["__api_key_sentinel"] = api_key + return self._create_provider_instance( + resolved, mid, **factory_kwargs, ) return LangChainChatProvider( - name=provider_type.value, - model_id=model_id or self.DEFAULT_MODELS.get(provider_type, "unknown"), + name=resolved.value, + model_id=model_id or self.DEFAULT_MODELS.get(resolved, "unknown"), llm_factory=llm_factory, max_retries=max_retries, supports_streaming=capabilities.supports_streaming, -- 2.52.0