Skip to content

perf(state): external-content FTS5 — stop storing message text three times - #57965

Closed
Aoshi-Dev wants to merge 1 commit into
NousResearch:mainfrom
Aoshi-Dev:perf/fts5-external-content
Closed

perf(state): external-content FTS5 — stop storing message text three times#57965
Aoshi-Dev wants to merge 1 commit into
NousResearch:mainfrom
Aoshi-Dev:perf/fts5-external-content

Conversation

@Aoshi-Dev

Copy link
Copy Markdown

What does this PR do?

Switches both session-store FTS5 tables (messages_fts, messages_fts_trigram) from inline-content mode to external content backed by a new messages_fts_source view over messages.

In inline mode, each FTS table keeps a full private copy of every message's indexed text in its *_content shadow table — so the same bytes live three times on disk: once in messages, once per FTS table. On a real-world 752 MB state.db (540 sessions / 49k messages), those two redundant copies alone accounted for 256 MB:

table size
messages (canonical) 185 MB
messages_fts_trigram_data (index) 254 MB
messages_fts_trigram_content (redundant copy) 128 MB
messages_fts_content (redundant copy) 128 MB
messages_fts_data (index) 33 MB

After migration + hermes sessions optimize: 752.5 MB → 496.4 MB (−34%), with search results and snippet() output byte-identical.

Why external content is safe now (the #16751 history)

The v11 migration deliberately moved away from external content ("Fixes #16751"), but the underlying bug wasn't external content itself — it was the delete trigger passing old.content while the insert trigger indexed content || tool_name || tool_calls, so the FTS5 'delete' command received text that never matched what was indexed.

This PR eliminates that failure class structurally: the view is the single source of truth for the indexed expression. 'rebuild' and snippet()/highlight() read the view, and the insert/delete/update triggers inline the same expression over NEW/OLD values (AFTER triggers can't consult the view because the row is already gone/changed). Insert, delete, and rebuild can no longer disagree about what text a rowid maps to.

Related Issue

Related to #43690 (FTS5 trigram bloat on tool_calls JSON — this removes the duplicated content half of that cost; the trigram index expansion itself remains) and #53415 (state.db size contributing to resident memory).

Type of Change

  • ♻️ Refactor (no behavior change) — storage-layout/perf change with an automatic schema migration; search behavior is unchanged

Changes Made

  • hermes_state.py
    • New messages_fts_source view; FTS_SQL / FTS_TRIGRAM_SQL now create external-content tables (content='messages_fts_source', content_rowid='id')
    • Delete/update triggers use the FTS5 'delete' command built from OLD values with the same concat expression the insert trigger uses
    • _rebuild_fts_indexes() uses the FTS5 'rebuild' command (external-content tables reject plain DELETE FROM)
    • SCHEMA_VERSION 17 → 18 with a v18 migration: drops inline-mode FTS tables, recreates from the new DDL, and rebuilds from the view. Skips when tables are already external-content (fresh DBs, or ancient DBs the v11 block just rebuilt with current DDL). Mirrors the v11 block's FTS5-unavailable error handling.
  • tests/test_hermes_state.py
    • TestFTS5ExternalContentMigration: hand-built v17 inline DB migrates with search/tool-token parity, *_content shadow tables gone, version bumped
    • delete/update consistency test: old tokens leave the index, new ones enter, FTS5 integrity-check passes (regression guard for the session_search does not index tool_calls or tool_name #16751 value-mismatch class)

How to Test

  1. pytest tests/test_hermes_state.py tests/test_state_db_malformed_repair.py -q — includes the new v17→v18 migration tests and the existing v10→v11 upgrade path
  2. On a real pre-v18 state.db: open it once (migration runs automatically, ~35 s for 49k messages), then hermes sessions optimize to reclaim the freed pages
  3. Verify parity: hermes sessions stats (same session/message counts), any search_messages() query returns identical hits and snippets

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched existing PRs — no duplicate
  • My PR contains only changes related to this fix/feature
  • I've run the state-store test suites (tests/test_hermes_state.py, tests/test_state_db_malformed_repair.py, tests/hermes_state/, WAL fallback, compression locks): 384 passed. Full pytest tests/ -q on this machine has pre-existing Windows-only failures (file-locking in TemporaryDirectory teardown) that are identical on a clean main checkout
  • I've added tests for my changes
  • I've tested on my platform: Windows 11, Python 3.13.12 (also exercised against a production 752 MB DB)

Documentation & Housekeeping

  • Docstrings/comments updated in hermes_state.py — schema comments document the view contract
  • cli-config.yaml.example — N/A (no config keys added)
  • CONTRIBUTING.md / AGENTS.md — N/A
  • Cross-platform impact considered — pure SQLite DDL/DML, no platform-specific code; FTS5 'delete'/'rebuild' commands and external-content views are long-standing FTS5 features
  • Tool descriptions/schemas — N/A

Screenshots / Logs

$ hermes sessions optimize   # after the v18 migration, real 540-session DB
Optimizing session store (FTS merge + VACUUM)…
Optimized 2 FTS index(es).
Database size: 752.5 MB -> 496.4 MB (reclaimed 256.1 MB)

$ hermes sessions stats
Total sessions: 540
Total messages: 49062

…times

Both messages_fts and messages_fts_trigram were inline-mode FTS5 tables,
each keeping a full private copy of every message's indexed text in its
*_content shadow table. On a real-world 752 MB state.db those two copies
alone accounted for ~256 MB (the same bytes living in messages, base FTS,
and trigram FTS).

Switch both tables to external content backed by a new
messages_fts_source view over messages:

- The view is the single source of truth for the indexed expression
  (content || tool_name || tool_calls). FTS5 reads it for 'rebuild' and
  for snippet()/highlight(), so search results and snippets are
  byte-identical to inline mode.
- Insert/delete/update triggers derive the text from the same expression
  over NEW/OLD values; the FTS5 'delete' command therefore always receives
  exactly what the insert indexed, structurally preventing the NousResearch#16751
  value-mismatch class of corruption that motivated the v11 move to
  inline mode.
- _rebuild_fts_indexes now uses the FTS5 'rebuild' command (external
  content tables reject plain DELETE FROM).
- v18 migration drops the inline tables and recreates + rebuilds from the
  view; skips when tables are already external (fresh DBs, or ancient DBs
  the v11 block just rebuilt with the current DDL).

Tests: hand-built v17 inline DB migrates with search/tool-token/snippet
parity and no *_content shadow tables; delete/update paths keep the index
consistent per FTS5 integrity-check.
@alt-glitch alt-glitch added type/perf Performance improvement or optimization comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 3, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the measured FTS storage-reduction work. Current main still uses inline FTS tables (hermes_state.py:913-965), so the performance premise remains relevant.

Problems

  • The migration cannot land as written: the PR uses schema v18, but current main is at SCHEMA_VERSION = 21 (hermes_state.py:143) and already uses current_version < 18 for gateway metadata backfill (hermes_state.py:1649-1662). GitHub also reports this branch as conflicting. The FTS migration needs a new version and must preserve the current migration chain.

Suggested changes

  • Rework the migration and fixture around a current-main pre-upgrade database, retaining the v18 gateway backfill and later migrations.
  • Update website/docs/developer-guide/session-storage.md:103-154 and its zh-Hans counterpart for the changed FTS layout and migration history.

This is an automated hermes-sweeper review.

Comment thread hermes_state.py
if current_version < 18:
# v18: switch both FTS tables from inline to external-content
# mode backed by the messages_fts_source view. Inline mode
# stored a full private copy of every message's text in each

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This v18 migration now collides with current main: SCHEMA_VERSION is 21 and v18 already performs the gateway metadata backfill. Please allocate a new migration version and retain the current v18/v20 blocks when salvaging.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing with credit — this exact idea (external-content FTS5 tables replacing the inline copies) shipped on main in PR #65798 (schema v23). Your PR was submitted first and correctly identified the 3x storage amplification; the merged implementation extends the same approach with a tool-row-free trigram index, a resumable/throttled rebuild engine, and an opt-in hermes sessions optimize-storage conversion path for existing installs.

Thanks @Aoshi-Dev — you were the first to bring the external-content design to the repo, and the real-world 752 MB analysis in this PR body was solid evidence for the direction.

@teknium1 teknium1 closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit 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/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

session_search does not index tool_calls or tool_name

4 participants