fix(model): address gate feedback #4857080754 — slug names, list models, get_config, single YAML parse
- _repair_bare_custom_provider_model matches display-named providers via _custom_provider_slug_from_name (custom:my-proxy matches 'My Proxy'). - _ordered_custom_provider_model_ids now handles dict keys, list strings, and list dicts with id/model/name, aligned with api/config.py catalog. - config_obj=None fallback uses get_config() instead of raw cfg. - _read_profile_model_config returns profile config dict too, avoiding a second YAML parse on hot display paths. - Tests added for slug matching, list-form models, and get_config fallback. Co-authored-by: b3nw <b3nw@users.noreply.github.com>
This commit is contained in:
1 parent
3682d237d7
commit
a477069e3a
2 files changed
+136
-54
No files matched your search
+73
-51
@@ -5760,14 +5760,16 @@ def _should_attach_codex_provider_context(model: str, raw_active_provider: str,
|
||||
)
|
||||
return False
|
||||
|
||||
|
||||
def _read_profile_model_config(
|
||||
session, requested_provider: str | None,
|
||||
) -> tuple[str | None, str | None]:
|
||||
"""Read model.provider and model.default from the session's profile config.
|
||||
session,
|
||||
requested_provider: str | None,
|
||||
) -> tuple[str | None, str | None, dict | None]:
|
||||
"""Read model.provider, model.default, and the full profile config dict.
|
||||
|
||||
Returns (profile_provider, profile_default_model). Both are None when the
|
||||
session has no profile or the profile config is unreadable.
|
||||
Returns (profile_provider, profile_default_model, profile_config_dict).
|
||||
The first two are None when the session has no profile or the profile config
|
||||
is unreadable; profile_config_dict is None in the same cases so callers only
|
||||
pay for one YAML parse.
|
||||
|
||||
When the session already has an explicit ``requested_provider``, the profile
|
||||
``model.provider`` is not returned (first tuple element is None) so profile
|
||||
@@ -5776,7 +5778,7 @@ def _read_profile_model_config(
|
||||
provider matches ``requested_provider`` after normalization.
|
||||
"""
|
||||
if not getattr(session, "profile", None):
|
||||
return None, None
|
||||
return None, None, None
|
||||
|
||||
try:
|
||||
from api.profiles import get_hermes_home_for_profile
|
||||
@@ -5784,16 +5786,16 @@ def _read_profile_model_config(
|
||||
_profile_home = get_hermes_home_for_profile(session.profile)
|
||||
_profile_cfg_path = os.path.join(str(_profile_home), "config.yaml")
|
||||
if not os.path.isfile(_profile_cfg_path):
|
||||
return None, None
|
||||
return None, None, None
|
||||
import yaml
|
||||
|
||||
with open(_profile_cfg_path, encoding="utf-8") as _f:
|
||||
_pcfg = yaml.safe_load(_f) or {}
|
||||
if not isinstance(_pcfg, dict):
|
||||
return None, None
|
||||
return None, None, None
|
||||
_model_cfg = _pcfg.get("model") or {}
|
||||
if not isinstance(_model_cfg, dict):
|
||||
return None, None
|
||||
return None, None, _pcfg
|
||||
_provider = (_model_cfg.get("provider") or "").strip() or None
|
||||
_default = (_model_cfg.get("default") or "").strip() or None
|
||||
except Exception:
|
||||
@@ -5802,15 +5804,15 @@ def _read_profile_model_config(
|
||||
getattr(session, "profile", None),
|
||||
exc_info=True,
|
||||
)
|
||||
return None, None
|
||||
return None, None, None
|
||||
|
||||
_requested = _clean_session_model_provider(requested_provider)
|
||||
if _requested:
|
||||
_profile_prov = _clean_session_model_provider(_provider)
|
||||
if _profile_prov != _requested:
|
||||
return None, None
|
||||
return None, _default
|
||||
return _provider, _default
|
||||
return None, None, _pcfg
|
||||
return None, _default, _pcfg
|
||||
return _provider, _default, _pcfg
|
||||
|
||||
|
||||
def _load_profile_config_dict(session) -> dict | None:
|
||||
@@ -5840,6 +5842,34 @@ def _load_profile_config_dict(session) -> dict | None:
|
||||
return None
|
||||
|
||||
|
||||
def _ordered_custom_provider_model_ids(entry: dict) -> list[str]:
|
||||
"""Model ids from a custom_providers entry (default model + dict/list models)."""
|
||||
ordered: list[str] = []
|
||||
_cp_model = str(entry.get("model") or "").strip()
|
||||
if _cp_model:
|
||||
ordered.append(_cp_model)
|
||||
_cp_models = entry.get("models")
|
||||
if isinstance(_cp_models, dict):
|
||||
for _key in _cp_models.keys():
|
||||
if isinstance(_key, str):
|
||||
_kid = _key.strip()
|
||||
if _kid and _kid not in ordered:
|
||||
ordered.append(_kid)
|
||||
elif isinstance(_cp_models, list):
|
||||
for _item in _cp_models:
|
||||
if isinstance(_item, str):
|
||||
_mid = _item.strip()
|
||||
if _mid and _mid not in ordered:
|
||||
ordered.append(_mid)
|
||||
elif isinstance(_item, dict):
|
||||
_mid = str(
|
||||
_item.get("id") or _item.get("model") or _item.get("name") or ""
|
||||
).strip()
|
||||
if _mid and _mid not in ordered:
|
||||
ordered.append(_mid)
|
||||
return ordered
|
||||
|
||||
|
||||
def _repair_bare_custom_provider_model(
|
||||
bare_model: str,
|
||||
provider: str | None,
|
||||
@@ -5851,11 +5881,12 @@ def _repair_bare_custom_provider_model(
|
||||
Returns the fully namespaced model id when ``bare_model`` matches the suffix
|
||||
of a registered id on ``custom_providers``; otherwise None. Model ids are
|
||||
scanned in config declaration order (default ``model`` first, then
|
||||
``models`` dict keys) so repair is deterministic when suffixes collide.
|
||||
``models`` dict keys or list entries) so repair is deterministic when
|
||||
suffixes collide.
|
||||
|
||||
When ``config_obj`` is set (typically the session profile's config.yaml),
|
||||
only that object's ``custom_providers`` are scanned. Otherwise falls back
|
||||
to the process-global config for backward-compatible test stubs.
|
||||
only that object's ``custom_providers`` are scanned. Otherwise uses
|
||||
``get_config()`` for the active global config (not the raw ``cfg`` alias).
|
||||
"""
|
||||
try:
|
||||
model = str(bare_model or "").strip()
|
||||
@@ -5864,38 +5895,36 @@ def _repair_bare_custom_provider_model(
|
||||
return None
|
||||
if prov != "custom" and not str(prov).startswith("custom:"):
|
||||
return None
|
||||
from api.config import _custom_provider_entries, cfg as _active_cfg
|
||||
from api.config import (
|
||||
_custom_provider_entries,
|
||||
_custom_provider_slug_from_name,
|
||||
get_config,
|
||||
)
|
||||
|
||||
if isinstance(config_obj, dict):
|
||||
_entries = _custom_provider_entries(config_obj)
|
||||
else:
|
||||
_cfg = get_config()
|
||||
_entries = _custom_provider_entries(
|
||||
_active_cfg if isinstance(_active_cfg, dict) else None
|
||||
_cfg if isinstance(_cfg, dict) else None
|
||||
)
|
||||
_cp_name = str(prov.split(":", 1)[1] if ":" in prov else "").strip()
|
||||
if not _cp_name:
|
||||
return None
|
||||
_cp_name_lower = _cp_name.lower()
|
||||
prov_norm = str(prov).strip().lower()
|
||||
raw_suffix = prov_norm.removeprefix("custom:")
|
||||
_matching_cp = None
|
||||
for _entry in _entries:
|
||||
_entry_name = str(_entry.get("name") or "").strip().lower()
|
||||
if _entry_name == _cp_name_lower:
|
||||
entry_name = str(_entry.get("name") or "").strip().lower()
|
||||
slug = _custom_provider_slug_from_name(_entry.get("name"))
|
||||
if not slug:
|
||||
continue
|
||||
if (
|
||||
prov_norm in {entry_name, slug}
|
||||
or raw_suffix == slug.removeprefix("custom:")
|
||||
):
|
||||
_matching_cp = _entry
|
||||
break
|
||||
if not _matching_cp:
|
||||
return None
|
||||
_ordered_ids: list[str] = []
|
||||
_cp_model = str(_matching_cp.get("model") or "").strip()
|
||||
if _cp_model:
|
||||
_ordered_ids.append(_cp_model)
|
||||
_cp_models = _matching_cp.get("models")
|
||||
if isinstance(_cp_models, dict):
|
||||
for _key in _cp_models.keys():
|
||||
if isinstance(_key, str):
|
||||
_kid = _key.strip()
|
||||
if _kid and _kid not in _ordered_ids:
|
||||
_ordered_ids.append(_kid)
|
||||
for _id in _ordered_ids:
|
||||
for _id in _ordered_custom_provider_model_ids(_matching_cp):
|
||||
if "/" in _id and _id.rsplit("/", 1)[-1] == model:
|
||||
return _id
|
||||
return None
|
||||
@@ -6355,8 +6384,7 @@ def _resolve_effective_session_model_for_display(session) -> str:
|
||||
"""
|
||||
original_model = getattr(session, "model", None) or ""
|
||||
requested_provider = getattr(session, "model_provider", None)
|
||||
_pp_provider, _pp_default = _read_profile_model_config(session, requested_provider)
|
||||
_pp_cfg = _load_profile_config_dict(session)
|
||||
_pp_provider, _pp_default, _pp_cfg = _read_profile_model_config(session, requested_provider)
|
||||
effective_model, _provider, _changed = _resolve_compatible_session_model_state(
|
||||
original_model or None,
|
||||
requested_provider,
|
||||
@@ -6380,8 +6408,7 @@ def _resolve_effective_session_model_for_display(session) -> str:
|
||||
def _resolve_effective_session_model_provider_for_display(session) -> str | None:
|
||||
original_model = getattr(session, "model", None) or ""
|
||||
requested_provider = getattr(session, "model_provider", None)
|
||||
_pp_provider, _pp_default = _read_profile_model_config(session, requested_provider)
|
||||
_pp_cfg = _load_profile_config_dict(session)
|
||||
_pp_provider, _pp_default, _pp_cfg = _read_profile_model_config(session, requested_provider)
|
||||
_model, provider, _changed = _resolve_compatible_session_model_state(
|
||||
original_model or None,
|
||||
requested_provider,
|
||||
@@ -18541,8 +18568,7 @@ def start_session_turn(
|
||||
# background task before its first human turn has an empty s.model, and
|
||||
# without the profile defaults the resolver would fall back to the global
|
||||
# DEFAULT_MODEL instead of the profile's configured default (greptile flag).
|
||||
_pp_provider, _pp_default = _read_profile_model_config(s, requested_provider)
|
||||
_pp_cfg = _load_profile_config_dict(s)
|
||||
_pp_provider, _pp_default, _pp_cfg = _read_profile_model_config(s, requested_provider)
|
||||
model, model_provider, normalized_model = _resolve_compatible_session_model_state(
|
||||
requested_model,
|
||||
requested_provider,
|
||||
@@ -18718,8 +18744,7 @@ def _handle_goal_command(handler, body):
|
||||
if "model_provider" in body
|
||||
else getattr(s, "model_provider", None)
|
||||
)
|
||||
_pp_provider, _pp_default = _read_profile_model_config(s, requested_provider)
|
||||
_pp_cfg = _load_profile_config_dict(s)
|
||||
_pp_provider, _pp_default, _pp_cfg = _read_profile_model_config(s, requested_provider)
|
||||
model, model_provider, normalized_model = _resolve_compatible_session_model_state(
|
||||
requested_model,
|
||||
requested_provider,
|
||||
@@ -18771,8 +18796,7 @@ def _handle_goal_command(handler, body):
|
||||
if "model_provider" in body
|
||||
else getattr(s, "model_provider", None)
|
||||
)
|
||||
_pp_provider, _pp_default = _read_profile_model_config(s, requested_provider)
|
||||
_pp_cfg = _load_profile_config_dict(s)
|
||||
_pp_provider, _pp_default, _pp_cfg = _read_profile_model_config(s, requested_provider)
|
||||
model, model_provider, normalized_model = _resolve_compatible_session_model_state(
|
||||
requested_model,
|
||||
requested_provider,
|
||||
@@ -18918,8 +18942,7 @@ def _handle_chat_start(handler, body, diag=None):
|
||||
if "model_provider" in body
|
||||
else getattr(s, "model_provider", None)
|
||||
)
|
||||
_pp_provider, _pp_default = _read_profile_model_config(s, requested_provider)
|
||||
_pp_cfg = _load_profile_config_dict(s)
|
||||
_pp_provider, _pp_default, _pp_cfg = _read_profile_model_config(s, requested_provider)
|
||||
explicit_model_pick = bool(body.get("explicit_model_pick"))
|
||||
moa_config = None
|
||||
if body.get("moa_config"):
|
||||
@@ -19034,8 +19057,7 @@ def _handle_chat_sync(handler, body):
|
||||
_sync_requested_provider = (
|
||||
body.get("model_provider") if "model_provider" in body else getattr(s, "model_provider", None)
|
||||
)
|
||||
_pp_provider, _pp_default = _read_profile_model_config(s, _sync_requested_provider)
|
||||
_pp_cfg = _load_profile_config_dict(s)
|
||||
_pp_provider, _pp_default, _pp_cfg = _read_profile_model_config(s, _sync_requested_provider)
|
||||
model, model_provider = _resolve_compatible_session_model_state(
|
||||
body.get("model") or s.model,
|
||||
_sync_requested_provider,
|
||||
|
||||
@@ -225,9 +225,69 @@ class TestRepairBareCustomProviderModel:
|
||||
]
|
||||
with patch("api.config.cfg", {"custom_providers": custom_cfg}):
|
||||
assert _repair_bare_custom_provider_model(
|
||||
"shared-suffix", "custom:llm-proxy"
|
||||
"shared-suffix", "custom:llm-proxy",
|
||||
config_obj={"custom_providers": custom_cfg},
|
||||
) == "org-a/shared-suffix"
|
||||
|
||||
def test_repairs_display_name_provider_via_slug(self):
|
||||
from api.routes import _repair_bare_custom_provider_model
|
||||
|
||||
cfg = {
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "My Proxy",
|
||||
"models": {"x-ai/grok-composer-2.5-fast": {}},
|
||||
}
|
||||
]
|
||||
}
|
||||
assert _repair_bare_custom_provider_model(
|
||||
"grok-composer-2.5-fast",
|
||||
"custom:my-proxy",
|
||||
config_obj=cfg,
|
||||
) == "x-ai/grok-composer-2.5-fast"
|
||||
|
||||
def test_repairs_list_form_models_catalog(self):
|
||||
from api.routes import _repair_bare_custom_provider_model
|
||||
|
||||
cfg = {
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "team",
|
||||
"models": [
|
||||
"profile-list/shared",
|
||||
{"id": "profile-list/other"},
|
||||
],
|
||||
}
|
||||
]
|
||||
}
|
||||
assert _repair_bare_custom_provider_model(
|
||||
"shared",
|
||||
"custom:team",
|
||||
config_obj=cfg,
|
||||
) == "profile-list/shared"
|
||||
|
||||
def test_uses_get_config_when_config_obj_is_none(self):
|
||||
from unittest.mock import patch
|
||||
from api.routes import _repair_bare_custom_provider_model
|
||||
|
||||
cfg = {
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "team",
|
||||
"models": {"vendor/shared": {}},
|
||||
}
|
||||
]
|
||||
}
|
||||
with patch("api.config.get_config") as mock_get_config:
|
||||
mock_get_config.return_value = cfg
|
||||
result = _repair_bare_custom_provider_model(
|
||||
"shared",
|
||||
"custom:team",
|
||||
config_obj=None,
|
||||
)
|
||||
assert result == "vendor/shared"
|
||||
mock_get_config.assert_called_once_with()
|
||||
|
||||
def test_malformed_custom_provider_name_fails_closed(self):
|
||||
from api.routes import _repair_bare_custom_provider_model
|
||||
|
||||
@@ -289,7 +349,7 @@ class TestReadProfileModelConfigWithExplicitProvider:
|
||||
lambda _p: str(profile_home),
|
||||
)
|
||||
|
||||
provider, default = _read_profile_model_config(_Session(), "custom:my-proxy")
|
||||
provider, default, _ = _read_profile_model_config(_Session(), "custom:my-proxy")
|
||||
assert provider is None
|
||||
assert default == "x-ai/grok-composer-2.5-fast"
|
||||
|
||||
@@ -313,6 +373,6 @@ class TestReadProfileModelConfigWithExplicitProvider:
|
||||
lambda _p: str(profile_home),
|
||||
)
|
||||
|
||||
provider, default = _read_profile_model_config(_Session(), "custom:other-proxy")
|
||||
provider, default, _ = _read_profile_model_config(_Session(), "custom:other-proxy")
|
||||
assert provider is None
|
||||
assert default is None
|
||||
Reference in new issue
Block a user