From 74a56b76b08bccc4b4a85076af15e2c176ab5542 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:25:27 -0700 Subject: [PATCH] test(moa): regression for aggregator-model thought_signature resolution (#66212) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds test_moa_gemini_aggregator_sanitize_uses_real_model: drives a full MoA tool-call turn (virtual-provider mode) with a Gemini aggregator and asserts the strict-API sanitize pass is invoked with the resolved aggregator model (gemini-3-pro-preview), never the virtual preset name once a slot is resolved — the exact path that stripped extra_content/thought_signature and made Gemini aggregators 400 (#65092). Writing the test surfaced a gap in the salvaged #66212 fix: in virtual- provider MoA mode (provider=moa, no moa_config threaded through run_conversation) the conversation-loop branch never fired because it only consulted moa_config. Extend it to fall back to the facade's last_aggregator_slot — the same source the handle_max_iterations fix uses — so both MoA entry modes resolve the real aggregator model. Also adds the contributors/emails mapping for the #15676 credit base. --- agent/conversation_loop.py | 20 +++- .../idrisalmalki@Idriss-MacBook-Air.local | 1 + tests/run_agent/test_moa_loop_mode.py | 92 +++++++++++++++++++ 3 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 contributors/emails/idrisalmalki@Idriss-MacBook-Air.local diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 0a7c36ddbd0..04b5cdb679e 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -1108,10 +1108,22 @@ def run_conversation( # the resolved aggregator model so Gemini aggregators # correctly preserve thought_signature (extra_content). _sanitize_model = agent.model - if agent.provider == "moa" and moa_config: - _agg = moa_config.get("aggregator") or {} - if _agg.get("model"): - _sanitize_model = _agg["model"] + if agent.provider == "moa": + if moa_config: + _agg = moa_config.get("aggregator") or {} + if _agg.get("model"): + _sanitize_model = _agg["model"] + if _sanitize_model == agent.model: + # Virtual-provider mode: no moa_config is threaded + # through run_conversation — the facade resolves the + # preset internally. Ask the facade for the resolved + # aggregator slot from the previous create() instead + # (set before any history replay that could carry + # thought_signature). + _moa_client = getattr(agent, "client", None) + _agg_slot = getattr(_moa_client, "last_aggregator_slot", None) + if _agg_slot and _agg_slot.get("model"): + _sanitize_model = _agg_slot["model"] agent._sanitize_tool_calls_for_strict_api(api_msg, model=_sanitize_model) # Keep 'reasoning_details' - OpenRouter uses this for multi-turn reasoning context # The signature field helps maintain reasoning continuity diff --git a/contributors/emails/idrisalmalki@Idriss-MacBook-Air.local b/contributors/emails/idrisalmalki@Idriss-MacBook-Air.local new file mode 100644 index 00000000000..8ea8798405e --- /dev/null +++ b/contributors/emails/idrisalmalki@Idriss-MacBook-Air.local @@ -0,0 +1 @@ +quantumbyte1617 diff --git a/tests/run_agent/test_moa_loop_mode.py b/tests/run_agent/test_moa_loop_mode.py index 1be3b488336..1ad39d315c0 100644 --- a/tests/run_agent/test_moa_loop_mode.py +++ b/tests/run_agent/test_moa_loop_mode.py @@ -610,6 +610,98 @@ def test_retry_same_provider_sync_preserves_extra_headers(monkeypatch): assert captured.get("extra_headers") == {"x-initiator": "user"} +def test_moa_gemini_aggregator_sanitize_uses_real_model(monkeypatch, tmp_path): + """MoA turns must sanitize tool_calls against the AGGREGATOR model, not the preset. + + Regression for #66212 / #65092: under MoA, ``agent.model`` holds the + virtual preset name (e.g. "review"), so passing it to + _sanitize_tool_calls_for_strict_api makes + _model_consumes_thought_signature() return False and strips + ``extra_content`` (Gemini thought_signature) from replayed tool_calls — + the Gemini aggregator then 400s with "Function call is missing a + thought_signature in functionCall parts." + """ + home = tmp_path / ".hermes" + home.mkdir() + (home / "config.yaml").write_text( + """ +moa: + default_preset: review + presets: + review: + reference_models: + - provider: openai-codex + model: gpt-5.5 + aggregator: + provider: gemini + model: gemini-3-pro-preview +""".strip(), + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + sanitize_models = [] + + tool_call = SimpleNamespace( + id="call_1", + type="function", + function=SimpleNamespace(name="read_file", arguments='{"path": "x"}'), + ) + + responses = iter( + [ + _response(None, tool_calls=[tool_call]), + _response("aggregator done"), + ] + ) + + def fake_call_llm(**kwargs): + if kwargs["task"] == "moa_reference": + return _response("reference advice") + return next(responses) + + monkeypatch.setattr("agent.moa_loop.call_llm", fake_call_llm) + + agent = AIAgent( + api_key="moa-virtual-provider", + base_url="moa://local", + model="review", + provider="moa", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + enabled_toolsets=["file"], + max_iterations=3, + ) + + real_sanitize = type(agent)._sanitize_tool_calls_for_strict_api + + def spy_sanitize(api_msg, model=None): + sanitize_models.append(model) + return real_sanitize(api_msg, model=model) + + monkeypatch.setattr( + type(agent), "_sanitize_tool_calls_for_strict_api", staticmethod(spy_sanitize) + ) + monkeypatch.setattr( + agent, "execute_tool", lambda *_a, **_k: "file contents", raising=False + ) + + result = agent.run_conversation("read the file") + + assert result["final_response"] == "aggregator done" + # Once the history contains an assistant tool_call turn, the sanitize + # pass must be asked about the REAL aggregator model — never the virtual + # preset name (which would strip Gemini's thought_signature). The very + # first API call may still see the preset (the facade hasn't resolved a + # slot yet), but no tool_calls exist in history at that point. + assert any(m == "gemini-3-pro-preview" for m in sanitize_models), sanitize_models + first_resolved = sanitize_models.index("gemini-3-pro-preview") + assert all( + m == "gemini-3-pro-preview" for m in sanitize_models[first_resolved:] + ), sanitize_models + + def test_moa_slot_runtime_falls_back_on_resolution_error(monkeypatch): """A slot whose provider can't be resolved still attempts the call with the bare provider/model rather than aborting the whole MoA turn."""