h-dv/code-index

Watch

Fork 0

simplify: replace ~160-line hand-rolled FTS5 pre-validator with run-then-fallback #21

New issue

Closed

opened 2026-07-07 13:50:03 +02:00 by buildagent · 1 comment

buildagent commented

2026-07-07 13:50:03 +02:00

Member

Summary

fts5_safe_query decides pass-through-vs-phrase-wrap using a ~160-line hand-rolled FTS5 grammar pre-validator (looks_like_fts5_syntax, parens_look_like_fts5_grouping, preceded_by_word, byte-walking depth/quote/NEAR() state) plus a 245-line test module. This replicates validation SQLite already performs and is necessarily an approximation of FTS5's real grammar, so it carries permanent false-positive/negative risk against the actual parser.

Where

crates/daemon/src/local_index.rs:1873-2035 (validator) + 2088-2333 (tests). Single production caller: search_text (local_index.rs:1325).

Severity

Low — a JUDGMENT CALL, not a defect. The current code is correct and heavily tested. This is a simplification/maintainability proposal, filed for tracking, not a bug.

Proposal

Execute the raw user query as MATCH first; only on a SQLite FTS5 parse error, retry phrase-wrapped (escape embedded quotes + wrap in "…"). SQLite is the authoritative source of truth for its own grammar, so this preserves the exact "pass valid syntax through / wrap invalid as a phrase" intent while collapsing the hand-rolled parser and its test surface.

Trade-off to weigh before doing this: a run-then-fallback issues up to two queries for invalid input, and turns a parse error into a caught-and-retried path (must ensure the first failed statement doesn't leave the connection in a bad state — it doesn't for read-only MATCH, but verify). Keep a small regression suite mapping representative inputs → expected wrapped/passed form.

Surfaced by the v0.5.8 ultradeep review (architecture dimension, CONFIRMED as a simplification opportunity).

Acceptance

fts5_safe_query's behavior (which inputs pass through vs get phrase-wrapped) is preserved for the existing test corpus.
Net reduction in hand-rolled parsing code; no new FTS5 crash surface for adversarial queries.

## Summary `fts5_safe_query` decides pass-through-vs-phrase-wrap using a ~160-line hand-rolled FTS5 grammar pre-validator (`looks_like_fts5_syntax`, `parens_look_like_fts5_grouping`, `preceded_by_word`, byte-walking depth/quote/NEAR() state) plus a 245-line test module. This replicates validation SQLite already performs and is necessarily an approximation of FTS5's real grammar, so it carries permanent false-positive/negative risk against the actual parser. ## Where `crates/daemon/src/local_index.rs:1873-2035` (validator) + `2088-2333` (tests). Single production caller: `search_text` (`local_index.rs:1325`). ## Severity Low — a JUDGMENT CALL, not a defect. The current code is correct and heavily tested. This is a simplification/maintainability proposal, filed for tracking, not a bug. ## Proposal Execute the raw user query as `MATCH` first; only on a SQLite FTS5 parse error, retry phrase-wrapped (escape embedded quotes + wrap in `"…"`). SQLite is the authoritative source of truth for its own grammar, so this preserves the exact "pass valid syntax through / wrap invalid as a phrase" intent while collapsing the hand-rolled parser and its test surface. Trade-off to weigh before doing this: a run-then-fallback issues up to two queries for invalid input, and turns a parse error into a caught-and-retried path (must ensure the first failed statement doesn't leave the connection in a bad state — it doesn't for read-only MATCH, but verify). Keep a small regression suite mapping representative inputs → expected wrapped/passed form. Surfaced by the v0.5.8 ultradeep review (architecture dimension, CONFIRMED as a simplification opportunity). ## Acceptance - `fts5_safe_query`'s behavior (which inputs pass through vs get phrase-wrapped) is preserved for the existing test corpus. - Net reduction in hand-rolled parsing code; no new FTS5 crash surface for adversarial queries.

buildagent added this to the v0.5.9 milestone

2026-07-07 13:50:17 +02:00

buildagent commented

2026-07-07 16:13:38 +02:00

Author

Member

Implemented on fix/ultradeep-review-findings (commit 820fd9d).

The ~160-line hand-rolled validator (looks_like_fts5_syntax + parens_look_like_fts5_grouping + preceded_by_word) is replaced by a cheap structural-operator scan plus SQLite as the authority on its own grammar:

plain text (no operator marker) → phrase-wrapped directly, no probe;
operator-bearing text → run the raw query; the MATCH value is a bind parameter so FTS5 parses it on first step — a syntax error means "not a real query" → fall back to the phrase form.

The probe runs only for operator-bearing input, so plain searches (the overwhelming majority) issue no second query. A failed read-only MATCH leaves the connection usable (verified).

Behavior preserved (acceptance): a new full_corpus_matches_legacy_validator_decisions test runs all 60 inputs from the retired validator's corpus against a real trigram files_fts and asserts identical pass-through-vs-phrase outcomes. I empirically confirmed SQLite reproduces the old validator's decision on every corpus case; the one input where a naive run-raw-first would diverge (a*b) is kept as a literal phrase by a 5-line prefix-token guard in has_fts5_operator.

Net +209/−420 in local_index.rs. No new FTS5 crash surface — malformed operator queries fall back via SQLite's own verdict. Closing.

Implemented on `fix/ultradeep-review-findings` (commit `820fd9d`). The ~160-line hand-rolled validator (`looks_like_fts5_syntax` + `parens_look_like_fts5_grouping` + `preceded_by_word`) is replaced by a cheap structural-operator scan plus **SQLite as the authority on its own grammar**: - plain text (no operator marker) → phrase-wrapped directly, **no probe**; - operator-bearing text → run the raw query; the MATCH value is a bind parameter so FTS5 parses it on first `step` — a syntax error means "not a real query" → fall back to the phrase form. The probe runs **only** for operator-bearing input, so plain searches (the overwhelming majority) issue no second query. A failed read-only MATCH leaves the connection usable (verified). **Behavior preserved (acceptance):** a new `full_corpus_matches_legacy_validator_decisions` test runs all 60 inputs from the retired validator's corpus against a real trigram `files_fts` and asserts identical pass-through-vs-phrase outcomes. I empirically confirmed SQLite reproduces the old validator's decision on **every** corpus case; the one input where a naive run-raw-first would diverge (`a*b`) is kept as a literal phrase by a 5-line prefix-token guard in `has_fts5_operator`. Net **+209/−420** in `local_index.rs`. No new FTS5 crash surface — malformed operator queries fall back via SQLite's own verdict. Closing.

buildagent closed this issue

2026-07-07 16:14:10 +02:00

dhoyer referenced this issue from a commit

2026-07-07 16:31:03 +02:00

simplify(daemon): FTS5 query via operator-scan + SQLite probe, not hand-rolled parser (#21)

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

h-dv/code-index#21

No description provided.

Rows
Columns