fix(state): stop rebuilding the whole FTS index on every open when the trigram tokenizer is missing

[briandevans]

What does this PR do?

SessionDB._init_schema decided whether the FTS triggers needed repair with this test, in two identical places:

triggers_need_repair = (
    self._fts_trigger_count(cursor) < len(_FTS_TRIGGERS)
)

_FTS_TRIGGERS is six names, and three of them are the messages_fts_trigram_* triggers. Those three are declared only inside FTS_TRIGRAM_SQL and LEGACY_FTS_TRIGRAM_SQL, whose CREATE VIRTUAL TABLE ... tokenize='trigram' needs a tokenizer SQLite only gained in 3.34. On an older build _ensure_fts_schema soft-fails that DDL through _is_trigram_unavailable_error and returns False — deliberately, so search degrades instead of breaking. The consequence is that those three triggers can never come into existence on such a host.

So the count is pinned at 3, 3 < 6 is permanently True, and the repair path runs on every SessionDB open, forever, holding the SQLite write lock. It never converges. Every hermes command, gateway start, dashboard request and cron tick re-indexes the whole message corpus, and the cost is linear in it.

This is ordinary LTS territory rather than an exotic build — Ubuntu 20.04 ships SQLite 3.31, RHEL/CentOS 8 and Alibaba Cloud Linux ship 3.26, Amazon Linux 2 is older still. Hermes has no minimum-SQLite gate precisely because it is meant to degrade gracefully there, so those hosts run fine and pay the tax silently.

There is a second effect. _rebuild_fts_indexes ends with:

cursor.execute(
    "DELETE FROM state_meta WHERE key IN "
    "('fts_rebuild_high_water', 'fts_rebuild_progress')"
)

which is correct after a genuine full rebuild, but running that rebuild unconditionally means an interrupted hermes sessions optimize-storage silently loses its resume point on the very next open. A chunked, throttled, progress-reported backfill is replaced by an unbounded foreground rebuild inside startup, every time.

The fix

Keep _FTS_TRIGGERS as the single source of truth and derive two subsets from it, then measure each half only against the DDL that can actually create it:

_FTS_TRIGRAM_TRIGGERS = tuple(n for n in _FTS_TRIGGERS if "_trigram_" in n)
_FTS_BASE_TRIGGERS = tuple(n for n in _FTS_TRIGGERS if n not in _FTS_TRIGRAM_TRIGGERS)

_fts_trigger_count gains an optional names sequence defaulting to the full set, so no existing caller changes, and both branches now gate on:

if base_triggers_missing or (trigram_enabled and trigram_triggers_missing):

Ordering is preserved and load-bearing: both counts are still taken before the DDL runs, so they describe the pre-repair state, while trigram_enabled is only known after _ensure_fts_schema. That is why the two halves are combined at the if rather than at the assignment.

Behaviour is unchanged wherever the tokenizer exists — a genuinely missing trigram trigger on a capable host still triggers the rebuild, and a genuinely missing base trigger still triggers it everywhere. Only the permanently unsatisfiable comparison changes.

Sibling sweep

grep -rn "_fts_trigger_count\|triggers_need_repair" over production code returns four sites, and all four are in this PR:

Site	Covered
hermes_state_schema.py legacy inline-FTS gate in _init_schema	fixed
hermes_state_schema.py v23 external-content gate in _init_schema	fixed
_fts_trigger_count body	parameterized
_FTS_TRIGGERS definition in hermes_state_common.py	unchanged — the subsets are derived from it, not a replacement

Deliberately excluded, because they do not share the root cause:

• _drop_fts_triggers iterates _FTS_TRIGGERS emitting DROP TRIGGER IF EXISTS. Dropping a trigger that was never created is a no-op; there is no count-vs-len comparison to get wrong.
• _FTS_CJK_TRIGGERS (hermes_state_search.py, hermes_state.py) has the same shape but a different mechanism: no count-vs-len comparison exists, and _ensure_fts_cjk_schema gates on _fts_cjk_loaded directly, so it cannot get stuck permanently true.

Related Issue

No filed issue — self-found defect, so there is nothing to auto-close here.

The population is independently documented by open #35931, which names the SQLite 3.26 hosts, and the tree has already accepted the general principle twice: merged #48688 ("survive SQLite builds without trigram tokenizer") and merged #77629 ("skip trigram sweep in _fts_rebuild_finish when unavailable"). This is the same idea applied to startup — when trigram is unavailable, stop doing useless work.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• hermes_state_schema.py — derive _FTS_BASE_TRIGGERS / _FTS_TRIGRAM_TRIGGERS from _FTS_TRIGGERS at module scope, with a comment explaining why the halves differ in availability.
• hermes_state_schema.py — _fts_trigger_count(cursor, names=_FTS_TRIGGERS) takes the set to count. The default preserves every existing caller. An empty names short-circuits to 0 rather than emitting name IN (), which is a SQLite syntax error.
• hermes_state_schema.py — both _init_schema FTS branches (legacy inline and v23 external-content) compute base_triggers_missing / trigram_triggers_missing and gate the rebuild on base_triggers_missing or (trigram_enabled and trigram_triggers_missing).
• tests/test_hermes_state.py — new TestFtsRebuildLoopWithoutTrigram, six tests, reusing the file's existing _NoTrigramConnection / _NoTrigramCursor rather than adding a fixture.

How to Test

The tests observe the actual SQL SessionDB issues during an open, via set_trace_callback on the connection, so they cannot pass if the production change is reverted.

1. Check out this branch and run the new class:

   pytest tests/test_hermes_state.py -k TestFtsRebuildLoopWithoutTrigram -v
   

2. Revert only the production hunk (git stash push hermes_state_schema.py) and re-run. Measured on this branch:

   Test	Before	After
   test_missing_trigram_tokenizer_does_not_rebuild_fts_on_every_open	INSERT INTO messages_fts(messages_fts) VALUES('rebuild') on every open	zero after the first
   test_legacy_inline_fts_without_trigram_does_not_rebuild_on_every_open	DELETE FROM messages_fts + full reinsert on every open	zero
   test_pending_fts_rebuild_markers_survive_a_trigramless_open	markers '30' / '10' → None / None	both survive
   test_missing_base_trigger_still_repairs_once	repairs, then keeps repairing forever	repairs exactly once, then converges
   test_fts_trigger_subsets_match_the_ddl	symbols do not exist	passes
   test_missing_trigram_trigger_still_repairs_where_the_tokenizer_exists	passes	passes (control — preserved behaviour)

   Five of the six go red without the production change; the sixth is the control that must stay green in both directions.

3. Or reproduce by hand on a host whose SQLite predates 3.34 (python3 -c "import sqlite3; print(sqlite3.sqlite_version)"): open a populated state.db twice with hermes sessions list and watch the second open re-index the corpus.

The two control tests are the point of the design: test_missing_base_trigger_still_repairs_once proves a real degradation is still repaired on a trigram-less host, and test_missing_trigram_trigger_still_repairs_where_the_tokenizer_exists proves a capable host is completely unaffected. test_fts_trigger_subsets_match_the_ddl pins the derived subsets against the DDL each trigger actually comes from, so renaming a trigger without moving it between DDL blocks fails loudly instead of quietly reintroducing an unsatisfiable check.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — I ran the state/FTS surface rather than the whole tree, and it is green: tests/test_hermes_state.py (226), tests/state/ + tests/hermes_state/ (129), and tests/test_fts_cjk_bigram.py, tests/test_fts_update_of_narrowing.py, tests/test_search_slow_query_log.py, tests/test_schema_read_probe.py, tests/test_hermes_state_readonly_preflight.py, tests/test_hermes_state_wal_fallback.py, tests/test_state_db_malformed_repair.py, tests/test_state_db_stats.py, tests/test_zeroed_state_db.py, tests/test_hermes_state_compression_busy_retry.py, tests/test_hermes_state_compression_locks.py (98) — 453 passed, 0 failed. Leaving this unticked rather than claim a full-tree run I did not complete; CI covers the rest.
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15 (Darwin 25.4), Python 3.11.15, SQLite 3.50.4. The trigram-less runtime is simulated with the test file's existing _NoTrigramConnection, which raises the exact no such tokenizer: trigram that an older SQLite does — I have not run this on a real SQLite < 3.34 host.

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A — docstrings and inline comments on the touched code; no user-facing docs affected
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A — no config keys
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A — the change is pure SQLite-capability logic with no platform-specific paths; only macOS was exercised locally
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Related / Positioning

#69085 (@eiritsu, "drop orphan FTS triggers on startup") edits the same two expressions for a different concern — it ORs a had_orphans flag in so that non-canonical duplicate triggers force a rebuild. It does not address this bug: with orphans absent, had_orphans is False and the comparison stays 3 < 6, so the permanent loop remains.

The two compose mechanically rather than conflicting — the merged form is:

if had_orphans or base_triggers_missing or (
    trigram_enabled and trigram_triggers_missing
):

I have deliberately not absorbed _drop_orphan_fts_triggers here, because it is a separate concern with its own scanning helper and its own tests, and folding it in would make this diff two ideas instead of one. Worth noting for whoever reviews them together: #69085's production hunks target hermes_state.py:3108-3181, but _init_schema no longer lives there since the 21c7ae85630 mixin split moved it to hermes_state_schema.py, so it will need a rebase before it can apply either way.

#82568 (@thanosapollo) also touches hermes_state_schema.py, but inside _reconcile_columns (hunks at @@ -394 and @@ -415) with its tests in a new file. No overlap with this change.

[Copilot]

Pull request overview

Fixes a startup performance/correctness bug in SessionDB where the FTS “triggers need repair” check could never converge on SQLite builds that lack the trigram tokenizer (SQLite < 3.34). The change makes the trigger-repair gate aware of which triggers are actually creatable on the current host, preventing repeated full rebuilds and preserving deferred rebuild resume markers.

Changes:

• Split the canonical _FTS_TRIGGERS set into “base” vs “trigram-only” subsets and only require the trigram subset when trigram DDL actually succeeds.
• Extend _fts_trigger_count to optionally count only a provided subset of trigger names (with a safe empty-set fast path).
• Add regression tests that trace executed SQL to ensure trigram-less hosts no longer rebuild FTS on every open while preserving real repair behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
hermes_state_schema.py	Avoids a permanently-unsatisfiable trigger-count gate by checking base vs trigram triggers against actual trigram availability.
tests/test_hermes_state.py	Adds trace-based regressions proving the rebuild loop is eliminated on trigram-less SQLite while control cases still rebuild when appropriate.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(state): stop rebuilding the whole FTS index on every open when the trigram tokenizer is missing

1. The subset split (hermes_state_schema.py ~lines 41–47) classifies triggers by name substring ("_trigram_" in n). The pinning test test_fts_trigger_subsets_match_the_ddl protects the current set, but a future trigger rename or DDL refactor could silently reintroduce an unsatisfiable gate. Deriving the subsets from the DDL constants themselves (membership in FTS_TRIGRAM_SQL / LEGACY_FTS_TRIGRAM_SQL) would make the classification self-maintaining.

2. Repair-gate asymmetry: the base half (base_triggers_missing) triggers a rebuild unconditionally — even when _fts_enabled is False because the base DDL itself soft-failed on a no-FTS5 host — while the trigram half is gated on trigram_enabled. This matches prior behavior, but since this PR is precisely about the repair gate, consider gating the base half on _fts_enabled too so a permanently-unsatisfiable base condition cannot loop either.

3. trigram_triggers_missing is measured before _ensure_fts_schema runs and combined with trigram_enabled (known only after) — correct per the comment, and the tests cover the key scenarios (trigramless converge, base-drop repair-once-then-quiet, trigram-drop repair on capable hosts). The statement-tracing test harness is a good pattern for this class of bug.

[teknium1]

Merged via PR #93441 — your commit was cherry-picked onto current main with your authorship preserved in git history (merge commit 608a56e).

The only adjustment was composing your split repair predicate with the new cross-process rebuild authority that landed in #93428 after you opened this: base_triggers_missing or (trigram_enabled and trigram_triggers_missing) now gates _run_admitted_startup_rebuild(...), so the rare legitimate rebuild is also serialized fail-closed across processes. Your statement-trace tests and the DDL-pinning test came through unchanged and pass.

Nice diagnosis — the permanently-unsatisfiable six-trigger gate on pre-3.34 SQLite was subtle, and the resume-marker loss angle made the impact concrete. Thanks!