Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions agent/model_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,42 @@ def get_model_context_length(
if config_context_length is not None and isinstance(config_context_length, int) and config_context_length > 0:
return config_context_length

# 0a. Config-driven overrides from custom_providers[].models[].context_length.
# When provider/base_url is known, scope the lookup to the matching custom
# provider entry to avoid collisions between identically-named models on
# different endpoints. When unknown, allow matching any custom provider.
try:
from hermes_cli.config import load_config
_cfg = load_config()
_custom_providers = _cfg.get("custom_providers", []) if isinstance(_cfg, dict) else []
if isinstance(_custom_providers, list):
Comment on lines +947 to +953

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block calls hermes_cli.config.load_config() on every get_model_context_length() invocation, which does a deep-merge with defaults and reads from disk. Since this function can be called in hot paths (e.g., gateway request handling), consider using the lightweight read_raw_config() or caching the custom_providers lookup (e.g., via an internal module-level cache with a short TTL) to avoid repeated disk I/O and merges.

Copilot uses AI. Check for mistakes.
_normalized_base_url = base_url.rstrip("/") if isinstance(base_url, str) and base_url else None
_active_provider = str(provider).strip().lower() if isinstance(provider, str) and provider.strip() else None
_valid_entries = [cp for cp in _custom_providers if isinstance(cp, dict)]
for cp in _valid_entries:
_cp_base = str(cp.get("base_url", "")).strip().rstrip("/")
_cp_name = str(cp.get("name", "")).strip().lower()

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base URL normalization is inconsistent: _normalized_base_url = base_url.rstrip('/') does not strip whitespace, while _cp_base uses .strip().rstrip('/'). Using .strip().rstrip('/') for base_url too would avoid surprising mismatches if base_url has leading/trailing spaces.

Copilot uses AI. Check for mistakes.

if _normalized_base_url is not None and _cp_base:
if _cp_base != _normalized_base_url:
continue
elif _active_provider is not None and _cp_name:
if _cp_name != _active_provider:
continue

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provider-name scoping here compares provider (lowercased) directly to custom_providers[].name (lowercased). In practice, provider values elsewhere in the codebase can be menu keys like custom:<normalized-name> (see hermes_cli/runtime_provider.py’s _normalize_custom_provider_name) and/or normalize spaces to -, so this equality check will miss valid matches when base_url isn’t provided. Consider normalizing both sides consistently (e.g., strip a leading custom: from provider and apply the same space→dash normalization to cp['name']) before comparing.

Copilot uses AI. Check for mistakes.
elif len(_valid_entries) > 1:
# Ambiguous: skip unscoped entries when multiple providers exist
continue
Comment on lines +942 to +978

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says “When unknown, allow matching any custom provider,” but the implementation intentionally skips unscoped entries when multiple providers exist (and only effectively allows unscoped matching when there is a single custom provider). Update the comment to reflect the actual behavior to avoid future confusion.

Copilot uses AI. Check for mistakes.

_models = cp.get("models", {})
if isinstance(_models, dict):
_model_entry = _models.get(model, {})
if isinstance(_model_entry, dict):
_ctx = _model_entry.get("context_length")
if isinstance(_ctx, int) and _ctx > 0:
return _ctx
except Exception:
pass

# Normalise provider-prefixed model names (e.g. "local:model-name" →
# "model-name") so cache lookups and server queries use the bare ID that
# local servers actually know about. Ollama "model:tag" colons are preserved.
Expand Down
97 changes: 97 additions & 0 deletions tests/agent/test_model_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -709,3 +709,100 @@ def test_special_chars_in_model_name(self, tmp_path):
with patch("agent.model_metadata._get_context_cache_path", return_value=cache_file):
save_context_length(model, url, 200000)
assert get_cached_context_length(model, url) == 200000


class TestCustomProvidersContextLength:
"""Tests for config-driven context_length overrides in custom_providers."""

@patch("hermes_cli.config.load_config")
def test_match_by_provider_name(self, mock_load_config):
mock_load_config.return_value = {
"custom_providers": [
{
"name": "zai-customize",
"base_url": "https://open.bigmodel.cn/api/coding/paas/v4",
"models": {
"glm-5.1": {"context_length": 200000},
},
}
]
}
result = get_model_context_length("glm-5.1", provider="zai-customize")
assert result == 200000

@patch("hermes_cli.config.load_config")

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage gap: provider-name scoping is only tested with provider='zai-customize', but other code paths use provider keys like custom:<normalized-name> (e.g., custom:local-(127.0.0.1:4141)). Add a unit test that passes a provider string in that form and verifies the correct custom_providers entry is matched (or intentionally not matched) so the scoping behavior is validated against real runtime values.

Suggested change
@patch("hermes_cli.config.load_config")
@patch("hermes_cli.config.load_config")
def test_match_by_runtime_custom_provider_key(self, mock_load_config):
mock_load_config.return_value = {
"custom_providers": [
{
"name": "local-(127.0.0.1:4141)",
"base_url": "http://127.0.0.1:4141/v1",
"models": {
"llama3.2": {"context_length": 8192},
},
}
]
}
result = get_model_context_length(
"llama3.2", provider="custom:local-(127.0.0.1:4141)"
)
assert result == 8192
@patch("hermes_cli.config.load_config")

Copilot uses AI. Check for mistakes.
def test_match_by_base_url(self, mock_load_config):
mock_load_config.return_value = {
"custom_providers": [
{
"name": "zai-customize",
"base_url": "https://open.bigmodel.cn/api/coding/paas/v4",
"models": {
"glm-5.1": {"context_length": 200000},
},
}
]
}
result = get_model_context_length(
"glm-5.1", base_url="https://open.bigmodel.cn/api/coding/paas/v4"
)
assert result == 200000

@patch("hermes_cli.config.load_config")
def test_unmatched_provider_is_ignored(self, mock_load_config):
"""A model name in a different custom provider must not leak across."""
mock_load_config.return_value = {
"custom_providers": [
{
"name": "zai-customize",
"base_url": "https://open.bigmodel.cn/api/coding/paas/v4",
"models": {
"glm-5.1": {"context_length": 200000},
},
},
{
"name": "other-provider",
"base_url": "https://other.example.com/v1",
"models": {
"glm-5.1": {"context_length": 128000},
},
},
]
}
result = get_model_context_length("glm-5.1", provider="zai-customize")
assert result == 200000

@patch("hermes_cli.config.load_config")
def test_single_provider_fallback_when_no_scope(self, mock_load_config):
"""With only one custom provider, allow matching even without provider/base_url."""
mock_load_config.return_value = {
"custom_providers": [
{
"name": "zai-customize",
"models": {
"glm-5.1": {"context_length": 200000},
},
}
]
}
result = get_model_context_length("glm-5.1")
assert result == 200000

@patch("hermes_cli.config.load_config")
def test_ambiguous_multiple_providers_skipped_without_scope(self, mock_load_config):
"""When multiple providers exist and no scope is given, skip to avoid collisions."""
mock_load_config.return_value = {
"custom_providers": [
{
"name": "zai-customize",
"models": {"glm-5.1": {"context_length": 200000}},
},
{
"name": "other-provider",
"models": {"glm-5.1": {"context_length": 128000}},
},
]
}
# Must NOT pick either custom provider value because scope is ambiguous
result = get_model_context_length("glm-5.1")
assert result not in (200000, 128000)

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_ambiguous_multiple_providers_skipped_without_scope currently allows get_model_context_length() to fall through into fetch_model_metadata(), which can perform a real network call to OpenRouter (making this unit test slower/flaky depending on environment). Patch agent.model_metadata.fetch_model_metadata to return {} (or patch requests.get) so the test is fully deterministic and offline.

Copilot uses AI. Check for mistakes.
Loading