From 2d6099754c5cd2706f1022dd9b041346535cfed2 Mon Sep 17 00:00:00 2001 From: exiao Date: Tue, 14 Jul 2026 15:14:44 -0400 Subject: [PATCH 1/7] fix(state): add sessions.trigram_fts flag to drop the trigram FTS index The secondary trigram FTS5 index (messages_fts_trigram) stores its own full copy of every message plus a 3-gram index (3-5x the raw text) and fires on every message INSERT. On heavy gateway+cron users it dominates state.db size and lengthens WAL write-lock holds enough to exhaust the 15-retry write-lock budget, surfacing as 'state.db routing save failed: database is locked' and stalling the gateway on restart. Add sessions.trigram_fts (default True = current behavior). When false, schema init drops the trigram index + its triggers and never recreates them; _trigram_available is forced False so search_messages uses the existing base-FTS/LIKE fallback. The base messages_fts word index (used by recall and session search) is unaffected. Setting it back to true recreates the index on next open. Patch note: ~/.hermes/plans/hermes-patches/2026-07-14-trigram-fts-config-flag.md --- hermes_cli/config.py | 12 +++++ hermes_state.py | 94 ++++++++++++++++++++++++++++++++++---- tests/test_hermes_state.py | 53 +++++++++++++++++++++ 3 files changed, 149 insertions(+), 10 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 0d7d0addf4096..eb6a2589f1c8c 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -3068,6 +3068,18 @@ def _ensure_hermes_home_managed(home: Path): # GBs of disk on heavy users. Opt in only if you have an external # tool that consumes the JSON files directly. "write_json_snapshots": False, + # Secondary trigram FTS5 index over message content, used for + # substring and CJK (Chinese/Japanese/Korean) search. It stores + # its own full copy of every message plus a 3-character-gram index, + # so it is by far the largest object in state.db (3-5x the raw + # message text) and fires on every message INSERT. On heavy users + # it dominates DB size and slows writes enough to blow the + # write-lock retry budget ("database is locked"). When false, the + # index is dropped at startup and never recreated; substring/CJK + # search transparently falls back to a LIKE scan (the base + # ``messages_fts`` word index — used by recall and session search — + # is unaffected). Default true preserves current behavior. + "trigram_fts": True, }, # Contextual first-touch onboarding hints (see agent/onboarding.py). diff --git a/hermes_state.py b/hermes_state.py index a0b6dfd09ccd1..2840509f77a99 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -175,6 +175,15 @@ def _delete_delegate_children(conn, parent_ids: List[str]) -> List[str]: "messages_fts_trigram_update", ) +# Subset of _FTS_TRIGGERS that feed the trigram index specifically. Dropped +# (leaving the base messages_fts triggers intact) when sessions.trigram_fts +# is disabled — see SessionDB._drop_trigram_fts. +_TRIGRAM_FTS_TRIGGERS = ( + "messages_fts_trigram_insert", + "messages_fts_trigram_delete", + "messages_fts_trigram_update", +) + def _set_last_init_error(msg: Optional[str]) -> None: """Record (or clear) the most recent state.db init failure. @@ -994,6 +1003,24 @@ def _connect_and_init(): # ── Core write helper ── + @staticmethod + def _trigram_fts_enabled() -> bool: + """Read ``sessions.trigram_fts`` from config (default True). + + Lazy import + best-effort: any config-load failure defaults to the + historical behavior (trigram enabled). Cheap enough to call once per + schema init; not on the message write hot path.""" + try: + from hermes_cli.config import load_config + + cfg = load_config() + sessions = cfg.get("sessions") if isinstance(cfg, dict) else None + if isinstance(sessions, dict) and "trigram_fts" in sessions: + return bool(sessions["trigram_fts"]) + except Exception: + pass + return True + @staticmethod def _is_fts5_unavailable_error(exc: sqlite3.OperationalError) -> bool: err = str(exc).lower() @@ -1058,6 +1085,35 @@ def _drop_fts_triggers(cursor: sqlite3.Cursor) -> None: except sqlite3.OperationalError: pass + @staticmethod + def _drop_trigram_fts(cursor: sqlite3.Cursor) -> bool: + """Drop the trigram FTS5 index and its triggers. Returns True if the + virtual table existed and was removed (so the caller knows a VACUUM + would reclaim space). Used when ``sessions.trigram_fts`` is disabled: + the trigram index is the single largest object in state.db and fires + on every message INSERT, so a heavy user opts out to shrink the DB and + speed up writes. Substring/CJK search falls back to a LIKE scan; the + base ``messages_fts`` word index is untouched.""" + for trigger in _TRIGRAM_FTS_TRIGGERS: + try: + cursor.execute(f"DROP TRIGGER IF EXISTS {trigger}") + except sqlite3.OperationalError: + pass + existed = False + try: + row = cursor.execute( + "SELECT 1 FROM sqlite_master " + "WHERE type = 'table' AND name = 'messages_fts_trigram'" + ).fetchone() + existed = row is not None + cursor.execute("DROP TABLE IF EXISTS messages_fts_trigram") + except sqlite3.OperationalError: + # FTS5 module missing entirely, or the shadow tables are in a + # state we can't drop cleanly — leave it be; nothing queries it + # when the flag is off. + pass + return existed + @staticmethod def _fts_trigger_count(cursor: sqlite3.Cursor) -> int: placeholders = ",".join("?" for _ in _FTS_TRIGGERS) @@ -1572,22 +1628,40 @@ def _init_schema(self): # FTS5 setup. Run the DDL even when the virtual table exists so # CREATE TRIGGER IF NOT EXISTS repairs trigger-only degradation from # an earlier no-FTS5 runtime. - triggers_need_repair = self._fts_trigger_count(cursor) < len(_FTS_TRIGGERS) + trigram_wanted = self._trigram_fts_enabled() + # Expected trigger count depends on whether the trigram index is + # enabled: with it off we intentionally keep only the 3 base + # messages_fts triggers, so comparing against all 6 would falsely + # flag a repair (and rebuild base FTS) on every startup. + expected_trigger_count = ( + len(_FTS_TRIGGERS) if trigram_wanted else len(_FTS_TRIGGERS) - len(_TRIGRAM_FTS_TRIGGERS) + ) + triggers_need_repair = self._fts_trigger_count(cursor) < expected_trigger_count self._fts_enabled = self._ensure_fts_schema(cursor, "messages_fts", FTS_SQL) # Trigram FTS5 for CJK/substring search. This is optional relative # to the main FTS table; if it cannot be created, CJK search falls - # back to LIKE. + # back to LIKE. When sessions.trigram_fts is disabled, drop it + # instead of creating it — it is the largest object in state.db and + # fires on every message INSERT. Search transparently falls back to + # LIKE (see search_messages / self._trigram_available). if self._fts_enabled: - trigram_enabled = self._ensure_fts_schema( - cursor, "messages_fts_trigram", FTS_TRIGRAM_SQL - ) - self._trigram_available = trigram_enabled - if triggers_need_repair: - self._rebuild_fts_indexes( - cursor, - include_trigram=trigram_enabled, + if trigram_wanted: + trigram_enabled = self._ensure_fts_schema( + cursor, "messages_fts_trigram", FTS_TRIGRAM_SQL ) + self._trigram_available = trigram_enabled + if triggers_need_repair: + self._rebuild_fts_indexes( + cursor, + include_trigram=trigram_enabled, + ) + else: + self._drop_trigram_fts(cursor) + self._trigram_available = False + if triggers_need_repair: + # Base FTS only — trigram is intentionally absent. + self._rebuild_fts_indexes(cursor, include_trigram=False) self._conn.commit() diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 99f2ccb71e9c6..a0d478c258f3b 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -547,6 +547,59 @@ def test_is_trigram_unavailable_error(self): assert SessionDB._is_trigram_unavailable_error(generic_err) is False assert SessionDB._is_trigram_unavailable_error(fts5_err) is False + def test_trigram_fts_config_flag_false_drops_index(self, tmp_path, monkeypatch): + """sessions.trigram_fts: false drops the trigram index + its triggers, + keeps base FTS + its triggers, and leaves word search working.""" + db_path = tmp_path / "state.db" + # Build a DB WITH the trigram index (default flag = True). + monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: True)) + seeded = SessionDB(db_path=db_path) + try: + seeded.create_session(session_id="s1", source="cli") + seeded.append_message("s1", role="user", content="the quick brown fox") + assert seeded._trigram_available is True + assert seeded._fts_table_exists("messages_fts_trigram") is True + finally: + seeded.close() + + # Reopen with the flag OFF — trigram must be dropped, base FTS intact. + monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: False)) + reopened = SessionDB(db_path=db_path) + try: + assert reopened._trigram_available is False + assert reopened._fts_table_exists("messages_fts_trigram") is False + assert reopened._fts_table_exists("messages_fts") is True + # Base triggers survive; trigram triggers are gone. + names = { + r[0] + for r in reopened._conn.execute( + "SELECT name FROM sqlite_master WHERE type='trigger' " + "AND name LIKE 'messages_fts%'" + ).fetchall() + } + assert names == { + "messages_fts_insert", + "messages_fts_delete", + "messages_fts_update", + } + # Word search still works through the base index. + assert len(reopened.search_messages("quick")) == 1 + # A new write does not resurrect the trigram index. + reopened.append_message("s1", role="assistant", content="lazy dog sleeps") + assert reopened._fts_table_exists("messages_fts_trigram") is False + assert len(reopened.search_messages("lazy")) == 1 + finally: + reopened.close() + + # Reopen again with the flag back ON — trigram is recreated. + monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: True)) + restored = SessionDB(db_path=db_path) + try: + assert restored._trigram_available is True + assert restored._fts_table_exists("messages_fts_trigram") is True + finally: + restored.close() + def test_db_initializes_without_trigram_tokenizer(self, tmp_path, monkeypatch): """SessionDB must not crash when FTS5 exists but trigram tokenizer is missing.""" real_connect = sqlite3.connect From 776d4666405395c41d975488380c96520d71c78f Mon Sep 17 00:00:00 2001 From: exiao Date: Tue, 14 Jul 2026 15:26:21 -0400 Subject: [PATCH 2/7] fix(state): evaluate base vs trigram FTS trigger repair independently Addresses two P2s from independent review of the sessions.trigram_fts flag: 1. Base-trigger repair could be masked when trigram is disabled. Comparing the total FTS trigger count against a flag-adjusted expectation let a DB with 0 base + 3 stale trigram triggers tie the threshold and skip the base FTS rebuild, leaving rows written during the gap unsearchable. Count base triggers on their own (_base_fts_trigger_count / _BASE_FTS_TRIGGERS) so base health is judged independently of trigram presence. 2. The v11 migration built and backfilled messages_fts_trigram before the runtime block dropped it, so a legacy v10 user opting out still paid the full trigram rebuild + WAL churn once. Gate the v11 trigram build on _trigram_fts_enabled() and drop any pre-existing copy instead. Regression test: test_trigram_disabled_still_repairs_missing_base_triggers. Suite: 75 passed. --- hermes_state.py | 80 +++++++++++++++++++++++++++++--------- tests/test_hermes_state.py | 55 ++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 18 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index 2840509f77a99..3740ddf8212f3 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -184,6 +184,16 @@ def _delete_delegate_children(conn, parent_ids: List[str]) -> List[str]: "messages_fts_trigram_update", ) +# The base messages_fts triggers, always expected regardless of the trigram +# flag. Used to detect base-trigger degradation independently of trigram +# presence (comparing against the 6-tuple total would let a healthy +# trigram-disabled DB mask 3 missing base triggers). +_BASE_FTS_TRIGGERS = ( + "messages_fts_insert", + "messages_fts_delete", + "messages_fts_update", +) + def _set_last_init_error(msg: Optional[str]) -> None: """Record (or clear) the most recent state.db init failure. @@ -1124,6 +1134,22 @@ def _fts_trigger_count(cursor: sqlite3.Cursor) -> int: ).fetchone() return int(row[0] if not isinstance(row, sqlite3.Row) else row[0]) + @staticmethod + def _base_fts_trigger_count(cursor: sqlite3.Cursor) -> int: + """Count only the base messages_fts triggers (excludes trigram). + + Base-trigger degradation must be detected independently of whether the + trigram triggers are present, otherwise a trigram-disabled DB with all + 3 base triggers missing but 3 trigram triggers still around would tie + the 6-tuple total and skip the base rebuild.""" + placeholders = ",".join("?" for _ in _BASE_FTS_TRIGGERS) + row = cursor.execute( + f"SELECT COUNT(*) FROM sqlite_master " + f"WHERE type = 'trigger' AND name IN ({placeholders})", + _BASE_FTS_TRIGGERS, + ).fetchone() + return int(row[0] if not isinstance(row, sqlite3.Row) else row[0]) + @staticmethod def _rebuild_fts_indexes( cursor: sqlite3.Cursor, @@ -1550,18 +1576,26 @@ def _init_schema(self): "COALESCE(tool_calls, '') " "FROM messages" ) - trigram_ok = self._ensure_fts_schema( - cursor, "messages_fts_trigram", FTS_TRIGRAM_SQL - ) - if trigram_ok: - cursor.execute( - "INSERT INTO messages_fts_trigram(rowid, content) " - "SELECT id, " - "COALESCE(content, '') || ' ' || " - "COALESCE(tool_name, '') || ' ' || " - "COALESCE(tool_calls, '') " - "FROM messages" + trigram_ok = False + if self._trigram_fts_enabled(): + trigram_ok = self._ensure_fts_schema( + cursor, "messages_fts_trigram", FTS_TRIGRAM_SQL ) + if trigram_ok: + cursor.execute( + "INSERT INTO messages_fts_trigram(rowid, content) " + "SELECT id, " + "COALESCE(content, '') || ' ' || " + "COALESCE(tool_name, '') || ' ' || " + "COALESCE(tool_calls, '') " + "FROM messages" + ) + else: + # Trigram disabled — don't create/backfill it here + # only to drop it in the runtime FTS block below. + # Saves the full trigram rebuild + WAL churn on the + # opt-out startup. Also drop any pre-existing copy. + self._drop_trigram_fts(cursor) if not base_fts_ok: fts_migrations_complete = False # Track trigram availability for CJK LIKE fallback. @@ -1629,14 +1663,24 @@ def _init_schema(self): # CREATE TRIGGER IF NOT EXISTS repairs trigger-only degradation from # an earlier no-FTS5 runtime. trigram_wanted = self._trigram_fts_enabled() - # Expected trigger count depends on whether the trigram index is - # enabled: with it off we intentionally keep only the 3 base - # messages_fts triggers, so comparing against all 6 would falsely - # flag a repair (and rebuild base FTS) on every startup. - expected_trigger_count = ( - len(_FTS_TRIGGERS) if trigram_wanted else len(_FTS_TRIGGERS) - len(_TRIGRAM_FTS_TRIGGERS) + # Detect trigger degradation on the base and trigram sets + # independently. Counting the 6-tuple total would let a healthy + # trigram-disabled DB (3 base + 0 trigram) tie a corrupt one + # (0 base + 3 stale trigram) and skip the base FTS rebuild. + base_triggers_ok = ( + self._base_fts_trigger_count(cursor) == len(_BASE_FTS_TRIGGERS) ) - triggers_need_repair = self._fts_trigger_count(cursor) < expected_trigger_count + trigram_triggers_present = ( + self._fts_trigger_count(cursor) - self._base_fts_trigger_count(cursor) + ) + if trigram_wanted: + triggers_need_repair = ( + not base_triggers_ok + or trigram_triggers_present < len(_TRIGRAM_FTS_TRIGGERS) + ) + else: + # Trigram intentionally absent — only base health matters. + triggers_need_repair = not base_triggers_ok self._fts_enabled = self._ensure_fts_schema(cursor, "messages_fts", FTS_SQL) # Trigram FTS5 for CJK/substring search. This is optional relative diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index a0d478c258f3b..1512fc456af82 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -600,6 +600,61 @@ def test_trigram_fts_config_flag_false_drops_index(self, tmp_path, monkeypatch): finally: restored.close() + def test_trigram_disabled_still_repairs_missing_base_triggers( + self, tmp_path, monkeypatch + ): + """Regression (reviewer P2): with trigram disabled, a DB missing base + FTS triggers must still trigger a base rebuild so word search stays + correct. The old 6-tuple count let 3 stale trigram triggers mask 3 + missing base triggers and skip the rebuild.""" + db_path = tmp_path / "state.db" + # Build a normal DB WITH trigram (default), seed a searchable row. + monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: True)) + seeded = SessionDB(db_path=db_path) + try: + seeded.create_session(session_id="s1", source="cli") + seeded.append_message("s1", role="user", content="alpha indexed row") + # Simulate base-trigger degradation: drop only the 3 base triggers, + # leaving the 3 trigram triggers in place (the exact tie the old + # total-count heuristic missed). + for trig in ( + "messages_fts_insert", + "messages_fts_delete", + "messages_fts_update", + ): + seeded._conn.execute(f"DROP TRIGGER IF EXISTS {trig}") + seeded._conn.commit() + # Write a row while base triggers are gone — it won't be in base FTS. + seeded.append_message("s1", role="assistant", content="betaunindexed row") + finally: + seeded.close() + + # Reopen with trigram DISABLED. base_triggers_ok must be False → repair. + monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: False)) + reopened = SessionDB(db_path=db_path) + try: + assert reopened._trigram_available is False + assert reopened._fts_table_exists("messages_fts_trigram") is False + # Base triggers restored (3 of them, none trigram). + names = { + r[0] + for r in reopened._conn.execute( + "SELECT name FROM sqlite_master WHERE type='trigger' " + "AND name LIKE 'messages_fts%'" + ).fetchall() + } + assert names == { + "messages_fts_insert", + "messages_fts_delete", + "messages_fts_update", + } + # The row written during the trigger gap must now be searchable — + # proves the base rebuild actually ran (not just trigger recreate). + assert len(reopened.search_messages("betaunindexed")) == 1 + assert len(reopened.search_messages("alpha")) == 1 + finally: + reopened.close() + def test_db_initializes_without_trigram_tokenizer(self, tmp_path, monkeypatch): """SessionDB must not crash when FTS5 exists but trigram tokenizer is missing.""" real_connect = sqlite3.connect From 06904f99546b85e8f065e4ee4b6a2a4a266e2a0a Mon Sep 17 00:00:00 2001 From: exiao Date: Tue, 14 Jul 2026 15:40:59 -0400 Subject: [PATCH 3/7] fix(state): hide rewound rows in the CJK LIKE search fallback Codex review P2: the CJK LIKE fallback in search_messages omitted the (m.active = 1 OR m.compacted = 1) predicate that the base and trigram FTS paths apply, so it returned messages the user rewound (active=0, compacted=0). Pre-existing, but this PR widens exposure because sessions.trigram_fts=false now routes 3+ char CJK queries into that fallback by config, not just short-CJK ones. Add the filter, gated on include_inactive to match the other paths. Regression test test_cjk_like_fallback_excludes_rewound_rows (fail-before/pass-after verified). Suite: 77 passed. Not fixed here: the same fallback drops AND/NOT boolean operators (long-standing, predates this PR, separate change). --- hermes_state.py | 7 +++++++ tests/test_hermes_state.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/hermes_state.py b/hermes_state.py index 3740ddf8212f3..f53e8b4235feb 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -4842,6 +4842,13 @@ def search_messages( ) like_params += [f"%{esc}%", f"%{esc}%", f"%{esc}%"] like_where = [f"({' OR '.join(token_clauses)})"] + if not include_inactive: + # Match the base + trigram FTS paths: hide rewound rows + # (active=0, compacted=0). Without this the LIKE fallback + # leaks messages the user took back — reachable for more + # installs now that sessions.trigram_fts can route 3+ char + # CJK queries here by config, not just short-CJK ones. + like_where.append("(m.active = 1 OR m.compacted = 1)") if source_filter is not None: like_where.append(f"s.source IN ({','.join('?' for _ in source_filter)})") like_params.extend(source_filter) diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 1512fc456af82..2581690de865a 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -754,6 +754,37 @@ def connect_without_trigram(*args, **kwargs): finally: db.close() + def test_cjk_like_fallback_excludes_rewound_rows(self, tmp_path, monkeypatch): + """Regression (Codex P2): the CJK LIKE fallback must hide rewound rows + (active=0, compacted=0) by default, like the base + trigram FTS paths. + Otherwise disabling trigram leaks messages the user took back.""" + real_connect = sqlite3.connect + db_path = tmp_path / "state.db" + + def connect_without_trigram(*args, **kwargs): + kwargs["factory"] = _NoTrigramConnection + return real_connect(*args, **kwargs) + + monkeypatch.setattr("hermes_state.sqlite3.connect", connect_without_trigram) + db = SessionDB(db_path=db_path) + try: + db.create_session(session_id="s1", source="cli") + db.append_message("s1", role="user", content="大别山项目计划书") + # Rewind it the way undo does: active=0, compacted=0. + db._conn.execute( + "UPDATE messages SET active = 0 WHERE session_id = 's1'" + ) + db._conn.commit() + + # Default search must NOT return the rewound row. + assert db.search_messages("大别山") == [] + # include_inactive=True still finds it. + results = db.search_messages("大别山", include_inactive=True) + assert len(results) == 1 + assert "大别山" in results[0]["snippet"] + finally: + db.close() + # ========================================================================= # Message storage From 8905d9acc51c19ea806a419a9f786cfcf96c938c Mon Sep 17 00:00:00 2001 From: exiao Date: Tue, 14 Jul 2026 15:45:37 -0400 Subject: [PATCH 4/7] fix(state): preserve AND/NOT boolean semantics in disabled-trigram CJK LIKE fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the active-row filter added in the prior commit. Codex P2: when sessions.trigram_fts is false, all 3+ char CJK searches route through the LIKE fallback, which flattened AND / OR / NOT to a single OR of every term. So '大别山项目 NOT 桂林项目' returned rows containing the excluded term and 'A AND B' returned rows matching only A. - Walk the tokens honoring AND / OR / NOT (adjacent terms default to AND per FTS5). NOT attaches as 'AND NOT (...)'. - COALESCE the LIKE columns to '' so a NULL tool_name/tool_calls yields FALSE, not NULL — otherwise 'AND NOT (FALSE OR NULL)' collapses to NULL and drops every row. - Log a VACUUM hint when the trigram index is actually dropped (it can be multiple GB; SQLite won't shrink the file automatically). Tests: test_cjk_like_fallback_preserves_{not,and}_operator (fail-before/ pass-after). tests/test_hermes_state.py: 344 passed. --- hermes_state.py | 80 +++++++++++++++++++++++++++++--------- tests/test_hermes_state.py | 24 ++++++++++++ 2 files changed, 85 insertions(+), 19 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index f53e8b4235feb..07d3f8054eca3 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1701,8 +1701,17 @@ def _init_schema(self): include_trigram=trigram_enabled, ) else: - self._drop_trigram_fts(cursor) + dropped = self._drop_trigram_fts(cursor) self._trigram_available = False + if dropped: + # The trigram index can be multiple GB. Dropping it + # frees pages but SQLite won't shrink the file on disk + # automatically — point the user at VACUUM. + logger.info( + "Dropped the trigram FTS index (sessions.trigram_fts " + "disabled). Run VACUUM on state.db to reclaim the " + "freed disk space." + ) if triggers_need_repair: # Base FTS only — trigram is intentionally absent. self._rebuild_fts_indexes(cursor, include_trigram=False) @@ -4826,28 +4835,61 @@ def search_messages( if not _trigram_succeeded: # Short / mixed CJK query, trigram unavailable, or trigram # <3 CJK chars. Fall back to LIKE substring search. - # For multi-token OR queries (e.g. "广西 OR 桂林 OR 漓江"), - # build one LIKE condition per non-operator token so each term - # is matched independently (#20494). - non_op_tokens = [ - t for t in raw_query.split() - if t.upper() not in {"AND", "OR", "NOT"} - ] or [raw_query] - token_clauses = [] + # Preserve the query's boolean structure (#20494): walk the + # tokens left-to-right, honoring AND / OR / NOT operators so + # `A NOT B` excludes B and `A AND B` requires both — the flat + # OR-everything shape silently over-matched when this fallback + # became the only path for disabled-trigram installs. like_params: list = [] - for tok in non_op_tokens: + + def _term_clause(tok: str) -> str: esc = tok.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") - token_clauses.append( - "(m.content LIKE ? ESCAPE '\\' OR m.tool_name LIKE ? ESCAPE '\\' OR m.tool_calls LIKE ? ESCAPE '\\')" + like_params.extend([f"%{esc}%", f"%{esc}%", f"%{esc}%"]) + # COALESCE to '' so a NULL column yields FALSE (not NULL) + # under LIKE — critical for the `AND NOT (...)` branch, + # where `NOT (FALSE OR NULL)` would otherwise be NULL and + # silently drop every row (tool_name/tool_calls are NULL on + # plain user/assistant messages). + return ( + "(COALESCE(m.content, '') LIKE ? ESCAPE '\\' " + "OR COALESCE(m.tool_name, '') LIKE ? ESCAPE '\\' " + "OR COALESCE(m.tool_calls, '') LIKE ? ESCAPE '\\')" ) - like_params += [f"%{esc}%", f"%{esc}%", f"%{esc}%"] - like_where = [f"({' OR '.join(token_clauses)})"] + + tokens = raw_query.split() + non_op_tokens = [ + t for t in tokens if t.upper() not in {"AND", "OR", "NOT"} + ] or [raw_query] + # Build the boolean expression. Adjacent terms without an + # explicit operator default to AND (FTS5 semantics). NOT + # attaches to the following term as `AND NOT (...)`. + expr_parts: list = [] + pending_op = None # None | "AND" | "OR" | "NOT" + for tok in tokens: + upper = tok.upper() + if upper in {"AND", "OR", "NOT"}: + pending_op = upper + continue + clause = _term_clause(tok) + if not expr_parts: + expr_parts.append(clause) + elif pending_op == "OR": + expr_parts.append(f"OR {clause}") + elif pending_op == "NOT": + expr_parts.append(f"AND NOT {clause}") + else: # explicit AND or implicit adjacency + expr_parts.append(f"AND {clause}") + pending_op = None + if not expr_parts: + # No CJK/word tokens survived (pure operators or empty) — + # match the raw query as a single substring. + expr_parts.append(_term_clause(raw_query)) + like_where = [f"({' '.join(expr_parts)})"] if not include_inactive: - # Match the base + trigram FTS paths: hide rewound rows - # (active=0, compacted=0). Without this the LIKE fallback - # leaks messages the user took back — reachable for more - # installs now that sessions.trigram_fts can route 3+ char - # CJK queries here by config, not just short-CJK ones. + # Mirror the FTS/trigram paths: hide rewound/undo rows + # (active=0, compacted=0) while keeping compaction-archived + # rows discoverable. Without this the LIKE fallback leaked + # deactivated messages for disabled-trigram installs. like_where.append("(m.active = 1 OR m.compacted = 1)") if source_filter is not None: like_where.append(f"s.source IN ({','.join('?' for _ in source_filter)})") diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 2581690de865a..730a221429004 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -1933,6 +1933,30 @@ def test_cjk_short_token_or_query_preserves_filters(self, db): assert len(results) == 1 assert results[0]["source"] == "telegram" + def test_cjk_like_fallback_preserves_not_operator(self, db): + """NOT in a CJK query must exclude the negated term in the LIKE + fallback (reached when sessions.trigram_fts routes long CJK here), + not silently OR every term together.""" + db.create_session(session_id="s1", source="cli") + db.create_session(session_id="s2", source="cli") + db.append_message("s1", role="user", content="大别山项目进展顺利") + db.append_message("s2", role="user", content="大别山项目和桂林项目对比") + db._trigram_available = False + results = db.search_messages("大别山项目 NOT 桂林项目") + session_ids = {r["session_id"] for r in results} + assert session_ids == {"s1"}, "NOT term must exclude the 桂林项目 row" + + def test_cjk_like_fallback_preserves_and_operator(self, db): + """AND in a CJK query must require both terms in the LIKE fallback.""" + db.create_session(session_id="s1", source="cli") + db.create_session(session_id="s2", source="cli") + db.append_message("s1", role="user", content="大别山项目和桂林项目一起讨论") + db.append_message("s2", role="user", content="只有大别山项目被提到") + db._trigram_available = False + results = db.search_messages("大别山项目 AND 桂林项目") + session_ids = {r["session_id"] for r in results} + assert session_ids == {"s1"}, "AND must require both terms" + # ========================================================================= # Session search and listing From 83f841a60ddbacdbde63a6de33ba66160921e6cd Mon Sep 17 00:00:00 2001 From: exiao Date: Tue, 14 Jul 2026 15:55:35 -0400 Subject: [PATCH 5/7] fix(state): don't report a trigram drop that a DB lock thwarted Codex re-review P2: _drop_trigram_fts swallowed a broad OperationalError around DROP TABLE and still returned existed=True, so a 'database is locked' during startup (gateway + cron) would set _trigram_available=False and log the new 'dropped, run VACUUM' hint while the table + triggers actually survived. Re-check sqlite_master after the DROP attempt and return True only when the table that existed is genuinely gone; return False on any lock/undroppable state so the caller's VACUUM hint never misleads. Test: test_drop_trigram_fts_returns_false_when_drop_thwarted. --- hermes_state.py | 40 +++++++++++++++++++++++++++----------- tests/test_hermes_state.py | 33 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index 07d3f8054eca3..c36d56794e0e5 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1097,13 +1097,20 @@ def _drop_fts_triggers(cursor: sqlite3.Cursor) -> None: @staticmethod def _drop_trigram_fts(cursor: sqlite3.Cursor) -> bool: - """Drop the trigram FTS5 index and its triggers. Returns True if the - virtual table existed and was removed (so the caller knows a VACUUM - would reclaim space). Used when ``sessions.trigram_fts`` is disabled: - the trigram index is the single largest object in state.db and fires - on every message INSERT, so a heavy user opts out to shrink the DB and - speed up writes. Substring/CJK search falls back to a LIKE scan; the - base ``messages_fts`` word index is untouched.""" + """Drop the trigram FTS5 index and its triggers. Returns True only if + the virtual table existed and was actually removed (so the caller knows + a VACUUM would reclaim space). Used when ``sessions.trigram_fts`` is + disabled: the trigram index is the single largest object in state.db and + fires on every message INSERT, so a heavy user opts out to shrink the DB + and speed up writes. Substring/CJK search falls back to a LIKE scan; the + base ``messages_fts`` word index is untouched. + + A concurrent writer can hold the DB lock during startup (gateway + cron), + so ``DROP`` may raise ``database is locked``. We do NOT treat that as a + successful drop: the table/triggers stay in place and writes keep paying + the trigram cost until the next uncontended startup, so returning False + keeps the caller from logging a misleading "dropped, run VACUUM" hint. + """ for trigger in _TRIGRAM_FTS_TRIGGERS: try: cursor.execute(f"DROP TRIGGER IF EXISTS {trigger}") @@ -1118,11 +1125,22 @@ def _drop_trigram_fts(cursor: sqlite3.Cursor) -> bool: existed = row is not None cursor.execute("DROP TABLE IF EXISTS messages_fts_trigram") except sqlite3.OperationalError: - # FTS5 module missing entirely, or the shadow tables are in a - # state we can't drop cleanly — leave it be; nothing queries it - # when the flag is off. + # FTS5 module missing, shadow tables in an undroppable state, or the + # DB is locked by another process. Re-check whether the table is + # actually gone before claiming success — a swallowed lock error + # must NOT report a drop that didn't happen. pass - return existed + if not existed: + return False + try: + still_there = cursor.execute( + "SELECT 1 FROM sqlite_master " + "WHERE type = 'table' AND name = 'messages_fts_trigram'" + ).fetchone() + except sqlite3.OperationalError: + # Can't even confirm state (locked) — assume not dropped. + return False + return still_there is None @staticmethod def _fts_trigger_count(cursor: sqlite3.Cursor) -> int: diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 730a221429004..dc03dd5a9623b 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -600,6 +600,39 @@ def test_trigram_fts_config_flag_false_drops_index(self, tmp_path, monkeypatch): finally: restored.close() + def test_drop_trigram_fts_returns_false_when_drop_thwarted( + self, tmp_path, monkeypatch + ): + """Regression (Codex P2): if DROP TABLE fails (e.g. the DB is locked by + another process), _drop_trigram_fts must return False so the caller does + NOT log a misleading 'dropped, run VACUUM' hint while the table survives.""" + db_path = tmp_path / "state.db" + monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: True)) + db = SessionDB(db_path=db_path) + try: + db.create_session(session_id="s1", source="cli") + db.append_message("s1", role="user", content="the quick brown fox") + assert db._fts_table_exists("messages_fts_trigram") is True + + real_conn = db._conn + + class _BlockDropCursor: + """Delegates to the real connection but raises on the trigram + DROP TABLE, simulating a lock held by another writer.""" + + def execute(self, sql, *args, **kwargs): + if "DROP TABLE" in sql and "messages_fts_trigram" in sql: + raise sqlite3.OperationalError("database is locked") + return real_conn.execute(sql, *args, **kwargs) + + dropped = db._drop_trigram_fts(_BlockDropCursor()) + # The DROP was thwarted → not a real drop. + assert dropped is False + # And the table really is still present. + assert db._fts_table_exists("messages_fts_trigram") is True + finally: + db.close() + def test_trigram_disabled_still_repairs_missing_base_triggers( self, tmp_path, monkeypatch ): From 9401caf9f3d3caec8c512778392f007ca667a4d9 Mon Sep 17 00:00:00 2001 From: exiao Date: Tue, 14 Jul 2026 22:28:33 -0400 Subject: [PATCH 6/7] fix: harden disabled trigram search and drops --- hermes_state.py | 51 +++++++++++++--------------- tests/test_hermes_state.py | 68 +++++++++++++++++++++++++++++++------- 2 files changed, 79 insertions(+), 40 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index c36d56794e0e5..fa867047a0f83 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1105,42 +1105,37 @@ def _drop_trigram_fts(cursor: sqlite3.Cursor) -> bool: and speed up writes. Substring/CJK search falls back to a LIKE scan; the base ``messages_fts`` word index is untouched. - A concurrent writer can hold the DB lock during startup (gateway + cron), - so ``DROP`` may raise ``database is locked``. We do NOT treat that as a - successful drop: the table/triggers stay in place and writes keep paying - the trigram cost until the next uncontended startup, so returning False - keeps the caller from logging a misleading "dropped, run VACUUM" hint. + The trigger + table drops must fail closed. A concurrent writer can hold + the DB lock during startup (gateway + cron), and swallowing that lock on + a trigger or table DROP can leave a stale trigram trigger pointing at a + missing table (or log that the table was dropped when it was not). Acquire + a write transaction up front so the sequence commits as a unit; suppress + only known FTS-unavailable errors and propagate lock/unrelated failures. """ - for trigger in _TRIGRAM_FTS_TRIGGERS: - try: - cursor.execute(f"DROP TRIGGER IF EXISTS {trigger}") - except sqlite3.OperationalError: - pass - existed = False + started_transaction = False try: + cursor.execute("BEGIN IMMEDIATE") + started_transaction = True + for trigger in _TRIGRAM_FTS_TRIGGERS: + cursor.execute(f"DROP TRIGGER IF EXISTS {trigger}") row = cursor.execute( "SELECT 1 FROM sqlite_master " "WHERE type = 'table' AND name = 'messages_fts_trigram'" ).fetchone() existed = row is not None cursor.execute("DROP TABLE IF EXISTS messages_fts_trigram") - except sqlite3.OperationalError: - # FTS5 module missing, shadow tables in an undroppable state, or the - # DB is locked by another process. Re-check whether the table is - # actually gone before claiming success — a swallowed lock error - # must NOT report a drop that didn't happen. - pass - if not existed: - return False - try: - still_there = cursor.execute( - "SELECT 1 FROM sqlite_master " - "WHERE type = 'table' AND name = 'messages_fts_trigram'" - ).fetchone() - except sqlite3.OperationalError: - # Can't even confirm state (locked) — assume not dropped. - return False - return still_there is None + cursor.execute("COMMIT") + started_transaction = False + return existed + except sqlite3.OperationalError as exc: + if started_transaction: + try: + cursor.execute("ROLLBACK") + except sqlite3.OperationalError: + pass + if SessionDB._is_fts5_unavailable_error(exc): + return False + raise @staticmethod def _fts_trigger_count(cursor: sqlite3.Cursor) -> int: diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index dc03dd5a9623b..0aca9ee68c9bf 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -600,12 +600,48 @@ def test_trigram_fts_config_flag_false_drops_index(self, tmp_path, monkeypatch): finally: restored.close() - def test_drop_trigram_fts_returns_false_when_drop_thwarted( + def test_trigram_disabled_cjk_like_fallback_preserves_boolean_ops( self, tmp_path, monkeypatch ): - """Regression (Codex P2): if DROP TABLE fails (e.g. the DB is locked by - another process), _drop_trigram_fts must return False so the caller does - NOT log a misleading 'dropped, run VACUUM' hint while the table survives.""" + """CJK LIKE fallback must keep FTS boolean semantics when trigram is off.""" + monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: False)) + db = SessionDB(db_path=tmp_path / "state.db") + try: + rows = { + "a": "大别山项目 预算复盘", + "ab": "大别山项目 桂林项目 联合简报", + "b": "桂林项目 单独计划", + "ac": "大别山项目 长江大桥 施工方案", + } + for sid, content in rows.items(): + db.create_session(session_id=sid, source="cli") + db.append_message(sid, role="user", content=content) + + assert db._trigram_available is False + assert db._fts_table_exists("messages_fts_trigram") is False + + not_results = db.search_messages( + "大别山项目 NOT 桂林项目", limit=10 + ) + assert {row["session_id"] for row in not_results} == {"a", "ac"} + + and_results = db.search_messages( + "大别山项目 AND 桂林项目", limit=10 + ) + assert {row["session_id"] for row in and_results} == {"ab"} + finally: + db.close() + + @pytest.mark.parametrize("blocked_sql", ["DROP TRIGGER", "DROP TABLE"]) + def test_drop_trigram_fts_propagates_locked_drop_failures( + self, tmp_path, monkeypatch, blocked_sql + ): + """A locked trigram trigger/table DROP must not be swallowed. + + The drop sequence is transactional, so a table DROP lock after trigger + drops rolls back the trigger drops instead of leaving stale/partial FTS + state behind. + """ db_path = tmp_path / "state.db" monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: True)) db = SessionDB(db_path=db_path) @@ -617,19 +653,27 @@ def test_drop_trigram_fts_returns_false_when_drop_thwarted( real_conn = db._conn class _BlockDropCursor: - """Delegates to the real connection but raises on the trigram - DROP TABLE, simulating a lock held by another writer.""" - def execute(self, sql, *args, **kwargs): - if "DROP TABLE" in sql and "messages_fts_trigram" in sql: + normalized = sql.upper() + if blocked_sql in normalized and "MESSAGES_FTS_TRIGRAM" in normalized: raise sqlite3.OperationalError("database is locked") return real_conn.execute(sql, *args, **kwargs) - dropped = db._drop_trigram_fts(_BlockDropCursor()) - # The DROP was thwarted → not a real drop. - assert dropped is False - # And the table really is still present. + with pytest.raises(sqlite3.OperationalError, match="database is locked"): + db._drop_trigram_fts(_BlockDropCursor()) + + # The whole sequence failed closed: table and trigram triggers stay + # together, so later writes neither lie about a drop nor hit a stale + # trigger pointing at a missing table. assert db._fts_table_exists("messages_fts_trigram") is True + triggers = { + row[0] + for row in db._conn.execute( + "SELECT name FROM sqlite_master WHERE type='trigger' " + "AND name LIKE 'messages_fts_trigram_%'" + ).fetchall() + } + assert triggers == set(hermes_state._TRIGRAM_FTS_TRIGGERS) finally: db.close() From c7576ed7c63bde40bb567220c707cedcbbbb856d Mon Sep 17 00:00:00 2001 From: exiao Date: Tue, 14 Jul 2026 22:49:50 -0400 Subject: [PATCH 7/7] fix(state): drop trigram triggers when tokenizer is missing When sessions.trigram_fts is false on a DB that has messages_fts_trigram but the SQLite build lacks the trigram tokenizer, DROP TABLE raises "no such tokenizer: trigram" and the rollback undoes the tokenizer-free trigger drops. The stale triggers then fire against the unusable vtable on every message INSERT and crash. Drop the triggers in a standalone committed step on the FTS-unavailable path so writes degrade to the base FTS/LIKE path. Addresses reviewer P2. --- hermes_state.py | 21 +++++++++++++++++ tests/test_hermes_state.py | 47 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/hermes_state.py b/hermes_state.py index fa867047a0f83..36bdf752466b8 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1134,6 +1134,27 @@ def _drop_trigram_fts(cursor: sqlite3.Cursor) -> bool: except sqlite3.OperationalError: pass if SessionDB._is_fts5_unavailable_error(exc): + # The DROP TABLE needs to load the vtable module/tokenizer, so a + # build missing the trigram tokenizer raises here and the rollback + # above also undid the (tokenizer-free) trigger drops. Leaving the + # triggers in place means every message INSERT fires + # messages_fts_trigram_insert against the unusable vtable and + # crashes. DROP TRIGGER does not need the tokenizer, so drop them + # in a standalone committed step and degrade to the base FTS/LIKE + # path. The orphaned trigram table is harmless without triggers. + try: + cursor.execute("BEGIN IMMEDIATE") + for trigger in _TRIGRAM_FTS_TRIGGERS: + cursor.execute(f"DROP TRIGGER IF EXISTS {trigger}") + cursor.execute("COMMIT") + except sqlite3.OperationalError: + # Locked/unavailable while dropping the triggers alone; leave + # the DB untouched (roll back the partial trigger drop) and + # report no drop so a later uncontended startup retries. + try: + cursor.execute("ROLLBACK") + except sqlite3.OperationalError: + pass return False raise diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 0aca9ee68c9bf..63817c0bbe1cc 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -677,6 +677,53 @@ def execute(self, sql, *args, **kwargs): finally: db.close() + def test_drop_trigram_fts_drops_triggers_when_tokenizer_missing( + self, tmp_path, monkeypatch + ): + """Regression (reviewer P2): when the SQLite build lacks the trigram + tokenizer, the DROP TABLE raises 'no such tokenizer: trigram' and the + rollback undoes the (tokenizer-free) trigger drops. Leaving the triggers + behind makes every later message INSERT fire messages_fts_trigram_insert + against the unusable vtable and crash. The drop must still remove the + triggers so writes degrade to the base FTS/LIKE path. + """ + db_path = tmp_path / "state.db" + monkeypatch.setattr(SessionDB, "_trigram_fts_enabled", staticmethod(lambda: True)) + db = SessionDB(db_path=db_path) + try: + db.create_session(session_id="s1", source="cli") + db.append_message("s1", role="user", content="the quick brown fox") + assert db._fts_table_exists("messages_fts_trigram") is True + + real_conn = db._conn + + class _NoTokenizerCursor: + def execute(self, sql, *args, **kwargs): + normalized = sql.upper() + if "DROP TABLE" in normalized and "MESSAGES_FTS_TRIGRAM" in normalized: + raise sqlite3.OperationalError("no such tokenizer: trigram") + return real_conn.execute(sql, *args, **kwargs) + + # Tokenizer-missing is a known FTS-unavailable error, so the drop + # reports False (nothing reclaimable) but must NOT leave stale + # triggers pointing at the unusable vtable. + assert db._drop_trigram_fts(_NoTokenizerCursor()) is False + + triggers = { + row[0] + for row in db._conn.execute( + "SELECT name FROM sqlite_master WHERE type='trigger' " + "AND name LIKE 'messages_fts_trigram_%'" + ).fetchall() + } + assert triggers == set() + + # With the triggers gone, a new message INSERT no longer crashes on + # the unusable trigram vtable. + db.append_message("s1", role="user", content="second message after drop") + finally: + db.close() + def test_trigram_disabled_still_repairs_missing_base_triggers( self, tmp_path, monkeypatch ):