From de47d19f1f860c22bbb2235511d486c6342d21a9 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:38:11 +0530 Subject: [PATCH] fix(models): one canonical override schema, fill-gap _default semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the model_overrides feature: - ONE canonical override schema everywhere. get_model_info previously merged the override dict raw into the models.dev catalog shape ({**raw, **override}), so the documented context_window/supports_* keys silently did nothing on that path (cost guard, inventory) while working in capabilities/context paths — same config key, two incompatible schemas. Overrides are now translated into the catalog shape at the get_model_info boundary (_override_to_catalog_shape), and sub-dicts (limit, modalities) are MERGED, not clobbered — an override setting only context_window no longer wipes the catalog's limit.output. - _default is now a FILL-GAP default, not an override: it applies only to models the catalog does not know (the #8731/#84482 self-unblock path) and never displaces catalog data. A _default: {context_window: 128000} can no longer clamp every model of a provider. Explicit per-provider+model entries keep their win-over-catalog semantics. - Early-chain _override_context_window (model_metadata step 0b) is explicit-only, so a _default can never preempt custom_providers per-model settings or live probes; fill-gap defaults apply at the lookup_models_dev_context catalog-miss boundary (step 5f) instead. This fixes the precedence inversion where a provider/global _default silently overrode an explicit per-endpoint per-model context_length. - Provider keys accept BOTH id spaces (Hermes id and models.dev id: copilot/github-copilot both work) and model ids match case-insensitively, mirroring catalog lookup. - Malformed override values (context_window: '512k') log a one-shot warning instead of being silently swallowed. - DEFAULT_CONFIG comment: removed the false family/dated-snapshot inheritance claim, documented the recognized field list, fill-gap semantics, and the id-space rule. - Tests: rewritten for the new contracts (fill-gap invariants, dual-id-space keys, sub-dict merge preservation, one-shot warning); added a real-config-yaml e2e plumbing test (mutation-checked: fails when the config key wiring is broken). --- agent/model_metadata.py | 12 +- agent/models_dev.py | 367 +++++++++++++++++++++++---------- hermes_cli/config_defaults.py | 37 +++- tests/agent/test_models_dev.py | 255 ++++++++++++++++++++--- 4 files changed, 512 insertions(+), 159 deletions(-) diff --git a/agent/model_metadata.py b/agent/model_metadata.py index c137bdd6a9..a638798a97 100644 --- a/agent/model_metadata.py +++ b/agent/model_metadata.py @@ -2639,11 +2639,13 @@ def get_model_context_length( logger.debug("MoA aggregator context-length resolution failed", exc_info=True) # Fall through to the generic default if aggregator resolution failed. - # 0b. model_overrides config — per-provider+model context_window override. - # This is the supported self-unblock path for models with wrong or missing - # context in models.dev (#84482) and for custom/local models not in the - # catalog (#8731). Checked before custom_providers (step 0c) and before any - # network probe so it never blocks. + # 0b. model_overrides config — EXPLICIT per-provider+model context_window + # override only (fill-gap _default entries are applied later, inside + # lookup_models_dev_context at step 5f, once the catalog has actually + # missed — so a _default can never preempt custom_providers or live + # probes). This is the supported self-unblock path for models with + # wrong context in models.dev (#84482) and for custom/local models + # (#8731). Config-read only; never blocks on the network. if provider and model: try: from agent.models_dev import _override_context_window diff --git a/agent/models_dev.py b/agent/models_dev.py index bcab594546..804694e7d8 100644 --- a/agent/models_dev.py +++ b/agent/models_dev.py @@ -500,28 +500,28 @@ def lookup_models_dev_context(provider: str, model: str) -> Optional[int]: Returns the context window in tokens, or None if not found. Handles case-insensitive matching and filters out context=0 entries. - A ``model_overrides`` config entry for this provider+model (or its - ``_default`` fallback) wins over the catalog value — this is the - supported self-unblock path for models with wrong or missing context - in models.dev (#84482). + An EXPLICIT ``model_overrides`` config entry for this provider+model + wins over the catalog value; ``_default`` entries fill the gap only + when the catalog has no answer — the supported self-unblock path for + models with wrong or missing context in models.dev (#84482). """ - # Config override — checked before catalog so it always wins. + # Explicit config override — checked before catalog so it always wins. override_ctx = _override_context_window(provider, model) if override_ctx is not None: return override_ctx mdev_provider_id = PROVIDER_TO_MODELS_DEV.get(provider) if not mdev_provider_id: - return None + return _default_override_context(provider) data = fetch_models_dev() provider_data = data.get(mdev_provider_id) if not isinstance(provider_data, dict): - return None + return _default_override_context(provider) models = provider_data.get("models", {}) if not isinstance(models, dict): - return None + return _default_override_context(provider) # Exact match entry = models.get(model) @@ -560,7 +560,16 @@ def lookup_models_dev_context(provider: str, model: str) -> Optional[int]: if ctx: return ctx - return None + # Catalog miss — a _default override may fill the gap (#84482). + return _default_override_context(provider) + + +def _default_override_context(provider: str) -> Optional[int]: + """Fill-gap context from a ``_default`` override, for catalog misses.""" + default = _default_model_override(provider) + if default is None: + return None + return _override_int(default, "context_window") def _extract_context(entry: Dict[str, Any]) -> Optional[int]: @@ -600,19 +609,27 @@ class ModelCapabilities: # Per-model metadata overrides (config.yaml → model_overrides) # # --------------------------------------------------------------------------- # # -# Resolution order for every query function below: -# 1. ``model_overrides..`` — explicit per-provider+model -# 2. ``model_overrides.._default`` — per-provider default -# 3. ``model_overrides._default`` — global default -# 4. models.dev / OpenRouter / hardcoded — normal catalog resolution +# Canonical override schema (the ONLY key space consumers accept): +# context_window, max_output_tokens, supports_tools, supports_vision, +# supports_reasoning, model_family # -# An override may set any subset of fields; unspecified fields fall through to -# the catalog value. For a model id NOT in the catalog, the override is the -# only source of metadata — this is the supported self-unblock path for new -# or custom models (#84482, #8731). +# Resolution semantics: +# 1. ``model_overrides..`` — explicit override. Always +# wins over the catalog for the fields it sets (partial patch). +# 2. ``model_overrides.._default`` / ``model_overrides._default`` +# — FILL-GAP defaults. They apply ONLY to models the catalog does not +# know (the #8731/#84482 self-unblock path for custom/local/new +# models) and never displace catalog data for known models. A +# ``_default: {context_window: 128000}`` therefore cannot clamp every +# catalog-known model of a provider. +# +# Provider keys accept the Hermes provider id (as used elsewhere in +# config.yaml) or the models.dev provider id. Model ids match exactly, +# then case-insensitively (mirroring catalog lookup). _OVERRIDE_CACHE: Optional[Dict[str, Any]] = None -_OVERRIDE_CACHE_CFG_HASH: int = 0 +_OVERRIDE_CACHE_CFG_ID: int = 0 +_OVERRIDE_WARNED_KEYS: set = set() def _load_model_overrides() -> Dict[str, Any]: @@ -621,76 +638,210 @@ def _load_model_overrides() -> Dict[str, Any]: Caches by ``id(cfg)`` so a config reload (new dict identity) invalidates automatically. Returns empty dict on any failure. """ - global _OVERRIDE_CACHE, _OVERRIDE_CACHE_CFG_HASH + global _OVERRIDE_CACHE, _OVERRIDE_CACHE_CFG_ID try: from hermes_cli.config import cfg_get, load_config_readonly cfg = load_config_readonly() cfg_id = id(cfg) - if cfg_id == _OVERRIDE_CACHE_CFG_HASH and _OVERRIDE_CACHE is not None: + if cfg_id == _OVERRIDE_CACHE_CFG_ID and _OVERRIDE_CACHE is not None: return _OVERRIDE_CACHE raw = cfg_get(cfg, "model_overrides", default={}) overrides = raw if isinstance(raw, dict) else {} _OVERRIDE_CACHE = overrides - _OVERRIDE_CACHE_CFG_HASH = cfg_id + _OVERRIDE_CACHE_CFG_ID = cfg_id return overrides except Exception: return {} -def _resolve_model_override( - provider: str, model: str -) -> Optional[Dict[str, Any]]: - """Resolve the override dict for a provider+model, or None. +def _provider_override_section(provider: str) -> Optional[Dict[str, Any]]: + """Return the override section for *provider*, or None. - Checks per-provider+model, then per-provider ``_default``, then global - ``_default``. Returns the first match (which may be partially populated — - callers only read the keys they care about). + Accepts either the Hermes provider id or the models.dev provider id as + the config key, so ``copilot`` and ``github-copilot`` both work + regardless of which id space a caller passes in. """ overrides = _load_model_overrides() if not overrides: return None - provider_key = (provider or "").strip() - model_key = (model or "").strip() - if not provider_key and not model_key: + if not provider_key: return None - # 1. Per-provider+model - provider_section = overrides.get(provider_key) - if isinstance(provider_section, dict) and model_key: - model_section = provider_section.get(model_key) - if isinstance(model_section, dict): - return model_section - - # 2. Per-provider _default - if isinstance(provider_section, dict): - default = provider_section.get("_default") - if isinstance(default, dict): - return default - - # 3. Global _default - global_default = overrides.get("_default") - if isinstance(global_default, dict): - return global_default + candidates = [provider_key] + mapped = PROVIDER_TO_MODELS_DEV.get(provider_key) + if mapped and mapped != provider_key: + candidates.append(mapped) + # Reverse: caller passed a models.dev id, config keyed by Hermes id. + for hermes_id, mdev_id in PROVIDER_TO_MODELS_DEV.items(): + if mdev_id == provider_key and hermes_id != provider_key: + candidates.append(hermes_id) + for key in candidates: + section = overrides.get(key) + if isinstance(section, dict): + return section return None -def _override_context_window( - provider: str, model: str -) -> Optional[int]: - """Return the overridden context_window, or None.""" - ov = _resolve_model_override(provider, model) - if ov is None: +def _explicit_model_override(provider: str, model: str) -> Optional[Dict[str, Any]]: + """Return the explicit per-provider+model override dict, or None. + + Model ids match exactly first, then case-insensitively (skipping the + ``_default`` sentinel), mirroring catalog lookup behavior. + """ + model_key = (model or "").strip() + if not model_key: return None - raw = ov.get("context_window") + section = _provider_override_section(provider) + if section is None: + return None + + entry = section.get(model_key) + if isinstance(entry, dict): + return entry + + model_lower = model_key.lower() + for mid, mdata in section.items(): + if mid == "_default": + continue + if mid.lower() == model_lower and isinstance(mdata, dict): + return mdata + return None + + +def _default_model_override(provider: str) -> Optional[Dict[str, Any]]: + """Return the fill-gap ``_default`` override for *provider*, or None. + + Checks the per-provider ``_default`` first, then the global one. Only + consulted for models the catalog does not know — see the block comment. + """ + section = _provider_override_section(provider) + if section is not None: + default = section.get("_default") + if isinstance(default, dict): + return default + overrides = _load_model_overrides() + global_default = overrides.get("_default") + if isinstance(global_default, dict): + return global_default + return None + + +def _override_for( + provider: str, model: str, *, catalog_hit: bool +) -> Optional[Dict[str, Any]]: + """Select the override dict for a lookup, honoring fill-gap semantics. + + Explicit per-provider+model overrides always apply. ``_default`` + entries apply only when the catalog has no entry for the model. + """ + explicit = _explicit_model_override(provider, model) + if explicit is not None: + return explicit + if catalog_hit: + return None + return _default_model_override(provider) + + +def _override_int(override: Dict[str, Any], key: str) -> Optional[int]: + """Coerce an override field to a positive int, warning once on garbage.""" + raw = override.get(key) if raw is None: return None try: - ctx = int(raw) - return ctx if ctx > 0 else None + value = int(raw) + if value > 0: + return value except (TypeError, ValueError): + pass + warn_key = (key, repr(raw)) + if warn_key not in _OVERRIDE_WARNED_KEYS: + _OVERRIDE_WARNED_KEYS.add(warn_key) + logger.warning( + "model_overrides: ignoring invalid %s value %r " + "(expected a positive integer)", key, raw, + ) + return None + + +def _override_context_window(provider: str, model: str) -> Optional[int]: + """Return the EXPLICITLY overridden context_window, or None. + + Explicit-only on purpose: this runs early in the resolution chain + (agent/model_metadata.py step 0b, before custom_providers and live + probes), where a ``_default`` must not preempt more specific sources. + Fill-gap defaults are applied later by ``lookup_models_dev_context`` + once the catalog has actually missed. + """ + ov = _explicit_model_override(provider, model) + if ov is None: return None + return _override_int(ov, "context_window") + + +def _override_to_catalog_shape(override: Dict[str, Any]) -> Dict[str, Any]: + """Translate canonical override keys into a models.dev-shaped patch. + + ``get_model_info``/``_parse_model_info`` consume the raw catalog shape + (``limit.context``, ``tool_call``, ...). All override consumers accept + ONE canonical schema (the documented ``context_window``/``supports_*`` + keys), so this boundary translates rather than forcing users to know + the internal catalog shape. + """ + patch: Dict[str, Any] = {} + limit: Dict[str, Any] = {} + ctx = _override_int(override, "context_window") + if ctx is not None: + limit["context"] = ctx + out = _override_int(override, "max_output_tokens") + if out is not None: + limit["output"] = out + if limit: + patch["limit"] = limit + if "supports_tools" in override: + patch["tool_call"] = bool(override["supports_tools"]) + if "supports_reasoning" in override: + patch["reasoning"] = bool(override["supports_reasoning"]) + if "supports_vision" in override: + patch["attachment"] = bool(override["supports_vision"]) + patch["_vision_override"] = bool(override["supports_vision"]) + if "model_family" in override: + patch["family"] = str(override["model_family"] or "") + return patch + + +def _merge_catalog_entry_with_override( + raw: Dict[str, Any], override: Dict[str, Any] +) -> Dict[str, Any]: + """Patch a catalog entry with a canonical-schema override. + + Sub-dicts (``limit``, ``modalities``) are merged, not clobbered — an + override setting only ``context_window`` must not wipe the catalog's + ``limit.output``. + """ + shaped = _override_to_catalog_shape(override) + merged = dict(raw) + limit_patch = shaped.pop("limit", None) + if limit_patch: + base_limit = raw.get("limit") + base_limit = dict(base_limit) if isinstance(base_limit, dict) else {} + base_limit.update(limit_patch) + merged["limit"] = base_limit + vision_override = shaped.pop("_vision_override", None) + if vision_override is not None: + base_mods = raw.get("modalities") + base_mods = dict(base_mods) if isinstance(base_mods, dict) else {} + input_mods = base_mods.get("input") + input_mods = list(input_mods) if isinstance(input_mods, list) else [] + if vision_override and "image" not in input_mods: + input_mods.append("image") + elif not vision_override and "image" in input_mods: + input_mods.remove("image") + base_mods["input"] = input_mods + merged["modalities"] = base_mods + merged.update(shaped) + return merged def _get_provider_models(provider: str) -> Optional[Dict[str, Any]]: @@ -736,14 +887,13 @@ def get_model_capabilities(provider: str, model: str) -> Optional[ModelCapabilit Uses the existing fetch_models_dev() and PROVIDER_TO_MODELS_DEV mapping. Returns None if model not found. - ``model_overrides`` config entries (per-provider+model, per-provider - ``_default``, or global ``_default``) win over catalog values. For a - model id NOT in the catalog, the override is the only source of - metadata — this is the supported self-unblock path for custom/local - models (#8731) and for models with wrong context in models.dev - (#84482). An override may set any subset of fields; unspecified fields - fall through to the catalog value (or sensible defaults when the model - is absent from the catalog entirely). + EXPLICIT ``model_overrides`` entries (per-provider+model) win over + catalog values for the fields they set. ``_default`` entries fill the + gap only for models the catalog does not know — the supported + self-unblock path for custom/local models (#8731) and for models with + wrong metadata in models.dev (#84482). An override may set any subset + of fields; unspecified fields fall through to the catalog value (or + sensible defaults when the model is absent from the catalog). Extracts from model entry fields: - reasoning (bool) → supports_reasoning @@ -753,14 +903,13 @@ def get_model_capabilities(provider: str, model: str) -> Optional[ModelCapabilit - limit.output (int) → max_output_tokens - family (str) → model_family """ - # Check config override first — it may fully replace the catalog entry - # or patch specific fields. For unknown models (not in catalog), the - # override is the sole source of metadata. - override = _resolve_model_override(provider, model) - models = _get_provider_models(provider) entry = _find_model_entry(models, model) if models is not None else None + # Select the override AFTER the catalog lookup: explicit overrides + # always apply; _default entries only fill gaps for catalog misses. + override = _override_for(provider, model, catalog_hit=entry is not None) + # If no catalog entry and no override, we can't resolve capabilities. if entry is None and override is None: return None @@ -812,20 +961,12 @@ def get_model_capabilities(provider: str, model: str) -> Optional[ModelCapabilit supports_vision = bool(override["supports_vision"]) if "supports_reasoning" in override: supports_reasoning = bool(override["supports_reasoning"]) - if "context_window" in override: - try: - ctx_ov = int(override["context_window"]) - if ctx_ov > 0: - context_window = ctx_ov - except (TypeError, ValueError): - pass - if "max_output_tokens" in override: - try: - out_ov = int(override["max_output_tokens"]) - if out_ov > 0: - max_output_tokens = out_ov - except (TypeError, ValueError): - pass + ctx_ov = _override_int(override, "context_window") + if ctx_ov is not None: + context_window = ctx_ov + out_ov = _override_int(override, "max_output_tokens") + if out_ov is not None: + max_output_tokens = out_ov if "model_family" in override: model_family = str(override["model_family"] or "") @@ -1040,50 +1181,52 @@ def get_model_info( Accepts Hermes or models.dev provider ID. Tries exact match then case-insensitive fallback. Returns None if not found. - ``model_overrides`` config entries (per-provider+model, per-provider - ``_default``, or global ``_default``) patch the catalog entry's fields - when present. For a model id NOT in the catalog, the override is the - sole source of metadata — this is the supported self-unblock path - for custom/local models (#8731) and for models with wrong context - in models.dev (#84482). + ``model_overrides`` entries use the SAME canonical schema as every + other consumer (``context_window``, ``max_output_tokens``, + ``supports_*``, ``model_family``) — they are translated into the + catalog shape at this boundary, and sub-dicts (``limit``, + ``modalities``) are merged rather than clobbered. EXPLICIT entries + patch known catalog models; ``_default`` entries fill the gap only + for models the catalog does not know (#8731, #84482). """ - override = _resolve_model_override(provider_id, model_id) - mdev_id = PROVIDER_TO_MODELS_DEV.get(provider_id, provider_id) + def _from_override_alone() -> Optional[ModelInfo]: + override = _override_for(provider_id, model_id, catalog_hit=False) + if override is None: + return None + shaped = _merge_catalog_entry_with_override({}, override) + shaped.pop("_vision_override", None) + return _parse_model_info(model_id, shaped, mdev_id) + data = fetch_models_dev() pdata = data.get(mdev_id) if not isinstance(pdata, dict): - # No catalog data — return from override alone if we have one. - if override is not None: - return _parse_model_info(model_id, override, mdev_id) - return None + return _from_override_alone() models = pdata.get("models", {}) if not isinstance(models, dict): + return _from_override_alone() + + def _with_override(mid: str, raw: Dict[str, Any]) -> ModelInfo: + override = _override_for(provider_id, model_id, catalog_hit=True) if override is not None: - return _parse_model_info(model_id, override, mdev_id) - return None + merged = _merge_catalog_entry_with_override(raw, override) + merged.pop("_vision_override", None) + return _parse_model_info(mid, merged, mdev_id) + return _parse_model_info(mid, raw, mdev_id) # Exact match raw = models.get(model_id) if isinstance(raw, dict): - if override is not None: - merged = {**raw, **override} - return _parse_model_info(model_id, merged, mdev_id) - return _parse_model_info(model_id, raw, mdev_id) + return _with_override(model_id, raw) # Case-insensitive fallback model_lower = model_id.lower() for mid, mdata in models.items(): if mid.lower() == model_lower and isinstance(mdata, dict): - if override is not None: - merged = {**mdata, **override} - return _parse_model_info(mid, merged, mdev_id) - return _parse_model_info(mid, mdata, mdev_id) + return _with_override(mid, mdata) - # Model not in catalog — return from override alone if we have one. - if override is not None: - return _parse_model_info(model_id, override, mdev_id) - - return None + # Model not in catalog — an override (explicit or _default) may still + # provide the metadata. + return _from_override_alone() diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index 4094873737..e52290d05b 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -2540,18 +2540,33 @@ DEFAULT_CONFIG = { }, # Per-model metadata overrides — manually declare context_window, - # max_output_tokens, capabilities, or cost for any provider+model. - # Overrides win over models.dev, OpenRouter, and hardcoded defaults. + # max_output_tokens, capabilities, or model family for any + # provider+model. Recognized fields: context_window, + # max_output_tokens, supports_tools, supports_vision, + # supports_reasoning, model_family. # - # Two scopes: - # 1. Per-provider+model: model_overrides.. - # 2. Per-provider default: model_overrides.._default - # 3. Global default: model_overrides._default + # Semantics: + # 1. Explicit (model_overrides..): wins over + # models.dev, OpenRouter, and hardcoded defaults for the fields + # it sets. NOTE: an explicit model.context_length (global) and a + # custom_providers per-model context_length are user settings at + # other layers and are consulted in the resolution chain order + # documented in agent/model_metadata.py. + # 2. Fill-gap defaults (model_overrides.._default and + # model_overrides._default): apply ONLY to models the catalog + # does not know. They never displace catalog data for known + # models, so a _default cannot accidentally clamp every model + # of a provider. # - # An unknown model id (not in models.dev) inherits base metadata from - # its family/dated-snapshot entry before patching, so overriding a - # model the catalog doesn't know yet is the supported self-unblock - # path (#84482). + # An unknown model id (not in models.dev) starts from safe defaults + # (200K context, tools on, vision/reasoning off) and the override + # patches the fields it sets — overriding a model the catalog + # doesn't know yet is the supported self-unblock path (#84482, + # #8731). + # + # Provider keys accept the Hermes provider id (as used elsewhere in + # this file) or the models.dev provider id; model ids match + # case-insensitively. # # Example: # model_overrides: @@ -2566,7 +2581,7 @@ DEFAULT_CONFIG = { # supports_vision: true # supports_reasoning: false # supports_tools: true - # _default: + # _default: # fill-gap only: models not in the catalog # context_window: 128000 "model_overrides": {}, diff --git a/tests/agent/test_models_dev.py b/tests/agent/test_models_dev.py index ee34dee110..1bc3365b69 100644 --- a/tests/agent/test_models_dev.py +++ b/tests/agent/test_models_dev.py @@ -9,8 +9,10 @@ import pytest from agent.models_dev import ( PROVIDER_TO_MODELS_DEV, _extract_context, + _default_model_override, + _explicit_model_override, _override_context_window, - _resolve_model_override, + _override_for, fetch_models_dev, get_model_capabilities, get_model_info, @@ -407,7 +409,7 @@ class TestModelOverrides: import agent.models_dev as md return patch.object(md, "_load_model_overrides", return_value=overrides_dict) - # --- _resolve_model_override --- + # --- override resolution --- def test_per_provider_model_override(self): """Per-provider+model override is found first.""" @@ -417,39 +419,90 @@ class TestModelOverrides: }, } with self._setup_overrides(overrides): - result = _resolve_model_override("upstage", "solar-pro4") + result = _explicit_model_override("upstage", "solar-pro4") assert result is not None assert result["context_window"] == 524288 - def test_per_provider_default_fallback(self): - """Per-provider _default is used when model not found.""" + def test_explicit_override_case_insensitive_model(self): + """Model ids match case-insensitively, mirroring catalog lookup.""" + overrides = { + "upstage": { + "Solar-Pro4": {"context_window": 524288}, + }, + } + with self._setup_overrides(overrides): + result = _explicit_model_override("upstage", "solar-pro4") + assert result is not None + assert result["context_window"] == 524288 + + def test_provider_key_accepts_either_id_space(self): + """Override keyed by Hermes id resolves for models.dev id and back.""" + overrides = { + "copilot": { + "my-model": {"context_window": 111111}, + }, + } + with self._setup_overrides(overrides): + # Caller passes the models.dev id; config keyed by Hermes id. + result = _explicit_model_override("github-copilot", "my-model") + assert result is not None + assert result["context_window"] == 111111 + + overrides = { + "github-copilot": { + "my-model": {"context_window": 222222}, + }, + } + with self._setup_overrides(overrides): + # Caller passes the Hermes id; config keyed by models.dev id. + result = _explicit_model_override("copilot", "my-model") + assert result is not None + assert result["context_window"] == 222222 + + def test_default_fills_gap_for_unknown_model(self): + """_default applies to models the catalog does not know.""" overrides = { "upstage": { "_default": {"context_window": 128000}, }, } with self._setup_overrides(overrides): - result = _resolve_model_override("upstage", "unknown-model") + result = _override_for("upstage", "unknown-model", catalog_hit=False) assert result is not None assert result["context_window"] == 128000 + def test_default_does_not_clamp_catalog_known_model(self): + """FILL-GAP semantics: _default never displaces catalog data. + + A `_default: {context_window: 128000}` must not clamp every + catalog-known model of the provider — it only fills catalog misses. + """ + overrides = { + "upstage": { + "_default": {"context_window": 128000}, + }, + "_default": {"context_window": 65536}, + } + with self._setup_overrides(overrides): + result = _override_for("upstage", "known-model", catalog_hit=True) + assert result is None + def test_global_default_fallback(self): - """Global _default is used when provider not found.""" + """Global _default is used when provider has no section.""" overrides = { "_default": {"context_window": 65536}, } with self._setup_overrides(overrides): - result = _resolve_model_override("unknown-provider", "unknown-model") + result = _default_model_override("unknown-provider") assert result is not None assert result["context_window"] == 65536 def test_no_override_returns_none(self): - """No override found returns None.""" with self._setup_overrides({}): - result = _resolve_model_override("anthropic", "claude-sonnet-4") - assert result is None + assert _explicit_model_override("anthropic", "claude-sonnet-4") is None + assert _default_model_override("anthropic") is None - def test_per_provider_model_beats_default(self): + def test_explicit_beats_default(self): """Per-provider+model wins over per-provider _default.""" overrides = { "upstage": { @@ -458,12 +511,11 @@ class TestModelOverrides: }, } with self._setup_overrides(overrides): - result = _resolve_model_override("upstage", "solar-pro4") + result = _override_for("upstage", "solar-pro4", catalog_hit=False) assert result is not None assert result["context_window"] == 524288 def test_per_provider_default_beats_global(self): - """Per-provider _default wins over global _default.""" overrides = { "upstage": { "_default": {"context_window": 128000}, @@ -471,11 +523,11 @@ class TestModelOverrides: "_default": {"context_window": 65536}, } with self._setup_overrides(overrides): - result = _resolve_model_override("upstage", "unknown-model") + result = _default_model_override("upstage") assert result is not None assert result["context_window"] == 128000 - # --- _override_context_window --- + # --- _override_context_window (explicit-only, early-chain) --- def test_override_context_window_returns_value(self): overrides = { @@ -502,6 +554,35 @@ class TestModelOverrides: ctx = _override_context_window("upstage", "bad-model") assert ctx is None + def test_override_context_window_ignores_default(self): + """Early-chain lookup is explicit-only: a _default must not preempt + more specific sources (custom_providers, live probes).""" + overrides = { + "upstage": { + "_default": {"context_window": 128000}, + }, + } + with self._setup_overrides(overrides): + ctx = _override_context_window("upstage", "syn-pro") + assert ctx is None + + def test_malformed_context_window_warns_once(self, caplog): + """Garbage values are rejected with a one-shot warning, not silence.""" + import logging + + import agent.models_dev as md + md._OVERRIDE_WARNED_KEYS.clear() + overrides = { + "upstage": { + "bad-model": {"context_window": "512k"}, + }, + } + with self._setup_overrides(overrides), caplog.at_level(logging.WARNING): + assert _override_context_window("upstage", "bad-model") is None + assert _override_context_window("upstage", "bad-model") is None + warnings = [r for r in caplog.records if "model_overrides" in r.message] + assert len(warnings) == 1 + # --- get_model_capabilities with overrides --- def test_caps_override_unknown_model(self): @@ -526,7 +607,7 @@ class TestModelOverrides: assert caps.supports_tools is True def test_caps_override_patches_existing_catalog_entry(self): - """Override patches specific fields on a known catalog entry (#84482).""" + """Explicit override patches specific fields on a known entry (#84482).""" overrides = { "anthropic": { "claude-sonnet-4": { @@ -544,8 +625,20 @@ class TestModelOverrides: assert caps.supports_vision is True assert caps.supports_tools is True + def test_caps_default_does_not_clamp_catalog_model(self): + """A _default must not displace catalog data for known models.""" + overrides = { + "anthropic": { + "_default": {"context_window": 1000}, + }, + } + with self._setup_overrides(overrides), \ + patch("agent.models_dev.fetch_models_dev", return_value=CAPS_REGISTRY): + caps = get_model_capabilities("anthropic", "claude-sonnet-4") + assert caps is not None + assert caps.context_window != 1000 + def test_caps_no_override_no_catalog_returns_none(self): - """No override and no catalog entry → None.""" with self._setup_overrides({}), \ patch("agent.models_dev.fetch_models_dev", return_value={}): caps = get_model_capabilities("anthropic", "unknown-model") @@ -571,7 +664,6 @@ class TestModelOverrides: # --- lookup_models_dev_context with overrides --- def test_context_lookup_override_wins_over_catalog(self): - """Override context_window wins over models.dev catalog value.""" overrides = { "anthropic": { "claude-opus-4-6": {"context_window": 500000}, @@ -583,7 +675,6 @@ class TestModelOverrides: assert ctx == 500000 def test_context_lookup_override_for_unknown_provider(self): - """Override works for providers not in PROVIDER_TO_MODELS_DEV.""" overrides = { "upstage": { "solar-pro4": {"context_window": 524288}, @@ -594,18 +685,46 @@ class TestModelOverrides: ctx = lookup_models_dev_context("upstage", "solar-pro4") assert ctx == 524288 - # --- get_model_info with overrides --- + def test_context_lookup_default_fills_catalog_miss(self): + """_default supplies context for a model the catalog lacks.""" + overrides = { + "anthropic": { + "_default": {"context_window": 77777}, + }, + } + with self._setup_overrides(overrides), \ + patch("agent.models_dev.fetch_models_dev", return_value=SAMPLE_REGISTRY): + ctx = lookup_models_dev_context("anthropic", "model-not-in-catalog") + assert ctx == 77777 + + def test_context_lookup_default_does_not_clamp_catalog(self): + """_default must not beat a catalog-known model's real context.""" + overrides = { + "anthropic": { + "_default": {"context_window": 1000}, + }, + } + with self._setup_overrides(overrides), \ + patch("agent.models_dev.fetch_models_dev", return_value=SAMPLE_REGISTRY): + ctx = lookup_models_dev_context("anthropic", "claude-opus-4-6") + assert ctx == 1000000 # catalog value, not the _default + + # --- get_model_info with overrides (canonical schema) --- def test_model_info_override_for_unknown_model(self): - """Override provides full metadata for a model not in the catalog.""" + """Canonical-schema override provides metadata for an unknown model. + + Same key space as every other consumer — context_window, + max_output_tokens, supports_* — NOT the internal catalog shape. + """ overrides = { "custom:my-vllm": { "my-llava-model": { - "name": "My LLaVA Model", - "family": "llava", - "reasoning": False, - "tool_call": True, - "limit": {"context": 8192, "output": 4096}, + "model_family": "llava", + "supports_reasoning": False, + "supports_tools": True, + "context_window": 8192, + "max_output_tokens": 4096, }, }, } @@ -613,7 +732,6 @@ class TestModelOverrides: patch("agent.models_dev.fetch_models_dev", return_value={}): info = get_model_info("custom:my-vllm", "my-llava-model") assert info is not None - assert info.name == "My LLaVA Model" assert info.family == "llava" assert info.context_window == 8192 assert info.max_output == 4096 @@ -621,11 +739,15 @@ class TestModelOverrides: assert info.reasoning is False def test_model_info_override_merges_with_catalog(self): - """Override patches specific fields on a known catalog entry.""" + """Override patches context without clobbering the catalog's output. + + The limit sub-dict is MERGED: an override setting only + context_window preserves the catalog's limit.output. + """ overrides = { "anthropic": { "claude-sonnet-4-6": { - "limit": {"context": 500000, "output": 64000}, + "context_window": 500000, }, }, } @@ -633,8 +755,79 @@ class TestModelOverrides: patch("agent.models_dev.fetch_models_dev", return_value=SAMPLE_REGISTRY): info = get_model_info("anthropic", "claude-sonnet-4-6") assert info is not None - # Override wins + # Override wins for the field it sets assert info.context_window == 500000 + # Sub-dict merge: catalog's limit.output survives + assert info.max_output == 64000 # Non-overridden fields preserved from catalog assert info.name == "claude-sonnet-4-6" + def test_model_info_default_does_not_clamp_catalog(self): + """_default fills gaps only — known models keep catalog metadata.""" + overrides = { + "anthropic": { + "_default": {"context_window": 1000}, + }, + } + with self._setup_overrides(overrides), \ + patch("agent.models_dev.fetch_models_dev", return_value=SAMPLE_REGISTRY): + info = get_model_info("anthropic", "claude-sonnet-4-6") + assert info is not None + assert info.context_window == 1000000 + + # --- e2e config plumbing (real config.yaml, no _load_model_overrides mock) --- + + def test_e2e_overrides_load_from_real_config_yaml(self, tmp_path, monkeypatch): + """The real config path works end-to-end: config.yaml on disk -> + load_config_readonly -> cfg_get -> override applied. + + Every other test mocks _load_model_overrides; this one exercises + the actual wiring (key name, cfg accessor, cache invalidation). + """ + import importlib + + import agent.models_dev as md + import hermes_cli.config as hc + + home = tmp_path / "hermes" + home.mkdir() + (home / "config.yaml").write_text( + "model_overrides:\n" + " upstage:\n" + " solar-pro4:\n" + " context_window: 524288\n", + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + # Reset caches that memoize config identity/paths. + monkeypatch.setattr(md, "_OVERRIDE_CACHE", None) + monkeypatch.setattr(md, "_OVERRIDE_CACHE_CFG_ID", 0) + hc_cache = getattr(hc, "_LOAD_CONFIG_CACHE", None) + if isinstance(hc_cache, dict): + hc_cache.clear() + raw_cache = getattr(hc, "_RAW_CONFIG_CACHE", None) + if isinstance(raw_cache, dict): + raw_cache.clear() + importlib.reload # no-op guard: modules stay loaded, caches cleared + + with patch("agent.models_dev.fetch_models_dev", return_value={}): + ctx = lookup_models_dev_context("upstage", "solar-pro4") + assert ctx == 524288 + + def test_model_info_vision_override_sets_input_modality(self): + """supports_vision: true surfaces as an image input modality.""" + overrides = { + "custom:my-vllm": { + "my-model": { + "supports_vision": True, + "context_window": 8192, + }, + }, + } + with self._setup_overrides(overrides), \ + patch("agent.models_dev.fetch_models_dev", return_value={}): + info = get_model_info("custom:my-vllm", "my-model") + assert info is not None + assert "image" in info.input_modalities + assert info.attachment is True