feat(moa): add reference model toggles - #59753
Conversation
Related: implements the per-reference MoA enable/disable feature requested in #59707 (kept open as the spec thread). Distinct from #59743, which adds the per-preset |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for implementing the per-reference MoA toggle requested in #59707. The feature premise is still valid on current main: hermes_cli/moa_config.py:92-110 drops per-reference enabled, and both execution paths fan out every slot (agent/moa_loop.py:683-690, 906-908).
Problems
- This branch predates current per-slot reasoning support. Current
hermes_cli/moa_config.py:107-109preservesreasoning_effort(commit3dca75b45c2edf38089ef07c22023afe357aebc1); merge the enabled handling into that normalizer rather than replacing it. - The runtime test covers persistent
MoAChatCompletions.create(), but the PR also changes one-shot aggregation. Add a disabled-reference test directly throughaggregate_moa_context()(agent/conversation_loop.py:877-893). - Add an accessible name to the dashboard switch; nearby dashboard toggle controls provide
aria-label(web/src/components/ToolsetConfigDrawer.tsx:252-256).
Suggested changes
- Salvage against current MoA slot semantics and retain
reasoning_effort,reference_max_tokens, andfanoutbehavior. - Cover both persistent and one-shot reference fan-out paths.
Automated hermes-sweeper review.
| @@ -87,12 +106,15 @@ def _clean_slot(slot: Any) -> dict[str, str] | None: | |||
| # an invalid slot is dropped, falling back to the preset's defaults. | |||
| if provider.lower() == "moa": | |||
| return None | |||
There was a problem hiding this comment.
Current main's _clean_slot preserves reasoning_effort (3dca75b45c2edf38089ef07c22023afe357aebc1, now hermes_cli/moa_config.py:107-109). When salvaging, compose enabled normalization with that behavior rather than replacing it, or normalization will discard existing per-slot reasoning settings.
| @@ -594,6 +594,7 @@ def aggregate_moa_context( | |||
| provider default applies — matching single-model agent behavior. Presets | |||
There was a problem hiding this comment.
Please add a direct aggregate_moa_context() test for this filter. The new runtime test covers persistent MoAChatCompletions.create(), but one-shot /moa reaches this separate path through agent/conversation_loop.py:877-893.
| className={cn( | ||
| "flex items-center gap-2 border border-border/50 bg-muted/20 px-3 py-2", | ||
| slot.enabled === false && "opacity-60" | ||
| )} |
There was a problem hiding this comment.
Add a stateful accessible name such as Disable reference ${index + 1} / Enable reference ${index + 1}. This switch has no associated label; existing dashboard switches provide aria-label.
…abled flag The per-reference-model enabled toggle (#59753 salvage) intentionally adds 'enabled' to normalized slot dicts. The two endpoint tests asserted the exact key set {provider, model} — convert them to subset + round-trip contracts so optional slot keys (enabled, reasoning_effort, max_tokens) don't break them again.
Follow-up for salvaged PR #59753 rebased over the per-slot reasoning_effort feature: _clean_slot now round-trips reasoning_effort AND enabled together; add a normalize→normalize regression test, update the validate/normalize agreement contract for the canonical enabled default, restore the desktop per-slot toggle test on the current autosave editor, and map oppenheimor's contributor email.
…abled flag The per-reference-model enabled toggle (#59753 salvage) intentionally adds 'enabled' to normalized slot dicts. The two endpoint tests asserted the exact key set {provider, model} — convert them to subset + round-trip contracts so optional slot keys (enabled, reasoning_effort, max_tokens) don't break them again.
Follow-up for salvaged PR #59753 rebased over the per-slot reasoning_effort feature: _clean_slot now round-trips reasoning_effort AND enabled together; add a normalize→normalize regression test, update the validate/normalize agreement contract for the canonical enabled default, restore the desktop per-slot toggle test on the current autosave editor, and map oppenheimor's contributor email.
…abled flag The per-reference-model enabled toggle (#59753 salvage) intentionally adds 'enabled' to normalized slot dicts. The two endpoint tests asserted the exact key set {provider, model} — convert them to subset + round-trip contracts so optional slot keys (enabled, reasoning_effort, max_tokens) don't break them again.
Follow-up for salvaged PR #59753 rebased over the per-slot reasoning_effort feature: _clean_slot now round-trips reasoning_effort AND enabled together; add a normalize→normalize regression test, update the validate/normalize agreement contract for the canonical enabled default, restore the desktop per-slot toggle test on the current autosave editor, and map oppenheimor's contributor email.
…abled flag The per-reference-model enabled toggle (#59753 salvage) intentionally adds 'enabled' to normalized slot dicts. The two endpoint tests asserted the exact key set {provider, model} — convert them to subset + round-trip contracts so optional slot keys (enabled, reasoning_effort, max_tokens) don't break them again.
|
Merged via cluster PR #70283 (commit 385a065) — your commit cherry-picked with authorship preserved, rebased over the per-slot reasoning_effort feature that landed after you opened this; _clean_slot now round-trips reasoning_effort, max_tokens, AND enabled together with a round-trip regression test. Implements #59707. Thanks! |
Follow-up for salvaged PR NousResearch#59753 rebased over the per-slot reasoning_effort feature: _clean_slot now round-trips reasoning_effort AND enabled together; add a normalize→normalize regression test, update the validate/normalize agreement contract for the canonical enabled default, restore the desktop per-slot toggle test on the current autosave editor, and map oppenheimor's contributor email.
…abled flag The per-reference-model enabled toggle (NousResearch#59753 salvage) intentionally adds 'enabled' to normalized slot dicts. The two endpoint tests asserted the exact key set {provider, model} — convert them to subset + round-trip contracts so optional slot keys (enabled, reasoning_effort, max_tokens) don't break them again.
What does this PR do?
Adds per-reference enable/disable support for MoA reference models.
Each MoA reference model now keeps an
enabledflag, defaulting totruefor existing configs. Disabled references stay in the preset, but are skipped during MoA inference so they do not make API calls or contribute to the aggregator context.Related Issue
feat #59707
Type of Change
Changes Made
enablednormalization for MoA presets.How to Test
Run MoA backend tests:
source .venv/bin/activate pytest tests/hermes_cli/test_moa_config.py tests/run_agent/test_moa_loop_mode.py tests/cli/test_moa_command.pyRun dashboard typecheck:
Manually verify in the dashboard:
Open Models -> Configure MoA presets, toggle a reference model off, save, and confirm the disabled reference stays in the preset but is skipped during MoA runs.
Checklist
Code
pytest tests/ -qand all tests passDocumentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs
Manually verified through
hermes dashboard.