Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions agent/auxiliary_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5657,6 +5657,17 @@ def _preserve_provider_with_base_url(prov: Optional[str]) -> bool:

_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.<task>, or {} when unavailable.
Expand Down Expand Up @@ -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.<task>.extra_body and return a shallow copy when valid."""
task_config = _get_auxiliary_task_config(task)
Expand Down Expand Up @@ -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 "")
Expand Down Expand Up @@ -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
Expand Down
189 changes: 189 additions & 0 deletions tests/agent/test_auxiliary_compression_timeout_floor.py
Original file line number Diff line number Diff line change
@@ -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
Loading