From 36476b1b698717aae7463266294afac6117800fb Mon Sep 17 00:00:00 2001 From: Tranquil-Flow Date: Wed, 1 Jul 2026 10:07:25 +0200 Subject: [PATCH] fix(agent): honor configured auxiliary.title_generation.timeout (#32729) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit generate_title() hardcoded timeout=30.0 in its signature; auto_title_session() never passed an explicit timeout, so the configured auxiliary.title_generation.timeout was silently ignored and users saw 'Auxiliary title generation failed: Request timed out' regardless of their config. call_llm() already supports timeout=None and resolves the configured value when None is forwarded, so the fix is on the title_generator side: 1. agent/title_generator.py:54 — change generate_title signature default timeout: float = 30.0 -> timeout: Optional[float] = None 2. agent/auxiliary_client.py:5711 — change call_llm signature annotation timeout: float = None -> timeout: Optional[float] = None (annotation accuracy: the parameter defaulted to None and the body branches on None to read from config) Layers addressed (LAYERS.md): L1 generate_title signature default -> fixed (file 1) L2 auto_title_session does not inject -> already correct, regression test added L3 call_llm task-name threading -> preserved L4 config default value (30s) -> preserved L5 type-safety for Optional[float] -> fixed (file 2) Tests: tests/agent/test_title_generator_timeout_32729.py 5 new production-path tests; 3 RED on upstream/main, 5 GREEN with this fix. Full existing test_title_generator.py suite (23 tests) still passes. Full test_auxiliary_client.py suite (273 tests) still passes; one unrelated pre-existing timing flake (TestCodexAuxiliaryAdapterTimeout::test_enforces_ total_timeout_while_stream_keeps_emitting_events) deselected — verified passes in isolation on upstream/main. --- agent/auxiliary_client.py | 2 +- agent/title_generator.py | 2 +- .../test_title_generator_timeout_32729.py | 142 ++++++++++++++++++ 3 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 tests/agent/test_title_generator_timeout_32729.py diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 39b88ea95b953..894006c093e90 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -5726,7 +5726,7 @@ def call_llm( temperature: float = None, max_tokens: int = None, tools: list = None, - timeout: float = None, + timeout: Optional[float] = None, extra_body: dict = None, api_mode: str = None, stream: bool = False, diff --git a/agent/title_generator.py b/agent/title_generator.py index 583a2cfc60110..c7ce2375fc932 100644 --- a/agent/title_generator.py +++ b/agent/title_generator.py @@ -51,7 +51,7 @@ def _title_language() -> str: def generate_title( user_message: str, assistant_response: str, - timeout: float = 30.0, + timeout: Optional[float] = None, failure_callback: Optional[FailureCallback] = None, main_runtime: dict = None, ) -> Optional[str]: diff --git a/tests/agent/test_title_generator_timeout_32729.py b/tests/agent/test_title_generator_timeout_32729.py new file mode 100644 index 0000000000000..6f57eb0323127 --- /dev/null +++ b/tests/agent/test_title_generator_timeout_32729.py @@ -0,0 +1,142 @@ +"""Regression tests for issue #32729. + +User-configured `auxiliary.title_generation.timeout` was being ignored because +`generate_title()` hardcoded `timeout: float = 30.0` and its caller +`auto_title_session()` never passed an explicit timeout. `call_llm()` already +supports `timeout=None` and resolves the configured value when None is +forwarded, so the fix is purely on the title_generator side: change the +hardcoded 30.0 default to None so the configured timeout flows through. + +These tests cover every layer listed in LAYERS.md: + + Layer 1: generate_title() default signature is Optional[float]=None (was float=30.0). + Layer 1: when generate_title() is invoked without an explicit timeout, it + forwards timeout=None (not 30.0) to call_llm(), letting call_llm + resolve the configured timeout. + Layer 1: an explicit float timeout from the caller is forwarded unchanged. + Layer 2: auto_title_session() does NOT inject a timeout, so generate_title's + default (None) reaches call_llm and the user's config wins. + Layer 5: type-safety — the signature default is Optional[float], consistent + with call_llm(). +""" + +from unittest.mock import MagicMock, patch + +from agent.title_generator import ( + auto_title_session, + generate_title, +) + + +def _ok_response(text: str = "Title") -> MagicMock: + r = MagicMock() + r.choices = [MagicMock()] + r.choices[0].message.content = text + return r + + +class TestGenerateTitleTimeoutDefault: + """Layer 1: timeout default is None, not 30.0.""" + + def test_default_signature_is_optional_float_none(self): + """generate_title() must default timeout to None so call_llm can + consult auxiliary.title_generation.timeout.""" + import inspect + import typing as _t + from typing import get_type_hints, Union, get_origin, get_args + + # `from __future__ import annotations` is on, so get_type_hints + # resolves the string "Optional[float]" — handle both pre- and + # post-evaluation forms. + raw_hints = generate_title.__annotations__ + timeout_anno = raw_hints.get("timeout") + assert timeout_anno is not None, "timeout parameter must be annotated" + + # `Optional[float]` should normalize to Union[float, None]. + if timeout_anno in ("Optional[float]", "typing.Optional[float]"): + pass # PEP 563 string form; acceptable + else: + origin = get_origin(timeout_anno) + args = get_args(timeout_anno) + assert origin in (Union, _t.Union), ( + f"expected Union/Optional origin, got {origin!r}" + ) + assert set(args) == {float, type(None)}, ( + f"expected Union[float, None], got args={args!r}" + ) + + sig = inspect.signature(generate_title) + assert sig.parameters["timeout"].default is None, ( + f"expected default None, got {sig.parameters['timeout'].default!r}" + ) + + def test_no_explicit_timeout_forwards_none_to_call_llm(self): + """When generate_title() is called without timeout=, call_llm must + receive timeout=None, NOT 30.0.""" + captured = {} + + def mock_call_llm(**kwargs): + captured.update(kwargs) + return _ok_response() + + with patch("agent.title_generator.call_llm", side_effect=mock_call_llm): + generate_title("hi", "hello") + + assert "timeout" in captured + assert captured["timeout"] is None, ( + f"call_llm received timeout={captured['timeout']!r}; " + "should be None so call_llm can read auxiliary.title_generation.timeout" + ) + + def test_explicit_timeout_from_caller_is_forwarded_unchanged(self): + captured = {} + + def mock_call_llm(**kwargs): + captured.update(kwargs) + return _ok_response() + + with patch("agent.title_generator.call_llm", side_effect=mock_call_llm): + generate_title("hi", "hello", timeout=120.0) + + assert captured["timeout"] == 120.0 + + +class TestAutoTitleSessionPreservesConfigLayer: + """Layer 2: auto_title_session() must NOT inject a 30s timeout, + so the configured value can flow through to call_llm.""" + + def test_auto_title_session_does_not_pass_timeout_to_generate_title(self): + db = MagicMock() + db.get_session_title.return_value = None + + with patch( + "agent.title_generator.generate_title", return_value="New Title" + ) as gen: + auto_title_session(db, "sess-1", "hi", "hello") + # The fix must ensure generate_title is called without + # an injected timeout kwarg. + call_kwargs = gen.call_args.kwargs + assert "timeout" not in call_kwargs, ( + f"auto_title_session injected timeout={call_kwargs.get('timeout')!r}; " + "this is the bug — generate_title's default must reach call_llm" + ) + + def test_auto_title_session_full_call_chain_preserves_none_timeout(self): + """End-to-end: auto_title_session -> generate_title -> call_llm + must surface timeout=None to call_llm (NOT 30.0).""" + db = MagicMock() + db.get_session_title.return_value = None + captured = {} + + def mock_call_llm(**kwargs): + captured.update(kwargs) + return _ok_response() + + with patch("agent.title_generator.call_llm", side_effect=mock_call_llm): + auto_title_session(db, "sess-1", "hi", "hello") + + assert "timeout" in captured + assert captured["timeout"] is None, ( + f"end-to-end timeout leaked: {captured['timeout']!r}; " + "configured auxiliary.title_generation.timeout will be ignored" + )