From 6f793ddbdcf1c5ab50bcfc4b9645172cd047a156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o?= Date: Fri, 14 Aug 2026 12:08:54 +0000 Subject: [PATCH] fix(session_search): apply after/before in SQL WHERE Push the session-start bounds into search_messages so FTS LIMIT cannot be filled by out-of-range hits. Covers FTS5, CJK, trigram, LIKE fallback, and the unindexed-gap supplement. Refs #86021. --- hermes_state_search.py | 32 ++++++++++++++++------ tests/tools/test_session_search.py | 44 ++++++++++++++++++++++++++++++ tools/session_search_tool.py | 2 +- 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/hermes_state_search.py b/hermes_state_search.py index 0cbdfeb37f..454272b9f7 100644 --- a/hermes_state_search.py +++ b/hermes_state_search.py @@ -137,10 +137,13 @@ def _search_select_sql(snippet_sql: str, from_sql: str, where: List[str], order_ def _search_filter_clauses( where: List[str], params: list, *, include_inactive: bool, source_filter: Optional[List[str]], - exclude_sources: Optional[List[str]], role_filter: Optional[List[str]]) -> None: - """Append the visibility/source/role predicates every search route shares. Live rows - (active=1) AND compaction-archived rows (compacted=1) are discoverable; only - rewind/undo rows (active=0, compacted=0) are hidden.""" + exclude_sources: Optional[List[str]], role_filter: Optional[List[str]], + after_ts: Optional[int] = None, before_ts: Optional[int] = None) -> None: + """Append the visibility/source/role/session-start predicates every search route shares. Live + rows (active=1) AND compaction-archived rows (compacted=1) are discoverable; only + rewind/undo rows (active=0, compacted=0) are hidden. ``after_ts``/``before_ts`` bound + ``sessions.started_at`` (inclusive / exclusive) inside the query so LIMIT cannot be + filled by out-of-window hits.""" if not include_inactive: where.append("(m.active = 1 OR m.compacted = 1)") # display_kind="hidden" rows are model-facing scaffolding the person never saw; a hit would confuse. @@ -154,6 +157,12 @@ def _search_filter_clauses( if role_filter: where.append(f"m.role IN ({','.join('?' for _ in role_filter)})") params.extend(role_filter) + if after_ts is not None: + where.append("s.started_at >= ?") + params.append(int(after_ts)) + if before_ts is not None: + where.append("s.started_at < ?") + params.append(int(before_ts)) class SessionSearchMixin: @@ -1011,6 +1020,7 @@ class SessionSearchMixin: self, query: str, source_filter: List[str] = None, exclude_sources: List[str] = None, role_filter: List[str] = None, limit: int = 20, offset: int = 0, sort: str = None, include_inactive: bool = False, fields: Optional[Collection[str]] = None, + after_ts: Optional[int] = None, before_ts: Optional[int] = None, ) -> List[Dict[str, Any]]: """:meth:`_search_messages_impl` plus one log line per slow search with the routing path taken. Threshold HERMES_SEARCH_SLOW_MS (default 1000; 0 logs every call).""" @@ -1019,7 +1029,8 @@ class SessionSearchMixin: try: rows = self._search_messages_impl( query, source_filter=source_filter, exclude_sources=exclude_sources, role_filter=role_filter, - limit=limit, offset=offset, sort=sort, include_inactive=include_inactive, fields=fields) + limit=limit, offset=offset, sort=sort, include_inactive=include_inactive, fields=fields, + after_ts=after_ts, before_ts=before_ts) return rows finally: elapsed_ms = (time.time() - started) * 1000.0 @@ -1032,12 +1043,15 @@ class SessionSearchMixin: self, query: str, source_filter: List[str] = None, exclude_sources: List[str] = None, role_filter: List[str] = None, limit: int = 20, offset: int = 0, sort: str = None, include_inactive: bool = False, fields: Optional[Collection[str]] = None, + after_ts: Optional[int] = None, before_ts: Optional[int] = None, ) -> List[Dict[str, Any]]: """FTS5 search across session messages (keywords, ``"phrases"``, AND/OR/NOT, ``prefix*``). Returns snippet + session metadata + 1-message context per hit; ``fields`` selects a projection. ``sort``: None = BM25 rank; "newest"/"oldest" = timestamp then rank (the CJK LIKE fallback ignores it). Rewound rows (``active=0, compacted=0``) are excluded - by default; compaction-archived rows ARE included; ``include_inactive`` = every row.""" + by default; compaction-archived rows ARE included; ``include_inactive`` = every row. + ``after_ts``/``before_ts`` bound ``sessions.started_at`` on every route (FTS5, CJK, + trigram, LIKE fallback, unindexed-gap supplement).""" result_fields = self._search_message_fields(fields) if not query or not query.strip(): return [] @@ -1045,7 +1059,8 @@ class SessionSearchMixin: if not query: return [] filters = dict(include_inactive=include_inactive, source_filter=source_filter, - exclude_sources=exclude_sources, role_filter=role_filter) + exclude_sources=exclude_sources, role_filter=role_filter, + after_ts=after_ts, before_ts=before_ts) # New oversized tool results index only a bounded prefix; an explicit tool-role search is the # opt-in full-body path and scans canonical rows via LIKE. if role_filter and "tool" in role_filter: @@ -1137,7 +1152,8 @@ class SessionSearchMixin: non_op_tokens = _non_operator_tokens(raw_query) or [raw_query] like_params: list = [p for tok in non_op_tokens for p in _like_params(tok)] like_where = [f"({' OR '.join([_LIKE_ANY_COLUMN_SQL] * len(non_op_tokens))})"] - filters = {k: route[k] for k in ("include_inactive", "source_filter", "exclude_sources", "role_filter")} + filters = {k: route[k] for k in ("include_inactive", "source_filter", "exclude_sources", "role_filter", + "after_ts", "before_ts")} _search_filter_clauses(like_where, like_params, **filters) # instr() for the snippet uses the first search token. return self._like_rows(like_where, [non_op_tokens[0], *like_params, route["limit"], route["offset"]], diff --git a/tests/tools/test_session_search.py b/tests/tools/test_session_search.py index d737b75571..edc53c2a1a 100644 --- a/tests/tools/test_session_search.py +++ b/tests/tools/test_session_search.py @@ -1220,6 +1220,50 @@ class TestDiscoveryTemporalNarrowing: )) assert result["success"] is False + def test_window_finds_in_range_session_buried_by_fts_limit(self, db, monkeypatch): + """A June hit must survive even if FTS rank would fill the scan with August. + + Teknium's ticket wants the bound in the search query (WHERE), not a + post-filter of the already-truncated FTS top-N. + """ + monkeypatch.setattr( + "tools.session_search_tool._DISCOVER_SCAN_LIMIT", 2 + ) + august = _unix(2026, 8, 10) + for i in range(4): + sid = f"s_aug_{i}" + db.create_session(sid, source="cli") + db._conn.execute( + "UPDATE sessions SET started_at = ?, title = ? WHERE id = ?", + (august, f"August {i}", sid), + ) + db.append_message( + sid, role="user", content="modpack modpack modpack modpack" + ) + db.append_message( + sid, role="assistant", content=("modpack details " * 20) + ) + db.create_session("s_june", source="cli") + db._conn.execute( + "UPDATE sessions SET started_at = ?, title = ? WHERE id = ?", + (_unix(2026, 6, 15), "June quiet", "s_june"), + ) + db.append_message("s_june", role="user", content="modpack") + db.append_message("s_june", role="assistant", content="ok") + db._conn.commit() + + result = json.loads(session_search( + query="modpack", + limit=5, + after="2026-06-01", + before="2026-07-01", + db=db, + )) + assert result["success"] is True + sids = [r["session_id"] for r in result["results"]] + assert "s_june" in sids + assert all(not sid.startswith("s_aug_") for sid in sids) + class TestDiscoverySessionExclusion: def test_exclude_session_ids_drops_named_session(self, db): diff --git a/tools/session_search_tool.py b/tools/session_search_tool.py index eca88c98c4..dc56ab5969 100644 --- a/tools/session_search_tool.py +++ b/tools/session_search_tool.py @@ -345,7 +345,7 @@ def _discover(db, query: str, role_filter: Optional[List[str]], limit: int, sort raw_results, err = _loud(lambda: db.search_messages( query=query, role_filter=role_filter or ["user", "assistant"], exclude_sources=list(_HIDDEN_SESSION_SOURCES), limit=_DISCOVER_SCAN_LIMIT, offset=0, sort=sort, - fields=_DISCOVER_SEARCH_FIELDS), "FTS5 search failed: %s", "Search failed") + fields=_DISCOVER_SEARCH_FIELDS, after_ts=after_ts, before_ts=before_ts), "FTS5 search failed: %s", "Search failed") if err: return err # Demote cron rows below interactive ones BEFORE dedup so a high-volume cron corpus