Repository navigation
fix(storage): repair skills_fts indexing on legacy databases - #2504
zichen0116 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
🤖 Open Code ReviewTarget: PR #2504 🔍 OpenCodeReview found 2 issue(s) in this PR. 1.
|
|
Description
Fixes #2500: on databases created before the
skills_ftsFTS5 sync triggers landed in001-initial.sql, theskillstable has noskills_fts_ai/_ad/_autriggers —001never re-runs on existing databases, and unlike policies FTS (which got repair migration009-policies-fts.sql) no later migration ever added them. Result: everyINSERTintoskills(including viaimportBundle()) leavesskills_ftsempty, so the keyword channel in skill retrieval (WHERE skills_fts MATCH ...incore/retrieval/tier1-skill.ts) never matches — exactly the reported symptom (policies/world-model FTS populated, skills FTS at 0).Implementation approach:
apps/memos-local-plugin/core/storage/migrations/020-skills-fts.sql: creates theskills_ftsvirtual table and the three sync triggers withIF NOT EXISTS, then backfillsSELECT id, name, invocation_guide FROM skills WHERE id NOT IN (SELECT skill_id FROM skills_fts)— same shape and repair pattern as009.apps/memos-local-plugin/core/storage/migrator.ts: guard020withtableExists(db, "skills")so partial-schema test harnesses don't fail boot (same guard style as009/012/018).The migration is a no-op on fresh databases (001 already created everything there).
Related Issue (Required): Fixes #2500
Type of change
How Has This Been Tested?
tests/unit/storage/skills-fts-migration.test.tssimulates a legacy DB (drops the triggers, empties FTS), reproduces the empty index, then applies the migration SQL: triggers restored, existing rows backfilled, trigramMATCHworks, replay is idempotent, and subsequent insert/update/delete stay in sync; plus a fresh-DB no-op case.npx vitest run tests/unit/storage/skills-fts-migration.test.ts tests/unit/storage/migrator.test.ts tests/unit/storage/fts-keyword.test.ts tests/unit/storage/keyword.test.ts→ 36/36 passed;npm run lint(tsc --noEmit) clean.Checklist