Skip to content

Migration safety: rebuild FTS after incompatible-index wipe, fail fast on embedder feature gap#210

Merged
CassidyPrather merged 4 commits into
mainfrom
pr/rust-migration-safety
Jul 17, 2026
Merged

Migration safety: rebuild FTS after incompatible-index wipe, fail fast on embedder feature gap#210
CassidyPrather merged 4 commits into
mainfrom
pr/rust-migration-safety

Conversation

@CorVous

@CorVous CorVous commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

Three defects found while verifying the migration of a live instance (Sapphire, Python-era data) onto the Rust port. All are general port fixes — the migration just exposed them.

  1. fix(history): rebuild FTS from DB when the on-disk index was wiped or empty. TantivyFts::open_or_recreate deletes any incompatible index dir (real case: every Python-written index — the on-disk schema has row_id.fieldnorms=false, the Rust schema builds true) and rebuild_fts had zero call sites, so a wipe left search permanently empty with no error. HistoryStore::open now repopulates any index that was recreated or is empty while its source table has rows (facts previously had no rebuild path at all). One-time, idempotent on reopen, INFO-logged.
  2. fix(run): fail fast on embedder feature-gap before opening the store. create_embedder(...).unwrap_or(None) swallowed FastembedMissing; a fastembed config on a binary built without local-embed mutated the store/FTS and then died in create_projectors with a message recommending backend="hash" — wrapped in the "token may be invalid" hint. resolve_embedder now runs before Familiar::load_from_disk and surfaces the real error (naming local-embed), honoring the factory doc's "refuses to start" contract.
  3. fix(activities): de-flake c1_state_cleared_before_wake_publish. Pre-existing since the Layer-3 port: the invariant holds in the implementation (state cleared strictly before wake publish, same task); the flake was recv_wake's 1s hang-guard racing 6–8 spawn_blocking DB round-trips under load (~5% idle, ~67% under CPU saturation). Raised to 30s — all six call sites .expect(Some), so no assertion weakened.

Plus a style: commit (rustfmt).

Verification

  • Full suite green under default features and --features discord; clippy -D warnings clean under both; fmt --all --check clean.
  • End-to-end against a copy of real production data (17 MB DB, 10132 turns / 3758 facts, Python-written tantivy 0.26.0 indexes): fail-fast aborts in ~7 ms with zero mutation; with the gap resolved, startup rebuilds both indexes to exact table parity and proceeds to gateway auth.
  • Search-content equivalence: 58 real queries against the rebuilt index vs an independent oracle (the Python familiar_en analyzer chain re-applied over the DB) — exact result-set match on every query; the original index was itself stale (33 orphaned turn docs, 13 live facts missing), so the rebuild is strictly more correct.
  • Flake fix: 20/20 single-threaded + 10/10 under full 8-core CPU saturation.

Notes

🤖 Generated with Claude Code

CorVous and others added 4 commits July 16, 2026 23:06
Opening a store over a foreign/incompatible tantivy index (e.g. the retired
Python impl's index with a different schema) makes open_or_recreate wipe it to
a fresh empty index. rebuild_fts had no call sites, so all indexed history was
silently lost and never repopulated.

open_dir now surfaces whether it took the recreate path; HistoryStore::open
repopulates any index that was recreated or is empty while its source table
still holds rows, from the DB, synchronously (small corpus), logging counts.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
create_embedder's error (e.g. fastembed without the local-embed extra) was
swallowed by .unwrap_or(None) at the async_main call site, so startup proceeded
to open and wipe the FTS and only died later in create_projectors under a
misleading Discord-token hint.

Extract a testable resolve_embedder seam and call it in run_inner BEFORE
Familiar::load_from_disk opens the store: on error, log the real message
(which names the fix) and exit 1. The embedder is threaded into async_main.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…o 30s

recv_wake's 1s timeout is only a hang-guard (every caller .expects Some; none
asserts None), but the runner does several spawn_blocking DB round-trips before
publishing the wake, which can outlast 1s on a loaded current-thread test
runtime — turning the guard into a false failure (~67% under CPU load). Raise
the bound to 30s; a genuinely-stuck test still fails, the race is gone.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@CassidyPrather
CassidyPrather merged commit 1e978dd into main Jul 17, 2026
2 checks passed
@CassidyPrather
CassidyPrather deleted the pr/rust-migration-safety branch July 17, 2026 05:34
CassidyPrather pushed a commit that referenced this pull request Jul 18, 2026
Union of #210's migration-safety work with this branch's issue fixes.
Only textual conflict was the run.rs test-import list; full suite,
clippy -D warnings, and fmt verified on the merged tree.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_017hGpXhasxotWvHbXVY3VC3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants