-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(provider): allow scoped LLM provider injection (#243) #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,7 @@ | |
| from skillspector import __version__ | ||
| from skillspector.graph import graph | ||
| from skillspector.logging_config import get_logger | ||
| from skillspector.providers import resolve_provider_credentials | ||
| from skillspector.providers import has_provider_binding, resolve_provider_credentials | ||
|
|
||
| if TYPE_CHECKING: | ||
| from mcp.server.fastmcp import FastMCP | ||
|
|
@@ -74,7 +74,7 @@ async def run_scan( | |
| if output_format not in VALID_FORMATS: | ||
| raise ValueError(f"output_format must be one of {VALID_FORMATS}, got {output_format!r}") | ||
|
|
||
| llm_available = resolve_provider_credentials() is not None | ||
| llm_available = has_provider_binding() or resolve_provider_credentials() is not None | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking: any binding is declared available even when its |
||
| llm_used = use_llm and llm_available | ||
|
|
||
| state: dict[str, Any] = { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ | |
| from __future__ import annotations | ||
|
|
||
| import os | ||
| from contextvars import ContextVar, Token | ||
| from typing import NoReturn | ||
|
|
||
| from langchain_core.language_models.chat_models import BaseChatModel | ||
|
|
@@ -67,14 +68,38 @@ | |
| "Use --no-llm to skip LLM analysis and run static checks only." | ||
| ) | ||
|
|
||
| _INJECTED_PROVIDER: ContextVar[LLMProvider | None] = ContextVar( | ||
| "skillspector_injected_provider", | ||
| default=None, | ||
| ) | ||
|
|
||
|
|
||
| def raise_no_llm_api_key_configured() -> NoReturn: | ||
| """Raise the shared no-LLM-credentials error.""" | ||
| raise ValueError(NO_LLM_API_KEY_MESSAGE) | ||
|
|
||
|
|
||
| def use_provider(provider: LLMProvider) -> Token[LLMProvider | None]: | ||
| """Bind *provider* for the current context.""" | ||
| return _INJECTED_PROVIDER.set(provider) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking: the provider binding is context-local, but |
||
|
|
||
|
|
||
| def reset_provider(token: Token[LLMProvider | None]) -> None: | ||
| """Restore the provider binding represented by *token*.""" | ||
| _INJECTED_PROVIDER.reset(token) | ||
|
|
||
|
|
||
| def has_provider_binding() -> bool: | ||
| """Return whether the current context has an injected provider.""" | ||
| return _INJECTED_PROVIDER.get() is not None | ||
|
|
||
|
|
||
| def _select_active_provider() -> LLMProvider: | ||
| """Construct the active provider based on ``SKILLSPECTOR_PROVIDER``.""" | ||
| injected_provider = _INJECTED_PROVIDER.get() | ||
| if injected_provider is not None: | ||
| return injected_provider | ||
|
|
||
| name = os.environ.get("SKILLSPECTOR_PROVIDER", "").strip().lower() | ||
|
|
||
| if name == "openai": | ||
|
|
@@ -166,6 +191,9 @@ def resolve_chat_model_credentials() -> tuple[str, str | None] | None: | |
| if creds is not None: | ||
| return creds | ||
|
|
||
| if has_provider_binding(): | ||
| return None | ||
|
|
||
| return _openai_fallback_provider().resolve_credentials() | ||
|
|
||
|
|
||
|
|
@@ -194,6 +222,9 @@ def create_chat_model( | |
| if llm is not None: | ||
| return llm | ||
|
|
||
| if has_provider_binding(): | ||
| raise_no_llm_api_key_configured() | ||
|
|
||
| from .openai import OpenAIProvider | ||
|
|
||
| if not isinstance(provider, OpenAIProvider): | ||
|
|
@@ -219,7 +250,10 @@ def create_chat_model( | |
| "get_active_provider", | ||
| "get_metadata_provider", | ||
| "has_cli_capability", | ||
| "has_provider_binding", | ||
| "reset_provider", | ||
| "raise_no_llm_api_key_configured", | ||
| "resolve_chat_model_credentials", | ||
| "resolve_provider_credentials", | ||
| "use_provider", | ||
| ] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking: checking the binding before
has_cli_capabilitysends an injected CLI-capable provider throughcreate_chat_model, which rejects CLI providers. Check CLI capability first and add a bound CLI-provider regression.