feat(state): add sessions.trigram_fts config flag to disable the trigram FTS index - #68394
feat(state): add sessions.trigram_fts config flag to disable the trigram FTS index#68394fadhlillah2 wants to merge 1 commit into
Conversation
…ram FTS index The messages_fts_trigram FTS5 index (CJK/substring recall) carries a per-message INSERT/UPDATE/DELETE trigger and is frequently the largest contributor to state.db growth. sessions.trigram_fts (default true) lets operators who don't need CJK/substring search opt out: creation is skipped across all three schema paths (v10 migration, v11 migration, steady-state init), existing triggers are dropped on the next open so writes stop paying the cost, and the shadow table is torn down in optimize_fts() where a paired VACUUM reclaims the space. CJK/substring search falls back to LIKE — the same graceful path taken when the trigram tokenizer is unavailable. The trigger-repair heuristic now expects only the triggers the active configuration requires, so a deliberately-absent trigram index no longer forces a full FTS rebuild on every open.
|
Thanks for preserving the config.yaml approach and for documenting the storage rationale. The underlying opt-out is still absent on current main, so this is not redundant. Problems
Suggested changes
Automated hermes-sweeper review. |
SummaryEleven PRs address or reference the FTS storage complex: #20239 and merged #65798 replace inline indexes with external-content layouts; #35826, #43701, and #65798 reduce trigram indexing of tool data; and #22710, #27770, #42190, #45916, #57761, #68089, and #68394 add variants of a trigram opt-out. The merged #65798 addresses the principal duplication and tool-row amplification causes, while the open #68394 targets the still-absent operator opt-out but predates the resulting v23 architecture. Related pull requests
Duplicates#22710 and #27770 are the same patch lineage; #42190 and #45916 substantially duplicate that HERMES_DISABLE_FTS_TRIGRAM approach, while #57761 implements the same opt-out under another environment variable. #68089 and #68394 are competing config.yaml-based successors; #20239, #35826, and #43701 supplied storage-layout or tool-indexing changes later consolidated in #65798. Suggested consolidationAuthor action on #68394: rebase onto current main, or split out the config-based opt-out and port it across hermes_state_schema.py:689-728 and hermes_state_search.py:393-438, including the optimize-storage lifecycle, an explicit CJK-bigram contract, and temp-HERMES_HOME YAML tests for default, disable, and re-enable. This follows the keep_open salvage review rather than bypassing it; the closed environment-variable duplicates #22710, #27770, #42190, #45916, and #57761 remain closed, #20239/#35826/#43701 are superseded by merged #65798, and #68089 can remain closed as the competing config-based predecessor. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I22478(["issue #22478 (closed)"])
I43690(["issue #43690 (closed)"])
I55233(["issue #55233 (closed)"])
subgraph Dup68089 ["PRs duplicating each other"]
P68089["PR #68089 (closed)"]
P68394["PR #68394 (open)"]
end
P68394 -.->|partial| I22478
P68394 -.->|partial| I43690
P68394 -->|fixes| I55233
class I22478 closed
class I43690 closed
class I55233 closed
class P68089 closed
class P68394 open
class P68394 target
click I22478 "https://github.com/NousResearch/hermes-agent/issues/22478"
click I43690 "https://github.com/NousResearch/hermes-agent/issues/43690"
click I55233 "https://github.com/NousResearch/hermes-agent/issues/55233"
click P68089 "https://github.com/NousResearch/hermes-agent/pull/68089"
click P68394 "https://github.com/NousResearch/hermes-agent/pull/68394"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 11 pull requests and 3 issues in this complex. Diffs were read for 10 of 11 PRs (rest unavailable); Assessment working set: 249 kB of PR diffs, 53 kB of issue/PR text, 37 kB of discussion (35 comments), 39 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What does this PR do?
Adds
sessions.trigram_fts(defaulttrue) — aconfig.yamlflag to disable themessages_fts_trigramFTS5 index, the secondary trigram index that powers CJK/substring recall insession_search.That index carries an INSERT/UPDATE/DELETE trigger on every message and is frequently the single largest contributor to
state.dbgrowth (on one production install: ~273 MB of a 549 MB DB — 88% of it FTS, mostly the doubled trigram data/content shadow tables). Operators who don't need CJK or substring search can now opt out; CJK/substring queries fall back toLIKE— the exact graceful path already taken when the SQLite build lacks the trigram tokenizer.Why this approach (and why the prior attempts were closed)
This is deliberately a config knob, not an env var. Every earlier attempt at this feature used a
HERMES_*environment variable and was closed under theAGENTS.mdpolicy that reserves.env/HERMES_*for credentials and requires behavioral flags inconfig.yaml:HERMES_DISABLE_FTS_TRIGRAM) — closed, config-policyHERMES_FTS_TRIGRAM=0) — closed, dup + config-policyThis PR is the first to implement the maintainers' stated requirement (a
sessions.*config key, matchingauto_prune/vacuum_after_prune/write_json_snapshots). It also folds in the two technical notes left on the closed cluster (#57761 → #27770): the trigger-repair count now tolerates the deliberately-absent trigram triggers, and all migration paths are covered — not just steady-state init.Related Issue
Fixes #55233 (and addresses the size evidence in #22478 / #43690).
Type of Change
Changes Made
hermes_cli/config.py— newsessions.trigram_fts: Truedefault (deep-merged at read time, no_config_versionbump needed since it's an optional default).hermes_state.py_trigram_fts_enabled()— reads the knob via a deferredload_config_readonly()import (keeps the storage layer cycle-free; a single global read keeps everySessionDBopen in agreement so the schema can't oscillate across processes)._drop_trigram_fts_triggers()— trigger-only drop (leaves base FTS intact)._init_schema. When disabled, creation/backfill is skipped and existing trigram triggers are dropped so writes stop paying the cost immediately.optimize_fts()— the deliberate, lock-held teardown point: drops the (possibly large) trigram shadow table when disabled so a followingVACUUMreclaims the pages. The heavyDROPis intentionally not in__init__(avoids contending with a concurrent writer during a plain open).tests/test_hermes_state.py— two tests: disabled path (no table, base search works, no rebuild loop, idempotent reopen) and re-enable flip-back (index rebuilt).website/docs/user-guide/sessions.md— documents the knob + LIKE fallback + reclaim path.How to Test
sessions.trigram_fts: false→messages_fts_trigramnever created;db._trigram_available is False; base word search still returns results; substring/CJK degrade toLIKE.true→ trigram index rebuilt on next open.Verified end-to-end against real runtime deps (disabled path, idempotent reopen, flip-back) plus the config-read path (
trigram_fts: false→False, absent →True).Checklist
Code
feat(state): ...)config.yamlperAGENTS.mdDocumentation & Housekeeping
website/docs/user-guide/sessions.md) + inline config schema commentcli-config.yaml.example— N/A (it does not enumerate thesessions.*keys; peersauto_prune/vacuum_after_pruneare not listed there either)CONTRIBUTING/AGENTS— N/A (no architecture change)