RomanNum3ral
/
hermes-agent
mirror of https://github.com/NousResearch/hermes-agent.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

`hermes sessions optimize-storage` aborts with 

```
Error: optimization failed: no such table: messages_fts_trigram
No data was lost. Re-run to resume.
```

on any install where the trigram FTS index is legitimately absent. The failure is
deterministic — re-running can never make progress, because the crash happens at the same
point every time — so the database is permanently stuck on the legacy high-footprint FTS
layout with no supported way forward.

Observed on a 5.4 GB production `state.db`. After the fix the same database optimized
successfully and shrank to 3.3 GB.

The trigram index is absent whenever the runtime cannot maintain it. On a SQLite build
without the `trigram` tokenizer, `_ensure_fts_schema()` returns `False`, so `__init__`
leaves `self._trigram_available = False` and no `messages_fts_trigram` table on disk. This
is a **supported degraded runtime**, not damage — CJK/substring search falls back to
`LIKE` and everything else works normally. `_is_fts5_unavailable_error()` and
`_warn_trigram_unavailable()` exist specifically to make this path graceful.

Two code paths write the boundary sweep for the deferred FTS rebuild, and only one of them
respects that flag:

| Function | Trigram `INSERT` guarded? |
|---|---|
| `fts_rebuild_step()` | ✅ `if include_trigram:` where `include_trigram = self._trigram_available` |
| `_fts_rebuild_finish()` | ❌ unconditional |

`_fts_rebuild_finish()` runs the boundary sweep at the *end* of the backfill. Its
unguarded `INSERT INTO messages_fts_trigram …` raises `OperationalError`, which propagates
out of `optimize_fts_storage()` and aborts the entire optimization — *after* the backfill
has already completed. Hence the characteristic output showing 100% progress immediately
before the error:

```
Rebuilding index: 100% (909,671/909,671)
Error: optimization failed: no such table: messages_fts_trigram
```

There is a second, quieter consequence. The teardown phase that reclaims the demoted
`fts_v22_trash_*` shadow tables runs *after* the backfill phase in
`optimize_fts_storage()`. Because the crash happens before teardown is ever reached, those
tables are never emptied or dropped — so the space the migration was supposed to reclaim
stays allocated indefinitely, and the leftover trash tables look (misleadingly) like
evidence of a half-finished migration.

Build a populated v23 database, set the deferred-rebuild markers, then reopen it on a
runtime where `_ensure_fts_schema('messages_fts_trigram', …)` returns `False` (exactly
what a SQLite build without the trigram tokenizer produces) and call
`optimize_fts_storage()`:

```
[precondition] trigram absent, _trigram_available=False, rebuild pending  ✓

RED  ✗ optimize_fts_storage raised OperationalError: no such table: messages_fts_trigram
```

With this patch applied, unchanged harness:

```
     optimize_fts_storage returned {'ok': True, 'vacuumed': None}
GREEN ✓ optimize ok; markers cleared; base FTS 'zebra' -> 200 hits
```

Full harness and transcripts in `TEST-EVIDENCE.md`.

Gate the sweep on `self._trigram_available`, exactly as `fts_rebuild_step()` already does:

```python
include_trigram = self._trigram_available

def _do(conn):
    ...
    if include_trigram:
        conn.execute("INSERT INTO messages_fts_trigram(...) ...")
```

The base `messages_fts` sweep and the marker cleanup are untouched, so the rebuild still
finalizes correctly and the index remains complete for every row it is responsible for.
The fix does not disable or weaken search to dodge the error — the regression tests assert
that base FTS still returns results afterwards.

`TestFtsRebuildFinishWithoutTrigram` in `tests/test_hermes_state.py`:

- `test_rebuild_finish_skips_trigram_when_unavailable` — drives `_fts_rebuild_finish()`
  directly on a trigram-less runtime; asserts it completes, clears both rebuild markers,
  and leaves base FTS searchable.
- `test_optimize_fts_storage_succeeds_without_trigram` — end-to-end through the public
  `optimize_fts_storage()` entry point; asserts `ok=True`, markers cleared, search intact.

Both use the existing `_NoTrigramConnection` helper already in the file. Both fail on
`main` with `no such table: messages_fts_trigram` and pass with this patch.

`tests/test_hermes_state.py` passes in full (463 tests → 465 with these two). `ruff` clean.

This PR is the crash only.

A companion PR narrows `_db_opens_cleanly()` so that
`hermes sessions repair --check-only` stops reporting a write-broken FTS schema as
healthy — the gap that makes this class of problem hard to diagnose in the first place.
The two are independent and can land in either order.
This commit is contained in:
Kyzcreig 2026-07-26 04:34:58 -07:00 committed by kshitij
parent 687dd632a1
commit 2f32092b38
2 changed files with 141 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
hermes_state_search.py
View File

@ -115,7 +115,16 @@ class SessionSearchMixin:
trigger activation): re-index any row near the boundary that the
index is missing. docsize has one row per indexed doc, so the
anti-join is exact and runs on a narrow id range.
The trigram half of the sweep is gated on ``self._trigram_available``
for the same reason ``fts_rebuild_step()`` gates its backfill INSERT:
when the SQLite build has no trigram tokenizer (or the table was
never created), an unconditional INSERT raises ``no such table``
and aborts the whole rebuild — taking ``optimize_fts_storage()``
down with it.
"""
include_trigram = self._trigram_available
def _do(conn):
hw_row = conn.execute(
"SELECT value FROM state_meta WHERE key = 'fts_rebuild_high_water'"
@ -132,14 +141,15 @@ class SessionSearchMixin:
"AND NOT EXISTS (SELECT 1 FROM messages_fts_docsize d WHERE d.id = m.id)",
(lo, hi),
)
conn.execute(
"INSERT INTO messages_fts_trigram(rowid, content, tool_name, tool_calls) "
"SELECT m.id, m.content, m.tool_name, m.tool_calls "
"FROM messages m "
"WHERE m.id > ? AND m.id <= ? AND m.role <> 'tool' "
"AND NOT EXISTS (SELECT 1 FROM messages_fts_trigram_docsize d WHERE d.id = m.id)",
(lo, hi),
)
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 "
"FROM messages m "
"WHERE m.id > ? AND m.id <= ? AND m.role <> 'tool' "
"AND NOT EXISTS (SELECT 1 FROM messages_fts_trigram_docsize d WHERE d.id = m.id)",
(lo, hi),
)
conn.execute(
"DELETE FROM state_meta WHERE key IN "
"('fts_rebuild_high_water', 'fts_rebuild_progress')"

123
tests/test_hermes_state.py
View File

@ -3894,3 +3894,126 @@ class TestInsightsToolCallIndex:
assert "WHERE" in sql
assert "role = 'assistant'" in sql
assert "tool_calls IS NOT NULL" in sql
class TestFtsRebuildFinishWithoutTrigram:
"""An FTS index that the runtime cannot maintain must not wedge the store.
Two independent failure sites shared one root shape: code that writes to
``messages_fts_trigram`` without first checking the table is actually
present. It is legitimately absent whenever the trigram index is
unavailable (SQLite build without the tokenizer), and it can also be left
absent by an interrupted migration or a partially-applied schema change.
"""
@staticmethod
def _seed(db_path, n=60):
seeded = SessionDB(db_path=db_path)
try:
seeded.create_session(session_id="s1", source="cli")
for i in range(n):
seeded.append_message(
"s1",
role=("user" if i % 3 == 0
else "assistant" if i % 3 == 1 else "tool"),
content=f"sentinel payload {i} zebra",
)
high_water = seeded._conn.execute(
"SELECT COALESCE(MAX(id), 0) FROM messages"
).fetchone()[0]
finally:
seeded.close()
return high_water
def test_rebuild_finish_skips_trigram_when_unavailable(
self, tmp_path, monkeypatch
):
"""optimize_fts_storage() completes when the trigram index is absent.
``fts_rebuild_step()`` already guards its backfill INSERT on
``_trigram_available``; ``_fts_rebuild_finish()``'s boundary sweep did
not, so finishing a deferred rebuild on a trigram-less runtime raised
``no such table: messages_fts_trigram`` and aborted the whole
optimization. The base index must still be swept and the markers
cleared.
"""
db_path = tmp_path / "state.db"
high_water = self._seed(db_path)
real_connect = sqlite3.connect
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:
assert db._trigram_available is False
# A trigram-less runtime leaves no trigram index on disk.
db._conn.execute("DROP TABLE IF EXISTS messages_fts_trigram")
db._conn.commit()
assert db._fts_table_exists("messages_fts_trigram") is False
# Put the DB in the pending-deferred-rebuild state.
for key, value in (
("fts_rebuild_high_water", str(high_water)),
("fts_rebuild_progress", str(high_water)),
):
db._conn.execute(
"INSERT INTO state_meta (key, value) VALUES (?, ?) "
"ON CONFLICT(key) DO UPDATE SET value = excluded.value",
(key, value),
)
db._conn.commit()
# Pre-fix this raised OperationalError("no such table: ...").
db._fts_rebuild_finish()
# The sweep ran to completion: markers cleared…
assert db.get_meta("fts_rebuild_high_water") is None
assert db.get_meta("fts_rebuild_progress") is None
# …and the base index is still usable (the fix must not disable
# real search to dodge the error).
assert db.search_messages("zebra")
finally:
db.close()
def test_optimize_fts_storage_succeeds_without_trigram(
self, tmp_path, monkeypatch
):
"""End-to-end: the public optimize entry point returns ok=True."""
db_path = tmp_path / "state.db"
high_water = self._seed(db_path)
real_connect = sqlite3.connect
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._conn.execute("DROP TABLE IF EXISTS messages_fts_trigram")
db._conn.commit()
assert db._trigram_available is False
for key, value in (
("fts_rebuild_high_water", str(high_water)),
("fts_rebuild_progress", "0"),
):
db._conn.execute(
"INSERT INTO state_meta (key, value) VALUES (?, ?) "
"ON CONFLICT(key) DO UPDATE SET value = excluded.value",
(key, value),
)
db._conn.commit()
result = db.optimize_fts_storage(vacuum=False)
assert result["ok"] is True
assert db.get_meta("fts_rebuild_high_water") is None
assert db.search_messages("zebra")
finally:
db.close()