Skip to content

fix(auxiliary): honor max_tokens for MoA reference/aggregator tasks - #58402

Closed
Janig88 wants to merge 4 commits into
NousResearch:mainfrom
Janig88:fix/reference-max-tokens-aux-client
Closed

Janig88 wants to merge 4 commits into
NousResearch:mainfrom
Janig88:fix/reference-max-tokens-aux-client

Conversation

@Janig88

@Janig88 Janig88 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

The reference_max_tokens config option (added in PR #56756 on Jul 2, 2026) is silently
ignored for all OpenAI-compatible providers. The value travels correctly through five layers
of MoA code (moa_config.pyconversation_loop.pyaggregate_moa_context()
_run_references_parallel()_run_reference()call_llm(task="moa_reference", max_tokens=800, ...)), but the final delivery layer — _build_call_kwargs() in
auxiliary_client.py — drops max_tokens for any provider that isn't Anthropic-compatible
or NVIDIA NIM.

This means the cap never worked for any user on OpenRouter, Z.AI (coding plan endpoint),
OpenAI, local providers, or GitHub Copilot. Every reference call ran uncapped regardless of
the config setting.

Root Cause — Two PRs That Collided

Date PR Author What it did
May 29, 2026 #34845 teknium1 Made _build_call_kwargs() stop sending max_tokens for all OpenAI-compatible providers (fix for #34530 — Copilot GPT-5 models 400 on max_tokens). Only Anthropic-compat endpoints keep it.
Jul 2, 2026 #56756 teknium1 Added reference_max_tokens config option. Wired it through moa_loop.py and conversation_loop.py (5 files, 117 insertions). Did NOT touch auxiliary_client.py — the file where max_tokens gets dropped.

Same author wrote both PRs one month apart. The second PR added the config key, 5 unit tests,
official docs, and correct wiring — but didn't realize the final delivery layer would silently
discard the value. No existing test caught this because the tests only exercise _build_call_kwargs()
with providers where max_tokens is already dropped (OpenAI-compat), so they assert its absence
and pass.

The code that drops it

agent/auxiliary_client.py, _build_call_kwargs():

if max_tokens is not None:
    if (
        _is_anthropic_compat_endpoint(provider, _effective_base)
        or _is_nvidia_nim
    ):
        kwargs["max_tokens"] = max_tokens
    # ← OpenAI-compatible providers: silently dropped, never reaches the API

Affected Providers

Provider Resolved URL Cap works before fix?
Z.AI (coding plan) api.z.ai/api/coding/paas/v4 No
OpenRouter openrouter.ai/api/v1 No
OpenAI api.openai.com/v1 No
GitHub Copilot api.githubcopilot.com No
Local (Ollama etc.) localhost:xxxx No
MiniMax / /anthropic endpoints Anthropic-compat Yes (coincidence)

The Fix

Thread the task parameter through all six _build_call_kwargs() call sites. When task
starts with moa_, max_tokens is always included in the request kwargs regardless of provider.

3 changes:

  1. Added task: Optional[str] = None parameter to _build_call_kwargs() signature
  2. Added _is_moa = bool(task) and str(task).startswith("moa_") check in the max_tokens block
  3. Threaded task=task from all six callers (call_llm main + fallback, async_call_llm main + fallback, _retry_same_provider_sync, _retry_same_provider_async)

Non-MoA auxiliary tasks (compression, titles, vision, etc.) keep PR #34845 behavior unchanged —
max_tokens is still dropped for OpenAI-compatible endpoints. No regressions.

Verification

End-to-end API calls (real Z.AI endpoint, GLM-5.2)

Prompt max_tokens Actual output tokens Cap honored?
"Write exactly 3 sentences about cats" none 315 N/A
"Write exactly 3 sentences about cats" 20 20
"Write a 500-word essay..." 20 20

Tests

  • 7 new regression tests in TestBuildCallKwargsMaxTokens:
    • 4 providers (ZAI, OpenRouter, Copilot, Nous) × task="moa_reference" → all send max_tokens
    • MiniMax + task="moa_aggregator" → unchanged Anthropic-compat behavior
    • Non-MoA tasks (compression, vision, titles, None, empty) → all still drop max_tokens
    • Prefix boundary: moa_reference ✓, moa_custom_future ✓, mopha_reference
  • 288 auxiliary_client tests pass (was 281, +7 new)
  • 84 MoA tests pass (11 test files)
  • Zero regressions

Copilot AI review requested due to automatic review settings July 4, 2026 17:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes MoA’s reference_max_tokens cap being ignored by ensuring the auxiliary request-kwargs builder can conditionally include an output-token limit for MoA reference/aggregator calls.

Changes:

  • Add task plumbing into agent/auxiliary_client.py::_build_call_kwargs() and thread it through all internal call sites so MoA calls can be treated specially.
  • Add MoA-focused regression tests asserting that MoA tasks include a token cap while non-MoA auxiliary tasks keep the “omit max_tokens by default” behavior.
  • Adjust retry paths to preserve the task context during same-provider retries and fallback calls.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
agent/auxiliary_client.py Threads task into _build_call_kwargs() and uses it to decide when to include an output token cap (MoA tasks).
tests/agent/test_auxiliary_client.py Adds regression tests covering MoA vs non-MoA max_tokens behavior in _build_call_kwargs().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread agent/auxiliary_client.py Outdated
Comment on lines 5795 to 5801
_is_moa = bool(task) and str(task).startswith("moa_")
if (
_is_anthropic_compat_endpoint(provider, _effective_base)
or _is_nvidia_nim
or _is_moa
):
kwargs["max_tokens"] = max_tokens
Comment thread tests/agent/test_auxiliary_client.py Outdated
base_url=base_url,
task="moa_reference",
)
assert kwargs["max_tokens"] == 800
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have labels Jul 4, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: competing/superset of #58261 (which forwards max_tokens for the gemini-native path only) — this PR restores reference_max_tokens for ALL moa_ tasks via a task-based gate in _build_call_kwargs(). Both fix the same root cause (max_tokens dropped for OpenAI-compat since #34845) for the documented reference_max_tokens contract (#56756). Also related to the aggregator-cap regression fix #57493. Maintainer should pick the canonical approach (broader moa_-task gate here vs. per-provider gate in #58261).

@Janig88
Janig88 force-pushed the fix/reference-max-tokens-aux-client branch from 6fc4103 to e9e2507 Compare July 7, 2026 13:05
Janig88 added 3 commits July 15, 2026 16:19
PR NousResearch#56756 added reference_max_tokens to cap MoA advisor output and cut
turn latency. The value is correctly threaded through five layers of MoA
code (moa_config → conversation_loop → aggregate_moa_context →
_run_references_parallel → _run_reference → call_llm(task='moa_reference',
max_tokens=800, ...)).

However, _build_call_kwargs() in auxiliary_client.py silently drops
max_tokens for all OpenAI-compatible providers (PR NousResearch#34845, which fixed
endpoints and NVIDIA NIM keep it. This means reference_max_tokens never
reached the API for the vast majority of providers.

The bug affects every OpenAI-compatible MoA reference/aggregator slot:
Z.AI (coding plan), OpenRouter, OpenAI, GitHub Copilot, and local
providers. Only Anthropic-compat endpoints (MiniMax, /anthropic URLs)
worked — by coincidence, not MoA-aware design.

Fix: thread the 'task' parameter through all six _build_call_kwargs()
call sites. When task starts with 'moa_', max_tokens is always included
in the request kwargs regardless of provider. Non-MoA auxiliary tasks
(compression, titles, vision, etc.) keep PR NousResearch#34845 behavior unchanged.

Verified end-to-end:
- Z.AI GLM-5.2 with max_tokens=50 → returned exactly 50 tokens
- Z.AI GLM-5.2 with max_tokens=20 → returned exactly 20 tokens
- Z.AI GLM-5.2 uncapped → returned 315 tokens
- 7 new regression tests covering 4 providers, Anthropic wire, non-MoA
  tasks, and prefix-matching boundary
- 288 auxiliary_client tests pass (was 281, +7 new), 84 MoA tests pass
- Zero regressions
Copilot review pointed out that hardcoding kwargs['max_tokens'] would
400 on models requiring max_completion_tokens (GPT-5 family, Copilot).
The existing auxiliary_max_tokens_param() helper already selects the
correct parameter name per model — use it instead of hardcoding.

Test updated to parametrize expected_key so the Copilot gpt-5.5 case
correctly asserts max_completion_tokens instead of max_tokens.

Addresses Copilot review comments on both files.
@Janig88
Janig88 force-pushed the fix/reference-max-tokens-aux-client branch from e9e2507 to 1ae2d5a Compare July 15, 2026 13:23

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the cap through the auxiliary request builder; the underlying reference-path omission is real on current main (agent/moa_loop.py:317-324, agent/auxiliary_client.py:6382-6414).

Problems

  • The new broad moa_ gate also covers moa_aggregator. In the one-shot path, agent/conversation_loop.py:892 passes reference_max_tokens into aggregate_moa_context, and agent/moa_loop.py:722-727 forwards that same value to the aggregator. Sending it after this change contradicts the advisors-only contract in website/docs/user-guide/features/mixture-of-agents.md:112-116.

Suggested changes

  • Limit the builder exception to task == "moa_reference", rather than the moa_ prefix.
  • Add a one-shot regression asserting the configured reference cap reaches advisor calls but not the moa_aggregator call.

Automated hermes-sweeper review.

Comment thread agent/auxiliary_client.py
)
_is_moa = bool(task) and str(task).startswith("moa_")
if (
_is_anthropic_compat_endpoint(provider, _effective_base)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also matches moa_aggregator. Current one-shot MoA passes reference_max_tokens through to that aggregator (agent/conversation_loop.py:892; agent/moa_loop.py:722-727), while the documented contract says this cap applies to advisors only. Restrict this exception to task == "moa_reference".

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
Per review feedback from teknium1: reference_max_tokens is an advisors-only
contract. The aggregator is the acting model and must not be capped by the
reference budget. Changed _is_moa from startswith('moa_') to exact match on
'moa_reference'. Added regression test proving aggregator does NOT receive
max_tokens.
@Janig88

Janig88 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Pushed `818ab4a` — scoped to exact match `task == "moa_reference"`. Added `test_moa_aggregator_does_not_get_max_tokens_on_openai_compat` proving the aggregator keeps getting max_tokens dropped. Renamed the prefix test to `test_moa_task_exact_match` covering both `moa_aggregator` and `moa_custom_future` as exclusions. PR description updated to reflect advisors-only scope.

@teknium1

Copy link
Copy Markdown
Collaborator

Merged via cluster PR #70279 (commit bc7212c) — all four of your commits cherry-picked with authorship preserved, including the exact-match moa_reference gate you added after the sweeper review. Your task-threading through _build_call_kwargs is the wire-layer backbone of the cluster. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants