fix(fts): match_phrase slop admits transpositions at Lucene cost 2 (#830)

[xerj-org]

What

match_phrase slop now follows Lucene SloppyPhraseMatcher move-distance semantics: a transposed (reordered) pair costs 2, so {"match_phrase":{"t":{"query":"quick brown","slop":2}}} matches a document reading brown quick — previously it returned zero hits at any slop. match_phrase_prefix and the multi_match phrase/phrase_prefix types gain the same behavior (they share the evaluator). Closes the behavioral half of #830; the doc-accuracy half was the a7ff459 stopgap (PR #831), whose "in-order only" comments this PR now replaces with the real semantics.

Root cause

Both sloppy-phrase evaluators — segment (xerj_fts::search::phrase_positions_match) and the memtable/stored-scan walk hand-mirrored on it (phrase_walk, xerj-engine index.rs) — were in-order greedy gap walks: anchor on the first term, match each later term at the earliest position strictly after the previous match, sum the gaps. A reordering can never satisfy "strictly after in query order", so transpositions never matched. Lucene's semantics (its class javadoc): for query "a b"~2, document x a b a y matches once as a b (distance 0) and once as b a (distance 2).

Fix

Slop is an edit distance in single-position term moves: pick one document position p_i per phrase term i; the distance is the span of the adjusted positions, max(p_i - i) - min(p_i - i); match when some choice keeps it <= slop and the chosen positions are distinct (one document token cannot fill two phrase slots — Lucene's repeat rule; "a a" does not match a doc holding one a). That is the distance Lucene's nextMatch computes (matchLength = end - pp.position over a pq of position - offset, SloppyPhraseMatcher.java:205-237); algorithm adapted, not code (Apache-2.0, cited in the comments). Out-of-order pairs cost >= 2 by construction, so transpositions appear at slop 2 and never at slop 1.

Deciding that predicate is the part the first round of this PR got wrong (see below). It is now a window sweep with an exact feasibility test: every adjusted position is a candidate window floor lo, and the window [lo, lo + slop] is accepted iff it admits a system of distinct representatives — one position per slot, no position used twice — decided by bipartite matching over each slot's admissible run. The sweep is complete: any assignment with span <= slop lies entirely inside the window floored at its own minimum adjusted position. slop == 0 keeps the exact-adjacency arm untouched.

Both arms call the one evaluator: phrase_walk materialises per-query-slot token-index position lists (the prefix slot folds its expansions into one list) and calls the same phrase_positions_match the segment positional clause uses, so slop cannot be evaluated differently on the two sides of a flush (#218/#222/#230 regression class). What the two arms do not share is a position model — the memtable arm re-analyses stored text with the standard analyzer while the segment arm reads indexed positions — so this is one evaluator, not flush-invariance "by construction"; a custom analyzer can still hand the arms different position lists.

• engine/crates/xerj-fts/src/search.rs — new slop>0 arm (window sweep + SDR); slop==0 arm unchanged; now pub
• engine/crates/xerj-engine/src/index.rs — phrase_walk delegates to it
• engine/crates/xerj-fts/src/lib.rs — export
• engine/crates/xerj-query/src/{ast,planner}.rs — a7ff459's stopgap comments replaced with the actual semantics
• CHANGELOG.md — Unreleased/Fixed entry

Review round 2 — repeated terms (9a93a36)

Review blocked the first implementation and was right. The minimal-window sweep it used advanced the pointer at the minimum adjusted position whenever a window was rejected, including when the rejection was a repeat collision. The colliding slot and the minimum slot are usually different, so on a collision the sweep advanced a non-colliding pointer; when that slot's term occurs once, its list exhausts and the sweep returns false without ever trying the assignment that separates the repeats. Every sloppy phrase with a repeated term could lose matches it had before #830, at any slop:

query	document	pre-#830	first round	now
"i said no no"~1,2,3,5,10	i said uh no no	true	false	true
"a b b"~1,2,4	a x b b	true	false	true
"no no no"~1,2,3	no way no no	true	false	true

ES/Lucene match all three ("i said no no"~1 has adjusted positions 0,0,1,1 — span 1). Lucene avoids the hole because advanceRpts (:317) advances the colliding repeat rather than the current minimum; the first round cited advanceRpts and implemented a different rule. Both arms delegate, so the regression was flush-invariant and invariantly wrong. The fix decides each window exactly (the SDR test above) instead of guessing which pointer to move.

Cost is unchanged in order: the pointer families only move forward (linear in total positions), and the matching runs only on windows where slots actually collide. Measured on 200k-position lists, worst case with no match: 2.4ms for two distinct terms, 7.8ms for a 3× repeated term — against 5.2ms for the first round and 4.0s for the pre-#830 quadratic walk.

Claims corrected. The first round's PR body, commit message, CHANGELOG entry and doc comment all said "every previously-matching doc still matches — the change is strictly additive". The telescoping argument behind it is sound for a fixed assignment but says nothing about the predicate, and with the search over assignments incomplete the claim was false. It is true of the current head, and now stated as something checked rather than argued (the exhaustive test below). The CHANGELOG no longer says "flush-invariant by construction", and the doc comment no longer says "distinct terms never share a document position" — SynonymFilter deliberately co-locates synonyms, so enforcing distinctness pairwise over all slots is stricter than Lucene on a synonym field; that divergence is now documented as deliberate and conservative (it can only withhold a match, and it is what the pre-#830 walk did) instead of asserted away.

Test proof

Fails on main:

New engine/crates/xerj-engine/tests/match_phrase_slop_transposition.rs — match_phrase, match_phrase_prefix, and multi_match phrase over docs adj="quick brown fox", rev="brown quick fox", gap="quick lazy brown": slop 0 → {adj}, slop 1 → {adj, gap} (transposition not admitted below 2), slop 2/3 → {adj, gap, rev}; every case asserted pre-flush and post-flush. Run against the unfixed code, all three fail exactly at the slop-2 rows:

assertion `left == right` failed: slop 2: transposition matches: PRE-flush (memtable) hit set
  left: {"adj", "gap"}
 right: {"adj", "gap", "rev"}

Fails on the first round of this PR (verified by reverting only the algorithm and re-running):

• sloppy_phrase_repeated_term_keeps_in_order_matches (xerj-fts) — the three rows in the table above, plus the negative that still binds: three no slots cannot be filled by two document tokens at slop 10.
• sloppy_phrase_agrees_with_brute_force_and_keeps_pre_830_hits (xerj-fts) — every document of length <= 5 and every phrase of length <= 3 over a 3-symbol alphabet at slop 0..=3 (56k cases), cross-checked against a brute-force reference and against a copy of the pre-es-compat: match_phrase slop is in-order only — transpositions never match (diverges from Lucene SloppyPhraseMatcher) #830 walk, which must never match something the evaluator rejects. That is the "strictly additive" claim as a test rather than a sentence.
• match_phrase_slop_repeated_term_still_matches, match_phrase_slop_all_slots_repeated, multi_match_phrase_slop_repeated_term_still_matches (xerj-engine) — the same three cases end to end, pre- and post-flush.

Out of tree, at the review's own standard: the four functions extracted verbatim from search.rs and compiled standalone against the brute-force oracle — 2,360,880 exhaustive cases (docs <= 7 tokens, phrases <= 4, slop 0..=5) and 500,000 randomised sparse-position cases: 0 disagreements with brute force, 0 pre-#830 matches lost.

Also corrected from the review's non-blocking notes: the match_phrase_slop_transposition.rs module doc claimed every case is asserted against the segment positional arm post-flush, but the planner keeps single-field slop > 0 phrases on the stored scan, so those rows re-run phrase_walk after the flush (the segment arm is covered by the xerj-fts unit tests and the multi_match case); and the [b, a] assertion in sloppy_phrase_transposition_semantics is satisfied by an in-order pair, which the comment now says outright.

Gates

cargo fmt --all --check clean; cargo clippy --release -p xerj-fts -p xerj-engine --all-targets -- -D warnings clean; xerj-fts full suite (90 unit + 1 doc-test) green; match_phrase_slop_transposition 6/6 and multi_match_phrase_positions 12/12 green; xerj-engine --lib phrase projection tests 6/6 green.

Closes #830.

[xerj-org]

Review response — both blocking findings addressed (9a93a36)

Finding 1 (wrong results on repeated terms): confirmed and fixed. I reproduced it first, the same way the review did — both versions of phrase_positions_match extracted verbatim into one file and compiled with rustc:

[i,said,no,no] vs 'i said uh no no'    slop= 1  old=true  new=false
[a,b,b] vs 'a x b b'                   slop= 2  old=true  new=false
[no,no,no] vs 'no way no no'           slop= 3  old=true  new=false

The mechanism is exactly as described: on a repeat collision the sweep advanced the pointer at the minimum adjusted position, which is usually a non-colliding slot, so a single-occurrence slot exhausted before the assignment that separates the repeats was ever tried.

The fix stops guessing which pointer to advance and decides each window instead. Sweep every candidate window floor lo (each adjusted position is one, via forward-only cursors); accept [lo, lo + slop] iff it admits a system of distinct representatives — one position per slot, none used twice — decided by bipartite matching over each slot's admissible run, truncated to n candidates (a slot with n+ candidates can always be re-pointed at a free one, so the truncation is SDR-preserving). The sweep is complete: any assignment with span <= slop sits entirely inside the window floored at its own minimum adjusted position. slop == 0 untouched.

Evidence, at the standard the review set — the four functions extracted verbatim from the new search.rs and compiled standalone against a brute-force oracle:

exhaustive checked=2360880 mismatch_vs_brute=0 pre830_matches_lost=0
sparse fuzz mismatch_vs_brute=0 pre830_matches_lost=0

(docs <= 7 tokens over a 3-symbol alphabet, phrases <= 4, slop 0..=5; plus 500k randomised sparse-position cases that no small document can express. pre830_matches_lost counts documents main matched and the new code does not — zero.)

Regression tests, all three of the review's cases plus the negative that still binds, and each verified to fail when only the algorithm is reverted to the previous head:

• sloppy_phrase_repeated_term_keeps_in_order_matches (xerj-fts) — the three cases; fails at "i said no no"~1 must match 'i said uh no no'.
• sloppy_phrase_agrees_with_brute_force_and_keeps_pre_830_hits (xerj-fts) — every doc of length <= 5 × every phrase of length <= 3 over a 3-symbol alphabet at slop 0..=3, against brute force and a copy of the pre-es-compat: match_phrase slop is in-order only — transpositions never match (diverges from Lucene SloppyPhraseMatcher) #830 walk; fails at doc [0, 1, 1] query [1, 0, 1] slop 2: evaluator disagrees with brute force.
• match_phrase_slop_repeated_term_still_matches, match_phrase_slop_all_slots_repeated, multi_match_phrase_slop_repeated_term_still_matches (xerj-engine) — end to end, pre- and post-flush.

Perf on 200k-position lists, no-match worst case: 2.4ms (2 distinct terms) and 7.8ms (3× repeated term), vs 5.2ms for the previous head and 4.0s for the pre-#830 walk. Same order, and the matching only runs on windows where slots actually collide.

Finding 2 (false safety claim): corrected everywhere it appeared. The commit message, PR body, CHANGELOG entry and search.rs doc comment all said "every previously-matching doc still matches — the change is strictly additive". As you note, telescoping is sound for a fixed assignment and says nothing about the predicate when the search over assignments is incomplete.

• The CHANGELOG (the line that reaches users) now states the additive property as something checked — naming the exhaustive test — and no longer says "flush-invariant by construction".
• The PR body is rewritten, with a "Review round 2" section that states the regression, the table of the three cases, and the retraction.
• The commit message of 9a93a36 retracts 9d17311's claim explicitly rather than quietly restating it (9d17311 is already published, so it is corrected in the log, not rewritten).
• The doc comments in search.rs and index.rs say one evaluator, not one position model: the arms share phrase_positions_match but the memtable arm re-analyses text with the standard analyzer while the segment arm reads indexed positions, so a custom analyzer can still hand them different lists.

Non-blocking notes. (1) The "distinct terms never share a document position" comment was false — SynonymFilter co-locates synonyms — and is gone; pairwise distinctness over all slots is now documented as a deliberate, conservative divergence (stricter than Lucene on a synonym field, can only withhold a match, and is what the pre-#830 walk did by requiring strictly increasing positions). (2) The "flush-invariant by construction" overstatement is corrected as above. (3) The [b, a] assertion in sloppy_phrase_transposition_semantics is satisfied by the in-order pair; the comment now says so outright and the slop-0 fact is pinned alongside. (4) The test-module doc no longer claims the post-flush half exercises the segment positional arm for single-field slop > 0 — the planner keeps those on the stored scan; the segment arm is covered by the xerj-fts unit tests and the multi_match case.

Gates. cargo fmt --all --check clean; cargo clippy --release -p xerj-fts -p xerj-engine --all-targets -- -D warnings clean; xerj-fts 90 unit + 1 doc-test green; match_phrase_slop_transposition 6/6, multi_match_phrase_positions 12/12, xerj-engine --lib phrase 6/6 green.

Residual risk. Distinctness is enforced pairwise over all slots because the evaluator sees position lists, not term identity; on a synonym-analyzed field two different co-located query terms are therefore rejected where Lucene would accept. That is pre-existing (the old walk required strictly increasing positions), one-directional (never a false positive), and now documented rather than denied — worth its own issue if a synonym field ever needs it.