From cc1725cbe50feef6d452bddc42784effae4373e5 Mon Sep 17 00:00:00 2001 From: srojk34 <286497132+srojk34@users.noreply.github.com> Date: Fri, 3 Jul 2026 06:17:12 +0300 Subject: [PATCH] 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. --- agent/conversation_loop.py | 2 +- agent/moa_loop.py | 24 +++--- tests/agent/test_moa_context_max_tokens.py | 99 ++++++++++++++++++++++ 3 files changed, 113 insertions(+), 12 deletions(-) create mode 100644 tests/agent/test_moa_context_max_tokens.py diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index bcaa80dce8c9..cf196b3814a8 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -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): diff --git a/agent/moa_loop.py b/agent/moa_loop.py index e075b0018073..182630fa2c4a 100644 --- a/agent/moa_loop.py +++ b/agent/moa_loop.py @@ -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, ) diff --git a/tests/agent/test_moa_context_max_tokens.py b/tests/agent/test_moa_context_max_tokens.py new file mode 100644 index 000000000000..5800b2a7764f --- /dev/null +++ b/tests/agent/test_moa_context_max_tokens.py @@ -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]