fix(llm): complete path-less OpenAI-compatible base URLs - #115
Conversation
A stored local base URL of `http://host:port` made the OpenAI SDK POST to
`/chat/completions` instead of `/v1/chat/completions`, because the SDK
appends the path relative to `base_url`. LM Studio answers the wrong path
with HTTP 200 and an `{"error": ...}` body, so nothing raises and the
agent reports "AI generated empty response" rather than a routing
mistake. The credential validator could not catch it either: model
listing rides LM Studio's native WebSocket API, which is happy without
the `/v1` suffix.
`normalize_openai_base_url` (services/llm/config.py) completes the URL
only when it carries no path, so an explicit gateway path such as
`/openai` is left alone. It is applied at the point of use in
OpenAIProvider — which makes an already-stored URL work without the
operator re-entering it — and when the local validator persists the URL,
so new entries are stored canonically.
Verified live against an LM Studio server: the same provider path that
returned an empty response now returns a completion.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
trohitg
left a comment
There was a problem hiding this comment.
Reviewed the diff, traced the blast radius, and verified the core claim empirically against the installed openai 2.53.0.
The diagnosis is right and the write-up is excellent. Issue #114's HTTP-200-with-an-error-body finding is the kind of thing that costs someone an afternoon, and it's documented precisely. Confirmed:
base_url 'http://h:1234' -> POST http://h:1234/chat/completions <- the bug
base_url 'http://h:1234/v1' -> POST http://h:1234/v1/chat/completions <- fixed
.rstrip("/") is safe — the SDK re-adds the slash itself (_base_client.py:414 _enforce_trailing_slash). So the direction is correct. My concerns are with the guard and the scope.
Blocking
1. localhost:1234 / 192.168.4.93:1234 are silently not fixed
urlsplit has no // to anchor on, so the whole string lands in path, which is truthy, so the helper returns it untouched:
'localhost:1234' -> scheme='localhost' netloc='' path='1234' => UNCHANGED
'192.168.4.93:1234' -> scheme='' netloc='' path='192.168.4.93:1234' => UNCHANGED
A missing scheme is the other half of the same paste mistake, and the validator still goes green: Ollama's _parse_host and LM Studio's api_host both accept scheme-less input. Green dot, broken agent — the exact UX this PR set out to remove. Every parametrized case carries a scheme, so CI can't see it.
The guard wants netloc, not path.
2. Not idempotent — and double-normalization is the real code path
Call site 2 persists; call site 1 re-normalizes the stored value on every client build (unifier.py:296 -> OpenAIProvider.__init__). So idempotency is load-bearing, and it doesn't hold:
'http://host:1234?x=1' -> '...?x=1/v1' -> '...?x=1/v1/v1' (grows every pass)
urlsplit reports path='' because ? terminates the authority, so /v1 is appended into the query string.
Both #1 and #2 close with the same change:
from urllib.parse import urlsplit, urlunsplit
def normalize_openai_base_url(base_url: str) -> str:
u = (base_url or "").strip().rstrip("/")
if not u:
return u
s = urlsplit(u if "//" in u else f"http://{u}")
if not s.netloc:
return u
return u if s.path else urlunsplit((s.scheme, s.netloc, "/v1", s.query, s.fragment))3. Blast radius is wider than the title suggests
url = proxy_url or base_url runs for all nine compat providers, and deepseek is path-less in llm_defaults.json:274. Its wire path moves to /v1/chat/completions — untested and unmentioned.
It's probably correct (native_llm_sdk.md:235 and deepseekChatModel.md:87 already document /v1, and DeepSeek serves both), which makes it a drift fix rather than a regression. But it leaves a split-brain: test_provider_self_registration.py:90 still pins the path-less string, so the registry and the live client now disagree and CI stays green either way. Cleaner to fix llm_defaults.json explicitly than to rewrite it implicitly at the point of use.
Same exposure for any already-working {provider}_proxy pointing at a root-mounted relay — it breaks after this with no migration and no opt-out.
4. Applied to one of four providers that take the same credential
openrouter.py:33 overrides __init__ and never calls super(), so openrouter_proxy bypasses the fix; anthropic.py:39 and gemini.py:62 use proxy_url raw. Same user input, same failure mode, three different behaviours afterwards. Worth either applying where they converge or scoping this explicitly to the local providers.
Should ship in the same PR
5. This breaks its own error message
_local_validator.py:75-80 picks the 404 hint with url.endswith("/v1"). Normalizing before the probe makes that always true, so the "URL likely needs to end with /v1" branch is now unreachable for the case it was written for — those users get "check the server version", which points at the wrong thing.
6. Empty-check ordering
_local_validator.py:259 rejects empty input before normalization, but normalization can produce empty: "/" -> "". That persists an empty {provider}_proxy, flips the catalogue to "configured", and ollama._parse_host("") then defaults to 127.0.0.1:11434 — probing a server the operator never named. Re-check after normalizing.
Notes
- Same defect one file over:
services/memory/vector_store.py:215-217passes the user'sembedding_endpointstraight intobase_url. The repo's own fixture (tests/nodes/test_ai_agents.py:91) uses the barehttp://localhost:11434, so/embeddingsinstead of/v1/embeddings. Out of scope here, but it's the same bug. - Conventions look good. The function-level import in
openai.pymatches house style (both existing config imports there are deferred, as in anthropic/gemini); the plugin importingservices.llm.configis fine and has precedent atnodes/vision/vision_analyze/__init__.py:131. The one import that doesn't fit isfrom urllib.parse import urlsplitinside the function body — stdlib, and it sits after the early return, so it's a per-callsys.moduleslookup. The module convention is top-level. - Test nits:
patch("openai.AsyncOpenAI", MagicMock())—patchalready creates the mock; every existing call site omits it. Missingfrom __future__ import annotations.call_args.kwargswhere the suite usescall_args[1]. And(None, "")passesNoneinto a parameter annotatedstr. - Coverage: nothing tests the
_local_validatorcall site — what actually gets persisted — or the store -> reload -> re-normalize round trip that the two-call-site design depends on.
Thanks for this one — the failure analysis in #114 is genuinely useful independent of the fix, and the "only complete a path-less URL" instinct is the right constraint. It just needs to key off netloc to catch the input that most often triggers the bug.
|
Thanks for the detailed writeup. Closing this one because it goes the opposite direction from RFC-0003 (RFC-0003-OPENAI-COMPATIBLE-PROVIDER-CONTRACT.md): a configured base URL is copied verbatim from vendor docs and never rewritten, and a user-supplied path-less URL gets resolved by probing GET {base}/models then {base}/v1/models at save time rather than by appending /v1. The LM Studio symptom in #114 is real, but the fix belongs in the save-time probe, not in the provider request path. |
|
Reopening. This was closed by mistake while triaging a different PR; the close comment above does not reflect a decision on this change. Apologies for the noise. |
Fixes #114.
Problem
A stored local base URL of
http://host:portmade the OpenAI SDK POST to/chat/completionsinstead of/v1/chat/completions— the SDK appends the path relative tobase_url.The failure mode is worth spelling out, because it is why this reads as a user error: LM Studio answers the wrong path with HTTP 200 and a body of
{"error":"Unexpected endpoint or method. (POST /chat/completions)"}. Nothing raises, so the agent reportsAI generated empty responseinstead of a routing mistake.The credential validator could not catch it either —
_fetch_lmstudio_modelsprobes over LM Studio's native WebSocket API (ws://host:port/llm), which is happy without the/v1suffix, so the URL validated green and only chat failed._strip_v1_pathshows the stored shape was already expected to end in/v1; nothing enforced it.Fix
normalize_openai_base_urlinservices/llm/config.pycompletes the URL only when it carries no path, so an explicit gateway path such as/openaior/api/v2is left alone — appending/v1there would break a working configuration.One implementation, applied at two points:
services/llm/providers/openai.py— at the point of use, which makes an already-stored URL work without the operator re-entering it. This covers Ollama too, which shares the provider class and the same{provider}_proxyshape.nodes/model/_local_validator.py— when the URL is persisted, so new entries are stored canonically.Verification
Live against an LM Studio server with no
/v1stored: the same provider path that returned an empty response now returns a completion (base_url -> http://host:1234/v1/, content'ok', usage reported).14 new tests in
server/tests/llm/test_base_url_normalization.py: barehost:port->/v1, an already-/v1URL unchanged, a gateway path unchanged, empty andNone, the provider applying the completion whether the URL arrives viaproxy_urlorbase_url, and plain OpenAI still passing nobase_urlat all so the SDK keeps its own default endpoint.Run on this branch (cherry-picked onto current
main, applied clean):tests/llm+test_plugin_contract.py+test_plugin_self_containment.py-> 326 passed, 36 deselected.Out of scope, noted in the issue
An HTTP 200 carrying an
{"error": ...}body should surface as a real error rather than an empty response. That is a separate change inopenai.py.🤖 Generated with Claude Code