mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
fix(auxiliary): route all MoA aux resolution through one shared aggregator helper
Follow-up to srojk34's explicit-provider unwrap (PR #56691): - Extract _resolve_moa_aggregator() as the single preset->aggregator resolver shared by _resolve_auto(), _resolve_task_provider_model(), and resolve_provider_client() so preset lookup/validation can't drift. - When the main provider is moa, the aggregator model is now the default for every UNSET auxiliary model: _read_main_model_for_aux() substitutes the preset's acting (aggregator) model wherever fallback chains pre-filled from _read_main_model() (router prefill, custom-endpoint fallback, named-custom default, external-process default, _try_main_agent_model_fallback). - Unwrap moa at the resolve_provider_client() chokepoint so direct callers (vision auto-detect, plugin code) can't dead-end in the unknown-provider branch, and unwrap the vision auto-detect main provider before capability probes run against the preset name. - Real-config tests: temp HERMES_HOME + actual config.yaml exercising the genuine load_config()/resolve_moa_preset() boundary.
This commit is contained in:
parent
cdfe562342
commit
2755bf558e
2 changed files with 270 additions and 38 deletions
|
|
@ -2303,6 +2303,62 @@ def _read_main_base_url() -> str:
|
|||
return ""
|
||||
|
||||
|
||||
def _resolve_moa_aggregator(preset_name: Optional[str]) -> Tuple[Optional[str], Optional[str]]:
|
||||
"""Resolve a MoA preset to its aggregator (provider, model) pair.
|
||||
|
||||
"moa" is a virtual provider — the acting model of a preset is its
|
||||
aggregator slot, and there is no real "moa" HTTP endpoint. Auxiliary
|
||||
tasks (title generation, compression, vision, commit messages, …) don't
|
||||
need the reference fan-out, so every aux resolution layer maps
|
||||
provider="moa"/model=<preset> to the aggregator's real provider+model
|
||||
through this single helper (shared by ``_resolve_auto``,
|
||||
``_resolve_task_provider_model``, and ``resolve_provider_client`` so the
|
||||
preset lookup and validation cannot drift between paths).
|
||||
|
||||
Args:
|
||||
preset_name: The MoA preset name (usually carried in the "model"
|
||||
field), or None/"" to resolve the user's default preset.
|
||||
|
||||
Returns:
|
||||
(aggregator_provider, aggregator_model), or (None, None) when the
|
||||
preset cannot be resolved (missing config, renamed/deleted preset,
|
||||
or a malformed aggregator slot).
|
||||
"""
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
from hermes_cli.moa_config import resolve_moa_preset
|
||||
|
||||
preset = resolve_moa_preset(load_config().get("moa") or {}, preset_name or None)
|
||||
agg = preset.get("aggregator") or {}
|
||||
agg_provider = str(agg.get("provider") or "").strip()
|
||||
agg_model = str(agg.get("model") or "").strip()
|
||||
if agg_provider and agg_model and agg_provider.lower() != "moa":
|
||||
return agg_provider, agg_model
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"MoA aggregator resolution failed for preset %r", preset_name, exc_info=True
|
||||
)
|
||||
return None, None
|
||||
|
||||
|
||||
def _read_main_model_for_aux() -> str:
|
||||
"""Main model with MoA presets unwrapped to the aggregator's model.
|
||||
|
||||
When the main provider is ``moa``, ``_read_main_model()`` returns a MoA
|
||||
*preset name* (e.g. "opus-gpt") — never a valid wire model id on any
|
||||
provider. Auxiliary fallback chains that pre-fill a missing model from
|
||||
the main model must use this reader instead, so unset aux models default
|
||||
to the preset's acting (aggregator) model. Returns "" when the main
|
||||
provider is moa but the preset cannot be resolved — sending nothing is
|
||||
strictly better than sending a preset name that 400s.
|
||||
"""
|
||||
model = _read_main_model()
|
||||
if (_read_main_provider() or "").strip().lower() == "moa":
|
||||
_, agg_model = _resolve_moa_aggregator(model)
|
||||
return agg_model or ""
|
||||
return model
|
||||
|
||||
|
||||
def _read_main_api_key_if_same_host(aux_base_url: str) -> str:
|
||||
"""Return the main api_key only when *aux_base_url* points at the same
|
||||
host as the main model's base_url.
|
||||
|
|
@ -2575,7 +2631,7 @@ def _try_custom_endpoint() -> Tuple[Optional[Any], Optional[str]]:
|
|||
return None, None
|
||||
if custom_base.lower().startswith(_CODEX_AUX_BASE_URL.lower()):
|
||||
return None, None
|
||||
model = _read_main_model() or "gpt-4o-mini"
|
||||
model = _read_main_model_for_aux() or "gpt-4o-mini"
|
||||
logger.debug("Auxiliary client: custom endpoint (%s, api_mode=%s)", model, custom_mode or "chat_completions")
|
||||
_clean_base, _dq = _extract_url_query_params(custom_base)
|
||||
_extra = {"default_query": _dq} if _dq else {}
|
||||
|
|
@ -4045,6 +4101,13 @@ def _try_main_agent_model_fallback(
|
|||
"""
|
||||
main_provider = (_read_main_provider() or "").strip()
|
||||
main_model = (_read_main_model() or "").strip()
|
||||
if main_provider.lower() == "moa":
|
||||
# MoA virtual provider: fall back to the preset's aggregator — the
|
||||
# acting model — instead of the unreachable "moa"/<preset-name> pair.
|
||||
_agg_provider, _agg_model = _resolve_moa_aggregator(main_model)
|
||||
if not _agg_provider or not _agg_model:
|
||||
return None, None, ""
|
||||
main_provider, main_model = _agg_provider, _agg_model
|
||||
if not main_provider or not main_model or main_provider.lower() in {"auto", ""}:
|
||||
return None, None, ""
|
||||
|
||||
|
|
@ -4445,26 +4508,17 @@ def _resolve_auto(
|
|||
# model. Resolve the MoA preset to its aggregator slot and continue Step 1
|
||||
# with that real provider+model. Mirrors the MoA context-length resolution.
|
||||
if main_provider == "moa":
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
from hermes_cli.moa_config import resolve_moa_preset
|
||||
|
||||
_preset = resolve_moa_preset(load_config().get("moa") or {}, main_model)
|
||||
_agg = _preset.get("aggregator") or {}
|
||||
_agg_provider = str(_agg.get("provider") or "").strip()
|
||||
_agg_model = str(_agg.get("model") or "").strip()
|
||||
if _agg_provider and _agg_model and _agg_provider.lower() != "moa":
|
||||
main_provider = _agg_provider
|
||||
main_model = _agg_model
|
||||
# The MoA virtual runtime carries a non-HTTP base_url
|
||||
# ("moa://local") and a placeholder api_key; they belong to the
|
||||
# facade, not the aggregator's real provider. Drop them so the
|
||||
# aggregator resolves through its own provider credentials.
|
||||
runtime_base_url = ""
|
||||
runtime_api_key = ""
|
||||
runtime_api_mode = ""
|
||||
except Exception:
|
||||
logger.debug("MoA aux resolution to aggregator failed", exc_info=True)
|
||||
_agg_provider, _agg_model = _resolve_moa_aggregator(main_model)
|
||||
if _agg_provider and _agg_model:
|
||||
main_provider = _agg_provider
|
||||
main_model = _agg_model
|
||||
# The MoA virtual runtime carries a non-HTTP base_url
|
||||
# ("moa://local") and a placeholder api_key; they belong to the
|
||||
# facade, not the aggregator's real provider. Drop them so the
|
||||
# aggregator resolves through its own provider credentials.
|
||||
runtime_base_url = ""
|
||||
runtime_api_key = ""
|
||||
runtime_api_mode = ""
|
||||
|
||||
if (main_provider and main_model
|
||||
and main_provider not in {"auto", ""}):
|
||||
|
|
@ -4719,6 +4773,27 @@ def resolve_provider_client(
|
|||
# Normalise aliases
|
||||
provider = _normalize_aux_provider(provider)
|
||||
|
||||
# MoA virtual provider chokepoint: "moa" is not a real HTTP provider —
|
||||
# its acting model is the preset's aggregator slot. The two resolver
|
||||
# layers above (_resolve_auto, _resolve_task_provider_model) already
|
||||
# unwrap their own paths, but callers that route here directly (vision
|
||||
# auto-detect, _try_main_agent_model_fallback, get_available_vision_backends,
|
||||
# plugin code) would otherwise dead-end in the unknown-provider branch.
|
||||
# ``model`` carries the preset name for moa calls; when the preset can't
|
||||
# be resolved we leave the call untouched and let the normal
|
||||
# missing-provider handling produce its diagnostic.
|
||||
if provider == "moa":
|
||||
_agg_provider, _agg_model = _resolve_moa_aggregator(model)
|
||||
if _agg_provider and _agg_model:
|
||||
original_provider = _agg_provider.strip().lower()
|
||||
provider = _normalize_aux_provider(_agg_provider)
|
||||
model = _agg_model
|
||||
# The moa:// facade endpoint and placeholder key belong to the
|
||||
# virtual runtime, not the aggregator's real provider.
|
||||
if explicit_base_url and str(explicit_base_url).lower().startswith("moa://"):
|
||||
explicit_base_url = None
|
||||
explicit_api_key = None
|
||||
|
||||
# Universal model-resolution fallback for concrete providers. ``auto`` is
|
||||
# intentionally excluded: `_resolve_auto(main_runtime=...)` returns the
|
||||
# model paired with the provider it actually selected. Pre-filling an auto
|
||||
|
|
@ -4739,6 +4814,10 @@ def resolve_provider_client(
|
|||
# the load-bearing step for OAuth providers: an xai-oauth user
|
||||
# with grok-4.3 configured gets grok-4.3 for title generation
|
||||
# instead of silently dropping to whatever Step-2 fallback (#31845).
|
||||
# When the main provider is MoA, ``_read_main_model_for_aux()``
|
||||
# substitutes the preset's aggregator model — the preset NAME is
|
||||
# never a valid wire model id, so unset aux models default to the
|
||||
# preset's acting model instead.
|
||||
#
|
||||
# Each provider branch below sees a non-empty ``model`` whenever the
|
||||
# user has *anything* configured — no provider-specific empty-model
|
||||
|
|
@ -4755,7 +4834,7 @@ def resolve_provider_client(
|
|||
# return the actual current runtime model when the caller did not explicitly
|
||||
# request one. (# compression-current-model)
|
||||
if not model and provider != "auto":
|
||||
model = _get_aux_model_for_provider(provider) or _read_main_model() or model
|
||||
model = _get_aux_model_for_provider(provider) or _read_main_model_for_aux() or model
|
||||
|
||||
def _needs_codex_wrap(client_obj, base_url_str: str, model_str: str) -> bool:
|
||||
"""Decide if a plain OpenAI client should be wrapped for Responses API.
|
||||
|
|
@ -5029,7 +5108,7 @@ def resolve_provider_client(
|
|||
model
|
||||
or custom_entry.get("model")
|
||||
or (main_runtime.get("model") if main_runtime else None)
|
||||
or _read_main_model()
|
||||
or _read_main_model_for_aux()
|
||||
or "gpt-4o-mini",
|
||||
provider,
|
||||
)
|
||||
|
|
@ -5264,7 +5343,7 @@ def resolve_provider_client(
|
|||
final_model = _normalize_resolved_model(
|
||||
model
|
||||
or (main_runtime.get("model") if main_runtime else None)
|
||||
or _read_main_model(),
|
||||
or _read_main_model_for_aux(),
|
||||
provider,
|
||||
)
|
||||
if provider == "copilot-acp":
|
||||
|
|
@ -5632,7 +5711,24 @@ def resolve_vision_provider_client(
|
|||
# 5. Stop
|
||||
main_provider = str(runtime.get("provider") or _read_main_provider())
|
||||
main_model = str(runtime.get("model") or _read_main_model())
|
||||
if main_provider and main_provider not in {"auto", ""}:
|
||||
if main_provider.strip().lower() == "moa":
|
||||
# MoA virtual provider: main_model is a preset NAME, and every
|
||||
# capability probe below (_PROVIDERS_WITHOUT_VISION,
|
||||
# _main_model_supports_vision, _resolve_provider_vision_default)
|
||||
# would run against a provider/model pair that doesn't exist on
|
||||
# any wire. Unwrap to the preset's aggregator slot first so the
|
||||
# checks and the eventual client target the real acting model.
|
||||
_agg_provider, _agg_model = _resolve_moa_aggregator(main_model)
|
||||
if _agg_provider and _agg_model:
|
||||
main_provider, main_model = _agg_provider, _agg_model
|
||||
# Drop the moa:// facade endpoint from the runtime view used
|
||||
# below — it belongs to the virtual provider, not the
|
||||
# aggregator's real provider.
|
||||
runtime = dict(runtime)
|
||||
runtime["base_url"] = ""
|
||||
runtime["api_key"] = ""
|
||||
runtime["api_mode"] = ""
|
||||
if main_provider and main_provider not in {"auto", "", "moa"}:
|
||||
# A provider-specific vision default wins over the user's chat model:
|
||||
# static overrides (xiaomi/zai) and catalog-backed discovery (the
|
||||
# DeepInfra profile hook) both yield a *known* vision-capable model,
|
||||
|
|
@ -6290,22 +6386,13 @@ def _resolve_task_provider_model(
|
|||
# "MOA_API_KEY environment variable" error for a provider that was never
|
||||
# meant to be reached over the wire. Auxiliary tasks don't need the
|
||||
# reference fan-out — resolve to the preset's aggregator slot instead,
|
||||
# exactly like the implicit path does.
|
||||
# exactly like the implicit path does (shared helper: _resolve_moa_aggregator).
|
||||
def _unwrap_moa_provider(prov: str, mdl: Optional[str]) -> Tuple[str, Optional[str]]:
|
||||
if prov.strip().lower() != "moa":
|
||||
return prov, mdl
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
from hermes_cli.moa_config import resolve_moa_preset
|
||||
|
||||
preset = resolve_moa_preset(load_config().get("moa") or {}, mdl)
|
||||
agg = preset.get("aggregator") or {}
|
||||
agg_provider = str(agg.get("provider") or "").strip()
|
||||
agg_model = str(agg.get("model") or "").strip()
|
||||
if agg_provider and agg_model and agg_provider.lower() != "moa":
|
||||
return agg_provider, agg_model
|
||||
except Exception:
|
||||
logger.debug("MoA aux resolution (explicit provider override) to aggregator failed", exc_info=True)
|
||||
agg_provider, agg_model = _resolve_moa_aggregator(mdl)
|
||||
if agg_provider and agg_model:
|
||||
return agg_provider, agg_model
|
||||
return prov, mdl
|
||||
|
||||
if provider and str(provider).strip().lower() == "moa":
|
||||
|
|
|
|||
|
|
@ -395,6 +395,151 @@ class TestResolveTaskProviderModel:
|
|||
assert model == "claude-haiku-4-5-20251001"
|
||||
|
||||
|
||||
class TestMoaAggregatorSharedResolution:
|
||||
"""The shared MoA→aggregator helper and the layers that consume it.
|
||||
|
||||
Real-config tests: write an actual config.yaml under a temp HERMES_HOME
|
||||
and exercise the genuine load_config() → resolve_moa_preset() boundary —
|
||||
no mocking of the configuration-resolution chain.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _write_moa_config(tmp_path, monkeypatch, default_preset="opus-gpt"):
|
||||
import yaml
|
||||
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir(exist_ok=True)
|
||||
(home / "config.yaml").write_text(
|
||||
yaml.safe_dump(
|
||||
{
|
||||
"moa": {
|
||||
"default_preset": default_preset,
|
||||
"presets": {
|
||||
"opus-gpt": {
|
||||
"enabled": True,
|
||||
"reference_models": [
|
||||
{"provider": "openrouter", "model": "openai/gpt-5.5"}
|
||||
],
|
||||
"aggregator": {
|
||||
"provider": "openrouter",
|
||||
"model": "anthropic/claude-opus-4.8",
|
||||
},
|
||||
},
|
||||
"nous-mix": {
|
||||
"enabled": True,
|
||||
"reference_models": [
|
||||
{"provider": "nous", "model": "hermes-4-70b"}
|
||||
],
|
||||
"aggregator": {
|
||||
"provider": "nous",
|
||||
"model": "hermes-4-405b",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
)
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
return home
|
||||
|
||||
def test_real_config_explicit_task_provider_moa(self, tmp_path, monkeypatch):
|
||||
"""auxiliary.<task>.provider: moa in a REAL config.yaml resolves to the
|
||||
aggregator through the genuine load_config()/resolve_moa_preset() path."""
|
||||
import yaml
|
||||
|
||||
home = self._write_moa_config(tmp_path, monkeypatch)
|
||||
cfg = yaml.safe_load((home / "config.yaml").read_text())
|
||||
cfg["auxiliary"] = {"title_generation": {"provider": "moa", "model": "opus-gpt"}}
|
||||
(home / "config.yaml").write_text(yaml.safe_dump(cfg))
|
||||
|
||||
resolved_provider, model, base_url, api_key, api_mode = _resolve_task_provider_model(
|
||||
task="title_generation",
|
||||
)
|
||||
|
||||
assert resolved_provider == "openrouter"
|
||||
assert model == "anthropic/claude-opus-4.8"
|
||||
assert base_url is None
|
||||
assert api_key is None
|
||||
|
||||
def test_real_config_explicit_task_provider_moa_default_preset(self, tmp_path, monkeypatch):
|
||||
"""provider: moa with no model resolves the default preset's aggregator."""
|
||||
import yaml
|
||||
|
||||
home = self._write_moa_config(tmp_path, monkeypatch, default_preset="nous-mix")
|
||||
cfg = yaml.safe_load((home / "config.yaml").read_text())
|
||||
cfg["auxiliary"] = {"compression": {"provider": "moa"}}
|
||||
(home / "config.yaml").write_text(yaml.safe_dump(cfg))
|
||||
|
||||
resolved_provider, model, base_url, api_key, api_mode = _resolve_task_provider_model(
|
||||
task="compression",
|
||||
)
|
||||
|
||||
assert resolved_provider == "nous"
|
||||
assert model == "hermes-4-405b"
|
||||
|
||||
def test_read_main_model_for_aux_unwraps_preset_name(self, tmp_path, monkeypatch):
|
||||
"""Main provider moa → the aux-facing main model is the aggregator's
|
||||
model, so every unset auxiliary model defaults to the acting model
|
||||
instead of the preset name."""
|
||||
from agent.auxiliary_client import _read_main_model_for_aux
|
||||
|
||||
self._write_moa_config(tmp_path, monkeypatch)
|
||||
with patch("agent.auxiliary_client._read_main_provider", return_value="moa"), \
|
||||
patch("agent.auxiliary_client._read_main_model", return_value="opus-gpt"):
|
||||
assert _read_main_model_for_aux() == "anthropic/claude-opus-4.8"
|
||||
|
||||
def test_read_main_model_for_aux_unresolvable_preset_returns_empty(self, tmp_path, monkeypatch):
|
||||
"""A moa main with a deleted/renamed preset yields "" — never the
|
||||
preset name, which would 400 on any wire."""
|
||||
from agent.auxiliary_client import _read_main_model_for_aux
|
||||
|
||||
self._write_moa_config(tmp_path, monkeypatch)
|
||||
with patch("agent.auxiliary_client._read_main_provider", return_value="moa"), \
|
||||
patch("agent.auxiliary_client._read_main_model", return_value="gone-preset"):
|
||||
assert _read_main_model_for_aux() == ""
|
||||
|
||||
def test_read_main_model_for_aux_passthrough_for_non_moa(self, monkeypatch):
|
||||
from agent.auxiliary_client import _read_main_model_for_aux
|
||||
|
||||
with patch("agent.auxiliary_client._read_main_provider", return_value="openrouter"), \
|
||||
patch("agent.auxiliary_client._read_main_model", return_value="anthropic/claude-opus-4.6"):
|
||||
assert _read_main_model_for_aux() == "anthropic/claude-opus-4.6"
|
||||
|
||||
def test_resolve_provider_client_direct_moa_unwraps(self, tmp_path, monkeypatch):
|
||||
"""Callers that hit resolve_provider_client("moa", <preset>) directly
|
||||
(vision auto-detect, plugin code) unwrap at the router chokepoint
|
||||
instead of dead-ending in the unknown-provider branch."""
|
||||
self._write_moa_config(tmp_path, monkeypatch)
|
||||
monkeypatch.setenv("OPENROUTER_API_KEY", "or-test-key")
|
||||
|
||||
client, model = resolve_provider_client("moa", "opus-gpt")
|
||||
|
||||
assert client is not None
|
||||
assert model == "anthropic/claude-opus-4.8"
|
||||
|
||||
def test_main_agent_fallback_uses_aggregator_for_moa_main(self, tmp_path, monkeypatch):
|
||||
"""_try_main_agent_model_fallback with a moa main resolves the
|
||||
aggregator instead of asking for a literal "moa" client."""
|
||||
from agent.auxiliary_client import _try_main_agent_model_fallback
|
||||
|
||||
self._write_moa_config(tmp_path, monkeypatch)
|
||||
with patch("agent.auxiliary_client._read_main_provider", return_value="moa"), \
|
||||
patch("agent.auxiliary_client._read_main_model", return_value="opus-gpt"), \
|
||||
patch("agent.auxiliary_client._is_provider_unhealthy", return_value=False), \
|
||||
patch("agent.auxiliary_client.resolve_provider_client") as mock_resolve:
|
||||
mock_client = MagicMock()
|
||||
mock_resolve.return_value = (mock_client, "anthropic/claude-opus-4.8")
|
||||
|
||||
client, model, label = _try_main_agent_model_fallback("anthropic", task="compression")
|
||||
|
||||
assert client is mock_client
|
||||
assert model == "anthropic/claude-opus-4.8"
|
||||
assert label == "main-agent(openrouter)"
|
||||
assert mock_resolve.call_args.kwargs["provider"] == "openrouter"
|
||||
assert mock_resolve.call_args.kwargs["model"] == "anthropic/claude-opus-4.8"
|
||||
|
||||
|
||||
class TestBuildCallKwargsMaxTokens:
|
||||
"""_build_call_kwargs should not cap output by default (#34530).
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue