From 7ba9ac6ab75f889928d5c82f345253ad7441dbd4 Mon Sep 17 00:00:00 2001 From: Edizzier Date: Sat, 5 Sep 2026 13:23:55 +0300 Subject: [PATCH] fix(gateway): stop printing sys.maxsize as an iteration ceiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebase of the existing fix onto current main: the god-file split that was in flight when this PR opened has landed, moving both original call sites out of gateway/run.py into gateway/run_busy.py and gateway/run_turn.py, which is why this PR showed a merge conflict. Also folds in a third call site (see below) that a competing PR (#102845, closed by its author in favor of this one) identified after this PR first opened. Three user-facing gateway status lines render "iteration N/M" from AIAgent.get_activity_summary()'s api_call_count / max_iterations pair: the long-running heartbeat (run_turn.GatewayTurnMixin. _run_agent_notify_long_running), the busy-session acknowledgment (run_busy.GatewayBusySessionMixin._compose_busy_ack_message), and the gateway-timeout diagnostic message shown to the user when a run is force-timed-out for inactivity (run_turn.GatewayTurnMixin. _run_agent_timeout_result). AIAgent.max_iterations defaults to sys.maxsize (unlimited tool-calling iterations for a top-level session -- see run_agent.py), so all three printed the literal 9223372036854775807 as the denominator, e.g.: ⏳ Working — 3 min — iteration 2/9223372036854775807, receiving stream response That reads as a bug rather than "unbounded" and is meaningless to a user. Add _format_iteration_progress() to gateway/run.py, a small shared formatting helper: once the configured max_iterations is at or above sys.maxsize, it renders "iteration N" alone and omits the denominator; a genuinely finite budget (e.g. a subagent's delegation. max_iterations: 250) still renders "iteration N/M" as before. All three call sites now go through it. The gateway-timeout diagnostic message's own operator-facing logger.error() call keeps the raw resolved value for diagnostics -- only the two user-facing diag_lines built from it are reformatted. Added tests/gateway/test_format_iteration_progress.py (the helper's own unit tests: unbounded default, above-sentinel values, finite budgets, and graceful handling of a missing/malformed max_iterations), tests/gateway/test_gateway_timeout_iteration_progress.py (both diagnostic-message branches, unbounded and finite, plus the heartbeat call site exercised end to end through its async polling loop), and a new regression test in tests/gateway/test_busy_session_ack.py:: TestBusySessionAck::test_status_detail_omits_denominator_for_unbounded_max_iterations that exercises the busy-ack call site end to end with the real sys.maxsize default and asserts the sentinel never reaches the rendered text. Defect 1 in the original report (a direct_result tool's raw output occasionally becoming the user-visible reply) is left for a separate change -- the reporter frames it as an open core-level design question ("a per-turn suppression option... would fix the class for all plugins"), not a drop-in fix, and it touches reply composition rather than status-line formatting. Part of #102806 (defect 2: the iteration-ceiling display; defect 1 is a separate change). --- gateway/run.py | 26 ++++ gateway/run_busy.py | 7 +- gateway/run_turn.py | 19 ++- tests/gateway/test_busy_session_ack.py | 43 +++++++ .../gateway/test_format_iteration_progress.py | 47 +++++++ ...test_gateway_timeout_iteration_progress.py | 116 ++++++++++++++++++ 6 files changed, 251 insertions(+), 7 deletions(-) create mode 100644 tests/gateway/test_format_iteration_progress.py create mode 100644 tests/gateway/test_gateway_timeout_iteration_progress.py diff --git a/gateway/run.py b/gateway/run.py index 0a62103a77..31f19f2e73 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -940,6 +940,32 @@ def _float_env(name: str, default: float) -> float: return float(default) +def _format_iteration_progress(api_call_count: Any, max_iterations: Any) -> str: + """Render an ``iteration N/M`` status fragment for a running turn. + + Shared by every user-facing status line that formats + ``AIAgent.get_activity_summary()``'s ``api_call_count`` / ``max_iterations`` pair: the + long-running heartbeat (``run_turn._run_agent_notify_long_running``), the busy-session + acknowledgment (``run_busy._handle_active_session_busy_message``), and the + gateway-timeout diagnostic message (``run_turn._run_agent_timeout_result``). + + ``AIAgent.max_iterations`` defaults to ``sys.maxsize`` (unlimited tool-calling + iterations — see ``run_agent.py``), so a top-level session almost never has a real + ceiling. Printing that default verbatim renders the literal ``9223372036854775807`` in + a user-facing status line, which reads as a bug rather than "unbounded" (#102806). Omit + the denominator once the configured ceiling is at or above that sentinel; a genuinely + finite budget (e.g. a subagent's ``delegation.max_iterations: 250``) still prints both + numbers. + """ + try: + _max = int(max_iterations) + except (TypeError, ValueError): + _max = None + if _max is None or _max >= sys.maxsize: + return f"iteration {api_call_count}" + return f"iteration {api_call_count}/{_max}" + + def _stamp_hygiene_compression_provenance( agent: Any, desc: str, provenance: "ActivityProvenance", debug_label: str) -> None: """Best-effort activity provenance stamp for hygiene compression transitions.""" diff --git a/gateway/run_busy.py b/gateway/run_busy.py index 3fcf447eb3..c3b75703b5 100644 --- a/gateway/run_busy.py +++ b/gateway/run_busy.py @@ -534,7 +534,8 @@ class GatewayBusySessionMixin: demoted_for_subagents: bool, demoted_for_compression: bool, ) -> str: from gateway.run import ( - _AGENT_PENDING_SENTINEL, _hermes_home, _load_gateway_config, _platform_config_key + _AGENT_PENDING_SENTINEL, _format_iteration_progress, _hermes_home, + _load_gateway_config, _platform_config_key, ) from gateway.display_config import resolve_display_setting @@ -556,7 +557,9 @@ class GatewayBusySessionMixin: status_parts.append(f"{elapsed_min} min elapsed") if summary.get("max_iterations", 0): status_parts.append( - f"iteration {summary.get('api_call_count', 0)}/{summary.get('max_iterations', 0)}" + _format_iteration_progress( + summary.get("api_call_count", 0), summary.get("max_iterations", 0) + ) ) if summary.get("current_tool"): status_parts.append(f"running: {summary.get('current_tool')}") diff --git a/gateway/run_turn.py b/gateway/run_turn.py index 3b7ee873fc..d5f5aacd42 100644 --- a/gateway/run_turn.py +++ b/gateway/run_turn.py @@ -3156,7 +3156,9 @@ class GatewayTurnMixin: def _run_agent_timeout_result(self, worker, turn_ctx: TurnContext) -> dict: """Synthetic failed run dict for an inactivity timeout, with the activity-tracker diagnostic; interrupts the agent if it is still running so the thread pool worker is freed.""" - from gateway.run import _INTERRUPT_REASON_TIMEOUT, request_hard_interrupt + from gateway.run import ( + _INTERRUPT_REASON_TIMEOUT, _format_iteration_progress, request_hard_interrupt, + ) session_key, result_holder, tools_holder = turn_ctx.session_key, turn_ctx.result_holder, turn_ctx.tools_holder _timed_out_agent = turn_ctx.agent_holder[0] _activity = self._agent_activity_summary(_timed_out_agent) @@ -3165,6 +3167,8 @@ class GatewayTurnMixin: _cur_tool = _activity.get("current_tool") _iter_n = _activity.get("api_call_count", 0) _iter_max = _activity.get("max_iterations", 0) + # Operator-facing log keeps the raw resolved value for diagnostics; only the two + # user-facing _diag_lines below go through _format_iteration_progress (#102806). logger.error( "Agent idle for %.0fs (timeout %.0fs) in session %s " "| last_activity=%s | iteration=%s/%s | tool=%s", @@ -3174,18 +3178,19 @@ class GatewayTurnMixin: if _timed_out_agent: request_hard_interrupt(_timed_out_agent, _INTERRUPT_REASON_TIMEOUT) _timeout_mins = int(worker.agent_timeout // 60) or 1 + _iter_progress = _format_iteration_progress(_iter_n, _iter_max) _diag_lines = [ f"⏱️ Agent inactive for {_timeout_mins} min — no tool calls or API responses." ] if _cur_tool: _diag_lines.append( f"The agent appears stuck on tool `{_cur_tool}` ({_secs_ago:.0f}s since last " - f"activity, iteration {_iter_n}/{_iter_max})." + f"activity, {_iter_progress})." ) else: _diag_lines.append( f"Last activity: {_last_desc} ({_secs_ago:.0f}s ago, " - f"iteration {_iter_n}/{_iter_max}). " + f"{_iter_progress}). " "The agent may have been waiting on an API response." ) _diag_lines.append( @@ -3711,7 +3716,9 @@ class GatewayTurnMixin: Interval: agent.gateway_notify_interval / HERMES_AGENT_NOTIFY_INTERVAL (default 180s; 0 or long_running_notifications=off disables).""" - from gateway.run import _float_env, _interim_metadata, _non_conversational_metadata + from gateway.run import ( + _float_env, _format_iteration_progress, _interim_metadata, _non_conversational_metadata, + ) _notify_start = time.time() _NOTIFY_INTERVAL = _float_env("HERMES_AGENT_NOTIFY_INTERVAL", 180) _long_running_mode = disp._display_surface_mode("long_running_notifications", default=True, allow_generic=True) @@ -3740,7 +3747,9 @@ class GatewayTurnMixin: if _a: _parts = [] if _want_iteration_detail: - _parts.append(f"iteration {_a['api_call_count']}/{_a['max_iterations']}") + _parts.append( + _format_iteration_progress(_a["api_call_count"], _a["max_iterations"]) + ) _action = _a.get("current_tool") or _a.get("last_activity_desc") if _action: _parts.append(str(_action)) diff --git a/tests/gateway/test_busy_session_ack.py b/tests/gateway/test_busy_session_ack.py index fc7beb9aa1..6c4ce15778 100644 --- a/tests/gateway/test_busy_session_ack.py +++ b/tests/gateway/test_busy_session_ack.py @@ -402,6 +402,49 @@ class TestBusySessionAck: assert "terminal" in content # current tool assert "10 min" in content # elapsed + @pytest.mark.asyncio + async def test_status_detail_omits_denominator_for_unbounded_max_iterations( + self, monkeypatch, + ): + """#102806: a top-level session's real max_iterations is sys.maxsize + (unlimited — see AIAgent's default). The busy-ack must not print that + literal sentinel as an iteration ceiling.""" + import sys + + import gateway.run as _gr + + monkeypatch.setattr( + _gr, + "_load_gateway_config", + lambda: {"display": {"platforms": {"telegram": {"busy_ack_detail": True}}}}, + ) + runner, sentinel = _make_runner() + runner._busy_input_mode = "interrupt" + adapter = _make_adapter() + + event = _make_event(text="yo") + sk = build_session_key(event.source) + + agent = MagicMock() + agent.get_activity_summary.return_value = { + "api_call_count": 3, + "max_iterations": sys.maxsize, + "current_tool": "terminal", + "last_activity_ts": time.time(), + "last_activity_desc": "terminal", + "seconds_since_activity": 0.5, + } + runner._running_agents[sk] = agent + runner._running_agents_ts[sk] = time.time() - 600 + runner.adapters[event.source.platform] = adapter + + await runner._handle_active_session_busy_message(event, sk) + + call_kwargs = adapter._send_with_retry.call_args + content = call_kwargs.kwargs.get("content", "") + assert "iteration 3" in content + assert str(sys.maxsize) not in content + class TestBusySessionOnboardingHint: """First-touch hint appended to the busy-ack the first time it fires.""" diff --git a/tests/gateway/test_format_iteration_progress.py b/tests/gateway/test_format_iteration_progress.py new file mode 100644 index 0000000000..bad9c20b39 --- /dev/null +++ b/tests/gateway/test_format_iteration_progress.py @@ -0,0 +1,47 @@ +"""Unit tests for gateway.run._format_iteration_progress (#102806). + +``AIAgent.max_iterations`` defaults to ``sys.maxsize`` (unlimited +tool-calling iterations for a top-level session — see ``run_agent.py``). +Three user-facing gateway status lines render ``iteration N/M`` from +``AIAgent.get_activity_summary()``'s ``api_call_count`` / ``max_iterations`` +pair: the long-running heartbeat, the busy-session acknowledgment, and the +gateway-timeout diagnostic message. All three share this helper; see +``test_busy_session_ack.py`` and ``test_gateway_timeout_iteration_progress.py`` +for the call sites' own end-to-end regression tests. +""" + +import sys + +from gateway.run import _format_iteration_progress + + +class TestFormatIterationProgress: + def test_unbounded_default_omits_denominator(self): + """sys.maxsize (AIAgent's actual default) prints iteration count alone.""" + assert _format_iteration_progress(2, sys.maxsize) == "iteration 2" + + def test_above_sys_maxsize_also_omits_denominator(self): + """Anything at or beyond the sentinel is treated as unbounded too.""" + assert _format_iteration_progress(5, sys.maxsize + 1) == "iteration 5" + + def test_finite_budget_still_shows_both_numbers(self): + """A real, finite ceiling (e.g. a subagent's max_iterations: 250) + keeps the existing N/M format — this is not a blanket format change.""" + assert _format_iteration_progress(7, 250) == "iteration 7/250" + + def test_small_finite_budget(self): + assert _format_iteration_progress(0, 90) == "iteration 0/90" + + def test_non_numeric_max_iterations_falls_back_to_unbounded_rendering(self): + """get_activity_summary() is a best-effort diagnostics snapshot; a + malformed/missing max_iterations must not raise inside a status-line + formatter (call sites wrap this in try/except, but the helper itself + should degrade gracefully rather than propagate a TypeError).""" + assert _format_iteration_progress(3, None) == "iteration 3" + assert _format_iteration_progress(3, "not-a-number") == "iteration 3" + + def test_api_call_count_is_rendered_verbatim(self): + """Only the denominator's unbounded-ness is special-cased; the + numerator (api_call_count) always prints as given.""" + assert _format_iteration_progress(123, 250) == "iteration 123/250" + assert _format_iteration_progress(123, sys.maxsize) == "iteration 123" diff --git a/tests/gateway/test_gateway_timeout_iteration_progress.py b/tests/gateway/test_gateway_timeout_iteration_progress.py new file mode 100644 index 0000000000..5cf5f590ad --- /dev/null +++ b/tests/gateway/test_gateway_timeout_iteration_progress.py @@ -0,0 +1,116 @@ +"""#102806: the gateway inactivity-timeout diagnostic message and the +long-running heartbeat -- the two remaining user-facing render sites +alongside the busy-ack (see test_busy_session_ack.py) -- must not print +sys.maxsize as an iteration ceiling either. + +``_run_agent_timeout_result`` builds the synthetic ``final_response`` shown +to the user when an agent run is force-timed-out for inactivity. It embeds +``iteration N/M`` twice (the "stuck on tool" branch and the "last activity" +branch). ``_run_agent_notify_long_running`` embeds it once, in the periodic +"still working" heartbeat. All three call sites (this file's two classes, +plus test_busy_session_ack.py's busy-ack test) share the same +``_format_iteration_progress`` helper; see test_format_iteration_progress.py +for the helper's own unit tests. +""" + +from __future__ import annotations + +import sys +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.run_turn import GatewayTurnMixin +from gateway.turn_context import TurnContext + + +def _make_worker(agent_timeout=1800.0): + return SimpleNamespace(agent_timeout=agent_timeout) + + +def _make_turn_ctx(agent): + return TurnContext(session_key="sess-1", agent_holder=[agent]) + + +class TestTimeoutDiagnosticIterationProgress: + def test_unbounded_max_iterations_omits_denominator_when_stuck_on_tool(self, monkeypatch): + mixin = GatewayTurnMixin() + monkeypatch.setattr("gateway.run.request_hard_interrupt", MagicMock(), raising=False) + agent = MagicMock() + agent.get_activity_summary.return_value = { + "last_activity_desc": "tool_call", + "seconds_since_activity": 42.0, + "current_tool": "terminal", + "api_call_count": 5, + "max_iterations": sys.maxsize, + } + result = mixin._run_agent_timeout_result(_make_worker(), _make_turn_ctx(agent)) + assert "iteration 5" in result["final_response"] + assert str(sys.maxsize) not in result["final_response"] + + def test_unbounded_max_iterations_omits_denominator_in_last_activity_branch(self, monkeypatch): + mixin = GatewayTurnMixin() + monkeypatch.setattr("gateway.run.request_hard_interrupt", MagicMock(), raising=False) + agent = MagicMock() + agent.get_activity_summary.return_value = { + "last_activity_desc": "api_call_streaming", + "seconds_since_activity": 12.0, + "current_tool": None, + "api_call_count": 2, + "max_iterations": sys.maxsize, + } + result = mixin._run_agent_timeout_result(_make_worker(), _make_turn_ctx(agent)) + assert "iteration 2" in result["final_response"] + assert str(sys.maxsize) not in result["final_response"] + + def test_finite_max_iterations_still_shows_both_numbers(self, monkeypatch): + mixin = GatewayTurnMixin() + monkeypatch.setattr("gateway.run.request_hard_interrupt", MagicMock(), raising=False) + agent = MagicMock() + agent.get_activity_summary.return_value = { + "last_activity_desc": "tool_call", + "seconds_since_activity": 5.0, + "current_tool": "code_exec", + "api_call_count": 7, + "max_iterations": 250, + } + result = mixin._run_agent_timeout_result(_make_worker(), _make_turn_ctx(agent)) + assert "iteration 7/250" in result["final_response"] + + +class TestLongRunningHeartbeatIterationProgress: + """The heartbeat's own render call, exercised end to end through its + async polling loop (one iteration, then the loop is told to stop).""" + + @pytest.mark.asyncio + async def test_heartbeat_omits_denominator_for_unbounded_max_iterations(self, monkeypatch): + monkeypatch.setenv("HERMES_AGENT_NOTIFY_INTERVAL", "0.01") + + mixin = GatewayTurnMixin() + adapter = MagicMock() + adapter.send = AsyncMock(return_value=SimpleNamespace(success=True, message_id="m1")) + mixin._adapter_for_source = MagicMock(return_value=adapter) + mixin._should_emit_long_running_notification = MagicMock(side_effect=[True, False]) + + agent = MagicMock() + agent.get_activity_summary.return_value = { + "api_call_count": 4, + "max_iterations": sys.maxsize, + "current_tool": "terminal", + } + + disp = MagicMock() + disp._display_surface_mode.return_value = "on" + disp.resolve_display_setting.return_value = True + + turn_ctx = TurnContext( + source=SimpleNamespace(chat_id="c1", platform="telegram"), + session_key="sess-1", agent_holder=[agent], + ) + + await mixin._run_agent_notify_long_running(disp, turn_ctx, [None]) + + sent_text = adapter.send.await_args.args[1] + assert "iteration 4" in sent_text + assert str(sys.maxsize) not in sent_text