fix(hts): create the concepts_search_fts FTS5 table it uses (#274)#280
Closed
mauripunzueta wants to merge 1 commit into
Closed
fix(hts): create the concepts_search_fts FTS5 table it uses (#274)#280mauripunzueta wants to merge 1 commit into
mauripunzueta wants to merge 1 commit into
Conversation
`crates/hts/src/backends/sqlite/value_set.rs` reads from and writes to a
`concepts_search_fts` FTS5 table (the typeahead ranker
`fts_candidates_ranked_for_system` and its populate path), but the table's
`CREATE VIRTUAL TABLE` existed nowhere in the repository. The first text-filtered
expansion of any system therefore fails hard:
no such table: concepts_search_fts
surfaced as an OperationOutcome exception, and it fails the test
`value_set_ops::expand_is_a_filter_with_text_filter_still_nests` on main.
This is a semantic merge conflict: PR #194 added the code that uses the table
(without the DDL) and was green because nothing in its tests reached the build
path; PR #233 added the test that does reach it and was green because its CI ran
against a base predating #194. Both merged, and the combination breaks.
Add the table next to its siblings in `schema.rs`, and clear it at startup in the
same two blocks that reset `concepts_fts` / `concepts_word_fts`:
* `term` is the only tokenised column — it carries the concept's preferred display
AND every designation value, which is what lets a query match a concept by any
synonym and rank the hits with bm25.
* `code` is retrieved (SELECT / GROUP BY) and `system_id` is a post-filter, so
both are UNINDEXED, exactly as the INSERT/SELECT/MATCH statements in
value_set.rs use them.
* Same `trigram case_sensitive 0` tokenizer as `concepts_fts`, which is the
fallback when this index is empty, so substring matching is consistent between
the two.
Verified against a real SQLite (3.45): the DDL applies, both populate INSERTs run,
the `EXISTS` populated-probe returns true, and a trigram MATCH on a designation
value returns the owning concept's code ranked by bm25.
Contributor
Author
|
Superseded by #273, which merged the same fix (create |
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
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.
Fixes #274.
The bug
crates/hts/src/backends/sqlite/value_set.rsreads from and writes to aconcepts_search_ftsFTS5 table (the typeahead rankerfts_candidates_ranked_for_systemand its populate pathpopulate_concepts_search_fts_for_system), but the table'sCREATE VIRTUAL TABLEexisted nowhere in the repository:So the first text-filtered expansion of any system fails hard with
no such table: concepts_search_fts, surfaced as an OperationOutcome exception.maincurrently failsvalue_set_ops::expand_is_a_filter_with_text_filter_still_nestson exactly this.Why CI didn't catch it — semantic merge conflict
expand_is_a_filter_with_text_filter_still_nests, which does reach itBoth merged green; the combination on
mainis broken.The fix
Add the table next to its siblings in
schema.rs, and clear it at startup in the same two blocks that resetconcepts_fts/concepts_word_fts.The shape is derived from how
value_set.rsuses it, not guessed:termis the only tokenised column — the INSERTs fill it from both the concept'sdisplayand every designationvalue, which is what lets a query match a concept by any synonym and rank withbm25.codeis retrieved (SELECT code … GROUP BY code) andsystem_idis a post-filter (WHERE system_id = ?), so both areUNINDEXED.trigram case_sensitive 0tokenizer asconcepts_fts, its empty-index fallback, so substring matching is consistent between the two.CREATE VIRTUAL TABLE IF NOT EXISTS concepts_search_fts USING fts5(system_id UNINDEXED, code UNINDEXED, term, tokenize='trigram case_sensitive 0');Verification
Against a real SQLite (3.45): the DDL applies, both populate INSERTs run, the
EXISTSpopulated-probe returns true, and a trigramMATCHon a designation value returns the owning concept's code ranked bybm25.@sandhums (author of #194) — please sanity-check the column/tokenizer choice against your intent for the search ranker; the behaviour is inferred from the queries, but you'd know if
termwas meant to carry anything else.Found while rebasing #261 (issue #200) onto main; #261's CI is blocked on this because GitHub tests the PR merged into main.