mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 03:00:23 +00:00
fix(config): PR #1970 lmstudio branch must honor cfg.model.base_url fallback
PR #1970 added a dedicated `elif pid == "lmstudio":` branch in `get_available_models()` that fetches the live /v1/models list when the hermes_cli helper doesn't have ids cached. The fallback path inside that branch only looked at `cfg["providers"]["lmstudio"]["base_url"]`, missing the historical config shape where the URL lives under `cfg["model"]`: model: provider: lmstudio base_url: http://192.168.1.22:1234/v1 ← here, not under providers.lmstudio providers: lmstudio: api_key: local-key 3 pre-existing tests in tests/test_issue1527_lmstudio_base_url_classification broke on stage-337 because of this — they passed on master, failed after the PR #1970 merge. The simpler fix is to enhance the already-introduced `_get_provider_base_url()` helper so it falls back to `cfg["model"]["base_url"]` when `cfg["model"]["provider"] == provider_id`, then use the helper inside the lmstudio branch instead of a direct lookup. This keeps the previous behaviour (where the generic configured-provider branch handled lmstudio via the model block) while preserving PR #1970's live-discovery additions. Belt-and-suspenders: `_get_provider_base_url()` explicitly does NOT inherit model.base_url for providers other than the active one — if a user's config says `model.provider: anthropic` and they have `providers.openai` configured without a base_url, openai must still resolve to None (use SDK default), not to the anthropic proxy URL. 6 new regression tests in tests/test_pr1970_lmstudio_base_url_fallback.py lock the two-location lookup, the precedence rule (explicit providers entry wins over model fallback), trailing-slash stripping, and the negative case (model.base_url MUST NOT leak to non-active providers). All 51 tests in the existing model-resolver + custom-provider banks still pass. Caught by maintainer review on stage-337 (full pytest with the new network isolation in place surfaced the regression that the fork-CI mock-server path would have hidden).
This commit is contained in:
+25
-3
@@ -1480,9 +1480,31 @@ def _custom_slug_rest_looks_like_host_port(rest: str) -> bool:
|
||||
|
||||
|
||||
def _get_provider_base_url(provider_id):
|
||||
"""Look up the configured base_url for a provider (e.g. lmstudio)."""
|
||||
"""Look up the configured base_url for a provider (e.g. lmstudio).
|
||||
|
||||
Checks two locations, in order:
|
||||
1. ``cfg["providers"][<provider_id>]["base_url"]`` — the explicit
|
||||
per-provider override.
|
||||
2. ``cfg["model"]["base_url"]`` — falls back here when
|
||||
``cfg["model"]["provider"] == provider_id``. This is the historical
|
||||
shape (the model block carries both the active provider AND the
|
||||
base URL for that provider in a single record).
|
||||
|
||||
Returns the URL stripped of trailing ``/`` if configured, otherwise None.
|
||||
"""
|
||||
prov_cfg = cfg.get("providers", {}).get(provider_id, {}) or {}
|
||||
return (prov_cfg.get("base_url") or "").rstrip("/") or None
|
||||
explicit = (prov_cfg.get("base_url") or "").strip().rstrip("/")
|
||||
if explicit:
|
||||
return explicit
|
||||
model_cfg = cfg.get("model", {}) or {}
|
||||
if isinstance(model_cfg, dict):
|
||||
model_provider = str(model_cfg.get("provider") or "").strip().lower()
|
||||
if model_provider == str(provider_id).strip().lower():
|
||||
model_base = (model_cfg.get("base_url") or "").strip().rstrip("/")
|
||||
if model_base:
|
||||
return model_base
|
||||
return None
|
||||
|
||||
|
||||
def resolve_model_provider(model_id: str) -> tuple:
|
||||
"""Resolve model name, provider, and base_url for AIAgent.
|
||||
@@ -3415,7 +3437,7 @@ def get_available_models() -> dict:
|
||||
# the profile's .env has been injected into the process environment.
|
||||
lm_cfg = cfg.get("providers", {}).get("lmstudio", {})
|
||||
if isinstance(lm_cfg, dict):
|
||||
lm_base_url = str(lm_cfg.get("base_url") or "").strip().rstrip("/")
|
||||
lm_base_url = _get_provider_base_url("lmstudio") or ""
|
||||
lm_api_key = str(lm_cfg.get("api_key") or "").strip()
|
||||
if lm_base_url:
|
||||
headers = {"User-Agent": "OpenAI/Python 1.0"}
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
"""Regression for PR #1970 LM Studio provider × cfg.model.base_url shape.
|
||||
|
||||
PR #1970 added `_get_provider_base_url()` + a dedicated lmstudio branch in
|
||||
`get_available_models()` for fetching live loaded models via the OpenAI-compatible
|
||||
/v1/models endpoint.
|
||||
|
||||
The initial implementation only looked at `cfg["providers"]["lmstudio"]["base_url"]`,
|
||||
missing the historical shape where users put `base_url` under `cfg["model"]`
|
||||
(when `cfg["model"]["provider"] == "lmstudio"`). That shape is what
|
||||
`tests/test_issue1527_lmstudio_base_url_classification.py` covers and what real
|
||||
users have in their config.yaml — 3 pre-existing tests started failing on stage-337
|
||||
because of this gap.
|
||||
|
||||
This regression test pins the helper's two-location lookup so a future change
|
||||
can't accidentally drop the model.base_url fallback again.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import api.config as config
|
||||
|
||||
|
||||
class _RestoreCfg:
|
||||
"""Context manager: snapshot cfg, restore on exit (test isolation)."""
|
||||
|
||||
def __enter__(self):
|
||||
import copy
|
||||
self._snapshot = copy.deepcopy(config.cfg)
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc):
|
||||
config.cfg.clear()
|
||||
config.cfg.update(self._snapshot)
|
||||
|
||||
|
||||
def test_get_provider_base_url_finds_explicit_providers_entry():
|
||||
"""When providers.<id>.base_url is set, return that value."""
|
||||
with _RestoreCfg():
|
||||
config.cfg.clear()
|
||||
config.cfg.update({
|
||||
"providers": {
|
||||
"lmstudio": {"base_url": "http://10.0.0.5:1234/v1", "api_key": "x"},
|
||||
},
|
||||
})
|
||||
assert config._get_provider_base_url("lmstudio") == "http://10.0.0.5:1234/v1"
|
||||
|
||||
|
||||
def test_get_provider_base_url_strips_trailing_slash():
|
||||
with _RestoreCfg():
|
||||
config.cfg.clear()
|
||||
config.cfg.update({
|
||||
"providers": {
|
||||
"lmstudio": {"base_url": "http://10.0.0.5:1234/v1/", "api_key": "x"},
|
||||
},
|
||||
})
|
||||
assert config._get_provider_base_url("lmstudio") == "http://10.0.0.5:1234/v1"
|
||||
|
||||
|
||||
def test_get_provider_base_url_falls_back_to_model_base_url():
|
||||
"""When providers.<id>.base_url is unset but cfg.model.base_url is set
|
||||
AND cfg.model.provider matches, the helper returns model.base_url."""
|
||||
with _RestoreCfg():
|
||||
config.cfg.clear()
|
||||
config.cfg.update({
|
||||
"model": {
|
||||
"provider": "lmstudio",
|
||||
"base_url": "http://192.168.1.22:1234/v1",
|
||||
"default": "qwen3.6-35b-a3b@q6_k",
|
||||
},
|
||||
"providers": {
|
||||
"lmstudio": {"api_key": "local-key"}, # no base_url here
|
||||
},
|
||||
})
|
||||
# Was returning None before the fix — the regression that broke
|
||||
# test_issue1527_lmstudio_base_url_classification.
|
||||
assert config._get_provider_base_url("lmstudio") == "http://192.168.1.22:1234/v1"
|
||||
|
||||
|
||||
def test_get_provider_base_url_returns_none_when_unconfigured():
|
||||
"""Unconfigured provider returns None (sentinel for 'use SDK default')."""
|
||||
with _RestoreCfg():
|
||||
config.cfg.clear()
|
||||
config.cfg.update({"providers": {}})
|
||||
assert config._get_provider_base_url("openai") is None
|
||||
assert config._get_provider_base_url("anthropic") is None
|
||||
assert config._get_provider_base_url("lmstudio") is None
|
||||
|
||||
|
||||
def test_get_provider_base_url_model_block_only_matches_active_provider():
|
||||
"""cfg.model.base_url must NOT leak to providers other than cfg.model.provider.
|
||||
|
||||
If model.provider is anthropic but providers.openai exists without base_url,
|
||||
_get_provider_base_url("openai") must still return None — otherwise we'd
|
||||
silently rewrite the OpenAI SDK target to an Anthropic endpoint URL.
|
||||
"""
|
||||
with _RestoreCfg():
|
||||
config.cfg.clear()
|
||||
config.cfg.update({
|
||||
"model": {
|
||||
"provider": "anthropic",
|
||||
"base_url": "https://my-anthropic-proxy.example.com/v1",
|
||||
},
|
||||
"providers": {
|
||||
"openai": {"api_key": "ok"}, # no base_url
|
||||
"anthropic": {"api_key": "ak"}, # no base_url
|
||||
},
|
||||
})
|
||||
# Active provider gets the model.base_url fallback.
|
||||
assert config._get_provider_base_url("anthropic") == "https://my-anthropic-proxy.example.com/v1"
|
||||
# OpenAI must NOT inherit it.
|
||||
assert config._get_provider_base_url("openai") is None
|
||||
|
||||
|
||||
def test_get_provider_base_url_explicit_wins_over_model_fallback():
|
||||
"""If both providers.<id>.base_url AND cfg.model.base_url are set with matching
|
||||
provider, the explicit providers entry wins."""
|
||||
with _RestoreCfg():
|
||||
config.cfg.clear()
|
||||
config.cfg.update({
|
||||
"model": {
|
||||
"provider": "lmstudio",
|
||||
"base_url": "http://wrong:1234/v1",
|
||||
},
|
||||
"providers": {
|
||||
"lmstudio": {"base_url": "http://correct:1234/v1", "api_key": "x"},
|
||||
},
|
||||
})
|
||||
assert config._get_provider_base_url("lmstudio") == "http://correct:1234/v1"
|
||||
Reference in New Issue
Block a user