mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-14 12:15:29 +02:00
fix(llm): normalize model environment values
This commit is contained in:
@@ -16,6 +16,7 @@ from llm.core.interface import (
|
||||
)
|
||||
from llm.core.types import LLMInput, LLMOutput, ModelInfo, ProviderType, ToolCall
|
||||
from llm.providers.constants import EMPTY_FILTERED_RESPONSE_ERROR
|
||||
from llm.providers.model_config import read_model_env
|
||||
from llm.providers.openai_compat import to_openai_message_param
|
||||
|
||||
ASTRAFLOW_BASE_URL = "https://api.umodelverse.ai/v1"
|
||||
@@ -54,8 +55,8 @@ class _AstraflowBaseProvider(LLMProvider):
|
||||
) -> None:
|
||||
self.api_key = api_key or os.environ.get(self.api_key_env) or ""
|
||||
self.base_url = base_url or os.environ.get(self.base_url_env, self.default_base_url)
|
||||
env_model = os.environ.get(self.model_env)
|
||||
fallback_model = os.environ.get(self.fallback_model_env) if self.fallback_model_env else None
|
||||
env_model = read_model_env(self.model_env)
|
||||
fallback_model = read_model_env(self.fallback_model_env) if self.fallback_model_env else None
|
||||
self.default_model = default_model or env_model or fallback_model or DEFAULT_ASTRAFLOW_MODEL
|
||||
self.client = OpenAI(api_key=self.api_key, base_url=self.base_url, _enforce_credentials=False)
|
||||
self._models = [
|
||||
|
||||
@@ -16,6 +16,7 @@ from llm.core.interface import (
|
||||
)
|
||||
from llm.core.types import LLMInput, LLMOutput, ModelInfo, ProviderType, ToolCall
|
||||
from llm.providers.constants import EMPTY_FILTERED_RESPONSE_ERROR
|
||||
from llm.providers.model_config import read_model_env
|
||||
from llm.providers.openai_compat import to_openai_message_param
|
||||
|
||||
ATLAS_BASE_URL = "https://api.atlascloud.ai/v1"
|
||||
@@ -68,7 +69,7 @@ class AtlasProvider(LLMProvider):
|
||||
or ""
|
||||
)
|
||||
self.base_url = base_url or os.environ.get(self.base_url_env, self.default_base_url)
|
||||
env_model = os.environ.get(self.model_env)
|
||||
env_model = read_model_env(self.model_env)
|
||||
self.default_model = default_model or env_model or DEFAULT_ATLAS_MODEL
|
||||
self.client = OpenAI(api_key=self.api_key, base_url=self.base_url, _enforce_credentials=False)
|
||||
self._models = [
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
"""Model environment parsing shared by the resolver and provider adapters."""
|
||||
|
||||
import os
|
||||
|
||||
|
||||
def read_model_env(name: str) -> str | None:
|
||||
"""Read a model ID, treating surrounding whitespace and quotes as config syntax."""
|
||||
value = os.environ.get(name, "").strip()
|
||||
if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}:
|
||||
value = value[1:-1].strip()
|
||||
return value or None
|
||||
@@ -19,6 +19,7 @@ from llm.core.types import (
|
||||
ProviderType,
|
||||
ToolCall,
|
||||
)
|
||||
from llm.providers.model_config import read_model_env
|
||||
|
||||
|
||||
class OllamaProvider(LLMProvider):
|
||||
@@ -30,7 +31,7 @@ class OllamaProvider(LLMProvider):
|
||||
default_model: str | None = None,
|
||||
) -> None:
|
||||
self.base_url = base_url or os.environ.get("OLLAMA_BASE_URL", "http://localhost:11434")
|
||||
self.default_model = default_model or os.environ.get("OLLAMA_MODEL", "llama3.2")
|
||||
self.default_model = default_model or read_model_env("OLLAMA_MODEL") or "llama3.2"
|
||||
self._models = [
|
||||
ModelInfo(
|
||||
name="llama3.2",
|
||||
|
||||
@@ -10,6 +10,7 @@ from llm.core.types import ProviderType
|
||||
from llm.providers.astraflow import AstraflowCNProvider, AstraflowProvider
|
||||
from llm.providers.atlas import AtlasProvider
|
||||
from llm.providers.claude import ClaudeProvider
|
||||
from llm.providers.model_config import read_model_env
|
||||
from llm.providers.ollama import OllamaProvider
|
||||
from llm.providers.openai import OpenAIProvider
|
||||
|
||||
@@ -78,19 +79,19 @@ def _resolve_default_model(
|
||||
provider_type: ProviderType, provider_cls: type[LLMProvider], saved_model: str | None,
|
||||
) -> str | None:
|
||||
for name in _MODEL_ENV_VARS[provider_cls]:
|
||||
model = os.environ.get(name)
|
||||
model = read_model_env(name)
|
||||
if model:
|
||||
return model
|
||||
|
||||
env_provider = _strip_env_value(os.environ.get("LLM_PROVIDER", "")).lower()
|
||||
if not env_provider or env_provider == provider_type.value:
|
||||
env_model = _strip_env_value(os.environ.get("LLM_MODEL", ""))
|
||||
env_model = read_model_env("LLM_MODEL")
|
||||
if env_model:
|
||||
return env_model
|
||||
return saved_model or None
|
||||
|
||||
|
||||
def get_provider(provider_type: ProviderType | str | None = None, **kwargs: str) -> LLMProvider:
|
||||
def get_provider(provider_type: ProviderType | str | None = None, **kwargs: str | None) -> LLMProvider:
|
||||
"""Create a provider, honoring the model selected by llm-select.
|
||||
|
||||
Built-in model defaults prefer explicit kwargs, then existing provider-specific
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
from collections.abc import Callable
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Literal
|
||||
|
||||
import pytest
|
||||
|
||||
from llm.cli.selector import save_config
|
||||
from llm.core.interface import LLMProvider
|
||||
from llm.core.types import ProviderType
|
||||
from llm.providers.astraflow import AstraflowCNProvider, AstraflowProvider
|
||||
from llm.providers.atlas import AtlasProvider
|
||||
from llm.providers.ollama import OllamaProvider
|
||||
from llm.providers.resolver import get_provider
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ProviderCase:
|
||||
provider_type: ProviderType
|
||||
constructor: Callable[..., LLMProvider]
|
||||
model_env: str
|
||||
builtin_model: str
|
||||
|
||||
|
||||
PROVIDERS = (
|
||||
ProviderCase(ProviderType.OLLAMA, OllamaProvider, "OLLAMA_MODEL", "llama3.2"),
|
||||
ProviderCase(ProviderType.ATLAS, AtlasProvider, "ATLAS_MODEL", "deepseek-ai/deepseek-v4-pro"),
|
||||
ProviderCase(ProviderType.ASTRAFLOW, AstraflowProvider, "ASTRAFLOW_MODEL", "gpt-4o-mini"),
|
||||
ProviderCase(ProviderType.ASTRAFLOW_CN, AstraflowCNProvider, "ASTRAFLOW_CN_MODEL", "gpt-4o-mini"),
|
||||
)
|
||||
PROVIDER_IDS = tuple(case.provider_type.value for case in PROVIDERS)
|
||||
BLANK_MODELS = (" \t ", ' " \t " ')
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def isolated_model_config(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
||||
monkeypatch.chdir(tmp_path)
|
||||
for name in (
|
||||
"LLM_PROVIDER", "LLM_MODEL", "OLLAMA_MODEL", "ATLAS_MODEL",
|
||||
"ASTRAFLOW_MODEL", "ASTRAFLOW_CN_MODEL", "OLLAMA_BASE_URL",
|
||||
"ATLAS_BASE_URL", "ASTRAFLOW_BASE_URL", "ASTRAFLOW_CN_BASE_URL",
|
||||
):
|
||||
# Record even absent variables: save_config(persist=True) writes them directly.
|
||||
monkeypatch.setenv(name, "")
|
||||
monkeypatch.delenv(name)
|
||||
for name in ("ATLAS_API_KEY", "ASTRAFLOW_API_KEY", "ASTRAFLOW_CN_API_KEY"):
|
||||
monkeypatch.setenv(name, "test-key")
|
||||
|
||||
|
||||
def set_provider_model(
|
||||
monkeypatch: pytest.MonkeyPatch, case: ProviderCase, value: str
|
||||
) -> None:
|
||||
monkeypatch.setenv(case.model_env, value)
|
||||
if case.provider_type == ProviderType.ASTRAFLOW_CN:
|
||||
monkeypatch.setenv("ASTRAFLOW_MODEL", value)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("case", PROVIDERS, ids=PROVIDER_IDS)
|
||||
@pytest.mark.parametrize("blank_model", BLANK_MODELS, ids=("whitespace", "quoted-whitespace"))
|
||||
@pytest.mark.parametrize("source", ("environment", "saved"))
|
||||
def test_blank_provider_model_falls_back_to_selected_model(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
case: ProviderCase,
|
||||
blank_model: str,
|
||||
source: Literal["environment", "saved"],
|
||||
) -> None:
|
||||
save_config(case.provider_type.value, "Selected/MixedCase-model", persist=source == "environment")
|
||||
set_provider_model(monkeypatch, case, blank_model)
|
||||
|
||||
assert get_provider().get_default_model() == "Selected/MixedCase-model"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("case", PROVIDERS, ids=PROVIDER_IDS)
|
||||
@pytest.mark.parametrize("blank_model", BLANK_MODELS, ids=("whitespace", "quoted-whitespace"))
|
||||
def test_blank_provider_model_without_selection_retains_builtin_default(
|
||||
monkeypatch: pytest.MonkeyPatch, case: ProviderCase, blank_model: str
|
||||
) -> None:
|
||||
set_provider_model(monkeypatch, case, blank_model)
|
||||
|
||||
assert get_provider(case.provider_type).get_default_model() == case.builtin_model
|
||||
assert case.constructor().get_default_model() == case.builtin_model
|
||||
|
||||
|
||||
@pytest.mark.parametrize("case", PROVIDERS, ids=PROVIDER_IDS)
|
||||
@pytest.mark.parametrize("model", (' "Provider/MixedCase-model" ', " 'Provider/MixedCase-model' "))
|
||||
def test_quoted_provider_model_is_normalized_without_changing_case(
|
||||
monkeypatch: pytest.MonkeyPatch, case: ProviderCase, model: str
|
||||
) -> None:
|
||||
monkeypatch.setenv("LLM_MODEL", "selected-model")
|
||||
monkeypatch.setenv(case.model_env, model)
|
||||
|
||||
assert get_provider(case.provider_type).get_default_model() == "Provider/MixedCase-model"
|
||||
assert case.constructor().get_default_model() == "Provider/MixedCase-model"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("case", PROVIDERS, ids=PROVIDER_IDS)
|
||||
def test_explicit_default_model_keeps_precedence_and_original_value(
|
||||
monkeypatch: pytest.MonkeyPatch, case: ProviderCase
|
||||
) -> None:
|
||||
save_config(case.provider_type.value, "selected-model", persist=True)
|
||||
set_provider_model(monkeypatch, case, ' "Provider/MixedCase-model" ')
|
||||
explicit_model = ' "Explicit/MixedCase-model" '
|
||||
|
||||
assert get_provider(default_model=explicit_model).get_default_model() == explicit_model
|
||||
assert case.constructor(default_model=explicit_model).get_default_model() == explicit_model
|
||||
|
||||
|
||||
@pytest.mark.parametrize("blank_model", BLANK_MODELS, ids=("whitespace", "quoted-whitespace"))
|
||||
def test_cn_blank_specific_model_uses_normalized_shared_model(
|
||||
monkeypatch: pytest.MonkeyPatch, blank_model: str
|
||||
) -> None:
|
||||
monkeypatch.setenv("ASTRAFLOW_CN_MODEL", blank_model)
|
||||
monkeypatch.setenv("ASTRAFLOW_MODEL", ' "Shared/MixedCase-model" ')
|
||||
monkeypatch.setenv("LLM_MODEL", "selected-model")
|
||||
|
||||
assert get_provider(ProviderType.ASTRAFLOW_CN).get_default_model() == "Shared/MixedCase-model"
|
||||
assert AstraflowCNProvider().get_default_model() == "Shared/MixedCase-model"
|
||||
+38
-19
@@ -1,5 +1,6 @@
|
||||
import io
|
||||
import json
|
||||
from pathlib import Path
|
||||
from unittest.mock import Mock
|
||||
|
||||
import pytest
|
||||
@@ -111,7 +112,7 @@ class TestGetProvider:
|
||||
|
||||
class TestSelectedModel:
|
||||
@pytest.fixture(autouse=True)
|
||||
def isolated_config(self, monkeypatch, tmp_path):
|
||||
def isolated_config(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
||||
monkeypatch.chdir(tmp_path)
|
||||
for name in (
|
||||
"LLM_PROVIDER", "LLM_MODEL", "OLLAMA_MODEL", "ATLAS_MODEL",
|
||||
@@ -124,7 +125,7 @@ class TestSelectedModel:
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "test-key")
|
||||
|
||||
@pytest.mark.parametrize("persist", [False, True])
|
||||
def test_selector_model_reaches_actual_request(self, monkeypatch, persist):
|
||||
def test_selector_model_reaches_actual_request(self, monkeypatch: pytest.MonkeyPatch, persist: bool) -> None:
|
||||
save_config("ollama", "mistral", persist=persist)
|
||||
urlopen = Mock(side_effect=lambda *args, **kwargs: io.BytesIO(
|
||||
b'{"message":{"content":"ok"},"model":"mistral"}'
|
||||
@@ -142,7 +143,9 @@ class TestSelectedModel:
|
||||
|
||||
@pytest.mark.parametrize("provider_type", list(ProviderType))
|
||||
@pytest.mark.parametrize("source", ["saved", "environment"])
|
||||
def test_selected_model_applies_to_every_builtin(self, monkeypatch, provider_type, source):
|
||||
def test_selected_model_applies_to_every_builtin(
|
||||
self, monkeypatch: pytest.MonkeyPatch, provider_type: ProviderType, source: str,
|
||||
) -> None:
|
||||
save_config(provider_type.value, "Selected/MixedCase-model", persist=source == "environment")
|
||||
|
||||
provider = get_provider()
|
||||
@@ -150,7 +153,7 @@ class TestSelectedModel:
|
||||
assert provider.provider_type == provider_type
|
||||
assert provider.get_default_model() == "Selected/MixedCase-model"
|
||||
|
||||
def test_environment_model_overrides_saved_model(self, monkeypatch):
|
||||
def test_environment_model_overrides_saved_model(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
save_config("ollama", "mistral")
|
||||
monkeypatch.setenv("LLM_MODEL", "codellama")
|
||||
|
||||
@@ -163,14 +166,16 @@ class TestSelectedModel:
|
||||
("astraflow_cn", "ASTRAFLOW_CN_MODEL"),
|
||||
("astraflow_cn", "ASTRAFLOW_MODEL"),
|
||||
])
|
||||
def test_provider_environment_retains_precedence(self, monkeypatch, provider_type, model_env):
|
||||
def test_provider_environment_retains_precedence(
|
||||
self, monkeypatch: pytest.MonkeyPatch, provider_type: str, model_env: str,
|
||||
) -> None:
|
||||
save_config(provider_type, "saved-model")
|
||||
monkeypatch.setenv("LLM_MODEL", "global-model")
|
||||
monkeypatch.setenv(model_env, "provider-model")
|
||||
|
||||
assert get_provider().get_default_model() == "provider-model"
|
||||
|
||||
def test_cn_specific_environment_overrides_shared_fallback(self, monkeypatch):
|
||||
def test_cn_specific_environment_overrides_shared_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
save_config("astraflow_cn", "saved-model")
|
||||
monkeypatch.setenv("ASTRAFLOW_MODEL", "shared-model")
|
||||
monkeypatch.setenv("ASTRAFLOW_CN_MODEL", "cn-model")
|
||||
@@ -178,21 +183,27 @@ class TestSelectedModel:
|
||||
assert get_provider().get_default_model() == "cn-model"
|
||||
|
||||
@pytest.mark.parametrize("provider_type", list(ProviderType))
|
||||
def test_explicit_default_model_overrides_selection(self, monkeypatch, provider_type):
|
||||
def test_explicit_default_model_overrides_selection(
|
||||
self, monkeypatch: pytest.MonkeyPatch, provider_type: ProviderType,
|
||||
) -> None:
|
||||
save_config(provider_type.value, "saved-model", persist=True)
|
||||
monkeypatch.setenv("OLLAMA_MODEL", "provider-model")
|
||||
|
||||
assert get_provider(default_model="explicit-model").get_default_model() == "explicit-model"
|
||||
|
||||
@pytest.mark.parametrize("default_model", [None, ""])
|
||||
def test_explicit_empty_default_retains_constructor_fallback(self, monkeypatch, default_model):
|
||||
def test_explicit_empty_default_retains_constructor_fallback(
|
||||
self, monkeypatch: pytest.MonkeyPatch, default_model: str | None,
|
||||
) -> None:
|
||||
save_config("ollama", "saved-model", persist=True)
|
||||
monkeypatch.setenv("OLLAMA_MODEL", "provider-model")
|
||||
|
||||
assert get_provider(default_model=default_model).get_default_model() == "provider-model"
|
||||
|
||||
@pytest.mark.parametrize("explicit", [False, True])
|
||||
def test_provider_override_does_not_inherit_saved_model(self, monkeypatch, explicit):
|
||||
def test_provider_override_does_not_inherit_saved_model(
|
||||
self, monkeypatch: pytest.MonkeyPatch, explicit: bool,
|
||||
) -> None:
|
||||
save_config("openai", "gpt-selected")
|
||||
if explicit:
|
||||
provider = get_provider("ollama")
|
||||
@@ -202,14 +213,18 @@ class TestSelectedModel:
|
||||
|
||||
assert provider.get_default_model() == "llama3.2"
|
||||
|
||||
def test_explicit_provider_does_not_inherit_another_environment_model(self, monkeypatch):
|
||||
def test_explicit_provider_does_not_inherit_another_environment_model(
|
||||
self, monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setenv("LLM_PROVIDER", "openai")
|
||||
monkeypatch.setenv("LLM_MODEL", "gpt-selected")
|
||||
|
||||
assert get_provider("ollama").get_default_model() == "llama3.2"
|
||||
|
||||
@pytest.mark.parametrize("env_provider", [None, " 'OLLAMA' "])
|
||||
def test_environment_model_applies_to_matching_explicit_provider(self, monkeypatch, env_provider):
|
||||
def test_environment_model_applies_to_matching_explicit_provider(
|
||||
self, monkeypatch: pytest.MonkeyPatch, env_provider: str | None,
|
||||
) -> None:
|
||||
if env_provider is not None:
|
||||
monkeypatch.setenv("LLM_PROVIDER", env_provider)
|
||||
monkeypatch.setenv("LLM_MODEL", ' "MixedCase/Model" ')
|
||||
@@ -218,31 +233,35 @@ class TestSelectedModel:
|
||||
|
||||
@pytest.mark.parametrize("source", ["saved", "environment"])
|
||||
@pytest.mark.parametrize("model", ["", " ", '""'])
|
||||
def test_empty_selected_model_retains_default(self, monkeypatch, source, model):
|
||||
def test_empty_selected_model_retains_default(
|
||||
self, monkeypatch: pytest.MonkeyPatch, source: str, model: str,
|
||||
) -> None:
|
||||
save_config("ollama", model, persist=source == "environment")
|
||||
|
||||
assert get_provider().get_default_model() == "llama3.2"
|
||||
|
||||
def test_empty_environment_model_falls_back_to_saved_model(self, monkeypatch):
|
||||
def test_empty_environment_model_falls_back_to_saved_model(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
save_config("ollama", "mistral")
|
||||
monkeypatch.setenv("LLM_MODEL", " ")
|
||||
|
||||
assert get_provider().get_default_model() == "mistral"
|
||||
|
||||
def test_quoted_saved_model_preserves_case(self, tmp_path):
|
||||
def test_quoted_saved_model_preserves_case(self, tmp_path: Path) -> None:
|
||||
tmp_path.joinpath(".llm.env").write_text(
|
||||
"LLM_PROVIDER='OLLAMA'\nLLM_MODEL='MixedCase/Model'\n"
|
||||
)
|
||||
|
||||
assert get_provider().get_default_model() == "MixedCase/Model"
|
||||
|
||||
def test_unpaired_saved_model_does_not_override_default_provider(self, tmp_path):
|
||||
def test_unpaired_saved_model_does_not_override_default_provider(self, tmp_path: Path) -> None:
|
||||
tmp_path.joinpath(".llm.env").write_text("LLM_MODEL=orphan-model\n")
|
||||
|
||||
assert get_provider().get_default_model() == "claude-sonnet-4-6"
|
||||
|
||||
@pytest.mark.parametrize("explicit", [False, True])
|
||||
def test_provider_override_does_not_read_invalid_saved_file(self, monkeypatch, tmp_path, explicit):
|
||||
def test_provider_override_does_not_read_invalid_saved_file(
|
||||
self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path, explicit: bool,
|
||||
) -> None:
|
||||
tmp_path.joinpath(".llm.env").write_bytes(b"\xff\xfe")
|
||||
monkeypatch.setenv("LLM_MODEL", "mistral")
|
||||
if explicit:
|
||||
@@ -253,9 +272,9 @@ class TestSelectedModel:
|
||||
|
||||
assert provider.get_default_model() == "mistral"
|
||||
|
||||
def test_registered_provider_keeps_its_constructor_contract(self, monkeypatch):
|
||||
def test_registered_provider_keeps_its_constructor_contract(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
class CustomProvider(OllamaProvider):
|
||||
def __init__(self, base_url=None):
|
||||
def __init__(self, base_url: str | None = None) -> None:
|
||||
super().__init__(base_url=base_url, default_model="custom-default")
|
||||
|
||||
monkeypatch.setattr(resolver, "_PROVIDER_MAP", resolver._PROVIDER_MAP.copy())
|
||||
@@ -268,7 +287,7 @@ class TestSelectedModel:
|
||||
assert provider.base_url == "http://localhost:1234"
|
||||
assert provider.get_default_model() == "custom-default"
|
||||
|
||||
def test_registered_provider_receives_explicit_default_model(self, monkeypatch):
|
||||
def test_registered_provider_receives_explicit_default_model(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
class CustomProvider(OllamaProvider):
|
||||
pass
|
||||
|
||||
|
||||
Reference in New Issue
Block a user