From 370a489fb4adb7268f6510439e25e8213bddb624 Mon Sep 17 00:00:00 2001 From: Tranquil-Flow <66773372+Tranquil-Flow@users.noreply.github.com> Date: Mon, 29 Jun 2026 16:25:29 +0200 Subject: [PATCH] fix(auxiliary): floor compression timeout so reasoning models don't fall back to marker (#54915) --- agent/auxiliary_client.py | 32 ++- ...est_auxiliary_compression_timeout_floor.py | 189 ++++++++++++++++++ 2 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 tests/agent/test_auxiliary_compression_timeout_floor.py diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 23d29493a84..22d410b3ffe 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -5657,6 +5657,17 @@ def _resolve_task_provider_model( _DEFAULT_AUX_TIMEOUT = 30.0 +# Compression summarises large conversation histories; a reasoning auxiliary +# model (e.g. Codex / GPT-5.5) can legitimately take longer than the default +# ``auxiliary.compression.timeout`` (120 s), causing the stream to time out and +# the compressor to fall back to the deterministic context marker (#54915). +# This is a bounded *floor* applied only to config-derived compression timeouts +# — it does not affect other auxiliary tasks and does not override an explicit +# per-call ``timeout=``. A floor is harmless for fast compression models +# (they finish before the deadline) and is a minimum, so a higher config value +# is kept unchanged. +_COMPRESSION_TIMEOUT_FLOOR_SECONDS = 300.0 + def _get_auxiliary_task_config(task: str) -> Dict[str, Any]: """Return the config dict for auxiliary., or {} when unavailable. @@ -5716,6 +5727,23 @@ def _get_task_timeout(task: str, default: float = _DEFAULT_AUX_TIMEOUT) -> float return default +def _effective_aux_timeout(task: str, timeout: Optional[float]) -> float: + """Resolve the effective timeout for an auxiliary LLM call. + + Uses the caller-provided ``timeout`` when given; otherwise reads + ``auxiliary.{task}.timeout`` from config via :func:`_get_task_timeout`. + For the ``compression`` task only, applies a bounded floor so a reasoning + model summarising a large context is not cut off by the default timeout + (#54915). The floor is intentionally skipped when the caller passes an + explicit ``timeout=`` — explicit per-call deadlines are always honoured — + and it is a minimum (``max``), so a config value already above it is kept. + """ + effective = timeout if timeout is not None else _get_task_timeout(task) + if timeout is None and task == "compression": + effective = max(effective, _COMPRESSION_TIMEOUT_FLOOR_SECONDS) + return effective + + def _get_task_extra_body(task: str) -> Dict[str, Any]: """Read auxiliary..extra_body and return a shallow copy when valid.""" task_config = _get_auxiliary_task_config(task) @@ -6150,7 +6178,7 @@ def call_llm( f"No LLM provider configured for task={task} provider={resolved_provider}. " f"Run: hermes setup") - effective_timeout = timeout if timeout is not None else _get_task_timeout(task) + effective_timeout = _effective_aux_timeout(task, timeout) # Log what we're about to do — makes auxiliary operations visible _base_info = str(getattr(client, "base_url", resolved_base_url) or "") @@ -6739,7 +6767,7 @@ async def async_call_llm( f"No LLM provider configured for task={task} provider={resolved_provider}. " f"Run: hermes setup") - effective_timeout = timeout if timeout is not None else _get_task_timeout(task) + effective_timeout = _effective_aux_timeout(task, timeout) # Pass the client's actual base_url (not just resolved_base_url) so # endpoint-specific temperature overrides can distinguish diff --git a/tests/agent/test_auxiliary_compression_timeout_floor.py b/tests/agent/test_auxiliary_compression_timeout_floor.py new file mode 100644 index 00000000000..64209ce8068 --- /dev/null +++ b/tests/agent/test_auxiliary_compression_timeout_floor.py @@ -0,0 +1,189 @@ +"""Regression tests for the compression-scoped auxiliary timeout floor (#54915). + +Context compression summarises large conversation histories. When the +resolved auxiliary provider is a reasoning model (e.g. Codex / GPT-5.5) the +summary can legitimately exceed the default ``auxiliary.compression.timeout`` +of 120 s, causing the stream to time out and the compressor to fall back to a +deterministic context marker — silently losing the LLM summary. + +The fix layers a *bounded* timeout floor on top of the config-derived +compression timeout, while honouring the four constraints from the issue: + + * Only the ``compression`` task gets the floor (other auxiliary tasks keep + their own timeouts). + * An explicit per-call ``timeout=`` override is **not** floored. + * The floor is a minimum — a config value already above it is unchanged. + * Both the sync (``call_llm``) and async (``async_call_llm``) paths are + covered. + +These tests exercise the real ``call_llm`` / ``async_call_llm`` production +paths with a mocked LLM client and assert the timeout that actually reaches +``client.chat.completions.create``. +""" + +from unittest.mock import patch, MagicMock, AsyncMock + +import pytest + +from agent.auxiliary_client import call_llm, async_call_llm + +# The committed bounded floor for config-derived compression timeouts. +# Behaviour contract (see AGENTS.md "Behavior contracts over snapshots"): +# compression's effective timeout must be at least this when it is +# config-derived. +COMPRESSION_TIMEOUT_FLOOR = 300.0 + +# The default ``auxiliary.compression.timeout`` shipped in the config schema +# (hermes_cli/config.py). Simulated here as the config-derived value. +COMPRESSION_CONFIG_TIMEOUT = 120.0 + + +def _ok_response(): + return {"ok": True} + + +def _client_sync(): + client = MagicMock() + client.base_url = "https://api.openai.com/v1" + client.chat.completions.create.return_value = _ok_response() + return client + + +def _client_async(): + client = MagicMock() + client.base_url = "https://api.openai.com/v1" + client.chat.completions.create = AsyncMock(return_value=_ok_response()) + return client + + +def _patches(client, *, task_timeout): + """Common mocks: provider resolution, cached client, response validation, + and the config-derived task timeout.""" + return ( + patch("agent.auxiliary_client._resolve_task_provider_model", + return_value=("openai-codex", "gpt-5.5", None, None, None)), + patch("agent.auxiliary_client._get_cached_client", + return_value=(client, "gpt-5.5")), + patch("agent.auxiliary_client._validate_llm_response", + side_effect=lambda resp, _task: resp), + patch("agent.auxiliary_client._get_task_timeout", + return_value=task_timeout), + ) + + +class TestCompressionTimeoutFloorSync: + """Sync ``call_llm`` applies the floor to config-derived compression timeouts.""" + + def test_config_derived_compression_timeout_is_raised_to_floor(self): + """Layer 1: compression with a 120 s config timeout must reach the + client with at least the 300 s floor.""" + client = _client_sync() + p1, p2, p3, p4 = _patches(client, task_timeout=COMPRESSION_CONFIG_TIMEOUT) + with p1, p2, p3, p4: + call_llm( + task="compression", + messages=[{"role": "user", "content": "summarise this"}], + ) + timeout = client.chat.completions.create.call_args.kwargs["timeout"] + assert timeout >= COMPRESSION_TIMEOUT_FLOOR, ( + f"compression timeout {timeout} should be >= floor " + f"{COMPRESSION_TIMEOUT_FLOOR}" + ) + assert timeout > COMPRESSION_CONFIG_TIMEOUT, ( + "the too-low config timeout must not pass through unchanged" + ) + + def test_explicit_per_call_timeout_is_not_floored(self): + """Layer 3: an explicit per-call ``timeout=`` override is honoured + even when it is below the floor.""" + client = _client_sync() + explicit = 60.0 + p1, p2, p3, p4 = _patches(client, task_timeout=COMPRESSION_CONFIG_TIMEOUT) + with p1, p2, p3, p4: + call_llm( + task="compression", + messages=[{"role": "user", "content": "x"}], + timeout=explicit, + ) + timeout = client.chat.completions.create.call_args.kwargs["timeout"] + assert timeout == explicit, ( + f"explicit per-call timeout {explicit} must not be floored, got {timeout}" + ) + + def test_non_compression_task_is_not_floored(self): + """Layer 4: only ``compression`` gets the floor; another auxiliary + task with the same low config timeout must pass it through.""" + client = _client_sync() + low = 30.0 + p1, p2, p3, p4 = _patches(client, task_timeout=low) + with p1, p2, p3, p4: + call_llm( + task="title_generation", + messages=[{"role": "user", "content": "x"}], + ) + timeout = client.chat.completions.create.call_args.kwargs["timeout"] + assert timeout == low, ( + f"non-compression task timeout must stay {low}, got {timeout}" + ) + + def test_higher_config_timeout_is_not_lowered(self): + """Layer 5: the floor is a minimum — a config value already above it + is kept unchanged (``max`` semantics).""" + client = _client_sync() + high = 600.0 + p1, p2, p3, p4 = _patches(client, task_timeout=high) + with p1, p2, p3, p4: + call_llm( + task="compression", + messages=[{"role": "user", "content": "x"}], + ) + timeout = client.chat.completions.create.call_args.kwargs["timeout"] + assert timeout == high, ( + f"config timeout {high} above the floor must be unchanged, got {timeout}" + ) + + +class TestCompressionTimeoutFloorAsync: + """Async ``async_call_llm`` mirrors the sync floor (Layer 2).""" + + @pytest.mark.asyncio + async def test_async_config_derived_compression_timeout_is_raised_to_floor(self): + client = _client_async() + p1, p2, p3, p4 = _patches(client, task_timeout=COMPRESSION_CONFIG_TIMEOUT) + with p1, p2, p3, p4: + await async_call_llm( + task="compression", + messages=[{"role": "user", "content": "summarise this"}], + ) + timeout = client.chat.completions.create.call_args.kwargs["timeout"] + assert timeout >= COMPRESSION_TIMEOUT_FLOOR, ( + f"async compression timeout {timeout} should be >= floor " + f"{COMPRESSION_TIMEOUT_FLOOR}" + ) + + @pytest.mark.asyncio + async def test_async_explicit_per_call_timeout_is_not_floored(self): + client = _client_async() + explicit = 45.0 + p1, p2, p3, p4 = _patches(client, task_timeout=COMPRESSION_CONFIG_TIMEOUT) + with p1, p2, p3, p4: + await async_call_llm( + task="compression", + messages=[{"role": "user", "content": "x"}], + timeout=explicit, + ) + timeout = client.chat.completions.create.call_args.kwargs["timeout"] + assert timeout == explicit + + @pytest.mark.asyncio + async def test_async_non_compression_task_is_not_floored(self): + client = _client_async() + low = 30.0 + p1, p2, p3, p4 = _patches(client, task_timeout=low) + with p1, p2, p3, p4: + await async_call_llm( + task="session_search", + messages=[{"role": "user", "content": "x"}], + ) + timeout = client.chat.completions.create.call_args.kwargs["timeout"] + assert timeout == low