Skip to content

feat(state): indexed session metadata search over title/id/display_name (row_id substrate + Unicode/trigram lanes) - #91341

Open
Skywind5487 wants to merge 13 commits into
NousResearch:mainfrom
Skywind5487:feat/session-metadata-search
Open

Skywind5487 wants to merge 13 commits into
NousResearch:mainfrom
Skywind5487:feat/session-metadata-search

Conversation

@Skywind5487

@Skywind5487 Skywind5487 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Find a conversation by what you remember about it — stored title, logical session id, or gateway display name — including Unicode/CJK and infix-style metadata queries, without regressing existing message-content search.

Today list_sessions_rich(search_query=...) (hermes_state.py:8646) matches only title and id with a %...% LIKE scan: an O(n) full-table scan that (a) cannot use any index, (b) does not match gateway display_name at all, and (c) has no CJK/trigram lane. This PR replaces that with candidate-first routing over three FTS5 external-content metadata lanes, keeping a bounded literal-safe LIKE fallback for zero-result / unavailable routes.

Indexed substrate. Three external-content FTS5 lanes over (title, id, display_name):

  • a raw Unicode lane (sessions_fts, always on);
  • an optional trigram lane over the compact projection (sessions_fts_trigram, infix/punctuation-normalized);
  • an optional CJK lane (sessions_fts_cjk, served only when a CJK tokenizer is available).

A classifier routes each query (lone-CJK → LIKE, CJK → cjk+unicode union, explicit FTS token syntax → unicode, plain 3+ char literal → trigram, else LIKE). The bounded canonical LIKE fallback (literal %/_/\ escaping, raw + compact fields) runs only on zero-result / unavailable routes — never an unbounded scan. list_sessions_rich(search_query=...) consumes the router candidate-first.

sessions.row_id migration (please read before reviewing). This is not generic schema cleanup. External-content FTS needs a stable integer document identity; the old id TEXT PRIMARY KEY only had a hidden rowid, which is not durable application-owned identity and can be renumbered by VACUUM (desynchronizing FTS from canonical rows). The resumable rebuild also needs post-capture rows to never reuse a deleted id below the captured high-water. The migration names row_id INTEGER PRIMARY KEY AUTOINCREMENT (keeping id TEXT NOT NULL UNIQUE as the public identity) and preserves every surviving legacy hidden rowid exactly, including deleted-row gaps. Cost: ≤1.35s one-time at 100k sessions, transactional (create/copy/drop/rename + index recreate). Rollback = restore the previous sessions table; crash-safety = the swap is one transaction (reopen after a crash resumes from the H/P progress marker, never serves a partial index). The alternative — keeping the hidden rowid — is not viable: hidden rowids are renumbered by VACUUM, so the FTS→canonical mapping can silently point at the wrong session, and post-capture rows could reuse a deleted id below the high-water, breaking the rebuild ownership invariant.

Why this approach is right (trade-off). The current LIKE path is O(n): measured 4.5ms at 1k sessions but 200–460ms at 10k–100k — a perceptible CLI/UI freeze and a blocking gateway query at realistic install sizes. The indexed route is sub-ms to ~7ms regardless of scale. The trigram lane is what makes infix / punctuation-normalized / display_name fast; CJK is kept as an optional lane because it gives no speedup on hosts without a CJK tokenizer (it degrades to the bounded LIKE fallback) — it is a capability-presence feature, not a performance feature, so it must not gate the core.

Related Issue

This is the upstream contribution of the fork's Session Metadata Search work. Related fork issues: Skywind5487#128 (implementation), #140 (benchmark), #141 (upstream gate). No upstream issue exists for this exact contract; the closest open upstream PRs are #71912 (display_name search), #89553 (Desktop stored-title), #87636 (Desktop/web fuzzy search), #67381 (title substring in search_messages), #75496 (CLI sessions list redesign) — see the Overlap section below.

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • hermes_state_common.py — session-metadata FTS DDL: sessions_fts (raw Unicode external-content), sessions_fts_cjk (optional), sessions_fts_trigram (compact projection via sessions_fts_trigram_src VIEW), shared compact-separator policy, per-lane H/P rebuild markers.
  • hermes_state_schema.pysessions.row_id migration (copy-swap-rename preserving exact legacy rowids incl. deleted-row holes), lane ensure/seed on open, optional-lane stale handling.
  • hermes_state_search.py — candidate router (_classify_metadata_query, _metadata_candidate_row_ids), per-lane MATCH helpers, bounded literal-safe LIKE fallback, resumable lane backfill driven from optimize_fts_storage.
  • hermes_state.pylist_sessions_rich(search_query=...) consumes the router candidate-first.
  • hermes_cli/web_routers/sessions.py — web session search route uses the metadata lane (stored-title-only sessions surface).
  • apps/desktop/src/app/chat/sidebar/index.tsx, apps/desktop/src/types/hermes.ts — Desktop result propagation for stored title/origin metadata.
  • Tests: tests/test_session_metadata_fts.py, tests/test_session_metadata_cjk_fts.py, tests/test_session_metadata_trigram_fts.py, tests/test_session_metadata_picker_routing.py, tests/hermes_cli/test_web_server_session_search.py, tests/hermes_cli/test_session_listing.py.

How to Test

  1. scripts/run_tests.sh tests/test_session_metadata_fts.py tests/test_session_metadata_cjk_fts.py tests/test_session_metadata_trigram_fts.py tests/test_session_metadata_picker_routing.py tests/hermes_cli/test_web_server_session_search.py -q → 40 passed.
  2. scripts/run_tests.sh tests/test_hermes_state.py tests/hermes_cli/test_session_listing.py tests/tools/test_session_search.py -q → 299 passed, 2 skipped.
  3. Manual: list_sessions_rich(order_by_last_active=True, search_query="report") returns the stored-title session via the trigram lane; search_query="finance" matches a gateway display_name; search_query="an94" matches AN-94 (punctuation-normalized); search_query="zzzznope" returns [] via the bounded fallback (no unbounded scan).
  4. Migration: opening a legacy id TEXT PRIMARY KEY DB migrates to row_id preserving exact rowids (incl. deleted-row holes); reopen is idempotent; optimize_fts_storage() drives the session lanes to completion.

Tested platform: Windows 10, Python 3.11.11, SQLite 3.47.1 (trigram tokenizer available; CJK tokenizer NOT available — the default install; the CJK capable path is covered by tests using a locally built tokenizer). Cross-platform: CJK/trigram lanes are optional and degrade to the bounded LIKE fallback on hosts without the tokenizer; the Unicode lane is the always-on substrate; no platform-specific code.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — targeted CI-parity wrapper run (40 + 299 green); full-suite wrapper pending before Ready-for-Review
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 10, Python 3.11.11, SQLite 3.47.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Benchmark evidence (#140)

Synthetic scale sweep (fresh DBs, live-trigger inserts, median of 25 warm calls):

query shape 1k L/idx 10k L/idx 50k L/idx 100k L/idx
ASCII infix title 4.5/0.26 (17x) 46/0.61 (76x) 232/1.65 (141x) 454/2.78 (164x)
punctuation-normalized 4.4/0.30 (15x) 42/0.47 (89x) 220/1.60 (137x) 444/3.68 (120x)
display_name 4.5/0.31 (15x) 44/1.15 (39x) 222/3.35 (66x) 429/7.10 (60x)
logical id 4.6/0.21 (21x) 43/0.20 (213x) 228/0.26 (876x) 419/0.29 (1466x)
CJK (degraded → LIKE) 4.4/4.6 (1x) 45/52 (0.9x) 223/224 (1x) 461/467 (1x)
zero-hit (fallback) 4.4/4.5 (1x) 52/50 (1x) 226/229 (1x) 425/427 (1x)

Real production DB (7,268 sessions / 231,513 messages, 1.67GB, schema 25):

query shape LIKE indexed speedup
ASCII infix title 5.91 0.14 41x
CJK title 6.29 0.06 63x
display_name 6.44 0.09 64x
logical id 5.84 0.37 16x
zero-hit 5.97 0.03 60x

Space vs time (real DB): raw metadata text (title+id+display_name) = 0.30MB → FTS index = 2.68MB (9.05x text→FTS expansion, but 0.2% of the 1.67GB DB). The entire session-metadata index costs 2.68MB and buys 16–64x faster metadata search, including CJK titles and gateway display_names the current LIKE path cannot match at all. One-time upgrade backfill (throttled): 7.9s@10k / 83s@100k; row_id migration ≤1.35s@100k, transactional.

Bug found during benchmarking: the lane MATCH selected a non-existent row_id column on the FTS5 external-content tables (they expose rowid), so every routed query silently fell back to the LIKE lane and the indexed substrate never served — while tests stayed green because they assert results, which the fallback reproduces. Fixed (SELECT rowid AS row_id) with route-asserting regressions (test_*_route_serves_via_fts_lane).

Overlap with open upstream PRs

Screenshots / Logs

N/A (backend + tests; manual exercise output in How to Test).

…search (#128)

Route the Desktop /api/sessions/search metadata lane through the shared
list_sessions_rich(search_query=...) seam and extend that seam to match
gateway display_name, so a session whose stored title or peer/chat name
matches the query but whose message body does not is found and rendered
with its stored title.

- hermes_state.list_sessions_rich: search_query lane now matches title,
  gateway display_name, and id (raw + punctuation-compacted), preserving
  literal %/_ escaping and compression-chain membership.
- hermes_cli/web_routers/sessions.py GET /api/sessions/search: insert a
  whole-store metadata-discovery pass (list_sessions_rich search_query)
  between the exact-id and message-content lanes, deduped by lineage root;
  stored title already survives via get_session_rich_row hydration.
- Desktop: SessionSearchResult gains optional title; searchResultToSession
  renders it instead of hard-coding title: null.
- Tests: behavior-level RED (stored-title-only session surfaces with its
  title) + display_name listing RED (raw, compact, literal wildcards).
… + update title contract (#128)

Code-review findings from the first slice:
- Extract _session_result_entry helper shared by the ID lane and metadata
  lane of GET /api/sessions/search, eliminating the Duplicated Code and
  Data Clumps smells (same row→preview→payload shape repeated across lanes).
- Fix started_at/session_started key mismatch: ID lane and metadata lane
  both hydrate started_at from get_session_rich_row but the metadata row
  also carries session_started; the helper now accepts both keys so either
  path feeds add_lineage_result correctly.
- Remove dead max(safe_limit * 4, safe_limit) → safe_limit * 4.
- Update SessionSearchResult.title? JSDoc to match the actual contract:
  the server hydrates title for ALL search lanes via get_session_rich_row,
  not just metadata/ID hits.
- Extract _session_result_entry shared by ID + metadata lanes, eliminating
  Duplicated Code and Data Clumps smells.
- Accept both started_at (real DB/content lane) and session_started (test
  fake) in the helper; normalise the test fake to started_at for parity.
- Remove dead max(safe_limit * 4, safe_limit) → safe_limit * 4.
- Fix SessionSearchResult.title JSDoc to match actual server contract
  (all lanes hydrate title via get_session_rich_row, not just metadata).
e4ac3ad's refactor broke GET /api/sessions/search: the metadata lane
(list_sessions_rich search_query) was nested inside the id-match branch so
stored-title-only sessions never surfaced, an undefined _lineage_row_payload
helper was referenced, and the loop body lost its indentation. Restore the
independent metadata lane (title/id/display_name discovery), keep the
Desktop session_started contract, and re-pin the key mismatch in the test.
…play_name

Named sessions.row_id (copy-swap-rename migration preserving exact legacy
rowids incl. deleted-row holes) plus a raw (title, id, display_name)
external-content sessions_fts with a resumable fts_session_rebuild_*
lifecycle (seed H/P, chunked backfill, finish clears markers). Empty DBs
seed no markers; historical rows are backfilled by the chunk engine, rows
committing after the claim are live-indexed by gated triggers.
…ycle

Optional sessions_fts_cjk (cjk_unicode61) over raw (title, id, display_name)
keyed by named row_id, with its own fts_session_cjk_* marker pair and stale
key so tokenizer availability never gates the complete Unicode index.
Mirrors the message-CJK self-heal: tokenizer-less hosts drop the triggers and
breadcrumb, capable hosts seed H/P over populated DBs and serve only after
the backfill clears the markers.
…/display_name

sessions_fts_trigram reads a derived compact projection (compact(title), RAW
id, compact(display_name)) through the sessions_fts_trigram_src VIEW so
punctuation-compacted infix queries match at index speed while sessions stays
canonical. Own fts_session_trigram_* marker pair keeps P target-specific; the
compact separator policy is defined once and shared by the SQL VIEW and the
Python query helper. Also narrows the Unicode-lane marker helper in the #128
unicode tests to fts_session_rebuild_* so sibling-lane markers don't leak in.
…KE fallback

Metadata discovery now routes through a classifier (lone-CJK -> like, CJK ->
cjk+unicode union, explicit token syntax -> unicode, plain 3+ char literal ->
trigram, else like) into the session-metadata FTS lanes, with a bounded
canonical LIKE fallback (literal %/_/\\ escaping, raw + compact fields) used
only on zero-result or unavailable routes. list_sessions_rich(search_query=...)
consumes the router candidate-first: FTS hits narrow the compression chain to
the resulting row_ids; the previous LIKE lane remains as the fallback.
…ane helper

The Unicode / CJK / trigram session-metadata lanes each carried a near-verbatim
rebuild_step / rebuild_status / seed / clear and a same-shape schema ensure,
differing only by marker prefix, table and source. Introduce a single
_SessionFtsLane identity (common.py) plus parameterized _fts_session_lane_step/
_status/_seed/_clear and _ensure_session_optional_lane helpers; the public
per-lane methods stay as thin wrappers. Also drop the dead _sessions_fts_available
flag (the Unicode lane's availability is implicit via the rebuild-gap check)
and stop writing it in SessionDB.__init__ / the Unicode ensure.
…orage

The three session-metadata lanes seeded H/P markers on any DB that already
had sessions at open (the upgrade path), but nothing ever ran the chunk
backfill: optimize_fts_storage only drove the message-FTS steps, so the
Unicode lane's rebuild gap never closed (every query fell to the whole-store
LIKE lane) and the optional CJK/trigram lanes never became available. Add a
Phase 1c that runs each lane's step to completion inside the same throttled
backfill loop, and pin the upgrade path with a regression test.
…ed status

The list_sessions_rich LIKE fallback kept a divergent compact policy (broad
\\W strip + inline REPLACE) from the router/trigram canonical policy; both now
use the single compact_session_metadata_text / _session_metadata_compact_sql
helpers, fixing a punctuation recall gap on the fallback path. The three
per-lane FTS candidate queries collapse onto one _fts_metadata_lane_match
helper, and MetadataCandidateResult.status (read only by tests) is dropped —
an empty row_ids tuple already means zero.
…metadata lane match

The lane MATCH in _fts_metadata_lane_match selected \
ow_id\, but the
external-content session FTS tables (sessions_fts / _cjk / _trigram) expose
their implicit \
owid\ — \
ow_id\ is only the name of the content-side
rowid column. Every lane MATCH raised OperationalError, was caught, and
silently fell back to the bounded LIKE lane, so the indexed metadata
substrate never served through the candidate router. The green tests did
not catch it because they assert results, which the LIKE fallback
reproduces identically.

Fix: SELECT rowid AS row_id. Add route-asserting regressions so a routed
query must actually serve via the trigram/unicode lane, not the fallback.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) area/sessions Session lifecycle, resume, persistence, history needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 21, 2026
…e the row_id prefix

The sessions.row_id migration is self-healing (idempotent on open, no-op
when row_id already exists), so it does not need a schema-version bump —
reverting SCHEMA_VERSION 27->26 keeps the version-gated migration chain
unchanged and fixes the change-detector test that pinned 26.

The migration inserts row_id as the FIRST sessions column, which broke the
lost-and-found page-level salvage lane: its 'columns are only ever
appended' invariant assumed an older record is a strict prefix of the
current column order. A current-layout salvaged row now leads with a NULL
row_id alias (INTEGER PRIMARY KEY stores NULL in the record) followed by
the logical id + source. Teach the mapper to strip that leading NULL
(guarding against message rows, whose third cell is a role, not a source)
and to drop row_id from the prefix column map so id lands in the id
column.
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

  1. Tests — the diff ships no test changes for what is the most invasive change possible: rebuilding the core sessions table under every live database, plus a three-lane FTS substrate with id-gated triggers, a resumable chunk backfill, and a query-classification router. Why it matters: every subtle behavior here (trigger WHEN-clause windows, marker seeding order, compact-policy parity between FTS and LIKE fallback) is exactly the kind that passes review and breaks six months later on a weird DB. Suggestion (minimum bar): a migration round-trip test (old layout -> swap -> row-count/{id:row_id} identity/index recreation), a table-driven test for _classify_metadata_query, and a rebuild-window test asserting no gap/duplicate between live triggers and chunk backfill.

  2. hermes_state_schema.py (_migrate_sessions_row_id) — the PRAGMA foreign_key_check runs AFTER COMMIT, so FK violations are discovered only once the destructive swap is durable; they get logged but can never be acted on. Why it matters: you already refuse to commit on count/identity mismatches — an undetected dangling messages.session_id reference deserves the same treatment. Suggestion: move the foreign_key_check inside the transaction, immediately before COMMIT, and raise (rollback) on violations.

  3. Same function — the copy happens in a single BEGIN IMMEDIATE over the whole table. Why it matters: on large installs the first open after upgrade blocks for the duration of a full table copy plus index rebuild (and transiently doubles file size), which reads to users as 'Hermes hangs on launch'. Suggestion: log start/duration/row-count around the migration so support can diagnose, and note the one-time cost in release notes; if stalls prove painful, a chunked copy with the same markers machinery you just built would fit naturally.

  4. hermes_state_common.py (SESSIONS_FTS_SQL triggers) — the H/P-gated trigger design is genuinely good (live-index outside the window, chunk backfill inside it, no gap). One residual assumption: AUTOINCREMENT makes row_id monotonic, so inserts below the progress mark cannot occur — true for Hermes writes, but any external tool doing an explicit-rowid INSERT into sessions mid-rebuild lands silently unindexed forever. Suggestion: one docstring sentence naming that invariant, or a defensive CHECK/trigger guard.

  5. hermes_cli/web_routers/sessions.py:347 — the metadata-discovery lane runs list_sessions_rich(search_query=..., limit>=4x, order_by_last_active=True, include_archived=True) on every search, i.e. a whole-store metadata match ordered by last activity, even when the FTS lanes will separately answer. Acceptable for interactive search, but on big stores this is the new dominant cost of every query. Suggestion: skip the lane when the Unicode/trigram router already produced candidates (pass its hit-count down), or bound it behind the same MAX_FTS5_QUERY_CHARS minimum-length rule the trigram lane uses.

Nice details elsewhere: the pre-drop {id: row_id} identity verification, dropping sessions_defaults index 0 and shifting NOT-NULL substitutes when stripping the rowid prefix, reusing the message-FTS stale-breadcrumb pattern for optional CJK lanes, and keeping id raw while only title/display_name go through the compact transform all show real care.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants