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, "