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.
This commit is contained in:
@@ -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"]],
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user