From 539267e11169a5186185a7ae1a8af09d78171684 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Fri, 17 Jul 2026 22:13:48 -0400 Subject: [PATCH 1/3] feat(provider): forward reasoning effort to OpenAI-compatible models (#283) Signed-off-by: Rod Boev --- .env.example | 2 + README.md | 1 + docs/DEVELOPMENT.md | 1 + src/skillspector/providers/chat_models.py | 21 ++-- tests/unit/test_constants.py | 1 + tests/unit/test_providers.py | 114 ++++++++++++++++++++++ 6 files changed, 132 insertions(+), 8 deletions(-) diff --git a/.env.example b/.env.example index 5e90ec6f..0f225bf3 100644 --- a/.env.example +++ b/.env.example @@ -17,6 +17,8 @@ NVIDIA_INFERENCE_KEY= # etc.); leave unset for stock api.openai.com. OPENAI_API_KEY= OPENAI_BASE_URL= +# Optional for OpenAI-compatible providers; unset or blank uses the provider default. +SKILLSPECTOR_REASONING_EFFORT= # For SKILLSPECTOR_PROVIDER=anthropic. ANTHROPIC_API_KEY= diff --git a/README.md b/README.md index 045121a5..3fd28ec3 100644 --- a/README.md +++ b/README.md @@ -557,6 +557,7 @@ Issues (2) | `NVIDIA_INFERENCE_KEY` | Credential for the `nv_build` provider (build.nvidia.com). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=nv_build` | | `OPENAI_API_KEY` | Credential for the OpenAI provider (`SKILLSPECTOR_PROVIDER=openai`). Also serves as the tier-2 fallback in the credential waterfall when the active provider returns no credentials. | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=openai` | | `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | Optional | +| `SKILLSPECTOR_REASONING_EFFORT` | Optional reasoning-effort literal for OpenAI-compatible providers; provider/model dependent. Unset or blank preserves provider-default behavior. | Optional | | `ANTHROPIC_API_KEY` | Credential for the Anthropic provider (`SKILLSPECTOR_PROVIDER=anthropic`). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=anthropic` | | `ANTHROPIC_PROXY_ENDPOINT_URL` | Full endpoint URL for the Anthropic proxy provider (Vertex-style raw-predict). | Required when `SKILLSPECTOR_PROVIDER=anthropic_proxy` | | `ANTHROPIC_PROXY_API_KEY` | Bearer token for the Anthropic proxy provider. | Required when `SKILLSPECTOR_PROVIDER=anthropic_proxy` | diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 048d0806..fd52fd49 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -297,6 +297,7 @@ Copy [.env.example](../.env.example) to `.env` in the project root and set value | `NVIDIA_INFERENCE_KEY` | Credential for `nv_build`. | `nvapi-...` | | `OPENAI_API_KEY` | Credential for `SKILLSPECTOR_PROVIDER=openai`. Also tier-2 fallback for non-OpenAI providers. | `sk-...` | | `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | `http://localhost:11434/v1` | +| `SKILLSPECTOR_REASONING_EFFORT` | Optional reasoning-effort literal for OpenAI-compatible providers; provider/model dependent. Unset or blank preserves provider-default behavior. | `high` | | `ANTHROPIC_API_KEY` | Credential for `SKILLSPECTOR_PROVIDER=anthropic`. | `sk-ant-...` | | `SKILLSPECTOR_MODEL` | Override the active provider's bundled default model (see [README.md](../README.md) for per-provider defaults). For `claude_cli`, this is passed as `--model` to the `claude` binary. | `gpt-5.2` | diff --git a/src/skillspector/providers/chat_models.py b/src/skillspector/providers/chat_models.py index 5ce78e04..17d531ed 100644 --- a/src/skillspector/providers/chat_models.py +++ b/src/skillspector/providers/chat_models.py @@ -18,6 +18,7 @@ from __future__ import annotations import logging +import os from urllib.parse import urlparse from langchain_core.language_models.chat_models import BaseChatModel @@ -64,11 +65,15 @@ def create_openai_compatible_chat_model( api_key, base_url = credentials validate_base_url(base_url) - return ChatOpenAI( - model=model, - base_url=base_url, - api_key=SecretStr(api_key), - max_completion_tokens=max_tokens, - timeout=timeout, - default_headers=default_headers, - ) + kwargs = { + "model": model, + "base_url": base_url, + "api_key": SecretStr(api_key), + "max_completion_tokens": max_tokens, + "timeout": timeout, + "default_headers": default_headers, + } + reasoning_effort = os.environ.get("SKILLSPECTOR_REASONING_EFFORT", "").strip() + if reasoning_effort: + kwargs["reasoning_effort"] = reasoning_effort + return ChatOpenAI(**kwargs) diff --git a/tests/unit/test_constants.py b/tests/unit/test_constants.py index 7f2789a6..6cfdabc6 100644 --- a/tests/unit/test_constants.py +++ b/tests/unit/test_constants.py @@ -36,6 +36,7 @@ def _clean_env(monkeypatch: pytest.MonkeyPatch): "NVIDIA_INFERENCE_KEY", "OPENAI_API_KEY", "OPENAI_BASE_URL", + "SKILLSPECTOR_REASONING_EFFORT", "ANTHROPIC_API_KEY", ): monkeypatch.delenv(key, raising=False) diff --git a/tests/unit/test_providers.py b/tests/unit/test_providers.py index fae2572f..410a2aa6 100644 --- a/tests/unit/test_providers.py +++ b/tests/unit/test_providers.py @@ -27,10 +27,12 @@ import pytest from langchain_anthropic import ChatAnthropic from langchain_openai import ChatOpenAI +from pydantic import SecretStr import skillspector.providers as providers_module from skillspector.providers import ( NO_LLM_API_KEY_MESSAGE, + chat_models, create_chat_model, get_active_provider, get_metadata_provider, @@ -113,6 +115,7 @@ def _clean_provider_env(monkeypatch: pytest.MonkeyPatch): monkeypatch.delenv("OPENAI_API_KEY", raising=False) monkeypatch.delenv("OPENAI_BASE_URL", raising=False) monkeypatch.delenv("OPENAI_PROJECT_ID", raising=False) + monkeypatch.delenv("SKILLSPECTOR_REASONING_EFFORT", raising=False) monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) monkeypatch.delenv("SKILLSPECTOR_MODEL", raising=False) monkeypatch.delenv("SKILLSPECTOR_MODEL_REGISTRY", raising=False) @@ -344,6 +347,117 @@ def test_builds_chat_openai_from_credentials(self) -> None: assert llm.max_tokens == 123 assert str(llm.openai_api_base).rstrip("/") == "http://localhost:1234/v1" + def test_reasoning_effort_configured(self, monkeypatch: pytest.MonkeyPatch) -> None: + captured: dict[str, object] = {} + + def fake_chat_openai(**kwargs: object) -> dict[str, object]: + captured.update(kwargs) + return kwargs + + monkeypatch.setattr(chat_models, "ChatOpenAI", fake_chat_openai) + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", " high ") + + create_openai_compatible_chat_model( + model="gpt-5.4", + credentials=("sk-x", "http://localhost:1234/v1"), + max_tokens=123, + ) + + assert captured["reasoning_effort"] == "high" + + def test_reasoning_effort_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: + captured: dict[str, object] = {} + + def fake_chat_openai(**kwargs: object) -> dict[str, object]: + captured.update(kwargs) + return kwargs + + monkeypatch.setattr(chat_models, "ChatOpenAI", fake_chat_openai) + + create_openai_compatible_chat_model( + model="gpt-5.4", + credentials=("sk-x", "http://localhost:1234/v1"), + max_tokens=123, + ) + + assert "reasoning_effort" not in captured + assert captured["max_completion_tokens"] == 123 + + @pytest.mark.parametrize("blank_value", [" ", "\t\n"]) + def test_reasoning_effort_blank( + self, monkeypatch: pytest.MonkeyPatch, blank_value: str + ) -> None: + captured: dict[str, object] = {} + + def fake_chat_openai(**kwargs: object) -> dict[str, object]: + captured.update(kwargs) + return kwargs + + monkeypatch.setattr(chat_models, "ChatOpenAI", fake_chat_openai) + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", blank_value) + + create_openai_compatible_chat_model( + model="gpt-5.4", + credentials=("sk-x", "http://localhost:1234/v1"), + max_tokens=123, + ) + + assert "reasoning_effort" not in captured + assert captured["max_completion_tokens"] == 123 + + def test_reasoning_effort_provider_matrix(self, monkeypatch: pytest.MonkeyPatch) -> None: + captured: dict[str, object] = {} + + def fake_chat_openai(**kwargs: object) -> dict[str, object]: + captured.clear() + captured.update(kwargs) + return kwargs + + monkeypatch.setattr(chat_models, "ChatOpenAI", fake_chat_openai) + cases = ( + (OpenAIProvider(), "OPENAI_API_KEY", "sk-x", "http://localhost:1234/v1"), + (NvBuildProvider(), "NVIDIA_INFERENCE_KEY", "nvapi-x", BUILD_BASE_URL), + ) + for provider, key, value, endpoint in cases: + monkeypatch.setenv(key, value) + if isinstance(provider, OpenAIProvider): + monkeypatch.setenv("OPENAI_BASE_URL", endpoint) + monkeypatch.setenv("OPENAI_PROJECT_ID", "proj_123") + for effort in (None, " ", " high "): + if effort is None: + monkeypatch.delenv("SKILLSPECTOR_REASONING_EFFORT", raising=False) + else: + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", effort) + provider.create_chat_model("model-x", max_tokens=123) + assert captured["base_url"] == endpoint + assert captured["max_completion_tokens"] == 123 + assert isinstance(captured["api_key"], SecretStr) + assert captured["api_key"].get_secret_value() == value + if isinstance(provider, OpenAIProvider): + assert captured["default_headers"] == {"OpenAI-Project": "proj_123"} + if effort is None or not effort.strip(): + assert "reasoning_effort" not in captured + else: + assert captured["reasoning_effort"] == "high" + + def test_reasoning_effort_passthrough(self, monkeypatch: pytest.MonkeyPatch) -> None: + captured: dict[str, object] = {} + + def fake_chat_openai(**kwargs: object) -> dict[str, object]: + captured.update(kwargs) + return kwargs + + monkeypatch.setattr(chat_models, "ChatOpenAI", fake_chat_openai) + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", "provider-specific-value") + + create_openai_compatible_chat_model( + model="gpt-5.4", + credentials=("sk-x", "http://localhost:1234/v1"), + max_tokens=123, + ) + + assert captured["reasoning_effort"] == "provider-specific-value" + class TestProviderSelection: """SKILLSPECTOR_PROVIDER selects which provider answers credentials.""" From 8784f21c1aafaad729f09924cbad1d5e7ad898fd Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Mon, 20 Jul 2026 14:15:56 -0400 Subject: [PATCH 2/3] feat(provider): keep reasoning effort consistent across Anthropic paths (#283) Signed-off-by: Rod Boev --- .env.example | 4 +- README.md | 2 +- docs/DEVELOPMENT.md | 2 +- .../providers/anthropic/provider.py | 21 ++++--- .../providers/anthropic_proxy/provider.py | 23 ++++--- src/skillspector/providers/chat_models.py | 16 +++++ tests/unit/test_anthropic_proxy_provider.py | 63 +++++++++++++++++++ tests/unit/test_providers.py | 50 +++++++++++++++ 8 files changed, 161 insertions(+), 20 deletions(-) diff --git a/.env.example b/.env.example index 0f225bf3..10225125 100644 --- a/.env.example +++ b/.env.example @@ -17,7 +17,9 @@ NVIDIA_INFERENCE_KEY= # etc.); leave unset for stock api.openai.com. OPENAI_API_KEY= OPENAI_BASE_URL= -# Optional for OpenAI-compatible providers; unset or blank uses the provider default. +# Optional for OpenAI-compatible and native Anthropic providers. OpenAI-compatible +# providers pass non-empty literals through; Anthropic accepts low|medium|high|xhigh|max. +# Unset or blank uses the provider default. SKILLSPECTOR_REASONING_EFFORT= # For SKILLSPECTOR_PROVIDER=anthropic. diff --git a/README.md b/README.md index 3fd28ec3..e1715ce8 100644 --- a/README.md +++ b/README.md @@ -557,7 +557,7 @@ Issues (2) | `NVIDIA_INFERENCE_KEY` | Credential for the `nv_build` provider (build.nvidia.com). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=nv_build` | | `OPENAI_API_KEY` | Credential for the OpenAI provider (`SKILLSPECTOR_PROVIDER=openai`). Also serves as the tier-2 fallback in the credential waterfall when the active provider returns no credentials. | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=openai` | | `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | Optional | -| `SKILLSPECTOR_REASONING_EFFORT` | Optional reasoning-effort literal for OpenAI-compatible providers; provider/model dependent. Unset or blank preserves provider-default behavior. | Optional | +| `SKILLSPECTOR_REASONING_EFFORT` | Optional provider-neutral reasoning-effort setting. OpenAI-compatible providers pass non-empty literals through; native Anthropic accepts `low`, `medium`, `high`, `xhigh`, or `max`. Unset or blank preserves provider-default behavior. | Optional | | `ANTHROPIC_API_KEY` | Credential for the Anthropic provider (`SKILLSPECTOR_PROVIDER=anthropic`). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=anthropic` | | `ANTHROPIC_PROXY_ENDPOINT_URL` | Full endpoint URL for the Anthropic proxy provider (Vertex-style raw-predict). | Required when `SKILLSPECTOR_PROVIDER=anthropic_proxy` | | `ANTHROPIC_PROXY_API_KEY` | Bearer token for the Anthropic proxy provider. | Required when `SKILLSPECTOR_PROVIDER=anthropic_proxy` | diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index fd52fd49..aad9a000 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -297,7 +297,7 @@ Copy [.env.example](../.env.example) to `.env` in the project root and set value | `NVIDIA_INFERENCE_KEY` | Credential for `nv_build`. | `nvapi-...` | | `OPENAI_API_KEY` | Credential for `SKILLSPECTOR_PROVIDER=openai`. Also tier-2 fallback for non-OpenAI providers. | `sk-...` | | `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | `http://localhost:11434/v1` | -| `SKILLSPECTOR_REASONING_EFFORT` | Optional reasoning-effort literal for OpenAI-compatible providers; provider/model dependent. Unset or blank preserves provider-default behavior. | `high` | +| `SKILLSPECTOR_REASONING_EFFORT` | Optional provider-neutral reasoning-effort setting. OpenAI-compatible providers pass non-empty literals through; native Anthropic accepts `low`, `medium`, `high`, `xhigh`, or `max`. Unset or blank preserves provider-default behavior. | `high` | | `ANTHROPIC_API_KEY` | Credential for `SKILLSPECTOR_PROVIDER=anthropic`. | `sk-ant-...` | | `SKILLSPECTOR_MODEL` | Override the active provider's bundled default model (see [README.md](../README.md) for per-provider defaults). For `claude_cli`, this is passed as `--model` to the `claude` binary. | `gpt-5.2` | diff --git a/src/skillspector/providers/anthropic/provider.py b/src/skillspector/providers/anthropic/provider.py index 53c38852..315f1b68 100644 --- a/src/skillspector/providers/anthropic/provider.py +++ b/src/skillspector/providers/anthropic/provider.py @@ -32,6 +32,7 @@ from pydantic import SecretStr from skillspector.providers import registry +from skillspector.providers.chat_models import resolve_anthropic_reasoning_effort # Documented for completeness — ChatAnthropic defaults here when base_url=None. ANTHROPIC_BASE_URL = "https://api.anthropic.com" @@ -67,14 +68,18 @@ def create_chat_model( return None api_key, _ = creds - return ChatAnthropic( - model_name=model, - api_key=SecretStr(api_key), - base_url=ANTHROPIC_BASE_URL, - max_tokens_to_sample=max_tokens, - timeout=timeout, - stop=None, - ) + kwargs = { + "model_name": model, + "api_key": SecretStr(api_key), + "base_url": ANTHROPIC_BASE_URL, + "max_tokens_to_sample": max_tokens, + "timeout": timeout, + "stop": None, + } + effort = resolve_anthropic_reasoning_effort() + if effort is not None: + kwargs["effort"] = effort + return ChatAnthropic(**kwargs) def get_context_length(self, model: str) -> int | None: return registry.lookup_context_length(REGISTRY_PATH, model) diff --git a/src/skillspector/providers/anthropic_proxy/provider.py b/src/skillspector/providers/anthropic_proxy/provider.py index 121920ad..d56b4319 100644 --- a/src/skillspector/providers/anthropic_proxy/provider.py +++ b/src/skillspector/providers/anthropic_proxy/provider.py @@ -53,6 +53,7 @@ from pydantic import SecretStr from skillspector.providers import registry +from skillspector.providers.chat_models import resolve_anthropic_reasoning_effort REGISTRY_PATH = str(Path(__file__).with_name("model_registry.yaml")) @@ -231,15 +232,19 @@ def create_chat_model( bearer_token, endpoint_url = creds - return _ChatAnthropicProxy( - proxy_endpoint_url=endpoint_url, - proxy_bearer_token=bearer_token, - model_name=model, - anthropic_api_key=SecretStr("anthropic-proxy-placeholder"), - max_tokens=max_tokens, - default_request_timeout=timeout, - stop_sequences=None, - ) + kwargs = { + "proxy_endpoint_url": endpoint_url, + "proxy_bearer_token": bearer_token, + "model_name": model, + "anthropic_api_key": SecretStr("anthropic-proxy-placeholder"), + "max_tokens": max_tokens, + "default_request_timeout": timeout, + "stop_sequences": None, + } + effort = resolve_anthropic_reasoning_effort() + if effort is not None: + kwargs["effort"] = effort + return _ChatAnthropicProxy(**kwargs) def get_context_length(self, model: str) -> int | None: return registry.lookup_context_length(REGISTRY_PATH, model) diff --git a/src/skillspector/providers/chat_models.py b/src/skillspector/providers/chat_models.py index 17d531ed..d5f5a432 100644 --- a/src/skillspector/providers/chat_models.py +++ b/src/skillspector/providers/chat_models.py @@ -27,6 +27,22 @@ logger = logging.getLogger(__name__) +_ANTHROPIC_REASONING_EFFORTS = ("low", "medium", "high", "xhigh", "max") + + +def resolve_anthropic_reasoning_effort() -> str | None: + """Resolve the optional reasoning effort accepted by native Anthropic APIs.""" + reasoning_effort = os.environ.get("SKILLSPECTOR_REASONING_EFFORT", "").strip() + if not reasoning_effort: + return None + if reasoning_effort not in _ANTHROPIC_REASONING_EFFORTS: + accepted = ", ".join(_ANTHROPIC_REASONING_EFFORTS) + raise ValueError( + f"Invalid SKILLSPECTOR_REASONING_EFFORT for Anthropic: {reasoning_effort!r}; " + f"expected one of: {accepted}" + ) + return reasoning_effort + def validate_base_url(url: str | None) -> None: """Warn if *url* is not a well-formed http(s) URL. diff --git a/tests/unit/test_anthropic_proxy_provider.py b/tests/unit/test_anthropic_proxy_provider.py index c3a909fb..fe8dd377 100644 --- a/tests/unit/test_anthropic_proxy_provider.py +++ b/tests/unit/test_anthropic_proxy_provider.py @@ -42,6 +42,7 @@ def _clean_env(monkeypatch: pytest.MonkeyPatch): monkeypatch.delenv("SKILLSPECTOR_MODEL", raising=False) monkeypatch.delenv("SKILLSPECTOR_PROVIDER", raising=False) monkeypatch.delenv("SKILLSPECTOR_SSL_VERIFY", raising=False) + monkeypatch.delenv("SKILLSPECTOR_REASONING_EFFORT", raising=False) monkeypatch.delenv("OPENAI_API_KEY", raising=False) monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) monkeypatch.delenv("NVIDIA_INFERENCE_KEY", raising=False) @@ -90,6 +91,57 @@ def test_creates_chat_anthropic_subclass(self, monkeypatch: pytest.MonkeyPatch) assert llm.model == "claude-sonnet-4-6" assert llm.max_tokens == 4096 + @pytest.mark.parametrize("effort", ["low", "medium", "high", "xhigh", "max"]) + def test_reasoning_effort_maps_to_proxy_constructor( + self, monkeypatch: pytest.MonkeyPatch, effort: str + ) -> None: + captured: dict[str, object] = {} + + def fake_proxy(**kwargs: object) -> dict[str, object]: + captured.update(kwargs) + return kwargs + + monkeypatch.setattr( + "skillspector.providers.anthropic_proxy.provider._ChatAnthropicProxy", fake_proxy + ) + monkeypatch.setenv("ANTHROPIC_PROXY_API_KEY", "bearer-tok") + monkeypatch.setenv("ANTHROPIC_PROXY_ENDPOINT_URL", "https://proxy.example.com/predict") + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", f" {effort} ") + + AnthropicProxyProvider().create_chat_model("claude-sonnet-4-6", max_tokens=4096) + + assert captured["effort"] == effort + + @pytest.mark.parametrize("value", [None, " "]) + def test_reasoning_effort_blank_or_unset_omits_effort( + self, monkeypatch: pytest.MonkeyPatch, value: str | None + ) -> None: + captured: dict[str, object] = {} + + def fake_proxy(**kwargs: object) -> dict[str, object]: + captured.update(kwargs) + return kwargs + + monkeypatch.setattr( + "skillspector.providers.anthropic_proxy.provider._ChatAnthropicProxy", fake_proxy + ) + monkeypatch.setenv("ANTHROPIC_PROXY_API_KEY", "bearer-tok") + monkeypatch.setenv("ANTHROPIC_PROXY_ENDPOINT_URL", "https://proxy.example.com/predict") + if value is not None: + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", value) + + AnthropicProxyProvider().create_chat_model("claude-sonnet-4-6", max_tokens=4096) + + assert "effort" not in captured + + def test_reasoning_effort_invalid_value_rejected(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("ANTHROPIC_PROXY_API_KEY", "bearer-tok") + monkeypatch.setenv("ANTHROPIC_PROXY_ENDPOINT_URL", "https://proxy.example.com/predict") + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", "invalid") + + with pytest.raises(ValueError, match="low, medium, high, xhigh, max"): + AnthropicProxyProvider().create_chat_model("claude-sonnet-4-6", max_tokens=4096) + class TestAnthropicProxyProviderMetadata: """Token-budget metadata and model resolution tests.""" @@ -211,6 +263,17 @@ def test_preserves_other_body_fields(self) -> None: assert body["max_tokens"] == 200 assert body["temperature"] == 0.5 + def test_preserves_output_config_effort(self) -> None: + _, body = self._make_request( + { + "model": "claude-sonnet-4-6", + "messages": [], + "max_tokens": 200, + "output_config": {"effort": "xhigh"}, + } + ) + assert body["output_config"]["effort"] == "xhigh" + class TestApiVersionConfiguration: """Tests for ANTHROPIC_PROXY_API_VERSION env var.""" diff --git a/tests/unit/test_providers.py b/tests/unit/test_providers.py index 410a2aa6..85448e67 100644 --- a/tests/unit/test_providers.py +++ b/tests/unit/test_providers.py @@ -30,6 +30,7 @@ from pydantic import SecretStr import skillspector.providers as providers_module +import skillspector.providers.anthropic.provider as anthropic_provider_module from skillspector.providers import ( NO_LLM_API_KEY_MESSAGE, chat_models, @@ -308,6 +309,55 @@ def test_creates_native_chat_anthropic(self, monkeypatch: pytest.MonkeyPatch) -> assert llm.model == "claude-opus-4-6" assert llm.max_tokens == 123 + @pytest.mark.parametrize("effort", ["low", "medium", "high", "xhigh", "max"]) + def test_reasoning_effort_accepted_values( + self, monkeypatch: pytest.MonkeyPatch, effort: str + ) -> None: + captured: dict[str, object] = {} + + def fake_chat_anthropic(**kwargs: object) -> dict[str, object]: + captured.update(kwargs) + return kwargs + + monkeypatch.setattr(anthropic_provider_module, "ChatAnthropic", fake_chat_anthropic) + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-x") + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", f" {effort} ") + + AnthropicProvider().create_chat_model("claude-opus-4-6", max_tokens=123) + + assert captured["effort"] == effort + + @pytest.mark.parametrize("value", [None, " ", "\t\n"]) + def test_reasoning_effort_blank_or_unset_omits_effort( + self, monkeypatch: pytest.MonkeyPatch, value: str | None + ) -> None: + captured: dict[str, object] = {} + + def fake_chat_anthropic(**kwargs: object) -> dict[str, object]: + captured.update(kwargs) + return kwargs + + monkeypatch.setattr(anthropic_provider_module, "ChatAnthropic", fake_chat_anthropic) + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-x") + if value is None: + monkeypatch.delenv("SKILLSPECTOR_REASONING_EFFORT", raising=False) + else: + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", value) + + AnthropicProvider().create_chat_model("claude-opus-4-6", max_tokens=123) + + assert "effort" not in captured + + @pytest.mark.parametrize("value", ["invalid", "HIGH"]) + def test_reasoning_effort_invalid_value_rejected( + self, monkeypatch: pytest.MonkeyPatch, value: str + ) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-x") + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", value) + + with pytest.raises(ValueError, match="low, medium, high, xhigh, max"): + AnthropicProvider().create_chat_model("claude-opus-4-6", max_tokens=123) + def test_create_chat_model_returns_none_without_key(self) -> None: # No ANTHROPIC_API_KEY → no client, signalling the caller to fall back. assert AnthropicProvider().create_chat_model("claude-opus-4-6", max_tokens=123) is None From bb5988c770092f90997e55957363e3edc3dccfcb Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Mon, 20 Jul 2026 16:40:09 -0400 Subject: [PATCH 3/3] fix(provider): keep reasoning effort pass-through consistent (#283) Signed-off-by: Rod Boev --- .env.example | 5 ++--- README.md | 2 +- docs/DEVELOPMENT.md | 2 +- .../providers/anthropic/provider.py | 4 ++-- .../providers/anthropic_proxy/provider.py | 4 ++-- src/skillspector/providers/chat_models.py | 18 ++++-------------- tests/unit/test_anthropic_proxy_provider.py | 12 ++---------- tests/unit/test_llm_utils.py | 1 + tests/unit/test_providers.py | 14 ++------------ 9 files changed, 17 insertions(+), 45 deletions(-) diff --git a/.env.example b/.env.example index 10225125..db03085a 100644 --- a/.env.example +++ b/.env.example @@ -17,9 +17,8 @@ NVIDIA_INFERENCE_KEY= # etc.); leave unset for stock api.openai.com. OPENAI_API_KEY= OPENAI_BASE_URL= -# Optional for OpenAI-compatible and native Anthropic providers. OpenAI-compatible -# providers pass non-empty literals through; Anthropic accepts low|medium|high|xhigh|max. -# Unset or blank uses the provider default. +# Optional provider- and model-dependent reasoning-effort setting. Non-empty values +# are trimmed and passed through unchanged; unset or blank uses the provider default. SKILLSPECTOR_REASONING_EFFORT= # For SKILLSPECTOR_PROVIDER=anthropic. diff --git a/README.md b/README.md index e1715ce8..8ecc75dc 100644 --- a/README.md +++ b/README.md @@ -557,7 +557,7 @@ Issues (2) | `NVIDIA_INFERENCE_KEY` | Credential for the `nv_build` provider (build.nvidia.com). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=nv_build` | | `OPENAI_API_KEY` | Credential for the OpenAI provider (`SKILLSPECTOR_PROVIDER=openai`). Also serves as the tier-2 fallback in the credential waterfall when the active provider returns no credentials. | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=openai` | | `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | Optional | -| `SKILLSPECTOR_REASONING_EFFORT` | Optional provider-neutral reasoning-effort setting. OpenAI-compatible providers pass non-empty literals through; native Anthropic accepts `low`, `medium`, `high`, `xhigh`, or `max`. Unset or blank preserves provider-default behavior. | Optional | +| `SKILLSPECTOR_REASONING_EFFORT` | Optional provider- and model-dependent reasoning-effort setting. Non-empty values are trimmed and passed through unchanged; unset or blank preserves provider-default behavior. | Optional | | `ANTHROPIC_API_KEY` | Credential for the Anthropic provider (`SKILLSPECTOR_PROVIDER=anthropic`). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=anthropic` | | `ANTHROPIC_PROXY_ENDPOINT_URL` | Full endpoint URL for the Anthropic proxy provider (Vertex-style raw-predict). | Required when `SKILLSPECTOR_PROVIDER=anthropic_proxy` | | `ANTHROPIC_PROXY_API_KEY` | Bearer token for the Anthropic proxy provider. | Required when `SKILLSPECTOR_PROVIDER=anthropic_proxy` | diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index aad9a000..6f94e79c 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -297,7 +297,7 @@ Copy [.env.example](../.env.example) to `.env` in the project root and set value | `NVIDIA_INFERENCE_KEY` | Credential for `nv_build`. | `nvapi-...` | | `OPENAI_API_KEY` | Credential for `SKILLSPECTOR_PROVIDER=openai`. Also tier-2 fallback for non-OpenAI providers. | `sk-...` | | `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | `http://localhost:11434/v1` | -| `SKILLSPECTOR_REASONING_EFFORT` | Optional provider-neutral reasoning-effort setting. OpenAI-compatible providers pass non-empty literals through; native Anthropic accepts `low`, `medium`, `high`, `xhigh`, or `max`. Unset or blank preserves provider-default behavior. | `high` | +| `SKILLSPECTOR_REASONING_EFFORT` | Optional provider- and model-dependent reasoning-effort setting. Non-empty values are trimmed and passed through unchanged; unset or blank preserves provider-default behavior. | `high` | | `ANTHROPIC_API_KEY` | Credential for `SKILLSPECTOR_PROVIDER=anthropic`. | `sk-ant-...` | | `SKILLSPECTOR_MODEL` | Override the active provider's bundled default model (see [README.md](../README.md) for per-provider defaults). For `claude_cli`, this is passed as `--model` to the `claude` binary. | `gpt-5.2` | diff --git a/src/skillspector/providers/anthropic/provider.py b/src/skillspector/providers/anthropic/provider.py index 315f1b68..fc1ea6da 100644 --- a/src/skillspector/providers/anthropic/provider.py +++ b/src/skillspector/providers/anthropic/provider.py @@ -32,7 +32,7 @@ from pydantic import SecretStr from skillspector.providers import registry -from skillspector.providers.chat_models import resolve_anthropic_reasoning_effort +from skillspector.providers.chat_models import resolve_reasoning_effort # Documented for completeness — ChatAnthropic defaults here when base_url=None. ANTHROPIC_BASE_URL = "https://api.anthropic.com" @@ -76,7 +76,7 @@ def create_chat_model( "timeout": timeout, "stop": None, } - effort = resolve_anthropic_reasoning_effort() + effort = resolve_reasoning_effort() if effort is not None: kwargs["effort"] = effort return ChatAnthropic(**kwargs) diff --git a/src/skillspector/providers/anthropic_proxy/provider.py b/src/skillspector/providers/anthropic_proxy/provider.py index d56b4319..46277f57 100644 --- a/src/skillspector/providers/anthropic_proxy/provider.py +++ b/src/skillspector/providers/anthropic_proxy/provider.py @@ -53,7 +53,7 @@ from pydantic import SecretStr from skillspector.providers import registry -from skillspector.providers.chat_models import resolve_anthropic_reasoning_effort +from skillspector.providers.chat_models import resolve_reasoning_effort REGISTRY_PATH = str(Path(__file__).with_name("model_registry.yaml")) @@ -241,7 +241,7 @@ def create_chat_model( "default_request_timeout": timeout, "stop_sequences": None, } - effort = resolve_anthropic_reasoning_effort() + effort = resolve_reasoning_effort() if effort is not None: kwargs["effort"] = effort return _ChatAnthropicProxy(**kwargs) diff --git a/src/skillspector/providers/chat_models.py b/src/skillspector/providers/chat_models.py index d5f5a432..ec4b62af 100644 --- a/src/skillspector/providers/chat_models.py +++ b/src/skillspector/providers/chat_models.py @@ -27,21 +27,11 @@ logger = logging.getLogger(__name__) -_ANTHROPIC_REASONING_EFFORTS = ("low", "medium", "high", "xhigh", "max") - -def resolve_anthropic_reasoning_effort() -> str | None: - """Resolve the optional reasoning effort accepted by native Anthropic APIs.""" +def resolve_reasoning_effort() -> str | None: + """Resolve the optional provider- and model-dependent reasoning effort.""" reasoning_effort = os.environ.get("SKILLSPECTOR_REASONING_EFFORT", "").strip() - if not reasoning_effort: - return None - if reasoning_effort not in _ANTHROPIC_REASONING_EFFORTS: - accepted = ", ".join(_ANTHROPIC_REASONING_EFFORTS) - raise ValueError( - f"Invalid SKILLSPECTOR_REASONING_EFFORT for Anthropic: {reasoning_effort!r}; " - f"expected one of: {accepted}" - ) - return reasoning_effort + return reasoning_effort or None def validate_base_url(url: str | None) -> None: @@ -89,7 +79,7 @@ def create_openai_compatible_chat_model( "timeout": timeout, "default_headers": default_headers, } - reasoning_effort = os.environ.get("SKILLSPECTOR_REASONING_EFFORT", "").strip() + reasoning_effort = resolve_reasoning_effort() if reasoning_effort: kwargs["reasoning_effort"] = reasoning_effort return ChatOpenAI(**kwargs) diff --git a/tests/unit/test_anthropic_proxy_provider.py b/tests/unit/test_anthropic_proxy_provider.py index fe8dd377..72751a53 100644 --- a/tests/unit/test_anthropic_proxy_provider.py +++ b/tests/unit/test_anthropic_proxy_provider.py @@ -91,8 +91,8 @@ def test_creates_chat_anthropic_subclass(self, monkeypatch: pytest.MonkeyPatch) assert llm.model == "claude-sonnet-4-6" assert llm.max_tokens == 4096 - @pytest.mark.parametrize("effort", ["low", "medium", "high", "xhigh", "max"]) - def test_reasoning_effort_maps_to_proxy_constructor( + @pytest.mark.parametrize("effort", ["provider-specific-value"]) + def test_reasoning_effort_passthrough( self, monkeypatch: pytest.MonkeyPatch, effort: str ) -> None: captured: dict[str, object] = {} @@ -134,14 +134,6 @@ def fake_proxy(**kwargs: object) -> dict[str, object]: assert "effort" not in captured - def test_reasoning_effort_invalid_value_rejected(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("ANTHROPIC_PROXY_API_KEY", "bearer-tok") - monkeypatch.setenv("ANTHROPIC_PROXY_ENDPOINT_URL", "https://proxy.example.com/predict") - monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", "invalid") - - with pytest.raises(ValueError, match="low, medium, high, xhigh, max"): - AnthropicProxyProvider().create_chat_model("claude-sonnet-4-6", max_tokens=4096) - class TestAnthropicProxyProviderMetadata: """Token-budget metadata and model resolution tests.""" diff --git a/tests/unit/test_llm_utils.py b/tests/unit/test_llm_utils.py index 7698d47b..fb0c57da 100644 --- a/tests/unit/test_llm_utils.py +++ b/tests/unit/test_llm_utils.py @@ -56,6 +56,7 @@ "OPENAI_API_KEY", "OPENAI_BASE_URL", "NVIDIA_INFERENCE_KEY", + "SKILLSPECTOR_REASONING_EFFORT", "SKILLSPECTOR_MODEL", "SKILLSPECTOR_PROVIDER", ) diff --git a/tests/unit/test_providers.py b/tests/unit/test_providers.py index 85448e67..3e558274 100644 --- a/tests/unit/test_providers.py +++ b/tests/unit/test_providers.py @@ -309,8 +309,8 @@ def test_creates_native_chat_anthropic(self, monkeypatch: pytest.MonkeyPatch) -> assert llm.model == "claude-opus-4-6" assert llm.max_tokens == 123 - @pytest.mark.parametrize("effort", ["low", "medium", "high", "xhigh", "max"]) - def test_reasoning_effort_accepted_values( + @pytest.mark.parametrize("effort", ["provider-specific-value"]) + def test_reasoning_effort_passthrough( self, monkeypatch: pytest.MonkeyPatch, effort: str ) -> None: captured: dict[str, object] = {} @@ -348,16 +348,6 @@ def fake_chat_anthropic(**kwargs: object) -> dict[str, object]: assert "effort" not in captured - @pytest.mark.parametrize("value", ["invalid", "HIGH"]) - def test_reasoning_effort_invalid_value_rejected( - self, monkeypatch: pytest.MonkeyPatch, value: str - ) -> None: - monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-x") - monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", value) - - with pytest.raises(ValueError, match="low, medium, high, xhigh, max"): - AnthropicProvider().create_chat_model("claude-opus-4-6", max_tokens=123) - def test_create_chat_model_returns_none_without_key(self) -> None: # No ANTHROPIC_API_KEY → no client, signalling the caller to fall back. assert AnthropicProvider().create_chat_model("claude-opus-4-6", max_tokens=123) is None