mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-08-28 05:33:24 +00:00
fix(search): apply the query's exclusions to the whole blended query
parse_user_query ORs three top-level Should clauses: the exact query, an optional fuzzy blend and an optional CJK bigram clause. The latter two are built from positive terms only and cannot express an exclusion, so each one re-admitted precisely the documents the exact clause had excluded: 'invoice NOT secret' returned the secret document as soon as ADVANCED_FUZZY_SEARCH_THRESHOLD was set, and '東京 NOT secret' returned it unconditionally, since nothing gates the CJK clause. Building the CJK clause from the AST does not fix this: there the excluded term is not the CJK one, so the clause legitimately contains 東京 and still matches the document. Hoist the exclusions instead. _ConjunctiveNegations walks the parsed tree for the subtrees that constrain every matching document, and each is emitted as an ordinary positive query attached with MustNot above the Must-ed blend. Or is not descended into: in 'invoice OR NOT secret' the negation is one branch's condition, and hoisting it would drop documents the other branch matches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
4e2d71513a
commit
352312e97d
@@ -267,6 +267,68 @@ _FIELD_BOOSTS = {"title": 2.0}
|
||||
_SIMPLE_FIELD_BOOSTS = {"simple_title": 2.0}
|
||||
|
||||
|
||||
class _ConjunctiveNegations(wc.ast.Visitor[tuple["wc.ast.Node", ...]]):
|
||||
"""Collect the subtrees an AST excludes from every document it matches.
|
||||
|
||||
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.
|
||||
|
||||
Node types with no negation to contribute (every leaf, ``Or``) fall
|
||||
through to ``generic_visit``.
|
||||
"""
|
||||
|
||||
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))
|
||||
|
||||
|
||||
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.
|
||||
"""
|
||||
try:
|
||||
return [
|
||||
(
|
||||
tantivy.Occur.MustNot,
|
||||
tantivy_emit(negation, index=index, registry=registry),
|
||||
)
|
||||
for negation in _ConjunctiveNegations().visit(ast)
|
||||
]
|
||||
except QueryError as e:
|
||||
raise _map_emit_error(e) from e
|
||||
|
||||
|
||||
def _any_of(clauses: list[tuple[tantivy.Occur, tantivy.Query]]) -> tantivy.Query:
|
||||
"""Collapse a clause list: none -> empty, one -> itself (no wasted
|
||||
single-clause boolean_query wrapping), many -> boolean_query(clauses)."""
|
||||
@@ -339,6 +401,10 @@ def parse_user_query(
|
||||
5. Optional CJK bigram clause, built from the same parsed AST for the
|
||||
same reason (see _build_ast_cjk_query): a CJK term the query negated
|
||||
or fielded must not resurface through it.
|
||||
6. When any optional 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.
|
||||
"""
|
||||
registry = get_field_registry(settings.SEARCH_LANGUAGE)
|
||||
result = wc.parse(
|
||||
@@ -377,7 +443,19 @@ def parse_user_query(
|
||||
if cjk_query is not None:
|
||||
clauses.append((tantivy.Occur.Should, cjk_query))
|
||||
|
||||
return _any_of(clauses)
|
||||
if len(clauses) == 1:
|
||||
return exact
|
||||
# The fuzzy and CJK clauses are built from positive terms only, so as
|
||||
# plain Shoulds beside the exact clause they re-admit 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, and cheaper than stripping them.
|
||||
negations = _negation_clauses(index, result.ast, registry)
|
||||
if not negations:
|
||||
return _any_of(clauses)
|
||||
return tantivy.Query.boolean_query(
|
||||
[(tantivy.Occur.Must, _any_of(clauses)), *negations],
|
||||
)
|
||||
|
||||
|
||||
# The three whoosh-compat kinds for a wildcard on a field that cannot
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
"""Negation must survive the blended query.
|
||||
|
||||
parse_user_query ORs an exact clause with optional fuzzy and CJK clauses.
|
||||
Each of those is built from positive terms only, so unless the query's
|
||||
exclusions are applied to the blend as a whole, a document the exact
|
||||
clause excluded is re-admitted by whichever other clause is enabled.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
|
||||
from documents.models import Document
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pytest_django.fixtures import SettingsWrapper
|
||||
|
||||
from documents.search._backend import TantivyBackend
|
||||
|
||||
pytestmark = [pytest.mark.search, pytest.mark.django_db]
|
||||
|
||||
|
||||
def _matched_ids(backend: TantivyBackend, query: str) -> set[int]:
|
||||
return set(backend.search_ids(query, user=None))
|
||||
|
||||
|
||||
def _index(backend: TantivyBackend, **kwargs: object) -> Document:
|
||||
doc = Document.objects.create(**kwargs)
|
||||
backend.add_or_update(doc)
|
||||
return doc
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fuzzy_enabled(settings: SettingsWrapper) -> None:
|
||||
"""Enable the fuzzy blend clause. The threshold doubles as a minimum
|
||||
score filter, so it is set to 0.0: every hit passes and the test sees
|
||||
the clause's matching behaviour, not the filter's."""
|
||||
settings.ADVANCED_FUZZY_SEARCH_THRESHOLD = 0.0
|
||||
|
||||
|
||||
class TestNegationConstrainsEveryClause:
|
||||
@pytest.mark.usefixtures("fuzzy_enabled")
|
||||
def test_fuzzy_clause_does_not_readmit_an_excluded_document(
|
||||
self,
|
||||
backend: TantivyBackend,
|
||||
) -> None:
|
||||
secret = _index(
|
||||
backend,
|
||||
title="Invoice A",
|
||||
content="invoice total secret",
|
||||
checksum="neg-fuzzy-1",
|
||||
)
|
||||
public = _index(
|
||||
backend,
|
||||
title="Invoice B",
|
||||
content="invoice total public",
|
||||
checksum="neg-fuzzy-2",
|
||||
)
|
||||
|
||||
assert _matched_ids(backend, "invoice") == {secret.pk, public.pk}
|
||||
assert _matched_ids(backend, "invoice NOT secret") == {public.pk}
|
||||
|
||||
def test_cjk_clause_does_not_readmit_an_excluded_document(
|
||||
self,
|
||||
backend: TantivyBackend,
|
||||
) -> None:
|
||||
"""The CJK clause legitimately carries 東京 here, so rebuilding it
|
||||
from the AST cannot help: only applying the exclusion above the
|
||||
blend keeps the secret document out."""
|
||||
secret = _index(
|
||||
backend,
|
||||
title="Tokyo A",
|
||||
content="東京都の秘密です secret",
|
||||
checksum="neg-cjk-1",
|
||||
)
|
||||
public = _index(
|
||||
backend,
|
||||
title="Tokyo B",
|
||||
content="東京都の報告書です public",
|
||||
checksum="neg-cjk-2",
|
||||
)
|
||||
|
||||
assert _matched_ids(backend, "東京") == {secret.pk, public.pk}
|
||||
assert _matched_ids(backend, "東京 NOT secret") == {public.pk}
|
||||
|
||||
@pytest.mark.usefixtures("fuzzy_enabled")
|
||||
def test_disjunctive_negation_still_admits_the_other_branch(
|
||||
self,
|
||||
backend: TantivyBackend,
|
||||
) -> None:
|
||||
"""'invoice OR NOT secret' excludes nothing on its own: a document
|
||||
matching the left branch stays in even though it contains secret."""
|
||||
secret_invoice = _index(
|
||||
backend,
|
||||
title="Invoice A",
|
||||
content="invoice total secret",
|
||||
checksum="neg-or-1",
|
||||
)
|
||||
unrelated = _index(
|
||||
backend,
|
||||
title="Recipe",
|
||||
content="flour and water",
|
||||
checksum="neg-or-2",
|
||||
)
|
||||
|
||||
assert _matched_ids(backend, "invoice OR NOT secret") == {
|
||||
secret_invoice.pk,
|
||||
unrelated.pk,
|
||||
}
|
||||
Reference in New Issue
Block a user