From 39779b051d2921927a342a2242d099c47f830ab7 Mon Sep 17 00:00:00 2001 From: Sayan Mandal Date: Fri, 11 Sep 2026 02:29:30 +0530 Subject: [PATCH] fix(eos_ai): normalize LLM URLs, add env var support, guard empty auth header - Add _normalize_openai_url() helper: prevents /v1/v1/chat/completions when base_url already contains /v1 (e.g. from OPENAI_BASE_URL env var) - Add _ensure_scheme() helper: prepends http:// to bare Ollama hosts like '192.168.1.50:11434' so urllib does not choke - Add _build_headers() method: omits Authorization header when api_key is empty, fixing compatibility with local servers (vLLM, llama.cpp) that reject a stray 'Bearer ' header - Add _parse_openai_response() method: returns success=False with a clear error message when choices is empty, instead of silently succeeding with empty text - Honour OLLAMA_HOST, OLLAMA_MODEL, OPENAI_BASE_URL, OPENAI_MODEL, EOS_LLM_URL, EOS_LLM_API_KEY, EOS_LLM_MODEL environment variables so the client is configurable without source changes - LLMClient.auto() picks up OLLAMA_MODEL and OPENAI_MODEL from env --- ebuild/eos_ai/llm_integration.py | 153 ++++++++--- ebuild/packages/recipe.py | 35 +++ ebuild/plugins/__init__.py | 2 +- tests/ebuild/test_build_dir_resolution.py | 1 - tests/ebuild/test_package_recipe.py | 2 +- tests/unit/test_ci_gate.py | 6 +- tests/unit/test_llm_client.py | 304 ++++++++++++++++++++++ 7 files changed, 468 insertions(+), 35 deletions(-) create mode 100644 tests/unit/test_llm_client.py diff --git a/ebuild/eos_ai/llm_integration.py b/ebuild/eos_ai/llm_integration.py index 57bbb094..63eedb36 100644 --- a/ebuild/eos_ai/llm_integration.py +++ b/ebuild/eos_ai/llm_integration.py @@ -21,6 +21,16 @@ client = LLMClient(provider="openai", model="gpt-4o") # uses OPENAI_API_KEY env var response = client.analyze(prompt) + +Environment variables: + OLLAMA_HOST Base URL for Ollama (default: http://localhost:11434) + OLLAMA_MODEL Model name for Ollama (default: llama3) + OPENAI_API_KEY API key for OpenAI + OPENAI_BASE_URL Base URL for OpenAI-compatible endpoint (default: https://api.openai.com) + OPENAI_MODEL Model name for OpenAI (default: gpt-4o-mini) + EOS_LLM_API_KEY API key for custom provider + EOS_LLM_URL Base URL for custom provider + EOS_LLM_MODEL Model name for custom provider """ from __future__ import annotations @@ -30,7 +40,7 @@ import urllib.error import urllib.request from dataclasses import dataclass -from typing import Optional +from typing import Dict, Optional @dataclass @@ -44,6 +54,37 @@ class LLMResponse: error: str = "" +def _normalize_openai_url(base_url: str) -> str: + """Normalize a base URL to the OpenAI chat completions endpoint. + + Handles all common forms users pass, preventing double path segments: + + - ``https://api.openai.com`` → .../v1/chat/completions + - ``http://localhost:8000/v1`` → .../v1/chat/completions (no /v1/v1) + - ``http://localhost:8000/v1/`` → .../v1/chat/completions (trailing slash) + - ``http://localhost:8000/v1/chat/completions`` → unchanged (idempotent) + """ + url = base_url.rstrip("/") + if url.endswith("/chat/completions"): + return url + if url.endswith("/v1"): + return url + "/chat/completions" + return url + "/v1/chat/completions" + + +def _ensure_scheme(host: str, default_scheme: str = "http") -> str: + """Prepend a scheme to a bare host string if one is missing. + + Example:: + + _ensure_scheme("192.168.1.50:11434") # → "http://192.168.1.50:11434" + _ensure_scheme("http://localhost:11434") # → unchanged + """ + if "://" in host: + return host + return f"{default_scheme}://{host}" + + class LLMClient: """Unified LLM client for hardware analysis. @@ -66,38 +107,94 @@ def __init__( timeout: int = 120, ): self.provider = provider - self.model = model - self.api_key = api_key - self.base_url = base_url self.timeout = timeout if provider == "ollama": - self.base_url = base_url or self.OLLAMA_URL + # Respect OLLAMA_HOST env var; ensure scheme is present on bare hosts + ollama_host = os.environ.get("OLLAMA_HOST", self.OLLAMA_URL) + self.base_url = base_url or _ensure_scheme(ollama_host) + self.model = model if model != "llama3" else os.environ.get("OLLAMA_MODEL", "llama3") + self.api_key = "" + elif provider == "openai": - self.base_url = base_url or self.OPENAI_URL + # Respect OPENAI_BASE_URL env var for custom-hosted OpenAI-compatible endpoints + openai_base = os.environ.get("OPENAI_BASE_URL", self.OPENAI_URL) + self.base_url = base_url or openai_base + self.model = model if model != "llama3" else os.environ.get("OPENAI_MODEL", "gpt-4o-mini") 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.model = model if model != "llama3" else os.environ.get("EOS_LLM_MODEL", "default") self.api_key = api_key or os.environ.get("EOS_LLM_API_KEY", "") + else: + self.base_url = base_url or "" + self.model = model + self.api_key = api_key or "" + + def _build_headers(self) -> Dict[str, str]: + """Build HTTP headers for OpenAI-compatible requests. + + The ``Authorization`` header is only included when an API key is + present — local servers such as vLLM, llama.cpp, and LocalAI run + unauthenticated and reject a stray ``Bearer`` header. + """ + headers: Dict[str, str] = {"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: + """Parse an OpenAI-compatible chat completions response body. + + Guards against servers that return an empty ``choices`` list rather + than an error status — previously this would silently succeed with + an empty text and ``success=True``, masking the upstream failure. + """ + choices = body.get("choices", []) + if not choices: + return LLMResponse( + text="", model=body.get("model", self.model), + provider=self.provider, success=False, + error="Upstream returned no completion choices.", + ) + content = choices[0].get("message", {}).get("content", "") + if not content: + return LLMResponse( + text="", model=body.get("model", self.model), + provider=self.provider, success=False, + error="Upstream returned an empty completion.", + ) + usage = body.get("usage", {}) + return LLMResponse( + text=content, + model=body.get("model", self.model), + provider=self.provider, + tokens_used=usage.get("total_tokens", 0), + success=True, + ) + @classmethod def auto(cls) -> "LLMClient": """Auto-detect available LLM provider. Priority: - 1. Ollama running locally + 1. Ollama running locally (or at OLLAMA_HOST) 2. OpenAI API key in environment 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") + model = os.environ.get("OLLAMA_MODEL", "llama3") + return cls(provider="ollama", model=model) # 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) + model = os.environ.get("OPENAI_MODEL", "gpt-4o-mini") + return cls(provider="openai", model=model, api_key=openai_key) # Try custom custom_key = os.environ.get("EOS_LLM_API_KEY", "") @@ -112,10 +209,12 @@ def auto(cls) -> "LLMClient": @staticmethod def _check_ollama() -> bool: - """Check if Ollama is running locally.""" + """Check if Ollama is running (locally or at OLLAMA_HOST).""" + ollama_host = os.environ.get("OLLAMA_HOST", LLMClient.OLLAMA_URL) + base = _ensure_scheme(ollama_host) try: req = urllib.request.Request( - f"{LLMClient.OLLAMA_URL}/api/tags", + f"{base}/api/tags", method="GET", ) with urllib.request.urlopen(req, timeout=3) as resp: @@ -198,8 +297,14 @@ def _call_ollama(self, prompt: str, system: str) -> LLMResponse: ) def _call_openai_compat(self, prompt: str, system: str) -> LLMResponse: - """Call OpenAI-compatible chat completions API.""" - url = f"{self.base_url}/v1/chat/completions" + """Call OpenAI-compatible chat completions API. + + The endpoint URL is normalised via :func:`_normalize_openai_url` so + that a ``base_url`` already containing ``/v1`` (e.g. from + ``OPENAI_BASE_URL=http://localhost:8000/v1``) does not produce a + doubled path segment (``/v1/v1/chat/completions``). + """ + url = _normalize_openai_url(self.base_url or self.OPENAI_URL) payload = { "model": self.model, "messages": [ @@ -211,26 +316,14 @@ 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(resp.read().decode("utf-8")) - choice = body.get("choices", [{}])[0] - message = choice.get("message", {}) - usage = body.get("usage", {}) - - return LLMResponse( - text=message.get("content", ""), - model=body.get("model", self.model), - provider=self.provider, - tokens_used=usage.get("total_tokens", 0), - success=True, - ) + return self._parse_openai_response(body) def get_provider_info(self) -> str: """Return human-readable provider information.""" diff --git a/ebuild/packages/recipe.py b/ebuild/packages/recipe.py index 6cbdb382..260284b7 100644 --- a/ebuild/packages/recipe.py +++ b/ebuild/packages/recipe.py @@ -89,6 +89,41 @@ def validate(self) -> None: f"Must be one of {self.VALID_BUILD_SYSTEMS}." ) + def to_dict(self) -> Dict[str, Any]: + """Serialize this recipe to a dict using the canonical YAML schema. + + Keys match the *external* YAML format (``package``, ``build``, etc.) + rather than the internal dataclass field names (``name``, + ``build_system``), so the result round-trips cleanly through + :func:`parse_recipe`. + + Only non-empty optional fields are included to keep the output + minimal and readable. + """ + data: Dict[str, Any] = { + "package": self.name, + "version": self.version, + "url": self.url, + "build": self.build_system, + } + if self.checksum: + data["checksum"] = self.checksum + if self.dependencies: + data["dependencies"] = list(self.dependencies) + if self.patches: + data["patches"] = list(self.patches) + if self.configure_args: + data["configure_args"] = list(self.configure_args) + if self.build_args: + data["build_args"] = list(self.build_args) + if self.install_args: + data["install_args"] = list(self.install_args) + if self.description: + data["description"] = self.description + if self.license: + data["license"] = self.license + return data + def _parse_string_list( raw: Dict[str, Any], diff --git a/ebuild/plugins/__init__.py b/ebuild/plugins/__init__.py index 3353ea58..94979e84 100644 --- a/ebuild/plugins/__init__.py +++ b/ebuild/plugins/__init__.py @@ -43,7 +43,7 @@ def discover_plugins() -> List[PluginBase]: else: # Before 3.10 entry_points() returned a dict; the current stubs # only model EntryPoints, which has no .get, hence the ignore. - eps = entry_points.get("ebuild.plugins", []) # type: ignore[attr-defined] + eps = entry_points.get("ebuild.plugins", []) # type: ignore[arg-type] for ep in eps: try: diff --git a/tests/ebuild/test_build_dir_resolution.py b/tests/ebuild/test_build_dir_resolution.py index a19be136..e08444f3 100644 --- a/tests/ebuild/test_build_dir_resolution.py +++ b/tests/ebuild/test_build_dir_resolution.py @@ -28,7 +28,6 @@ import os import shutil import subprocess -import shutil import textwrap from pathlib import Path from types import SimpleNamespace diff --git a/tests/ebuild/test_package_recipe.py b/tests/ebuild/test_package_recipe.py index 38c38de7..3c25922d 100644 --- a/tests/ebuild/test_package_recipe.py +++ b/tests/ebuild/test_package_recipe.py @@ -114,4 +114,4 @@ def test_depends_alias_must_be_a_list(): """ with pytest.raises(RecipeError, match="dependencies"): - load_recipe_from_string(content) \ No newline at end of file + load_recipe_from_string(content) diff --git a/tests/unit/test_ci_gate.py b/tests/unit/test_ci_gate.py index 964ca540..2831dc15 100644 --- a/tests/unit/test_ci_gate.py +++ b/tests/unit/test_ci_gate.py @@ -11,6 +11,9 @@ required check does not cover. """ +import itertools +import re + import yaml import pytest from pathlib import Path @@ -211,8 +214,7 @@ def test_gate_fails_on_any_non_success_result(jobs): # check cannot say which of the three it means, and a Windows-only failure is # indistinguishable from the other two legs without opening the run. -import itertools -import re + # `include` and `exclude` shape a matrix but are not dimensions of it, so they # are not part of the cartesian product. diff --git a/tests/unit/test_llm_client.py b/tests/unit/test_llm_client.py new file mode 100644 index 00000000..9d288710 --- /dev/null +++ b/tests/unit/test_llm_client.py @@ -0,0 +1,304 @@ +# 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