mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-18 14:52:04 +00:00
fix(params): send max_completion_tokens for newer OpenAI families on custom endpoints
Third-party OpenAI-compatible endpoints (self-hosted gateways, OpenRouter,
Azure proxies) fronting gpt-4o / gpt-4.1 / gpt-5+ / o1-o4 models silently
received max_tokens and 400'd with unsupported_parameter, because the three
kwarg-selection sites only checked base_url_hostname(...) == "api.openai.com"
and fell through to max_tokens on every other host. The constraint is
enforced server-side by the model family, not by the URL, so name-based
detection is required as a fallback.
Changes:
- utils.py: new shared helper model_forces_max_completion_tokens(model) that
prefix-matches gpt-4o, gpt-4.1, gpt-5, o1, o3, o4 families on normalized
(lowercased, vendor-prefix-stripped) names.
- run_agent.py: _max_tokens_param ORs the helper into the URL check.
- agent/auxiliary_client.py:
- auxiliary_max_tokens_param gains an optional keyword-only model arg.
- _build_call_kwargs inline branch applies the same check for both
provider == "custom" and non-custom paths.
Tests:
- tests/test_model_forces_max_completion_tokens.py: 31 new cases covering
positive families, negatives (classic gpt-4, claude, llama, mistral, qwen,
deepseek), vendor prefixes, case-insensitivity, whitespace, None/empty,
and substring-not-prefix guards.
- tests/run_agent/test_run_agent.py::TestMaxTokensParam: 5 new model-based
cases (custom + gpt-5.4, openrouter + gpt-4o-mini, custom + o1-preview,
classic gpt-4-turbo keeps max_tokens, llama3 keeps max_tokens).
- tests/agent/test_auxiliary_client.py::TestAuxiliaryMaxTokensParam: new
class, 7 tests covering the URL x model matrix.
This commit is contained in:
parent
ab55008631
commit
19c07c4037
6 changed files with 316 additions and 11 deletions
|
|
@ -102,7 +102,7 @@ OpenAI = _OpenAIProxy() # module-level name, resolves lazily on call/isinstance
|
|||
from agent.credential_pool import load_pool
|
||||
from hermes_cli.config import get_hermes_home
|
||||
from hermes_constants import OPENROUTER_BASE_URL
|
||||
from utils import base_url_host_matches, base_url_hostname, normalize_proxy_env_vars
|
||||
from utils import base_url_host_matches, base_url_hostname, model_forces_max_completion_tokens, normalize_proxy_env_vars
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -4300,13 +4300,15 @@ def get_auxiliary_extra_body() -> dict:
|
|||
return _nous_extra_body() if auxiliary_is_nous else {}
|
||||
|
||||
|
||||
def auxiliary_max_tokens_param(value: int) -> dict:
|
||||
def auxiliary_max_tokens_param(value: int, *, model: Optional[str] = None) -> dict:
|
||||
"""Return the correct max tokens kwarg for the auxiliary client's provider.
|
||||
|
||||
|
||||
OpenRouter and local models use 'max_tokens'. Direct OpenAI with newer
|
||||
models (gpt-4o, o-series, gpt-5+) requires 'max_completion_tokens'.
|
||||
models (gpt-4o, gpt-4.1, gpt-5+, o-series) requires 'max_completion_tokens'.
|
||||
The Codex adapter translates max_tokens internally, so we use max_tokens
|
||||
for it as well.
|
||||
for it as well. Pass ``model`` so third-party OpenAI-compatible endpoints
|
||||
fronting the newer families are also recognised — URL-only detection
|
||||
misses the case where a custom base URL serves e.g. ``gpt-5.4``.
|
||||
"""
|
||||
custom_base = _current_custom_base_url()
|
||||
or_key = os.getenv("OPENROUTER_API_KEY")
|
||||
|
|
@ -4316,6 +4318,9 @@ def auxiliary_max_tokens_param(value: int) -> dict:
|
|||
and _read_nous_auth() is None
|
||||
and base_url_hostname(custom_base) in {"api.openai.com", "api.githubcopilot.com"}):
|
||||
return {"max_completion_tokens": value}
|
||||
# ...and for any caller serving a newer OpenAI-family model by name.
|
||||
if model_forces_max_completion_tokens(model):
|
||||
return {"max_completion_tokens": value}
|
||||
return {"max_tokens": value}
|
||||
|
||||
|
||||
|
|
|
|||
23
run_agent.py
23
run_agent.py
|
|
@ -196,7 +196,7 @@ from agent.tool_dispatch_helpers import (
|
|||
_extract_error_preview,
|
||||
_trajectory_normalize_msg, # noqa: F401 # re-exported for tests that `from run_agent import _trajectory_normalize_msg`
|
||||
)
|
||||
from utils import atomic_json_write, base_url_host_matches, base_url_hostname, is_truthy_value
|
||||
from utils import atomic_json_write, base_url_host_matches, base_url_hostname, is_truthy_value, model_forces_max_completion_tokens
|
||||
|
||||
|
||||
|
||||
|
|
@ -1253,13 +1253,24 @@ class AIAgent:
|
|||
def _max_tokens_param(self, value: int) -> dict:
|
||||
"""Return the correct max tokens kwarg for the current provider.
|
||||
|
||||
OpenAI's newer models (gpt-4o, o-series, gpt-5+) require
|
||||
'max_completion_tokens'. Azure OpenAI also requires
|
||||
'max_completion_tokens' for gpt-5.x models served via the
|
||||
OpenAI-compatible endpoint. OpenRouter, local models, and older
|
||||
OpenAI's newer models (gpt-4o, gpt-4.1, gpt-5+, o-series) require
|
||||
'max_completion_tokens'. Azure OpenAI and GitHub Copilot also require
|
||||
'max_completion_tokens' for those families served via their
|
||||
OpenAI-compatible endpoints. OpenRouter, local models, and older
|
||||
OpenAI models use 'max_tokens'.
|
||||
|
||||
The check is URL-first (api.openai.com / Azure / Copilot all use the
|
||||
new kwarg), then falls back to a model-name check so third-party
|
||||
OpenAI-compatible endpoints fronting those models are recognised —
|
||||
URL-only detection misses that case and silently sends the wrong
|
||||
kwarg, which the upstream model rejects with a 400.
|
||||
"""
|
||||
if self._is_direct_openai_url() or self._is_azure_openai_url() or self._is_github_copilot_url():
|
||||
if (
|
||||
self._is_direct_openai_url()
|
||||
or self._is_azure_openai_url()
|
||||
or self._is_github_copilot_url()
|
||||
or model_forces_max_completion_tokens(self.model)
|
||||
):
|
||||
return {"max_completion_tokens": value}
|
||||
return {"max_tokens": value}
|
||||
|
||||
|
|
|
|||
|
|
@ -3791,3 +3791,82 @@ class TestAuxUnhealthyCache:
|
|||
)
|
||||
# After the 402, OpenRouter is in the unhealthy cache.
|
||||
assert _is_provider_unhealthy("openrouter") is True
|
||||
|
||||
|
||||
# ── auxiliary_max_tokens_param ──────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestAuxiliaryMaxTokensParam:
|
||||
"""Verify the kwarg emitted by ``auxiliary_max_tokens_param`` across
|
||||
URL / provider / model-name combinations. Regression cover: a custom
|
||||
OpenAI-compatible endpoint serving ``gpt-5.x`` was silently getting
|
||||
``max_tokens`` and 400-ing on ``unsupported_parameter``."""
|
||||
|
||||
def test_direct_openai_returns_max_completion_tokens(self):
|
||||
with (
|
||||
patch("agent.auxiliary_client._current_custom_base_url",
|
||||
return_value="https://api.openai.com/v1"),
|
||||
patch("agent.auxiliary_client._read_nous_auth", return_value=None),
|
||||
):
|
||||
assert auxiliary_max_tokens_param(4096) == {"max_completion_tokens": 4096}
|
||||
|
||||
def test_local_endpoint_without_model_uses_max_tokens(self):
|
||||
with (
|
||||
patch("agent.auxiliary_client._current_custom_base_url",
|
||||
return_value="http://localhost:11434/v1"),
|
||||
patch("agent.auxiliary_client._read_nous_auth", return_value=None),
|
||||
):
|
||||
assert auxiliary_max_tokens_param(4096) == {"max_tokens": 4096}
|
||||
|
||||
def test_openrouter_api_key_present_keeps_max_tokens_without_model_hint(self, monkeypatch):
|
||||
monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-v1-test")
|
||||
with (
|
||||
patch("agent.auxiliary_client._current_custom_base_url",
|
||||
return_value="https://openrouter.ai/api/v1"),
|
||||
patch("agent.auxiliary_client._read_nous_auth", return_value=None),
|
||||
):
|
||||
assert auxiliary_max_tokens_param(4096) == {"max_tokens": 4096}
|
||||
|
||||
# Model-name fallback — this is the regression guard.
|
||||
|
||||
def test_custom_endpoint_serving_gpt5_uses_max_completion_tokens(self):
|
||||
"""Third-party gateway + gpt-5.x: name-based detection must kick in."""
|
||||
with (
|
||||
patch("agent.auxiliary_client._current_custom_base_url",
|
||||
return_value="https://my-gateway.example.com/v1"),
|
||||
patch("agent.auxiliary_client._read_nous_auth", return_value=None),
|
||||
):
|
||||
assert auxiliary_max_tokens_param(4096, model="gpt-5.4") == {
|
||||
"max_completion_tokens": 4096
|
||||
}
|
||||
|
||||
def test_openrouter_serving_gpt4o_uses_max_completion_tokens(self, monkeypatch):
|
||||
monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-v1-test")
|
||||
with (
|
||||
patch("agent.auxiliary_client._current_custom_base_url",
|
||||
return_value="https://openrouter.ai/api/v1"),
|
||||
patch("agent.auxiliary_client._read_nous_auth", return_value=None),
|
||||
):
|
||||
assert auxiliary_max_tokens_param(4096, model="openai/gpt-4o-mini") == {
|
||||
"max_completion_tokens": 4096
|
||||
}
|
||||
|
||||
def test_custom_endpoint_serving_classic_llama_keeps_max_tokens(self):
|
||||
with (
|
||||
patch("agent.auxiliary_client._current_custom_base_url",
|
||||
return_value="https://my-gateway.example.com/v1"),
|
||||
patch("agent.auxiliary_client._read_nous_auth", return_value=None),
|
||||
):
|
||||
assert auxiliary_max_tokens_param(4096, model="llama3-70b") == {
|
||||
"max_tokens": 4096
|
||||
}
|
||||
|
||||
def test_empty_model_falls_back_to_url_only(self):
|
||||
"""No model hint → only the URL-based rule applies."""
|
||||
with (
|
||||
patch("agent.auxiliary_client._current_custom_base_url",
|
||||
return_value="https://my-gateway.example.com/v1"),
|
||||
patch("agent.auxiliary_client._read_nous_auth", return_value=None),
|
||||
):
|
||||
assert auxiliary_max_tokens_param(4096, model="") == {"max_tokens": 4096}
|
||||
assert auxiliary_max_tokens_param(4096, model=None) == {"max_tokens": 4096}
|
||||
|
|
|
|||
|
|
@ -5063,6 +5063,41 @@ class TestMaxTokensParam:
|
|||
result = agent._max_tokens_param(4096)
|
||||
assert result == {"max_completion_tokens": 4096}
|
||||
|
||||
# ── Model-name fallback for non-openai.com endpoints serving newer families ──
|
||||
|
||||
def test_returns_max_completion_tokens_for_gpt5_on_custom_endpoint(self, agent):
|
||||
"""Custom OpenAI-compatible endpoint serving gpt-5.x must also use
|
||||
max_completion_tokens — otherwise the server 400s on max_tokens."""
|
||||
agent.base_url = "https://my-gateway.example.com/v1"
|
||||
agent.model = "gpt-5.4"
|
||||
result = agent._max_tokens_param(4096)
|
||||
assert result == {"max_completion_tokens": 4096}
|
||||
|
||||
def test_returns_max_completion_tokens_for_gpt4o_on_openrouter(self, agent):
|
||||
agent.base_url = "https://openrouter.ai/api/v1"
|
||||
agent.model = "openai/gpt-4o-mini"
|
||||
result = agent._max_tokens_param(4096)
|
||||
assert result == {"max_completion_tokens": 4096}
|
||||
|
||||
def test_returns_max_completion_tokens_for_o1_on_custom_endpoint(self, agent):
|
||||
agent.base_url = "https://custom.example.com/v1"
|
||||
agent.model = "o1-preview"
|
||||
result = agent._max_tokens_param(4096)
|
||||
assert result == {"max_completion_tokens": 4096}
|
||||
|
||||
def test_returns_max_tokens_for_classic_gpt4_on_openrouter(self, agent):
|
||||
"""Classic gpt-4 (non-omni) still uses max_tokens. Don't over-match."""
|
||||
agent.base_url = "https://openrouter.ai/api/v1"
|
||||
agent.model = "openai/gpt-4-turbo"
|
||||
result = agent._max_tokens_param(4096)
|
||||
assert result == {"max_tokens": 4096}
|
||||
|
||||
def test_returns_max_tokens_for_llama_on_local(self, agent):
|
||||
agent.base_url = "http://localhost:11434/v1"
|
||||
agent.model = "llama3"
|
||||
result = agent._max_tokens_param(4096)
|
||||
assert result == {"max_tokens": 4096}
|
||||
|
||||
|
||||
class TestGpt5ApiModeRouting:
|
||||
"""Verify provider-specific GPT-5 API-mode routing."""
|
||||
|
|
|
|||
137
tests/test_model_forces_max_completion_tokens.py
Normal file
137
tests/test_model_forces_max_completion_tokens.py
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
"""Targeted tests for ``utils.model_forces_max_completion_tokens``.
|
||||
|
||||
This helper decides whether a given model name requires the newer
|
||||
``max_completion_tokens`` kwarg (rather than the legacy ``max_tokens``) on
|
||||
``/v1/chat/completions``. It protects against the 400 ``unsupported_parameter``
|
||||
error seen when third-party OpenAI-compatible endpoints serve gpt-4o / 4.1 /
|
||||
5.x / o-series models by name and the caller only checks the URL host.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from utils import model_forces_max_completion_tokens
|
||||
|
||||
|
||||
# ─── Positive cases: families that require max_completion_tokens ────────────
|
||||
|
||||
|
||||
class TestPositiveCases:
|
||||
def test_gpt_5_bare(self):
|
||||
assert model_forces_max_completion_tokens("gpt-5") is True
|
||||
|
||||
def test_gpt_5_point_release(self):
|
||||
# The case the user actually hit — gpt-5.4 on a custom OpenAI-compatible
|
||||
# endpoint was being sent max_tokens and getting 400 back.
|
||||
assert model_forces_max_completion_tokens("gpt-5.4") is True
|
||||
|
||||
def test_gpt_5_mini(self):
|
||||
assert model_forces_max_completion_tokens("gpt-5-mini") is True
|
||||
|
||||
def test_gpt_5_nano(self):
|
||||
assert model_forces_max_completion_tokens("gpt-5-nano") is True
|
||||
|
||||
def test_gpt_4o(self):
|
||||
assert model_forces_max_completion_tokens("gpt-4o") is True
|
||||
|
||||
def test_gpt_4o_mini(self):
|
||||
assert model_forces_max_completion_tokens("gpt-4o-mini") is True
|
||||
|
||||
def test_gpt_4_1(self):
|
||||
assert model_forces_max_completion_tokens("gpt-4.1") is True
|
||||
|
||||
def test_gpt_4_1_mini(self):
|
||||
assert model_forces_max_completion_tokens("gpt-4.1-mini") is True
|
||||
|
||||
def test_o1(self):
|
||||
assert model_forces_max_completion_tokens("o1") is True
|
||||
|
||||
def test_o1_preview(self):
|
||||
assert model_forces_max_completion_tokens("o1-preview") is True
|
||||
|
||||
def test_o1_mini(self):
|
||||
assert model_forces_max_completion_tokens("o1-mini") is True
|
||||
|
||||
def test_o3(self):
|
||||
assert model_forces_max_completion_tokens("o3") is True
|
||||
|
||||
def test_o3_mini(self):
|
||||
assert model_forces_max_completion_tokens("o3-mini") is True
|
||||
|
||||
def test_o4_mini(self):
|
||||
# Future-proofing — o4 is already listed publicly.
|
||||
assert model_forces_max_completion_tokens("o4-mini") is True
|
||||
|
||||
|
||||
# ─── Negative cases: older or non-OpenAI families still use max_tokens ──────
|
||||
|
||||
|
||||
class TestNegativeCases:
|
||||
def test_gpt_3_5_turbo(self):
|
||||
assert model_forces_max_completion_tokens("gpt-3.5-turbo") is False
|
||||
|
||||
def test_gpt_4(self):
|
||||
# Classic gpt-4 (non-omni) still uses max_tokens on chat completions.
|
||||
assert model_forces_max_completion_tokens("gpt-4") is False
|
||||
|
||||
def test_gpt_4_turbo(self):
|
||||
assert model_forces_max_completion_tokens("gpt-4-turbo") is False
|
||||
|
||||
def test_claude_family(self):
|
||||
assert model_forces_max_completion_tokens("claude-3-opus") is False
|
||||
assert model_forces_max_completion_tokens("claude-sonnet-4-6") is False
|
||||
|
||||
def test_llama_family(self):
|
||||
assert model_forces_max_completion_tokens("llama3") is False
|
||||
assert model_forces_max_completion_tokens("llama-3-70b-instruct") is False
|
||||
|
||||
def test_mistral_family(self):
|
||||
assert model_forces_max_completion_tokens("mistral-7b-instruct") is False
|
||||
|
||||
def test_qwen_family(self):
|
||||
assert model_forces_max_completion_tokens("qwen2.5-72b") is False
|
||||
|
||||
def test_deepseek_family(self):
|
||||
assert model_forces_max_completion_tokens("deepseek-chat") is False
|
||||
|
||||
|
||||
# ─── Edge cases ─────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestEdgeCases:
|
||||
def test_empty_string(self):
|
||||
assert model_forces_max_completion_tokens("") is False
|
||||
|
||||
def test_none(self):
|
||||
assert model_forces_max_completion_tokens(None) is False # type: ignore[arg-type]
|
||||
|
||||
def test_whitespace_only(self):
|
||||
assert model_forces_max_completion_tokens(" ") is False
|
||||
|
||||
def test_case_insensitive(self):
|
||||
assert model_forces_max_completion_tokens("GPT-5.4") is True
|
||||
assert model_forces_max_completion_tokens("Gpt-4o-Mini") is True
|
||||
assert model_forces_max_completion_tokens("O3-MINI") is True
|
||||
|
||||
def test_leading_trailing_whitespace(self):
|
||||
assert model_forces_max_completion_tokens(" gpt-5 ") is True
|
||||
|
||||
def test_vendor_prefix_stripped(self):
|
||||
# OpenRouter-style "vendor/model" names should match the tail.
|
||||
assert model_forces_max_completion_tokens("openai/gpt-5.4") is True
|
||||
assert model_forces_max_completion_tokens("openai/gpt-4o-mini") is True
|
||||
assert model_forces_max_completion_tokens("openai/o3-mini") is True
|
||||
|
||||
def test_vendor_prefix_with_non_matching_tail(self):
|
||||
assert model_forces_max_completion_tokens("openai/gpt-3.5-turbo") is False
|
||||
assert model_forces_max_completion_tokens("anthropic/claude-3-opus") is False
|
||||
|
||||
def test_fake_prefix_not_matched(self):
|
||||
# "o-series-but-not-really" doesn't start with o1/o3/o4.
|
||||
assert model_forces_max_completion_tokens("omni-chat") is False
|
||||
# "ox" isn't an o-series model, and "olive" / "opus" shouldn't collide.
|
||||
assert model_forces_max_completion_tokens("ox-large") is False
|
||||
assert model_forces_max_completion_tokens("opus-3") is False
|
||||
|
||||
def test_gpt_5_substring_in_middle_not_matched(self):
|
||||
# Only a prefix should match — "local-gpt-5-clone" is a different model.
|
||||
assert model_forces_max_completion_tokens("local-gpt-5-clone") is False
|
||||
38
utils.py
38
utils.py
|
|
@ -355,6 +355,44 @@ def base_url_hostname(base_url: str) -> str:
|
|||
return (parsed.hostname or "").lower().rstrip(".")
|
||||
|
||||
|
||||
# ─── Model Capability Detection ──────────────────────────────────────────────
|
||||
|
||||
|
||||
def model_forces_max_completion_tokens(model: str) -> bool:
|
||||
"""Return True for model families that require ``max_completion_tokens``.
|
||||
|
||||
OpenAI's newer families reject ``max_tokens`` on /v1/chat/completions with
|
||||
HTTP 400 ``unsupported_parameter`` — the caller must send
|
||||
``max_completion_tokens`` instead. This covers:
|
||||
|
||||
- ``gpt-4o`` / ``gpt-4o-mini`` / ``gpt-4o-*``
|
||||
- ``gpt-4.1`` / ``gpt-4.1-*``
|
||||
- ``gpt-5`` / ``gpt-5.x`` / ``gpt-5-*``
|
||||
- ``o1`` / ``o1-*``
|
||||
- ``o3`` / ``o3-*``
|
||||
- ``o4`` / ``o4-*``
|
||||
|
||||
Handles vendor prefixes like ``openai/gpt-5.4`` by stripping to the tail.
|
||||
The URL-based check (``base_url_hostname == "api.openai.com"``) misses
|
||||
third-party OpenAI-compatible endpoints (custom OpenAI gateways,
|
||||
OpenRouter) that front these models and enforce the same parameter
|
||||
constraint, so name-based detection is required as a fallback.
|
||||
"""
|
||||
m = (model or "").strip().lower()
|
||||
if not m:
|
||||
return False
|
||||
if "/" in m:
|
||||
m = m.rsplit("/", 1)[-1]
|
||||
return (
|
||||
m.startswith("gpt-4o")
|
||||
or m.startswith("gpt-4.1")
|
||||
or m.startswith("gpt-5")
|
||||
or m.startswith("o1")
|
||||
or m.startswith("o3")
|
||||
or m.startswith("o4")
|
||||
)
|
||||
|
||||
|
||||
def base_url_host_matches(base_url: str, domain: str) -> bool:
|
||||
"""Return True when the base URL's hostname is ``domain`` or a subdomain.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue