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 <sunsky.lau@gmail.com>
This commit is contained in:
@@ -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):
|
||||
|
||||
+23
-21
@@ -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');
|
||||
|
||||
@@ -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"
|
||||
|
||||
+27
-15
@@ -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.
|
||||
|
||||
+146
-1
@@ -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()
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user