From 5a7bee0fa8970f53d4617b32fa58c7f8c8028bad Mon Sep 17 00:00:00 2001 From: Andrew Wikel Date: Mon, 17 Aug 2026 01:49:25 -0500 Subject: [PATCH] fix(state): exclude tool calls from trigram FTS Keep structured tool_calls searchable through the standard FTS index while removing their repetitive JSON from the trigram projection. Reuse the existing optimize-storage rebuild path for deployed v1 layouts. Co-authored-by: liuhao1024 --- hermes_state.py | 22 ++++++ hermes_state_common.py | 44 +++++------ hermes_state_schema.py | 7 +- hermes_state_search.py | 42 +++++++---- tests/test_hermes_state.py | 147 ++++++++++++++++++++++++++++++++++++- 5 files changed, 223 insertions(+), 39 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index 217e18435a..166c39a519 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -6040,6 +6040,28 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) # means a legacy shape that doesn't index tool metadata → optimize. return "tool_name" not in sql + @staticmethod + def _db_has_trigram_tool_calls_projection(cursor: sqlite3.Cursor) -> bool: + """True when the trigram vtable still includes tool_calls payload.""" + row = cursor.execute( + "SELECT sql FROM sqlite_master " + "WHERE type = 'table' AND name = 'messages_fts_trigram'" + ).fetchone() + if row is None: + return False + sql = (row[0] if not isinstance(row, sqlite3.Row) else row["sql"]) or "" + return "tool_calls" in sql.lower() + + @classmethod + def _db_needs_fts_storage_upgrade( + cls, cursor: sqlite3.Cursor + ) -> bool: + """True when the current FTS storage layout should be treated as stale.""" + return ( + cls._db_has_legacy_inline_fts(cursor) + or cls._db_has_trigram_tool_calls_projection(cursor) + ) + def _warn_trigram_unavailable(self, exc: sqlite3.OperationalError) -> None: """Log once that the trigram tokenizer is missing; base FTS5 stays enabled.""" if getattr(self, "_trigram_unavailable_warned", False): diff --git a/hermes_state_common.py b/hermes_state_common.py index 95914e704a..9b04fa5ba3 100644 --- a/hermes_state_common.py +++ b/hermes_state_common.py @@ -363,9 +363,9 @@ SCHEMA_VERSION = 29 # reaches the current version when a DB is either born fresh or explicitly # optimized via ``hermes sessions optimize-storage``. A legacy DB sits at # layout 0 (marker absent) with a working inline index until the user opts in. -# 1 = v23 external-content layout (content/tool_name/tool_calls, -# tool-row-excluded trigram) -FTS_STORAGE_VERSION = 1 +# 1 = v23 external-content layout with a tool-row-excluded trigram +# 2 = trigram also excludes structured tool_calls JSON +FTS_STORAGE_VERSION = 2 # Tool results are often multi-megabyte machine payloads. Index a useful # prefix for new tool rows instead of tokenizing the entire body while the @@ -772,15 +772,19 @@ END; # matching. The trigram tokenizer creates overlapping 3-byte sequences so # substring queries work natively for any script (CJK, Thai, etc.). # -# The trigram index is the most expensive index in state.db, and tool output -# plus cron transcripts are overwhelmingly machine-generated text. The index -# therefore reads through ``messages_fts_trigram_src``, a view that excludes -# both classes. They stay fully stored in ``messages`` and searchable via the -# standard ``messages_fts`` index; they just don't get trigram treatment. -# ``search_messages`` routes explicit tool/cron CJK searches to LIKE. +# The trigram index is the most expensive index in state.db (~2.6x the size +# of the text it covers). Tool output (~90% of message bytes, machine noise) +# and cron transcripts are excluded: the index reads through +# ``messages_fts_trigram_src``, a view that skips both classes. They stay +# fully stored in ``messages`` and searchable via the standard +# ``messages_fts`` index; they just don't get trigram (CJK substring) +# treatment. ``search_messages`` routes explicit tool/cron CJK searches to +# LIKE for the same reason. Structured ``tool_calls`` JSON likewise stays +# searchable through ``messages_fts``; excluding it here avoids indexing +# repetitive JSON syntax as trigrams (FTS_STORAGE_VERSION 2). FTS_TRIGRAM_SQL = """ CREATE VIEW IF NOT EXISTS messages_fts_trigram_src AS - SELECT m.id, m.role, m.content, m.tool_name, m.tool_calls + SELECT m.id, m.role, m.content, m.tool_name FROM messages AS m JOIN sessions AS s ON s.id = m.session_id WHERE m.role <> 'tool' AND s.source <> 'cron'; @@ -788,7 +792,6 @@ CREATE VIEW IF NOT EXISTS messages_fts_trigram_src AS CREATE VIRTUAL TABLE IF NOT EXISTS messages_fts_trigram USING fts5( content, tool_name, - tool_calls, content='messages_fts_trigram_src', content_rowid='id', tokenize='trigram' @@ -803,8 +806,8 @@ WHEN new.role <> 'tool' OR new.id <= COALESCE((SELECT CAST(value AS INTEGER) FROM state_meta WHERE key = 'fts_rebuild_progress'), -1)) BEGIN - INSERT INTO messages_fts_trigram(rowid, content, tool_name, tool_calls) - VALUES (new.id, new.content, new.tool_name, new.tool_calls); + INSERT INTO messages_fts_trigram(rowid, content, tool_name) + VALUES (new.id, new.content, new.tool_name); END; CREATE TRIGGER IF NOT EXISTS messages_fts_trigram_delete AFTER DELETE ON messages @@ -816,28 +819,27 @@ WHEN old.role <> 'tool' OR old.id <= COALESCE((SELECT CAST(value AS INTEGER) FROM state_meta WHERE key = 'fts_rebuild_progress'), -1)) BEGIN - INSERT INTO messages_fts_trigram(messages_fts_trigram, rowid, content, tool_name, tool_calls) - VALUES ('delete', old.id, old.content, old.tool_name, old.tool_calls); + INSERT INTO messages_fts_trigram(messages_fts_trigram, rowid, content, tool_name) + VALUES ('delete', old.id, old.content, old.tool_name); END; CREATE TRIGGER IF NOT EXISTS messages_fts_trigram_update -AFTER UPDATE OF content, tool_name, tool_calls, role ON messages +AFTER UPDATE OF content, tool_name, role ON messages WHEN (old.content IS NOT new.content OR old.tool_name IS NOT new.tool_name - OR old.tool_calls IS NOT new.tool_calls OR old.role IS NOT new.role) AND (old.id > COALESCE((SELECT CAST(value AS INTEGER) FROM state_meta WHERE key = 'fts_rebuild_high_water'), -1) OR old.id <= COALESCE((SELECT CAST(value AS INTEGER) FROM state_meta WHERE key = 'fts_rebuild_progress'), -1)) BEGIN - INSERT INTO messages_fts_trigram(messages_fts_trigram, rowid, content, tool_name, tool_calls) - SELECT 'delete', old.id, old.content, old.tool_name, old.tool_calls + INSERT INTO messages_fts_trigram(messages_fts_trigram, rowid, content, tool_name) + SELECT 'delete', old.id, old.content, old.tool_name WHERE old.role <> 'tool' AND EXISTS (SELECT 1 FROM sessions WHERE id = old.session_id AND source <> 'cron'); - INSERT INTO messages_fts_trigram(rowid, content, tool_name, tool_calls) - SELECT new.id, new.content, new.tool_name, new.tool_calls + INSERT INTO messages_fts_trigram(rowid, content, tool_name) + SELECT new.id, new.content, new.tool_name WHERE new.role <> 'tool' AND EXISTS (SELECT 1 FROM sessions WHERE id = new.session_id AND source <> 'cron'); diff --git a/hermes_state_schema.py b/hermes_state_schema.py index f25e04afd8..d1152a366b 100644 --- a/hermes_state_schema.py +++ b/hermes_state_schema.py @@ -1528,7 +1528,10 @@ class SessionSchemaMixin: # advances to SCHEMA_VERSION here like every other migration — # future v24+ migrations land automatically for legacy-FTS # users too. Only the FTS *layout* waits for opt-in. - if fts5_available and self._db_has_legacy_inline_fts(cursor): + if ( + fts5_available + and self._db_needs_fts_storage_upgrade(cursor) + ): self.set_meta("fts_optimize_available", "1", cursor=cursor) if current_version < 25: @@ -1561,7 +1564,7 @@ class SessionSchemaMixin: # transition actually completes. if ( fts5_available - and not self._db_has_legacy_inline_fts(cursor) + and not self._db_needs_fts_storage_upgrade(cursor) and cursor.execute( "SELECT 1 FROM state_meta " "WHERE key = 'fts_rebuild_high_water' LIMIT 1" diff --git a/hermes_state_search.py b/hermes_state_search.py index c487eee09d..68af1e1164 100644 --- a/hermes_state_search.py +++ b/hermes_state_search.py @@ -165,8 +165,8 @@ class SessionSearchMixin: ) if include_trigram: conn.execute( - "INSERT INTO messages_fts_trigram(rowid, content, tool_name, tool_calls) " - "SELECT m.id, m.content, m.tool_name, m.tool_calls " + "INSERT INTO messages_fts_trigram(rowid, content, tool_name) " + "SELECT m.id, m.content, m.tool_name " "FROM messages m JOIN sessions s ON s.id = m.session_id " "WHERE m.id > ? AND m.id <= ? AND m.role <> 'tool' " "AND s.source <> 'cron' " @@ -325,8 +325,8 @@ class SessionSearchMixin: if include_trigram: conn.execute( "INSERT INTO messages_fts_trigram" - "(rowid, content, tool_name, tool_calls) " - "SELECT m.id, m.content, m.tool_name, m.tool_calls " + "(rowid, content, tool_name) " + "SELECT m.id, m.content, m.tool_name " "FROM messages m JOIN sessions s ON s.id = m.session_id " "WHERE m.id > ? AND m.id <= ? AND m.role <> 'tool' " "AND s.source <> 'cron'", @@ -657,10 +657,11 @@ class SessionSearchMixin: is a legacy inline-FTS install that can be optimized to the v23 external-content schema, or a previous optimize run was interrupted (legacy vtables already demoted, but backfill markers and/or trash - tables remain) and re-running would resume it, or the CJK-bigram - index needs a backfill/rebuild on this tokenizer-capable host, or - a prior demote left an empty external-content index without markers - (healable on re-run). + tables remain) and re-running would resume it, or this DB is v23 with the + old tool-calls-inclusive trigram projection (repairable via this same + migration flow), or the CJK-bigram index needs a backfill/rebuild on this + tokenizer-capable host, or a prior demote left an empty external-content + index without markers (healable on re-run). False for fresh and fully-optimized installs (and when FTS5 is unavailable).""" if not self._fts_enabled or self.read_only: @@ -668,6 +669,8 @@ class SessionSearchMixin: with self._read_ctx() as conn: if self._db_has_legacy_inline_fts(conn): return True + if self._db_has_trigram_tool_calls_projection(self._conn): + return True # Interrupted optimize: demotion already removed the legacy # vtables (so the check above is False), but the transition is # unfinished until the backfill markers are cleared and the @@ -694,7 +697,7 @@ class SessionSearchMixin: return self._fts_external_index_empty_with_messages(conn) def _demote_legacy_fts_to_trash(self) -> int: - """Demote the legacy inline FTS vtables and stage their shadow tables + """Demote upgrade-eligible FTS vtables and stage their shadow tables for chunked teardown. Returns MAX(messages.id) as the rebuild high water. O(1) schema surgery — the heavy delete is deferred to the chunked teardown, exactly as the validated auto path did. @@ -767,9 +770,16 @@ class SessionSearchMixin: progress_cb: Optional[Callable[[Dict[str, Any]], None]] = None, vacuum: bool = True, ) -> Dict[str, Any]: - """Migrate a legacy v22 inline-FTS DB to the v23 external-content - schema, foreground and to completion. Safe to re-run: if a previous - attempt was interrupted it resumes from the progress marker. + """Repair an older FTS layout into the current v23-compatible shape, + foreground and to completion. + + Supports two paths: + - legacy-v22 inline -> demote to v23 external-content + - v23 installs where ``messages_fts_trigram`` still stores + ``tool_calls`` payloads + + Safe to re-run: if a previous attempt was interrupted it resumes from + the progress marker. ``progress_cb`` receives {"phase", "percent", "indexed", "total"} dicts for a CLI progress bar. Returns a summary dict. @@ -793,11 +803,13 @@ class SessionSearchMixin: # finishing the backfill + teardown — this is what makes re-running # after an interruption safe. with self._lock: - legacy = self._db_has_legacy_inline_fts(self._conn) + needs_storage_upgrade = self._db_needs_fts_storage_upgrade( + self._conn + ) pending = self.get_meta("fts_rebuild_high_water") is not None - if legacy and not pending: + if needs_storage_upgrade and not pending: self._demote_legacy_fts_to_trash() - elif pending and not legacy: + elif pending and not needs_storage_upgrade: # Resume mid-demote: markers exist, empty v23 tables may still be # missing if the process died between the staged demote commit and # schema ensure. Re-ensure is IF NOT EXISTS and cheap. diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index a52a90aa65..4ee1503f1a 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -11,7 +11,13 @@ import pytest import hermes_state from agent.session_activity import ActivityProvenance -from hermes_state import SCHEMA_SQL, SCHEMA_VERSION, SessionDB +from hermes_state import ( + FTS_SQL, + FTS_STORAGE_VERSION, + SCHEMA_SQL, + SCHEMA_VERSION, + SessionDB, +) class _NoFtsCursor(sqlite3.Cursor): @@ -3634,6 +3640,145 @@ class TestFTSExternalContentMigration: finally: db.close() + def test_v23_rebuild_from_trigram_tool_calls_projection(self, tmp_path): + """v23 installs built with historical trigram projection should be + repaired via optimize-storage: trigram must drop tool_calls while + standard messages_fts keeps indexing them.""" + db_path = tmp_path / "v23-toolcalls.db" + + # Build an external-content DB that is already at schema version 23, + # but with the old tool_calls-inclusive trigram projection. + conn = sqlite3.connect(str(db_path)) + conn.executescript(SCHEMA_SQL) + conn.executescript(FTS_SQL) + conn.executescript( + """ + DROP TRIGGER IF EXISTS messages_fts_trigram_insert; + DROP TRIGGER IF EXISTS messages_fts_trigram_delete; + DROP TRIGGER IF EXISTS messages_fts_trigram_update; + DROP TABLE IF EXISTS messages_fts_trigram; + DROP VIEW IF EXISTS messages_fts_trigram_src; + + CREATE VIEW IF NOT EXISTS messages_fts_trigram_src AS + SELECT id, role, content, tool_name, tool_calls + FROM messages + WHERE role <> 'tool'; + + CREATE VIRTUAL TABLE messages_fts_trigram USING fts5( + content, + tool_name, + tool_calls, + content='messages_fts_trigram_src', + content_rowid='id', + tokenize='trigram' + ); + + CREATE TRIGGER messages_fts_trigram_insert AFTER INSERT ON messages + WHEN new.role <> 'tool' + BEGIN + INSERT INTO messages_fts_trigram(rowid, content, tool_name, tool_calls) + VALUES (new.id, new.content, new.tool_name, new.tool_calls); + END; + + CREATE TRIGGER messages_fts_trigram_delete AFTER DELETE ON messages + WHEN old.role <> 'tool' + BEGIN + INSERT INTO messages_fts_trigram(messages_fts_trigram, rowid, content, tool_name, tool_calls) + VALUES ('delete', old.id, old.content, old.tool_name, old.tool_calls); + END; + + CREATE TRIGGER messages_fts_trigram_update + AFTER UPDATE OF content, tool_name, tool_calls, role ON messages + WHEN (old.content IS NOT new.content + OR old.tool_name IS NOT new.tool_name + OR old.tool_calls IS NOT new.tool_calls + OR old.role IS NOT new.role) + BEGIN + INSERT INTO messages_fts_trigram(messages_fts_trigram, rowid, content, tool_name, tool_calls) + SELECT 'delete', old.id, old.content, old.tool_name, old.tool_calls + WHERE old.role <> 'tool'; + INSERT INTO messages_fts_trigram(rowid, content, tool_name, tool_calls) + SELECT new.id, new.content, new.tool_name, new.tool_calls + WHERE new.role <> 'tool'; + END; + """ + ) + # Simulate the historical v23 projection shipped before this fix. + conn.execute( + "INSERT OR REPLACE INTO state_meta (key, value) VALUES ('fts_storage_version', '1')" + ) + conn.execute( + "INSERT OR REPLACE INTO state_meta (key, value) VALUES ('fts_optimize_available', '1')" + ) + conn.commit() + conn.close() + + conn = sqlite3.connect(str(db_path)) + conn.execute( + "INSERT INTO sessions (id, source, started_at) VALUES (?, ?, ?)", + ("s1", "cli", time.time()), + ) + conn.execute( + "INSERT INTO messages (session_id, timestamp, role, content, tool_name, tool_calls) " + "VALUES (?, ?, ?, ?, ?, ?)", + ( + "s1", + time.time(), + "assistant", + "部署完成 assistant content", + "legacyTool", + '{"name": "legacy", "arguments": "UNIQUE_TOOLCALL_TOKEN_43701"}', + ), + ) + conn.commit() + assert conn.execute( + "SELECT rowid FROM messages_fts_trigram WHERE messages_fts_trigram MATCH 'UNIQUE_TOOLCALL_TOKEN_43701'" + ).fetchall() + conn.close() + + db = SessionDB(db_path=db_path) + try: + assert db._conn is not None + assert db.fts_optimize_available() is True + assert db.get_meta("fts_storage_version") == "1" + + original_ensure = db._ensure_fts_schema + + def interrupt_after_demote(cursor, table_name, ddl): + if table_name == "messages_fts_trigram": + raise RuntimeError("injected trigram rebuild interruption") + return original_ensure(cursor, table_name, ddl) + + db._ensure_fts_schema = interrupt_after_demote + with pytest.raises(RuntimeError, match="injected trigram"): + db.optimize_fts_storage(vacuum=False) + assert db.get_meta("fts_rebuild_high_water") is not None + assert db.fts_optimize_available() is True + assert db.get_meta("fts_storage_version") == "1" + + db._ensure_fts_schema = original_ensure + result = db.optimize_fts_storage(vacuum=False) + assert result["ok"] is True + + # messages_fts stays in the tool-calls search path. + assert len(db.search_messages("UNIQUE_TOOLCALL_TOKEN_43701")) == 1 + # New trigram schema excludes tool_calls from trigram projection. + assert not db._conn.execute( + "SELECT 1 FROM messages_fts_trigram WHERE messages_fts_trigram MATCH 'UNIQUE_TOOLCALL_TOKEN_43701' LIMIT 1" + ).fetchone() + assert db._conn.execute( + "SELECT 1 FROM messages_fts_trigram " + "WHERE messages_fts_trigram MATCH '部署完成' LIMIT 1" + ).fetchone() + trigger_sql = db._conn.execute( + "SELECT sql FROM sqlite_master " + "WHERE type = 'trigger' AND name = 'messages_fts_trigram_update'" + ).fetchone()[0] + assert "tool_calls" not in trigger_sql + assert db.get_meta("fts_storage_version") == str(FTS_STORAGE_VERSION) + finally: + db.close() +