mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
fix(tools): make the tool-search context gate provider-aware (#68589)
_resolve_active_context_length() called get_model_context_length() with the model id alone, so provider-enforced windows (e.g. Codex OAuth's 272K for gpt-5.x vs the direct API's 1.05M) never reached the tool-search activation gate — it sized against generic metadata for the same slug. Resolve the runtime provider for the configured model and pass provider, base_url, and api_key through. If credential resolution fails (offline, no keys), degrade to a provider+base_url-only lookup so the static provider-aware fallbacks still apply; explicit model.context_length keeps short-circuiting as before (#46620). Gap flagged during review of #16735.
This commit is contained in:
parent
b49b1e5b93
commit
03841c9658
2 changed files with 150 additions and 1 deletions
|
|
@ -589,7 +589,37 @@ def _resolve_active_context_length() -> int:
|
|||
# CLI startup. See issue #46620.
|
||||
raw_ctx = model_cfg.get("context_length")
|
||||
config_ctx = raw_ctx if isinstance(raw_ctx, int) and raw_ctx > 0 else None
|
||||
return int(get_model_context_length(model_id, config_context_length=config_ctx) or 0)
|
||||
# Provider-aware resolution: providers like Codex OAuth enforce a
|
||||
# different (lower) window than the direct API for the same slug, and
|
||||
# their resolvers key off provider/base_url/api_key. Without these,
|
||||
# the gate sizes against generic metadata (e.g. 1.05M for gpt-5.5
|
||||
# instead of Codex's enforced 272K). Credential resolution failing
|
||||
# (offline, no keys) degrades to a provider+base_url-only lookup so
|
||||
# the static provider-aware fallbacks still apply.
|
||||
provider = str(model_cfg.get("provider") or "").strip()
|
||||
base_url = str(model_cfg.get("base_url") or "").strip()
|
||||
api_key = ""
|
||||
if provider:
|
||||
try:
|
||||
from hermes_cli.runtime_provider import resolve_runtime_provider
|
||||
rt = resolve_runtime_provider(
|
||||
requested=provider, target_model=model_id
|
||||
) or {}
|
||||
base_url = str(rt.get("base_url") or base_url or "").strip()
|
||||
api_key = str(rt.get("api_key") or "").strip()
|
||||
except Exception as rt_exc:
|
||||
logger.debug(
|
||||
"Runtime credential resolution failed for tool-search "
|
||||
"context gate (provider=%s): %s — using config values only",
|
||||
provider, rt_exc,
|
||||
)
|
||||
return int(get_model_context_length(
|
||||
model_id,
|
||||
base_url=base_url,
|
||||
api_key=api_key,
|
||||
config_context_length=config_ctx,
|
||||
provider=provider,
|
||||
) or 0)
|
||||
except Exception as e:
|
||||
logger.debug("Could not resolve active context length: %s", e)
|
||||
return 0
|
||||
|
|
|
|||
119
tests/tools/test_tool_search_context_provider.py
Normal file
119
tests/tools/test_tool_search_context_provider.py
Normal file
|
|
@ -0,0 +1,119 @@
|
|||
"""Regression coverage for provider-aware context sizing in the tool-search gate.
|
||||
|
||||
``model_tools._resolve_active_context_length()`` feeds ``should_activate``'s
|
||||
window-fraction check. Providers like Codex OAuth enforce a lower context
|
||||
window than the direct API for the same slug (e.g. gpt-5.5 is 1.05M on the
|
||||
API but 272K on the Codex route), and ``get_model_context_length()`` only
|
||||
applies those provider-aware resolutions when it receives the provider,
|
||||
base_url, and credential. Before this coverage existed the gate called the
|
||||
resolver with the model id alone, so Codex sessions sized activation against
|
||||
generic direct-API metadata.
|
||||
"""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
|
||||
def _model_cfg(**overrides):
|
||||
cfg = {
|
||||
"model": "gpt-5.6-sol",
|
||||
"provider": "openai-codex",
|
||||
"base_url": "",
|
||||
}
|
||||
cfg.update(overrides)
|
||||
return {"model": cfg}
|
||||
|
||||
|
||||
class TestResolveActiveContextLengthProviderAware:
|
||||
def test_passes_provider_base_url_and_key_from_runtime(self):
|
||||
"""Resolved runtime credentials must reach get_model_context_length."""
|
||||
import model_tools
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_get_ctx(model_id, base_url="", api_key="", config_context_length=None, provider=""):
|
||||
captured.update(
|
||||
model=model_id, base_url=base_url, api_key=api_key,
|
||||
config_ctx=config_context_length, provider=provider,
|
||||
)
|
||||
return 272_000
|
||||
|
||||
with patch("hermes_cli.config.load_config", return_value=_model_cfg()), \
|
||||
patch("hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
return_value={"base_url": "https://chatgpt.com/backend-api/codex",
|
||||
"api_key": "tok-live"}) as mock_rt, \
|
||||
patch("agent.model_metadata.get_model_context_length", side_effect=fake_get_ctx):
|
||||
ctx = model_tools._resolve_active_context_length()
|
||||
|
||||
assert ctx == 272_000
|
||||
assert captured["provider"] == "openai-codex"
|
||||
assert captured["base_url"] == "https://chatgpt.com/backend-api/codex"
|
||||
assert captured["api_key"] == "tok-live"
|
||||
mock_rt.assert_called_once_with(
|
||||
requested="openai-codex", target_model="gpt-5.6-sol"
|
||||
)
|
||||
|
||||
def test_offline_credential_failure_degrades_to_config_values(self):
|
||||
"""Runtime resolution raising must not zero the gate — the resolver is
|
||||
still called with the configured provider/base_url and an empty key so
|
||||
static provider-aware fallbacks apply."""
|
||||
import model_tools
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_get_ctx(model_id, base_url="", api_key="", config_context_length=None, provider=""):
|
||||
captured.update(base_url=base_url, api_key=api_key, provider=provider)
|
||||
return 272_000
|
||||
|
||||
with patch("hermes_cli.config.load_config",
|
||||
return_value=_model_cfg(base_url="https://chatgpt.com/backend-api/codex")), \
|
||||
patch("hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
side_effect=RuntimeError("no credentials")), \
|
||||
patch("agent.model_metadata.get_model_context_length", side_effect=fake_get_ctx):
|
||||
ctx = model_tools._resolve_active_context_length()
|
||||
|
||||
assert ctx == 272_000
|
||||
assert captured["provider"] == "openai-codex"
|
||||
assert captured["base_url"] == "https://chatgpt.com/backend-api/codex"
|
||||
assert captured["api_key"] == ""
|
||||
|
||||
def test_no_provider_configured_skips_runtime_resolution(self):
|
||||
"""Without a provider in config, behavior matches the legacy path: no
|
||||
runtime resolution attempt, resolver called with empty routing."""
|
||||
import model_tools
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_get_ctx(model_id, base_url="", api_key="", config_context_length=None, provider=""):
|
||||
captured.update(base_url=base_url, provider=provider)
|
||||
return 200_000
|
||||
|
||||
with patch("hermes_cli.config.load_config",
|
||||
return_value={"model": {"model": "some-model"}}), \
|
||||
patch("hermes_cli.runtime_provider.resolve_runtime_provider") as mock_rt, \
|
||||
patch("agent.model_metadata.get_model_context_length", side_effect=fake_get_ctx):
|
||||
ctx = model_tools._resolve_active_context_length()
|
||||
|
||||
assert ctx == 200_000
|
||||
assert captured["provider"] == ""
|
||||
mock_rt.assert_not_called()
|
||||
|
||||
def test_config_context_length_still_short_circuits(self):
|
||||
"""Explicit model.context_length must keep winning (issue #46620)."""
|
||||
import model_tools
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_get_ctx(model_id, base_url="", api_key="", config_context_length=None, provider=""):
|
||||
captured["config_ctx"] = config_context_length
|
||||
return config_context_length or 0
|
||||
|
||||
with patch("hermes_cli.config.load_config",
|
||||
return_value=_model_cfg(context_length=150_000)), \
|
||||
patch("hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
return_value={"base_url": "https://chatgpt.com/backend-api/codex",
|
||||
"api_key": "tok"}), \
|
||||
patch("agent.model_metadata.get_model_context_length", side_effect=fake_get_ctx):
|
||||
ctx = model_tools._resolve_active_context_length()
|
||||
|
||||
assert ctx == 150_000
|
||||
assert captured["config_ctx"] == 150_000
|
||||
Loading…
Add table
Add a link
Reference in a new issue