[getappz]

Closes #341.

What

Converts all four fts5 indexes from standalone tables kept in sync by hand to
external-content tables driven by triggers, following the
observations_fts pattern in src/memory/schema.rs.

index	crate	manual write sites removed
store_docs_fts	agentflare-store	3
store_docs_fts	flare-docs	3
skills_fts	skill-registry	2
tools_fts	gateway-registry	2

Why the store_docs_fts triggers are guarded rather than straight mirrors

Soft-deleted documents stay out of the index — that is what the manual code
did, and it is what stops #339's future purge from being the only thing
keeping the index from growing without bound.

That constraint is load-bearing, not cosmetic. On an external-content table a
'delete' command naming a row that is not in the index corrupts it. A
resurrecting upsert (doc_upsert on a soft-deleted path) is an UPDATE whose
old row was already unindexed, so an unguarded AFTER UPDATE would issue
exactly that corrupting delete on every resurrection. Hence
WHERE old.deleted_at IS NULL / WHERE new.deleted_at IS NULL on each arm.
resurrecting_a_soft_deleted_document_indexes_it_exactly_once asserts this
via fts5's own 'integrity-check', which is what actually fails on the
unguarded version.

AFTER UPDATE is scoped to the indexed columns (OF content, deleted_at;
OF name, description, body, tags, neg_text). doc_upsert_with_opts follows
its content write with up to nine single-column UPDATEs, and skill-registry
writes last_used_at + bandit counters on every skill load — a bare
AFTER UPDATE would re-tokenize whole documents for each.

Upgrade path

• agentflare-store has rusqlite_migration, so it is a new M::up.
• The two registries do not. open_db detects a legacy standalone index
  (sqlite_master.sql NOT LIKE '%content=%'), drops it, and refills from the
  base table in the same open — waiting for the next rebuild would leave
  skill_search / gateway tool search empty in between.

Repair path

Adds Store::doc_fts_rebuild. store_documents has a TEXT primary key, so
its rowids are implicit and VACUUM may renumber them — which would desync
this index and doc_search's rowid JOIN (that coupling predates this PR).
Anything that VACUUMs, including the eviction work in #339, must call it
afterwards. It repopulates explicitly rather than using fts5 'rebuild',
which would index the soft-deleted rows the triggers deliberately keep out.

A trap worth flagging for reviewers

SELECT count(*) on an external-content fts5 table is answered from the
content table, so it reports every base-table row whether or not it is
indexed. Three pre-existing assertions named "fts stays in sync" were
measuring nothing once converted; they now go through MATCH, which is
index-driven, and assert the negative case too (a replaced/deleted row must
stop matching). This is also why the first draft of the real-database test
"failed" at 10858 vs 10857.

Verification

• cargo test --workspace — green. One unrelated flake,
  flare-git-core worktree::run_output_timeout_kills_the_child_not_just_abandons_it,
  which passes in isolation on this branch and on clean master and passed on
  a prior full run; it is a process-kill timing test under parallel load.
• cargo clippy --workspace --all-targets --all-features — no new findings
  (the 21 pre-existing local-toolchain findings from feat: add supervisor auto-dispatch loop (item #408) #376 are unchanged).
• Migrated a real 9.2 MB / 10,858-document ~/.agentflare/flare-docs.db
  (crates/flare-docs/tests/real_db_migration.rs, skipped when no such file
  exists): applies cleanly, passes fts5 'integrity-check', every sampled
  live document stays matchable and the soft-deleted one does not.

Not included

The issue also suggests surfacing PRAGMA integrity_check and an FTS rebuild
escape hatch through agentflare doctor. doctor today is entirely about
agent config wiring (hosts, rules) with no database surface, so that is a
separate change; this PR ships the repair primitive it would call.

Summary by CodeRabbit

• Improvements
  • Full-text search indexes for documents, tools, and skills now stay automatically synchronized via trigger-driven external-content indexing.
  • Soft-deleted documents/tools/skills remain excluded from search, even after edits or restores.
  • Added a document search index rebuild operation to recover searchability after intentional inconsistencies.
  • Existing databases are automatically upgraded to the trigger-driven FTS format during open.
  • Bulk write and rebuild paths were updated to avoid manual index maintenance.
• Tests
  • Added/updated integration and real-database migration tests to verify search matchability and legacy upgrades.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 337588f6-d63c-405d-9cb8-8785c50421db

📥 Commits

Reviewing files that changed from the base of the PR and between 9805b52 and e7b972b.

📒 Files selected for processing (4)

• crates/agentflare-store/src/documents.rs
• crates/flare-docs/tests/real_db_migration.rs
• crates/gateway-registry/src/db.rs
• crates/skill-registry/src/db.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• crates/agentflare-store/src/documents.rs
• crates/gateway-registry/src/db.rs
• crates/skill-registry/src/db.rs

---

📝 Walkthrough

Walkthrough

The change converts document, gateway, and skill FTS5 indexes to external-content tables synchronized by SQLite triggers. Manual FTS writes are removed, legacy indexes are migrated on open, and rebuild and integration tests validate live-row searchability and soft-delete behavior.

Changes

External-content FTS migration

Layer / File(s)	Summary
FTS schemas and migrations crates/agentflare-store/src/migrations.rs	Adds external-content store_docs_fts triggers, rebuild SQL, migration sequencing, and migration tests covering live rows, soft deletes, schema shape, and trigger updates.
Document write paths crates/agentflare-store/src/documents.rs	Adds Store::doc_fts_rebuild(), removes manual FTS synchronization from upserts and deletes, and updates tests for soft-delete, resurrection, edits, and rebuild behavior.
Docs cache batch writes crates/flare-docs/src/store.rs, crates/flare-docs/tests/real_db_migration.rs	Removes manual FTS writes from reconciliation and batch upserts, and adds real-database migration coverage for live and soft-deleted documents.
Gateway FTS conversion crates/gateway-registry/src/db.rs	Converts legacy tools_fts databases on open, adds trigger-driven synchronization, changes rebuild logic to write only tools, and updates matchability tests.
Skill FTS conversion crates/skill-registry/src/db.rs	Converts legacy skills_fts databases on open, adds trigger-driven synchronization, changes rebuild logic to write only skills, and updates deletion and rebuild tests.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Store
  participant store_documents
  participant store_docs_fts
  Store->>store_documents: Insert or update indexed document
  store_documents->>store_docs_fts: Trigger updates external-content index
  Store->>store_documents: Soft-delete document
  store_documents->>store_docs_fts: Trigger removes searchable entry
  Store->>store_docs_fts: Execute rebuild SQL when requested

Loading

Possibly related PRs

• getappz/agentflare#323 — Both modify document upsert behavior in documents.rs.
• getappz/agentflare#337 — Both involve document soft-deletion and FTS behavior.
• getappz/agentflare#346 — Both modify the docs batch write path and its FTS maintenance.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed, but it misses the required Summary, Test plan, and Notes for reviewers sections from the template.	Add the required headings: ## Summary, ## Test plan with the three checkboxes, and ## Notes for reviewers covering risks and backwards compatibility.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: moving FTS5 indexes to trigger-driven synchronization instead of manual writes.
Linked Issues check	✅ Passed	The changes match #341: all four FTS indexes become external-content tables with triggers, legacy indexes are migrated, and rebuild support is added.
Out of Scope Changes check	✅ Passed	All visible changes support the FTS trigger migration, tests, legacy conversion, or repair path; no unrelated code appears introduced.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch task/341

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

crates/agentflare-store/src/migrations.rs (1)

119-123: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Consider wrapping the rebuild in a transaction.

execute_batch autocommits each statement, so a failure after 'delete-all' leaves the index empty until someone reruns the rebuild. BEGIN; ... COMMIT; makes it all-or-nothing. (Note doc_fts_rebuild is also called on a connection that may already be in a transaction — if so, keep as-is.)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/agentflare-store/src/migrations.rs` around lines 119 - 123, Update
FTS_REBUILD_SQL to wrap the delete-all and repopulation statements in
BEGIN/COMMIT so rebuilding is atomic. Ensure doc_fts_rebuild preserves the
existing behavior when called on a connection already inside a transaction,
avoiding nested transaction errors.

crates/agentflare-store/src/documents.rs (1)

813-837: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Assertion doesn't actually test what the comment claims.

fts_hits == 1 holds whether or not the nine optional-column UPDATEs fire the trigger — an unscoped AFTER UPDATE would still leave exactly one entry. To pin the UPDATE OF content, deleted_at scoping, assert on the trigger definition (e.g. that sqlite_master.sql for store_docs_fts_au contains UPDATE OF content, deleted_at), or do a second doc_upsert_with_opts that changes only optional fields and confirm the index is untouched.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/agentflare-store/src/documents.rs` around lines 813 - 837, The test
optional_field_updates_do_not_disturb_the_index does not verify trigger scoping
because a single FTS hit passes even with an unscoped trigger. Strengthen it by
inspecting sqlite_master for the store_docs_fts_au trigger and asserting its SQL
contains UPDATE OF content, deleted_at, or perform a second optional-only
doc_upsert_with_opts and verify the index remains unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/gateway-registry/src/db.rs`:
- Around line 38-62: Update apply_schema so the legacy tools_fts drop, SCHEMA
recreation, and backfill INSERT execute within one transaction, committing only
after all steps succeed and rolling back on failure. Preserve the existing
legacy detection and avoid skipping recovery when the migration cannot complete.

---

Nitpick comments:
In `@crates/agentflare-store/src/documents.rs`:
- Around line 813-837: The test optional_field_updates_do_not_disturb_the_index
does not verify trigger scoping because a single FTS hit passes even with an
unscoped trigger. Strengthen it by inspecting sqlite_master for the
store_docs_fts_au trigger and asserting its SQL contains UPDATE OF content,
deleted_at, or perform a second optional-only doc_upsert_with_opts and verify
the index remains unchanged.

In `@crates/agentflare-store/src/migrations.rs`:
- Around line 119-123: Update FTS_REBUILD_SQL to wrap the delete-all and
repopulation statements in BEGIN/COMMIT so rebuilding is atomic. Ensure
doc_fts_rebuild preserves the existing behavior when called on a connection
already inside a transaction, avoiding nested transaction errors.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8387028f-cab9-4f77-b188-4ad47d8175a5

📥 Commits

Reviewing files that changed from the base of the PR and between 5dfb4a5 and 9805b52.

📒 Files selected for processing (6)

• crates/agentflare-store/src/documents.rs
• crates/agentflare-store/src/migrations.rs
• crates/flare-docs/src/store.rs
• crates/flare-docs/tests/real_db_migration.rs
• crates/gateway-registry/src/db.rs
• crates/skill-registry/src/db.rs