diff --git a/agent/background_review.py b/agent/background_review.py index 065c18624e..9627bc92bc 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -844,7 +844,27 @@ def _detach_fork_compression(review_agent: Any) -> None: review_agent._review_defer_compaction_before_first_response = True -def _fork_init_kwargs(agent: Any, rt: Dict[str, Any], routed: bool, max_iterations: int) -> Dict[str, Any]: +def _routed_reasoning_config(task_cfg: Optional[Dict[str, Any]]) -> Optional[Dict[str, Any]]: + """``reasoning_config`` for a ROUTED fork from ``auxiliary.background_review.reasoning_effort`` + (#94825). The routed branch never inherits the parent's effort (its vocabulary may be invalid for + the routed provider), but an explicit per-task pin is the user's choice for THAT model and must + win over provider defaults, as every other aux task already does via ``_get_task_extra_body``. + None = unset (provider default); an unknown level warns and falls through to the default.""" + effort = _background_review_task_config(task_cfg).get("reasoning_effort") + if effort is None or effort == "": + return None + from hermes_constants import VALID_REASONING_EFFORTS, parse_reasoning_effort + parsed = parse_reasoning_effort(effort) + if parsed is None: + logger.warning( + "auxiliary.background_review.reasoning_effort %r is not a valid level (none, %s) — using " + "the routed provider's default", effort, ", ".join(VALID_REASONING_EFFORTS), + ) + return parsed + + +def _fork_init_kwargs(agent: Any, rt: Dict[str, Any], routed: bool, max_iterations: int, + task_cfg: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: """AIAgent constructor kwargs for the review fork. skip_memory=True: an external memory plugin scoped to the parent's session_id would leak the harness prompt into the user's real memory namespace; built-in MEMORY.md/USER.md state is re-bound by the caller. Toolsets match the @@ -865,6 +885,8 @@ def _fork_init_kwargs(agent: Any, rt: Dict[str, Any], routed: bool, max_iteratio kwargs.update(acp_command=rt["command"], acp_args=rt.get("args") or []) if not routed: kwargs.update(_same_model_parity_kwargs(agent)) + elif (routed_cfg := _routed_reasoning_config(task_cfg)) is not None: + kwargs["reasoning_config"] = routed_cfg return kwargs @@ -906,7 +928,7 @@ def build_cache_parity_fork( # separate, tracked issue (#94825) and are left alone. if not _routed and write_origin == "background_review": _warn_ignored_reasoning_effort(agent, task_cfg) - review_agent = AIAgent(**_fork_init_kwargs(agent, _rt, _routed, max_iterations)) + review_agent = AIAgent(**_fork_init_kwargs(agent, _rt, _routed, max_iterations, task_cfg)) review_agent._memory_write_origin = review_agent._memory_write_context = write_origin review_agent._memory_store = agent._memory_store review_agent._memory_enabled = agent._memory_enabled diff --git a/tests/agent/test_background_review_routed_effort.py b/tests/agent/test_background_review_routed_effort.py new file mode 100644 index 0000000000..8435ae5f3f --- /dev/null +++ b/tests/agent/test_background_review_routed_effort.py @@ -0,0 +1,50 @@ +"""Routed background reviews honor ``auxiliary.background_review.reasoning_effort`` (#94825). + +The review fork is a full AIAgent, not an auxiliary_client call. The routed branch deliberately +skips the PARENT's reasoning_config (its effort vocabulary may be invalid for the routed model), +but an explicitly configured per-task effort must win over provider defaults — mirroring how every +other auxiliary task folds the same key into ``extra_body.reasoning``. +""" +from __future__ import annotations + +import logging +from unittest.mock import patch + +import run_agent +import agent.background_review as bg_review +from agent.background_review import build_cache_parity_fork + +from tests.agent.test_background_review_cache_parity import _make_agent_stub, _make_recorder_class + +ROUTED_RUNTIME = { + "provider": "openrouter", "model": "aux-cheap-model", "api_key": "test-key", + "base_url": None, "api_mode": None, "credential_pool": None, "request_overrides": {}, + "max_tokens": None, "command": None, "args": [], "routed": True, +} + + +def _routed_fork_kwargs(task_cfg): + captured = {} + agent = _make_agent_stub(run_agent.AIAgent) + agent.reasoning_config = {"enabled": True, "effort": "high"} + with patch.object(run_agent, "AIAgent", _make_recorder_class(captured)), \ + patch.object(bg_review, "_resolve_review_runtime", return_value=ROUTED_RUNTIME): + _fork, _rt, routed = build_cache_parity_fork(agent, task_cfg, max_iterations=5) + assert routed + return captured["init_kwargs"] + + +def test_routed_review_applies_configured_effort_not_parents(): + kwargs = _routed_fork_kwargs({"reasoning_effort": "xhigh"}) + assert kwargs["reasoning_config"] == {"enabled": True, "effort": "xhigh"} + # ``none`` disables thinking on the routed fork, same vocabulary as every other aux task. + assert _routed_fork_kwargs({"reasoning_effort": "none"})["reasoning_config"] == {"enabled": False} + + +def test_routed_review_falls_back_to_provider_default(caplog): + # Unset: provider default, and the parent's ``high`` is NOT smuggled across the route. + assert "reasoning_config" not in _routed_fork_kwargs({"reasoning_effort": ""}) + assert "reasoning_config" not in _routed_fork_kwargs({}) + with caplog.at_level(logging.WARNING): + assert "reasoning_config" not in _routed_fork_kwargs({"reasoning_effort": "ludicrous"}) + assert "ludicrous" in caplog.text