perf(session-search): project fields before enrichment
This commit is contained in:
parent
f327c898e2
commit
ffb54305c4
|
|
@ -353,6 +353,14 @@ async def search_sessions(
|
|||
source_filter=include_sources,
|
||||
exclude_sources=exclude_list or None,
|
||||
limit=fetch_limit,
|
||||
fields=(
|
||||
"session_id",
|
||||
"role",
|
||||
"snippet",
|
||||
"source",
|
||||
"model",
|
||||
"session_started",
|
||||
),
|
||||
)
|
||||
|
||||
for m in matches:
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import os
|
|||
import re
|
||||
import sqlite3
|
||||
import time
|
||||
from typing import Any, Callable, Dict, List, Optional, Tuple
|
||||
from typing import Any, Callable, Collection, Dict, List, Optional, Tuple
|
||||
|
||||
from agent.skill_commands import describe_skill_invocation
|
||||
from hermes_state_common import (
|
||||
|
|
@ -35,6 +35,36 @@ logger = logging.getLogger("hermes_state")
|
|||
class SessionSearchMixin:
|
||||
"""See module docstring — mixin for SessionDB (Search cluster)."""
|
||||
|
||||
_SEARCH_MESSAGE_RESULT_FIELDS = (
|
||||
"id",
|
||||
"session_id",
|
||||
"role",
|
||||
"snippet",
|
||||
"timestamp",
|
||||
"tool_name",
|
||||
"source",
|
||||
"model",
|
||||
"session_started",
|
||||
"context",
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def _search_message_fields(
|
||||
cls, fields: Optional[Collection[str]]
|
||||
) -> Optional[Tuple[str, ...]]:
|
||||
"""Validate and canonically order an optional result projection."""
|
||||
if fields is None:
|
||||
return None
|
||||
if isinstance(fields, str):
|
||||
raise TypeError("search fields must be a collection of field names, not a string")
|
||||
requested = set(fields)
|
||||
unknown = requested.difference(cls._SEARCH_MESSAGE_RESULT_FIELDS)
|
||||
if unknown:
|
||||
raise ValueError(f"unknown search result field(s): {', '.join(sorted(unknown))}")
|
||||
return tuple(
|
||||
field for field in cls._SEARCH_MESSAGE_RESULT_FIELDS if field in requested
|
||||
)
|
||||
|
||||
def _try_incremental_merge_fts(self) -> None:
|
||||
"""Run one bounded FTS5 merge pass without failing the completed write."""
|
||||
if not self._fts_enabled:
|
||||
|
|
@ -1274,6 +1304,7 @@ class SessionSearchMixin:
|
|||
offset: int = 0,
|
||||
sort: str = None,
|
||||
include_inactive: bool = False,
|
||||
fields: Optional[Collection[str]] = None,
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""Instrumented wrapper around :meth:`_search_messages_impl`.
|
||||
|
||||
|
|
@ -1295,6 +1326,7 @@ class SessionSearchMixin:
|
|||
offset=offset,
|
||||
sort=sort,
|
||||
include_inactive=include_inactive,
|
||||
fields=fields,
|
||||
)
|
||||
return rows
|
||||
finally:
|
||||
|
|
@ -1344,6 +1376,7 @@ class SessionSearchMixin:
|
|||
offset: int = 0,
|
||||
sort: str = None,
|
||||
include_inactive: bool = False,
|
||||
fields: Optional[Collection[str]] = None,
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""
|
||||
Full-text search across session messages using FTS5.
|
||||
|
|
@ -1356,6 +1389,9 @@ class SessionSearchMixin:
|
|||
|
||||
Returns matching messages with session metadata, content snippet,
|
||||
and surrounding context (1 message before and after the match).
|
||||
``fields`` selects a result projection; omitting it preserves the
|
||||
complete legacy result. Context is only loaded when that projection
|
||||
consumes it.
|
||||
|
||||
``sort`` controls temporal ordering:
|
||||
- ``None`` (default): FTS5 BM25 relevance only. Time-neutral.
|
||||
|
|
@ -1373,6 +1409,8 @@ class SessionSearchMixin:
|
|||
pre-compaction transcript stays discoverable after in-place compaction
|
||||
(#38763). Pass ``include_inactive=True`` to search every row regardless.
|
||||
"""
|
||||
result_fields = self._search_message_fields(fields)
|
||||
|
||||
if not self._fts_enabled:
|
||||
return []
|
||||
|
||||
|
|
@ -1458,6 +1496,7 @@ class SessionSearchMixin:
|
|||
# (indexed substring matching with ranking and snippets). For shorter
|
||||
# CJK queries (1-2 chars), trigram can't match (it needs ≥9 UTF-8
|
||||
# bytes = 3 CJK chars), so we fall back to LIKE.
|
||||
matches: List[Dict[str, Any]] = []
|
||||
is_cjk = self._contains_cjk(query)
|
||||
if is_cjk:
|
||||
raw_query = query.strip('"').strip()
|
||||
|
|
@ -1821,10 +1860,14 @@ class SessionSearchMixin:
|
|||
if tri_matches:
|
||||
matches = tri_matches
|
||||
|
||||
# Add surrounding context (1 message before + after each match).
|
||||
# Each query takes its own fresh read transaction via _read_ctx, so
|
||||
# we never hold a lock across N sequential queries.
|
||||
for match in matches:
|
||||
# Add surrounding context (1 message before + after each match) only
|
||||
# when the selected result projection consumes it. Each query takes
|
||||
# its own fresh read transaction via _read_ctx, so we never hold a
|
||||
# lock across N sequential queries.
|
||||
context_matches = (
|
||||
matches if result_fields is None or "context" in result_fields else ()
|
||||
)
|
||||
for match in context_matches:
|
||||
try:
|
||||
with self._read_ctx() as conn:
|
||||
ctx_cursor = conn.execute(
|
||||
|
|
@ -1888,6 +1931,12 @@ class SessionSearchMixin:
|
|||
for match in matches:
|
||||
match.pop("content", None)
|
||||
|
||||
if result_fields is not None:
|
||||
matches = [
|
||||
{field: match[field] for field in result_fields if field in match}
|
||||
for match in matches
|
||||
]
|
||||
|
||||
return matches
|
||||
|
||||
def _search_unindexed_gap(
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ class _FakeSessionDB:
|
|||
|
||||
closed = False
|
||||
opened_read_only = None
|
||||
requested_fields = None
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
type(self).opened_read_only = kwargs.get("read_only")
|
||||
|
|
@ -57,8 +58,16 @@ class _FakeSessionDB:
|
|||
)
|
||||
][:limit]
|
||||
|
||||
def search_messages(self, query, source_filter=None, exclude_sources=None, limit=20):
|
||||
def search_messages(
|
||||
self,
|
||||
query,
|
||||
source_filter=None,
|
||||
exclude_sources=None,
|
||||
limit=20,
|
||||
fields=None,
|
||||
):
|
||||
assert query == "20260603*"
|
||||
type(self).requested_fields = fields
|
||||
rows = [
|
||||
{
|
||||
"session_id": "20260603_090200_exact",
|
||||
|
|
@ -98,10 +107,13 @@ class _FakeSessionDB:
|
|||
|
||||
def test_desktop_session_search_merges_id_matches_before_content_matches(monkeypatch):
|
||||
_FakeSessionDB.opened_read_only = None
|
||||
_FakeSessionDB.requested_fields = None
|
||||
monkeypatch.setattr("hermes_state.SessionDB", _FakeSessionDB)
|
||||
|
||||
response = asyncio.run(web_server.search_sessions(q="20260603", limit=2))
|
||||
|
||||
assert _FakeSessionDB.requested_fields is not None
|
||||
assert "context" not in _FakeSessionDB.requested_fields
|
||||
# ID match surfaces first; the content hit on the SAME session is deduped
|
||||
# by lineage root (not double-listed); the unrelated content hit follows.
|
||||
assert response == {
|
||||
|
|
|
|||
|
|
@ -656,8 +656,26 @@ class TestFTS5Search:
|
|||
assert isinstance(results[0]["context"], list)
|
||||
assert len(results[0]["context"]) > 0
|
||||
|
||||
def test_search_fields_project_results_without_changing_default(self, db):
|
||||
db.create_session(session_id="s1", source="cli")
|
||||
db.append_message("s1", role="user", content="Tell me about Kubernetes")
|
||||
db.append_message("s1", role="assistant", content="Kubernetes is an orchestrator.")
|
||||
|
||||
projected = db.search_messages(
|
||||
"Kubernetes", fields=("session_id", "role", "snippet")
|
||||
)
|
||||
default = db.search_messages("Kubernetes")
|
||||
|
||||
assert len(projected) == len(default) == 2
|
||||
assert all(set(row) == {"session_id", "role", "snippet"} for row in projected)
|
||||
assert [
|
||||
(row["session_id"], row["role"], row["snippet"])
|
||||
for row in projected
|
||||
] == [
|
||||
(row["session_id"], row["role"], row["snippet"])
|
||||
for row in default
|
||||
]
|
||||
assert all("context" in row and row["context"] for row in default)
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -113,6 +113,29 @@ class TestBrowseShape:
|
|||
# =========================================================================
|
||||
|
||||
class TestDiscoveryShape:
|
||||
def test_discovery_field_plan_preserves_full_default_result(self, db, monkeypatch):
|
||||
_seed_modpack_sessions(db)
|
||||
original = db.search_messages
|
||||
requested_fields = None
|
||||
|
||||
def search_spy(*args, **kwargs):
|
||||
nonlocal requested_fields
|
||||
requested_fields = kwargs.get("fields")
|
||||
return original(*args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(db, "search_messages", search_spy)
|
||||
|
||||
result = json.loads(session_search(query="modpack", limit=1, db=db))
|
||||
|
||||
assert result["success"] is True
|
||||
assert requested_fields is not None
|
||||
assert "context" not in requested_fields
|
||||
assert len(result["results"]) == 1
|
||||
hit = result["results"][0]
|
||||
assert "bookend_start" in hit
|
||||
assert hit["messages"]
|
||||
assert "bookend_end" in hit
|
||||
|
||||
def test_discovery_result_has_bookends_and_window(self, db):
|
||||
_seed_modpack_sessions(db)
|
||||
result = json.loads(session_search(query="modpack", limit=3, db=db))
|
||||
|
|
|
|||
|
|
@ -55,6 +55,18 @@ _DEMOTED_SESSION_SOURCES = ("cron",)
|
|||
# the handful of distinct sessions a typical query returns.
|
||||
_DISCOVER_SCAN_LIMIT = 300
|
||||
|
||||
# Raw FTS rows are only a discovery-plan input. The final response hydrates
|
||||
# its own anchored message window and bookends after lineage deduplication.
|
||||
_DISCOVER_SEARCH_FIELDS = (
|
||||
"id",
|
||||
"session_id",
|
||||
"role",
|
||||
"snippet",
|
||||
"source",
|
||||
"model",
|
||||
"session_started",
|
||||
)
|
||||
|
||||
# Prefixes that identify generated context-compaction handoff summaries.
|
||||
# These are inserted by agent/context_compressor.py as normal user/assistant
|
||||
# messages but contain machine-generated summary metadata — not user content.
|
||||
|
|
@ -699,6 +711,7 @@ def _discover(
|
|||
# of cron rows are still in hand for the demotion pass below.
|
||||
offset=0,
|
||||
sort=sort,
|
||||
fields=_DISCOVER_SEARCH_FIELDS,
|
||||
)
|
||||
except Exception as e:
|
||||
logging.error("FTS5 search failed: %s", e, exc_info=True)
|
||||
|
|
|
|||
Loading…
Reference in New Issue