fix(eos_ai): normalize LLM URLs, improve error resilience, and add unit test suite

[sapandeep31]

Summary

This PR addresses critical URL path handling bugs, error response edge cases, scheme normalization, and provider configuration gaps in the embedded AI subsystem's LLMClient (ebuild/eos_ai/llm_integration.py), and introduces a dedicated unit test suite (tests/unit/test_eos_ai_llm.py) with 34 comprehensive tests (~96% branch/statement coverage).

Prior to this PR, LLMClient had zero unit tests in the repository, and several failure modes impacted developers integrating local/cloud LLMs into hardware analysis pipelines.

Standards & Research Alignment

Our implementation was designed and cross-verified against official ecosystem documentation and standards:

1. OpenAI API & SDK Conventions (OpenAI API Docs, openai-python):
   • The canonical base URL route is https://api.openai.com/v1, and the chat completions resource is /chat/completions.
   • In existing tools (e.g. LangChain, LiteLLM, vLLM), developers configure base URLs interchangeably with or without /v1 (e.g., http://localhost:8000/v1 vs https://api.openai.com) and with or without trailing slashes. Naive string concatenation previously resulted in duplicate path segments (/v1/v1/chat/completions) or double slashes (//).
   • Added support for standard OPENAI_BASE_URL and OPENAI_MODEL environment variables.
2. Fail-Safe Completion Parsing:
   • When an upstream API returns empty choices ({"choices": []}) or empty completion text, LLMResponse(success=False, error="Upstream returned no completion choices") is returned. This guarantees fail-safe behavior and prevents downstream consumers (EosHardwareAnalyzer.analyze_with_llm) from falsely inflating hardware profile confidence or stamping unverified llm_analyzed metadata.
3. Ollama API Specifications (Ollama REST API Docs):
   • Model presence is verified via GET /api/tags, and synchronous completions require POST /api/generate with "stream": false.
   • Added support for standard OLLAMA_HOST and OLLAMA_MODEL environment variables.
   • Scheme normalization via _ensure_scheme automatically prepends http:// to schemeless host strings (e.g., 192.168.1.50:11434), supporting both ambient env vars and explicit base_url arguments.
4. Headless & Local Inference Runners (vLLM, llama.cpp, LocalAI):
   • Self-hosted model endpoints typically operate unauthenticated. Requiring a mandatory api_key for custom providers prevented developers from using local servers. The Authorization: Bearer header is now strictly omitted when api_key is empty or absent.
5. Resilient Response Parsing & Exception Narrowing:
   • Structured urllib.error.HTTPError decoding: extracts nested error.message from JSON error bodies across HTTP 401, 429, and 500 status codes with clean fallback to raw status text.
   • Narrowed decoding exception handler to (OSError, UnicodeDecodeError, AttributeError).

Changes

1. URL Normalization & Scheme Handling:
   • _normalize_openai_url: Safely strips trailing slashes and resolves endpoints whether the user passes a root domain (https://api.openai.com), a versioned path (http://localhost:8000/v1/), or a full resource URL (http://localhost:8000/v1/chat/completions).
   • _normalize_ollama_url: Normalizes Ollama base URLs to /api/generate.
   • _ensure_scheme: Guarantees scheme presence (http://) on Ollama URLs, whether provided via OLLAMA_HOST or explicit base_url.
   • _check_ollama: Resolves target /api/tags on the configured base_url (or OLLAMA_HOST) instead of hardcoding localhost.
2. Fail-Safe Completions & Error Extraction:
   • Empty choices / empty text return LLMResponse(success=False, error="Upstream returned no completion choices").
   • Extracted reusable _error_message(payload) helper used across both HTTP error decoding and OpenAI error-payload inspection.
   • Guarded HTTPError body reading with (OSError, UnicodeDecodeError, AttributeError).
3. Documentation:
   • Updated docs/ai-input-formats.md with current auto-detection priority order and a reference table for all 8 environment variables (OLLAMA_HOST, OLLAMA_MODEL, OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL, EOS_LLM_URL, EOS_LLM_API_KEY, EOS_LLM_MODEL).
4. Comprehensive Unit Test Suite (tests/unit/test_eos_ai_llm.py):
   • 34 unit tests covering initialization, URL normalization edge cases, scheme prefixing, provider auto-detection priority, is_available() matrix, Ollama payload construction, OpenAI chat completions parsing, error decoding, standard environment variables, and EosHardwareAnalyzer.analyze_with_llm enrichment and regression checks.
   • Added @pytest.fixture(autouse=True) with monkeypatch.delenv(..., raising=False) ensuring clean test isolation from ambient developer environment variables.
   • Negative testing ("the one check that matters") verified.

Test Plan

# Run new unit test suite with coverage
.venv/bin/pytest --cov=ebuild.eos_ai.llm_integration tests/unit/test_eos_ai_llm.py -v
# 34 passed in 0.14s (95.98% coverage on llm_integration.py)

# Run existing AI tests
.venv/bin/pytest tests/ebuild/test_eos_ai.py -v
# 24 passed in 0.03s

# Run importability tests
.venv/bin/pytest tests/unit/test_sources_are_importable.py -v
# 61 passed in 2.96s

# Lint and style check with project gate tool (ruff)
.venv/bin/ruff check ebuild/eos_ai/llm_integration.py tests/unit/test_eos_ai_llm.py
# All checks passed!

Negative testing ("the one check that matters"):

• Bypassed empty choices guard (if False and (not choices or not content):); observed test_empty_choices_handled_safely_without_index_error fail with assert True is False and test_analyzer_unchanged_when_llm_returns_empty_choices fail with assert 0.9 == 0.8 (demonstrating false confidence inflation caught).
• Restored code and confirmed all 34 tests pass cleanly.

[srpatcha]

Review — ebuild#120 "fix(eos_ai): normalize LLM URLs, improve error resilience, and add unit test suite"

head: b7b7b15 author: sapandeep31 ci: none reported

Verdict: Genuinely good work — the URL normalisation, the optional Authorization header and 546 lines of first-ever tests for LLMClient all land correctly — but one of the "resilience" changes converts a path that used to fail safe into one that silently reports success and inflates a hardware profile's confidence, and the new test suite pins that behaviour rather than catching it.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	ebuild/eos_ai/llm_integration.py (_call_openai_compat, empty-choices guard) + tests/unit/test_eos_ai_llm.py::test_empty_choices_handled_safely_without_index_error	A response of {"choices": []} now yields LLMResponse(text="", success=True). Before this diff the same body raised IndexError on body.get("choices", [{}])[0], which analyze()'s except Exception turned into success=False — the profile was left alone. Now it passes the if not response.success: return profile gate at ebuild/eos_ai/eos_hw_analyzer.py:647, so analyze_with_llm() runs its keyword scan over an empty string, finds nothing, and still appends llm_analyzed:<provider> (:661) and does profile.confidence = min(profile.confidence + 0.1, 1.0) (:662). An upstream API that answered nothing raises the reported confidence of a hardware profile by 0.1 and stamps it as LLM-analysed. That is a fail-safe path turned fail-silent, in the very function the PR describes as hardening, and the new test asserts resp.success is True so a future reader will take it as intended.	Treat "no usable content" as a failure, not an empty success: after computing content, if not choices or not content: return LLMResponse(text="", model=self.model, provider=self.provider, success=False, error="Upstream returned no completion choices"). Then change the test to assert resp.success is False and add one asserting analyze_with_llm() leaves confidence at its input value. The IndexError guard is still worth keeping — the point is what it reports, not that it no longer crashes.
2	Medium	tests/unit/test_eos_ai_llm.py — test_default_init, test_auto_detect_prefers_ollama_if_online, test_auto_detect_falls_back_to_openai_if_ollama_offline, test_is_available_logic	The suite reads the ambient environment for four variables this PR itself introduces. test_default_init asserts base_url == "http://localhost:11434" while __init__ now consults OLLAMA_HOST. test_auto_detect_prefers_ollama_if_online asserts model == "llama3" while auto() now consults OLLAMA_MODEL; the OpenAI equivalent asserts "gpt-4o-mini" against OPENAI_MODEL; test_is_available_logic's LLMClient(provider="openai", ...) depends on OPENAI_BASE_URL being unset. Any developer who has OLLAMA_HOST exported — exactly the users this PR is written for — gets red tests on an unmodified checkout. test_auto_detect_falls_back_to_custom_url already does monkeypatch.delenv("OPENAI_API_KEY", raising=False), so the technique is known; it is just applied to one variable out of six.	Add a module-scoped autouse fixture: @pytest.fixture(autouse=True) that monkeypatch.delenvs OLLAMA_HOST, OLLAMA_MODEL, OPENAI_BASE_URL, OPENAI_MODEL, OPENAI_API_KEY, EOS_LLM_URL, EOS_LLM_API_KEY, EOS_LLM_MODEL with raising=False. Tests that want a variable then set it explicitly, as test_ollama_respects_ollama_host_env_var already does.
3	Medium	ebuild/eos_ai/llm_integration.py (__init__, ollama branch) and (auto(), ollama branch)	Scheme normalisation is applied to OLLAMA_HOST but not to an explicitly passed base_url: default_url gets the http:// prefix, then self.base_url = (base_url or default_url).rstrip("/") discards default_url entirely when a caller supplies one. So LLMClient(provider="ollama", base_url="192.168.1.50:11434") — the same string that works as OLLAMA_HOST, per test_ollama_respects_ollama_host_env_var — builds 192.168.1.50:11434/api/generate and urllib.request raises ValueError: unknown url type, which surfaces through the generic except Exception as an opaque message rather than "missing scheme". The identical five-line prefix block is also written twice, in __init__ and in auto() — duplication the diff introduces (brief §10), and the reason the two paths could drift apart in the first place.	Extract it once and apply it after resolution, not before: @staticmethod def _ensure_scheme(url: str) -> str: return url if not url or url.startswith(("http://", "https://")) else f"http://{url}", then self.base_url = self._ensure_scheme(base_url or default_url).rstrip("/") and the same call in auto(). Add a test for the explicit-base_url-without-scheme case.
4	Medium	docs/ai-input-formats.md:103-107	Behaviour changed and the document describing it is now wrong. Line 104 says Ollama "checks http://localhost:11434" — it now checks OLLAMA_HOST first. Line 106 says Custom "uses EOS_LLM_API_KEY + EOS_LLM_URL + EOS_LLM_MODEL" — EOS_LLM_API_KEY is no longer required, which is one of the PR's headline changes. OPENAI_BASE_URL, OLLAMA_MODEL and OPENAI_MODEL are new and undocumented anywhere. Per brief §11 and the project's own rule, a change that makes existing documentation wrong is not finished.	Update the auto-detection list at docs/ai-input-formats.md:103-107 to the new precedence, and add a short table of the eight environment variables the client now reads.
5	Medium	PR CI	No checks ran. gh pr checks 120 reports none on fix/eos-ai-llm-resilience-and-tests, the bundle's checks.txt is empty, and the PR is BLOCKED. Everything in the Test Plan is the author's local run; nothing is independently reproduced. The Test Plan is unusually well-specified for this org — commands, counts, and a stated negative-control experiment — so this is about CI not having executed, not about the claims being hollow.	Re-trigger the workflow. .github/workflows/ci.yml runs ruff check . and the pytest suite, which covers items 1-4 of the Test Plan.
6	Low	ebuild/eos_ai/llm_integration.py (HTTPError handler, except Exception: pass)	Bare except Exception: pass around the error-body read. .ai/reviewer.md lists a swallowed exception as a finding. It is bounded — the code falls through to f"HTTP {e.code}: {e.reason}" — but it will also silently absorb a bug in the parsing block above it, including the str(inner) calls.	Narrow it to except (OSError, UnicodeDecodeError, AttributeError): pass, which is the set that can actually arise from e.read().decode() on a closed or non-text body.
7	Low	ebuild/eos_ai/llm_integration.py (_call_openai_compat)	Two small redundancies in the new code: the isinstance(body, dict) and in the error-payload check is dead — the guard three lines above already returned for non-dict bodies; and the expression inner.get("message", str(inner)) if isinstance(inner, dict) else str(inner) is written twice, once here and once in the HTTPError handler.	Drop the redundant isinstance, and lift the message extraction into a @staticmethod _error_message(payload) -> str used by both sites.
8	Low	PR body, "Test Plan"	The lint step ran .venv/bin/flake8 --ignore=E501,E731,E741,F403,F405,F541,F841 .... This repo's gate is ruff check . (.github/workflows/ci.yml:58), configured in pyproject.toml:34-51; flake8 appears only in the weekly job with different arguments (.github/workflows/weekly.yml:31). Re-typing the ignore list onto a different tool proves that tool's opinion, not the gate's. I ran the right one: ruff check with the project's select/ignore over both changed files passes with no diagnostics, so there is no actual lint defect here — the claim is just not evidence for the check that will run.	Quote ruff check . in the Test Plan instead.

Checked and clear: exception ordering in analyze() is correct (HTTPError before URLError, of which it is a subclass, and a bare TimeoutError after both). asdict-style serialisation is not involved. The pytest.mark.ebuild marker is registered in pytest.ini:16, so --strict-markers (pytest.ini:28) will not reject the new file. The optional-Authorization change is right and test_custom_endpoint_omits_bearer_header_when_api_key_empty asserts the header's absence rather than its emptiness, which is the stronger check.

Architecture conformance

Conforms, with one boundary question worth recording rather than blocking on.

Master design §9.2 sets the SDK rule that matters here — "No mandatory cloud connection." This code satisfies it: EosHardwareAnalyzer.analyze_with_llm() returns the profile unchanged when is_available() is false (eos_hw_analyzer.py:641-642), the rule engine works with provider="none", and §19's "Cloud services must remain optional" is respected. Widening is_available() so a custom provider needs only a base_url (no API key) moves toward §9.2, not away — it is what lets a self-hosted vLLM or llama.cpp endpoint work without an account. §21 tier placement is unchanged: this is Tier 1 ebuild, and nothing here imports from a higher tier — the LLM is reached over HTTP, not by depending on the Tier 3 eAI repository, so §5.1's dependency direction holds. eBuild remains a developer-time tool and not a runtime dependency (§5.1), since none of this is compiled into firmware.

The boundary question: ebuild/eos_ai/ is a third AI-named surface in the org alongside the eAI repository (§21 Tier 3) and eosllm, while Appendix C directs the foundation to "consolidate overlapping AI names under eAI" and §16.1 to "expose eAI Vision, eAI Audio, eAI LLM and eAI Tiny as subproducts rather than unrelated top-level brands". The master design describes eAI as an on-device inference platform and says nothing at all about LLM-assisted developer tooling that runs on the workstation and calls a third-party API — which is what this module is. Under §21.1 it does not warrant its own repository (one consumer, no independent release lifecycle), so keeping it inside ebuild is the right call today; the naming is what collides. Recorded as a proposal in .ai/autoreview/proposals/2026-09.md rather than held against this PR.

Proposed changes

Smallest sequence, in order:

1. Make empty choices a failure (finding 1) and flip the two assertions in test_empty_choices_handled_safely_without_index_error. Add the analyze_with_llm() confidence-unchanged test — this is the one that would have caught it.
2. Add the autouse environment-clearing fixture (finding 2). Do this before 3, so the new test in 3 is not itself environment-dependent.
3. Extract _ensure_scheme(), call it in both __init__ and auto(), and add the explicit-base_url-without-scheme test (finding 3).
4. Update docs/ai-input-formats.md:103-107 and add the environment-variable table (finding 4).
5. Sweep findings 6 and 7 — three lines total.
6. Re-run ruff check . and python3 -m pytest tests/ -q, and replace the Test Plan's flake8 line with the ruff invocation.

Not checked

• Nothing in this repository was executed by this review beyond ruff. ebuild has a dirty working tree (4 files) and this pipeline leaves such repos untouched, so pytest tests/ was NOT RUN. The ruff check cited in finding 8 was run against copies of the two changed files extracted from head b7b7b15 into a temporary directory, with this repo's select/ignore passed on the command line — that is not the same as ruff check . over the whole tree.
• The "32 passed, 96.60% coverage" claim is unverified. No CI ran and I did not run pytest. The commands are plausible and specific, but the numbers are the author's, not observed here.
• The stated negative-control experiment is unverified — "intentionally altered _normalize_openai_url … observed 3 tests failing" is exactly the right thing to have done and exactly the kind of claim that leaves no artifact. Taken at face value, not confirmed.
• Finding 1's failure sequence is traced through the code (llm_integration.py → eos_hw_analyzer.py:641-662), not observed in a run.
• No live endpoint of any kind was contacted: the Ollama /api/tags probe, real OpenAI 401/429 bodies, and vLLM's actual response shape are all untested here and mocked in the suite.
• Whether any other caller of LLMClient outside eos_hw_analyzer.py depends on the old success=True-on-empty behaviour. ebuild/ was searched for .analyze(; other repositories were not.
• Thread-safety and concurrent use of LLMClient, and the behaviour of the timeout parameter against a slow-but-responding server. Neither is exercised.

---

Automated architecture review of b7b7b1529772 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[sapandeep31]

Thank you for the thorough and constructive architectural review! All 8 findings have been addressed in commit 047c111:

1. Fail-Safe on Empty Choices (Finding 1):

   • In _call_openai_compat: When not choices or not content, LLMClient now returns LLMResponse(text="", model=self.model, provider=self.provider, success=False, error="Upstream returned no completion choices").
   • Updated assertions in test_empty_choices_handled_safely_without_index_error to verify resp.success is False and resp.error == "Upstream returned no completion choices".
   • Added test_analyzer_unchanged_when_llm_returns_empty_choices in TestHardwareAnalyzerIntegrationWithLLM verifying that EosHardwareAnalyzer.analyze_with_llm leaves profile.confidence completely unchanged (0.8) and does not stamp llm_analyzed when upstream returns empty choices.
   • Performed negative control verification: temporarily bypassing this check resulted in AssertionError: assert 0.9 == 0.8 with features=['llm_analyzed:openai'], proving the regression was caught and eliminated.

2. Ambient Environment Isolation (Finding 2):

   • Added @pytest.fixture(autouse=True) in tests/unit/test_eos_ai_llm.py that strips all 8 ambient LLM variables (OLLAMA_HOST, OLLAMA_MODEL, OPENAI_BASE_URL, OPENAI_MODEL, OPENAI_API_KEY, EOS_LLM_URL, EOS_LLM_API_KEY, EOS_LLM_MODEL) using monkeypatch.delenv(..., raising=False). Tests now run consistently on clean or customized checkouts alike.

3. Scheme Normalization (Finding 3):

   • Extracted @staticmethod def _ensure_scheme(url: str) -> str to prepend http:// to schemeless URLs.
   • Applied _ensure_scheme() to the resolved URL in __init__ for provider="ollama" (so explicit base_url="192.168.1.50:11434" works seamlessly) and in auto().
   • Added unit test test_init_normalizes_scheme_for_explicit_base_url_without_scheme.

4. Documentation (Finding 4):

   • Updated docs/ai-input-formats.md with the updated auto-detection priority order.
   • Added a Markdown reference table documenting all 8 environment variables, their defaults, providers, and descriptions.

5. CI Execution (Finding 5):

   • Pushed commit 047c111 to the PR branch. Upstream GitHub Actions workflows (CI — ebuild, CodeQL, Simulation Test) are now queued in action_required status awaiting maintainer runner approval for outside fork contributors.

6. Narrow Exception Handling (Finding 6):

   • Replaced bare except Exception: pass in the HTTPError body reader with except (OSError, UnicodeDecodeError, AttributeError): pass.

7. Code Redundancies & Deduplication (Finding 7):

   • Extracted @staticmethod def _error_message(payload: Any) -> str and reused it across both HTTPError response parsing and _call_openai_compat JSON error payloads.
   • Removed the redundant isinstance(body, dict) check in _call_openai_compat.

8. Test Plan Tooling (Finding 8):

   • Updated the PR description Test Plan to reference .venv/bin/ruff check ebuild/eos_ai/llm_integration.py tests/unit/test_eos_ai_llm.py (matching the repo CI linter gate).
   • Test suite now has 34 passing tests with 95.98% statement/branch coverage.

[srpatcha]

Review — ebuild#120 "fix(eos_ai): normalize LLM URLs, improve error resilience, and add unit test suite"

head: 047c111 author: sapandeep31 ci: none — zero checks have run

Verdict: Follow-up. All eight previous findings are addressed, and this time I could run
the suite: 34 passed, ruff check clean on both changed files, and the author's stated
negative control reproduces byte for byte — removing the empty-choices guard fails exactly
the two tests they said it fails, with exactly the assert 0.9 == 0.8 and
features=['llm_analyzed:openai'] they reported. That is the rarest thing in this backlog:
a claimed verification that holds up when someone else runs it.

Two things remain, neither of them the author's fault, and one new finding of my own.

Previous findings

#	Was	Now	Evidence
1	High — {"choices": []} returned success=True, inflating profile.confidence by 0.1 and stamping llm_analyzed	Resolved	llm_integration.py _call_openai_compat now returns success=False, error="Upstream returned no completion choices" on not choices or not content. Verified by execution and by negative control: with the guard deleted, test_empty_choices_handled_safely_without_index_error fails assert True is False and test_analyzer_unchanged_when_llm_returns_empty_choices fails assert 0.9 == 0.8. Both pass with it.
2	Medium — six env vars read ambiently by the suite	Resolved	@pytest.fixture(autouse=True) clean_ambient_llm_env delenvs all eight with raising=False.
3	Medium — scheme normalisation skipped an explicit base_url; the prefix block was duplicated	Resolved as scoped	_ensure_scheme() extracted as a @staticmethod, called in __init__'s ollama branch on base_url or default_url and in auto(). Duplication gone. New test_init_normalizes_scheme_for_explicit_base_url_without_scheme passes. See finding 2 below for what the helper still does not cover.
4	Medium — docs/ai-input-formats.md:103-107 described the old precedence	Resolved	Precedence list rewritten and an eight-row environment-variable table added. I spot-checked the defaults against the code: OLLAMA_URL = "http://localhost:11434", OPENAI_URL = "https://api.openai.com", OLLAMA_MODEL → llama3, OPENAI_MODEL → gpt-4o-mini, EOS_LLM_MODEL → default. All five match.
5	Medium — no CI had run	Still open	checks.txt is still empty — zero passing, zero failing — and mergeStateStatus is still BLOCKED. The author reports the workflows sitting in action_required awaiting maintainer approval for an outside fork. Nothing here is in their control.
6	Low — bare except Exception: pass	Resolved	Now except (OSError, UnicodeDecodeError, AttributeError): pass.
7	Low — dead isinstance and a duplicated message expression	Resolved	_error_message() extracted and used at both sites; the redundant isinstance(body, dict) is gone.
8	Low — Test Plan quoted flake8, not the repo gate	Resolved	The Test Plan now runs ruff. It scopes it to the two changed files rather than ruff check .; see finding 1 for why that distinction matters here more than usual.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	.github/workflows/ci.yml:58 (on origin/master)	The repo's lint gate is already red on master, so this PR will fail CI the moment it is allowed to run, for reasons that have nothing to do with it. ci.yml:58 runs ruff check .; I ran that on a clean origin/master worktree and got 4 errors — F811 Redefinition of unused 'shutil' at tests/ebuild/test_build_dir_resolution.py:31, W292 No newline at end of file at tests/ebuild/test_package_recipe.py:117, and E402 Module level import not at top of file ×2 at tests/unit/test_ci_gate.py:214,215. None of those files is touched by this PR, and ruff check over this PR's own files reports All checks passed!. This is a blocker for every open ebuild PR, not just this one.	Separate PR against master, three files: delete the duplicate import shutil, add the trailing newline, move the two mid-file stdlib imports in test_ci_gate.py to the top (or # noqa: E402 them if their position is deliberate — the comment block above them suggests it might be). ruff check . --fix handles two of the four. Verify with ruff check . returning clean.
2	Medium	ebuild/eos_ai/llm_integration.py — __init__ openai and custom branches	_ensure_scheme() was extracted to stop exactly this and is applied to one provider out of three. The ollama branch does self.base_url = self._ensure_scheme(raw_url).rstrip("/"), but the openai branch is still (base_url or default_url).rstrip("/") over OPENAI_BASE_URL, and the custom branch is still (base_url or "").rstrip("/") over EOS_LLM_URL. So EOS_LLM_URL=192.168.1.50:8000 — a self-hosted vLLM endpoint, which is the headline use case this PR adds keyless support for — still produces ValueError: unknown url type from urllib.request, surfaced through the generic handler as an opaque message. is_available() returns True for it (bool(self.base_url)), so the analyzer tries and fails rather than declining cleanly. Same defect, same helper, two lines from being fixed.	Apply the helper in both remaining branches: self.base_url = self._ensure_scheme(base_url or default_url).rstrip("/") for openai, and self.base_url = self._ensure_scheme(base_url or "").rstrip("/") for custom — _ensure_scheme already returns "" for falsy input, so the custom branch stays correct when unset. Add the EOS_LLM_URL-without-scheme test alongside the ollama one.
3	Low	ebuild/eos_ai/llm_integration.py — _call_openai_compat, the new guard	The error string is "Upstream returned no completion choices", but the condition is not choices or not content. A response with a well-formed choices[0].message.content == "" — a model that replied with nothing, which is a different upstream fault from returning no choices at all — reports the wrong cause. Both should fail, so the behaviour is right; only the diagnostic is misleading, and this message is what a user debugging a flaky endpoint will see. Per §9.2's "actionable diagnostics with remediation guidance".	Split the message: error="Upstream returned no completion choices" if not choices else "Upstream returned an empty completion". One line, and the existing test asserts only the first string, so it stays green.

Verification performed for this review

Detached scratch worktrees under .ai/autoreview/state/verify/. ebuild's working tree
is dirty (TASKS.md, ebuild/cli/integration.py,
tests/ebuild/test_integration_initramfs_security.py modified, smart-sensor/ untracked)
— it was not touched, stashed, reset or checked out; the worktrees are independent.

Check	Result
pytest tests/unit/test_eos_ai_llm.py -q on this head	PASS — 34 passed in 0.10s. Matches the body's "34 passed" exactly.
ruff check ebuild/eos_ai/llm_integration.py tests/unit/test_eos_ai_llm.py	PASS — All checks passed! Matches the body's Test Plan.
ruff check . (the actual CI gate) on this head	FAIL — 4 errors, all in files this PR does not touch
ruff check . on clean origin/master	FAIL — same 4 errors. Pre-existing; finding 1.
Negative control: deleted the not choices or not content guard, re-ran	2 failed, 32 passed — assert True is False and assert 0.9 == 0.8 with features=['llm_analyzed:openai']. Reproduces the author's stated negative control precisely. Guard restored afterwards.
pytest tests/ -q (whole suite)	NOT RUN — 27 collection errors, all ModuleNotFoundError: No module named 'click' / 'yaml'. Environmental: CI does pip install -e . (ci.yml:54) and this host has neither. Not a repo defect.
Coverage claim "95.98%"	NOT VERIFIED — pytest-cov is not installed here.

Worth noting that test_eos_ai_llm.py runs standalone on a bare interpreter while 27 other
modules cannot — llm_integration.py imports only json, os, urllib and dataclasses.
That is a real property of the design, not an accident, and it is why this suite is cheap
to run anywhere.

New commit reviewed on its own merits

047c111 is the only commit since b7b7b15. _error_message() slightly changes behaviour
from the code it replaces — inner.get("message", str(inner)) could return a non-string,
str(payload.get("message", payload)) always returns a string — which is an improvement, and
test_json_error_payload_in_response still passes. The narrowed except no longer catches
json.JSONDecodeError, but that is caught explicitly on the line above, so nothing is lost.
_ensure_scheme("") returns "" rather than "http://", which is what keeps the custom
branch correct today. No defect introduced.

Architecture conformance

Conforms. §9.2's "no mandatory cloud connection" holds: analyze_with_llm() returns the
profile unchanged when is_available() is false, the rule engine runs with
provider="none", and §19's "cloud services must remain optional" is respected. Finding 1
of the previous review mattered under §28 as much as under correctness — a profile whose
confidence rose because an endpoint said nothing is exactly the "claim without evidence"
that section exists to prevent, and the fix puts it right. §21 placement unchanged: Tier 1
ebuild, no import from a higher tier — the LLM is reached over HTTP rather than by
depending on the Tier 3 eAI repo, so §5.1's direction holds, and none of this is compiled
into firmware, so eBuild stays a developer-time tool and not a runtime dependency.

The naming question raised last time — ebuild/eos_ai/ as a third AI-named surface
alongside the eAI repository and eosllm, against Appendix C's "consolidate overlapping
AI names under eAI" — is unchanged and already recorded as a proposal in
.ai/autoreview/proposals/2026-09.md. Still not held against this PR: under §21.1 the
module has one consumer and no independent release lifecycle, so keeping it inside ebuild
is correct.

Blocked status

Blocked on the maintainers, not the author. Two gates, neither reachable from this
branch: workflow approval for an outside-fork contributor (finding 5 of the previous
review), and the pre-existing ruff check . failure on master (finding 1 above), which
will turn the Lint job red as soon as approval is granted. Fixing the latter first would
save a confusing red run.

Not checked

• Everything in CI. Zero checks have run on this PR, so lint, CodeQL, the simulation
  job and the full pytest suite are all NOT RUN for this head. Every result above is
  mine, on a Linux host, with ruff 0.16.5 — CI installs ruff unpinned (ci.yml:55), so a
  newer release could report a different set than the four I saw.
• The full pytest tests/ suite — NOT RUN (missing click, yaml). I therefore have
  no evidence about regressions outside tests/unit/test_eos_ai_llm.py, including
  tests/ebuild/test_eos_ai.py, which the body says passes 24 tests.
• Coverage — NOT VERIFIED. The 95.98% figure is the author's.
• No live endpoint was contacted. The Ollama /api/tags probe, real OpenAI 401/429
  bodies and vLLM's actual response shape are mocked throughout; whether a real vLLM
  returns choices: [] in the shape the new guard expects is unknown.
• Finding 2's failure mode was reasoned, not run. I did not construct a
  LLMClient(provider="custom", base_url="192.168.1.50:8000") and observe the ValueError;
  it follows from the branch not calling _ensure_scheme and from the ollama case that
  motivated the original finding.
• Whether any caller outside eos_hw_analyzer.py depended on the old success=True-on-empty
  behaviour. Only ebuild/ was searched; other repos were not.

---

Automated architecture review of 047c11180ae2 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

This is a solid fix — thank you. I checked it against #127, which targets the same bug: this branch handles more of the surface (keyless custom in is_available(), _check_ollama() probing the configured base_url, HTTP error bodies, docs) and stays inside eos_ai, so I'd prefer to land this one. Suite is green locally apart from the pre-existing test_index_sync failures, ruff and mypy show nothing new.

A few small things, none blocking:

• llm_integration.py:80/83 — _ensure_scheme() only runs for Ollama; OPENAI_BASE_URL=localhost:8000 produces an unschemed URL that urlopen rejects. Applying it to all three providers would be consistent.
• • llm_integration.py:346-352 — the "no completion choices" message also fires when a choice exists but content is empty/null; a separate message would make logs easier to read.
• • • llm_integration.py:153 — auto() picking custom from EOS_LLM_URL alone is a behaviour change; please add a CHANGELOG.md line alongside the docs update.
• • • • Please add Signed-off-by (git commit -s --amend / rebase) per CONTRIBUTING.md.
        Approving. Expect a conflict with #127 in llm_integration.py if it merges first; I've asked over there to coordinate.

[sapandeep31]

This is a solid fix — thank you. I checked it against #127, which targets the same bug: this branch handles more of the surface (keyless custom in is_available(), _check_ollama() probing the configured base_url, HTTP error bodies, docs) and stays inside eos_ai, so I'd prefer to land this one. Suite is green locally apart from the pre-existing test_index_sync failures, ruff and mypy show nothing new.

A few small things, none blocking:

• llm_integration.py:80/83 — _ensure_scheme() only runs for Ollama; OPENAI_BASE_URL=localhost:8000 produces an unschemed URL that urlopen rejects. Applying it to all three providers would be consistent.
• • llm_integration.py:346-352 — the "no completion choices" message also fires when a choice exists but content is empty/null; a separate message would make logs easier to read.
• • • llm_integration.py:153 — auto() picking custom from EOS_LLM_URL alone is a behaviour change; please add a CHANGELOG.md line alongside the docs update.
• • • • Please add Signed-off-by (git commit -s --amend / rebase) per CONTRIBUTING.md.
        Approving. Expect a conflict with fix(eos_ai): LLMClient URL normalization, env var support, and unit test suite #127 in llm_integration.py if it merges first; I've asked over there to coordinate.