perf(state): CJK bigram FTS index — replace trigram+LIKE routing for session search - #65544
perf(state): CJK bigram FTS index — replace trigram+LIKE routing for session search#65544Soju06 wants to merge 4 commits into
Conversation
ecb04f3 to
e2c308b
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a verified performance gap: current main still sends 1–2-character CJK terms to the LIKE route (hermes_state.py:5353-5380).
Problems
hermes_state.py:1293-1295drops v2 triggers when one process cannot load the extension, but leaves the ready marker intact. A later extension-capable open recreates triggers athermes_state.py:2051and accepts that marker at:1300-1310; writes made in between are absent from an index now considered ready.- This branch predates current main's FTS recovery work (
9e1b1d753).repair_state_db_schema()probes/rebuilds only v1 tables (hermes_state.py:561-668), while maintenance enumerates only v1 indexes (:7311). A v1-retired v2 DB needs these paths updated together. scripts/fts_v2_migrate.py:15-17still instructs users to putHERMES_FTS_V2_READin.env; the advertised config defaults are not added tohermes_cli/config.py.
Suggested changes
- Make trigger loss invalidate v2 durably; require a complete verified backfill before reads resume.
- Integrate v2/tokenizer loading into repair, rebuild, and optimize paths, with end-to-end recovery coverage.
- Add config defaults and remove the user-facing
.envcutover path.
Automated hermes-sweeper review.
| ) | ||
| with self._lock: | ||
| for trig in present_triggers: | ||
| self._conn.execute(f"DROP TRIGGER IF EXISTS {trig}") |
There was a problem hiding this comment.
Dropping triggers leaves fts_v2_ready=1. A later extension-capable SessionDB recreates these triggers in _init_schema() and _probe_fts_v2() accepts the old marker, so rows written during this gap are silently missing from a supposedly ready v2 index. Persist an invalid state here and require a verified full backfill before re-enabling reads.
|
|
||
| conn.execute("BEGIN IMMEDIATE") | ||
| try: | ||
| for typ, name in present: |
There was a problem hiding this comment.
After dropping v1, current main maintenance still enumerates only messages_fts and messages_fts_trigram in SessionDB._FTS_TABLES (hermes_state.py:7311), so optimize/rebuild no longer covers the serving v2 index. Please integrate v2 with those paths before making v1 retirement available.
| content). Progress persists in state_meta, so a killed run resumes. | ||
| 3. Integrity-check the FTS index and print sample-query timings. | ||
|
|
||
| Read cutover is separate and reversible: set HERMES_FTS_V2_READ=1 in |
There was a problem hiding this comment.
This is a user-facing non-secret behavior setting, so it must be configured through agent.fts_v2_read in config.yaml rather than .env. Add the documented defaults to hermes_cli/config.py and update this cutover instruction.
e2c308b to
4b2d707
Compare
|
Thanks for the review — all three points addressed. Trigger-drop invalidation: dropping the v2 triggers now durably invalidates the index by flipping the Repair/maintenance integration: Config surface: Also restacked onto the updated #65541. |
|
@teknium1 Gentle ping — all points from the review here have been addressed (summary in the comment above), the branch is rebased on current main, and CI is green. Ready for another look whenever convenient. |
…ecall reads The gateway shares ONE SessionDB across every agent, so every recall/browse read (session_search discover/scroll/browse, memory prefetch, title resolve) queued behind every writer flush on self._lock — one Python lock in front of a WAL database that natively supports concurrent readers. Measured convoy: a 0.23s FTS query stretched to 112s and a browse flush to 137s while 6-8 concurrent turns flushed hundreds of tool results. Fix: under WAL, read-only methods (get_session, resolve_session_by_title, list_sessions_rich, get_messages, get_messages_around, get_anchored_view, search_messages) run on a per-thread mode=ro connection via _read_ctx(), taking no lock at all. Fresh read transactions begin per statement, so read-your-committed-writes holds for flush-then-search patterns. Non-WAL (NFS DELETE fallback) or read-conn open failure keeps the legacy locked single-connection path, remembered per thread to avoid per-query retries.
…igram+LIKE routing unicode61 indexes a CJK run as ONE token, so 2-char Korean terms (일본, 구글, 우리, ...) could never match and hermes routed any query containing one to a 3-column LIKE full-table scan — measured 3-6.4s CPU per query on the 6.8GB state.db and the #1 base cost behind session_search's 12.4s production average. This adds a ~250-line loadable FTS5 tokenizer (native/fts5_cjk) that wraps unicode61 and re-emits CJK runs as overlapping character bigrams, plus one standalone 3-column FTS5 table (messages_fts_v2) whose single MATCH path serves every query shape the legacy code split across unicode61/trigram/ LIKE. Sub-token positions make FTS5 phrase semantics equal exact substring matching down to 2 chars. Lone 1-char CJK terms stay on the legacy path. Rollout is online and reversible: scripts/fts_v2_migrate.py creates the table + triggers, then backfills in idempotent DELETE+INSERT batches with resume state in state_meta; reads cut over only when HERMES_FTS_V2_READ=1. Triggers are scoped AFTER UPDATE OF content columns, so flag-only updates stop re-tokenizing whole messages (a hidden write amplification in v1). A self-heal guard drops the v2 triggers when a process cannot load the tokenizer, keeping message writes alive with a stale index instead of failing every INSERT.
…ibution One INFO line per slow search (HERMES_SEARCH_SLOW_MS, default 1000ms; 0 logs every call) naming the path taken (fts_v2 / fts5 / trigram / like_scan), elapsed time, row count, and the query. The 2026-07 session_search investigation needed turn-trace archaeology plus workload replay to discover that short-CJK queries were full-scanning the table — with this line the next routing regression is a journalctl grep.
… path Config SoT: agent.fts_v2_read and agent.search_slow_ms become config.yaml-authoritative, bridged to HERMES_FTS_V2_READ / HERMES_SEARCH_SLOW_MS at both gateway bridge sites (startup export + per-turn reload), following the house pattern from gateway-max-iterations-config-authority / config-knob-bridges: config wins over stale env, env stays the cross-process carrier and no-config override. Both knobs carry documented defaults in DEFAULT_CONFIG (fts_v2_read: true, search_slow_ms: 1000); the migration docs point at config.yaml, not .env. Read default flips to ON, gated by a state_meta fts_v2_ready marker so a DB where only the dual-write triggers ever ran (backfill incomplete) never serves searches from a partial index — fts_v2_migrate.py sets the marker only after verification (FTS integrity-check + rowcount parity against messages); _init_schema sets it when creating v2 on an empty DB. Fresh DBs with a loadable tokenizer are now v2-native (no v1/trigram tables at all), and a DB whose v1 objects were retired is never 'repaired' back to an empty messages_fts. The marker is also the durable invalidation channel: when a tokenizer-less process drops the v2 triggers to keep writes working, or when _init_schema recreates the table on a populated DB after an offline repair, the marker flips to needs_backfill — a later extension-capable open recreates the triggers but keeps reads on the legacy route until fts_v2_migrate.py completes a verified full re-backfill (persisted snapshot/progress from prior runs are discarded). While unready, the v1 tables are ensured/rebuilt as the serving fallback rather than skipped. Maintenance and repair treat v2 as a first-class index: messages_fts_v2 joins _FTS_TABLES (optimize/rebuild enumeration, existence-probed like the trigram table), repair_state_db_schema and the _db_opens_cleanly write probe load the cjk_unicode61 tokenizer best-effort and cover v2 in the in-place 'rebuild' pass, and a missing FTS table with dangling sync triggers is now reported unhealthy instead of slipping past the no-such-table carve-out. scripts/fts_v1_drop.py retires the six v1 triggers + two tables behind a preflight (tokenizer loadable, v2 objects present, ready marker, integrity-check, rowcount parity); after the drop the off-flag is ignored rather than silently blinding every search, since there is nothing left to fall back to. VACUUM stays opt-in.
4b2d707 to
4709013
Compare
…native extension) unicode61 indexes a CJK run as ONE token, so 2-char Korean terms (일본, 구글, 우리, ...) can never match it and the trigram tokenizer needs >=3 chars per term — any query containing a 1-2 char CJK token falls through to a LIKE full-table scan (measured 3-6.4s CPU per query on a 6.8GB production state.db; the #1 base cost behind a 12.4s session_search average on CJK workloads). This ships a ~250-line loadable FTS5 tokenizer (no deps) that wraps unicode61: maximal CJK runs inside its tokens are re-emitted as overlapping character bigrams (Lucene CJKAnalyzer semantics), everything else passes through unchanged. FTS5 phrase semantics turn consecutive sub-tokens into exact substring matching down to 2-char terms at index speed. Build: native/fts5_cjk/build.sh -> ~/.hermes/lib/libfts5_cjk.so (override: HERMES_FTS5_CJK_SO). Salvaged from PR #65544; the schema integration lands separately on the v23 external-content layout.
…ibution One INFO line per slow search naming the path taken (fts_cjk / fts5 / trigram / like_scan), elapsed time, row count, and the query. The 2026-07 session_search investigation needed turn-trace archaeology plus workload replay to discover that short-CJK queries were full-scanning the table — with this line the next routing regression is a journalctl grep. Threshold: sessions.search_slow_ms (default 1000ms; 0 logs every call), bridged to HERMES_SEARCH_SLOW_MS. Salvaged from PR #65544 (adapted to the v23 schema in follow-up commits).
…content layout Integration layer for the cjk_unicode61 tokenizer, rebuilt on the v23 schema (the contributed integration in PR #65544 predated it): - messages_fts_cjk: external-content FTS5 over a tool-row-excluding view (same v23 storage discipline as the trigram index it supersedes — zero inline text copies). Serves EVERY CJK query shape the legacy routing split between trigram (>=3 chars/token) and LIKE full scans (1-2 char tokens). Lone 1-char CJK runs and role_filter=['tool'] queries keep their legacy routes. - Dedicated marker pair (fts_cjk_rebuild_high_water/progress) gates the id-scoped triggers, so a cjk-only backfill never gates the complete messages_fts/trigram triggers. - Transitions ride (the existing throttled/resumable chunk engine): fresh DBs are born with the index; legacy v22 DBs land on v23+cjk in one run; already-optimized v23 DBs gaining the tokenizer get a marker-gated backfill; live writes are indexed immediately in every case. - Tokenizer-loss self-heal: a process that can't load the extension drops the cjk triggers (writes keep working), leaves a stale breadcrumb, and the index is rebuilt from scratch on the next optimize run — triggers are never reinstalled over a gap (external-content 'delete' on an unindexed rowid is the FTS5 corruption hazard the marker gating exists to prevent). - Capability classification: 'no such tokenizer: cjk_unicode61' joins the degraded-runtime error class everywhere (read probe, write probe, repair) so tokenizer absence is never misclassified as corruption. - Config: sessions.cjk_fts (default on, inert without the .so) and sessions.search_slow_ms in config.yaml, bridged to env by CLI + gateway (startup + per-turn reload). build.sh falls back to vendored SQLite headers so no libsqlite3-dev is needed. Slow-query log path attribution updated: fts_cjk / fts5 / trigram / like_scan. Tests: 14 lifecycle tests (fresh/legacy/stale/backfill paths, tokenizer-loss round-trip) + 5 config-bridge tests + slow-log suite.
|
Merged via #69423 (rebase-merge — your tokenizer and slow-query-log commits are on main under your authorship: f13f845..). The The schema integration was rebuilt rather than cherry-picked, because #65798 (schema v23, external-content FTS) merged after your branch and replaced the storage model this PR integrated against:
Also kept: your trigger-drop invalidation ordering (marker first, crash-conservative), the lone-1-char-run LIKE routing, and the routing-path slow-log attribution (now Thanks for the thorough work across three review rounds — the responsiveness on the sweeper findings made this salvage straightforward. #65541 (read-path split) is untouched by this merge and remains open for separate review. |
The LIKE fallback OR-joined every non-operator token, so "报告 NOT 草稿"
returned exactly the drafts it was asked to exclude and AND degraded to
OR. Group tokens into OR-separated conjunctive buckets that mirror FTS5
semantics, including operator precedence verified against live FTS5:
implicit AND binds tighter than NOT ("a NOT b c" == "a NOT (b AND c)")
while explicit AND binds looser ("a NOT b AND c" == "(a NOT b) AND c").
NULL tool columns are COALESCEd inside negated terms so NOT does not
silently drop ordinary messages, and the snippet anchor is the first
positive token.
Rebased onto current main after the messages_fts_cjk bigram index
(NousResearch#65544) landed: the bigram route passes operators through to FTS5
correctly, but the LIKE fallback it retains — for DBs where the index
is absent or its backfill is pending, role='tool' queries, and lone
1-char CJK runs — still OR-joined everything. The regression tests now
set _fts_cjk_available = False so they keep exercising that path; all
six fail without the fix.
…native extension) unicode61 indexes a CJK run as ONE token, so 2-char Korean terms (일본, 구글, 우리, ...) can never match it and the trigram tokenizer needs >=3 chars per term — any query containing a 1-2 char CJK token falls through to a LIKE full-table scan (measured 3-6.4s CPU per query on a 6.8GB production state.db; the #1 base cost behind a 12.4s session_search average on CJK workloads). This ships a ~250-line loadable FTS5 tokenizer (no deps) that wraps unicode61: maximal CJK runs inside its tokens are re-emitted as overlapping character bigrams (Lucene CJKAnalyzer semantics), everything else passes through unchanged. FTS5 phrase semantics turn consecutive sub-tokens into exact substring matching down to 2-char terms at index speed. Build: native/fts5_cjk/build.sh -> ~/.hermes/lib/libfts5_cjk.so (override: HERMES_FTS5_CJK_SO). Salvaged from PR NousResearch#65544; the schema integration lands separately on the v23 external-content layout.
…ibution One INFO line per slow search naming the path taken (fts_cjk / fts5 / trigram / like_scan), elapsed time, row count, and the query. The 2026-07 session_search investigation needed turn-trace archaeology plus workload replay to discover that short-CJK queries were full-scanning the table — with this line the next routing regression is a journalctl grep. Threshold: sessions.search_slow_ms (default 1000ms; 0 logs every call), bridged to HERMES_SEARCH_SLOW_MS. Salvaged from PR NousResearch#65544 (adapted to the v23 schema in follow-up commits).
…content layout Integration layer for the cjk_unicode61 tokenizer, rebuilt on the v23 schema (the contributed integration in PR NousResearch#65544 predated it): - messages_fts_cjk: external-content FTS5 over a tool-row-excluding view (same v23 storage discipline as the trigram index it supersedes — zero inline text copies). Serves EVERY CJK query shape the legacy routing split between trigram (>=3 chars/token) and LIKE full scans (1-2 char tokens). Lone 1-char CJK runs and role_filter=['tool'] queries keep their legacy routes. - Dedicated marker pair (fts_cjk_rebuild_high_water/progress) gates the id-scoped triggers, so a cjk-only backfill never gates the complete messages_fts/trigram triggers. - Transitions ride (the existing throttled/resumable chunk engine): fresh DBs are born with the index; legacy v22 DBs land on v23+cjk in one run; already-optimized v23 DBs gaining the tokenizer get a marker-gated backfill; live writes are indexed immediately in every case. - Tokenizer-loss self-heal: a process that can't load the extension drops the cjk triggers (writes keep working), leaves a stale breadcrumb, and the index is rebuilt from scratch on the next optimize run — triggers are never reinstalled over a gap (external-content 'delete' on an unindexed rowid is the FTS5 corruption hazard the marker gating exists to prevent). - Capability classification: 'no such tokenizer: cjk_unicode61' joins the degraded-runtime error class everywhere (read probe, write probe, repair) so tokenizer absence is never misclassified as corruption. - Config: sessions.cjk_fts (default on, inert without the .so) and sessions.search_slow_ms in config.yaml, bridged to env by CLI + gateway (startup + per-turn reload). build.sh falls back to vendored SQLite headers so no libsqlite3-dev is needed. Slow-query log path attribution updated: fts_cjk / fts5 / trigram / like_scan. Tests: 14 lifecycle tests (fresh/legacy/stale/backfill paths, tokenizer-loss round-trip) + 5 config-bridge tests + slow-log suite.
…ibution One INFO line per slow search naming the path taken (fts_cjk / fts5 / trigram / like_scan), elapsed time, row count, and the query. The 2026-07 session_search investigation needed turn-trace archaeology plus workload replay to discover that short-CJK queries were full-scanning the table — with this line the next routing regression is a journalctl grep. Threshold: sessions.search_slow_ms (default 1000ms; 0 logs every call), bridged to HERMES_SEARCH_SLOW_MS. Salvaged from PR NousResearch#65544 (adapted to the v23 schema in follow-up commits).
…content layout Integration layer for the cjk_unicode61 tokenizer, rebuilt on the v23 schema (the contributed integration in PR NousResearch#65544 predated it): - messages_fts_cjk: external-content FTS5 over a tool-row-excluding view (same v23 storage discipline as the trigram index it supersedes — zero inline text copies). Serves EVERY CJK query shape the legacy routing split between trigram (>=3 chars/token) and LIKE full scans (1-2 char tokens). Lone 1-char CJK runs and role_filter=['tool'] queries keep their legacy routes. - Dedicated marker pair (fts_cjk_rebuild_high_water/progress) gates the id-scoped triggers, so a cjk-only backfill never gates the complete messages_fts/trigram triggers. - Transitions ride (the existing throttled/resumable chunk engine): fresh DBs are born with the index; legacy v22 DBs land on v23+cjk in one run; already-optimized v23 DBs gaining the tokenizer get a marker-gated backfill; live writes are indexed immediately in every case. - Tokenizer-loss self-heal: a process that can't load the extension drops the cjk triggers (writes keep working), leaves a stale breadcrumb, and the index is rebuilt from scratch on the next optimize run — triggers are never reinstalled over a gap (external-content 'delete' on an unindexed rowid is the FTS5 corruption hazard the marker gating exists to prevent). - Capability classification: 'no such tokenizer: cjk_unicode61' joins the degraded-runtime error class everywhere (read probe, write probe, repair) so tokenizer absence is never misclassified as corruption. - Config: sessions.cjk_fts (default on, inert without the .so) and sessions.search_slow_ms in config.yaml, bridged to env by CLI + gateway (startup + per-turn reload). build.sh falls back to vendored SQLite headers so no libsqlite3-dev is needed. Slow-query log path attribution updated: fts_cjk / fts5 / trigram / like_scan. Tests: 14 lifecycle tests (fresh/legacy/stale/backfill paths, tokenizer-loss round-trip) + 5 config-bridge tests + slow-log suite.
Stacked on #65541 (read-path split) — review the last three commits.
Problem
search_messagesroutes queries three ways: unicode61 FTS5, the trigram table, or a LIKE fallback. unicode61 indexes a CJK run as ONE token, so a 2-char Korean term (일본, 구글, 우리, …) can never match it; the trigram tokenizer needs ≥3 chars per term. Result: any query containing a 1-2 char CJK token full-scans messages with LIKE across three columns — measured 3-6.4s CPU per query on a 6.8GB production DB, and the dominant base cost behind a 12.4s in-gatewaysession_searchaverage (Korean/Japanese/Chinese conversation workloads hit this constantly — 2-char words are everywhere in Korean).The v1 layout also triple-stores text (standalone FTS content copies in both tables; the trigram index alone was larger than the messages data) and its unconditional
AFTER UPDATEtriggers re-tokenize whole messages on flag-only updates.Design
cjk_unicode61tokenizer (native/fts5_cjk/fts5_cjk.c, ~250 lines, no deps): wraps unicode61; CJK runs inside its tokens are re-emitted as overlapping character bigrams (Lucene CJKAnalyzer semantics), everything else passes through. FTS5 turns consecutive tokens from one query term into a phrase, so a bare term like 캘린더 gets exact substring semantics down to 2 chars at index speed. Built withnative/fts5_cjk/build.sh→~/.hermes/lib/libfts5_cjk.so(override:HERMES_FTS5_CJK_SO).messages_fts_v2: one standalone 3-column FTS5 table whose single MATCH path replaces the 3-way routing. Triggers are scopedAFTER UPDATE OF content, tool_name, tool_calls. Lone 1-char CJK terms keep the LIKE path (a bigram index only holds unigrams for isolated chars).scripts/fts_v2_migrate.py): create table+triggers → idempotent DELETE+INSERT batched backfill (resume state instate_meta) → integrity-check → sets afts_v2_readymarker. Reads are gated on that marker so a partially-backfilled index is never served. Production run: 385k messages in 91s with live writers.agent.fts_v2_read(default on once the index is ready) andagent.search_slow_msare config.yaml-authoritative, bridged to env at both the startup export block and the per-turn reload path. A slow-search log line names the routing path (fts_v2/fts5/trigram/like_scan) so future routing regressions are a journalctl grep.scripts/fts_v1_drop.py, operator-run): preflight (tokenizer loadable, v2 objects present, ready marker, integrity-check, rowcount parity) then drops the six v1 triggers + two tables. Freed ~5.9GB on our production DB. Fresh DBs with a loadable tokenizer are v2-native and never create the v1 tables.Production results (fork, 2026-07-16)
나쵸 minpeter"shared default" AND "웅기"구글 캘린더 일정 없음 브리핑 …Recall parity held on a replayed real-query benchmark (10/13 confirmed-relevant targets found, identical to legacy), and v2's FTS5 semantics fix a latent bug: the LIKE fallback matched multi-token queries as OR instead of AND.
Tests
tests/test_fts_v2_cjk.py(tokenizer built on the fly; skips without a C toolchain),tests/test_search_slow_query_log.py,tests/gateway/test_fts_v2_config_bridge.py— bigram matching incl. no-false-positive-across-words, trigger mirroring, flag-only-update scoping, self-heal, partial-backfill refusal, fresh-DB v2-native, config bridges.