mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(auxiliary): floor compression timeout so reasoning models don't fall back to marker (#54915)
This commit is contained in:
parent
5e685999af
commit
370a489fb4
2 changed files with 219 additions and 2 deletions
|
|
@ -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.<task>, 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.<task>.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
|
||||
|
|
|
|||
189
tests/agent/test_auxiliary_compression_timeout_floor.py
Normal file
189
tests/agent/test_auxiliary_compression_timeout_floor.py
Normal file
|
|
@ -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…
Add table
Add a link
Reference in a new issue