Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/skillspector/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -68,8 +68,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}


Expand Down
26 changes: 26 additions & 0 deletions src/skillspector/providers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,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(
model: str,
*,
Expand Down Expand Up @@ -248,6 +273,7 @@ def create_chat_model(
"NO_LLM_API_KEY_MESSAGE",
"create_chat_model",
"get_active_provider",
"get_model_config_provider",
"get_metadata_provider",
"has_cli_capability",
"has_provider_binding",
Expand Down
23 changes: 22 additions & 1 deletion tests/nodes/test_build_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from skillspector.constants import MODEL_CONFIG
from skillspector.nodes.build_context import build_context
from skillspector.providers import reset_provider, use_provider
from skillspector.providers.openai import OpenAIProvider
from skillspector.state import SkillspectorState


Expand Down Expand Up @@ -133,7 +134,9 @@ def test_build_context_empty_directory_is_valid_empty_scan(tmp_path: Path) -> No
assert result["model_config"] == MODEL_CONFIG


def test_build_context_model_config_uses_bound_provider(tmp_path: Path) -> None:
def test_build_context_model_config_uses_bound_provider(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
class _BoundProvider:
DEFAULT_MODEL = "bound-default"
SLOT_DEFAULTS = {"meta_analyzer": "bound-meta"}
Expand All @@ -153,6 +156,7 @@ def resolve_credentials(self) -> tuple[str, str | None] | None:
def create_chat_model(self, model: str, *, max_tokens: int, timeout: float | None = 120):
return object()

monkeypatch.setenv("OPENAI_API_KEY", "sk-test-fallback-must-not-win")
token = use_provider(_BoundProvider())
try:
result = build_context({"skill_path": str(tmp_path)})
Expand All @@ -163,6 +167,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_skips_skip_dirs(tmp_path: Path) -> None:
"""Skip dirs like __pycache__ and node_modules are not included in components."""
_make_skill_spec_dir(tmp_path)
Expand Down
51 changes: 51 additions & 0 deletions tests/unit/test_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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."""
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/test_llm_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from pydantic import BaseModel

from skillspector import llm_utils
from skillspector.constants import build_model_config
from skillspector.llm_utils import (
AgentCLIChatModel,
_extract_json_object,
Expand Down Expand Up @@ -456,6 +457,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:
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/test_mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@

from skillspector import mcp_server
from skillspector.mcp_server import run_scan
from skillspector.nodes.build_context import build_context
from skillspector.providers import reset_provider, use_provider
from skillspector.providers.openai import OpenAIProvider


def _write_skill(tmp_path: Path, body: str = "# Safe skill") -> Path:
Expand Down Expand Up @@ -80,6 +82,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:
Expand Down