mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
test(moa): regression for aggregator-model thought_signature resolution (#66212)
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.
This commit is contained in:
parent
16950a4568
commit
74a56b76b0
3 changed files with 109 additions and 4 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
quantumbyte1617
|
||||
|
|
@ -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."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue