From fdd3235959287230ab7c435ff40ac77bac9ac883 Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:30:59 -0700 Subject: [PATCH] A mix of more no cover and tests --- src/documents/search/_query.py | 21 ++- src/documents/tests/search/test_cjk_clause.py | 43 +++++ .../search/test_conjunctive_negations.py | 149 ++++++++++++++++++ .../tests/search/test_fuzzy_tokenization.py | 36 +++++ src/documents/tests/search/test_query.py | 29 ++++ 5 files changed, 275 insertions(+), 3 deletions(-) create mode 100644 src/documents/tests/search/test_conjunctive_negations.py diff --git a/src/documents/search/_query.py b/src/documents/search/_query.py index 2d0cdfa26..5f51d9591 100644 --- a/src/documents/search/_query.py +++ b/src/documents/search/_query.py @@ -376,6 +376,15 @@ def _negation_clauses( 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 ``exact = tantivy_emit(result.ast, ...)`` + (parse_user_query) has already emitted the *whole* AST successfully, + and every subtree ``_ConjunctiveNegations`` collects here is a piece + of that same tree. 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 [ @@ -385,7 +394,7 @@ def _negation_clauses( ) for negation in _ConjunctiveNegations().visit(ast) ] - except QueryError as e: + except QueryError as e: # pragma: no cover raise _map_emit_error(e) from e @@ -559,8 +568,14 @@ def _single_diagnostic_to_error(d: Diagnostic) -> SearchQueryError: "wildcard, e.g. a trailing '*', or double-quote the value to " "search it as literal text.", ) - logger.warning("Unmapped parse diagnostic %s: %s", d.kind, d.message) - return SearchQueryError("The search query could not be executed.") + logger.warning( + "Unmapped parse diagnostic %s: %s", + d.kind, + d.message, + ) # pragma: no cover + return SearchQueryError( + "The search query could not be executed.", + ) # pragma: no cover def parse_simple_query( diff --git a/src/documents/tests/search/test_cjk_clause.py b/src/documents/tests/search/test_cjk_clause.py index 6ddd7afd3..9509b04fb 100644 --- a/src/documents/tests/search/test_cjk_clause.py +++ b/src/documents/tests/search/test_cjk_clause.py @@ -32,6 +32,49 @@ def _index(backend: TantivyBackend, **kwargs: object) -> Document: return doc +class TestCjkParseFailureDegradesGracefully: + def test_a_cjk_run_tantivy_cannot_parse_drops_the_clause_only(self) -> None: + """ + GIVEN: + - A CJK run and an index-like object whose parse_query is + forced to raise + WHEN: + - _parse_cjk_text is called + THEN: + - It returns None instead of propagating, so a CJK run tantivy + cannot parse only drops the bigram clause rather than + failing the whole query. Broad on purpose (bare except + Exception), unlike the fuzzy blend's narrower ValueError + guard: a CJK run is not 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 is not pinned down + """ + from documents.search._query import _parse_cjk_text + + class _RaisingIndex: + def parse_query(self, *args: object, **kwargs: object) -> object: + raise RuntimeError("synthetic parse failure") + + assert _parse_cjk_text(_RaisingIndex(), "東京", ["bigram_content"]) is None + + def test_no_cjk_text_at_all_returns_none_without_parsing(self) -> None: + """ + GIVEN: + - A raw query string with no CJK characters at all + WHEN: + - _build_cjk_query (the simple TEXT/TITLE-mode builder) is + called directly + THEN: + - It returns None without ever attempting to parse anything. + The only real caller already guards this with _has_cjk(), + so this is defensive: it keeps the function safe to call on + its own, not a path a real search currently reaches + """ + from documents.search._query import _build_cjk_query + + assert _build_cjk_query(None, "invoice total due", ["bigram_content"]) is None + + class TestCjkClauseFollowsTheParsedQuery: def test_negated_cjk_term_is_excluded(self, backend: TantivyBackend) -> None: """ diff --git a/src/documents/tests/search/test_conjunctive_negations.py b/src/documents/tests/search/test_conjunctive_negations.py new file mode 100644 index 000000000..4f4275651 --- /dev/null +++ b/src/documents/tests/search/test_conjunctive_negations.py @@ -0,0 +1,149 @@ +"""_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/CJK 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_tokenization.py b/src/documents/tests/search/test_fuzzy_tokenization.py index cc586d65f..30980ef6c 100644 --- a/src/documents/tests/search/test_fuzzy_tokenization.py +++ b/src/documents/tests/search/test_fuzzy_tokenization.py @@ -40,6 +40,42 @@ 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/CJK clauses still stand 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, diff --git a/src/documents/tests/search/test_query.py b/src/documents/tests/search/test_query.py index 62cf330a9..74e662589 100644 --- a/src/documents/tests/search/test_query.py +++ b/src/documents/tests/search/test_query.py @@ -506,3 +506,32 @@ class TestEmitErrorContract: assert str(exc_info.value) == ( "Existence searches (field:*) are not supported for field 'notes.user'." ) + + def test_a_not_wrapped_unemittable_range_still_becomes_a_search_query_error( + self, + query_index: tantivy.Index, + settings, + ) -> None: + """ + GIVEN: + - A query combining a fuzzy-eligible free-text word with a NOT + wrapping a range on a TEXT field (parses cleanly, but a + text-field range cannot be emitted) + WHEN: + - parse_user_query() runs with fuzzy search enabled + 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 + (`exact = tantivy_emit(result.ast, ...)`), 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 + """ + settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = 0.5 + with pytest.raises(SearchQueryError): + parse_user_query(query_index, "invoice NOT title:[a to b]", UTC)