mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(model): narrow custom-provider fallback exclusion to real custom: syntax
Per hermes-sweeper review on #56671: the fallback exclusion matched any
canonical string starting with "custom" (e.g. "customproxy"), not just
the durable named-custom-provider syntax ("custom" bucket or
"custom:<name>" slugs). Narrow it to an exact/prefix match on that
syntax so unrelated vendor names aren't accidentally exempted from the
openrouter fallback.
Also clarifies the test suite: a properly configured custom:<name>
provider now resolves via resolve_custom_provider before this fallback
is ever reached (added upstream in 9a15fad0d6), so the existing test
was mislabeled as exercising that primary path when it was actually
exercising the fallback-safety-net case (missing/unresolved config
entry). Split into explicit primary-path and fallback-safety-net tests,
plus a regression test for the "customproxy"-style false positive.
This commit is contained in:
parent
a5ea9a6fd4
commit
1cd5f52b3e
2 changed files with 76 additions and 12 deletions
|
|
@ -1518,10 +1518,15 @@ def _normalize_main_model_assignment(provider: str, model: str) -> tuple[str, st
|
|||
# mismatch, entry missing from custom_providers/providers) must still
|
||||
# not be treated as a stray vendor prefix -- it isn't a known Hermes
|
||||
# provider/alias, but it also isn't the analytics-vendor case this
|
||||
# fallback exists for.
|
||||
# fallback exists for. Match only the durable named-custom syntax
|
||||
# (bare "custom" bucket, or "custom:<name>" per
|
||||
# ``providers.custom_provider_slug``) -- a bare ``startswith("custom")``
|
||||
# would also swallow unrelated unconfigured vendor names that merely
|
||||
# happen to start with "custom" (e.g. "customproxy").
|
||||
is_custom_provider_slug = canonical == "custom" or canonical.startswith("custom:")
|
||||
if (
|
||||
canonical not in _KNOWN_PROVIDER_NAMES
|
||||
and not canonical.startswith("custom")
|
||||
and not is_custom_provider_slug
|
||||
and "/" in model_in
|
||||
):
|
||||
# Vendor prefix posing as a provider (analytics fallback). Resolve
|
||||
|
|
|
|||
|
|
@ -9,23 +9,82 @@ a slash-bearing model id (``ollama/glm-5.2``) was indistinguishable from the
|
|||
"vendor prefix posing as a provider" analytics-fallback case, and got silently
|
||||
rewritten to ``provider: openrouter`` in ``config.yaml`` -- reassigning the
|
||||
provider entirely, not just mangling the model id.
|
||||
|
||||
A properly *configured* ``custom:<name>`` provider is now resolved earlier,
|
||||
via ``resolve_custom_provider`` against ``custom_providers``/``providers`` in
|
||||
config -- that's the primary, intended path and isn't this module's concern
|
||||
to re-test. What's tested here is the fallback safety net for when that
|
||||
resolution comes up empty (typo, config drift, entry removed) -- a
|
||||
``custom:<name>`` slug must still not be misread as a stray analytics vendor
|
||||
prefix and reassigned to openrouter, even though it isn't in
|
||||
``_KNOWN_PROVIDER_NAMES`` either.
|
||||
"""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from hermes_cli.web_server import _normalize_main_model_assignment
|
||||
|
||||
|
||||
class TestNamedCustomProviderIsNotTreatedAsStrayVendorPrefix:
|
||||
def test_named_custom_provider_slug_is_preserved(self):
|
||||
assert _normalize_main_model_assignment("custom:litellm", "ollama/glm-5.2") == (
|
||||
"custom:litellm",
|
||||
"ollama/glm-5.2",
|
||||
)
|
||||
def _no_custom_providers_configured():
|
||||
"""Patch load_config so resolve_user_provider/resolve_custom_provider
|
||||
both come up empty, forcing execution into the fallback path under
|
||||
test -- independent of whatever config.yaml happens to be on disk."""
|
||||
return patch("hermes_cli.web_server.load_config", return_value={})
|
||||
|
||||
|
||||
class TestUnresolvedNamedCustomProviderIsNotTreatedAsStrayVendorPrefix:
|
||||
"""Covers the case where ``resolve_custom_provider`` finds no match --
|
||||
e.g. ``custom:litellm`` was configured once, then the entry was renamed
|
||||
or dropped from ``custom_providers``, but old sessions/config still
|
||||
reference the old slug.
|
||||
"""
|
||||
|
||||
def test_unresolved_named_custom_provider_slug_is_preserved(self):
|
||||
with _no_custom_providers_configured():
|
||||
assert _normalize_main_model_assignment("custom:litellm", "ollama/glm-5.2") == (
|
||||
"custom:litellm",
|
||||
"ollama/glm-5.2",
|
||||
)
|
||||
|
||||
def test_bare_custom_bucket_is_preserved(self):
|
||||
assert _normalize_main_model_assignment("custom", "ollama/glm-5.2") == (
|
||||
"custom",
|
||||
"ollama/glm-5.2",
|
||||
)
|
||||
with _no_custom_providers_configured():
|
||||
assert _normalize_main_model_assignment("custom", "ollama/glm-5.2") == (
|
||||
"custom",
|
||||
"ollama/glm-5.2",
|
||||
)
|
||||
|
||||
def test_unconfigured_non_custom_vendor_name_still_falls_back(self):
|
||||
"""A name that merely starts with the substring "custom" but isn't
|
||||
the durable ``custom:<name>`` syntax (no colon) is NOT exempted --
|
||||
it's just another unknown vendor label and should still hit the
|
||||
openrouter fallback like any other unrecognized provider string.
|
||||
"""
|
||||
with _no_custom_providers_configured():
|
||||
assert _normalize_main_model_assignment(
|
||||
"customproxy", "anthropic/claude-opus-4.6"
|
||||
) == ("openrouter", "anthropic/claude-opus-4.6")
|
||||
|
||||
|
||||
class TestConfiguredNamedCustomProviderResolvesViaPrimaryPath:
|
||||
"""The primary, intended path: a ``custom:<name>`` slug that IS present
|
||||
in ``custom_providers`` resolves through ``resolve_custom_provider``
|
||||
before the fallback under test above is ever reached.
|
||||
"""
|
||||
|
||||
def test_configured_named_custom_provider_resolves(self):
|
||||
cfg = {
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "litellm",
|
||||
"base_url": "http://localhost:4000/v1",
|
||||
"key_env": "LITELLM_API_KEY",
|
||||
}
|
||||
]
|
||||
}
|
||||
with patch("hermes_cli.web_server.load_config", return_value=cfg):
|
||||
assert _normalize_main_model_assignment(
|
||||
"custom:litellm", "ollama/glm-5.2"
|
||||
) == ("custom:litellm", "ollama/glm-5.2")
|
||||
|
||||
|
||||
class TestStrayVendorPrefixFallbackStillWorks:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue