From 02a54e01ced45d6df200322b575e6cf711f143ca Mon Sep 17 00:00:00 2001 From: yoniebans Date: Wed, 13 May 2026 11:44:29 +0200 Subject: [PATCH] feat(session_search): user-configurable default_mode via config.yaml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default mode is normally 'summary' (LLM recap of matched sessions). This commit lets a user override that via: # ~/.hermes/config.yaml tools: session_search: default_mode: fast Useful for power users who want to live with fast-as-default for a few days and see how it feels — without having to pass mode='fast' on every call. The summary path is still one explicit kwarg away. Resolution order at call time: 1. Explicit mode= argument from the LLM (always wins) 2. tools.session_search.default_mode in ~/.hermes/config.yaml 3. 'summary' (final fallback) Implementation: - New helper _resolve_user_default_mode() in tools/session_search_tool.py reads the value via hermes_cli.config.load_config(). Wrapped in functools.lru_cache so the YAML read happens at most once per process (config changes need a CLI / TUI restart, which is the existing convention). - Validates: must be a string, must be 'fast' or 'summary'. Anything else (including 'guided', which needs anchors and can't stand alone) logs a warning and falls back to 'summary'. The user gets feedback when they typo their config. - session_search()'s mode normaliser checks for None/empty/non-string first and resolves the user default before applying alias mapping. Explicit modes still take precedence over config. - Both dispatch sites in run_agent.py changed from mode=function_args.get('mode', 'summary') → mode=function_args.get('mode'). Hardcoding 'summary' at dispatch would shadow the new config-default layer. Added a guard assert in test_run_agent_special_session_search_paths_forward_mode so a regression to the old shape fails loudly. - Schema description gets one extra sentence acknowledging the user-configurable default so the LLM's own description of the tool reflects reality. Tests (+8): - test_unset_mode_falls_back_to_summary_when_config_missing - test_user_can_configure_fast_as_default - test_user_can_configure_summary_as_default_explicitly - test_invalid_default_mode_warns_and_falls_back (typo test) - test_guided_as_default_mode_is_rejected - test_non_string_default_mode_falls_back (bogus YAML types) - test_explicit_mode_argument_overrides_user_default - test_unset_mode_with_config_default_fast_runs_fast_path (e2e) 93/93 session_search + get_messages_around tests passing. This is thread 2 of the prompt-tuning / default-mode plan from the spike: thread 1 was the schema-description iteration (still in progress on the spike page); thread 2 lets users carry the experiment around in their own config while we converge on whether to flip the global default in the schema. --- run_agent.py | 4 +- tests/tools/test_session_search.py | 140 ++++++++++++++++++++++++++++- tools/session_search_tool.py | 70 ++++++++++++++- 3 files changed, 208 insertions(+), 6 deletions(-) diff --git a/run_agent.py b/run_agent.py index 4909697785..1048f2a937 100644 --- a/run_agent.py +++ b/run_agent.py @@ -10295,7 +10295,7 @@ class AIAgent: limit=function_args.get("limit", 3), db=session_db, current_session_id=self.session_id, - mode=function_args.get("mode", "summary"), + mode=function_args.get("mode"), session_id=function_args.get("session_id"), around_message_id=function_args.get("around_message_id"), window=function_args.get("window", 5), @@ -10926,7 +10926,7 @@ class AIAgent: limit=function_args.get("limit", 3), db=session_db, current_session_id=self.session_id, - mode=function_args.get("mode", "summary"), + mode=function_args.get("mode"), session_id=function_args.get("session_id"), around_message_id=function_args.get("around_message_id"), window=function_args.get("window", 5), diff --git a/tests/tools/test_session_search.py b/tests/tools/test_session_search.py index 5c0ccf4038..d805124417 100644 --- a/tests/tools/test_session_search.py +++ b/tests/tools/test_session_search.py @@ -633,17 +633,153 @@ class TestSessionSearch: mock_db.search_messages.assert_called_once() def test_run_agent_special_session_search_paths_forward_mode(self): - """run_agent has two direct session_search call sites outside registry dispatch.""" + """run_agent has two direct session_search call sites outside registry dispatch. + + Both dispatch sites now pass ``mode=function_args.get("mode")`` (no + hardcoded "summary" fallback) so that an unset mode flows through to + the tool's normaliser, which resolves the user-configured default via + ``_resolve_user_default_mode()``. Hardcoding "summary" at the dispatch + layer would silently shadow that config. + """ from pathlib import Path source = (Path(__file__).parent.parent.parent / "run_agent.py").read_text() # Both dispatch sites pass mode= as their next-to-last group of kwargs; # the new guided-mode kwargs (session_id/around_message_id/window) follow. - assert source.count('mode=function_args.get("mode", "summary")') == 2 + assert source.count('mode=function_args.get("mode")') == 2 # And both dispatch sites carry the guided-mode handles assert source.count('around_message_id=function_args.get("around_message_id")') == 2 assert source.count('window=function_args.get("window", 5)') == 2 assert source.count('anchors=function_args.get("anchors")') == 2 + # Guard against a regression to hardcoded "summary" — the config-default + # plumbing only works if dispatch doesn't shadow None with "summary". + assert 'mode=function_args.get("mode", "summary")' not in source, ( + "dispatch sites must pass mode=function_args.get(\"mode\") (no default) " + "so the user-configured default_mode can take effect" + ) + + # ----------------------------------------------------------------- + # User-configurable default mode (tools.session_search.default_mode + # in ~/.hermes/config.yaml). Lets a user opt into fast-as-default + # without having to pass mode= on every call. + # ----------------------------------------------------------------- + + def _clear_default_mode_cache(self): + """Reset the lru_cache between tests so config changes are honoured.""" + from tools.session_search_tool import _resolve_user_default_mode + _resolve_user_default_mode.cache_clear() + + def test_unset_mode_falls_back_to_summary_when_config_missing(self, monkeypatch): + """With no config, an unset mode resolves to 'summary'.""" + from tools.session_search_tool import _resolve_user_default_mode + self._clear_default_mode_cache() + # Force load_config import to fail → fallback path. + import sys + monkeypatch.setitem(sys.modules, "hermes_cli.config", None) + assert _resolve_user_default_mode() == "summary" + + def test_user_can_configure_fast_as_default(self, monkeypatch): + """tools.session_search.default_mode: fast → unset mode resolves to 'fast'.""" + from tools.session_search_tool import _resolve_user_default_mode + self._clear_default_mode_cache() + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"tools": {"session_search": {"default_mode": "fast"}}}, + ) + assert _resolve_user_default_mode() == "fast" + + def test_user_can_configure_summary_as_default_explicitly(self, monkeypatch): + """Explicit summary in config behaves identically to the implicit default.""" + from tools.session_search_tool import _resolve_user_default_mode + self._clear_default_mode_cache() + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"tools": {"session_search": {"default_mode": "summary"}}}, + ) + assert _resolve_user_default_mode() == "summary" + + def test_invalid_default_mode_warns_and_falls_back(self, monkeypatch, caplog): + """Typo'd / unknown value logs a warning and falls back to 'summary'.""" + from tools.session_search_tool import _resolve_user_default_mode + import logging + self._clear_default_mode_cache() + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"tools": {"session_search": {"default_mode": "smary"}}}, + ) + with caplog.at_level(logging.WARNING): + assert _resolve_user_default_mode() == "summary" + # User sees feedback about the typo. + assert any("smary" in rec.message for rec in caplog.records) + + def test_guided_as_default_mode_is_rejected(self, monkeypatch): + """guided requires anchors and can't be a standalone default — falls back to 'summary'.""" + from tools.session_search_tool import _resolve_user_default_mode + self._clear_default_mode_cache() + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"tools": {"session_search": {"default_mode": "guided"}}}, + ) + assert _resolve_user_default_mode() == "summary" + + def test_non_string_default_mode_falls_back(self, monkeypatch): + """Bogus types (int, dict, etc.) in YAML fall back gracefully, no crash.""" + from tools.session_search_tool import _resolve_user_default_mode + self._clear_default_mode_cache() + for bad in (42, ["fast"], {"mode": "fast"}, True): + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda b=bad: {"tools": {"session_search": {"default_mode": b}}}, + ) + self._clear_default_mode_cache() + assert _resolve_user_default_mode() == "summary", f"bad value {bad!r} should fall back" + + def test_explicit_mode_argument_overrides_user_default(self, monkeypatch): + """User config sets fast-as-default, but explicit mode='summary' still wins.""" + from unittest.mock import MagicMock + from tools.session_search_tool import session_search + + self._clear_default_mode_cache() + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"tools": {"session_search": {"default_mode": "fast"}}}, + ) + mock_db = MagicMock() + mock_db.search_messages.return_value = [] + + result = json.loads(session_search(query="anything", db=mock_db, mode="fast")) + assert result["mode"] == "fast" + # ...and explicit summary still produces summary even when default is fast. + result = json.loads(session_search(query="anything", db=mock_db, mode="summary")) + assert result["mode"] == "summary" + + def test_unset_mode_with_config_default_fast_runs_fast_path(self, monkeypatch): + """End-to-end: config says default=fast, caller passes mode=None → fast hits returned, no LLM.""" + from unittest.mock import MagicMock + from tools.session_search_tool import session_search + + self._clear_default_mode_cache() + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"tools": {"session_search": {"default_mode": "fast"}}}, + ) + + async def fail_summarize(*_args, **_kwargs): + raise AssertionError("fast mode must not invoke the summariser") + monkeypatch.setattr("tools.session_search_tool._summarize_session", fail_summarize) + + mock_db = MagicMock() + mock_db.search_messages.return_value = [ + {"session_id": "sid", "id": 7, "content": "match", "source": "cli", + "session_started": 1709400000, "model": "test"}, + ] + mock_db.get_session.return_value = {"id": "sid", "parent_session_id": None, + "source": "cli", "started_at": 1709400000} + + # mode=None mimics what the dispatcher passes when the LLM omits 'mode'. + result = json.loads(session_search(query="match", db=mock_db, mode=None)) + assert result["mode"] == "fast" + assert result["count"] == 1 def test_current_child_session_excludes_parent_lineage(self): """Compression/delegation parents should be excluded for the active child session.""" diff --git a/tools/session_search_tool.py b/tools/session_search_tool.py index a90723a697..0a24fcc442 100644 --- a/tools/session_search_tool.py +++ b/tools/session_search_tool.py @@ -23,6 +23,62 @@ from typing import Dict, Any, List, Optional, Union from agent.auxiliary_client import async_call_llm, extract_content_or_reasoning MAX_SESSION_CHARS = 100_000 + + +# Default mode is summary unless the user opts into a different one via +# ``tools.session_search.default_mode`` in ~/.hermes/config.yaml. Only ``fast`` +# and ``summary`` are valid defaults — guided requires anchors and can't be +# used standalone. Wrapped in lru_cache so the YAML read happens at most once +# per process; the CLI / TUI is the typical caller and config changes need a +# restart anyway. +_VALID_DEFAULT_MODES = ("fast", "summary") + + +def _resolve_user_default_mode() -> str: + """Look up ``tools.session_search.default_mode`` from ~/.hermes/config.yaml. + + Returns "summary" if unset, invalid, or the config loader is unavailable + (e.g. tests, tools loaded outside the CLI). Logs a one-time warning on + invalid values so users get feedback when they typo their config. + """ + try: + from hermes_cli.config import load_config + config = load_config() or {} + except ImportError: + logging.debug("hermes_cli.config not available; default_mode falls back to 'summary'") + return "summary" + except Exception as e: + logging.debug("Failed to load config for session_search default_mode: %s", e, exc_info=True) + return "summary" + + raw = ( + config.get("tools", {}) + .get("session_search", {}) + .get("default_mode") + ) + if raw is None: + return "summary" + if not isinstance(raw, str): + logging.warning( + "tools.session_search.default_mode in config.yaml must be a string, got %r — falling back to 'summary'", + raw, + ) + return "summary" + normalised = raw.strip().lower() + if normalised not in _VALID_DEFAULT_MODES: + logging.warning( + "tools.session_search.default_mode=%r is not one of %s — falling back to 'summary'. " + "(guided requires anchors and cannot be a default.)", + raw, _VALID_DEFAULT_MODES, + ) + return "summary" + return normalised + + +# Process-level cache so repeated session_search calls don't re-read YAML. +# Cleared by tests via _resolve_user_default_mode.cache_clear() when needed. +import functools # noqa: E402 — local to the cache wrap +_resolve_user_default_mode = functools.lru_cache(maxsize=1)(_resolve_user_default_mode) MAX_SUMMARY_TOKENS = 10000 @@ -671,7 +727,15 @@ def session_search( from hermes_state import format_session_db_unavailable return tool_error(format_session_db_unavailable(), success=False) - mode = (mode or "summary").strip().lower() if isinstance(mode, str) else "summary" + # Mode normalisation. ``None`` / empty string / non-string → fall back to + # the user's configured default (via ~/.hermes/config.yaml, see + # ``_resolve_user_default_mode``). Defaults to "summary" if unset. We only + # resolve the user default when the caller didn't pass an explicit mode — + # an explicit "fast" or "summary" or "guided" wins regardless of config. + if not isinstance(mode, str) or not mode.strip(): + mode = _resolve_user_default_mode() + else: + mode = mode.strip().lower() if mode in ("summarized", "summarise", "summarize", "deep"): mode = "summary" if mode in ("drill", "drilldown", "drill-down", "anchor", "around"): @@ -972,7 +1036,9 @@ SESSION_SEARCH_SCHEMA = { "2. Keyword search (with query): Search for specific topics across all past sessions. " "Defaults to mode='summary', returning LLM-generated recaps of the matched sessions (the recall " "you usually want). Set mode='fast' for cheap, instant FTS snippet hits when you only need to " - "discover which sessions touched a topic.\n" + "discover which sessions touched a topic. The default can be overridden per-user via " + "``tools.session_search.default_mode: fast`` in ~/.hermes/config.yaml — when no mode is passed " + "explicitly, that user-configured value applies (then 'summary' as the final fallback).\n" "3. Drill-down (mode='guided'): When a fast-mode result looks promising but you need the " "actual conversation around it, call again with mode='guided', session_id from the result, " "and around_message_id=match_message_id from the same result. The pair (session_id, "