refactor(state): drop the retired tool high-water marker on realign; reuse the shared projection SQL
Follow-up to the aligned-projection salvage (#114191): - the realign migration deletes the now-meaningless state_meta key fts_tool_full_content_high_water instead of leaving inert residue, and the empty-store and populated-store branches share one do_align() body; - _fts_tool_prefix_migration_requires_rebuild is dead after the realign (the shape probe it guarded is false once do_align ran, and a deferred align marks the index stale before that branch is reached) -> removed; - the legacy inline triggers use _FTS_NEW_INDEXED_CONTENT_SQL again instead of four inlined copies of the same CASE expression; - tests trimmed to two invariants (strict-probe survives churn; a raw-messages index realigns once on open, retired marker gone).
This commit is contained in:
@@ -926,9 +926,7 @@ CREATE VIRTUAL TABLE IF NOT EXISTS messages_fts USING fts5(
|
||||
CREATE TRIGGER IF NOT EXISTS messages_fts_insert AFTER INSERT ON messages BEGIN
|
||||
INSERT INTO messages_fts(rowid, content) VALUES (
|
||||
new.id,
|
||||
COALESCE(CASE WHEN new.role = 'tool'
|
||||
THEN substr(COALESCE(new.content, ''), 1, {FTS_TOOL_CONTENT_PREFIX_CHARS})
|
||||
ELSE new.content END, '')
|
||||
COALESCE({_FTS_NEW_INDEXED_CONTENT_SQL}, '')
|
||||
|| ' ' || COALESCE(new.tool_name, '') || ' ' || COALESCE(new.tool_calls, '')
|
||||
);
|
||||
END;
|
||||
@@ -942,9 +940,7 @@ AFTER UPDATE OF content, tool_name, tool_calls, role ON messages BEGIN
|
||||
DELETE FROM messages_fts WHERE rowid = old.id;
|
||||
INSERT INTO messages_fts(rowid, content) VALUES (
|
||||
new.id,
|
||||
COALESCE(CASE WHEN new.role = 'tool'
|
||||
THEN substr(COALESCE(new.content, ''), 1, {FTS_TOOL_CONTENT_PREFIX_CHARS})
|
||||
ELSE new.content END, '')
|
||||
COALESCE({_FTS_NEW_INDEXED_CONTENT_SQL}, '')
|
||||
|| ' ' || COALESCE(new.tool_name, '') || ' ' || COALESCE(new.tool_calls, '')
|
||||
);
|
||||
END;
|
||||
@@ -960,9 +956,7 @@ CREATE VIRTUAL TABLE IF NOT EXISTS messages_fts_trigram USING fts5(
|
||||
CREATE TRIGGER IF NOT EXISTS messages_fts_trigram_insert AFTER INSERT ON messages BEGIN
|
||||
INSERT INTO messages_fts_trigram(rowid, content) VALUES (
|
||||
new.id,
|
||||
COALESCE(CASE WHEN new.role = 'tool'
|
||||
THEN substr(COALESCE(new.content, ''), 1, {FTS_TOOL_CONTENT_PREFIX_CHARS})
|
||||
ELSE new.content END, '')
|
||||
COALESCE({_FTS_NEW_INDEXED_CONTENT_SQL}, '')
|
||||
|| ' ' || COALESCE(new.tool_name, '') || ' ' || COALESCE(new.tool_calls, '')
|
||||
);
|
||||
END;
|
||||
@@ -976,9 +970,7 @@ AFTER UPDATE OF content, tool_name, tool_calls, role ON messages BEGIN
|
||||
DELETE FROM messages_fts_trigram WHERE rowid = old.id;
|
||||
INSERT INTO messages_fts_trigram(rowid, content) VALUES (
|
||||
new.id,
|
||||
COALESCE(CASE WHEN new.role = 'tool'
|
||||
THEN substr(COALESCE(new.content, ''), 1, {FTS_TOOL_CONTENT_PREFIX_CHARS})
|
||||
ELSE new.content END, '')
|
||||
COALESCE({_FTS_NEW_INDEXED_CONTENT_SQL}, '')
|
||||
|| ' ' || COALESCE(new.tool_name, '') || ' ' || COALESCE(new.tool_calls, '')
|
||||
);
|
||||
END;
|
||||
|
||||
@@ -133,6 +133,9 @@ _STATE_META_UPSERT_SQL = (
|
||||
"INSERT INTO state_meta (key, value) VALUES (?, ?) ON CONFLICT(key) DO UPDATE SET value = excluded.value"
|
||||
)
|
||||
_CLEAR_REBUILD_MARKERS_SQL = "DELETE FROM state_meta WHERE key IN ('fts_rebuild_high_water', 'fts_rebuild_progress')"
|
||||
# FTS_STORAGE_VERSION < 3 truncated tool rows only above a moving state_meta mark; the aligned
|
||||
# projection truncates by role alone, so the retired marker is dropped with the realign.
|
||||
_DROP_RETIRED_TOOL_HIGH_WATER_SQL = "DELETE FROM state_meta WHERE key = 'fts_tool_full_content_high_water'"
|
||||
|
||||
|
||||
def _legacy_inline_reinsert_sql(table: str, indent: int, *, delete_first: bool = False) -> str:
|
||||
@@ -350,35 +353,29 @@ class SessionSchemaMixin:
|
||||
if not self._fts_index_is_misaligned_source(cursor):
|
||||
return
|
||||
has_messages = cursor.execute("SELECT 1 FROM messages LIMIT 1").fetchone() is not None
|
||||
|
||||
def do_align() -> None:
|
||||
for name in _FTS_BASE_TRIGGERS:
|
||||
cursor.execute(f"DROP TRIGGER IF EXISTS {name}")
|
||||
cursor.execute("DROP TABLE IF EXISTS messages_fts")
|
||||
self._ensure_fts_schema(cursor, "messages_fts", FTS_SQL)
|
||||
if has_messages:
|
||||
cursor.execute("INSERT INTO messages_fts(messages_fts) VALUES('rebuild')")
|
||||
cursor.execute(_CLEAR_REBUILD_MARKERS_SQL)
|
||||
cursor.execute(_DROP_RETIRED_TOOL_HIGH_WATER_SQL)
|
||||
cursor.execute(_STATE_META_UPSERT_SQL, ("fts_storage_version", str(FTS_STORAGE_VERSION)))
|
||||
|
||||
if not has_messages:
|
||||
# Nothing indexed and nothing to index: just swap the shape in place.
|
||||
# Nothing indexed and nothing to index: swap the shape in place, no rebuild authority needed.
|
||||
cursor.execute("SAVEPOINT fts_align_empty")
|
||||
try:
|
||||
for name in _FTS_BASE_TRIGGERS:
|
||||
cursor.execute(f"DROP TRIGGER IF EXISTS {name}")
|
||||
cursor.execute("DROP TABLE IF EXISTS messages_fts")
|
||||
self._execute_ddl_script_transactional(cursor, FTS_SQL)
|
||||
cursor.execute(_STATE_META_UPSERT_SQL, ("fts_storage_version", str(FTS_STORAGE_VERSION)))
|
||||
do_align()
|
||||
cursor.execute("RELEASE SAVEPOINT fts_align_empty")
|
||||
except BaseException:
|
||||
cursor.execute("ROLLBACK TO SAVEPOINT fts_align_empty")
|
||||
cursor.execute("RELEASE SAVEPOINT fts_align_empty")
|
||||
raise
|
||||
return
|
||||
self._fts_tool_prefix_migration_requires_rebuild = True
|
||||
|
||||
def do_align() -> None:
|
||||
self._execute_ddl_script_transactional(cursor, f"""
|
||||
DROP TRIGGER IF EXISTS messages_fts_insert;
|
||||
DROP TRIGGER IF EXISTS messages_fts_delete;
|
||||
DROP TRIGGER IF EXISTS messages_fts_update;
|
||||
""")
|
||||
cursor.execute("DROP TABLE IF EXISTS messages_fts")
|
||||
self._ensure_fts_schema(cursor, "messages_fts", FTS_SQL)
|
||||
cursor.execute("INSERT INTO messages_fts(messages_fts) VALUES('rebuild')")
|
||||
cursor.execute(_CLEAR_REBUILD_MARKERS_SQL)
|
||||
cursor.execute(_STATE_META_UPSERT_SQL, ("fts_storage_version", str(FTS_STORAGE_VERSION)))
|
||||
|
||||
self._run_admitted_startup_rebuild(cursor, do_align)
|
||||
|
||||
@staticmethod
|
||||
@@ -1184,10 +1181,9 @@ DROP TRIGGER IF EXISTS messages_fts_update;
|
||||
base_sql, trigram_sql = _FTS_DDL[legacy_fts]
|
||||
# Measure before any DDL. Publishing missing base triggers before rebuild admission lets
|
||||
# another process write through an index whose bootstrap/repair has no owner (#105790).
|
||||
base_triggers_missing = self._fts_triggers_missing(cursor, _FTS_BASE_TRIGGERS) or (
|
||||
getattr(self, "_fts_tool_prefix_migration_requires_rebuild", False)
|
||||
and self._fts_index_is_misaligned_source(cursor)
|
||||
) or "messages_fts" in orphan_repaired
|
||||
base_triggers_missing = (
|
||||
self._fts_triggers_missing(cursor, _FTS_BASE_TRIGGERS) or "messages_fts" in orphan_repaired
|
||||
)
|
||||
trigram_triggers_missing = (
|
||||
self._fts_triggers_missing(cursor, _FTS_TRIGRAM_TRIGGERS) or "messages_fts_trigram" in orphan_repaired
|
||||
)
|
||||
|
||||
@@ -77,19 +77,6 @@ def test_strict_integrity_probe_survives_tool_row_churn(db):
|
||||
_strict_integrity_probe(db)
|
||||
|
||||
|
||||
def test_tool_rows_keep_full_content_reachable_without_indexing_it(db):
|
||||
"""Bounding the base index must not lose the explicit tool search path."""
|
||||
tool_id = db.append_message(
|
||||
"session", role="tool", content=LONG_TOOL_ROW, tool_name="terminal"
|
||||
)
|
||||
|
||||
assert db.search_messages("tailtoken") == []
|
||||
assert [
|
||||
row["id"] for row in db.search_messages("tailtoken", role_filter=["tool"])
|
||||
] == [tool_id]
|
||||
_strict_integrity_probe(db)
|
||||
|
||||
|
||||
def test_index_reading_raw_messages_realigns_once_on_open(tmp_path):
|
||||
"""A store whose index still reads raw ``messages`` (the shipped shape for
|
||||
versions before the aligned projection) realigns on the next open, records
|
||||
@@ -121,6 +108,10 @@ def test_index_reading_raw_messages_realigns_once_on_open(tmp_path):
|
||||
"INSERT INTO state_meta(key, value) VALUES('fts_storage_version', '2') "
|
||||
"ON CONFLICT(key) DO UPDATE SET value = '2'"
|
||||
)
|
||||
first._conn.execute(
|
||||
"INSERT OR REPLACE INTO state_meta(key, value) VALUES('fts_tool_full_content_high_water', ?)",
|
||||
(str(row_id),),
|
||||
)
|
||||
with pytest.raises(sqlite3.DatabaseError):
|
||||
_strict_integrity_probe(first)
|
||||
first.close()
|
||||
@@ -128,6 +119,7 @@ def test_index_reading_raw_messages_realigns_once_on_open(tmp_path):
|
||||
migrated = SessionDB(db_path=path)
|
||||
try:
|
||||
assert migrated.get_meta("fts_storage_version") == str(FTS_STORAGE_VERSION)
|
||||
assert migrated.get_meta("fts_tool_full_content_high_water") is None
|
||||
_strict_integrity_probe(migrated)
|
||||
index_sql = migrated._conn.execute(
|
||||
"SELECT sql FROM sqlite_master WHERE name = 'messages_fts'"
|
||||
|
||||
Reference in New Issue
Block a user