fix(fts): scope memories_fts update triggers to indexed columns#166
Open
J0nd1sk wants to merge 1 commit into
Open
fix(fts): scope memories_fts update triggers to indexed columns#166J0nd1sk wants to merge 1 commit into
J0nd1sk wants to merge 1 commit into
Conversation
J0nd1sk
changed the title
`memories_fts` is external-content FTS5 maintained by two `AFTER UPDATE ON memories` triggers with no column scope. Every metadata-only write fired the delete+reinsert pair, so each `cmd_memory_search` (via `_retrieval_practice_boost` bumping `confidence`/`recalled_count` on every hit) eroded the inverted index. Searches progressively lost rows; `docsize` bled out. `q_value`, `trust_score`, `ewc_importance` and other metadata writes did the same. Scope both update triggers to `AFTER UPDATE OF content, category, tags, indexed, retired_at`. Metadata churn no longer touches FTS; `indexed` and `retired_at` stay in scope so promotion and retire still maintain the index. - `init_schema.sql`: scoped triggers - migration `083`: drop and recreate scoped on existing DBs, plus a one-time rebuild to repair indexes already corrupted in the field - `mcp_server` startup: rescope legacy triggers on installs that never run migrations (pip wheels do not ship `db/migrations/`) - `_ensure_fts_index_consistent` now returns `False` on a healthy index instead of `None`, matching its docstring (surfaced by the new tests) The search-quality bench was itself degraded by this bug; the fix raises overall `p_at_1` from 0.60 to 0.80 and `recall@10` from 0.51 to 0.85, so the baseline is refreshed.
J0nd1sk
force-pushed
the
fix/152-fts-trigger-scope
branch
from
2c1d8f1 to
c5a2b1c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #152.
memories_fts(external-content FTS5) is maintained by two unscopedAFTER UPDATE ON memoriestriggers, so every metadata-only write fires the delete+reinsert pair.cmd_memory_searchcalls_retrieval_practice_booston each hit (bumpingconfidence/recalled_count), so every search erodes the inverted index:docsizebleeds out and search progressively returns fewer rows.q_value,trust_score,ewc_importanceand other metadata writes do the same.Fix
Scope both update triggers to
AFTER UPDATE OF content, category, tags, indexed, retired_at. Metadata churn no longer touches FTS;indexedandretired_atstay in scope so promotion and retire still maintain the index. A dedicatedAFTER UPDATE OF retired_atpurge trigger was tried and rejected in migration 048 due to FTS5 statement-level batching; this keeps the single delete leg and only narrows its trigger columns, so retire firing is unchanged.Reaches every install shape:
init_schema.sqlfor fresh DBs083drops and recreates the scoped triggers on migrated DBs, plus a one-time rebuild to repair indexes already corrupted in the fieldmcp_serverfor pip-wheel installs that do not shipdb/migrations/Also fixes a latent bug surfaced by the new tests:
_ensure_fts_index_consistentreturnedNoneon a healthy index instead ofFalse(its documented contract).Impact
The repo's own search-quality bench was degraded by this bug. The fix raises overall
p_at_1from 0.60 to 0.80 andrecall@10from 0.51 to 0.85 (baseline refreshed).Tests
tests/test_memories_fts_trigger_scope.py: metadata updates no longer erodedocsize(raw SQL and via the real_retrieval_practice_boost); content edit, retire, and delete still maintained. Full suite: 2519 passed; the 6 remaining failures pre-exist onmainin my environment and are unrelated.