Commit Graph

12 Commits

Author SHA1 Message Date
izumi0uu 1527a81b5e fix(state): keep canonical writes available when FTS is corrupt 2026-08-09 14:10:21 -07:00
Teknium 90311ee75f fix(search): strip % from non-CJK FTS5 queries
Closes the residual the contributor's own triage comment flagged: % was
excluded from the special-char class to protect the CJK LIKE fallback,
but a non-CJK query never reaches that fallback (is_cjk gates it), so
'50%' still hit MATCH raw and silently returned zero results. Strip %
whenever the sanitized query contains no CJK; the CJK path keeps its
pre-existing contract. Regression tests for both directions.
2026-08-08 19:17:09 -07:00
Drexuxux c595dcb955 fix(search): strip the FTS5 special characters the sanitizer was missing
_sanitize_fts5_query's strip step only removed +{}():"^ . Every other
character FTS5's grammar rejects outside a quoted phrase reached MATCH
raw and raised, and — as the step's own comment says about the colon it
was fixed for — the execute site swallows that into zero results. Session
search silently found nothing for ordinary queries:

  it's            fts5: syntax error near "'"
  gateway/run.py  fts5: syntax error near "/"
  user@host       fts5: syntax error near "@"
  a,b             fts5: syntax error near ","
  why?            fts5: syntax error near "?"
  e=mc2           fts5: syntax error near "="

Complete the class and assemble it with re.escape, because written as a
regex literal the backslash was eaten as an escape and never made it in
(C:\path\file still raised after the first pass).

Measured against a real FTS5 table over 651 realistic queries:
373 unparsable before, 77 after. The remainder is leading/trailing "." and
"-", which #43889 already covers.

% is deliberately left in: the CJK path falls back to a LIKE search that
needs it literal and escapes wildcards itself, so stripping it widened
those queries onto unrelated rows (test_cjk_like_escapes_wildcards).
2026-08-08 19:17:09 -07:00
kshitij fecba5afcc refactor(agent): fold simplify findings — DB picker parity, single scan, canonical strip delegation
Review-pass follow-ups (three parallel reviewers, findings verified):

- hermes_state_search.py list_recent_user_messages now drops legacy
  standalone compaction handoffs in the decode loop (SQL can't see them:
  durable role=user, no display_kind). Closes the /undo N pairing skew
  where the in-memory count (new predicate) and the DB soft-delete pick
  (old predicate) targeted different turns on legacy sessions. Fetches
  with headroom so the requested limit is still honored. 3 new tests,
  mutation-checked (no-op'ing the skip fails 2/3).
- _should_skip_model_call_for_reference_handoff: single drive-check scan
  (was two — once inside the restore helper, once after); the restore
  helper no longer re-scans and its return value now decides the verdict.
- _final_response_from_messages replaced by the _HANDOFF_SKIP_FINAL_RESPONSE
  constant it always returned (parameter was unused).
- _handoff_carries_live_user_content delegates to the canonical
  _strip_context_summary_handoff_message — also fixes the edge where a
  merged-shaped row with an EMPTY preserved prior tail was wrongly
  treated as carrying live content.
- Site-level guard test for rollback.restore with a legacy handoff row
  (predicate-in-context, complements the unit tests).
2026-08-07 19:44:35 +05:30
Chen Jin 23dce021a5 perf(fts): drain trash tables with a high-water marker instead of re-scanning
_fts_teardown_trash_step deleted rows via 'WHERE key IN (SELECT key
LIMIT N)' — each chunk's subquery re-scanned from the start of the
table, so chunk k skipped past (k-1)xN already-deleted rows: O(n²)
total row visits. On a v22 shadow table with ~230K rows that is on the
order of 10^8 row visits, turning optimize-storage teardown into a
multi-hour grind on slow disks, with a write lock held per chunk.

Single-column INTEGER-PK trash tables now drain via a fts_teardown_<tbl>_progress
high-water marker mirroring fts_rebuild_step: each chunk claims rows
past the marker (SELECT ... WHERE key > ? ORDER BY key LIMIT N), deletes
the claimed range, and publishes the new marker in the same transaction.
Per-chunk work is bounded → O(n) total.

TEXT-PK tables (the FTS config shadow table, pk like 'version') and
compound-key tables fall back to the legacy chunked delete — those are
small by construction.

Fixes #79324
2026-08-07 18:40:47 +05:30
kshitij 52a5fc0048 refactor(state): consolidate SQL LIKE escaping onto one shared helper
Follow-up to #79722, which introduced _escape_like in hermes_state.py for
the prune/archive filter fix. The same three-replace escape chain existed
as five more inline copies in hermes_state.py and two in
hermes_state_search.py (which must not import hermes_state — cycle).

Move the helper to hermes_state_common.escape_like (the module that exists
for exactly this) and route every copy through it:

- hermes_state.py: session-ID prefix resolution, find_session_by_title,
  get_next_title_in_lineage, the _like_pattern closure in list projection,
  and the kanban cwd retag
- hermes_state_search.py: the two LIKE-fallback token escapes

hermes_state re-imports it as _escape_like for back-compat. No behavior
change: every site produces byte-identical SQL patterns.
2026-08-06 04:28:44 +05:30
Kyzcreig 2f32092b38 hermes sessions optimize-storage aborts with
```
Error: optimization failed: no such table: messages_fts_trigram
No data was lost. Re-run to resume.
```

on any install where the trigram FTS index is legitimately absent. The failure is
deterministic — re-running can never make progress, because the crash happens at the same
point every time — so the database is permanently stuck on the legacy high-footprint FTS
layout with no supported way forward.

Observed on a 5.4 GB production `state.db`. After the fix the same database optimized
successfully and shrank to 3.3 GB.

The trigram index is absent whenever the runtime cannot maintain it. On a SQLite build
without the `trigram` tokenizer, `_ensure_fts_schema()` returns `False`, so `__init__`
leaves `self._trigram_available = False` and no `messages_fts_trigram` table on disk. This
is a **supported degraded runtime**, not damage — CJK/substring search falls back to
`LIKE` and everything else works normally. `_is_fts5_unavailable_error()` and
`_warn_trigram_unavailable()` exist specifically to make this path graceful.

Two code paths write the boundary sweep for the deferred FTS rebuild, and only one of them
respects that flag:

| Function | Trigram `INSERT` guarded? |
|---|---|
| `fts_rebuild_step()` | ✅ `if include_trigram:` where `include_trigram = self._trigram_available` |
| `_fts_rebuild_finish()` | ❌ unconditional |

`_fts_rebuild_finish()` runs the boundary sweep at the *end* of the backfill. Its
unguarded `INSERT INTO messages_fts_trigram …` raises `OperationalError`, which propagates
out of `optimize_fts_storage()` and aborts the entire optimization — *after* the backfill
has already completed. Hence the characteristic output showing 100% progress immediately
before the error:

```
Rebuilding index: 100% (909,671/909,671)
Error: optimization failed: no such table: messages_fts_trigram
```

There is a second, quieter consequence. The teardown phase that reclaims the demoted
`fts_v22_trash_*` shadow tables runs *after* the backfill phase in
`optimize_fts_storage()`. Because the crash happens before teardown is ever reached, those
tables are never emptied or dropped — so the space the migration was supposed to reclaim
stays allocated indefinitely, and the leftover trash tables look (misleadingly) like
evidence of a half-finished migration.

Build a populated v23 database, set the deferred-rebuild markers, then reopen it on a
runtime where `_ensure_fts_schema('messages_fts_trigram', …)` returns `False` (exactly
what a SQLite build without the trigram tokenizer produces) and call
`optimize_fts_storage()`:

```
[precondition] trigram absent, _trigram_available=False, rebuild pending  ✓

RED  ✗ optimize_fts_storage raised OperationalError: no such table: messages_fts_trigram
```

With this patch applied, unchanged harness:

```
     optimize_fts_storage returned {'ok': True, 'vacuumed': None}
GREEN ✓ optimize ok; markers cleared; base FTS 'zebra' -> 200 hits
```

Full harness and transcripts in `TEST-EVIDENCE.md`.

Gate the sweep on `self._trigram_available`, exactly as `fts_rebuild_step()` already does:

```python
include_trigram = self._trigram_available

def _do(conn):
    ...
    if include_trigram:
        conn.execute("INSERT INTO messages_fts_trigram(...) ...")
```

The base `messages_fts` sweep and the marker cleanup are untouched, so the rebuild still
finalizes correctly and the index remains complete for every row it is responsible for.
The fix does not disable or weaken search to dodge the error — the regression tests assert
that base FTS still returns results afterwards.

`TestFtsRebuildFinishWithoutTrigram` in `tests/test_hermes_state.py`:

- `test_rebuild_finish_skips_trigram_when_unavailable` — drives `_fts_rebuild_finish()`
  directly on a trigram-less runtime; asserts it completes, clears both rebuild markers,
  and leaves base FTS searchable.
- `test_optimize_fts_storage_succeeds_without_trigram` — end-to-end through the public
  `optimize_fts_storage()` entry point; asserts `ok=True`, markers cleared, search intact.

Both use the existing `_NoTrigramConnection` helper already in the file. Both fail on
`main` with `no such table: messages_fts_trigram` and pass with this patch.

`tests/test_hermes_state.py` passes in full (463 tests → 465 with these two). `ruff` clean.

This PR is the crash only.

A companion PR narrows `_db_opens_cleanly()` so that
`hermes sessions repair --check-only` stops reporting a write-broken FTS schema as
healthy — the gap that makes this class of problem hard to diagnose in the first place.
The two are independent and can land in either order.
2026-08-03 19:09:15 +05:30
Jakub Wolniewicz ffb54305c4 perf(session-search): project fields before enrichment 2026-08-03 17:50:58 +05:30
kshitijk4poor 1e2e69db98 perf(state): truncate FTS index with 'delete-all' instead of plain DELETE
_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.
2026-08-02 21:36:11 +05:30
kshitijk4poor 2febb5823c perf(state): probe empty FTS index with EXISTS instead of COUNT(*)
_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).
2026-08-02 21:36:11 +05:30
Adolanium b2d5995fc6 fix(state): do not stamp empty FTS after interrupted optimize-storage 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).
2026-08-02 21:36:11 +05:30
teknium1 21c7ae8563 refactor: split SessionDB into Search/Schema/Portability mixins (mechanical move, ~2.9K LOC out of hermes_state.py) 2026-07-29 10:14:19 -07:00