fix(gateway): stop printing sys.maxsize as an iteration ceiling
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).
This commit is contained in:
@@ -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."""
|
||||
|
||||
+5
-2
@@ -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')}")
|
||||
|
||||
+14
-5
@@ -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))
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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"
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user