Conversation
…opping it _build_call_kwargs only added max_tokens to the request kwargs for Anthropic-compat and NVIDIA NIM endpoints. For every other provider (OpenRouter, custom/local, OpenAI direct) it computed max_tokens but never put it in kwargs — so an explicit cap from any auxiliary caller was silently ignored. This affected MoA reference_max_tokens (did nothing unless the reference model was on an Anthropic-compat endpoint) and title_generation (max_tokens=500 was dropped on the floor). Now: when max_tokens is explicitly non-None, forward it via auxiliary_max_tokens_param() which picks the correct wire field (max_tokens vs max_completion_tokens) per model. Anthropic-compat and NIM still get max_tokens directly (mandatory field on that wire). The existing retry logic strips the param on 400 if a provider rejects it, so forwarding is safe. Tests updated from asserting "max_tokens is always omitted" to asserting the correct forwarding behavior per provider/model.
MoA reference_max_tokens is preset-level — one cap for all reference
models. When mixing a verbose model with a terse one, a single cap is
either too tight for the terse model or too loose for the verbose one.
Now each reference slot can optionally carry its own max_tokens:
reference_models:
- provider: openrouter
model: deepseek/deepseek-v4-pro
max_tokens: *** # per-slot cap, overrides preset-level
- provider: openai-codex
model: gpt-5.5
# no max_tokens → falls back to preset-level reference_max_tokens
_clean_slot (moa_config.py) preserves an optional max_tokens field on
the slot dict, coerced via _coerce_int_or_none. _run_reference
(moa_loop.py) reads slot-level max_tokens first, falling back to the
preset-level cap passed by the caller. Slots without the field are
unaffected — backward compatible.
Type hints on slot-handling functions updated from dict[str, str] to
dict[str, Any] to reflect the now-heterogeneous slot shape.
alt-glitch
added
type/bug
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the dropped preset-level MoA cap. Current main does have a real contract mismatch: reference_max_tokens is documented at website/docs/user-guide/features/mixture-of-agents.md:112, reaches _run_references_parallel() at agent/moa_loop.py:1002, and is then omitted by _build_call_kwargs() for non-Anthropic/NIM routes at agent/auxiliary_client.py:6382.
Problems
- The blanket forwarding at
agent/auxiliary_client.py:6147reverses the deliberate omission introduced by2062a840; current regression coverage attests/agent/test_auxiliary_client.py:278requires omission for OpenAI-compatible routes, including ZAI vision. Retrying after a provider rejection does not preserve that behavior. hermes_cli/moa_config.py:96adds a new per-slot user-facingmax_tokensconfiguration surface. Please keep that separate from repairing the existing documentedreference_max_tokensbehavior.
Suggested changes
- Scope the salvage to the existing preset-level MoA cap and preserve the current safe omission behavior for unrelated auxiliary tasks.
- Add a
call_llm-level regression test for the actual wire kwargs and provider-rejection path, rather than testing only_build_call_kwargs.
This is an automated hermes-sweeper review.
| # Use the model-aware helper to pick max_tokens vs | ||
| # max_completion_tokens. The retry logic below handles any | ||
| # provider that rejects the param by stripping and retrying. | ||
| kwargs.update( |
There was a problem hiding this comment.
This broadens explicit output-cap forwarding to every non-Anthropic/NIM auxiliary route, but current main intentionally omits those caps to avoid OpenAI-compatible and ZAI failures (2062a840; tests/agent/test_auxiliary_client.py:278). Please scope the repair to the documented MoA reference-cap path, or preserve the existing omission behavior for unrelated auxiliary tasks.
| # default) = no cap, so existing slots are unaffected. Allows tuning | ||
| # each advisor's output length independently — useful when one model | ||
| # is verbose and another is terse. | ||
| slot_mt = _coerce_int_or_none(slot.get("max_tokens")) |
There was a problem hiding this comment.
This is a new user-facing per-slot max_tokens configuration surface. Please split it from the repair for the already documented preset-level reference_max_tokens setting; the two changes need separate maintainer direction.
teknium1
added
sweeper:risk-compatibility
|
The per-slot max_tokens slice of this draft was merged via cluster PR #70279 (commit bc7212c) with your authorship preserved on that commit — _clean_slot whitelist, slot-cap-overrides-preset precedence, and your tests. The blanket _build_call_kwargs forwarding was NOT taken: it reverses the deliberate #34845 omission-by-default design, which is a maintainer decision that stays parked on your issue #60388 (thanks for filing that too). Closing the draft with the shipped half credited. |
4 participants
What does this PR do?
This branch improves how the harness handles max_token settings in config.yaml, including fixing some outstanding bugs which caused documented
max_tokens-like settings to be silently dropped.Related Issue
Fixes #60388
Type of Change
Changes Made
How to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
hermes --toolsets skills -q "Use the X skill to do Y"Screenshots / Logs