From 20d309a413b79324f29c13faf26cba0799d9deff Mon Sep 17 00:00:00 2001 From: Trenton H <797416+stumpylog@users.noreply.github.com> Date: Thu, 17 Sep 2026 14:55:13 -0700 Subject: [PATCH] Enhancement: Match fuzzy terms in place inside the parsed query (#14157) * Feature: match fuzzy terms in place inside the parsed query Fuzzy matching was a separate clause OR'd in above the query: a flat bag of the query's words, re-parsed through tantivy's own parser, blended beside the exact clause. Nothing around a term reached it, so a fielded term fuzzed across every default field, a filter did not constrain it, and an exclusion had to be hoisted back over the whole blend to stop the clause re-admitting what the query had just excluded. Widen each leaf where it sits instead, through emit()'s rewrite_leaf hook, so fielding, negation, AND, REQUIRE and positive filters constrain the fuzzy match exactly as they constrain the exact one. Each of a leaf's words becomes a Fuzzy leaf on the leaf's own field, boosted to 0.1, beside the leaf and any CJK alternative it already had. * Hello? --- docs/configuration.md | 2 + pyproject.toml | 2 +- src/documents/search/_query.py | 465 ++++++++--------- src/documents/tests/search/test_acceptance.py | 15 +- src/documents/tests/search/test_any_of.py | 43 ++ src/documents/tests/search/test_cjk_clause.py | 14 +- .../tests/search/test_cjk_leaf_rewrite.py | 7 +- .../tests/search/test_cjk_run_extraction.py | 2 +- .../tests/search/test_cjk_widening.py | 21 +- .../search/test_conjunctive_negations.py | 149 ------ .../tests/search/test_fuzzy_alternative.py | 226 +++++++++ .../tests/search/test_fuzzy_scoring.py | 134 +++++ .../tests/search/test_fuzzy_tokenization.py | 127 +++-- .../tests/search/test_fuzzy_widening.py | 467 ++++++++++++++++++ .../tests/search/test_negated_leaf_ids.py | 208 ++++++++ src/documents/tests/search/test_query.py | 21 +- src/documents/tests/search/test_widen_leaf.py | 133 +++++ 17 files changed, 1522 insertions(+), 514 deletions(-) create mode 100644 src/documents/tests/search/test_any_of.py delete mode 100644 src/documents/tests/search/test_conjunctive_negations.py create mode 100644 src/documents/tests/search/test_fuzzy_alternative.py create mode 100644 src/documents/tests/search/test_fuzzy_scoring.py create mode 100644 src/documents/tests/search/test_fuzzy_widening.py create mode 100644 src/documents/tests/search/test_negated_leaf_ids.py create mode 100644 src/documents/tests/search/test_widen_leaf.py diff --git a/docs/configuration.md b/docs/configuration.md index 315188520..c7cf69366 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1279,6 +1279,8 @@ Tantivy stemmer equivalent, stemming is disabled. matching. Fuzzy results rank below exact matches. A value of `0.5` is a reasonable starting point. Leave unset to disable fuzzy matching entirely. + Words of a single character are not fuzzy-matched, since a single-character approximate match would match nearly every term in the index. + Defaults to unset (disabled). #### [`PAPERLESS_SANITY_TASK_CRON=`](#PAPERLESS_SANITY_TASK_CRON) {#PAPERLESS_SANITY_TASK_CRON} diff --git a/pyproject.toml b/pyproject.toml index 6d6f9ada6..2027e5f8c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -247,7 +247,7 @@ per-file-ignores."src/documents/models.py" = [ isort.force-single-line = true [tool.codespell] -ignore-words-list = "criterias,afterall,valeu,ureue,equest,ure,assertIn,Oktober,commitish,NIN,nin" +ignore-words-list = "criterias,afterall,valeu,ureue,equest,ure,assertIn,Oktober,commitish,NIN,nin,reprot" skip = """\ src-ui/src/locale/*,src-ui/pnpm-lock.yaml,src-ui/e2e/*,src/paperless_mail/tests/samples/*,src/paperless/tests/samples\ /mail/*,src/documents/tests/samples/*,*.po,*.json\ diff --git a/src/documents/search/_query.py b/src/documents/search/_query.py index 87a7ff10b..b2c21879e 100644 --- a/src/documents/search/_query.py +++ b/src/documents/search/_query.py @@ -1,5 +1,6 @@ from __future__ import annotations +import functools import logging import unicodedata from functools import cache @@ -22,6 +23,7 @@ from documents.search._errors import MultipleSearchQueryErrors from documents.search._errors import SearchQueryError from documents.search._registry import get_field_registry from documents.search._tokenizer import _bigram_analyzer +from documents.search._tokenizer import paperless_text_analyzer from documents.search._tokenizer import simple_search_tokens if TYPE_CHECKING: @@ -164,10 +166,9 @@ def _parse_cjk_text( try: return index.parse_query(cjk_text, fields) except Exception: - # Broad on purpose, unlike _try_parse_fuzzy_query's narrower - # ValueError: cjk_text isn't filtered to a guaranteed-safe token - # set the way the fuzzy blend's word string is, so the exact - # failure mode tantivy could raise here isn't pinned down. + # Broad on purpose: cjk_text isn't filtered to a guaranteed-safe + # token set, so the exact failure mode tantivy could raise here + # isn't pinned down. logger.debug( "Skipping CJK search clause: could not parse CJK text: %r", cjk_text, @@ -196,110 +197,6 @@ def _build_cjk_query( return _parse_cjk_text(index, cjk_text, fields) -# A joined fuzzy word string must stay plain words: it goes back through -# tantivy's own query parser, and the raw query text the clause collects -# routinely carries characters that parser reads as grammar (a colon, a -# bracket, a quote, a leading -). Each token is cut into its word runs and -# only those are kept, so no field syntax, pattern, range or grouping can -# reach the parser. Cutting rather than dropping the whole token is what -# keeps ordinary hyphenated, dotted and quoted input ("COVID-19", -# "hello@example.com", "tax reports") contributing to the clause at all. -_WORD_RUN_RE = regex.compile(r"\w+") - -# The one piece of tantivy grammar that survives the cut: its boolean -# keywords are themselves word runs. Only these exact spellings are -# grammar there ("And"/"and" are ordinary terms), so lowercasing exactly -# these turns them back into the ordinary terms the field analyzer used to -# make of them, before the clause switched to raw text. Left alone, a -# quoted phrase would silently restructure the clause ("tax AND reports" -# becoming a conjunction) or fail to parse and drop it entirely -# ("tax AND", or "IN" anywhere). -# -# Only these words are touched: tantivy lowercases query terms with the -# field's own analyzer, and doing it ourselves first is not always the -# same operation (Python folds a final sigma to a different letter than -# tantivy does, and turns Turkish 'İ' into a sequence tantivy then splits -# in two), which would search for terms the index does not contain. -_TANTIVY_KEYWORDS: Final[frozenset[str]] = frozenset({"AND", "OR", "NOT", "IN"}) - - -def _try_parse_fuzzy_query( - index: tantivy.Index, - ast: wc.ast.Node, - registry: wc.FieldRegistry, -) -> tantivy.Query | None: - """Build the fuzzy blend clause from the parsed query's free-text - words, or None if it has none. - - The clause is built by handing tantivy's own query parser a plain - word string (there's no clean AST-level fuzzy equivalent to - whoosh-compat's parse tree, and fuzzy matching was always an - approximate, secondary, 0.1-boosted clause). The words come from - whoosh_compat's ``free_text_tokens`` over the already-parsed AST, - never from the raw query string: raw whoosh grammar (date keywords, - ``[2005 to 2009]`` ranges, bracket-class wildcards) is not tantivy - syntax, and feeding it here used to knock the fuzzy clause out for - the whole query the moment any such construct appeared alongside a - typo'd word. The helper also keeps excluded terms out: a ``NOT``'d - word must not resurface through the fuzzy clause. - - Chosen trade-off: a term explicitly fielded on one of the default - search fields (``correspondent:acme``) contributes its text to the - word string UNFIELDED, so the fuzzy clause searches it across all - default fields rather than just the one the user named. That is - recall-only widening on a secondary 0.1-boosted clause the score - threshold already disciplines, accepted in exchange for never feeding - field syntax to tantivy's parser. What the word string guarantees is - exactly that: no field prefix, pattern, range, grouping or quoting - survives, and the boolean keywords that do survive (they are word - runs) are lowercased into ordinary terms; see _TANTIVY_KEYWORDS. - - The words are the query's RAW text, not the analyzer's output - (``analyzed=False``), because ``index.parse_query`` analyzes whatever - it is given and analysis is not idempotent: ``universities`` stems to - ``univers``, and handing that back stems it again to ``univ``, a term - the index does not contain. ``prefix=True`` hid this as over-broad - matching (``univ`` also prefixes ``unicycle``) rather than as no - matches at all. Raw text is untokenized, which is why it is cut into - word runs above rather than taken whole. - - The ValueError guard stays as insurance (the word string is plain - tokens, so tantivy accepting it is expected, not assumed): on a parse - failure the fuzzy clause is skipped and the exact clause stands, - rather than the whole query failing. - """ - tokens = wc.free_text_tokens( - ast, - registry=registry, - fields=_DEFAULT_SEARCH_FIELDS, - analyzed=False, - ) - words = list( - dict.fromkeys( - word.lower() if word in _TANTIVY_KEYWORDS else word - for token in tokens - for word in _WORD_RUN_RE.findall(token) - ), - ) - if not words: - return None - fuzzy_text = " ".join(words) - try: - return index.parse_query( - fuzzy_text, - _DEFAULT_SEARCH_FIELDS, - field_boosts=_FIELD_BOOSTS, - fuzzy_fields={f: (True, 1, True) for f in _DEFAULT_SEARCH_FIELDS}, - ) - except ValueError: - logger.debug( - "Skipping fuzzy search clause: token string is not valid " - "tantivy query syntax: %r", - fuzzy_text, - ) - return None - - _DEFAULT_SEARCH_FIELDS: Final[list[str]] = [ "title", "content", @@ -322,8 +219,8 @@ _SIMPLE_FIELD_BOOSTS = {"simple_title": 2.0} @cache def _get_emit_field_registry(language: str | None) -> wc.FieldRegistry: """The parse registry plus the CJK bigram fields, for analyzing and - emitting a query whose CJK leaves _widen_cjk_leaf has widened. Cached - per language, on the same trigger get_field_registry() rebuilds on. + emitting a query whose leaves _widen_leaf has widened. Cached per + language, on the same trigger get_field_registry() rebuilds on. Never used to parse: the bigram fields are internal (absent from PUBLIC_FIELDS), and queries are still parsed against @@ -370,32 +267,30 @@ def _collapse( return node_cls(children=tuple(children), **span) -def _widen_cjk_leaf(leaf: wc.ast.Term | wc.ast.Phrase) -> wc.ast.Node: - """``rewrite_leaf`` hook for emit(): widen a CJK term or phrase on a - default search field to ``Or(leaf, alternative)`` where it sits, and - leave every other leaf as it is. +def _leaf_span(leaf: wc.ast.Term | wc.ast.Phrase) -> dict[str, int | None]: + """The startchar/endchar kwargs a leaf's alternatives are built with, + so a rewritten leaf still points at the same span of the original + query text.""" + return {"startchar": leaf.startchar, "endchar": leaf.endchar} + + +def _cjk_alternative(leaf: wc.ast.Term | wc.ast.Phrase) -> wc.ast.Node | None: + """Build the bigram alternative for a CJK leaf, or None if it gets none. The content analyzer keeps an unspaced CJK run as one token, so only - the bigram fields can find a CJK term inside running text. Widening in - place, rather than OR-ing a separate bigram clause in at the top, keeps - every AND, NOT, REQUIRE, boost and field restriction around the leaf - applying to its bigram match too. analyze() calls this for negated - leaves as well, and they are widened on purpose, so ``NOT X`` excludes - exactly what ``X`` matches. + the bigram fields can find a CJK term inside running text. The alternative is built from the leaf's own text, split where the content analyzer would split it. What each kind of piece contributes, and how the pieces combine, is commented at the step that decides it. - - analyze() only offers Term and Phrase leaves, so Prefix and Wildcard - patterns are never widened. + None means there was nothing to build one from. """ if leaf.field is None or leaf.field.name not in _CJK_BIGRAM_FIELDS: - return leaf + return None text = str(leaf.text) if not _has_cjk(text): - return leaf - span = {"startchar": leaf.startchar, "endchar": leaf.endchar} + return None + span = _leaf_span(leaf) bigram_field = wc.FieldRef(_CJK_BIGRAM_FIELDS[leaf.field.name]) cjk_terms: list[wc.ast.Node] = [] latin_terms: list[wc.ast.Node] = [] @@ -432,91 +327,160 @@ def _widen_cjk_leaf(leaf: wc.ast.Term | wc.ast.Phrase) -> wc.ast.Node: if not pieces: # A few hundred codepoints match _CJK_RE but yield no token at all # from the simple tokenizer (CJK radicals, circled and squared - # forms), leaving nothing to widen with. The leaf then analyzes to - # the same nothing it does today. - return leaf + # forms), leaving nothing to widen with. The caller then leaves the + # leaf as it is, and it analyzes to the same nothing it does today. + return None # Separated latin is required alongside the CJK side, which is what # stops "invoice NOT 東京-report" from excluding every 東京 document. - alternative = _collapse(wc.ast.And, pieces, span) + return _collapse(wc.ast.And, pieces, span) + + +# The index analyzer minus stemming: the same word boundaries and the same +# drops (remove_long, characters the simple tokenizer discards). The field's +# pattern_normalizer does the one stemming step. +_FUZZY_WORD_SPLITTER: Final = paperless_text_analyzer(None) + + +def _fuzzy_alternative(leaf: wc.ast.Term | wc.ast.Phrase) -> wc.ast.Node | None: + """Build the near-match alternative for a leaf, or None if it gets none. + + Each of the leaf's words becomes a Fuzzy leaf on the leaf's own field. + A Term's words are OR'd, which is the per-word recall the old clause + had and what lets ``COVID-19`` match on one half. A Phrase's are + AND-ed: quoting asks for more than the bare words, so an Or there + would make a quoted phrase match strictly more than the same words + unquoted. Adjacency is out of reach either way, so requiring every + word is the floor. + + Words come from the index analyzer minus its stemmer, so a leaf gets a + fuzzy side exactly when its exact side has tokens. A regex split would + keep words the index never holds (``__``, or a word past remove_long), + whose exact side analyzes to nothing, leaving a required fuzzy clause + that can never match. + + Words of one character are skipped: with prefix matching, a + one-character fuzzy term matches every term in the field. + + CJK words are skipped entirely. The content analyzer keeps an unspaced + CJK run as one token, so a prefix Fuzzy over it matches any run within + one edit of its start: ``東京`` would match a document holding only + ``京都の観光案内``, the very thing the bigram fields' multitoken=AND + exists to prevent (see _get_emit_field_registry). A two-character CJK + word is as broad here as the one-character word the length guard + already rejects, and _cjk_alternative supplies the in-run recall + anyway, so there is nothing to gain and precision to lose. + + Fuzzy text goes through the field's pattern_normalizer rather than its + analyzer, and the splitter's output is already lowercased and folded, + so the word is stemmed exactly once. + """ + words = [ + word + for word in _FUZZY_WORD_SPLITTER.analyze(str(leaf.text)) + if len(word) > 1 and not _has_cjk(word) + ] + if not words: + return None + span = _leaf_span(leaf) + group = wc.ast.And if isinstance(leaf, wc.ast.Phrase) else wc.ast.Or + leaves: list[wc.ast.Node] = [ + wc.ast.Fuzzy(field=leaf.field, text=word, distance=1, prefix=True, **span) + for word in words + ] + return _collapse(group, leaves, span) + + +def _widen_leaf( + leaf: wc.ast.Term | wc.ast.Phrase, + *, + fuzzy: bool, + negated: frozenset[int], +) -> wc.ast.Node: + """``rewrite_leaf`` hook for emit(): widen a leaf on a default search + field to ``Or(leaf, alternatives...)`` where it sits, and leave every + other leaf as it is. + + Widening in place, rather than OR-ing a separate clause in at the top, + keeps every AND, NOT, REQUIRE, boost, field restriction and positive + filter around the leaf applying to its widened match too. + + A leaf can gain a CJK alternative, a fuzzy one, or both. A negated + leaf keeps its CJK alternative, because NOT X should exclude exactly + what X matches, but gets no fuzzy one: with prefix matching, NOT tax + would otherwise exclude "taxi" and "taxonomy". Negated leaves are + identified by identity through the pre-scan, since the hook cannot see + a leaf's context. + + analyze() only offers Term and Phrase leaves, so Prefix and Wildcard + patterns are never widened. + """ + if leaf.field is None or leaf.field.name not in _DEFAULT_SEARCH_FIELDS: + return leaf + span = _leaf_span(leaf) + alternatives: list[wc.ast.Node] = [] + cjk = _cjk_alternative(leaf) + if cjk is not None: + alternatives.append(cjk) + if fuzzy and id(leaf) not in negated: + near = _fuzzy_alternative(leaf) + if near is not None: + alternatives.append(wc.ast.Boosted(child=near, boost=0.1, **span)) + if not alternatives: + return leaf # The leaf itself, not a copy: analyze() then keeps it combined the way # its enclosing group says rather than the way this Or would, and a long # run its analyzer drops to nothing leaves just the alternative. - return wc.ast.Or(children=(leaf, alternative), **span) + return wc.ast.Or(children=(leaf, *alternatives), **span) -class _ConjunctiveNegations(wc.ast.Visitor[tuple["wc.ast.Node", ...]]): - """Collect the subtrees an AST excludes from every document it matches. +def _negated_leaf_ids(node: wc.ast.Node) -> frozenset[int]: + """Return the id() of every Term and Phrase under a negation. - A negation reached through ``And``/``AndNot``/``Require`` (and through - the required half of an ``AndMaybe``) constrains the whole query, so it - can be re-stated above the blend. ``Or`` is deliberately not descended - into: in ``invoice OR NOT secret`` the negation is one branch's own - condition, and hoisting it would throw away documents the other branch - matches. Nor is a collected subtree descended into, since a negation - inside a negation is not an exclusion. + A negated leaf keeps its CJK alternative but gets no fuzzy one, and + the hook cannot see a leaf's context, so the tree is walked once here + and the hook compares by identity. analyze() guarantees it is handed + the input tree's own leaf objects, which is what makes identity work. - Node types with no negation to contribute (every leaf, ``Or``) fall - through to ``generic_visit``. + Negative positions are Not.child and AndNot.negative, and nothing + else in the node set. Leaves are collected at any depth and under any + number of negations: over-collecting costs a widening, while missing + a negated leaf would let NOT tax exclude "taxi". + + Iterative, and total over node types: this runs outside emit()'s + error conversion, so an exception here would reach the generic 500 + handler. """ - - def generic_visit(self, node: wc.ast.Node) -> tuple[wc.ast.Node, ...]: - return () - - def visit_not(self, node: wc.ast.Not) -> tuple[wc.ast.Node, ...]: - return (node.child,) - - def visit_andnot(self, node: wc.ast.AndNot) -> tuple[wc.ast.Node, ...]: - return (*self.visit(node.positive), node.negative) - - def visit_and(self, node: wc.ast.And) -> tuple[wc.ast.Node, ...]: - return tuple( - negation for child in node.children for negation in self.visit(child) - ) - - def visit_boosted(self, node: wc.ast.Boosted) -> tuple[wc.ast.Node, ...]: - return self.visit(node.child) - - def visit_andmaybe(self, node: wc.ast.AndMaybe) -> tuple[wc.ast.Node, ...]: - return self.visit(node.required) - - def visit_require(self, node: wc.ast.Require) -> tuple[wc.ast.Node, ...]: - return (*self.visit(node.scored), *self.visit(node.filter_only)) + negated: set[int] = set() + stack: list[tuple[wc.ast.Node, bool]] = [(node, False)] + while stack: + current, under_negation = stack.pop() + if isinstance(current, (wc.ast.Term, wc.ast.Phrase)): + if under_negation: + negated.add(id(current)) + elif isinstance(current, wc.ast.Not): + stack.append((current.child, True)) + elif isinstance(current, wc.ast.AndNot): + stack.append((current.positive, under_negation)) + stack.append((current.negative, True)) + elif isinstance(current, wc.ast.AndMaybe): + stack.append((current.required, under_negation)) + stack.append((current.optional, under_negation)) + elif isinstance(current, wc.ast.Require): + stack.append((current.scored, under_negation)) + stack.append((current.filter_only, under_negation)) + elif isinstance(current, wc.ast.Boosted): + stack.append((current.child, under_negation)) + elif isinstance(current, (wc.ast.And, wc.ast.Or)): + stack.extend((child, under_negation) for child in current.children) + return frozenset(negated) -def _negation_clauses( - index: tantivy.Index, - ast: wc.ast.Node, - registry: wc.FieldRegistry, -) -> list[tuple[tantivy.Occur, tantivy.Query]]: - """MustNot clauses for everything ``ast`` excludes conjunctively. - - Each excluded subtree is emitted as its own positive query and attached - with ``MustNot``, rather than emitting a negative query and hoping - tantivy accepts a bare one. - - The except branch has no reachable trigger under the current control - flow: this only runs after parse_user_query has already emitted the - exact clause from the whole parsed AST (widened, when the query has CJK - text), and every subtree ``_ConjunctiveNegations`` collects here is a - piece of that same parsed tree. The re-emit here uses the public - registry, without the widening hook, while the exact clause was emitted - against ``_get_emit_field_registry()``; the two registries agree on - every public field, and the hook only adds nodes, so this does not - reopen the branch. Kept as insurance, not dead weight: re-emitting a - subtree in isolation is not proven identical to emitting it in context, - just believed to be, and this is the seam that finds out if that - belief is ever wrong. - """ - try: - return [ - ( - tantivy.Occur.MustNot, - tantivy_emit(negation, index=index, registry=registry), - ) - for negation in _ConjunctiveNegations().visit(ast) - ] - except QueryError as e: # pragma: no cover - raise _map_emit_error(e) from e +# Weight of the tiebreak clause that restores relevance ordering among +# near-miss results. The widened tree is const-scored so the threshold +# cannot cut a near-miss by the BM25 spread of the query's correctly +# spelled words; this small share of its real score orders them again. +# A BM25 spread above 0.1/_FUZZY_TIEBREAK can still cut one. +_FUZZY_TIEBREAK: Final[float] = 0.01 def _any_of(clauses: list[tuple[tantivy.Occur, tantivy.Query]]) -> tantivy.Query: @@ -567,8 +531,8 @@ def parse_user_query( tz: tzinfo, ) -> tantivy.Query: """ - Parse user query through whoosh-compat, widen CJK terms, then blend in - the optional fuzzy clause. + Parse user query through whoosh-compat, then widen its leaves and emit, + once or twice depending on whether fuzzy matching is on. 1. wc.parse() against the shared FieldRegistry (whoosh grammar -> AST). Bare notes:/custom_fields: prefixes resolve to their default subpath @@ -577,31 +541,23 @@ def parse_user_query( 2. Any diagnostics (bad dates/numbers) map to SearchQueryError subclasses and raise, the view returns HTTP 400 with every offending field listed, not just the first. - 3. When the query has CJK text, emit()'s rewrite_leaf hook - (_widen_cjk_leaf) rewrites each CJK term in the AST to also match - its bigram field, in place, so the rest of the query constrains the - bigram match too. The tree is analyzed and emitted against - _get_emit_field_registry(), which adds the bigram fields; the query - itself was parsed without them. + 3. With fuzzy off, the AST is emitted once. If the query has CJK text, + emit()'s rewrite_leaf hook (_widen_leaf) rewrites each CJK term in + the AST to also match its bigram field, in place, so the rest of + the query constrains the bigram match too. With fuzzy on, the AST + is emitted twice through the same hook: once with fuzzy=True for + the widened tree used as a filter, and once with fuzzy=False for a + normally scored tree, and the two are blended into a boolean query + (see the comment above the blend for why). Either way the tree is + analyzed and emitted against _get_emit_field_registry() whenever + CJK or fuzzy widening applies, which adds the bigram fields; the + query itself was parsed without them. emit() turns the AST into a tantivy.Query directly (no string round-trip). A QueryError is routed by its Diagnostic's Cause (_map_emit_error): a construct that parses but can't execute against tantivy (e.g. a text-field range) is a 400, a registry/schema mismatch is logged and re-raised, and an INTERNAL defect is re-raised. - 4. Optional fuzzy blend (ADVANCED_FUZZY_SEARCH_THRESHOLD) builds a - plain word string from the parsed AST's free-text tokens - (whoosh_compat.free_text_tokens) and feeds THAT to - index.parse_query, never raw_query, whose whoosh grammar (date - keywords, bracket-class wildcards, etc.) tantivy's parser rejects, - which used to silently knock the fuzzy clause out of any mixed - query (see _try_parse_fuzzy_query). - 5. When the fuzzy clause was added, the query's conjunctive exclusions - are restated as MustNot above the blend (_negation_clauses): a - clause built from positive terms cannot express them, and as a bare - Should it would undo them. The restated exclusions come from the - unwidened AST, so they are content-only for CJK terms: a known gap - that goes away once fuzzy is also widened in the tree. """ registry = get_field_registry(settings.SEARCH_LANGUAGE) result = wc.parse( @@ -614,44 +570,53 @@ def parse_user_query( if result.diagnostics: raise _diagnostics_to_error(result.diagnostics) - emit_registry, rewrite_leaf = registry, None - if _has_cjk(raw_query): - emit_registry = _get_emit_field_registry(settings.SEARCH_LANGUAGE) - rewrite_leaf = _widen_cjk_leaf - try: - exact = tantivy_emit( + fuzzy_on = settings.ADVANCED_FUZZY_SEARCH_THRESHOLD is not None + cjk = _has_cjk(raw_query) + emit_registry = ( + _get_emit_field_registry(settings.SEARCH_LANGUAGE) + if cjk or fuzzy_on + else registry + ) + negated = _negated_leaf_ids(result.ast) if fuzzy_on else frozenset() + + def emit_widened(*, fuzzy: bool) -> tantivy.Query: + hook = ( + functools.partial(_widen_leaf, fuzzy=fuzzy, negated=negated) + if cjk or fuzzy + else None + ) + return tantivy_emit( result.ast, index=index, registry=emit_registry, - rewrite_leaf=rewrite_leaf, + rewrite_leaf=hook, ) + + try: + if not fuzzy_on: + return emit_widened(fuzzy=False) + widened = emit_widened(fuzzy=True) + scored = emit_widened(fuzzy=False) except QueryError as e: raise _map_emit_error(e) from e - clauses: list[tuple[tantivy.Occur, tantivy.Query]] = [ - (tantivy.Occur.Should, exact), - ] - - threshold = settings.ADVANCED_FUZZY_SEARCH_THRESHOLD - if threshold is not None: - fuzzy = _try_parse_fuzzy_query(index, result.ast, registry) - if fuzzy is not None: - clauses.append( - (tantivy.Occur.Should, tantivy.Query.boost_query(fuzzy, 0.1)), - ) - - if len(clauses) == 1: - return exact - # The fuzzy clause is built from positive terms only, so as a plain - # Should beside the exact clause it re-admits exactly the documents the - # query excluded. Restate the exclusions once, above the whole blend. - # Redundant against the exact clause, which already carries them, but - # idempotently so. - negations = _negation_clauses(index, result.ast, registry) - if not negations: - return _any_of(clauses) + # The widened tree supplies the matched set at a flat score, so a + # near-miss is not ranked by how well the query's correctly spelled + # words matched. The CJK-only tree adds real scoring back for what + # matched exactly, and the last clause reintroduces the widened tree's + # own scoring at a small weight so near-misses still rank among + # themselves. const_score_query discards every boost inside it, the + # title field's 2.0 included, so a title match earns its boost through + # the second and third clauses rather than the first. return tantivy.Query.boolean_query( - [(tantivy.Occur.Must, _any_of(clauses)), *negations], + [ + (tantivy.Occur.Must, tantivy.Query.const_score_query(widened, 0.1)), + (tantivy.Occur.Should, scored), + ( + tantivy.Occur.Should, + tantivy.Query.boost_query(widened, _FUZZY_TIEBREAK), + ), + ], ) diff --git a/src/documents/tests/search/test_acceptance.py b/src/documents/tests/search/test_acceptance.py index 4372d943b..10fd41fc8 100644 --- a/src/documents/tests/search/test_acceptance.py +++ b/src/documents/tests/search/test_acceptance.py @@ -261,10 +261,10 @@ class TestUnregisteredIdFieldFoldsToLiteralText: class TestFuzzyBlendSurvivesWhooshGrammar: """A query mixing whoosh-only grammar (a date keyword) with a typo'd free-text word must still fuzzy-match the intended document when - ADVANCED_FUZZY_SEARCH_THRESHOLD is enabled. The fuzzy clause is built - from the parsed query's free-text tokens (whoosh_compat's - free_text_tokens), never from the raw query string, so whoosh grammar - that tantivy's own parser rejects cannot knock the fuzzy clause out.""" + ADVANCED_FUZZY_SEARCH_THRESHOLD is enabled. Fuzzy widening happens + inside the already-parsed AST (_widen_leaf, via emit()), never by + re-parsing the raw query string, so whoosh grammar that tantivy's own + parser rejects cannot knock the fuzzy side out.""" def test_typo_fuzzy_matches_alongside_date_keyword( self, @@ -280,11 +280,10 @@ class TestFuzzyBlendSurvivesWhooshGrammar: rejects ("added:today") with a one-transposition misspelling of a word in the indexed content THEN: - - The document still matches, because the fuzzy clause is - built from the parsed query's free-text tokens - (whoosh_compat's free_text_tokens), never from the raw + - The document still matches, because fuzzy widening happens + inside the already-parsed AST, never by re-parsing the raw query string, so grammar tantivy's parser cannot handle - cannot knock the fuzzy clause out + cannot knock the fuzzy side out """ settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = 0.5 with time_machine.travel(FROZEN_NOW, tick=False): diff --git a/src/documents/tests/search/test_any_of.py b/src/documents/tests/search/test_any_of.py new file mode 100644 index 000000000..47361a258 --- /dev/null +++ b/src/documents/tests/search/test_any_of.py @@ -0,0 +1,43 @@ +"""_any_of, the clause-list helper. + +One test, for the empty-list case its callers never produce but which the +helper still has to answer for. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +from documents.search._query import _any_of + +if TYPE_CHECKING: + from collections.abc import Callable + + from documents.models import Document + from documents.search._backend import TantivyBackend + +pytestmark = [pytest.mark.search, pytest.mark.django_db] + + +class TestAnyOf: + def test_an_empty_clause_list_matches_nothing( + self, + backend: TantivyBackend, + index_document: Callable[..., Document], + ) -> None: + """ + GIVEN: + - An indexed document, and no clauses at all + WHEN: + - _any_of is called with an empty list and the result is run + THEN: + - It matches no documents, rather than raising or matching + everything + """ + index_document(title="x", content="x") + + results = backend._index.searcher().search(_any_of([]), limit=10) + + assert len(results.hits) == 0 diff --git a/src/documents/tests/search/test_cjk_clause.py b/src/documents/tests/search/test_cjk_clause.py index cf3bae5b8..19e827e56 100644 --- a/src/documents/tests/search/test_cjk_clause.py +++ b/src/documents/tests/search/test_cjk_clause.py @@ -99,7 +99,7 @@ class TestCjkClauseFollowsTheParsedQuery: ("threshold", "expected"), [ pytest.param(None, {"titled"}, id="fuzzy_off"), - pytest.param(0.0, {"titled", "content_only"}, id="fuzzy_on"), + pytest.param(0.0, {"titled"}, id="fuzzy_on"), ], ) def test_fielded_cjk_term_searches_only_that_field( @@ -118,14 +118,10 @@ class TestCjkClauseFollowsTheParsedQuery: WHEN: - "title:東京" is searched THEN: - - With fuzzy off, only the titled document matches: the CJK - clause honours the field, so 'title:東京' must not match a - document whose 東京 is only in the content. With fuzzy on, - the content-only document is also readmitted, because the - fuzzy clause contributes every free-text term UNFIELDED by - design (see _try_parse_fuzzy_query) on its own - 0.1-boosted terms -- a documented trade-off, pinned here so - it stays deliberate + - Only the titled document matches, whether fuzzy is on or + off: both the CJK alternative and the fuzzy alternative are + widened in place on the fielded leaf, so 'title:東京' still + must not match a document whose 東京 is only in the content """ settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = threshold content_only = index_document( diff --git a/src/documents/tests/search/test_cjk_leaf_rewrite.py b/src/documents/tests/search/test_cjk_leaf_rewrite.py index 52d231ff6..815ec5d22 100644 --- a/src/documents/tests/search/test_cjk_leaf_rewrite.py +++ b/src/documents/tests/search/test_cjk_leaf_rewrite.py @@ -17,7 +17,7 @@ import whoosh_compat.ast as wc_ast from documents.search._query import _DEFAULT_SEARCH_FIELDS from documents.search._query import _FIELD_BOOSTS from documents.search._query import _get_emit_field_registry -from documents.search._query import _widen_cjk_leaf +from documents.search._query import _widen_leaf from documents.search._registry import get_field_registry if TYPE_CHECKING: @@ -44,6 +44,11 @@ def _widened(original: wc_ast.Node, bigram_side: wc_ast.Node) -> wc_ast.Or: return wc_ast.Or(children=(original, bigram_side)) +def _widen_cjk_leaf(leaf: wc_ast.Term | wc_ast.Phrase) -> wc_ast.Node: + """The hook as the CJK work used it: no fuzzy side, nothing negated.""" + return _widen_leaf(leaf, fuzzy=False, negated=frozenset()) + + def _analyze(tree: wc_ast.Node) -> wc_ast.Node: return wc.analyze( tree, diff --git a/src/documents/tests/search/test_cjk_run_extraction.py b/src/documents/tests/search/test_cjk_run_extraction.py index b1641823b..8ed3ef21d 100644 --- a/src/documents/tests/search/test_cjk_run_extraction.py +++ b/src/documents/tests/search/test_cjk_run_extraction.py @@ -2,7 +2,7 @@ _CJK_RE decides both what is indexed into the bigram fields (extract_cjk_text) and how a query's CJK text is cut into runs -(_widen_cjk_leaf). A character it misses splits a word in two on both +(_widen_leaf). A character it misses splits a word in two on both sides. For the katakana prolonged sound mark that leaves one-character runs, which have no bigrams, so the word could not be found through the bigram fields at all. diff --git a/src/documents/tests/search/test_cjk_widening.py b/src/documents/tests/search/test_cjk_widening.py index c063565ff..26aea8108 100644 --- a/src/documents/tests/search/test_cjk_widening.py +++ b/src/documents/tests/search/test_cjk_widening.py @@ -159,7 +159,7 @@ class TestTokenizerDroppedCodepoints: WHEN: - "⺀" is searched THEN: - - Nothing matches: _widen_cjk_leaf finds no pieces to widen + - Nothing matches: _widen_leaf finds no CJK pieces to widen with and returns the leaf as it is, which analyzes to the same nothing it always did """ @@ -570,22 +570,22 @@ class TestOtherDefaultFields: assert matched_ids("invoice NOT 東京") == {latin.pk} -class TestFuzzyOnGap: +class TestFuzzyDoesNotReadmit: @pytest.mark.parametrize( ("query", "threshold", "expected"), [ pytest.param("invoice NOT 東京", None, {"latin"}, id="not_fuzzy_off"), - pytest.param("invoice NOT 東京", 0.0, {"latin", "cjk"}, id="not_fuzzy_on"), + pytest.param("invoice NOT 東京", 0.0, {"latin"}, id="not_fuzzy_on"), pytest.param("東京 AND invoice", None, {"cjk"}, id="and_fuzzy_off"), pytest.param( "東京 AND invoice", 0.0, - {"cjk", "cjk_only", "latin"}, + {"cjk"}, id="and_fuzzy_on", ), ], ) - def test_fuzzy_on_readmits_what_the_structure_excludes( + def test_fuzzy_on_matches_fuzzy_off_for_structure( self, index_document: Callable[..., Document], matched_ids: Callable[[str], set[int]], @@ -602,13 +602,10 @@ class TestFuzzyOnGap: WHEN: - "invoice NOT 東京" or "東京 AND invoice" is searched THEN: - - With fuzzy off, the query's structure decides. With fuzzy on, - the separate fuzzy clause matches any one of the query's - words on its own, and the exclusion restated above it is - content-only, so it re-admits what NOT and AND excluded. A - known interim gap, not a regression (both settings returned - these before), removed when fuzzy moves into the query tree - too; pinned so that change flips it deliberately + - The result is the same whether fuzzy is on or off: fuzzy + widening happens inside the tree now, so a CJK NOT or AND + still constrains both the exact and the fuzzy side, and + neither re-admits what the structure excludes """ settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = threshold cjk = index_document( diff --git a/src/documents/tests/search/test_conjunctive_negations.py b/src/documents/tests/search/test_conjunctive_negations.py deleted file mode 100644 index 8feb5f394..000000000 --- a/src/documents/tests/search/test_conjunctive_negations.py +++ /dev/null @@ -1,149 +0,0 @@ -"""_ConjunctiveNegations, the AST visitor that collects the subtrees a -query excludes from every document it matches, and _any_of, the clause-list -collapsing helper it feeds into. - -Result-level proof that a negation reached through NOT/AND survives the -fuzzy blend lives in test_query_negation.py. These are direct unit -tests of the visitor's dispatch for the rarer grammar shapes -(AndNot/Boosted/AndMaybe/Require) that file's real-corpus queries don't -happen to exercise, plus the empty-clause-list case of _any_of. -""" - -from __future__ import annotations - -from typing import TYPE_CHECKING - -import pytest -import whoosh_compat.ast as wc_ast - -from documents.models import Document -from documents.search._query import _any_of -from documents.search._query import _ConjunctiveNegations - -if TYPE_CHECKING: - from documents.search._backend import TantivyBackend - -pytestmark = [pytest.mark.search, pytest.mark.django_db] - - -def _term(text: str) -> wc_ast.Term: - return wc_ast.Term(field=None, text=text) - - -class TestConjunctiveNegationsVisitor: - def test_visit_andnot_hoists_the_negative_branch(self) -> None: - """ - GIVEN: - - An AndNot(positive=a, negative=b) node - WHEN: - - _ConjunctiveNegations visits it - THEN: - - The negative branch is collected as an exclusion, since - AndNot requires positive and excludes negative - """ - negative = _term("b") - node = wc_ast.AndNot(positive=_term("a"), negative=negative) - assert _ConjunctiveNegations().visit(node) == (negative,) - - def test_visit_andnot_also_collects_negations_already_in_the_positive_branch( - self, - ) -> None: - """ - GIVEN: - - An AndNot node whose positive branch already contains a NOT - WHEN: - - _ConjunctiveNegations visits it - THEN: - - Both the positive branch's own negation and the AndNot's - negative branch are collected - """ - excluded_in_positive = _term("excluded") - negative = _term("negative") - node = wc_ast.AndNot( - positive=wc_ast.Not(child=excluded_in_positive), - negative=negative, - ) - assert _ConjunctiveNegations().visit(node) == (excluded_in_positive, negative) - - def test_visit_boosted_passes_through_to_the_child(self) -> None: - """ - GIVEN: - - A Boosted node (e.g. "(invoice NOT secret)^2") wrapping a - NOT - WHEN: - - _ConjunctiveNegations visits it - THEN: - - The negation inside the boosted child is still collected: a - boost must not shield an exclusion from being hoisted - """ - excluded = _term("secret") - node = wc_ast.Boosted(child=wc_ast.Not(child=excluded), boost=2.0) - assert _ConjunctiveNegations().visit(node) == (excluded,) - - def test_visit_andmaybe_only_descends_into_required(self) -> None: - """ - GIVEN: - - An AndMaybe(required=a, optional=b) node where both required - and optional contain their own NOT - WHEN: - - _ConjunctiveNegations visits it - THEN: - - Only the negation in the required branch is collected. The - optional branch is not a conjunctive constraint on the whole - query (documents that fail it still match), so hoisting a - negation from it would exclude documents the query does not - actually exclude - """ - excluded_in_required = _term("excluded_in_required") - excluded_in_optional = _term("excluded_in_optional") - node = wc_ast.AndMaybe( - required=wc_ast.Not(child=excluded_in_required), - optional=wc_ast.Not(child=excluded_in_optional), - ) - assert _ConjunctiveNegations().visit(node) == (excluded_in_required,) - - def test_visit_require_descends_into_both_branches(self) -> None: - """ - GIVEN: - - A Require(scored=a, filter_only=b) node where both scored - and filter_only contain their own NOT - WHEN: - - _ConjunctiveNegations visits it - THEN: - - Both negations are collected: Require constrains the whole - query with both branches, one merely scored and the other - filter-only, so both are conjunctive - """ - excluded_in_scored = _term("excluded_in_scored") - excluded_in_filter = _term("excluded_in_filter") - node = wc_ast.Require( - scored=wc_ast.Not(child=excluded_in_scored), - filter_only=wc_ast.Not(child=excluded_in_filter), - ) - assert _ConjunctiveNegations().visit(node) == ( - excluded_in_scored, - excluded_in_filter, - ) - - -class TestAnyOfEmptyClauseList: - def test_no_clauses_returns_a_query_that_matches_nothing( - self, - backend: TantivyBackend, - ) -> None: - """ - GIVEN: - - No clauses at all - WHEN: - - _any_of is called with an empty list - THEN: - - It returns tantivy's empty_query() rather than raising or - wrapping zero clauses in a boolean_query, and running it - against a real index matches no documents - """ - doc = Document.objects.create(title="x", content="x", checksum="any-of-empty") - backend.add_or_update(doc) - - query = _any_of([]) - results = backend._index.searcher().search(query, limit=10) - assert len(results.hits) == 0 diff --git a/src/documents/tests/search/test_fuzzy_alternative.py b/src/documents/tests/search/test_fuzzy_alternative.py new file mode 100644 index 000000000..b788bc7c4 --- /dev/null +++ b/src/documents/tests/search/test_fuzzy_alternative.py @@ -0,0 +1,226 @@ +"""_fuzzy_alternative, the near-match side of the widening hook. + +A leaf's words each become a Fuzzy leaf on the leaf's own field, so a +typo in one word of a term still finds the document while everything +around the term keeps constraining it. A Term's words are OR'd for +per-word recall; a Phrase's are AND-ed, because quoting must not widen a +search. CJK words get no fuzzy side at all. +""" + +from __future__ import annotations + +import pytest +import whoosh_compat as wc +import whoosh_compat.ast as wc_ast + +from documents.search._query import _fuzzy_alternative + +pytestmark = pytest.mark.search + +_CONTENT = wc.FieldRef("content") +_TITLE = wc.FieldRef("title") + +# 130 characters, past the analyzer's 129-byte remove_long limit (128 is +# kept, 129 is dropped), so the index never holds it and neither side +# should search for it. +_TOO_LONG = "x" * 130 + + +def _content(text: str) -> wc_ast.Term: + return wc_ast.Term(field=_CONTENT, text=text) + + +def _fuzzy(field: wc.FieldRef, text: str) -> wc_ast.Fuzzy: + return wc_ast.Fuzzy(field=field, text=text, distance=1, prefix=True) + + +class TestTheAlternative: + def test_a_single_word_becomes_one_fuzzy_leaf(self) -> None: + """ + GIVEN: + - A one-word term + WHEN: + - The fuzzy alternative is built + THEN: + - It is a single Fuzzy leaf on the same field, distance 1 and + prefix matching, which is what the old clause used + """ + assert _fuzzy_alternative(_content("invoice")) == _fuzzy(_CONTENT, "invoice") + + def test_the_leafs_own_field_is_used(self) -> None: + """ + GIVEN: + - A term fielded on title + WHEN: + - The fuzzy alternative is built + THEN: + - The Fuzzy leaf is on title, not spread across the default + fields. This is the fielding fix: today's clause searches a + fielded word everywhere + """ + assert _fuzzy_alternative(wc_ast.Term(field=_TITLE, text="invoice")) == _fuzzy( + _TITLE, + "invoice", + ) + + def test_several_words_are_ored(self) -> None: + """ + GIVEN: + - A hyphenated term the analyzer splits into two words + WHEN: + - The fuzzy alternative is built + THEN: + - The words are OR'd. AND would be stricter than the exact + side, which is OR'd for an unfielded term, and would lose + today's per-word recall inside a term + """ + assert _fuzzy_alternative(_content("COVID-19")) == wc_ast.Or( + children=(_fuzzy(_CONTENT, "covid"), _fuzzy(_CONTENT, "19")), + ) + + def test_words_are_split_where_the_index_splits_them(self) -> None: + """ + GIVEN: + - A term the analyzer lowercases and folds + WHEN: + - The fuzzy alternative is built + THEN: + - The Fuzzy text is the analyzer's output, not the raw text, + so it is in the same shape as the index terms + """ + assert _fuzzy_alternative(_content("Éclair")) == _fuzzy(_CONTENT, "eclair") + + def test_the_splitter_does_not_stem(self) -> None: + """ + GIVEN: + - A word the stemmer would change + WHEN: + - The fuzzy alternative is built + THEN: + - The word is unstemmed. Fuzzy text goes through the field's + pattern_normalizer, which stems it once; stemming here too + would search for a term the index does not hold + """ + assert _fuzzy_alternative(_content("universities")) == _fuzzy( + _CONTENT, + "universities", + ) + + @pytest.mark.parametrize( + "text", + [ + pytest.param("x", id="one_character"), + pytest.param("__", id="only_dropped_characters"), + pytest.param(_TOO_LONG, id="past_remove_long"), + pytest.param("", id="empty"), + pytest.param(" ", id="whitespace"), + ], + ) + def test_text_with_no_usable_word_gets_no_alternative(self, text: str) -> None: + """ + GIVEN: + - Text with no word the index could hold: a single character, + characters the tokenizer drops, a word past remove_long, or + nothing at all + WHEN: + - The fuzzy alternative is built + THEN: + - None, so the leaf is left alone. Returning an alternative + here would collapse Or(leaf, fuzzy) to a required clause + that can never match, making a query return LESS with fuzzy + on than off + """ + assert _fuzzy_alternative(_content(text)) is None + + def test_a_one_character_word_is_dropped_from_a_longer_term(self) -> None: + """ + GIVEN: + - A term mixing a one-character word with a real one + WHEN: + - The fuzzy alternative is built + THEN: + - Only the real word survives. A one-character prefix fuzzy + term matches every term in the field + """ + assert _fuzzy_alternative(_content("h52.1")) == _fuzzy(_CONTENT, "h52") + + def test_the_leaf_span_is_copied(self) -> None: + """ + GIVEN: + - A leaf with a source span, whose words both survive the + one-character filter so there is an Or to inspect + WHEN: + - The fuzzy alternative is built + THEN: + - Every node it builds carries that span, so an emit-time + diagnostic still points into the query text + """ + leaf = wc_ast.Term(field=_CONTENT, text="ab-cd", startchar=4, endchar=9) + + alternative = _fuzzy_alternative(leaf) + + assert isinstance(alternative, wc_ast.Or) + spans = { + (node.startchar, node.endchar) + for node in (alternative, *alternative.children) + } + assert spans == {(4, 9)} + + +class TestPhrasesAreNotWidened: + def test_a_multi_word_phrase_requires_every_word(self) -> None: + """ + GIVEN: + - A quoted two-word phrase + WHEN: + - Its fuzzy alternative is built + THEN: + - The words are And-combined, where a Term's are Or-combined + (test_a_hyphenated_term_matches_on_one_word pins that). The + parser's default group is And, so an Or here would let the + quoted phrase match strictly more than the same two words + unquoted + """ + leaf = wc_ast.Phrase(field=_CONTENT, text="tax report") + + assert _fuzzy_alternative(leaf) == wc_ast.And( + children=(_fuzzy(_CONTENT, "tax"), _fuzzy(_CONTENT, "report")), + ) + + +class TestCjkGetsNoFuzzySide: + @pytest.mark.parametrize( + "text", + [ + pytest.param("東京", id="cjk_only"), + pytest.param("東京都の報告書", id="longer_run"), + pytest.param("서울", id="hangul"), + ], + ) + def test_a_cjk_word_is_skipped(self, text: str) -> None: + """ + GIVEN: + - A CJK term + WHEN: + - Its fuzzy alternative is built + THEN: + - There is none. The content analyzer keeps the run as one + token, so a prefix Fuzzy over it would match any run within + one edit of its start, which is the "東京都 matches 京都" + failure the bigram fields exist to avoid + """ + assert _fuzzy_alternative(_content(text)) is None + + def test_latin_beside_cjk_still_gets_its_fuzzy_side(self) -> None: + """ + GIVEN: + - A term mixing a CJK run and a latin word + WHEN: + - Its fuzzy alternative is built + THEN: + - Only the latin word is fuzzed. Skipping CJK words must not + cost the latin half its near-match + """ + leaf = _content("東京 report") + + assert _fuzzy_alternative(leaf) == _fuzzy(_CONTENT, "report") diff --git a/src/documents/tests/search/test_fuzzy_scoring.py b/src/documents/tests/search/test_fuzzy_scoring.py new file mode 100644 index 000000000..17b7d3cf8 --- /dev/null +++ b/src/documents/tests/search/test_fuzzy_scoring.py @@ -0,0 +1,134 @@ +"""What survives ADVANCED_FUZZY_SEARCH_THRESHOLD. + +The rest of the fuzzy tests run at threshold 0.0 so they see matching +behavior rather than the filter. These run at 0.5, the value the docs +recommend, because the filter has properties of its own that nothing else +covers: a near-miss must not be cut according to how well the query's +correctly spelled words matched, and an exact match must outrank one that +is only partly exact. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +if TYPE_CHECKING: + from collections.abc import Callable + + from pytest_django.fixtures import SettingsWrapper + + from documents.models import Document + from documents.search._backend import TantivyBackend + +pytestmark = [pytest.mark.search, pytest.mark.django_db] + +# Long enough that BM25 scores it well below a short document holding the +# same word, which is what makes the spread these tests are about. +_PADDING = "lorem ipsum dolor sit amet " * 40 + + +@pytest.fixture(autouse=True) +def _threshold(settings: SettingsWrapper) -> None: + settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = 0.5 + + +class TestNearMissesAreNotCutByTheSpread: + def test_two_equal_near_misses_both_survive( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - Two documents that both contain a rare correctly spelled + word and both near-match a misspelled one. One has the rare + word in its title, the other buried in a long body, so BM25 + scores them very differently + WHEN: + - Both words are searched at threshold 0.5 + THEN: + - Both survive. Neither is a better answer than the other: + they differ only in where the correctly spelled word sits. + If the widened tree were scored normally, the fuzzy side + would inherit the BM25 spread of the correct word and the + weaker document would be cut + """ + titled = index_document( + title="Zarquon", + content=f"invoices {_PADDING}", + ) + buried = index_document( + title="Nothing", + content=f"{_PADDING} zarquon invoicing", + ) + + assert matched_ids("zarquon invoce") == {titled.pk, buried.pk} + + def test_near_misses_still_rank_among_themselves( + self, + backend: TantivyBackend, + index_document: Callable[..., Document], + ) -> None: + """ + GIVEN: + - A document near-matching the query in several fields and one + near-matching it in a single field + WHEN: + - The misspelled word is searched + THEN: + - Both survive, and the multi-field one ranks first. Flat + scoring alone would tie every near-miss and lose relevance + ordering entirely, which is what the tiebreak clause exists + to prevent + """ + strong = index_document( + title="Invoices", + content="invoices for invoicing", + ) + weak = index_document(title="Nothing", content="invoices") + + hits = backend.search_ids("invoce", user=None) + + assert set(hits) == {strong.pk, weak.pk} + assert hits[0] == strong.pk + + +class TestExactOutranksNearMiss: + def test_a_fully_exact_document_outranks_a_partly_exact_one( + self, + backend: TantivyBackend, + index_document: Callable[..., Document], + ) -> None: + """ + GIVEN: + - A document matching both query words exactly but buried in a + long body, and one matching the first word exactly several + times while only near-matching the second + WHEN: + - Both words are searched, spelled correctly + THEN: + - The fully exact document ranks first. docs/configuration.md + promises fuzzy results rank below exact matches, and the + widened tree alone would break it: a partly exact document + collects real BM25 for what it did match plus a constant for + what it only near-matched + + Only the ordering is asserted, not that the partly exact document + survives the threshold. Spec 4.2a accepts that a large enough BM25 + spread still cuts a near-miss, so whether it survives depends on + the corpus and is not a property to pin. + """ + index_document( + title="Invoice Invoice Invoice", + content="reportage weekly", + ) + fully = index_document( + title="Nothing", + content=f"{_PADDING} invoice report", + ) + + hits = backend.search_ids("invoice report", user=None) + + assert hits[0] == fully.pk diff --git a/src/documents/tests/search/test_fuzzy_tokenization.py b/src/documents/tests/search/test_fuzzy_tokenization.py index 7337fb63e..8eaff30bb 100644 --- a/src/documents/tests/search/test_fuzzy_tokenization.py +++ b/src/documents/tests/search/test_fuzzy_tokenization.py @@ -1,9 +1,10 @@ -"""The words the fuzzy blend clause hands back to tantivy's parser. +"""The words a leaf contributes to its fuzzy alternative. -The clause re-parses a word string through tantivy, which analyzes it -again, so the words must be the query's raw text rather than the analyzed -text (analysis is not idempotent), and must still be split into plain -words so that hyphenated, dotted and quoted terms keep contributing. +Each leaf is widened in the tree now, so nothing is re-parsed as a string +and a boolean keyword can no longer be read as grammar. What still has to +hold is that a word is stemmed exactly once (analysis is not idempotent) +and that hyphenated, dotted and quoted terms keep contributing their +words. """ from __future__ import annotations @@ -40,42 +41,6 @@ def fuzzy_enabled(settings: SettingsWrapper) -> None: settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = 0.0 -class TestFuzzyClauseParseFailureDegradesGracefully: - def test_a_word_string_tantivy_rejects_drops_the_clause_only(self) -> None: - """ - GIVEN: - - A parsed query with free-text words, and an index-like - object whose parse_query is forced to raise ValueError - WHEN: - - _try_parse_fuzzy_query is called - THEN: - - It returns None instead of propagating, so a fuzzy word - string tantivy's own parser rejects only drops the fuzzy - clause: the exact clause still stands rather than the - whole query failing. The ValueError guard is insurance (the - word string is plain tokens, so tantivy accepting it is - expected, not assumed) - """ - import whoosh_compat as wc - - from documents.search._query import _DEFAULT_SEARCH_FIELDS - from documents.search._query import _try_parse_fuzzy_query - from documents.search._registry import get_field_registry - - registry = get_field_registry(None) - result = wc.parse( - "invoice", - registry=registry, - default_fields=_DEFAULT_SEARCH_FIELDS, - ) - - class _RaisingIndex: - def parse_query(self, *args: object, **kwargs: object) -> object: - raise ValueError("synthetic parse failure") - - assert _try_parse_fuzzy_query(_RaisingIndex(), result.ast, registry) is None - - class TestFuzzyClauseWords: def test_a_stemmed_word_is_not_stemmed_a_second_time( self, @@ -175,10 +140,10 @@ class TestFuzzyClauseWords: class TestBooleanKeywordsInRawText: - """Tantivy's boolean keywords are word runs, so they survive the cut - into words and its own parser reads them as grammar. Raw query text - reaches that parser with its case intact, so a quoted phrase can carry - them in.""" + """Tantivy's boolean keywords used to reach its parser with their case + intact, through the word string the old clause was re-parsed from, so a + quoted phrase could smuggle grammar in. Leaves are built as AST nodes + now, which closes that off structurally; these pin it shut.""" @pytest.fixture def corpus(self, backend: TantivyBackend) -> dict[str, int]: @@ -207,38 +172,41 @@ class TestBooleanKeywordsInRawText: } @pytest.mark.parametrize( - "query", + ("keyword_spelling", "ordinary_spelling"), [ - pytest.param('"tax AND reports"', id="and"), - pytest.param('"tax OR reports"', id="or"), - pytest.param('"tax NOT reports"', id="not"), - pytest.param('"tax IN reports"', id="in"), + pytest.param('"tax AND reports"', '"tax and reports"', id="and"), + pytest.param('"tax OR reports"', '"tax or reports"', id="or"), + pytest.param('"tax NOT reports"', '"tax not reports"', id="not"), + pytest.param('"tax IN reports"', '"tax in reports"', id="in"), ], ) def test_a_keyword_inside_a_phrase_stays_an_ordinary_word( self, backend: TantivyBackend, corpus: dict[str, int], - query: str, + keyword_spelling: str, + ordinary_spelling: str, ) -> None: """ GIVEN: - Three documents: one with both "taxation" and "reportage", one with only "taxation", one with only "reportage" WHEN: - - Searching for a quoted phrase carrying a tantivy boolean - keyword as one of its words (e.g. '"tax AND reports"') + - A quoted phrase carries a tantivy boolean keyword as one of + its words, spelled in upper case and in lower case THEN: - - The keyword stays an ordinary word inside the phrase, and - the fuzzy clause matches all three documents, the same - disjunction as the plain '"tax reports"' phrase: AND must - not turn it into a conjunction, NOT must not give it its own - exclusion, IN must not fail the parse + - Both spellings match the same documents, so the keyword is + an ordinary word of the phrase rather than grammar: AND does + not make it a conjunction, NOT does not give it its own + exclusion, IN does not fail the parse. Only the upper-case + spelling was ever grammar """ - assert _matched_ids(backend, '"tax reports"') == set(corpus.values()) - assert _matched_ids(backend, query) == set(corpus.values()) + assert _matched_ids(backend, keyword_spelling) == _matched_ids( + backend, + ordinary_spelling, + ) - def test_a_trailing_keyword_does_not_drop_the_clause( + def test_a_phrase_needs_a_near_match_for_every_word( self, backend: TantivyBackend, corpus: dict[str, int], @@ -248,14 +216,33 @@ class TestBooleanKeywordsInRawText: - Three documents: one with both "taxation" and "reportage", one with only "taxation", one with only "reportage" WHEN: - - Searching for '"tax AND"', a phrase ending in a tantivy - syntax error + - '"tax reports"' is searched, both words misspelled THEN: - - The fuzzy clause still matches on "tax"; 'tax AND' alone is - a syntax error to tantivy's parser, which would otherwise - cost the whole query its fuzzy clause + - Only the document near-matching both words comes back. A + quoted phrase asks for more than the bare words, so its + fuzzy side requires every one of them """ - assert _matched_ids(backend, '"tax AND"') == { - corpus["both"], - corpus["tax_only"], - } + assert _matched_ids(backend, '"tax reports"') == {corpus["both"]} + + def test_a_trailing_keyword_is_just_a_word( + self, + backend: TantivyBackend, + corpus: dict[str, int], + ) -> None: + """ + GIVEN: + - Three documents: one with both "taxation" and "reportage", + one with only "taxation", one with only "reportage" + WHEN: + - '"tax AND"' is searched, a phrase that used to be a tantivy + syntax error once the clause was re-parsed as a string + - The same phrase is searched with the keyword in lower case + THEN: + - Both match the same documents, and neither raises. Nothing + is re-parsed any more, so a trailing keyword cannot cost the + query its fuzzy side + """ + assert _matched_ids(backend, '"tax AND"') == _matched_ids( + backend, + '"tax and"', + ) diff --git a/src/documents/tests/search/test_fuzzy_widening.py b/src/documents/tests/search/test_fuzzy_widening.py new file mode 100644 index 000000000..95de0a634 --- /dev/null +++ b/src/documents/tests/search/test_fuzzy_widening.py @@ -0,0 +1,467 @@ +"""Fuzzy matching applied inside the parsed query rather than beside it. + +With a threshold set, every leaf gains a near-match alternative where it +sits, so fielding, negation, AND and positive filters all constrain the +fuzzy match exactly as they constrain the exact one. The old clause was a +flat bag of words OR'd in at the top level, which none of them reached. + +Threshold is 0.0 here so these tests see the matching behavior, not the +score filter. The filter has its own file, test_fuzzy_scoring.py. +""" + +from __future__ import annotations + +import datetime +from typing import TYPE_CHECKING + +import pytest + +from documents.models import Document +from documents.models import StoragePath + +if TYPE_CHECKING: + from collections.abc import Callable + + from pytest_django.fixtures import SettingsWrapper + + +pytestmark = [pytest.mark.search, pytest.mark.django_db] + + +@pytest.fixture(autouse=True) +def _fuzzy_on(settings: SettingsWrapper) -> None: + settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = 0.0 + + +class TestStructureIsHonoured: + @pytest.fixture + def near_matches_both( + self, + index_document: Callable[..., Document], + ) -> Document: + """One document near-matching both query words, one only the first.""" + both = index_document(title="A", content="invoices report") + index_document(title="B", content="invoices only") + return both + + def test_a_fielded_term_fuzzes_only_that_field( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - One document with a near-miss of the word in its title, one + with a near-miss in its content only + WHEN: + - "title:invoce" is searched + THEN: + - Only the title document matches. The old clause searched a + fielded word across every default field + """ + titled = index_document(title="Invoces", content="nothing") + index_document(title="Nothing", content="invoces here") + + assert matched_ids("title:invoce") == {titled.pk} + + def test_every_word_needs_a_near_match( + self, + near_matches_both: Document, + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - A document near-matching both words, and one near-matching + only the first + WHEN: + - "invoce reprot" is searched (both words misspelled) + THEN: + - Only the document near-matching both survives. The old + clause OR'd the words, so anything near one of them matched + """ + assert matched_ids("invoce reprot") == {near_matches_both.pk} + + def test_a_negated_word_is_not_fuzzed( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - A document containing "taxi" and one containing "tax" + WHEN: + - "invoice NOT tax" is searched + THEN: + - The "taxi" document survives and the "tax" one does not. A + negated leaf keeps its exact side only: fuzzing it with + prefix matching would exclude every word starting near it + """ + taxi = index_document(title="A", content="invoice taxi fare") + index_document(title="B", content="invoice tax return") + + assert matched_ids("invoice NOT tax") == {taxi.pk} + + def test_a_negation_inside_a_branch_still_binds( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - An "invoice" document containing "secret", one without, and + a "bill" document + WHEN: + - "(invoce AND NOT secret) OR bill" is searched + THEN: + - The secret document stays out. The old clause restated only + top-level conjunctive exclusions, so an Or branch's NOT was + never applied to the fuzzy side + """ + index_document(title="A", content="invoices secret") + clean = index_document(title="B", content="invoices only") + bill = index_document(title="C", content="bill") + + assert matched_ids("(invoce AND NOT secret) OR bill") == { + clean.pk, + bill.pk, + } + + def test_a_structured_filter_constrains_the_fuzzy_match( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - Two near-miss documents created in different years + WHEN: + - "created:2024 invoce" is searched + THEN: + - Only the 2024 one matches. The old blend restated negations + above the fuzzy clause but never positive constraints, so a + filter did not reach the fuzzy side at all. + + created is the right field to test with: it is a DATE field, + so it has no fuzzy side of its own and cannot be widened. + type: would not test this, because it is an alias for + document_type, which is one of the five default search + fields and so gets widened like any other leaf + """ + matching = index_document( + title="A", + content="invoices", + created=datetime.date(2024, 6, 1), + ) + index_document( + title="B", + content="invoices", + created=datetime.date(2023, 6, 1), + ) + + assert matched_ids("created:2024 invoce") == {matching.pk} + + def test_a_filter_on_a_non_default_field_constrains_the_fuzzy_match( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - Two near-miss documents with different storage paths + WHEN: + - "path:archive invoce" is searched + THEN: + - Only the matching one comes back. storage_path is a TEXT + field that is NOT one of the default search fields, so it + gets no fuzzy side; the release note promises this case and + the date test above does not cover it + """ + archive = StoragePath.objects.create(name="archive", path="archive/{title}") + other = StoragePath.objects.create(name="misc", path="misc/{title}") + matching = index_document( + title="A", + content="invoices", + storage_path=archive, + ) + index_document( + title="B", + content="invoices", + storage_path=other, + ) + + assert matched_ids("path:archive invoce") == {matching.pk} + + def test_a_require_filters_on_the_widened_side_too( + self, + near_matches_both: Document, + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - A document near-matching both words and one near-matching + only the scored side + WHEN: + - "invoce REQUIRE reprot" is searched + THEN: + - Only the document near-matching both comes back. The + filter-only side is unscored but still filters, and it is + widened like any other leaf, so a near miss satisfies it + """ + assert matched_ids("invoce REQUIRE reprot") == {near_matches_both.pk} + + +class TestRecallIsKept: + def test_a_typo_still_finds_its_document( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - A correctly spelled document + WHEN: + - A one-edit misspelling is searched + THEN: + - It matches. This is what the whole feature is for + """ + doc = index_document(title="A", content="invoice total") + + assert matched_ids("invoce") == {doc.pk} + + def test_a_hyphenated_term_matches_on_one_word( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - A document holding a near-miss of one half of a hyphenated + term + WHEN: + - "COVID-19" is searched + THEN: + - It matches. The words inside a term are OR'd on the fuzzy + side, which is the per-word recall the old clause had + """ + doc = index_document(title="A", content="covidx cases") + + assert matched_ids("COVID-19") == {doc.pk} + + def test_a_word_the_index_cannot_hold_does_not_narrow_the_query( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - An "invoice" document + WHEN: + - "invoice __" is searched, where __ is characters the + tokenizer discards entirely + THEN: + - It still matches. If such a word got a fuzzy alternative, + the leaf would collapse to a required clause that can never + match and the query would return less with fuzzy on than off + """ + doc = index_document(title="A", content="invoice total") + + assert matched_ids("invoice __") == {doc.pk} + + def test_a_one_character_word_no_longer_matches_everything( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - Two unrelated documents + WHEN: + - "x invoice" is searched, where x is a one-character word no + document holds + THEN: + - Nothing matches. One-character words get no fuzzy side, and + the exact side requires a term no document has. Today this + query matches the whole corpus, because a one-character + prefix fuzzy term matches every term in the field + """ + index_document(title="A", content="invoice total") + index_document(title="B", content="unrelated") + + assert matched_ids("x invoice") == set() + + +class TestCjkAndFuzzyTogether: + def test_a_cjk_term_keeps_its_bigram_side_with_fuzzy_on( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - A document with a CJK term inside an unspaced run + WHEN: + - That term is searched with fuzzy on + THEN: + - It matches through the bigram side, which sits in the same + Or as the fuzzy side and is unaffected by it + """ + doc = index_document( + title="A", + content="東京都の公共文書について", + ) + + assert matched_ids("東京") == {doc.pk} + + def test_a_negated_cjk_term_keeps_its_bigram_side( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - An "invoice" document whose only 東京 is inside a run, and + one with no 東京 at all + WHEN: + - "invoice NOT 東京" is searched with fuzzy on + THEN: + - The 東京 document is excluded. A negated leaf loses its + fuzzy side but keeps its CJK one, so the exclusion still + reaches inside the run + """ + index_document(title="A", content="invoice 東京都の報告書") + clean = index_document(title="B", content="invoice only") + + assert matched_ids("invoice NOT 東京") == {clean.pk} + + +class TestTheHookIsSkipped: + def test_a_plain_query_passes_no_hook( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + settings: SettingsWrapper, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """ + GIVEN: + - Fuzzy off and a query with no CJK in it + WHEN: + - It is parsed + THEN: + - emit() is called once, with rewrite_leaf=None. Nothing is + widened, so the query is exactly what it was before any of + this work + """ + settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = None + from documents.search import _query + + calls: list[object] = [] + real = _query.tantivy_emit + + def spy(*args: object, **kwargs: object) -> object: + calls.append(kwargs.get("rewrite_leaf")) + return real(*args, **kwargs) + + monkeypatch.setattr(_query, "tantivy_emit", spy) + index_document(title="A", content="invoice") + + matched_ids("invoice") + + assert calls == [None] + + +class TestTheEmitRegistryIsInvisible: + def test_a_non_cjk_query_gives_the_same_result_under_either_registry( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """ + GIVEN: + - A plain latin document and a non-CJK query with fuzzy on, + which now selects the registry carrying the internal bigram + fields even though nothing in the query is CJK + WHEN: + - The same query runs with that selection forced back to the + public registry + THEN: + - The same documents come back. The bigram fields are absent + from PUBLIC_FIELDS and the two registries agree on every + field a query can name, so adding them changes nothing for + a query that never reaches them + """ + from documents.search import _query + from documents.search._registry import get_field_registry + + index_document(title="A", content="invoice total") + index_document(title="B", content="unrelated") + + with_bigram_fields = matched_ids("invoce") + + monkeypatch.setattr( + _query, + "_get_emit_field_registry", + lambda language: get_field_registry(language), + ) + + assert matched_ids("invoce") == with_bigram_fields + + +class TestQuotingDoesNotWiden: + @pytest.fixture + def pks(self, index_document: Callable[..., Document]) -> dict[str, int]: + """One document per word, and one holding both.""" + return { + "tax": index_document(title="A", content="tax invoice").pk, + "report": index_document(title="B", content="report invoice").pk, + "both": index_document(title="C", content="tax report").pk, + } + + @pytest.mark.parametrize( + "query", + [ + pytest.param("tax report", id="unquoted"), + pytest.param('"tax report"', id="quoted"), + ], + ) + def test_a_quoted_phrase_needs_every_word_like_the_bare_words( + self, + matched_ids: Callable[[str], set[int]], + pks: dict[str, int], + query: str, + ) -> None: + """ + GIVEN: + - A document per word, and one holding both + WHEN: + - The words are searched unquoted and as a quoted phrase + THEN: + - Only the document holding both matches, either way. With + the phrase's words Or'd on the fuzzy side, the quoted form + matched every document, including near-misses of one word + """ + assert matched_ids(query) == {pks["both"]} + + +class TestCjkIsNotFuzzed: + def test_a_cjk_term_does_not_match_a_run_sharing_its_start( + self, + index_document: Callable[..., Document], + matched_ids: Callable[[str], set[int]], + ) -> None: + """ + GIVEN: + - A document holding 東京都 and one holding only 京都, each + inside a longer unspaced run + WHEN: + - "東京" is searched with fuzzy on + THEN: + - Only the 東京都 document matches. A prefix Fuzzy over the + whole run would match the 京都 document too, undoing what + the bigram fields' multitoken=AND guarantees with fuzzy off + """ + tokyo = index_document(title="A", content="東京都の報告書") + index_document(title="B", content="京都の観光案内について") + + assert matched_ids("東京") == {tokyo.pk} diff --git a/src/documents/tests/search/test_negated_leaf_ids.py b/src/documents/tests/search/test_negated_leaf_ids.py new file mode 100644 index 000000000..bea7069ec --- /dev/null +++ b/src/documents/tests/search/test_negated_leaf_ids.py @@ -0,0 +1,208 @@ +"""_negated_leaf_ids, the pre-scan that tells the widening hook which +leaves sit under a negation. + +A negated leaf keeps its CJK alternative but gets no fuzzy one: with +prefix matching, NOT tax would otherwise exclude "taxi" and "taxonomy", +which is not what excluding a word means. The hook cannot see a leaf's +context, so the tree is walked once up front and the hook compares by +identity. +""" + +from __future__ import annotations + +import pytest +import whoosh_compat as wc +import whoosh_compat.ast as wc_ast + +from documents.search._query import _negated_leaf_ids + +pytestmark = pytest.mark.search + +_CONTENT = wc.FieldRef("content") + + +def _term(text: str) -> wc_ast.Term: + return wc_ast.Term(field=_CONTENT, text=text) + + +class TestCollection: + def test_a_bare_tree_has_no_negated_leaves(self) -> None: + """ + GIVEN: + - A tree with no negation in it + WHEN: + - It is pre-scanned + THEN: + - The set is empty + """ + tree = wc_ast.And(children=(_term("invoice"), _term("report"))) + + assert _negated_leaf_ids(tree) == frozenset() + + def test_a_leaf_under_not_is_collected(self) -> None: + """ + GIVEN: + - A term under a Not + WHEN: + - The tree is pre-scanned + THEN: + - That leaf's id is collected, and the positive one is not + """ + positive = _term("invoice") + negated = _term("secret") + tree = wc_ast.And(children=(positive, wc_ast.Not(child=negated))) + + ids = _negated_leaf_ids(tree) + + assert id(negated) in ids + assert id(positive) not in ids + + def test_the_negative_side_of_andnot_is_collected(self) -> None: + """ + GIVEN: + - An AndNot, whose positive and negative sides are both terms + WHEN: + - The tree is pre-scanned + THEN: + - Only the negative side is collected + """ + positive = _term("invoice") + negated = _term("secret") + tree = wc_ast.AndNot(positive=positive, negative=negated) + + ids = _negated_leaf_ids(tree) + + assert id(negated) in ids + assert id(positive) not in ids + + def test_every_leaf_beneath_a_negation_is_collected(self) -> None: + """ + GIVEN: + - A Not whose child is a group of several leaves, one of them + a Phrase + WHEN: + - The tree is pre-scanned + THEN: + - All of them are collected, at any depth and whatever the + leaf type + """ + a = _term("alpha") + b = _term("beta") + phrase = wc_ast.Phrase(field=_CONTENT, text="gamma delta") + tree = wc_ast.Not( + child=wc_ast.Or( + children=(a, wc_ast.And(children=(b, phrase))), + ), + ) + + assert _negated_leaf_ids(tree) == frozenset({id(a), id(b), id(phrase)}) + + def test_a_double_negation_stays_collected(self) -> None: + """ + GIVEN: + - A term under two nested Nots + WHEN: + - The tree is pre-scanned + THEN: + - It is still collected. The rule is parity-blind on purpose: + the cost of over-collecting is a lost widening, while + under-collecting would make a negation exclude far more than + the user asked + """ + leaf = _term("tax") + tree = wc_ast.Not(child=wc_ast.Not(child=leaf)) + + assert _negated_leaf_ids(tree) == frozenset({id(leaf)}) + + @pytest.mark.parametrize( + "build", + [ + pytest.param( + lambda leaf: wc_ast.Boosted(child=leaf, boost=2.0), + id="boosted", + ), + pytest.param( + lambda leaf: wc_ast.Require(scored=leaf, filter_only=leaf), + id="require", + ), + pytest.param( + lambda leaf: wc_ast.AndMaybe(required=leaf, optional=leaf), + id="andmaybe", + ), + ], + ) + def test_containers_that_are_not_negations_collect_nothing( + self, + build, + ) -> None: + """ + GIVEN: + - A leaf inside a container that is not a negation + WHEN: + - The tree is pre-scanned + THEN: + - Nothing is collected. Only Not.child and AndNot.negative are + negative positions + """ + leaf = _term("invoice") + + assert _negated_leaf_ids(build(leaf)) == frozenset() + + def test_a_bare_leaf_is_accepted(self) -> None: + """ + GIVEN: + - A tree that is a single leaf, with no container at all + WHEN: + - It is pre-scanned + THEN: + - The set is empty and nothing raises + """ + assert _negated_leaf_ids(_term("invoice")) == frozenset() + + +class TestTotality: + @pytest.mark.parametrize( + "node", + [ + pytest.param(wc_ast.Every(), id="every"), + pytest.param(wc_ast.Nothing(), id="nothing"), + pytest.param( + wc_ast.Wildcard(field=_CONTENT, pattern="inv*"), + id="wildcard", + ), + pytest.param( + wc_ast.Fuzzy(field=_CONTENT, text="invoce", distance=1, prefix=True), + id="fuzzy", + ), + ], + ) + def test_an_unrecognised_node_does_not_raise(self, node: wc_ast.Node) -> None: + """ + GIVEN: + - A node type the walk does not collect from, including one + (Fuzzy) that this code will later build itself + WHEN: + - It is pre-scanned, bare and under a Not + THEN: + - Nothing raises. This function runs outside emit()'s error + conversion, so an exception here is an unconverted 500 + """ + assert _negated_leaf_ids(node) == frozenset() + assert _negated_leaf_ids(wc_ast.Not(child=node)) == frozenset() + + def test_a_deep_tree_does_not_exhaust_the_stack(self) -> None: + """ + GIVEN: + - A tree nested far deeper than the parser's 200-group cap + WHEN: + - It is pre-scanned + THEN: + - It completes. The walk is iterative, so depth costs heap + rather than Python stack frames + """ + leaf = _term("invoice") + node: wc_ast.Node = leaf + for _ in range(5000): + node = wc_ast.Not(child=node) + + assert _negated_leaf_ids(node) == frozenset({id(leaf)}) diff --git a/src/documents/tests/search/test_query.py b/src/documents/tests/search/test_query.py index 6cf9cb31f..aae802b85 100644 --- a/src/documents/tests/search/test_query.py +++ b/src/documents/tests/search/test_query.py @@ -87,15 +87,15 @@ class TestParseUserQuery: ) -> None: """ GIVEN: - - The fuzzy blend clause enabled (ADVANCED_FUZZY_SEARCH_THRESHOLD - set), and a query that is valid whoosh grammar tantivy's own - query parser (used only by the fuzzy blend clause) cannot parse + - Fuzzy matching enabled (ADVANCED_FUZZY_SEARCH_THRESHOLD set), + and a query using whoosh grammar the widened emit still has + to handle correctly WHEN: - parse_user_query() parses it THEN: - It returns a tantivy.Query rather than raising: the fuzzy - clause (_try_parse_fuzzy_query) must degrade gracefully - instead of failing the whole query + side must degrade gracefully instead of failing the whole + query """ settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = 0.5 assert isinstance(parse_user_query(query_index, raw_query, UTC), tantivy.Query) @@ -522,15 +522,10 @@ class TestEmitErrorContract: THEN: - The emit failure is caught and mapped to a SearchQueryError (400) rather than propagating as a raw QueryError. This - fails at the whole-tree "exact" clause emission (the - `tantivy_emit` call in parse_user_query), the same path + fails at the first widened emit (the `tantivy_emit` call + inside `emit_widened` in parse_user_query), the same path TestRealQueriesRouteCorrectly::test_text_range_is_a_400_naming_the_field - already covers without the NOT wrapper: `_negation_clauses` - never runs here, since the whole-tree emit already raises - before negations are ever computed. `_negation_clauses`'s - own except QueryError branch re-emits an already-successful - tree's own subtree in isolation, so it has no reachable - trigger under the current control flow + already covers without the NOT wrapper """ settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = 0.5 with pytest.raises(SearchQueryError): diff --git a/src/documents/tests/search/test_widen_leaf.py b/src/documents/tests/search/test_widen_leaf.py new file mode 100644 index 000000000..258960c4d --- /dev/null +++ b/src/documents/tests/search/test_widen_leaf.py @@ -0,0 +1,133 @@ +"""_widen_leaf, the hook that adds a CJK alternative, a fuzzy one, or both. + +The alternatives are independent: a leaf can qualify for either, and a +negated leaf keeps its CJK side while losing its fuzzy one. +""" + +from __future__ import annotations + +import pytest +import whoosh_compat as wc +import whoosh_compat.ast as wc_ast + +from documents.search._query import _cjk_alternative +from documents.search._query import _fuzzy_alternative +from documents.search._query import _widen_leaf + +pytestmark = pytest.mark.search + +_CONTENT = wc.FieldRef("content") +_NOTES = wc.FieldRef("notes", "note") + + +def _content(text: str) -> wc_ast.Term: + return wc_ast.Term(field=_CONTENT, text=text) + + +def _widen( + leaf: wc_ast.Term, + *, + fuzzy: bool = True, + negated: frozenset[int] = frozenset(), +) -> wc_ast.Node: + return _widen_leaf(leaf, fuzzy=fuzzy, negated=negated) + + +class TestWhichAlternativesAreAdded: + def test_a_latin_leaf_gains_only_the_fuzzy_side(self) -> None: + """ + GIVEN: + - A latin term, which has no CJK in it + WHEN: + - The hook runs with fuzzy on + THEN: + - The Or holds the leaf and the boosted fuzzy alternative, + and nothing else + """ + leaf = _content("invoice") + + assert _widen(leaf) == wc_ast.Or( + children=( + leaf, + wc_ast.Boosted(child=_fuzzy_alternative(leaf), boost=0.1), + ), + ) + + def test_a_cjk_leaf_gains_only_the_cjk_side_when_fuzzy_is_off(self) -> None: + """ + GIVEN: + - A CJK term + WHEN: + - The hook runs with fuzzy off + THEN: + - Only the bigram alternative is added, which is exactly what + the CJK work shipped + """ + leaf = _content("東京") + + assert _widen(leaf, fuzzy=False) == wc_ast.Or( + children=(leaf, _cjk_alternative(leaf)), + ) + + def test_a_cjk_leaf_gains_only_its_bigram_side(self) -> None: + """ + GIVEN: + - A CJK term + WHEN: + - The hook runs with fuzzy on + THEN: + - The Or holds the leaf and the bigram alternative, and no + fuzzy one. A prefix Fuzzy over a whole unspaced run matches + any run within one edit of its start, and _cjk_alternative + already supplies the in-run recall + """ + leaf = _content("東京") + + assert _fuzzy_alternative(leaf) is None + assert _widen(leaf) == wc_ast.Or(children=(leaf, _cjk_alternative(leaf))) + + def test_a_negated_leaf_keeps_cjk_and_loses_fuzzy(self) -> None: + """ + GIVEN: + - A CJK term whose id is in the negated set + WHEN: + - The hook runs with fuzzy on + THEN: + - The bigram alternative survives and the fuzzy one does not. + NOT X should exclude what X matches, which needs the bigram + side, but prefix fuzzy matching would exclude far more + """ + leaf = _content("東京") + + assert _widen(leaf, negated=frozenset({id(leaf)})) == wc_ast.Or( + children=(leaf, _cjk_alternative(leaf)), + ) + + def test_a_negated_latin_leaf_is_returned_unchanged(self) -> None: + """ + GIVEN: + - A latin term whose id is in the negated set + WHEN: + - The hook runs with fuzzy on + THEN: + - The leaf itself comes back. It qualifies for no alternative + at all, so there is no Or to build + """ + leaf = _content("tax") + + assert _widen(leaf, negated=frozenset({id(leaf)})) is leaf + + def test_a_leaf_outside_the_default_fields_is_returned_unchanged(self) -> None: + """ + GIVEN: + - A term on a JSON subpath field, which is not one of the five + default search fields + WHEN: + - The hook runs with fuzzy on + THEN: + - The leaf comes back untouched. Widening is scoped to the + default search fields, as it was for CJK + """ + leaf = wc_ast.Term(field=_NOTES, text="invoice") + + assert _widen(leaf) is leaf