diff --git a/src/skillspector/constants.py b/src/skillspector/constants.py index 79864145..b2358457 100644 --- a/src/skillspector/constants.py +++ b/src/skillspector/constants.py @@ -18,7 +18,7 @@ import logging import os -from skillspector.providers import get_metadata_provider +from skillspector.providers import get_metadata_provider, get_model_config_provider logger = logging.getLogger(__name__) @@ -74,8 +74,8 @@ def _resolve_slot_model(slot: str, provider=None) -> str: def build_model_config() -> dict[str, str]: - """Resolve the model map for the currently active provider.""" - provider = get_metadata_provider() + """Resolve the model map for the provider that will build chat models.""" + provider = get_model_config_provider() return {slot: _resolve_slot_model(slot, provider) for slot in _MODEL_SLOTS} diff --git a/src/skillspector/providers/__init__.py b/src/skillspector/providers/__init__.py index 7cf07ef3..4a725298 100644 --- a/src/skillspector/providers/__init__.py +++ b/src/skillspector/providers/__init__.py @@ -213,6 +213,31 @@ def resolve_chat_model_credentials() -> tuple[str, str | None] | None: return _openai_fallback_provider().resolve_credentials() +def get_model_config_provider() -> ModelMetadataProvider: + """Return the provider whose model defaults match graph chat-model routing. + + Explicit bindings, CLI providers, and Bedrock's native AWS credential path + remain authoritative. Unbound API-key providers use OpenAI metadata only + when their own credentials are absent and the OpenAI fallback is configured. + """ + provider = _select_active_provider() + from .bedrock import BedrockProvider + + if ( + has_provider_binding() + or has_cli_capability(provider) + or isinstance(provider, BedrockProvider) + ): + return provider + if provider.resolve_credentials() is not None: + return provider + + fallback = _openai_fallback_provider() + if fallback.resolve_credentials() is not None: + return fallback + return provider + + def create_chat_model_with_provider( model: str, *, @@ -281,6 +306,7 @@ def create_chat_model( "create_chat_model", "create_chat_model_with_provider", "get_active_provider", + "get_model_config_provider", "get_metadata_provider", "has_cli_capability", "has_provider_binding", diff --git a/tests/nodes/test_build_context.py b/tests/nodes/test_build_context.py index 51a917de..3a505b8f 100644 --- a/tests/nodes/test_build_context.py +++ b/tests/nodes/test_build_context.py @@ -35,6 +35,7 @@ from skillspector.inspection_ledger import LedgerReason from skillspector.nodes.build_context import build_context from skillspector.providers import reset_provider, use_provider +from skillspector.providers.openai import OpenAIProvider from skillspector.python_ast import ParsedPythonFile, get_python_ast from skillspector.state import ( MAX_WORKFLOW_ARTIFACTS, @@ -582,6 +583,23 @@ def create_chat_model(self, model: str, *, max_tokens: int, timeout: float | Non assert result["model_config"]["meta_analyzer"] == "bound-meta" +def test_build_context_model_config_matches_openai_fallback( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + for key in ( + "SKILLSPECTOR_PROVIDER", + "SKILLSPECTOR_MODEL", + "NVIDIA_INFERENCE_KEY", + "NVIDIA_INFERENCE_METADATA_KEY", + ): + monkeypatch.delenv(key, raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai-only") + + result = build_context({"skill_path": str(tmp_path)}) + + assert result["model_config"]["default"] == OpenAIProvider.DEFAULT_MODEL + + def test_build_context_inventories_but_excludes_valid_root_oms_signature( tmp_path: Path, ) -> None: diff --git a/tests/unit/test_constants.py b/tests/unit/test_constants.py index 6cfdabc6..bf0a2ba9 100644 --- a/tests/unit/test_constants.py +++ b/tests/unit/test_constants.py @@ -23,6 +23,10 @@ import pytest from skillspector.providers import registry +from skillspector.providers.bedrock import BedrockProvider +from skillspector.providers.codex_cli import CodexCLIProvider +from skillspector.providers.nv_build import NvBuildProvider +from skillspector.providers.openai import OpenAIProvider @pytest.fixture(autouse=True) @@ -96,6 +100,53 @@ def test_whitespace_only_slot_env_is_ignored(self, monkeypatch: pytest.MonkeyPat # Whitespace-only treated as unset — falls through to provider. assert mod.MODEL_CONFIG["meta_analyzer"] != " " + def test_openai_fallback_uses_openai_defaults(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai-only") + + config = _reload_constants().build_model_config() + + provider = OpenAIProvider() + assert config["default"] == provider.resolve_model() + assert config["meta_analyzer"] == provider.resolve_model("meta_analyzer") + + def test_slot_override_wins_over_openai_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai-only") + monkeypatch.setenv("SKILLSPECTOR_MODEL_META_ANALYZER", "custom/meta-model") + + config = _reload_constants().build_model_config() + + assert config["default"] == OpenAIProvider.DEFAULT_MODEL + assert config["meta_analyzer"] == "custom/meta-model" + + def test_configured_provider_precedes_openai_fallback( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("SKILLSPECTOR_PROVIDER", "nv_build") + monkeypatch.setenv("NVIDIA_INFERENCE_KEY", "nvapi-test") + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai") + + config = _reload_constants().build_model_config() + + assert config["default"] == NvBuildProvider.DEFAULT_MODEL + + def test_cli_provider_precedes_openai_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("SKILLSPECTOR_PROVIDER", "codex_cli") + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai") + + config = _reload_constants().build_model_config() + + assert config["default"] == CodexCLIProvider.DEFAULT_MODEL + + def test_bedrock_native_auth_precedes_openai_fallback( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("SKILLSPECTOR_PROVIDER", "bedrock") + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai") + + config = _reload_constants().build_model_config() + + assert config["default"] == BedrockProvider.DEFAULT_MODEL + class TestModelValidation: """_validate_model_config warns or raises on unknown model IDs.""" diff --git a/tests/unit/test_llm_utils.py b/tests/unit/test_llm_utils.py index 7642f909..dbc0c8df 100644 --- a/tests/unit/test_llm_utils.py +++ b/tests/unit/test_llm_utils.py @@ -32,6 +32,7 @@ from pydantic import BaseModel from skillspector import llm_utils +from skillspector.constants import build_model_config from skillspector.inference_usage import InferenceUsageCollector from skillspector.llm_utils import ( AgentCLIChatModel, @@ -659,6 +660,17 @@ def test_openai_fallback_uses_openai_default_model( assert _chat_model_name(llm) == OpenAIProvider.DEFAULT_MODEL + def test_graph_model_config_matches_openai_fallback_client( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai-only") + + model = build_model_config()["default"] + llm = get_chat_model(model=model) + + assert model == OpenAIProvider.DEFAULT_MODEL + assert _chat_model_name(llm) == OpenAIProvider.DEFAULT_MODEL + def test_explicit_model_still_overrides_openai_fallback( self, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/unit/test_mcp_server.py b/tests/unit/test_mcp_server.py index 1d2430c9..3a176bfe 100644 --- a/tests/unit/test_mcp_server.py +++ b/tests/unit/test_mcp_server.py @@ -27,7 +27,9 @@ from skillspector import mcp_server from skillspector.mcp_server import run_scan from skillspector.models import Finding +from skillspector.nodes.build_context import build_context from skillspector.providers import reset_provider, use_provider +from skillspector.providers.openai import OpenAIProvider from skillspector.suppression import SuppressedFinding @@ -84,6 +86,41 @@ async def test_run_scan_reports_llm_available_with_credentials( assert result["scan_mode"] == "static-only" +async def test_run_scan_openai_fallback_builds_matching_graph_model_config( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + for key in ( + "SKILLSPECTOR_PROVIDER", + "SKILLSPECTOR_MODEL", + "NVIDIA_INFERENCE_KEY", + "NVIDIA_INFERENCE_METADATA_KEY", + ): + monkeypatch.delenv(key, raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai-only") + monkeypatch.setattr(mcp_server, "is_llm_available", lambda: (True, None)) + _write_skill(tmp_path) + captured: dict[str, str] = {} + + class _Graph: + async def ainvoke(self, state, config): + context = build_context({"skill_path": state["input_path"]}) + captured.update(context["model_config"]) + return { + "filtered_findings": [], + "risk_score": 0, + "risk_severity": "LOW", + "risk_recommendation": "OK", + "report_body": "report", + } + + monkeypatch.setattr(mcp_server, "graph", _Graph()) + + result = await run_scan(str(tmp_path), use_llm=True, output_format="json") + + assert result["llm_used"] is True + assert captured["default"] == OpenAIProvider.DEFAULT_MODEL + + async def test_run_scan_uses_bound_provider_without_credentials( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: