fix(state): do not stamp empty FTS after interrupted optimize-storage demote (salvage #72717) - #76832
Merged
kshitijk4poor merged 3 commits intoAug 2, 2026
Conversation
… demote Demote wrote the empty v23 schema via executescript inside BEGIN IMMEDIATE, which commits early and can leave trash + empty indexes without rebuild markers. Re-run then tore down trash and stamped fts_storage_version with docsize=0, permanently losing historical session search. Stage markers with the demote, create schema only after they are durable, heal empty-index bookkeeping on resume, and refuse settle until the base index is populated. Settle refusal returns ok=False instead of raising, and resume fails fast if the base v23 table cannot be re-created. Orphan-marker repair only resets a missing fts_rebuild_progress to 0 once the index is known empty: the chunk worker replays its whole selected id range without an anti-join, so a partially indexed DB that lost only its progress key is first reset to a known-empty surface, then rebuilt. Ported onto the SessionDB mixin split (hermes_state_search.py / hermes_state_schema.py).
_fts_external_index_empty_with_messages runs on every writable open via the _init_schema fts_storage_version stamp condition. COUNT(*) is a full b-tree scan on both messages and messages_fts_docsize (~100ms per open on a 2M-row DB, measured); the function only ever compares against zero, so EXISTS(SELECT 1 ...) gives the identical boolean in O(1).
…LETE _reset_fts_index_to_empty used a no-WHERE DELETE, whose docstring claimed FTS5 treats it as an efficient drop-all. That's true only for ordinary rowid tables — on external-content FTS5 each deleted row's tokens are regenerated from the content table, making it O(rows) (measured ~12us/row: 0.22s @100k, 5.2s @400k, ~25s projected @2m) while holding the write lock. It also corrupts the index when indexed rows have diverged from messages — exactly the broken-bookkeeping shape this repair path handles. The FTS5 'delete-all' special command is the documented O(1) truncate for external-content tables (measured 1.6ms @100k) and truncates unconditionally regardless of divergence.
kshitijk4poor
enabled auto-merge (rebase)
August 2, 2026 16:02
19 tasks
Open
1 task
5 tasks
This was referenced Aug 8, 2026
Skywind5487
referenced
this pull request
in Skywind5487/hermes-agent
Aug 9, 2026
This was referenced Aug 9, 2026
Closed
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvages #72717 by @Adolanium — cherry-picked to preserve authorship, with one efficiency follow-up commit on top.
Context — what this fixes, for whom
Any long-time user who runs
hermes sessions optimize-storage(the v22→v23 FTS storage migration) and has the process die mid-run — laptop lid close, ctrl-C, OOM — can hit a crash window where the re-run permanently destroys search over their entire message history while reporting success. The interrupted demote leaves empty v23 index tables with no backfill markers; the re-run tears down the old index (the only remaining copy of the indexed data), stampsfts_storage_versionas optimized, andfts_optimize_available()returns False forever after.session_searchsilently returns nothing for all historical messages, and the user is never offered the repair.Verified side-by-side on current main (0a62610) vs this branch, identical simulated crash-window state, then
optimize_fts_storage(vacuum=False):{"ok": true}{"ok": true}search_messages('deployment')afterfts_optimize_available()What the fix does (all from #72717, kept verbatim)
BEGIN IMMEDIATEas the demote, beforeexecutescriptcreates the empty v23 schema (which implicitly commits and therefore runs outside the write transaction, same rule as the CJK recreate path). A crash at any point now leaves a resumable claim.{"ok": False, "reason": ...}instead of silently succeeding._repair_optimize_bookkeeping()runs at the top of every optimize — re-seeds a full backfill claim for the unmarked-empty-index shape (clearing a premature stamp), and repairs orphanedhigh_water-without-progressmeta (resetting a partially-populated index to empty first, because the chunk worker replays whole id ranges without an anti-join — otherwise every surviving row would be duplicated)._init_schemastamp condition andfts_optimize_available()also gained the empty-index leg, so all threefts_storage_versionwriters are guarded and a damaged DB keeps being offered the repair.Follow-up commit (ours)
_fts_external_index_empty_with_messagesis evaluated on every writable open via the_init_schemastamp condition. The original usedCOUNT(*)on bothmessagesandmessages_fts_docsize— a full b-tree scan measured at ~100ms per open on a 2M-row DB (the heavy installs this subsystem exists for are 25GB). The function only compares against zero, so the follow-up switches toEXISTS(SELECT 1 ...)— identical boolean, O(1) (measured 0.05ms).Verification
tests/test_hermes_state.py: 147 passed (includes the PR's 313 lines of new tests: crash-window restore, premature-stamp heal, orphan-marker repair, no-duplicate partial rebuild, markers-before-schema ordering, settle refusal — plus FTS5integrity-checkassertions)tests/state/test_fts_runtime_rebuild.py,tests/test_fts_cjk_bigram.py,tests/test_session_db_read_path_split.py): 18 passed, 4 pre-existing environmental skipshermes_state_search.py/hermes_state_schema.pyto main makes all 7 new guard tests fail; restoring goes green — the EXISTS follow-up does not defeat the fix's testsoptimize_fts_storageishermes_cli/sessions_cmd.py(optimize-storage command), which already handlesok: False; no gateway/web/TUI callersCloses #72717 (superseded by this salvage — original author credited via cherry-pick authorship).
Post-review follow-up (commit 3)
The simplify pass caught that
_reset_fts_index_to_empty's plain no-WHEREDELETEis O(rows) on external-content FTS5 (measured ~12µs/row — 0.22s @100k, 5.2s @400k, ~25s projected @2m — while holding the write lock), and its 'efficient drop-all' claim only holds for ordinary rowid tables. Swapped to the FTS5'delete-all'special command: the documented O(1) truncate for external-content tables (measured 1.6ms @100k) that also truncates safely when indexed rows have diverged frommessages— exactly the broken-bookkeeping shape this repair path handles. Mutation check re-run on the final stack: all 7 guard tests fail on reverted production code, pass restored.