fix: route the curator review fork and gateway /compress agent through resolve_reasoning_config
Two more production AIAgent() sites (same class as #85153) never passed reasoning_config, so agent.reasoning_effort was ignored there. The /compress agent uses the gateway's session-aware resolver (session /reasoning > per-model > global), the curator the shared chokepoint against its review model. Also pins the Feishu comment-agent sibling shipped earlier on this branch with a test.
This commit is contained in:
@@ -1047,10 +1047,16 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]:
|
||||
acp_command = rp.get("command")
|
||||
if isinstance(acp_command, str) and acp_command:
|
||||
agent_kwargs.update(acp_command=acp_command, acp_args=list(rp.get("args") or []))
|
||||
from hermes_cli.config import load_config_readonly
|
||||
from hermes_constants import resolve_reasoning_config
|
||||
|
||||
review_agent = AIAgent(
|
||||
model=model_name, provider=provider, api_key=rp.get("api_key"), base_url=rp.get("base_url"),
|
||||
api_mode=rp.get("api_mode"), credential_pool=rp.get("credential_pool"),
|
||||
request_overrides=request_overrides, **agent_kwargs,
|
||||
# Same chokepoint as every other surface: without it ``agent.reasoning_effort`` never reaches
|
||||
# the review fork and the transport applies its default effort (a 400 on non-reasoning models).
|
||||
reasoning_config=resolve_reasoning_config(load_config_readonly(), model_name),
|
||||
# No ``terminal``: a shell mv/cp/rm under the skills tree writes bytes
|
||||
# with NO ledger entry, so rollback would restore a hollow skill. Every
|
||||
# mutation goes through ledgered skill_manage; dropping the toolset
|
||||
|
||||
@@ -545,6 +545,9 @@ class GatewaySessionCommandsMixin:
|
||||
if platform_key is not None:
|
||||
runtime_kwargs["platform"] = platform_key
|
||||
runtime_kwargs["gateway_session_key"] = session_key
|
||||
# Same reasoning setting as a live turn (session ``/reasoning`` > per-model > global): without it
|
||||
# the transport applies its default effort — a 400 on non-reasoning models.
|
||||
runtime_kwargs["reasoning_config"] = self._resolve_session_reasoning_config(source=source, model=model)
|
||||
|
||||
tmp_agent = await self._build_manual_compression_agent(session_entry.session_id, model, runtime_kwargs)
|
||||
try:
|
||||
|
||||
@@ -946,6 +946,44 @@ def test_review_fork_forwards_runtime_pool_and_overrides(curator_env, monkeypatc
|
||||
assert captured["kwargs"]["request_overrides"] == fake_overrides
|
||||
|
||||
|
||||
def test_review_fork_receives_configured_reasoning(curator_env, monkeypatch):
|
||||
"""#85153 class: the curator's review fork is an ``AIAgent()`` built from config, so ``agent.reasoning_effort``
|
||||
must reach it through the shared ``resolve_reasoning_config`` chokepoint (resolved against the review model)."""
|
||||
curator = curator_env["curator"]
|
||||
import importlib
|
||||
importlib.reload(curator)
|
||||
captured = {}
|
||||
cfg = {"model": {"provider": "openai-api", "default": "gpt-4o-mini"}, "agent": {"reasoning_effort": "none"}}
|
||||
|
||||
class _StubAgent:
|
||||
def __init__(self, *args, **kwargs):
|
||||
captured["kwargs"] = kwargs
|
||||
self._memory_write_origin = "assistant_tool"
|
||||
self._memory_nudge_interval = 0
|
||||
self._skill_nudge_interval = 0
|
||||
self._session_messages = []
|
||||
|
||||
def run_conversation(self, user_message=None, **kwargs):
|
||||
return {"final_response": "ok"}
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr("hermes_cli.config.load_config", lambda: cfg)
|
||||
monkeypatch.setattr("hermes_cli.config.load_config_readonly", lambda: cfg)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kwargs: {"provider": "openai-api", "api_key": "k", "base_url": "https://api.openai.com/v1",
|
||||
"api_mode": "codex_responses"},
|
||||
)
|
||||
monkeypatch.setattr("run_agent.AIAgent", _StubAgent)
|
||||
|
||||
meta = curator._run_llm_review("review prompt")
|
||||
|
||||
assert meta.get("error") is None, meta.get("error")
|
||||
assert captured["kwargs"]["reasoning_config"] == {"enabled": False}
|
||||
|
||||
|
||||
def test_review_fork_uses_runtime_model_and_output_cap(curator_env, monkeypatch):
|
||||
curator = curator_env["curator"]
|
||||
import importlib
|
||||
|
||||
@@ -267,6 +267,37 @@ async def test_compress_command_preserves_platform_and_gateway_session_key():
|
||||
assert kwargs["gateway_session_key"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_compress_command_agent_receives_configured_reasoning():
|
||||
"""#85153 class: the throwaway /compress agent is an ``AIAgent()`` built from gateway config, so
|
||||
``agent.reasoning_effort: none`` must reach it like a normal gateway turn — otherwise the transport
|
||||
applies its default effort (a 400 on non-reasoning models)."""
|
||||
history = _make_history()
|
||||
runner = _make_runner(history)
|
||||
agent_instance = MagicMock()
|
||||
agent_instance.shutdown_memory_provider = MagicMock()
|
||||
agent_instance.close = MagicMock()
|
||||
agent_instance._cached_system_prompt = ""
|
||||
agent_instance.tools = None
|
||||
agent_instance.context_compressor.has_content_to_compress.return_value = True
|
||||
agent_instance.session_id = "sess-1"
|
||||
agent_instance._compress_context.return_value = (list(history), "")
|
||||
agent_instance._compression_skipped_due_to_lock = False
|
||||
|
||||
with (
|
||||
patch("gateway.run._load_gateway_config", return_value={"agent": {"reasoning_effort": "none"}}),
|
||||
patch("gateway.run._resolve_runtime_agent_kwargs", return_value={"api_key": "test-key"}),
|
||||
patch("gateway.run._resolve_gateway_model", return_value="gpt-4o-mini"),
|
||||
patch("run_agent.AIAgent", return_value=agent_instance) as mock_agent,
|
||||
patch("agent.model_metadata.estimate_request_tokens_rough", return_value=100),
|
||||
):
|
||||
await runner._handle_compress_command(_make_event())
|
||||
|
||||
assert mock_agent.call_count == 1
|
||||
_, kwargs = mock_agent.call_args
|
||||
assert kwargs["reasoning_config"] == {"enabled": False}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_compress_command_passes_tool_messages_to_compressor():
|
||||
"""Tool results must reach _compress_context (#3854).
|
||||
|
||||
@@ -8,6 +8,7 @@ from unittest.mock import AsyncMock, Mock, patch
|
||||
from plugins.platforms.feishu.feishu_comment import (
|
||||
parse_drive_comment_event,
|
||||
_ALLOWED_NOTICE_TYPES,
|
||||
_resolve_model_and_runtime,
|
||||
_sanitize_comment_text,
|
||||
)
|
||||
|
||||
@@ -138,5 +139,24 @@ class TestWikiReverseLookup(unittest.TestCase):
|
||||
self.assertEqual(query_dict["obj_type"], "docx")
|
||||
|
||||
|
||||
class TestResolveModelAndRuntime(unittest.TestCase):
|
||||
def test_configured_reasoning_reaches_the_comment_agent(self):
|
||||
"""#85153 sibling: the comment agent is an ``AIAgent()`` built from gateway config like every other
|
||||
surface, so ``agent.reasoning_effort: none`` must ride ``runtime_kwargs`` (resolved against the
|
||||
comment agent's model, so per-model overrides apply)."""
|
||||
cfg = {"agent": {"reasoning_effort": "none", "reasoning_overrides": {"gpt-5.6": "high"}}}
|
||||
with patch("gateway.run._load_gateway_config", return_value=cfg), \
|
||||
patch("gateway.run._resolve_gateway_model", return_value="gpt-4o-mini"), \
|
||||
patch("gateway.run._resolve_runtime_agent_kwargs", return_value={"provider": "openai-api", "api_key": "k"}):
|
||||
model, runtime_kwargs = _resolve_model_and_runtime()
|
||||
self.assertEqual(model, "gpt-4o-mini")
|
||||
self.assertEqual(runtime_kwargs["reasoning_config"], {"enabled": False})
|
||||
with patch("gateway.run._load_gateway_config", return_value=cfg), \
|
||||
patch("gateway.run._resolve_gateway_model", return_value="gpt-5.6"), \
|
||||
patch("gateway.run._resolve_runtime_agent_kwargs", return_value={"provider": "openai-api", "api_key": "k"}):
|
||||
_model, runtime_kwargs = _resolve_model_and_runtime()
|
||||
self.assertEqual(runtime_kwargs["reasoning_config"], {"enabled": True, "effort": "high"})
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user