From 311155fade43d1993ea968909a2ad36762345bbc Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 14 Oct 2026 21:46:48 +0400 Subject: [PATCH] refactor(profiles): build the seed routing view once; expand `enabled` too Review follow-ups on the env-ref name fix: - Resolve and locate the selected entry on one routing view, so the key/index invariant is explicit rather than two separate rebuilds. - `enabled` is read by the matcher and runtime sees it expanded, so include it in the routing fields. - Rename `_routed` to `_expand_routing_fields`; correct the `_get_named_custom_provider` docstring (it receives a snapshot, not necessarily the raw file). - Add a legacy `base_url: ${LAB_URL}` case (raw URL refs keep working). --- hermes_cli/profile_model_seed.py | 25 +++++++++++---------- hermes_cli/runtime_provider_custom.py | 4 ++-- tests/hermes_cli/test_profile_model_seed.py | 13 ++++++++--- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/hermes_cli/profile_model_seed.py b/hermes_cli/profile_model_seed.py index d90616830d..d9204eb5f7 100644 --- a/hermes_cli/profile_model_seed.py +++ b/hermes_cli/profile_model_seed.py @@ -19,10 +19,10 @@ def _requested_provider(model_cfg) -> str | None: # Fields name lookups match on. Runtime sees them ``${VAR}``-expanded (load_config expands the # whole file), so matching expands only these; copied entries stay raw and secrets stay refs. -_ROUTING_FIELDS = ("name", "provider_key", "base_url", "baseUrl", "url", "api") +_ROUTING_FIELDS = ("name", "provider_key", "base_url", "baseUrl", "url", "api", "enabled") -def _routed(entry): +def _expand_routing_fields(entry): if not isinstance(entry, dict): return entry return {k: _expand_env_vars(v) if k in _ROUTING_FIELDS else v for k, v in entry.items()} @@ -34,17 +34,16 @@ def _routing_view(cfg: dict) -> dict: view = dict(cfg) providers = cfg.get("providers") if isinstance(providers, dict): - view["providers"] = {k: _routed(v) for k, v in providers.items()} + view["providers"] = {k: _expand_routing_fields(v) for k, v in providers.items()} custom = cfg.get("custom_providers") if isinstance(custom, list): - view["custom_providers"] = [_routed(e) for e in custom] + view["custom_providers"] = [_expand_routing_fields(e) for e in custom] return view -def _resolve(provider: str, cfg: dict): - """Runtime named-endpoint lookup on *cfg* (routing fields expanded) without reading any - credential.""" - return _get_named_custom_provider(provider, config=_routing_view(cfg), resolve_credentials=False) +def _resolve(provider: str, view: dict): + """Runtime named-endpoint lookup on a routing *view* without reading any credential.""" + return _get_named_custom_provider(provider, config=view, resolve_credentials=False) def _source_of(cfg: dict, resolved: dict): @@ -81,7 +80,9 @@ def launch_model_seed(source_cfg: dict) -> dict: requested = _normalize_custom_provider_name(provider) providers = source_cfg.get("providers") providers = providers if isinstance(providers, dict) else {} - selected = _resolve(provider, source_cfg) + # Resolve and locate on ONE view so the found key/index addresses the raw entry. + view = _routing_view(source_cfg) + selected = _resolve(provider, view) if selected is None: exact = providers.get(provider) # Bare ``custom`` runs as provider ``custom`` and reads ``providers.custom`` tuning too. @@ -92,7 +93,7 @@ def launch_model_seed(source_cfg: dict) -> dict: ): seed["providers"] = {provider: exact} return seed - found = _source_of(_routing_view(source_cfg), selected) + found = _source_of(view, selected) if found is not None: section, where = found seed[section] = ( @@ -107,7 +108,7 @@ def _aliases(entry, key: str = "") -> frozenset[str]: """Every identity name-based lookups (runtime, ``/model`` pinning, pickers) accept for *entry*.""" if not isinstance(entry, dict): return frozenset() - entry = _routed(entry) + entry = _expand_routing_fields(entry) key = key or str(entry.get("provider_key") or "") return custom_provider_aliases(str(entry.get("name") or key), key) @@ -125,7 +126,7 @@ def merge_endpoint_seed(dst_cfg: dict, seed: dict) -> dict: seed_custom = seed.get("custom_providers") or [] provider = _requested_provider(seed.get("model")) claimed: frozenset[str] = frozenset() - if provider and _resolve(provider, seed) is not None: + if provider and _resolve(provider, _routing_view(seed)) is not None: claimed = frozenset().union( *(_aliases(e, str(k)) for k, e in seed_providers.items()), *(_aliases(e) for e in seed_custom), diff --git a/hermes_cli/runtime_provider_custom.py b/hermes_cli/runtime_provider_custom.py index 428f4512b3..34a9af0070 100644 --- a/hermes_cli/runtime_provider_custom.py +++ b/hermes_cli/runtime_provider_custom.py @@ -194,8 +194,8 @@ def _get_named_custom_provider( requested_provider: str, *, config: Optional[dict[str, Any]] = None, resolve_credentials: bool = True, ) -> Optional[dict[str, Any]]: - """Resolve a named endpoint. Creation-time config copying supplies raw config and skips - credential reads; ordinary runtime callers keep their existing scoped resolution.""" + """Resolve a named endpoint. Creation-time config copying supplies its own config snapshot + and skips credential reads; ordinary runtime callers keep their existing scoped resolution.""" requested_norm = _normalize_custom_provider_name(requested_provider or "") if not requested_norm or requested_norm == "auto" or _shadowed_by_builtin(requested_norm): return None diff --git a/tests/hermes_cli/test_profile_model_seed.py b/tests/hermes_cli/test_profile_model_seed.py index 6f943decbb..0ab725a203 100644 --- a/tests/hermes_cli/test_profile_model_seed.py +++ b/tests/hermes_cli/test_profile_model_seed.py @@ -158,10 +158,17 @@ def test_model_seed_selects_env_referenced_provider(monkeypatch): assert seed == {"model": source["model"], "providers": {"lab": endpoint}} -@pytest.mark.parametrize("schema", ["providers", "custom_providers"]) -def test_model_seed_matches_env_referenced_endpoint_names(monkeypatch, schema): +@pytest.mark.parametrize( + "schema,endpoint", + [ + ("providers", {"name": "${LAB_NAME}", "base_url": "http://127.0.0.1:11434/v1"}), + ("custom_providers", {"name": "${LAB_NAME}", "base_url": "http://127.0.0.1:11434/v1"}), + ("custom_providers", {"name": "Lab", "base_url": "${LAB_URL}"}), + ], +) +def test_model_seed_matches_env_referenced_endpoint_names(monkeypatch, schema, endpoint): monkeypatch.setenv("LAB_NAME", "Lab") - endpoint = {"name": "${LAB_NAME}", "base_url": "http://127.0.0.1:11434/v1"} + monkeypatch.setenv("LAB_URL", "http://127.0.0.1:11434/v1") catalog = {"gateway": endpoint} if schema == "providers" else [endpoint] source = {"model": {"provider": "custom:lab", "default": "synthetic-model"}, schema: catalog}