From 80e3574ecadb7e249e92c4bf24d8a840a364795f Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Fri, 7 Aug 2026 22:47:51 -0700 Subject: [PATCH] Release exp-v0.52.177: scope aux-title test module stubs (#6661) (#6848) * test(#6630): isolate auxiliary client fixtures * test(#6630): route both aux-title suites through one scoped module stub * test(#6630): drop dead import and record the patch.dict restore constraint * Release exp-v0.52.177: scope auxiliary-title test module stubs, stop cross-file sys.modules leak (#6661) --------- Co-authored-by: Rod Boev Co-authored-by: nesquena-hermes --- CHANGELOG.md | 2 + tests/_aux_client_helpers.py | 52 ++++++++++++++++++++ tests/test_2235_initial_aux_title.py | 25 ++++++---- tests/test_title_aux_routing.py | 71 ++++++++++++++++++++++++---- 4 files changed, 132 insertions(+), 18 deletions(-) create mode 100644 tests/_aux_client_helpers.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a45e656..8082ad90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ - **Internal: removed a dead `rowIndex` parameter from the settled-scene row projector.** `attachLiveStream`'s `pushRow` helper carried an unused positional index left over from before final-segment eligibility moved to a row-identity `WeakSet`; dropping it is a pure no-op refactor (differential execution confirms byte-identical settled scenes — ordering, dedupe, final-prefix suppression, and sequence assignment all unchanged). Thanks @webtecnica. (#6258) +- **Internal: auxiliary-title test suites no longer leak a synthetic `agent` module across the process.** `test_title_aux_routing.py` and `test_2235_initial_aux_title.py` installed stub `agent` / `agent.auxiliary_client` modules via `sys.modules.setdefault` at import time and never removed them, so any later test importing `agent.context_compressor` in the same process got the stub and failed (breaking `test_issue4685_post_compression_context_metering.py` by run order). Both suites now route the install through a shared `tests/_aux_client_helpers.py` chokepoint scoped with `unittest.mock.patch.dict`, plus four tests covering restore-by-identity, absent-key cleanup, present-but-`None` preservation, and restore-on-raise. Test-only, no production change. Thanks @rodboev. (#6661, #6630) + - **Internal: clarified the transcript-virtualization boot comment (opt-in default retained).** The comment above the boot-time `_virtualizeTranscript` apply now records that the #4346 Phase B footer-jitter fix resolved the original scroll-up flicker root cause, while virtualization stays opt-in (default OFF) until battle-tested further. Zero behavior change — the apply predicate, settings-load fallback, checkbox, and server default all remain opt-in/OFF and in agreement. Thanks @webtecnica. (#6318, #6155) - **Internal: added regression coverage for the empty-`tool_calls` drop on the metadata-restoration mirror path.** `_api_safe_message_positions()` must apply the same `tool_calls: []` drop as the main API sanitizer (#5737) so metadata restoration never treats a `tool_calls: []` row as a distinct message; a new test pins that empty arrays lose the key while populated tool-call chains (and their tool results) survive untouched. Test-only, no production change. Thanks @webtecnica. (#6803, #6796) diff --git a/tests/_aux_client_helpers.py b/tests/_aux_client_helpers.py new file mode 100644 index 00000000..68cd493d --- /dev/null +++ b/tests/_aux_client_helpers.py @@ -0,0 +1,52 @@ +"""Shared scaffolding for the auxiliary title-generation test suites. + +``test_title_aux_routing.py`` and ``test_2235_initial_aux_title.py`` both need +``agent.auxiliary_client`` importable so ``api.streaming`` can be patched +against it. Both used to install synthetic modules at import time with +``sys.modules.setdefault`` and never remove them, so whichever ran first left +the stub in place for the rest of the process and the next file to import +``agent.context_compressor`` got the stub instead of the real module (#6630). + +Scope the install to the test that needs it and let ``mock.patch.dict`` own the +restore: it snapshots the mapping and puts it back on exit, so a key that was +absent is deleted, a key that was present is restored by identity, and a key +holding ``None`` stays ``None``. ``tests/test_4413_seed_provider_models.py`` +already stubs ``sys.modules`` this way, including the ``None`` case. + +One constraint that comes with it: ``patch.dict`` restores by clearing the +mapping and replacing it wholesale, so any module first imported inside the +scope is evicted on exit and re-imported later as a new object. That is +harmless for these suites, which import everything they touch at collection +time, but it is the thing to check before reusing this helper around code +that imports lazily. +""" +from __future__ import annotations + +import sys +import types +from unittest import mock + + +def auxiliary_client_modules(): + """Context manager installing stub ``agent`` / ``agent.auxiliary_client``. + + The stubs are fresh objects owned by this helper, so the ``auxiliary_client`` + attribute is set on our own module and no pre-existing ``agent`` module is + mutated. Restoration is therefore entirely ``patch.dict``'s job. + """ + agent_stub = types.ModuleType("agent") + auxiliary_client_stub = types.ModuleType("agent.auxiliary_client") + agent_stub.auxiliary_client = auxiliary_client_stub + return mock.patch.dict( + sys.modules, + {"agent": agent_stub, "agent.auxiliary_client": auxiliary_client_stub}, + ) + + +def patch_tg_config(config_dict): + """Make ``_get_auxiliary_task_config`` return ``config_dict``.""" + return mock.patch( + "agent.auxiliary_client._get_auxiliary_task_config", + return_value=config_dict, + create=True, + ) diff --git a/tests/test_2235_initial_aux_title.py b/tests/test_2235_initial_aux_title.py index 720b22d0..a5024a6b 100644 --- a/tests/test_2235_initial_aux_title.py +++ b/tests/test_2235_initial_aux_title.py @@ -10,24 +10,29 @@ Covers: 3. Refresh path parity — configured aux routing still applies to the adaptive title refresh path. """ -import sys import threading -import types import unittest from unittest.mock import MagicMock, patch -# Stub agent.auxiliary_client so it is importable in the test environment -# (the real package lives in hermes-agent, which is not installed here). -_agent_stub = types.ModuleType('agent') -_aux_stub = types.ModuleType('agent.auxiliary_client') -sys.modules.setdefault('agent', _agent_stub) -sys.modules.setdefault('agent.auxiliary_client', _aux_stub) -_agent_stub.auxiliary_client = _aux_stub +import pytest + +from tests._aux_client_helpers import auxiliary_client_modules, patch_tg_config + + +@pytest.fixture(autouse=True) +def _install_auxiliary_client_modules(): + """Scope the synthetic hermes-agent modules to each test (#6630). + + This file carried the same unscoped sys.modules.setdefault install as + test_title_aux_routing.py, so it leaked the stub the same way. + """ + with auxiliary_client_modules(): + yield def _patch_tg_config(config_dict): """Return a patch context manager that makes _get_auxiliary_task_config return config_dict.""" - return patch('agent.auxiliary_client._get_auxiliary_task_config', return_value=config_dict, create=True) + return patch_tg_config(config_dict) def _make_provisional_session(user_text, assistant_text='Here is the answer.'): diff --git a/tests/test_title_aux_routing.py b/tests/test_title_aux_routing.py index 24c927ca..5b643b79 100644 --- a/tests/test_title_aux_routing.py +++ b/tests/test_title_aux_routing.py @@ -13,18 +13,73 @@ import types import unittest from unittest.mock import MagicMock, patch -# Stub agent.auxiliary_client so it is importable in the test environment -# (the real package lives in hermes-agent, which is not installed here). -_agent_stub = types.ModuleType('agent') -_aux_stub = types.ModuleType('agent.auxiliary_client') -sys.modules.setdefault('agent', _agent_stub) -sys.modules.setdefault('agent.auxiliary_client', _aux_stub) -_agent_stub.auxiliary_client = _aux_stub +import pytest + +from tests._aux_client_helpers import auxiliary_client_modules, patch_tg_config + + +@pytest.fixture(autouse=True) +def _install_auxiliary_client_modules(): + """Scope the synthetic hermes-agent modules to each test (#6630).""" + with auxiliary_client_modules(): + yield def _patch_tg_config(config_dict): """Return a patch context manager that makes _get_auxiliary_task_config return config_dict.""" - return patch('agent.auxiliary_client._get_auxiliary_task_config', return_value=config_dict, create=True) + return patch_tg_config(config_dict) + + +class TestAuxiliaryClientModuleRestore(unittest.TestCase): + """Restoration properties the #6630 fix depends on. + + These drive the helper directly. The fixture boundary itself is proved by + the cross-file run in the PR description: running this file before + tests/test_issue4685_post_compression_context_metering.py used to fail that + file's agent.context_compressor import and no longer does. + """ + + def test_prior_modules_restored_by_identity(self): + prior_agent = types.ModuleType("agent") + prior_aux = types.ModuleType("agent.auxiliary_client") + prior_agent.auxiliary_client = prior_aux + with patch.dict(sys.modules, {"agent": prior_agent, "agent.auxiliary_client": prior_aux}): + with auxiliary_client_modules(): + self.assertIsNot(sys.modules["agent"], prior_agent) + self.assertIsNot(sys.modules["agent.auxiliary_client"], prior_aux) + self.assertIs(sys.modules["agent"], prior_agent) + self.assertIs(sys.modules["agent.auxiliary_client"], prior_aux) + self.assertIs(prior_agent.auxiliary_client, prior_aux) + + def test_absent_keys_removed(self): + with patch.dict(sys.modules): + sys.modules.pop("agent", None) + sys.modules.pop("agent.auxiliary_client", None) + with auxiliary_client_modules(): + self.assertIn("agent", sys.modules) + self.assertIn("agent.auxiliary_client", sys.modules) + self.assertNotIn("agent", sys.modules) + self.assertNotIn("agent.auxiliary_client", sys.modules) + + def test_prior_none_value_preserved(self): + """A present-but-None entry is not an absent entry.""" + with patch.dict(sys.modules, {"agent": None, "agent.auxiliary_client": None}): + with auxiliary_client_modules(): + self.assertIsNotNone(sys.modules["agent"]) + self.assertIn("agent", sys.modules) + self.assertIsNone(sys.modules["agent"]) + self.assertIsNone(sys.modules["agent.auxiliary_client"]) + + def test_restored_after_exception(self): + prior_agent = types.ModuleType("agent") + prior_aux = types.ModuleType("agent.auxiliary_client") + prior_agent.auxiliary_client = prior_aux + with patch.dict(sys.modules, {"agent": prior_agent, "agent.auxiliary_client": prior_aux}): + with self.assertRaisesRegex(RuntimeError, "boom"): + with auxiliary_client_modules(): + raise RuntimeError("boom") + self.assertIs(sys.modules["agent"], prior_agent) + self.assertIs(sys.modules["agent.auxiliary_client"], prior_aux) class TestAuxTitleConfigured(unittest.TestCase):