diff --git a/ebuild/eos_ai/llm_integration.py b/ebuild/eos_ai/llm_integration.py index 2a530e0..5436dbe 100644 --- a/ebuild/eos_ai/llm_integration.py +++ b/ebuild/eos_ai/llm_integration.py @@ -61,19 +61,36 @@ def _http_url(url: str) -> str: return url +def _ensure_scheme(url: str) -> str: + """Prefix ``http://`` to a schemeless host such as ``OLLAMA_HOST``. + + Ollama documents ``OLLAMA_HOST`` as ``host:port`` (``0.0.0.0:11434``), + which ``urlparse`` reads as scheme ``0.0.0.0`` and ``_http_url`` refuses. + A value that already names a scheme is returned unchanged. + """ + url = url.strip() + return url if "://" in url else f"http://{url}" + + def _openai_chat_url(base_url: str) -> str: """Join *base_url* to the chat-completions path without doubling ``/v1``. OpenAI-compatible servers document the base as either the origin (``https://api.openai.com``) or the v1 root (``https://api.openai.com/v1``). - Always appending ``/v1/chat/completions`` made the second form 404. + Always appending ``/v1/chat/completions`` made the second form 404. A URL + that already ends in ``/chat/completions`` is returned as it is. """ base = _http_url(base_url).rstrip("/") + if base.endswith("/chat/completions"): + return base if base.endswith("/v1"): return f"{base}/chat/completions" return f"{base}/v1/chat/completions" +_normalize_openai_url = _openai_chat_url + + def _error_text(exc: BaseException) -> str: """Render *exc* for the caller without echoing credentials.""" text = str(exc) @@ -132,29 +149,53 @@ class LLMClient: OLLAMA_URL = "http://localhost:11434" OPENAI_URL = "https://api.openai.com" + # Per-provider default model, used only when neither the caller nor the + # provider's environment variable names one. ``model`` defaults to None + # rather than a real model name: a sentinel such as "llama3" cannot tell + # "not given" from an explicit model="llama3". + DEFAULT_MODELS = { + "ollama": "llama3", + "openai": "gpt-4o-mini", + "custom": "default", + } + MODEL_ENV = { + "ollama": "OLLAMA_MODEL", + "openai": "OPENAI_MODEL", + "custom": "EOS_LLM_MODEL", + } + def __init__( self, provider: str = "ollama", - model: str = "llama3", + model: Optional[str] = None, api_key: Optional[str] = None, base_url: Optional[str] = None, timeout: int = 120, ): self.provider = provider - self.model = model self.api_key = api_key self.base_url = base_url self.timeout = timeout + env_var = self.MODEL_ENV.get(provider) + env_model = os.environ.get(env_var, "") if env_var else "" + self.model = model or env_model or self.DEFAULT_MODELS.get(provider, "none") + if provider == "ollama": - self.base_url = base_url or self.OLLAMA_URL + self.base_url = base_url or self._ollama_env_url() or self.OLLAMA_URL elif provider == "openai": - self.base_url = base_url or self.OPENAI_URL + self.base_url = base_url or os.environ.get("OPENAI_BASE_URL", "") or self.OPENAI_URL self.api_key = api_key or os.environ.get("OPENAI_API_KEY", "") elif provider == "custom": - self.base_url = base_url or "" + self.base_url = base_url or os.environ.get("EOS_LLM_URL", "") self.api_key = api_key or os.environ.get("EOS_LLM_API_KEY", "") + @staticmethod + def _ollama_env_url() -> str: + """``OLLAMA_HOST`` as a URL, or "" when it is unset.""" + host = os.environ.get("OLLAMA_HOST", "").strip() + return _ensure_scheme(host) if host else "" + @classmethod def auto(cls) -> "LLMClient": """Auto-detect available LLM provider. @@ -165,22 +206,19 @@ def auto(cls) -> "LLMClient": 3. EOS_LLM_API_KEY + EOS_LLM_URL in environment 4. None (returns a client that will fail gracefully) """ - # Try Ollama - if cls._check_ollama(): - return cls(provider="ollama", model="llama3") + # Try Ollama, where OLLAMA_HOST says it is (localhost by default) + if cls._check_ollama(cls._ollama_env_url() or None): + return cls(provider="ollama") # Try OpenAI openai_key = os.environ.get("OPENAI_API_KEY", "") if openai_key: - return cls(provider="openai", model="gpt-4o-mini", api_key=openai_key) + return cls(provider="openai", api_key=openai_key) - # Try custom - custom_key = os.environ.get("EOS_LLM_API_KEY", "") - custom_url = os.environ.get("EOS_LLM_URL", "") - if custom_key and custom_url: - model = os.environ.get("EOS_LLM_MODEL", "default") - return cls(provider="custom", model=model, - api_key=custom_key, base_url=custom_url) + # Try custom. The key is optional: local OpenAI-compatible servers + # (vLLM, llama.cpp, LM Studio) usually run without one. + if os.environ.get("EOS_LLM_URL", ""): + return cls(provider="custom") # No provider available return cls(provider="none", model="none") @@ -214,7 +252,9 @@ def is_available(self) -> bool: if self.provider == "ollama": return self._check_ollama(self.base_url) if self.provider in ("openai", "custom"): - if not (self.api_key and self.base_url): + # openai needs a key; a custom endpoint may be a keyless local + # server, so only its URL is required. + if not self.base_url or (self.provider == "openai" and not self.api_key): return False try: _http_url(self.base_url) @@ -300,25 +340,47 @@ def _call_openai_compat(self, prompt: str, system: str) -> LLMResponse: } data = json.dumps(payload).encode("utf-8") - headers = { - "Content-Type": "application/json", - "Authorization": f"Bearer {self.api_key}", - } - req = urllib.request.Request(url, data=data, headers=headers, method="POST") + req = urllib.request.Request( + url, data=data, headers=self._build_headers(), method="POST" + ) with urllib.request.urlopen(req, timeout=self.timeout) as resp: body = json.loads(_read_limited(resp).decode("utf-8")) - choices = body.get("choices") or [{}] - choice = choices[0] if choices else {} - message = choice.get("message") or {} - usage = body.get("usage") or {} + return self._parse_openai_response(body) + + def _build_headers(self) -> dict: + """Request headers; Authorization only when there is a key. + + A bare ``Authorization: Bearer `` (empty key) is rejected by some + keyless local servers and says nothing to the rest. + """ + headers = {"Content-Type": "application/json"} + if self.api_key: + headers["Authorization"] = f"Bearer {self.api_key}" + return headers + + def _parse_openai_response(self, body: dict) -> LLMResponse: + """Turn a chat-completions body into an LLMResponse. + No choices is a failure with its own message, not an empty success. + """ + choices = body.get("choices") or [] + model = body.get("model", self.model) + usage = body.get("usage") or {} + tokens = int(usage.get("total_tokens") or 0) + if not choices: + return LLMResponse( + text="", model=model, provider=self.provider, + tokens_used=tokens, success=False, + error="LLM returned no completion choices", + ) + message = (choices[0] or {}).get("message") or {} return _completed( text=message.get("content") or "", - model=body.get("model", self.model), + model=model, provider=self.provider, - tokens_used=int(usage.get("total_tokens") or 0), + tokens_used=tokens, ) def get_provider_info(self) -> str: diff --git a/tests/unit/test_llm_client.py b/tests/unit/test_llm_client.py new file mode 100644 index 0000000..d2310ac --- /dev/null +++ b/tests/unit/test_llm_client.py @@ -0,0 +1,347 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""Unit tests for LLMClient (ebuild.eos_ai.llm_integration). + +All tests are pure Python with zero network calls — every outbound request +is intercepted by unittest.mock so the suite runs fully offline and is safe +to run in CI without credentials. + +Coverage targets: +- URL normalisation (_normalize_openai_url, _ensure_scheme) +- Environment variable priority (OLLAMA_HOST, OPENAI_BASE_URL, OPENAI_MODEL, …) +- Authorization header behaviour (present ↔ absent) +- Response parsing (_parse_openai_response, including empty-choices guard) +- Provider auto-detection (LLMClient.auto()) +- is_available() for every provider branch +- get_provider_info() formatting +""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from ebuild.eos_ai.llm_integration import ( + LLMClient, + _ensure_scheme, + _normalize_openai_url, +) + + +# ── Fixtures ────────────────────────────────────────────────────────────────── + + +@pytest.fixture(autouse=True) +def clean_env(monkeypatch): + """Strip every LLM-related env var before each test for full isolation.""" + for var in ( + "OPENAI_API_KEY", + "OPENAI_BASE_URL", + "OPENAI_MODEL", + "OLLAMA_HOST", + "OLLAMA_MODEL", + "EOS_LLM_API_KEY", + "EOS_LLM_URL", + "EOS_LLM_MODEL", + ): + monkeypatch.delenv(var, raising=False) + + +# ── _normalize_openai_url ───────────────────────────────────────────────────── + + +class TestNormalizeOpenAIUrl: + """_normalize_openai_url must produce exactly one /v1/chat/completions.""" + + def test_base_domain_appends_full_path(self): + assert _normalize_openai_url("https://api.openai.com") == ( + "https://api.openai.com/v1/chat/completions" + ) + + def test_versioned_base_no_double_v1(self): + result = _normalize_openai_url("http://localhost:8000/v1") + assert result == "http://localhost:8000/v1/chat/completions" + assert "/v1/v1/" not in result + + def test_trailing_slash_stripped_then_path_appended(self): + result = _normalize_openai_url("http://localhost:8000/v1/") + assert result == "http://localhost:8000/v1/chat/completions" + + def test_full_url_is_idempotent(self): + full = "http://localhost:8000/v1/chat/completions" + assert _normalize_openai_url(full) == full + + def test_no_slash_domain(self): + result = _normalize_openai_url("http://myproxy.internal") + assert result.endswith("/v1/chat/completions") + + +# ── _ensure_scheme ──────────────────────────────────────────────────────────── + + +class TestEnsureScheme: + """_ensure_scheme must prepend http:// to schemeless hosts only.""" + + def test_bare_host_port_gets_http(self): + assert _ensure_scheme("192.168.1.50:11434") == "http://192.168.1.50:11434" + + def test_bare_hostname_gets_http(self): + assert _ensure_scheme("myserver") == "http://myserver" + + def test_existing_http_preserved(self): + assert _ensure_scheme("http://localhost:11434") == "http://localhost:11434" + + def test_existing_https_preserved(self): + assert _ensure_scheme("https://secure-host:11434") == "https://secure-host:11434" + + +# ── LLMClient.__init__ — env var support ────────────────────────────────────── + + +class TestEnvVarSupport: + """Constructor must read the documented environment variables.""" + + def test_ollama_host_env_var(self, monkeypatch): + monkeypatch.setenv("OLLAMA_HOST", "http://remote-box:11434") + client = LLMClient(provider="ollama") + assert "remote-box" in client.base_url + + def test_ollama_host_bare_gets_scheme(self, monkeypatch): + monkeypatch.setenv("OLLAMA_HOST", "192.168.1.10:11434") + client = LLMClient(provider="ollama") + assert client.base_url.startswith("http://") + + def test_ollama_model_env_var(self, monkeypatch): + monkeypatch.setenv("OLLAMA_MODEL", "mistral") + client = LLMClient(provider="ollama") + assert client.model == "mistral" + + def test_openai_base_url_env_var(self, monkeypatch): + monkeypatch.setenv("OPENAI_BASE_URL", "http://localhost:8000/v1") + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + client = LLMClient(provider="openai") + assert "localhost:8000" in (client.base_url or "") + + def test_openai_model_env_var(self, monkeypatch): + monkeypatch.setenv("OPENAI_MODEL", "gpt-4-turbo") + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + client = LLMClient(provider="openai") + assert client.model == "gpt-4-turbo" + + def test_eos_custom_env_vars(self, monkeypatch): + monkeypatch.setenv("EOS_LLM_URL", "http://custom:9000") + monkeypatch.setenv("EOS_LLM_API_KEY", "mykey") + monkeypatch.setenv("EOS_LLM_MODEL", "phi3") + client = LLMClient(provider="custom") + assert client.base_url == "http://custom:9000" + assert client.api_key == "mykey" + assert client.model == "phi3" + + +# ── _build_headers — Authorization guard ────────────────────────────────────── + + +class TestBuildHeaders: + """Authorization header must be absent when the API key is empty.""" + + def test_empty_api_key_no_auth_header(self): + """Local servers (vLLM, llama.cpp) must not receive an Authorization header.""" + client = LLMClient(provider="custom", base_url="http://localhost:8000", api_key="") + headers = client._build_headers() + assert "Authorization" not in headers + + def test_non_empty_api_key_sends_bearer(self): + client = LLMClient(provider="openai", api_key="sk-abc123") + headers = client._build_headers() + assert headers.get("Authorization") == "Bearer sk-abc123" + + def test_content_type_always_present(self): + client = LLMClient(provider="ollama") + assert client._build_headers()["Content-Type"] == "application/json" + + +# ── _parse_openai_response ──────────────────────────────────────────────────── + + +class TestParseOpenAIResponse: + """_parse_openai_response must guard against empty/missing choices.""" + + def _make_client(self) -> LLMClient: + return LLMClient(provider="openai", api_key="sk-test") + + def test_empty_choices_returns_failure(self): + """An empty choices list must not raise IndexError and must be success=False.""" + client = self._make_client() + body = {"choices": [], "model": "gpt-4o", "usage": {"total_tokens": 0}} + resp = client._parse_openai_response(body) + assert resp.success is False + assert "no completion" in resp.error.lower() + + def test_empty_content_returns_failure(self): + client = self._make_client() + body = { + "choices": [{"message": {"content": ""}}], + "model": "gpt-4o", + "usage": {}, + } + resp = client._parse_openai_response(body) + assert resp.success is False + + def test_valid_response_parsed_correctly(self): + client = self._make_client() + body = { + "choices": [{"message": {"content": "STM32H7 with UART detected"}}], + "model": "gpt-4o", + "usage": {"total_tokens": 42}, + } + resp = client._parse_openai_response(body) + assert resp.success is True + assert resp.text == "STM32H7 with UART detected" + assert resp.tokens_used == 42 + assert resp.model == "gpt-4o" + + def test_missing_choices_key_returns_failure(self): + client = self._make_client() + body = {"model": "gpt-4o"} # no 'choices' key at all + resp = client._parse_openai_response(body) + assert resp.success is False + + +# ── LLMClient.auto() ────────────────────────────────────────────────────────── + + +class TestAutoDetect: + """auto() must select providers in documented priority order.""" + + def test_auto_prefers_ollama_when_available(self): + with patch.object(LLMClient, "_check_ollama", return_value=True): + client = LLMClient.auto() + assert client.provider == "ollama" + + def test_auto_picks_openai_when_ollama_unavailable(self, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-key") + with patch.object(LLMClient, "_check_ollama", return_value=False): + client = LLMClient.auto() + assert client.provider == "openai" + + def test_auto_picks_custom_when_no_ollama_no_openai(self, monkeypatch): + monkeypatch.setenv("EOS_LLM_API_KEY", "mykey") + monkeypatch.setenv("EOS_LLM_URL", "http://custom:9000") + with patch.object(LLMClient, "_check_ollama", return_value=False): + client = LLMClient.auto() + assert client.provider == "custom" + + def test_auto_returns_none_when_nothing_available(self): + with patch.object(LLMClient, "_check_ollama", return_value=False): + client = LLMClient.auto() + assert client.provider == "none" + + def test_auto_respects_ollama_model_env_var(self, monkeypatch): + monkeypatch.setenv("OLLAMA_MODEL", "phi3") + with patch.object(LLMClient, "_check_ollama", return_value=True): + client = LLMClient.auto() + assert client.model == "phi3" + + +# ── is_available() ──────────────────────────────────────────────────────────── + + +class TestIsAvailable: + + def test_none_provider_not_available(self): + client = LLMClient(provider="none") + assert client.is_available() is False + + def test_openai_with_key_and_url_is_available(self): + client = LLMClient(provider="openai", api_key="sk-x", base_url="https://api.openai.com") + assert client.is_available() is True + + def test_openai_without_key_not_available(self): + client = LLMClient(provider="openai", api_key="", base_url="https://api.openai.com") + assert client.is_available() is False + + def test_ollama_availability_delegates_to_check(self): + client = LLMClient(provider="ollama") + with patch.object(LLMClient, "_check_ollama", return_value=True): + assert client.is_available() is True + with patch.object(LLMClient, "_check_ollama", return_value=False): + assert client.is_available() is False + + +# ── analyze() — no provider ─────────────────────────────────────────────────── + + +class TestAnalyzeNoProvider: + + def test_none_provider_returns_graceful_failure(self): + client = LLMClient(provider="none") + resp = client.analyze("describe this hardware") + assert resp.success is False + assert resp.text == "" + assert "no llm provider" in resp.error.lower() + + +# ── get_provider_info() ─────────────────────────────────────────────────────── + + +class TestGetProviderInfo: + + def test_none_provider_info(self): + assert "No LLM" in LLMClient(provider="none").get_provider_info() + + def test_ollama_provider_info_contains_model(self): + client = LLMClient(provider="ollama", model="llama3") + info = client.get_provider_info() + assert "llama3" in info + assert "Ollama" in info + + def test_openai_provider_info(self): + client = LLMClient(provider="openai", model="gpt-4o", api_key="sk-x") + info = client.get_provider_info() + assert "OpenAI" in info + assert "gpt-4o" in info + + +# ── Review items (ebuild#127, 2026-09-14) ───────────────────────────────────── + + +class TestReviewItems: + """Regressions for the four points raised in review.""" + + def test_explicit_llama3_is_kept_for_openai(self): + # A "llama3" sentinel replaced an explicit model="llama3" for openai. + client = LLMClient(provider="openai", model="llama3", api_key="k") + assert client.model == "llama3" + + def test_per_provider_default_when_no_model_given(self): + assert LLMClient(provider="ollama").model == "llama3" + assert LLMClient(provider="openai", api_key="k").model == "gpt-4o-mini" + assert LLMClient(provider="custom", base_url="http://x:1").model == "default" + + def test_keyless_custom_endpoint_is_available(self): + client = LLMClient(provider="custom", base_url="http://localhost:8000", api_key="") + assert client.is_available() is True + + def test_custom_without_url_is_not_available(self): + assert LLMClient(provider="custom", api_key="k").is_available() is False + + def test_auto_picks_keyless_custom(self, monkeypatch): + monkeypatch.setenv("EOS_LLM_URL", "http://localhost:8000/v1") + with patch.object(LLMClient, "_check_ollama", return_value=False): + client = LLMClient.auto() + assert client.provider == "custom" + assert client.is_available() is True + + def test_is_available_probes_the_configured_ollama_url(self): + client = LLMClient(provider="ollama", base_url="http://gpu-box:11434") + with patch.object(LLMClient, "_check_ollama", return_value=True) as probe: + assert client.is_available() is True + probe.assert_called_once_with("http://gpu-box:11434") + + def test_auto_probes_ollama_host(self, monkeypatch): + monkeypatch.setenv("OLLAMA_HOST", "0.0.0.0:11434") + with patch.object(LLMClient, "_check_ollama", return_value=False) as probe: + LLMClient.auto() + probe.assert_called_once_with("http://0.0.0.0:11434")