Skip to content

fix(auxiliary): unwrap explicit provider:moa to its aggregator, not the literal name - #575

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56691
Open

fix(auxiliary): unwrap explicit provider:moa to its aggregator, not the literal name#575
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56691

Conversation

@hashbender

Copy link
Copy Markdown
Owner

What does this PR do?

_resolve_task_provider_model() in agent/auxiliary_client.py returned an explicit provider="moa" override — either a caller-passed arg (e.g. a per-task model override naming a MoA preset) or auxiliary.<task>.provider: moa in config.yaml — verbatim, with no MoA-preset unwrap. Only the implicit "main provider is moa" path inside _resolve_auto() unwraps to the aggregator slot (NousResearch#53827, already merged) — this function never goes through _resolve_auto() at all, so the explicit case was never covered.

MoA is a virtual provider with no real HTTP endpoint: resolve_provider_client() looks "moa" up in PROVIDER_REGISTRY (no such entry — it's not a real HTTP provider), falls to the unknown-provider dead end, and call_llm surfaces a nonsensical "Provider 'moa' is set in config.yaml but no API key was found. Set the MOA_API_KEY environment variable..." error.

Verified live before the fix: _resolve_task_provider_model('title_generation') with auxiliary.title_generation.provider: moa configured returned ('moa', None, None, None, None) unresolved.

Fix

Mirrors NousResearch#53827's aggregator-resolution approach exactly: when either the explicit provider arg or the config-derived cfg_provider is "moa", resolve the named (or default) MoA preset via resolve_moa_preset() and continue with its aggregator's real provider+model, dropping any explicit base_url/api_key (the moa:// virtual endpoint and placeholder key belong to the facade, not the aggregator's real provider — same reasoning as NousResearch#53827). If the preset can't be resolved (e.g. renamed/deleted), it degrades gracefully to the pre-fix behavior instead of raising harder.

  • _unwrap_moa_provider() helper + call sites for both the explicit-arg and config-derived provider="moa" cases.
  • Tightened base_url/api_key parameter types to Optional[str] (matching their actual None-accepting behavior) — incidentally resolved 5 pre-existing ty diagnostics at call sites (verified via before/after diagnostic diff).

Testing

  • 5 new regression tests in tests/agent/test_auxiliary_client.py::TestResolveTaskProviderModel: explicit-arg unwrap, config-derived unwrap, default-preset fallback when no model is configured, graceful degradation on preset-resolution failure, and a non-moa regression guard.
  • Full existing suite: tests/agent/test_auxiliary_client.py (285 tests) + all MoA-specific suites (test_moa_config.py, test_moa_trace_streamed_capture.py, test_moa_aggregator_cost_slot.py, test_moa_switch_api_mode.py, test_moa_slot_api_mode.py, test_auxiliary_main_first.py — the fix(moa): resolve auxiliary tasks to the aggregator, not the preset name NousResearch/hermes-agent#53827 reference test file) — 336 tests total, 0 failures.
  • ruff check clean. ty check on agent/auxiliary_client.py: diffed diagnostics before/after — net 5 pre-existing diagnostics resolved, zero new ones introduced.

Test plan

  • uv run --frozen --extra dev python -m pytest tests/agent/test_auxiliary_client.py -q — 285 passed
  • uv run --frozen --extra dev python -m pytest tests/hermes_cli/test_moa_config.py tests/agent/test_moa_trace_streamed_capture.py tests/agent/test_moa_aggregator_cost_slot.py tests/agent/test_moa_switch_api_mode.py tests/agent/test_moa_slot_api_mode.py tests/agent/test_auxiliary_main_first.py -q — 51 passed
  • uv run --frozen --extra dev ruff check agent/auxiliary_client.py tests/agent/test_auxiliary_client.py — clean
  • uv run --frozen --extra dev ty check agent/auxiliary_client.py — no new diagnostics (verified via before/after diff), 5 pre-existing ones resolved

Mirror-of: NousResearch#56691
NousResearch#56691

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant