Skip to content

fix(state): rebuild wrong-shaped FTS vtables before the optimize backfill - #88696

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-72716
Open

liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-72716

Conversation

@liuhao1024

Copy link
Copy Markdown

What does this PR do?

Fixes the interrupted-resume shape reported as a follow-up on #72716 (comment, 2026-08-17): fts_rebuild_high_water/fts_rebuild_progress markers are set while messages_fts / messages_fts_trigram exist as v22-shaped single-column content vtables. _ensure_fts_schema's DDL is CREATE VIRTUAL TABLE IF NOT EXISTS, which silently accepts the wrong-shaped table — the first fts_rebuild_step INSERT then fails with table messages_fts has no column named tool_name, and because the step loop treats every OperationalError as retryable, optimize-storage hangs forever (reproduced: infinite retry on a 3-row database; the reporter hit it on a real DB with high_water=60993).

This PR adds _drop_mismatched_fts_vtables() — probe each FTS vtable's column set (via SELECT ... LIMIT 0 description), drop any that lack the v23 columns, and reset fts_rebuild_progress to 0 (rows claimed under the wrong shape may be missing even though the marker advanced). The guard runs after the legacy/pending branch dispatch, so every path reaches the backfill with correctly-shaped tables — including the legacy and pending combination that previously skipped both branches and fell straight into the dead loop. The surviving markers drive a full re-population, so a resume now completes instead of hanging: search for historical rows is restored and the markers clear.

The original empty-stamp settle defect from #72716 was already fixed by #76832 (merged); this covers the shape that fix missed.

Related Issue

Fixes #72716 (interrupted-resume shape)

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_state_search.py: new _drop_mismatched_fts_vtables() — column-set probe + drop + progress reset for both vtables; called after the legacy/pending dispatch in optimize_fts_storage() followed by a re-ensure, with docstrings documenting the IF-NOT-EXISTS blind spot and the [Bug]: optimize-storage can stamp empty FTS after interrupted demote (permanent search loss) #72716 follow-up shape
  • tests/test_hermes_state.py: new test_optimize_resume_rebuilds_wrong_shaped_vtables — builds the reporter's exact durable state (single-column vtables + markers + advanced progress), asserts the v23 INSERT fails on the wrong shape (sanity), then asserts the optimize run completes, the vtable comes back with the v23 column set, the index is fully populated (docsize count == messages count), historical search works, and the markers clear

How to Test

  1. python -m pytest tests/test_hermes_state.py::TestFTSExternalContentMigration -q — Observed result: 15 passed (the fix(state): do not stamp empty FTS after interrupted optimize-storage demote (salvage #72717) #76832 regression tests continue to pass: the shape guard is a no-op on correctly-shaped tables).
  2. python -m pytest tests/test_hermes_state.py tests/test_state_db_malformed_repair.py tests/test_journal_mode_config.py -q — Observed result: 272 passed, 1 failed — the single failure (TestFTS5Search::test_search_projection_skips_context_enrichment_queries) reproduces identically with this change stashed (pre-existing local environment noise, not a regression).
  3. The new test pins the reporter's scenario: pre-fix, optimize_fts_storage on that state hangs in the retry loop (verified locally: the backfill step returns "more" forever with progress frozen); post-fix it completes in ~4s with search restored.

Checklist

Code

  • I have read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this is not a duplicate (fix(state): do not stamp empty FTS after interrupted optimize-storage demote (salvage #72717) #76832 is the merged fix for the original shape; no open PR covers the wrong-shaped-vtable resume)
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I have run the FTS migration classes and the full hermes_state suite (272 passed; the one failure is pre-existing local noise verified via a stashed baseline)
  • I have added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I have tested on my platform: macOS 15 (arm64)

Documentation & Housekeeping

For New Skills

N/A

Screenshots / Logs

$ python -m pytest "tests/test_hermes_state.py::TestFTSExternalContentMigration" -q
15 passed in 1.19s
# Reporter's shape, end-to-end (in-process):
optimize took 4.19 ok: True
search deployment: 1
markers: None None

…fill

A follow-up interrupted-resume shape of NousResearch#72716 (reported on the issue,
2026-08-17): fts_rebuild markers set while messages_fts /
messages_fts_trigram are v22-shaped single-column content vtables. The
resume path's _ensure_fts_schema uses CREATE VIRTUAL TABLE IF NOT
EXISTS, which silently ACCEPTS the wrong-shaped table, and the very
first fts_rebuild_step INSERT then fails with "no column named
tool_name" - the step loop treats every OperationalError as retryable,
so optimize-storage hangs forever on a 3-row database.

Add _drop_mismatched_fts_vtables(): probe each vtable's column set,
drop any that lack the v23 columns, and reset fts_rebuild_progress to
0 (rows claimed under the wrong shape may be missing even though the
marker advanced). It runs after the legacy/pending branch dispatch, so
every path - including the legacy+pending combination that previously
skipped both branches - reaches the backfill with correctly-shaped
tables; the markers then drive a full re-population.

Fixes NousResearch#72716 (interrupted-resume shape; the original empty-stamp
settle was fixed by NousResearch#76832)
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 17, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

  1. hermes_state_search.py:_drop_mismatched_fts_vtables — Positive: the root cause is precisely identified (IF NOT EXISTS silently accepting a v22 single-column vtable left by an interrupted demote) and the fix respects the existing recovery machinery rather than bypassing it — dropping mismatched shapes and resetting fts_rebuild_progress to 0 lets the chunked backfill repopulate from scratch while keeping markers durable across crashes during the repair itself. The guard placement before any backfill path also closes the legacy+pending combination that skipped both resume branches.

  2. tests — Positive: the regression builds a genuinely wrong-shaped database, first asserts the pre-fix failure mode (INSERT dies on the missing column) so the test can't pass vacuously, then proves full recovery — v23 shape restored, docsite counts matching messages, search returning hits, markers cleared. No change requested.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: optimize-storage can stamp empty FTS after interrupted demote (permanent search loss)

3 participants