From ce0f4838b0e05bc7b5d731bf37dd7eff09e27ceb Mon Sep 17 00:00:00 2001 From: yoniebans Date: Fri, 15 May 2026 18:31:21 +0200 Subject: [PATCH] style(session_search): tighten verbose inline comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass over comments added during the iterative development of this PR, trimming where they restated the code, repeated themselves, or read as journal-style narration. Net -22 comment lines; behaviour unchanged, 123 tests still passing. Notable trims: - DEFAULT_CONFIG module header: 9 lines → 4. Dropped the 'auxiliary started as aux-LLM routing but in practice groups per-tool config' digression — irrelevant to readers of this module. - get_anchored_view bookend-SQL filter block: 8 lines → 5. The 'let me check…-shaped assistant messages' over-narration is gone; the SQL filter rationale survives. - Fast-mode lineage-grouping IMPORTANT block: 12 lines → 8. The '#regression introduced by the original match_message_id rollout' meta-note removed (the comment now states the contract directly). - Fast-mode result-emission comment: 8 lines → 3. The 'lineage_root is the dict key…' explanation was restating the variables; the load-bearing one-liner (emit raw_sid + match_message_id) stays. - sort normalisation comment: 4 lines → 3. - role_filter parse comment: 5 lines → 3. - ORDER BY comment in search_messages: 3 lines → 2. - LIKE fallback ordering comment: 4 lines → 2. --- hermes_state.py | 24 ++++++---------- tools/session_search_tool.py | 54 +++++++++++++----------------------- 2 files changed, 28 insertions(+), 50 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index 88b90622d5..bdbb288faa 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1743,14 +1743,11 @@ class SessionDB: window_min_id = window_rows[0]["id"] window_max_id = window_rows[-1]["id"] - # Fetch bookends only if there's space outside the window. The SQL - # filters by id range + role + non-empty content so we don't pull - # tool blobs we'd just drop, and so tool-call-only assistant turns - # (content='' with tool_calls populated) don't crowd out the actual - # prose openings/closings. Bookends exist to surface the session's - # spoken goal + resolution; "let me check..."-shaped assistant - # messages without content carry signal in tool_calls but aren't - # useful here. ``bookend=0`` short-circuits both queries. + # Fetch bookends only if there's space outside the window. SQL filters + # by id range, role, and non-empty content — tool-call-only assistant + # turns (content='' with tool_calls populated) are excluded so they + # don't crowd out the actual prose openings/closings. ``bookend=0`` + # short-circuits both queries. bookend_start_rows: List[Any] = [] bookend_end_rows: List[Any] = [] if bookend > 0: @@ -2111,9 +2108,8 @@ class SessionDB: else: sort_norm = None - # Build the ORDER BY clause shared by both FTS5 paths (main + trigram). - # When sort is set, timestamp is primary and rank is the tiebreaker; - # otherwise rank alone (current behaviour). + # ORDER BY shared by both FTS5 paths. With sort set, timestamp is + # primary and rank is the tiebreaker; otherwise rank alone. if sort_norm == "newest": order_by_sql = "ORDER BY m.timestamp DESC, rank" elif sort_norm == "oldest": @@ -2267,10 +2263,8 @@ class SessionDB: if role_filter: like_where.append(f"m.role IN ({','.join('?' for _ in role_filter)})") like_params.extend(role_filter) - # LIKE fallback has no FTS5 rank to combine with, so the - # ordering options collapse to just timestamp direction. - # Default and "newest" both produce DESC (preserving prior - # behaviour); "oldest" flips to ASC. + # LIKE fallback has no rank to combine with — just timestamp + # direction. Default/"newest" → DESC; "oldest" → ASC. like_order_sql = ( "ORDER BY m.timestamp ASC" if sort_norm == "oldest" diff --git a/tools/session_search_tool.py b/tools/session_search_tool.py index 0791e4d6c0..034e1a1518 100644 --- a/tools/session_search_tool.py +++ b/tools/session_search_tool.py @@ -25,15 +25,10 @@ from agent.auxiliary_client import async_call_llm, extract_content_or_reasoning MAX_SESSION_CHARS = 100_000 -# Default mode is fast unless the user opts into a different one via -# ``auxiliary.session_search.default_mode`` in ~/.hermes/config.yaml. Lives -# alongside the other session_search-scoped knobs (provider, model, -# max_concurrency) — "auxiliary" started as aux-LLM routing but in practice -# groups per-tool config by tool name. 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. +# Default mode is fast unless the user sets ``auxiliary.session_search.default_mode`` +# in ~/.hermes/config.yaml. Only ``fast`` and ``summary`` are valid — guided +# requires anchors. Resolver is lru_cache-wrapped so the YAML read happens at +# most once per process; restart to pick up config changes. _VALID_DEFAULT_MODES = ("fast", "summary") _FALLBACK_DEFAULT_MODE = "fast" @@ -833,10 +828,9 @@ def session_search( if mode not in ("fast", "summary", "guided"): mode = _resolve_user_default_mode() - # Normalise sort. Only "newest" / "oldest" are accepted; anything else - # (including None) collapses to None = FTS5 rank-only ordering, which is - # the historical default. sort only affects fast mode — log and ignore - # in summary / guided / recent so misuse is visible but non-fatal. + # Normalise sort — only "newest"/"oldest" are accepted; anything else + # collapses to None (FTS5 rank-only). Sort affects fast mode only; logged + # and ignored elsewhere so misuse is visible but non-fatal. sort_norm: Optional[str] = None if isinstance(sort, str): candidate = sort.strip().lower() @@ -880,11 +874,9 @@ def session_search( query = query.strip() try: - # Parse role filter. When caller didn't pass one, default to - # user+assistant — tool messages are usually noisy (serialised tool - # calls, large outputs) and rarely the signal someone is searching - # for. Callers can opt back in by passing role_filter='user,assistant,tool' - # or just 'tool' when debugging tool output. + # Parse role filter. Defaults to user+assistant; tool messages are + # usually noisy and rarely the signal. Caller opts back in via + # role_filter='user,assistant,tool' or 'tool'. role_list = None if role_filter and role_filter.strip(): role_list = [r.strip() for r in role_filter.split(",") if r.strip()] @@ -947,15 +939,12 @@ def session_search( # session lineage. Compression and delegation create child sessions # that still belong to the same active conversation. # - # IMPORTANT: we group BY parent (so the user sees one entry per - # conversation lineage), but we preserve the raw FTS5 session_id on - # the surviving result. The raw sid is the only sid that pairs - # validly with ``match_message_id``; rewriting it to the parent - # produces a "{parent_sid, child_message_id}" handle that guided - # mode cannot resolve (#regression introduced by the original - # match_message_id rollout). See the parent_session_id field in - # fast-mode output for the lineage-root link the user expects to - # see. + # IMPORTANT: group BY parent (one entry per conversation lineage), but + # preserve the raw FTS5 session_id on the surviving result. Only the + # raw sid pairs validly with ``match_message_id``; rewriting it to the + # parent produces a {parent_sid, child_message_id} handle that guided + # mode cannot resolve. ``parent_session_id`` is exposed separately for + # the lineage-root link the user expects to see. seen_sessions = {} for result in raw_results: raw_sid = result["session_id"] @@ -979,14 +968,9 @@ def session_search( if mode == "fast": results = [] for lineage_root, match_info in seen_sessions.items(): - # ``lineage_root`` is the dict key (resolved parent — used for - # dedup grouping). ``match_info["session_id"]`` is the raw FTS5 - # row's session — the only sid that pairs with - # ``match_info["id"]`` (the message id). Emit the pair (raw sid + - # match_message_id) so the agent's follow-up - # mode='guided' call has a valid {session_id, around_message_id} - # handle. ``parent_session_id`` (if different) tells the agent - # which conversation lineage this fragment belongs to. + # Emit (raw_sid + match_message_id) so the agent's follow-up + # guided call has a valid {session_id, around_message_id}. + # ``parent_session_id`` (if different) carries the lineage root. hit_sid = match_info.get("session_id") or lineage_root try: session_meta = db.get_session(lineage_root) or {}