mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
fix(moa): stop reference_max_tokens from also capping the aggregator
aggregate_moa_context's single max_tokens parameter was applied to both the reference fan-out (_run_references_parallel) and the aggregator's own synthesis call_llm. #53580 explicitly removed a hardcoded cap from the aggregator call because it truncated long aggregator syntheses; #56756 (reference_max_tokens, added to speed up the advisor fan-out) reintroduced the same shared cap by passing it to both calls, silently regressing #53580's fix. Rename the parameter to reference_max_tokens (matching the caller's own moa_config key) and stop forwarding it to the aggregator's call_llm invocation, which now always runs uncapped as intended.
This commit is contained in:
parent
5be99b6fce
commit
cc1725cbe5
3 changed files with 113 additions and 12 deletions
|
|
@ -1099,7 +1099,7 @@ def run_conversation(
|
|||
aggregator=moa_config.get("aggregator") or {},
|
||||
temperature=_preset_temperature(moa_config, "reference_temperature"),
|
||||
aggregator_temperature=_preset_temperature(moa_config, "aggregator_temperature"),
|
||||
max_tokens=moa_config.get("reference_max_tokens"),
|
||||
reference_max_tokens=moa_config.get("reference_max_tokens"),
|
||||
)
|
||||
if _moa_context:
|
||||
for _msg in reversed(api_messages):
|
||||
|
|
|
|||
|
|
@ -687,23 +687,26 @@ def aggregate_moa_context(
|
|||
aggregator: dict[str, str],
|
||||
temperature: float | None = None,
|
||||
aggregator_temperature: float | None = None,
|
||||
max_tokens: int | None = None,
|
||||
reference_max_tokens: int | None = None,
|
||||
) -> str:
|
||||
"""Run configured reference models and synthesize their advice.
|
||||
|
||||
Failures are returned as model-specific notes instead of aborting the normal
|
||||
agent loop; the main model can still act with partial context.
|
||||
|
||||
``max_tokens`` is ``None`` by default: MoA does not cap reference or
|
||||
aggregator output, so each model uses its own maximum. ``call_llm`` omits
|
||||
the parameter entirely when it is ``None`` (see its docstring), which also
|
||||
sidesteps providers that reject ``max_tokens`` outright. A hardcoded cap
|
||||
here previously truncated long aggregator syntheses.
|
||||
``reference_max_tokens`` applies ONLY to the reference fan-out — the
|
||||
aggregator's own synthesis call is never capped, so it always uses its
|
||||
model's own maximum. ``call_llm`` omits the parameter entirely when it
|
||||
is ``None`` (see its docstring), which also sidesteps providers that
|
||||
reject ``max_tokens`` outright. A hardcoded cap on the aggregator call
|
||||
previously truncated long aggregator syntheses (#53580) — passing
|
||||
``reference_max_tokens`` to both calls here would silently reintroduce
|
||||
that regression.
|
||||
|
||||
``temperature`` / ``aggregator_temperature`` are ``None`` by default:
|
||||
like max_tokens, ``call_llm`` omits temperature when None so the
|
||||
provider default applies — matching single-model agent behavior. Presets
|
||||
may still pin explicit values.
|
||||
like ``reference_max_tokens``, ``call_llm`` omits temperature when None
|
||||
so the provider default applies — matching single-model agent behavior.
|
||||
Presets may still pin explicit values.
|
||||
"""
|
||||
reference_outputs: list[tuple[str, str, Any]] = []
|
||||
ref_messages = _reference_messages(api_messages)
|
||||
|
|
@ -711,7 +714,7 @@ def aggregate_moa_context(
|
|||
reference_models,
|
||||
ref_messages,
|
||||
temperature=temperature,
|
||||
max_tokens=max_tokens,
|
||||
max_tokens=reference_max_tokens,
|
||||
)
|
||||
|
||||
joined = "\n\n".join(
|
||||
|
|
@ -748,7 +751,6 @@ def aggregate_moa_context(
|
|||
task="moa_aggregator",
|
||||
messages=agg_messages,
|
||||
temperature=aggregator_temperature,
|
||||
max_tokens=max_tokens,
|
||||
reasoning_config=_aggregator_reasoning_config(aggregator),
|
||||
**agg_runtime,
|
||||
)
|
||||
|
|
|
|||
99
tests/agent/test_moa_context_max_tokens.py
Normal file
99
tests/agent/test_moa_context_max_tokens.py
Normal file
|
|
@ -0,0 +1,99 @@
|
|||
"""Regression test for aggregate_moa_context's reference/aggregator max_tokens split.
|
||||
|
||||
PR #53580 removed a hardcoded ``max_tokens`` cap from the aggregator's
|
||||
synthesis call because it truncated long aggregator syntheses. PR #56756
|
||||
(feat(moa): add reference_max_tokens to cap advisor output and cut turn
|
||||
latency) later reintroduced a single ``max_tokens`` parameter shared by BOTH
|
||||
the reference fan-out and the aggregator call in ``aggregate_moa_context`` —
|
||||
silently regressing the exact bug #53580 fixed: setting
|
||||
``reference_max_tokens`` to speed up the advisors also truncates the
|
||||
aggregator's own synthesis, which is the context the main agent actually
|
||||
uses.
|
||||
|
||||
``aggregate_moa_context`` and its reference/aggregator calls both go through
|
||||
``call_llm`` (task="moa_reference" vs task="moa_aggregator"), so mocking
|
||||
just that one function exercises the real fan-out/aggregation code path.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def _response(content: str = "ok"):
|
||||
message = SimpleNamespace(content=content, tool_calls=[])
|
||||
choice = SimpleNamespace(message=message, finish_reason="stop")
|
||||
return SimpleNamespace(choices=[choice], usage=None, model="fake")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def hermes_home(tmp_path, monkeypatch):
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
return home
|
||||
|
||||
|
||||
def test_aggregator_call_never_receives_reference_max_tokens(hermes_home, monkeypatch):
|
||||
"""reference_max_tokens must cap only the reference fan-out — the
|
||||
aggregator's own call_llm invocation must not receive max_tokens at all
|
||||
(call_llm omits it entirely when None; see its own docstring)."""
|
||||
from agent.moa_loop import aggregate_moa_context
|
||||
|
||||
calls: list[dict] = []
|
||||
|
||||
def fake_call_llm(**kwargs):
|
||||
calls.append(kwargs)
|
||||
return _response("advice" if kwargs.get("task") == "moa_reference" else "synthesis")
|
||||
|
||||
monkeypatch.setattr("agent.moa_loop.call_llm", fake_call_llm)
|
||||
|
||||
aggregate_moa_context(
|
||||
user_prompt="clean the db",
|
||||
api_messages=[{"role": "user", "content": "clean the db"}],
|
||||
reference_models=[{"provider": "openrouter", "model": "openai/gpt-5.5"}],
|
||||
aggregator={"provider": "openrouter", "model": "anthropic/claude-opus-4.8"},
|
||||
reference_max_tokens=600,
|
||||
)
|
||||
|
||||
reference_calls = [c for c in calls if c.get("task") == "moa_reference"]
|
||||
aggregator_calls = [c for c in calls if c.get("task") == "moa_aggregator"]
|
||||
assert len(reference_calls) == 1
|
||||
assert len(aggregator_calls) == 1
|
||||
|
||||
# The reference fan-out is capped as configured.
|
||||
assert reference_calls[0]["max_tokens"] == 600
|
||||
# The aggregator's synthesis call must be uncapped — not even max_tokens=None,
|
||||
# the kwarg must be absent entirely (matches call_llm's omit-when-None contract).
|
||||
assert "max_tokens" not in aggregator_calls[0]
|
||||
|
||||
|
||||
def test_aggregator_call_uncapped_when_reference_max_tokens_unset(hermes_home, monkeypatch):
|
||||
"""Sanity check: with no reference_max_tokens configured, the reference
|
||||
call still explicitly passes max_tokens=None (call_llm itself decides
|
||||
whether to omit it on the wire), while the aggregator call structurally
|
||||
never carries a max_tokens kwarg at all — the pre-#56756 default MoA
|
||||
behavior for the aggregator, preserved regardless of the reference cap."""
|
||||
from agent.moa_loop import aggregate_moa_context
|
||||
|
||||
calls: list[dict] = []
|
||||
|
||||
def fake_call_llm(**kwargs):
|
||||
calls.append(kwargs)
|
||||
return _response("advice" if kwargs.get("task") == "moa_reference" else "synthesis")
|
||||
|
||||
monkeypatch.setattr("agent.moa_loop.call_llm", fake_call_llm)
|
||||
|
||||
aggregate_moa_context(
|
||||
user_prompt="clean the db",
|
||||
api_messages=[{"role": "user", "content": "clean the db"}],
|
||||
reference_models=[{"provider": "openrouter", "model": "openai/gpt-5.5"}],
|
||||
aggregator={"provider": "openrouter", "model": "anthropic/claude-opus-4.8"},
|
||||
)
|
||||
|
||||
reference_calls = [c for c in calls if c.get("task") == "moa_reference"]
|
||||
aggregator_calls = [c for c in calls if c.get("task") == "moa_aggregator"]
|
||||
assert reference_calls[0]["max_tokens"] is None
|
||||
assert "max_tokens" not in aggregator_calls[0]
|
||||
Loading…
Add table
Add a link
Reference in a new issue