diff --git a/docs/superpowers/plans/2026-06-15-search-error-shapes-followup.md b/docs/superpowers/plans/2026-06-15-search-error-shapes-followup.md deleted file mode 100644 index 0b5173e63..000000000 --- a/docs/superpowers/plans/2026-06-15-search-error-shapes-followup.md +++ /dev/null @@ -1,790 +0,0 @@ -# Search Error Shapes Follow-up Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Replace the generic advanced-search HTTP 400 ("Error listing search results, check logs for more detail.") with three specific, user-fixable `SearchQueryError` subclasses (`UnknownFieldError`, `InvalidFieldValueError`, `MalformedQueryError`). - -**Architecture:** Two detection layers feeding the _existing_ `except SearchQueryError` handler in `UnifiedSearchViewSet.list` (no view change). (1) A **proactive** numeric-value validator inside `translate_query`'s render pass (`_translate.py`) raises `InvalidFieldValueError` before the query reaches Tantivy. (2) A **backstop** wrapper around `index.parse_query` in `parse_user_query` (`_query.py`) maps residual Tantivy `ValueError` message prefixes (`Field does not exist:`, `Syntax Error:`, `Expected a valid integer:`) into the right subclass, so nothing leaks Rust internals or hits the generic 400. - -**Tech Stack:** Python 3.11+, Django, `tantivy` (tantivy-py 0.26.0), `regex`, stdlib `difflib`, pytest + pytest-django. All commands run via `uv run` from `src/`. - -**Spec:** `docs/superpowers/specs/2026-06-15-search-error-shapes-followup-design.md` (read it first). - -**Reference facts (empirically verified 2026-06-15):** - -- Tantivy `index.parse_query` raises `ValueError` with exactly these prefixes: `Field does not exist: ''`, `Syntax Error: `, `Expected a valid integer: 'ParseIntError { kind: InvalidDigit }'`. -- `page_count:>5`, `asn:<10`, `page_count:>=5`, `asn:[1 TO 10]`, `tag_id:1,2,3` parse OK (comparison operators produce correct `RangeQuery`). -- `asn:[1 TO]` / `asn:[TO 10]` are a **Syntax Error** (open numeric ranges unsupported; only open _date_ ranges work via sentinels). -- `scan()` only tokenizes fields in `KNOWN_FIELDS`; unknown `foobar:hello` stays a `Passthrough` and only fails at `parse_query` -> detected by the backstop, not proactively. -- `difflib.get_close_matches("correspondent", pool)` -> `["correspondent"]`; `has_tags`/`http`/`12` -> `[]` (bare message). -- `tantivy.Schema` exposes no field-name list, so the drift guard is parse-based. - -## File Structure - -| File | Responsibility | Change | -| ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------ | --------------- | -| `src/documents/search/_translate.py` | Error classes, field-set constants, proactive numeric validation in `_render`, Tantivy-error mapper + hint helpers | Modify | -| `src/documents/search/_query.py` | Backstop wrapper around `index.parse_query` in `parse_user_query` | Modify | -| `src/documents/search/__init__.py` | Re-export new error classes for the view import | Modify (verify) | -| `src/documents/tests/search/test_error_shapes.py` | All unit tests for the new behavior (dedicated file per subject) | Create | -| `src/documents/tests/test_api_search.py` | One view-level 400 integration test (mirrors existing `test_search_added_invalid_date`) | Modify | - -**Test command convention:** single-file runs disable xdist: -`cd src && uv run pytest documents/tests/search/test_error_shapes.py --override-ini="addopts=" -v` - ---- - -### Task 1: Error classes and field-set constants - -**Files:** - -- Modify: `src/documents/search/_translate.py` (add `import difflib`; add constants and classes after the existing `InvalidDateQuery` class, around line 337) -- Test: `src/documents/tests/search/test_error_shapes.py` (create) - -- [ ] **Step 1: Write the failing test** - -Create `src/documents/tests/search/test_error_shapes.py`: - -```python -from __future__ import annotations - -import pytest - -from documents.search._translate import FIELD_ALIASES -from documents.search._translate import KNOWN_FIELDS -from documents.search._translate import NUMERIC_FIELDS -from documents.search._translate import SEARCHABLE_FIELDS -from documents.search._translate import InvalidFieldValueError -from documents.search._translate import MalformedQueryError -from documents.search._translate import SearchQueryError -from documents.search._translate import UnknownFieldError - - -@pytest.mark.search -class TestErrorClasses: - def test_all_subclass_search_query_error(self): - assert issubclass(UnknownFieldError, SearchQueryError) - assert issubclass(InvalidFieldValueError, SearchQueryError) - assert issubclass(MalformedQueryError, SearchQueryError) - - def test_unknown_field_message_without_suggestion(self): - err = UnknownFieldError("has_tags") - assert err.field == "has_tags" - assert err.suggestion is None - assert str(err) == "Unknown search field 'has_tags'." - - def test_unknown_field_message_with_suggestion(self): - err = UnknownFieldError("correspondent", suggestion="correspondent") - assert err.suggestion == "correspondent" - assert str(err) == ( - "Unknown search field 'corespondent'. Did you mean 'correspondent'?" - ) - - def test_invalid_field_value_message_with_field(self): - err = InvalidFieldValueError("asn", "notanumber") - assert err.field == "asn" - assert err.value == "notanumber" - assert str(err) == "Field 'asn' expects a number, got 'notanumber'." - - def test_invalid_field_value_generic_message(self): - err = InvalidFieldValueError() - assert "number" in str(err).lower() - assert "ParseIntError" not in str(err) - - def test_malformed_query_message(self): - err = MalformedQueryError("Unbalanced quote in search query.") - assert str(err) == "Unbalanced quote in search query." - - -@pytest.mark.search -class TestFieldSets: - def test_numeric_fields_are_known(self): - assert NUMERIC_FIELDS <= KNOWN_FIELDS - - def test_searchable_excludes_aliases(self): - assert SEARCHABLE_FIELDS == KNOWN_FIELDS - set(FIELD_ALIASES) - # aliases must NOT be suggestable - for alias in FIELD_ALIASES: - assert alias not in SEARCHABLE_FIELDS -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cd src && uv run pytest documents/tests/search/test_error_shapes.py --override-ini="addopts=" -v` -Expected: FAIL with `ImportError: cannot import name 'NUMERIC_FIELDS'` (and the other new names). - -- [ ] **Step 3: Write minimal implementation** - -In `src/documents/search/_translate.py`, add `import difflib` to the stdlib import group (after line 2, before `from dataclasses import dataclass`): - -```python -import difflib -``` - -Then, immediately after the `InvalidDateQuery` class (after line 336), add: - -```python -class UnknownFieldError(SearchQueryError): - """Raised when a query scopes on a field name that does not exist.""" - - def __init__(self, field: str, suggestion: str | None = None) -> None: - self.field = field - self.suggestion = suggestion - message = f"Unknown search field {field!r}." - if suggestion: - message += f" Did you mean {suggestion!r}?" - super().__init__(message) - - -class InvalidFieldValueError(SearchQueryError): - """Raised when a numeric field receives a non-numeric value.""" - - def __init__(self, field: str | None = None, value: str | None = None) -> None: - self.field = field - self.value = value - if field is not None and value is not None: - message = f"Field {field!r} expects a number, got {value!r}." - else: - message = "A numeric field in the search query received a non-numeric value." - super().__init__(message) - - -class MalformedQueryError(SearchQueryError): - """Raised for structural syntax errors (unbalanced quotes/brackets, etc.).""" -``` - -Add the field-set constants next to `KNOWN_FIELDS` (after line 92, after the `KNOWN_FIELDS` definition): - -```python -# Numeric (unsigned-int) fields. Values must be integers, optionally prefixed by -# a comparison operator (>, <, >=, <=). Validated proactively in _render. -NUMERIC_FIELDS = frozenset( - { - "asn", - "page_count", - "num_notes", - "correspondent_id", - "document_type_id", - "storage_path_id", - "tag_id", - "owner_id", - "viewer_id", - }, -) - -# Canonical user-facing field names for validation and did-you-mean suggestions. -# Aliases are excluded so a typo is never "corrected" to a deprecated alias. -SEARCHABLE_FIELDS = KNOWN_FIELDS - frozenset(FIELD_ALIASES) -``` - -Note: `SEARCHABLE_FIELDS` references `FIELD_ALIASES`, which is defined above `KNOWN_FIELDS` (line 54), so this ordering is valid. - -- [ ] **Step 4: Run test to verify it passes** - -Run: `cd src && uv run pytest documents/tests/search/test_error_shapes.py --override-ini="addopts=" -v` -Expected: PASS (all `TestErrorClasses` and `TestFieldSets` cases green). - -- [ ] **Step 5: Commit** - -```bash -git add src/documents/search/_translate.py src/documents/tests/search/test_error_shapes.py -git commit -m "feat(search): add error-shape classes and field-set constants" -``` - ---- - -### Task 2: Proactive numeric-value validation in `translate_query` - -**Files:** - -- Modify: `src/documents/search/_translate.py` (add `_validate_numeric`; hook into `_render` at lines 484-503) -- Test: `src/documents/tests/search/test_error_shapes.py` - -- [ ] **Step 1: Write the failing test** - -Append to `src/documents/tests/search/test_error_shapes.py`: - -```python -from datetime import UTC - -from documents.search._translate import translate_query - - -@pytest.mark.search -class TestProactiveNumericValidation: - @pytest.mark.parametrize( - ("query", "field", "value"), - [ - ("asn:notanumber", "asn", "notanumber"), - ("num_notes:abc", "num_notes", "abc"), - ("page_count:[foo TO bar]", "page_count", "foo"), - ("tag_id:1,foo", "tag_id", "foo"), - ], - ) - def test_non_numeric_value_raises(self, query, field, value): - with pytest.raises(InvalidFieldValueError) as exc_info: - translate_query(query, UTC) - assert exc_info.value.field == field - assert exc_info.value.value == value - - @pytest.mark.parametrize( - "query", - [ - "asn:5", - "asn:>5", - "asn:<10", - "page_count:>=5", - "page_count:<=5", - "asn:[1 TO 10]", - "tag_id:1,2,3", - "viewer_id:1,2", - "asn:[1 TO]", # open numeric range: passes the integer check here - "asn:[TO 10]", - ], - ) - def test_valid_numeric_values_do_not_raise(self, query): - # Should not raise InvalidFieldValueError. (Open numeric ranges still fail - # later at parse_query as a Syntax Error -> MalformedQueryError, but NOT - # here in the value validator.) - translate_query(query, UTC) - - def test_alias_numeric_field_validated(self): - # type_id is a numeric alias -> document_type_id; must still validate. - with pytest.raises(InvalidFieldValueError): - translate_query("type_id:abc", UTC) -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cd src && uv run pytest "documents/tests/search/test_error_shapes.py::TestProactiveNumericValidation" --override-ini="addopts=" -v` -Expected: FAIL — `test_non_numeric_value_raises` cases do not raise (values currently pass through to Tantivy unvalidated). - -- [ ] **Step 3: Write minimal implementation** - -In `src/documents/search/_translate.py`, add a module-level regex near the other operator patterns (after line 510, near `_SPACED_OP_RE`): - -```python -# Leading comparison operator on a numeric value (asn:>5, page_count:<=10). -_COMPARISON_PREFIX_RE = regex.compile(r"^(>=|<=|>|<)") -``` - -Add the validator helper (place it just above `_render`, around line 483): - -```python -def _validate_numeric(field: str, value: str) -> None: - """Raise InvalidFieldValueError if a numeric-field value is not an integer. - - Strips a single leading comparison operator (>, <, >=, <=) and surrounding - quotes first so comparison queries pass. An empty value (open range bound) - is accepted here; an open numeric bracket-range still fails downstream at - parse_query as a Syntax Error, surfaced as MalformedQueryError. - """ - candidate = _COMPARISON_PREFIX_RE.sub("", value.strip().strip("\"'")).strip() - if candidate == "": - return - if not candidate.isdigit(): - raise InvalidFieldValueError(field, value) -``` - -Modify `_render` (lines 490-502) to validate numeric fields. Replace the `FieldValueList`, `FieldValue`, and `FieldRange` branches with: - -```python - if isinstance(tok, FieldValueList): - field = FIELD_ALIASES.get(tok.field, tok.field) - if field in NUMERIC_FIELDS: - for v in tok.values: - _validate_numeric(field, v) - return " AND ".join(f"{field}:{v}" for v in tok.values) - if isinstance(tok, FieldValue): - field = FIELD_ALIASES.get(tok.field, tok.field) - if field in DATE_FIELDS: - return translate_scalar(field, tok.value, tz) - if field in NUMERIC_FIELDS: - _validate_numeric(field, tok.value) - return f"{field}:{tok.value}" - if isinstance(tok, FieldRange): - field = FIELD_ALIASES.get(tok.field, tok.field) - if field in DATE_FIELDS: - return translate_range(field, tok.lo, tok.hi, tz) - if field in NUMERIC_FIELDS: - _validate_numeric(field, tok.lo) - _validate_numeric(field, tok.hi) - return f"{field}:{tok.open}{tok.lo} TO {tok.hi}{tok.close}" -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `cd src && uv run pytest "documents/tests/search/test_error_shapes.py::TestProactiveNumericValidation" --override-ini="addopts=" -v` -Expected: PASS. - -- [ ] **Step 5: Run the full translate test file to check for regressions** - -Run: `cd src && uv run pytest documents/tests/search/test_translate.py --override-ini="addopts=" -q` -Expected: PASS (no existing translate behavior broken). - -- [ ] **Step 6: Commit** - -```bash -git add src/documents/search/_translate.py src/documents/tests/search/test_error_shapes.py -git commit -m "feat(search): proactively validate numeric field values" -``` - ---- - -### Task 3: Tantivy-error mapper and malformed-query hint - -**Files:** - -- Modify: `src/documents/search/_translate.py` (add `_suggest_field`, `_malformed_hint`, `map_tantivy_error`) -- Test: `src/documents/tests/search/test_error_shapes.py` - -- [ ] **Step 1: Write the failing test** - -Append to `src/documents/tests/search/test_error_shapes.py`: - -```python -from documents.search._translate import map_tantivy_error - - -@pytest.mark.search -class TestMapTantivyError: - def test_unknown_field_maps_with_suggestion(self): - exc = ValueError("Field does not exist: 'corespondent'") - mapped = map_tantivy_error(exc, "correspondent:foo") - assert isinstance(mapped, UnknownFieldError) - assert mapped.field == "correspondent" - assert mapped.suggestion == "correspondent" - - def test_unknown_field_maps_without_suggestion(self): - exc = ValueError("Field does not exist: 'has_tags'") - mapped = map_tantivy_error(exc, "has_tags:true") - assert isinstance(mapped, UnknownFieldError) - assert mapped.field == "has_tags" - assert mapped.suggestion is None - - def test_integer_error_maps_to_invalid_value(self): - exc = ValueError("Expected a valid integer: 'ParseIntError { kind: InvalidDigit }'") - mapped = map_tantivy_error(exc, "asn:x") - assert isinstance(mapped, InvalidFieldValueError) - assert "ParseIntError" not in str(mapped) - - @pytest.mark.parametrize( - ("raw", "fragment"), - [ - ('title:"abc', "quote"), - ("(invoice OR bill", "parenthes"), - ("created:[2020 TO 2021", "bracket"), - ("invoice AND", "AND/OR/NOT"), - ("OR invoice", "AND/OR/NOT"), - ], - ) - def test_syntax_error_maps_to_specific_hint(self, raw, fragment): - exc = ValueError(f"Syntax Error: {raw}") - mapped = map_tantivy_error(exc, raw) - assert isinstance(mapped, MalformedQueryError) - assert fragment.lower() in str(mapped).lower() - assert raw not in str(mapped) # never echo the raw query verbatim - - def test_balanced_open_numeric_range_gets_generic_hint(self): - # asn:[1 TO] is a Syntax Error but brackets ARE balanced: must NOT claim - # "unbalanced bracket". - exc = ValueError("Syntax Error: asn:[1 TO ]") - mapped = map_tantivy_error(exc, "asn:[1 TO]") - assert isinstance(mapped, MalformedQueryError) - assert "unbalanced" not in str(mapped).lower() - - def test_unrecognized_message_returns_none(self): - exc = ValueError("Some brand new tantivy error") - assert map_tantivy_error(exc, "whatever") is None -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cd src && uv run pytest "documents/tests/search/test_error_shapes.py::TestMapTantivyError" --override-ini="addopts=" -v` -Expected: FAIL with `ImportError: cannot import name 'map_tantivy_error'`. - -- [ ] **Step 3: Write minimal implementation** - -In `src/documents/search/_translate.py`, add near the other error helpers (after the `MalformedQueryError` class is fine; place all three together at the end of the error-class section): - -```python -_FIELD_MISSING_RE = regex.compile(r"^Field does not exist: '(?P[^']*)'") - -_GENERIC_MALFORMED = ( - "Could not parse the search query. Check for unbalanced quotes, brackets, " - "or parentheses, or a misplaced AND/OR/NOT operator." -) - - -def _suggest_field(field: str) -> str | None: - """Return the closest valid field name to ``field``, or None.""" - matches = difflib.get_close_matches(field, SEARCHABLE_FIELDS, n=1) - return matches[0] if matches else None - - -def _malformed_hint(raw_query: str) -> str: - """Best-effort specific hint for a structural error; generic fallback. - - Only claims a specific cause when it is structurally evident (unbalanced - delimiters or a clearly misplaced boolean operator); otherwise returns the - generic message so we never assert a wrong-but-confident cause. - """ - if raw_query.count('"') % 2 != 0: - return "Unbalanced quote in the search query." - if raw_query.count("(") != raw_query.count(")"): - return "Unbalanced parenthesis in the search query." - if ( - raw_query.count("[") != raw_query.count("]") - or raw_query.count("{") != raw_query.count("}") - ): - return "Unbalanced bracket in the search query." - upper = raw_query.strip().upper() - if upper.startswith(("AND ", "OR ")) or upper.endswith((" AND", " OR", " NOT")): - return "Misplaced AND/OR/NOT operator in the search query." - return _GENERIC_MALFORMED - - -def map_tantivy_error(exc: ValueError, raw_query: str) -> SearchQueryError | None: - """Map a tantivy parse_query ValueError to a user-safe SearchQueryError. - - Returns None when the message is not a recognised family, so the caller can - re-raise the original (preserving today's generic 400 for truly unknown - errors rather than inventing a misleading message). - """ - message = str(exc) - m = _FIELD_MISSING_RE.match(message) - if m is not None: - field = m.group("field") - return UnknownFieldError(field, _suggest_field(field)) - if message.startswith("Expected a valid integer"): - return InvalidFieldValueError() - if message.startswith("Syntax Error"): - return MalformedQueryError(_malformed_hint(raw_query)) - return None -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `cd src && uv run pytest "documents/tests/search/test_error_shapes.py::TestMapTantivyError" --override-ini="addopts=" -v` -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add src/documents/search/_translate.py src/documents/tests/search/test_error_shapes.py -git commit -m "feat(search): map tantivy parse errors to user-safe messages" -``` - ---- - -### Task 4: Backstop wrapper wired into `parse_user_query` - -**Files:** - -- Modify: `src/documents/search/_query.py` (import `map_tantivy_error`; add `_parse_query_friendly`; use it at lines 216-220 and 238-244) -- Test: `src/documents/tests/search/test_error_shapes.py` - -- [ ] **Step 1: Write the failing test** - -Append to `src/documents/tests/search/test_error_shapes.py`: - -```python -import tantivy - -from documents.search._query import parse_user_query -from documents.search._translate import SearchQueryError as _SQE # noqa: F401 - - -@pytest.mark.search -class TestBackstopViaParseUserQuery: - """Uses the module-scope ``index`` fixture from conftest.py.""" - - def test_unknown_field_raises_unknown_field_error(self, index: tantivy.Index): - with pytest.raises(UnknownFieldError) as exc_info: - parse_user_query(index, "foobar:hello", UTC) - assert exc_info.value.field == "foobar" - - def test_unknown_field_suggestion(self, index: tantivy.Index): - with pytest.raises(UnknownFieldError) as exc_info: - parse_user_query(index, "correspondent:bob", UTC) - assert exc_info.value.suggestion == "correspondent" - - def test_legacy_backend_field_is_unknown(self, index: tantivy.Index): - with pytest.raises(UnknownFieldError) as exc_info: - parse_user_query(index, "has_tags:true", UTC) - assert exc_info.value.field == "has_tags" - - @pytest.mark.parametrize( - "query", - ["(invoice OR bill", "invoice AND", "OR invoice", 'title:"abc'], - ) - def test_syntax_error_raises_malformed(self, index: tantivy.Index, query): - with pytest.raises(MalformedQueryError): - parse_user_query(index, query, UTC) - - def test_open_numeric_range_is_malformed_not_unbalanced(self, index: tantivy.Index): - with pytest.raises(MalformedQueryError) as exc_info: - parse_user_query(index, "asn:[1 TO]", UTC) - assert "unbalanced" not in str(exc_info.value).lower() - - @pytest.mark.parametrize( - "query", - ["page_count:>5", "asn:<10", "page_count:>=5", "asn:[1 TO 10]", "tag_id:1,2,3"], - ) - def test_comparison_and_range_queries_succeed(self, index: tantivy.Index, query): - assert isinstance(parse_user_query(index, query, UTC), tantivy.Query) - - @pytest.mark.parametrize( - "query", - ["notes.user:alice", "custom_fields.name:invoice"], - ) - def test_dotted_json_subfields_not_flagged(self, index: tantivy.Index, query): - assert isinstance(parse_user_query(index, query, UTC), tantivy.Query) - - def test_numeric_mismatch_raises_invalid_value(self, index: tantivy.Index): - # Proactive pass fires inside translate_query before parse_query. - with pytest.raises(InvalidFieldValueError) as exc_info: - parse_user_query(index, "asn:notanumber", UTC) - assert exc_info.value.field == "asn" -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cd src && uv run pytest "documents/tests/search/test_error_shapes.py::TestBackstopViaParseUserQuery" --override-ini="addopts=" -v` -Expected: FAIL — unknown-field/syntax cases currently raise the bare Tantivy `ValueError`, not the new subclasses (the `index.parse_query` calls are unwrapped). The numeric-mismatch and success cases may already pass. - -- [ ] **Step 3: Write minimal implementation** - -In `src/documents/search/_query.py`, add the import alongside the existing translate imports (after line 13): - -```python -from documents.search._translate import map_tantivy_error -``` - -Add a module-level helper (place it just above `parse_user_query`, before line 176): - -```python -def _parse_query_friendly( - index: tantivy.Index, - query_str: str, - raw_query: str, - default_fields: list[str], - **kwargs, -) -> tantivy.Query: - """Call index.parse_query, translating Tantivy ValueErrors into user-safe - SearchQueryError subclasses. Unrecognised errors are re-raised unchanged.""" - try: - return index.parse_query(query_str, default_fields, **kwargs) - except SearchQueryError: - raise - except ValueError as exc: - mapped = map_tantivy_error(exc, raw_query) - if mapped is not None: - raise mapped from exc - raise -``` - -In `parse_user_query`, replace the exact-query parse (lines 216-220): - -```python - exact = _parse_query_friendly( - index, - query_str, - raw_query, - DEFAULT_SEARCH_FIELDS, - field_boosts=_FIELD_BOOSTS, - ) -``` - -and the fuzzy parse (lines 238-244): - -```python - fuzzy = _parse_query_friendly( - index, - query_str, - raw_query, - DEFAULT_SEARCH_FIELDS, - field_boosts=_FIELD_BOOSTS, - # (prefix=True, distance=1, transposition_cost_one=True) — edit-distance fuzziness - fuzzy_fields={f: (True, 1, True) for f in DEFAULT_SEARCH_FIELDS}, - ) -``` - -(`SearchQueryError` is already imported in `_query.py` at line 12.) - -- [ ] **Step 4: Run test to verify it passes** - -Run: `cd src && uv run pytest "documents/tests/search/test_error_shapes.py::TestBackstopViaParseUserQuery" --override-ini="addopts=" -v` -Expected: PASS. - -- [ ] **Step 5: Run the full query test file for regressions** - -Run: `cd src && uv run pytest documents/tests/search/test_query.py --override-ini="addopts=" -q` -Expected: PASS (existing `parse_user_query` behavior, including `InvalidDateQuery` propagation, intact). - -- [ ] **Step 6: Commit** - -```bash -git add src/documents/search/_query.py src/documents/tests/search/test_error_shapes.py -git commit -m "feat(search): wrap parse_query to surface friendly error shapes" -``` - ---- - -### Task 5: Guard tests (pin prefixes + drift) and view-level 400 - -**Files:** - -- Modify: `src/documents/search/__init__.py` (verify the new error classes are exported; add if missing) -- Test: `src/documents/tests/search/test_error_shapes.py` (pin + drift guards) -- Test: `src/documents/tests/test_api_search.py` (one view-level integration test) - -- [ ] **Step 1: Verify the search package exports the new classes** - -Run: `cd src && rg -n "SearchQueryError|InvalidDateQuery|__all__" documents/search/__init__.py` - -If `SearchQueryError` is re-exported there (the view imports `from documents.search import SearchQueryError`), add the three new classes the same way. Example edit — add to the existing `from documents.search._translate import ...` block and to `__all__` if present: - -```python -from documents.search._translate import InvalidFieldValueError -from documents.search._translate import MalformedQueryError -from documents.search._translate import UnknownFieldError -``` - -(The subclasses route through the existing `except SearchQueryError` handler regardless, so exporting is for discoverability/consumers. Skip if the package does not re-export error classes.) - -- [ ] **Step 2: Write the failing pin + drift guard tests** - -Append to `src/documents/tests/search/test_error_shapes.py`: - -```python -from documents.search._query import DEFAULT_SEARCH_FIELDS -from documents.search._query import _FIELD_BOOSTS -from documents.search._translate import SEARCHABLE_FIELDS as _SEARCHABLE - - -@pytest.mark.search -class TestTantivyPinnedPrefixes: - """If a tantivy-py upgrade changes these prefixes, the backstop silently - regresses to the generic 400. Pin them so the upgrade fails loudly.""" - - def _err(self, index: tantivy.Index, raw: str) -> str: - with pytest.raises(ValueError) as exc_info: - index.parse_query(raw, DEFAULT_SEARCH_FIELDS, field_boosts=_FIELD_BOOSTS) - return str(exc_info.value) - - def test_unknown_field_prefix(self, index: tantivy.Index): - assert self._err(index, "foobar:hello").startswith("Field does not exist:") - - def test_syntax_error_prefix(self, index: tantivy.Index): - assert self._err(index, "(invoice OR bill").startswith("Syntax Error") - - def test_integer_error_prefix(self, index: tantivy.Index): - assert self._err(index, "asn:notanumber").startswith("Expected a valid integer") - - -@pytest.mark.search -class TestFieldDriftGuard: - """Every user-facing searchable field must be a real schema field. tantivy - exposes no field-name list, so we assert via parse: a real field never raises - 'Field does not exist'.""" - - @pytest.mark.parametrize("field", sorted(_SEARCHABLE)) - def test_searchable_field_exists_in_schema(self, index: tantivy.Index, field): - try: - index.parse_query( - f"{field}:1", - DEFAULT_SEARCH_FIELDS, - field_boosts=_FIELD_BOOSTS, - ) - except ValueError as exc: - # A type/syntax error proves the field EXISTS; only "does not exist" - # is a drift failure. - assert "Field does not exist" not in str(exc), ( - f"{field!r} is in SEARCHABLE_FIELDS but missing from the schema" - ) -``` - -- [ ] **Step 3: Run the guard tests to verify they pass** - -Run: `cd src && uv run pytest "documents/tests/search/test_error_shapes.py::TestTantivyPinnedPrefixes" "documents/tests/search/test_error_shapes.py::TestFieldDriftGuard" --override-ini="addopts=" -v` -Expected: PASS. (These assert current truth; they guard against future drift. If `TestFieldDriftGuard` fails now, `SEARCHABLE_FIELDS` lists a name not in the schema — fix `KNOWN_FIELDS`/`NUMERIC_FIELDS`, not the test.) - -- [ ] **Step 4: Write the failing view-level test** - -In `src/documents/tests/test_api_search.py`, locate `test_search_added_invalid_date` (around line 765) and add this test directly after it, inside the same `TestDocumentSearchApi` class (mirrors that test's structure): - -```python - def test_search_unknown_field_returns_400(self) -> None: - """ - GIVEN: - - A query scoping on a non-existent field - WHEN: - - The search API is called - THEN: - - HTTP 400 with the unknown-field message under the "query" key - """ - response = self.client.get("/api/documents/?query=foobar:hello") - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertIn("foobar", str(response.data["query"])) -``` - -- [ ] **Step 5: Run the view-level test to verify it passes** - -Run: `cd src && uv run pytest "documents/tests/test_api_search.py::TestDocumentSearchApi::test_search_unknown_field_returns_400" --override-ini="addopts=" -v` -Expected: PASS (the existing `except SearchQueryError` handler converts `UnknownFieldError` to `ValidationError({"query": [...]})`). - -- [ ] **Step 6: Commit** - -```bash -git add src/documents/search/__init__.py src/documents/tests/search/test_error_shapes.py src/documents/tests/test_api_search.py -git commit -m "test(search): pin tantivy error prefixes, guard field drift, view 400" -``` - ---- - -### Task 6: Full suite + lint - -**Files:** none (verification only) - -- [ ] **Step 1: Run the whole search test directory** - -Run: `cd src && uv run pytest documents/tests/search/ --override-ini="addopts=" -q` -Expected: PASS. - -- [ ] **Step 2: Run the API search tests** - -Run: `cd src && uv run pytest documents/tests/test_api_search.py --override-ini="addopts=" -q` -Expected: PASS. - -- [ ] **Step 3: Lint the changed files** - -Run: `cd src && uv run ruff check documents/search/_translate.py documents/search/_query.py documents/tests/search/test_error_shapes.py` -Expected: no errors (fix any import-ordering/formatting issues ruff reports; run `uv run ruff format` on the same files if needed). - -- [ ] **Step 4: Final commit (only if lint produced changes)** - -```bash -git add -A -git commit -m "chore(search): lint error-shapes follow-up" -``` - ---- - -## Self-Review - -**Spec coverage:** - -- `UnknownFieldError` (+ did-you-mean, legacy backend fields as unknown) -> Tasks 1, 3, 4. -- `InvalidFieldValueError` (proactive + backstop) -> Tasks 1, 2, 4. -- `MalformedQueryError` (balance-check, no verbatim echo, open-range caveat) -> Tasks 1, 3, 4. -- Hybrid detection (proactive scanner + backstop wrapper) -> Tasks 2, 4. -- `>`/`<` left working + validator allows operators -> Task 2 (`test_valid_numeric_values_do_not_raise`), Task 4 (`test_comparison_and_range_queries_succeed`). -- Single source of truth + drift guard -> Task 1 (`SEARCHABLE_FIELDS`), Task 5 (`TestFieldDriftGuard`). -- Message-prefix pin test -> Task 5 (`TestTantivyPinnedPrefixes`). -- Dotted-JSON / open-numeric-range / view-400 -> Tasks 4 and 5. -- Out of scope (frontend, URL search) -> correctly untouched. - -**Placeholder scan:** none — every code step shows full code and exact commands. - -**Type/name consistency:** `UnknownFieldError(field, suggestion=)`, `InvalidFieldValueError(field=None, value=None)`, `MalformedQueryError(message)`, `NUMERIC_FIELDS`, `SEARCHABLE_FIELDS`, `_validate_numeric(field, value)`, `_suggest_field(field)`, `_malformed_hint(raw_query)`, `map_tantivy_error(exc, raw_query)`, `_parse_query_friendly(index, query_str, raw_query, default_fields, **kwargs)` are used identically across all tasks. diff --git a/docs/superpowers/plans/2026-10-14-search-error-details.md b/docs/superpowers/plans/2026-10-14-search-error-details.md new file mode 100644 index 000000000..ef6c4976a --- /dev/null +++ b/docs/superpowers/plans/2026-10-14-search-error-details.md @@ -0,0 +1,1286 @@ +# Structured Advanced-Search Errors Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Every advanced-search HTTP 400 also returns, per error, its field, location, offending text and suggested fix (from whoosh-compat's `Diagnostic`), plus one server-computed corrected query, while the existing `query` message list stays byte-identical. + +**Architecture:** `SearchQueryError` gains optional `field`/`start`/`end`/`suggestion` attributes, filled from the `Diagnostic` in the two mapping functions in `_query.py`. A pure helper in `_errors.py` turns an error plus the parsed query string into the structured entries and the corrected query. A small DRF `APIException` subclass in `views.py` builds the 400 body without DRF's string coercion and replaces the two existing `ValidationError` raises. + +**Tech Stack:** Python 3.11+, Django REST Framework, whoosh-compat 0.3 (`Diagnostic`), tantivy-py 0.26, pytest + pytest-django. + +**Spec:** `docs/superpowers/specs/2026-10-14-search-error-details-design.md` (read it first). + +## Global Constraints + +- **No plan metadata anywhere in the change.** No task numbers, step numbers, "per the spec/plan", spec or plan file names, or design-section names in code, comments, docstrings, test names, test docstrings, or commit messages. Comments explain the code on its own terms. +- **No em dashes** in code, comments, docstrings, or commit messages. +- **Commit messages:** a `Feature:`-prefixed subject line, then a short paragraph on the problem and a short paragraph on the solution. No bullet lists, no conventional-commit prefixes, no `Co-Authored-By` or other attribution trailers. +- **Tests:** pytest style only (no `TestCase`), grouped in classes, every fixture param, fixture return and test signature type-annotated, GIVEN/WHEN/THEN docstring on every test, `pytest.param(..., id=...)` on every parametrize case, `mocker` rather than `unittest.mock.patch`. +- **Scope:** backend only. Do not touch `src-ui/`. A co-maintainer adapts the frontend on the same branch. +- **Messages unchanged:** `str()` of every existing `SearchQueryError` subclass stays byte-identical; the `query` key of every search 400 stays the same list of strings. +- **Lazy imports in `views.py`:** `documents.search` is only imported function-locally there (it pulls in tantivy). Keep it that way; type-only imports go under `TYPE_CHECKING`. +- **No version-defensive code:** target whoosh-compat 0.3 and DRF exactly as pinned; no `getattr`/`try` shims for other versions. +- **Lint/types:** `ruff check` and `ruff format` (the global binary, not `uv run ruff`), and `uv run pyrefly check`, clean on touched files. + +## Delegation (model tiers) + +Each task names the tier to dispatch it to. The rule used: Haiku for mechanical work where the plan carries every line; Sonnet for code with real edge cases or framework behavior to get right; Opus for the final whole-branch review, where the job is judging the change against the spec rather than writing it. + +| Task | Tier | Why | +| ------------------------------ | ------ | ----------------------------------------------------------------------------------- | +| Error model and detail helper | Sonnet | Span bounds, right-to-left splicing and overlap rules are easy to get subtly wrong. | +| Diagnostic plumbing | Sonnet | Touches every mapping branch; end-to-end tests pin library contracts. | +| 400 body in the views | Sonnet | DRF coercion behavior and the lazy-import constraint in `views.py`. | +| Branch verification | Haiku | Run commands, report output, fix only formatting. | +| Final review (after all tasks) | Opus | Whole-branch review against the spec and the Review Focus list below. | + +## Review Focus + +Inputs the spec implies that are most likely to bite, each pinned by a test in the owning task: + +- A character outside the Basic Multilingual Plane (emoji, CJK Extension B) before the error: `text` must still be the right substring. (Error model task, `test_text_after_astral_character`.) +- A decomposed (NFD) character before the error in the request: spans index the NFC-normalized query, so `text` and `suggested_query` must be computed from the normalized string, not the raw param. (Views task, `test_decomposed_input_spans_index_the_normalized_query`.) +- A query with one fixable and one unfixable error: the fix is applied and the other part is left as typed, never dropped. (Error model task, `test_unfixable_error_does_not_block_fix`; views task, `test_list_endpoint_returns_structured_errors`.) +- A search error raised where no `query` param exists (an over-long `text`, or the `more_like_id` path): no crash, entries carry `text: null` and `suggested_query` is `null`. (Views task, `test_missing_query_param_yields_null_text` and `test_over_long_text_param_yields_null_location`.) +- JSON rendering: `start`/`end` must arrive as JSON integers and absent values as JSON `null`, not the strings `"8"` and `"None"` DRF's `ValidationError` would produce. (Views task, asserted on `response.json()`.) + +## Before starting + +Branch from `dev` with a `feature-` prefix (hyphen, not slash), for example `feature-search-error-details`. That prefix makes CI build a Docker image the co-maintainer can run while adding the frontend half. + +## File Structure + +| File | Responsibility | Change | +| -------------------------------------------------- | ----------------------------------------------------------------------------------------- | ------ | +| `src/documents/search/_errors.py` | Error classes carry location and fix; `search_query_error_details` builds entries and fix | Modify | +| `src/documents/search/__init__.py` | Export `search_query_error_details` and `SearchQueryErrorDetail` | Modify | +| `src/documents/search/_query.py` | Copy `field`/span/`suggestion` from each `Diagnostic` onto the error it builds | Modify | +| `src/documents/views.py` | `SearchQueryValidationError`; the two `except SearchQueryError` sites raise it | Modify | +| `src/documents/tests/search/test_error_details.py` | Unit tests for the error attributes and the detail helper | Create | +| `src/documents/tests/search/test_error_routing.py` | Plumbing tests, including end-to-end span/suggestion pins against real parses | Modify | +| `src/documents/tests/test_api_search_errors.py` | API tests for the 400 body on the list and bulk-edit endpoints | Modify | + +--- + +### Task 1: Error model and detail helper + +**Model tier:** Sonnet + +**Files:** + +- Modify: `src/documents/search/_errors.py` +- Modify: `src/documents/search/__init__.py` +- Create: `src/documents/tests/search/test_error_details.py` + +**Interfaces:** + +- Consumes: nothing new. +- Produces: + - `SearchQueryError(message: str, *, field: str | None = None, start: int | None = None, end: int | None = None, suggestion: str | None = None)` with attributes `.field`, `.start`, `.end`, `.suggestion`. + - `InvalidDateQuery(field: str | None, value: str | None, *, start: int | None = None, end: int | None = None, suggestion: str | None = None)` and the same signature for `InvalidNumberQuery`. Both keep `.value`. + - `class SearchQueryErrorDetail(TypedDict)` with keys `message: str`, `field: str | None`, `start: int | None`, `end: int | None`, `text: str | None`, `suggestion: str | None`. + - `search_query_error_details(e: SearchQueryError, query: str) -> tuple[list[SearchQueryErrorDetail], str | None]`, exported from `documents.search`. + +- [ ] **Step 1: Write the failing tests** + +Create `src/documents/tests/search/test_error_details.py`: + +```python +from __future__ import annotations + +import pytest + +from documents.search import InvalidDateQuery +from documents.search import InvalidNumberQuery +from documents.search import MultipleSearchQueryErrors +from documents.search import QueryTooLongError +from documents.search import SearchQueryError +from documents.search import search_query_error_details +from documents.search import search_query_error_messages + +pytestmark = pytest.mark.search + + +class TestErrorLocationAttributes: + def test_base_error_defaults_to_no_location(self) -> None: + """ + GIVEN: + - A SearchQueryError built with only a message + WHEN: + - Its location attributes are read + THEN: + - field, start, end and suggestion are all None and the + message is unchanged + """ + error = SearchQueryError("The search query is nested too deeply.") + assert error.field is None + assert error.start is None + assert error.end is None + assert error.suggestion is None + assert str(error) == "The search query is nested too deeply." + + @pytest.mark.parametrize( + ("error_cls", "expected_message"), + [ + pytest.param( + InvalidDateQuery, + "Invalid date value 'december 2019' for field 'created'.", + id="date", + ), + pytest.param( + InvalidNumberQuery, + "Invalid numeric value 'december 2019' for field 'created'.", + id="number", + ), + ], + ) + def test_typed_errors_carry_location_and_keep_message( + self, + error_cls: type[InvalidDateQuery] | type[InvalidNumberQuery], + expected_message: str, + ) -> None: + """ + GIVEN: + - An InvalidDateQuery or InvalidNumberQuery built with a span + and a suggestion + WHEN: + - Its attributes and message are read + THEN: + - field, value, start, end and suggestion are set and the + message is exactly what it was before spans existed + """ + error = error_cls( + "created", + "december 2019", + start=8, + end=21, + suggestion='"december 2019"', + ) + assert error.field == "created" + assert error.value == "december 2019" + assert error.start == 8 + assert error.end == 21 + assert error.suggestion == '"december 2019"' + assert str(error) == expected_message + + +class TestSearchQueryErrorDetails: + def test_single_error_with_suggestion(self) -> None: + """ + GIVEN: + - One date error spanning "december 2019" with a quoting fix + WHEN: + - Details are built for the query it was raised from + THEN: + - One entry carries every field, and the corrected query has the + value quoted + """ + query = "created:december 2019" + error = InvalidDateQuery( + "created", + "december 2019", + start=8, + end=21, + suggestion='"december 2019"', + ) + errors, suggested = search_query_error_details(error, query) + assert errors == [ + { + "message": "Invalid date value 'december 2019' for field 'created'.", + "field": "created", + "start": 8, + "end": 21, + "text": "december 2019", + "suggestion": '"december 2019"', + }, + ] + assert suggested == 'created:"december 2019"' + + def test_two_suggestions_applied_right_to_left(self) -> None: + """ + GIVEN: + - Two errors with fixes, the second fix longer than the text it + replaces + WHEN: + - Details are built + THEN: + - Both fixes land in the right place, because splicing runs from + the end of the query backwards + """ + query = "created:may 2020 added:june 2021" + first = InvalidDateQuery( + "created", + "may 2020", + start=8, + end=16, + suggestion='"may 2020"', + ) + second = InvalidDateQuery( + "added", + "june 2021", + start=23, + end=32, + suggestion='"june 2021"', + ) + _, suggested = search_query_error_details( + MultipleSearchQueryErrors([first, second]), + query, + ) + assert suggested == 'created:"may 2020" added:"june 2021"' + + def test_unfixable_error_does_not_block_fix(self) -> None: + """ + GIVEN: + - One error with a fix and one without + WHEN: + - Details are built + THEN: + - The fix is applied and the unfixable part is left as typed + """ + query = "created:december 2019 asn:x" + errors, suggested = search_query_error_details( + MultipleSearchQueryErrors( + [ + InvalidDateQuery( + "created", + "december 2019", + start=8, + end=21, + suggestion='"december 2019"', + ), + InvalidNumberQuery("asn", "x", start=26, end=27), + ], + ), + query, + ) + assert [entry["text"] for entry in errors] == ["december 2019", "x"] + assert [entry["suggestion"] for entry in errors] == ['"december 2019"', None] + assert suggested == 'created:"december 2019" asn:x' + + @pytest.mark.parametrize( + "errors", + [ + pytest.param( + [SearchQueryError("msg", suggestion='"a b"')], + id="suggestion-without-span", + ), + pytest.param( + [SearchQueryError("msg", start=5, end=99, suggestion='"a b"')], + id="span-past-end", + ), + pytest.param( + [ + SearchQueryError("one", start=0, end=6, suggestion="x"), + SearchQueryError("two", start=4, end=9, suggestion="y"), + ], + id="overlapping-spans", + ), + ], + ) + def test_unsafe_suggestions_yield_no_corrected_query( + self, + errors: list[SearchQueryError], + ) -> None: + """ + GIVEN: + - Suggestions whose span is missing, out of range, or overlaps + another suggestion + WHEN: + - Details are built for "abcdefghij" + THEN: + - No corrected query is offered, since any splice could corrupt + the query + """ + _, suggested = search_query_error_details( + MultipleSearchQueryErrors(errors), + "abcdefghij", + ) + assert suggested is None + + def test_out_of_range_span_has_no_text(self) -> None: + """ + GIVEN: + - An error whose span runs past the end of the query + WHEN: + - Details are built + THEN: + - The entry keeps its offsets but its text is None + """ + errors, _ = search_query_error_details( + SearchQueryError("msg", start=5, end=99), + "abcdefghij", + ) + assert errors[0]["start"] == 5 + assert errors[0]["text"] is None + + def test_no_suggestions_yield_no_corrected_query(self) -> None: + """ + GIVEN: + - An error with a span but no fix + WHEN: + - Details are built + THEN: + - The entry has its text and no corrected query is offered + """ + errors, suggested = search_query_error_details( + InvalidNumberQuery("asn", "x", start=4, end=5), + "asn:x", + ) + assert errors[0]["text"] == "x" + assert suggested is None + + def test_text_after_astral_character(self) -> None: + """ + GIVEN: + - A query starting with an emoji (outside the Basic Multilingual + Plane) before the bad date, with spans counted in code points + WHEN: + - Details are built + THEN: + - text is exactly the bad value and the fix lands on it + """ + query = "\U0001f600 created:december 2019" + error = InvalidDateQuery( + "created", + "december 2019", + start=10, + end=23, + suggestion='"december 2019"', + ) + errors, suggested = search_query_error_details(error, query) + assert errors[0]["text"] == "december 2019" + assert suggested == '\U0001f600 created:"december 2019"' + + def test_query_too_long_has_null_location(self) -> None: + """ + GIVEN: + - A QueryTooLongError, which is raised before parsing + WHEN: + - Details are built + THEN: + - One entry with its message and every location field None + """ + error = QueryTooLongError(5000, 4096) + errors, suggested = search_query_error_details(error, "") + assert errors == [ + { + "message": str(error), + "field": None, + "start": None, + "end": None, + "text": None, + "suggestion": None, + }, + ] + assert suggested is None + + def test_entry_order_matches_messages(self) -> None: + """ + GIVEN: + - Several errors aggregated in one MultipleSearchQueryErrors + WHEN: + - Details and the plain message list are both built + THEN: + - Entries appear in the same order as the messages + """ + aggregate = MultipleSearchQueryErrors( + [ + InvalidNumberQuery("asn", "x", start=4, end=5), + InvalidDateQuery("created", "bad", start=14, end=17), + SearchQueryError("The search query is nested too deeply."), + ], + ) + errors, _ = search_query_error_details(aggregate, "asn:x created:bad") + assert [entry["message"] for entry in errors] == ( + search_query_error_messages(aggregate) + ) +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `uv run pytest src/documents/tests/search/test_error_details.py` +Expected: FAIL at import with `ImportError: cannot import name 'search_query_error_details'`. + +- [ ] **Step 3: Implement the error attributes and the helper** + +Replace the contents of `src/documents/search/_errors.py` with: + +```python +from __future__ import annotations + +from itertools import pairwise +from typing import TYPE_CHECKING +from typing import TypedDict + +if TYPE_CHECKING: + from collections.abc import Sequence + + +class SearchQueryError(ValueError): + """ + Base for user-fixable search query errors. + + Carries a message safe to surface to the user (no internal details). The + view layer catches this and returns an HTTP 400, so any future subclass + gets the same treatment. + + field, start and end locate the problem when it is known: start/end are + code-point offsets into the query string that was parsed. suggestion is + replacement text for that span when exactly one rewrite of the query + would fix it. + """ + + def __init__( + self, + message: str, + *, + field: str | None = None, + start: int | None = None, + end: int | None = None, + suggestion: str | None = None, + ) -> None: + self.field = field + self.start = start + self.end = end + self.suggestion = suggestion + super().__init__(message) + + +class InvalidDateQuery(SearchQueryError): + """Raised when a date field value or range bound cannot be parsed.""" + + def __init__( + self, + field: str | None, + value: str | None, + *, + start: int | None = None, + end: int | None = None, + suggestion: str | None = None, + ) -> None: + self.value = value + super().__init__( + f"Invalid date value {value!r} for field {field!r}.", + field=field, + start=start, + end=end, + suggestion=suggestion, + ) + + +class InvalidNumberQuery(SearchQueryError): + """Raised when a numeric field value or range bound cannot be parsed.""" + + def __init__( + self, + field: str | None, + value: str | None, + *, + start: int | None = None, + end: int | None = None, + suggestion: str | None = None, + ) -> None: + self.value = value + super().__init__( + f"Invalid numeric value {value!r} for field {field!r}.", + field=field, + start=start, + end=end, + suggestion=suggestion, + ) +``` + +Keep `QueryTooLongError`, `MultipleSearchQueryErrors` and `search_query_error_messages` exactly as they are (their `super().__init__(message)` calls still work), then append: + +```python +class SearchQueryErrorDetail(TypedDict): + """One user-fixable error, located in the query string that was parsed.""" + + message: str + field: str | None + start: int | None + end: int | None + text: str | None + suggestion: str | None + + +def _leaf_errors(e: SearchQueryError) -> tuple[SearchQueryError, ...]: + if isinstance(e, MultipleSearchQueryErrors): + return e.errors + return (e,) + + +def _span(e: SearchQueryError, query: str) -> tuple[int, int] | None: + """The error's (start, end) if it lies inside query, else None.""" + start, end = e.start, e.end + if start is None or end is None or not 0 <= start <= end <= len(query): + return None + return start, end + + +def _suggested_query( + errors: Sequence[SearchQueryError], + query: str, +) -> str | None: + """Apply every suggestion to query, or None if none can be applied safely. + + A suggestion with no usable span, or two whose spans overlap, means any + splice could corrupt the query, so nothing is offered at all. Errors + without a suggestion are left as typed. + """ + fixes: list[tuple[int, int, str]] = [] + for error in errors: + if error.suggestion is None: + continue + span = _span(error, query) + if span is None: + return None + fixes.append((span[0], span[1], error.suggestion)) + if not fixes: + return None + fixes.sort() + if any(later[0] < earlier[1] for earlier, later in pairwise(fixes)): + return None + fixed = query + # Right to left, so each splice leaves the spans before it valid. + for start, end, suggestion in reversed(fixes): + fixed = fixed[:start] + suggestion + fixed[end:] + return fixed + + +def search_query_error_details( + e: SearchQueryError, + query: str, +) -> tuple[list[SearchQueryErrorDetail], str | None]: + """The structured errors and corrected query for a SearchQueryError. + + query must be the exact string that was parsed (after normalization), + since that is what the offsets index. text is cut here rather than left + to clients because the offsets count code points, which JavaScript + string indexing does not. + """ + leaves = _leaf_errors(e) + details: list[SearchQueryErrorDetail] = [] + for leaf in leaves: + span = _span(leaf, query) + details.append( + { + "message": str(leaf), + "field": leaf.field, + "start": leaf.start, + "end": leaf.end, + "text": query[span[0] : span[1]] if span is not None else None, + "suggestion": leaf.suggestion, + }, + ) + return details, _suggested_query(leaves, query) +``` + +In `src/documents/search/__init__.py`, add these imports beside the other `_errors` imports: + +```python +from documents.search._errors import SearchQueryErrorDetail +from documents.search._errors import search_query_error_details +``` + +and add `"SearchQueryErrorDetail"` and `"search_query_error_details"` to `__all__`, keeping it sorted the way it is now (uppercase names first, then lowercase). + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `uv run pytest src/documents/tests/search/test_error_details.py src/documents/tests/search/test_error_routing.py` +Expected: PASS (the routing tests confirm existing messages are unchanged). + +- [ ] **Step 5: Lint** + +Run: `ruff check src/documents/search src/documents/tests/search/test_error_details.py` then `ruff format src/documents/search src/documents/tests/search/test_error_details.py` +Expected: no errors. + +- [ ] **Step 6: Commit** + +```bash +git add src/documents/search/_errors.py src/documents/search/__init__.py src/documents/tests/search/test_error_details.py +git commit -m "Feature: Locate search query errors and build a corrected query + +A SearchQueryError only carried a message, so nothing downstream could say +where in the query a problem was or offer a fix. + +SearchQueryError and its date/number subclasses now carry an optional field, +code-point span and suggested replacement. search_query_error_details turns +an error and the parsed query into one entry per error, including the +offending text, plus a corrected query built by applying every suggestion +right to left. No corrected query is offered when any suggestion's span is +missing, out of range or overlapping." +``` + +--- + +### Task 2: Diagnostic plumbing + +**Model tier:** Sonnet + +**Files:** + +- Modify: `src/documents/search/_query.py` (`_map_emit_error`, `_single_diagnostic_to_error`) +- Modify: `src/documents/tests/search/test_error_routing.py` + +**Interfaces:** + +- Consumes: `SearchQueryError`, `InvalidDateQuery`, `InvalidNumberQuery` keyword arguments `field`/`start`/`end`/`suggestion` from Task 1. +- Produces: every `SearchQueryError` raised by `parse_user_query` carries the diagnostic's `field` (as `str(FieldRef)`), `startchar`→`start`, `endchar`→`end` and `suggestion`. + +- [ ] **Step 1: Write the failing tests** + +In `src/documents/tests/search/test_error_routing.py`, add to the imports: + +```python +from documents.search._errors import InvalidDateQuery +from documents.search._errors import InvalidNumberQuery +from documents.search._errors import MultipleSearchQueryErrors +``` + +Then append these classes: + +```python +class TestDiagnosticLocationIsCopied: + """field, span and suggestion survive the Diagnostic -> error mapping.""" + + @pytest.mark.parametrize( + ("kind", "field", "field_kind", "expected_cls"), + [ + pytest.param( + DiagnosticKind.BAD_DATE, + FieldRef("created"), + FieldKind.DATE, + InvalidDateQuery, + id="bad-date", + ), + pytest.param( + DiagnosticKind.BAD_NUMBER, + FieldRef("asn"), + FieldKind.U64, + InvalidNumberQuery, + id="bad-number", + ), + pytest.param( + DiagnosticKind.PATTERN_ON_NUMERIC, + FieldRef("asn"), + FieldKind.U64, + SearchQueryError, + id="pattern-on-numeric", + ), + pytest.param( + DiagnosticKind.SINGLE_CHAR_BRACKET_RANGE, + FieldRef("title"), + FieldKind.TEXT, + SearchQueryError, + id="single-char-bracket-range", + ), + pytest.param( + DiagnosticKind.TOO_DEEP, + None, + None, + SearchQueryError, + id="too-deep-no-field", + ), + ], + ) + def test_parse_diagnostic_location_is_copied( + self, + kind: DiagnosticKind, + field: FieldRef | None, + field_kind: FieldKind | None, + expected_cls: type[SearchQueryError], + ) -> None: + """ + GIVEN: + - A parse-time Diagnostic with a span and a suggestion + WHEN: + - _single_diagnostic_to_error maps it + THEN: + - The error is the expected class and carries the canonical + field name (or None), the span and the suggestion + """ + diagnostic = Diagnostic( + kind=kind, + cause=cause_for(kind), + message=_LIBRARY_PROSE, + startchar=3, + endchar=9, + field=field, + field_kind=field_kind, + raw_value="raw", + suggestion='"raw"', + ) + error = _single_diagnostic_to_error(diagnostic) + assert type(error) is expected_cls + assert error.field == (str(field) if field is not None else None) + assert (error.start, error.end) == (3, 9) + assert error.suggestion == '"raw"' + + def test_emit_diagnostic_location_is_copied(self) -> None: + """ + GIVEN: + - An emit-time TEXT_RANGE QueryError on a JSON subpath, with a span + WHEN: + - _map_emit_error maps it + THEN: + - The 400 error carries the dotted field name and the span + """ + diagnostic = Diagnostic( + kind=DiagnosticKind.TEXT_RANGE, + cause=cause_for(DiagnosticKind.TEXT_RANGE), + message=_LIBRARY_PROSE, + startchar=0, + endchar=27, + field=FieldRef("custom_fields", "value"), + field_kind=FieldKind.JSON, + ) + error = _map_emit_error(QueryError(diagnostic)) + assert error.field == "custom_fields.value" + assert (error.start, error.end) == (0, 27) + assert error.suggestion is None + + +class TestRealQueryLocations: + """Pins the whoosh-compat span and suggestion contracts paperless relies + on, against real parses: an upgrade that changes them fails here.""" + + def test_parse_time_spans_and_suggestion( + self, + query_index: tantivy.Index, + ) -> None: + """ + GIVEN: + - A real query index + WHEN: + - parse_user_query is called with an unquoted multi-word date and + a bad number ("created:december 2019 asn:x") + THEN: + - Two errors: the date located at 8..21 with a quoting fix, the + number at 26..27 with none + """ + with pytest.raises(MultipleSearchQueryErrors) as excinfo: + parse_user_query(query_index, "created:december 2019 asn:x", UTC) + date_error, number_error = excinfo.value.errors + assert (date_error.field, date_error.start, date_error.end) == ( + "created", + 8, + 21, + ) + assert date_error.suggestion == '"december 2019"' + assert (number_error.field, number_error.start, number_error.end) == ( + "asn", + 26, + 27, + ) + assert number_error.suggestion is None + + def test_emit_time_span( + self, + query_index: tantivy.Index, + ) -> None: + """ + GIVEN: + - A real query index + WHEN: + - parse_user_query is called with a range on a text field + ("title:[a TO b]"), which only fails at emit time + THEN: + - The error names "title" and spans the range, 6..14 + """ + with pytest.raises(SearchQueryError) as excinfo: + parse_user_query(query_index, "title:[a TO b]", UTC) + error = excinfo.value + assert (error.field, error.start, error.end) == ("title", 6, 14) +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `uv run pytest src/documents/tests/search/test_error_routing.py` +Expected: the new tests FAIL (`error.field`/`error.start` are `None`); the existing tests still pass. + +- [ ] **Step 3: Copy the location in both mapping functions** + +In `src/documents/search/_query.py`, change the last line of `_map_emit_error` from `return SearchQueryError(_user_facing_emit_message(d))` to: + +```python + return SearchQueryError( + _user_facing_emit_message(d), + field=str(d.field) if d.field is not None else None, + start=d.startchar, + end=d.endchar, + suggestion=d.suggestion, + ) +``` + +Replace the body of `_single_diagnostic_to_error` with: + +```python +def _single_diagnostic_to_error(d: Diagnostic) -> SearchQueryError: + # d.field is a FieldRef, not a str: str(d.field) gives the canonical + # dotted name (an aliased query, e.g. type:, reports document_type). + field_name = str(d.field) if d.field is not None else None + if d.kind is DiagnosticKind.BAD_DATE: + return InvalidDateQuery( + field_name, + d.raw_value, + start=d.startchar, + end=d.endchar, + suggestion=d.suggestion, + ) + if d.kind is DiagnosticKind.BAD_NUMBER: + return InvalidNumberQuery( + field_name, + d.raw_value, + start=d.startchar, + end=d.endchar, + suggestion=d.suggestion, + ) + if d.kind is DiagnosticKind.TOO_DEEP: + message = "The search query is nested too deeply." + elif d.kind in _PATTERN_ON_KINDS: + kind_label = f" ({d.field_kind.name.lower()})" if d.field_kind else "" + message = ( + f"Wildcard patterns are not supported for field {field_name!r}{kind_label}." + ) + elif d.kind is DiagnosticKind.SINGLE_CHAR_BRACKET_RANGE: + field_label = f" for field {field_name!r}" if field_name else "" + message = ( + f"{d.raw_value!r} looks like a bracket range{field_label}, but " + "'[' is not a wildcard character on its own. Combine it with a " + "wildcard, e.g. a trailing '*', or double-quote the value to " + "search it as literal text." + ) + else: + logger.warning( + "Unmapped parse diagnostic %s: %s", + d.kind, + d.message, + ) # pragma: no cover + message = "The search query could not be executed." # pragma: no cover + return SearchQueryError( + message, + field=field_name, + start=d.startchar, + end=d.endchar, + suggestion=d.suggestion, + ) +``` + +The message strings must stay character-for-character identical to the current ones; only the control flow changes so every branch shares one constructor call. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `uv run pytest src/documents/tests/search/test_error_routing.py src/documents/tests/search/test_error_details.py src/documents/tests/search/test_query.py` +Expected: PASS. If `test_parse_time_spans_and_suggestion` or `test_emit_time_span` reports different offsets, stop and report the actual values rather than editing the expectations: they pin library behavior the payload depends on. + +- [ ] **Step 5: Lint** + +Run: `ruff check src/documents/search/_query.py src/documents/tests/search/test_error_routing.py` then `ruff format` on the same files. +Expected: no errors. + +- [ ] **Step 6: Commit** + +```bash +git add src/documents/search/_query.py src/documents/tests/search/test_error_routing.py +git commit -m "Feature: Carry whoosh-compat diagnostic locations onto search errors + +whoosh-compat reports a field, a character span and sometimes a one-line fix +for every diagnostic, but mapping a diagnostic to a SearchQueryError kept +only the message. + +Both the parse-time and emit-time mappings now copy the field, span and +suggestion onto the error they build. Tests against real parses pin the +offsets and the multi-word date suggestion so a whoosh-compat upgrade that +changes them fails loudly." +``` + +--- + +### Task 3: 400 body in the views + +**Model tier:** Sonnet + +**Files:** + +- Modify: `src/documents/views.py` (new class after `_get_tantivy_query_and_mode`; the `except SearchQueryError` blocks in `UnifiedSearchViewSet.list`, around line 2771, and in `DocumentSelectionMixin._get_search_document_ids`, around line 2930) +- Modify: `src/documents/tests/test_api_search_errors.py` + +**Interfaces:** + +- Consumes: `search_query_error_details`, `search_query_error_messages` from `documents.search`; `normalize_search_text` from `documents.search._query`; the error attributes from Tasks 1 and 2. +- Produces: `SearchQueryValidationError(error: SearchQueryError, raw_query: object | None)` in `documents.views`, an `APIException` with status 400 whose body is `{"query": list[str], "query_errors": list[SearchQueryErrorDetail], "suggested_query": str | None}`. + +- [ ] **Step 1: Write the failing tests** + +In `src/documents/tests/test_api_search_errors.py`, add to the runtime imports: + +```python +from typing import Any +from typing import cast + +from documents.search import InvalidDateQuery +from documents.views import _MAX_QUERY_LENGTH +from documents.views import SearchQueryValidationError +``` + +Then append: + +```python +@pytest.mark.usefixtures("searchable_document") +class TestStructuredQueryErrors: + """The 400 body locates each error and offers a corrected query, on top + of the unchanged message list.""" + + def test_list_endpoint_returns_structured_errors( + self, + admin_client: APIClient, + ) -> None: + """ + GIVEN: + - A query with an unquoted multi-word date and a bad number + WHEN: + - The document list endpoint is queried + THEN: + - A 400 whose "query" messages are unchanged, whose + "query_errors" locate both errors with JSON integers and + nulls, and whose "suggested_query" quotes only the date + """ + response = admin_client.get( + "/api/documents/", + {"query": "created:december 2019 asn:x"}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + body = response.json() + assert body["query"] == [ + "Invalid date value 'december 2019' for field 'created'.", + "Invalid numeric value 'x' for field 'asn'.", + ] + assert body["query_errors"] == [ + { + "message": "Invalid date value 'december 2019' for field 'created'.", + "field": "created", + "start": 8, + "end": 21, + "text": "december 2019", + "suggestion": '"december 2019"', + }, + { + "message": "Invalid numeric value 'x' for field 'asn'.", + "field": "asn", + "start": 26, + "end": 27, + "text": "x", + "suggestion": None, + }, + ] + assert body["suggested_query"] == 'created:"december 2019" asn:x' + + def test_unfixable_query_has_null_suggested_query( + self, + admin_client: APIClient, + ) -> None: + """ + GIVEN: + - A query whose only error has no fix ("asn:x") + WHEN: + - The document list endpoint is queried + THEN: + - suggested_query is JSON null, not the string "None" + """ + response = admin_client.get("/api/documents/", {"query": "asn:x"}) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["suggested_query"] is None + + def test_spans_index_the_query_param_alongside_text( + self, + admin_client: APIClient, + ) -> None: + """ + GIVEN: + - A plain text search combined with an advanced query filter + WHEN: + - The document list endpoint is queried + THEN: + - The error's span and text index the "query" param, not "text" + """ + response = admin_client.get( + "/api/documents/", + {"text": "invoice", "query": "created:december 2019"}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + entry = response.json()["query_errors"][0] + assert (entry["start"], entry["end"], entry["text"]) == ( + 8, + 21, + "december 2019", + ) + + def test_decomposed_input_spans_index_the_normalized_query( + self, + admin_client: APIClient, + ) -> None: + """ + GIVEN: + - A query whose first word is spelled with a decomposed accent + ("cafe" plus a combining acute), which the backend + NFC-normalizes before parsing + WHEN: + - The document list endpoint is queried with a bad date after it + THEN: + - text is the bad value and suggested_query is built from the + normalized query, so nothing is shifted by one character + """ + response = admin_client.get( + "/api/documents/", + {"query": "café created:december 2019"}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + body = response.json() + assert body["query_errors"][0]["text"] == "december 2019" + assert body["suggested_query"] == 'café created:"december 2019"' + + def test_over_long_text_param_yields_null_location( + self, + admin_client: APIClient, + ) -> None: + """ + GIVEN: + - A "text" search longer than the maximum query length, rejected + before any parsing + WHEN: + - The document list endpoint is queried + THEN: + - One error entry with every location field null and no + suggested query + """ + response = admin_client.get( + "/api/documents/", + {"text": "x" * (_MAX_QUERY_LENGTH + 1)}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + body = response.json() + assert len(body["query_errors"]) == 1 + entry = body["query_errors"][0] + assert [entry[key] for key in ("field", "start", "end", "text")] == [ + None, + None, + None, + None, + ] + assert body["suggested_query"] is None + + def test_bulk_edit_returns_structured_errors( + self, + admin_client: APIClient, + ) -> None: + """ + GIVEN: + - A bulk edit whose selection filter has an unquoted multi-word + date + WHEN: + - The bulk_edit endpoint is called + THEN: + - The 400 carries the same structured errors and corrected + query as the list endpoint + """ + response = admin_client.post( + "/api/documents/bulk_edit/", + { + "documents": [], + "all": True, + "filters": {"query": "created:december 2019"}, + "method": "set_document_type", + "parameters": {"document_type": None}, + }, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + body = response.json() + assert body["query_errors"][0]["text"] == "december 2019" + assert body["suggested_query"] == 'created:"december 2019"' + + +class TestSearchQueryValidationError: + def test_missing_query_param_yields_null_text(self) -> None: + """ + GIVEN: + - A located SearchQueryError raised where there is no "query" + param (the more_like_id path, or an over-long "text") + WHEN: + - SearchQueryValidationError is built with no raw query + THEN: + - The entry keeps its offsets, its text is None, and no + corrected query is offered + """ + error = InvalidDateQuery( + "created", + "december 2019", + start=8, + end=21, + suggestion='"december 2019"', + ) + detail = cast("dict[str, Any]", SearchQueryValidationError(error, None).detail) + assert detail["query_errors"][0]["text"] is None + assert detail["suggested_query"] is None +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `uv run pytest src/documents/tests/test_api_search_errors.py` +Expected: FAIL at import with `ImportError: cannot import name 'SearchQueryValidationError'`. + +- [ ] **Step 3: Add the exception class** + +In `src/documents/views.py`, add `from rest_framework.exceptions import APIException` beside the other `rest_framework.exceptions` imports, and add to the existing `if TYPE_CHECKING:` block (line 279): + +```python + from documents.search import SearchQueryError +``` + +`views.py` has no `from __future__ import annotations`, so the annotation below must stay a string (`"SearchQueryError"`); unquoted it raises `NameError` at import time. + +Add this class directly after `_get_tantivy_query_and_mode`: + +```python +class SearchQueryValidationError(APIException): + """HTTP 400 for a user-fixable search query error. + + "query" keeps the plain message list existing clients read. + "query_errors" locates each error in the query and "suggested_query" is + the query with every available fix applied, or None. + + The body is assigned to self.detail directly because passing it to + APIException/ValidationError runs every value through force_str, which + would turn the offsets into strings and None into "None". + """ + + status_code = status.HTTP_400_BAD_REQUEST + + def __init__(self, error: "SearchQueryError", raw_query: object | None) -> None: + from documents.search import search_query_error_details + from documents.search import search_query_error_messages + from documents.search._query import normalize_search_text + + super().__init__() + # The offsets index the query exactly as it was parsed: str() as in + # _get_tantivy_query_and_mode, then the backend's NFC normalization. + query = normalize_search_text(str(raw_query)) if raw_query is not None else "" + errors, suggested_query = search_query_error_details(error, query) + detail: dict[str, object] = { + "query": search_query_error_messages(error), + "query_errors": errors, + "suggested_query": suggested_query, + } + self.detail = detail +``` + +- [ ] **Step 4: Raise it from both call sites** + +In `UnifiedSearchViewSet.list`, replace + +```python + raise ValidationError({"query": search_query_error_messages(e)}) from e +``` + +with + +```python + raise SearchQueryValidationError(e, request.query_params.get("query")) from e +``` + +and delete the now-unused `from documents.search import search_query_error_messages` at the top of `list`. + +In `DocumentSelectionMixin._get_search_document_ids`, replace + +```python + raise ValidationError({"query": search_query_error_messages(e)}) from e +``` + +with + +```python + raise SearchQueryValidationError(e, filters.get("query")) from e +``` + +and delete the now-unused `from documents.search import search_query_error_messages` import in that method. Leave the comments above both raises as they are. + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `uv run pytest src/documents/tests/test_api_search_errors.py src/documents/tests/test_api_search.py` +Expected: PASS. `test_api_search.py` confirms the existing `"added" in str(response.data["query"])`-style assertions still hold. + +- [ ] **Step 6: Lint** + +Run: `ruff check src/documents/views.py src/documents/tests/test_api_search_errors.py` then `ruff format` on the same files. +Expected: no errors (in particular no F401 for the removed imports). + +- [ ] **Step 7: Commit** + +```bash +git add src/documents/views.py src/documents/tests/test_api_search_errors.py +git commit -m "Feature: Return located errors and a corrected query on search 400s + +A search query 400 only listed messages, so a client could not show where +the problem was or offer the fix whoosh-compat had already worked out. + +Search 400s from the document list and the selection endpoints now add +query_errors, one entry per error with its field, offsets, offending text +and suggestion, and suggested_query with every fix applied. The existing +query message list is unchanged. The body is set directly on an +APIException subclass because DRF's ValidationError would stringify the +offsets and nulls." +``` + +--- + +### Task 4: Branch verification + +**Model tier:** Haiku + +**Files:** none, unless formatting needs a fix. + +**Interfaces:** none. + +- [ ] **Step 1: Run the search test package and the touched API tests** + +Run: `uv run pytest src/documents/tests/search/ src/documents/tests/test_api_search_errors.py src/documents/tests/test_api_search.py` +Expected: PASS. Report any failure with its full output; do not change test expectations to make them pass. + +- [ ] **Step 2: Lint and format the whole change** + +Run: `ruff check src/documents/search src/documents/views.py src/documents/tests/search src/documents/tests/test_api_search_errors.py` and `ruff format --check` on the same paths. +Expected: no errors and no files to reformat. + +- [ ] **Step 3: Type check** + +Run: `uv run pyrefly check` +Expected: no new errors in `src/documents/search/_errors.py`, `src/documents/search/_query.py` or `src/documents/views.py` compared with the baseline (`.pyrefly-baseline.json`). + +- [ ] **Step 4: Commit only if formatting changed anything** + +```bash +git add -u +git commit -m "Feature: Format search error detail changes + +Formatting after the search error detail work left files that ruff would +rewrite. + +Applied ruff format; no behavior change." +``` + +Skip this step when Steps 2 and 3 were already clean. + +--- + +### Final review + +**Model tier:** Opus + +Review the whole branch against `docs/superpowers/specs/2026-10-14-search-error-details-design.md`, the Global Constraints and the Review Focus list above. Check in particular that no plan metadata or em dashes leaked into code, comments, test docstrings or commit messages, that every existing message string is unchanged, and that `views.py` still imports `documents.search` only function-locally. diff --git a/docs/superpowers/specs/2026-06-15-search-error-shapes-followup-design.md b/docs/superpowers/specs/2026-06-15-search-error-shapes-followup-design.md deleted file mode 100644 index 4448b9052..000000000 --- a/docs/superpowers/specs/2026-06-15-search-error-shapes-followup-design.md +++ /dev/null @@ -1,262 +0,0 @@ -# Friendlier advanced-search error shapes - -**Status:** design / ready for implementation plan. -**Follow-up to:** the `InvalidDateQuery` work on branch `fix/search-query-translation` -(PR #13010), itself specced in -`docs/superpowers/done/specs/2026-06-14-search-query-translation-design.md`. -**Builds on:** the `SearchQueryError(ValueError)` base in -`documents/search/_translate.py` and the single `except SearchQueryError` handler -in `UnifiedSearchViewSet.list` (`documents/views.py:2612`), which re-raises as DRF -`ValidationError({"query": [msg]})`. Any new subclass surfaces through that one -handler automatically, so this work is purely additive. - -## Problem - -Every advanced-search failure other than the now-handled invalid date lands in -the view's generic `except Exception` and returns -`HttpResponseBadRequest("Error listing search results, check logs for more -detail.")` (`views.py:2617-2621`). `index.parse_query(...)` runs _outside_ the -`translate_query` try/except in `parse_user_query` (`_query.py:220-235`), so -anything Tantivy rejects bypasses `SearchQueryError` entirely and gets the -unhelpful generic 400. Some Tantivy errors also leak Rust internals (e.g. -`ParseIntError { kind: InvalidDigit }`) if surfaced verbatim. - -## Ground truth: what Tantivy raises (empirically re-verified 2026-06-15) - -Probed against a real index built from `documents.search._schema.build_schema` + -`_tokenizer.register_tokenizers`, running each query through `translate_query` -then `index.parse_query(..., DEFAULT_SEARCH_FIELDS, field_boosts=_FIELD_BOOSTS)`. -`index.parse_query` raises `ValueError` with three distinguishable message -families: - -| Family | Example inputs | Tantivy message | -| ---------------- | ---------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------ | -| Unknown field | `foobar:hello`, `owner:5`, `has_tags:true`, `is_shared:true` | `Field does not exist: 'foobar'` | -| Syntax error | `(invoice OR bill`, `created:[2020 TO 2021`, `invoice AND`, `OR invoice`, `title:"abc`, `:value`, `AND OR` | `Syntax Error: ` | -| Wrong value type | `asn:notanumber`, `page_count:[foo TO bar]` | `Expected a valid integer: 'ParseIntError { kind: InvalidDigit }'` | - -**Parses OK (NOT errors):** `page_count:>5`, `asn:<10`, `page_count:>=5` (these -produce _correct_ Tantivy `RangeQuery` objects — see the `>`/`<` decision below), -`page_count:5` (TermQuery), `title:[a TO b]` (Str range on a text field), -`title:~~~` (EmptyQuery), `` (empty query). - -## The `>`/`<`/`>=`/`<=` decision (investigated) - -The stub flagged these as "parses OK but possibly silently wrong vs Whoosh." That -is **incorrect** — verified empirically: - -- `page_count:>5` -> `RangeQuery { lower_bound: Excluded(5), upper_bound: Unbounded }` -- `page_count:>=5` -> `RangeQuery { lower_bound: Included(5), ... }` -- `asn:<10` -> `RangeQuery { lower_bound: Unbounded, upper_bound: Excluded(10) }` - -Tantivy's query parser supports comparison operators natively and produces -correct range queries. They were _never_ supported in paperless-Whoosh (no -`GtLtPlugin`; confirmed in the base design §3 and against the old schema on -`main`), so there is no prior behavior to "match" and nothing is silently wrong. - -**Decision: leave them as-is.** They work correctly and are effectively a free -capability gain over Whoosh. Do not reject them. The numeric value validator -(below) must explicitly _allow_ a leading `>`, `<`, `>=`, or `<=` so it does not -wrongly reject working comparison queries. Document them as supported. - -Note the asymmetry (verified): the _comparison_ forms work, but _open -bracket-ranges_ on numeric fields do NOT. `asn:[1 TO]` and `asn:[TO 10]` -translate verbatim and Tantivy raises `Syntax Error` on them — only open _date_ -ranges are rewritten to bounded sentinels (`OPEN_LO`/`OPEN_HI`) by -`translate_range`. So `asn:>1` works but `asn:[1 TO]` is malformed; readers must -not assume bracket-ranges behave like the comparison operators. - -## Clarification: the old "Whoosh-only" fields were backend internals - -The stub proposed targeted "this field isn't available in full-text search; use -the filter sidebar" messages for `owner`, `has_tags`, `is_shared`, etc. Checking -the old Whoosh code still present on `main` (`src/documents/index.py`, -`make_schema`) shows these were **backend filter / permission fields, not -user-facing search syntax**: - -- `has_correspondent`, `has_tag`, `has_type`, `has_path`, `has_owner`, - `has_custom_fields`, `is_shared`, `custom_field_count`, and `owner` (username - text) were populated at index time so the permission and list-filter machinery - could query them. `get_permissions_criterias()` builds - `query.Term("has_owner", ...)`, `query.Term("owner_id", ...)`, - `query.Term("viewer_id", ...)` programmatically. -- The user-facing full-text parser (`DelayedFullTextQuery`) advertised only - `["content", "title", "correspondent", "tag", "type", "notes", -"custom_fields"]`. (Whoosh's generic `FieldsPlugin` would technically parse a - typed `has_tag:true`, but that was never a designed or documented feature.) - -**Consequence:** no curated per-field messages. These names are simply _unknown -fields_ and flow through `UnknownFieldError` + did-you-mean like any typo. Note -that `owner_id` and `viewer_id` legitimately remain queryable (present in -`KNOWN_FIELDS` and the new schema, still used for permission filtering), so they -are correctly _not_ unknown; `owner`, the `has_*` booleans, `is_shared`, and -`custom_field_count` are correctly absent and read as plain unknown fields. - -## Proposed error shapes (all `SearchQueryError` subclasses) - -All three live next to `InvalidDateQuery` in `_translate.py` and inherit the -"message is safe to surface" contract. - -### 1. `UnknownFieldError` - -`Unknown search field 'corespondent'.` plus a `Did you mean 'correspondent'?` -suggestion via `difflib.get_close_matches(field, suggestion_pool, n=1)`. The -suggestion pool is the user-facing field set derived from `KNOWN_FIELDS` (see -single-source-of-truth below). Typos get a suggestion; names with no close match -(e.g. `has_tags`) get the bare unknown-field message. - -### 2. `InvalidFieldValueError` - -Sibling of `InvalidDateQuery`. For numeric fields: `Field 'asn' expects a number, -got 'notanumber'.` Carries `field` + `value` attributes like `InvalidDateQuery`. -Removes the `ParseIntError { ... }` Rust leak. - -### 3. `MalformedQueryError` - -Structural syntax errors. A cheap balance-check pass gives specific hints for the -common cases (unbalanced `"`, `[`, `(`, dangling/leading `AND`/`OR`) before -falling back to a clean generic "check for unbalanced quotes, brackets, or -parentheses." Tantivy's message has no location and echoes the whole query, so it -is **never** surfaced verbatim. - -Caveat: an open numeric bracket-range (`asn:[1 TO]`) reaches this path as a -`Syntax Error` even though its brackets are _balanced_. The balance-check must not -confidently assert "unbalanced brackets" for it — when brackets/quotes/parens are -balanced, fall back to the generic hint rather than a wrong-but-specific one. - -## Detection strategy: hybrid, split by what each layer can see - -The scanner and the parse-wrapper see different things; assigning each error -shape to the layer that can detect it cleanly avoids false positives. - -### Proactive numeric validation in the scanner (`_translate.py`) - -`scan()` already tokenizes recognized `field:value` and `field:[range]` clauses -into `FieldValue` / `FieldRange` for fields in `KNOWN_FIELDS`. Add a -`NUMERIC_FIELDS` set (`asn`, `page_count`, `num_notes`, and the `*_id` fields) -and validate those tokens' values during translation, raising -`InvalidFieldValueError` before the string ever reaches `index.parse_query`. - -Validation rules: - -- Strip a single leading comparison operator (`>=`, `<=`, `>`, `<`) before the - integer check, so comparison queries pass. -- For ranges, validate each present bound (`lo`, `hi`) as an integer. An empty - bound passes the _integer_ check (an empty string is not a bad integer), but be - aware that an open numeric bracket-range (`asn:[1 TO]`) is still rejected - downstream by Tantivy as a `Syntax Error` (see the `>`/`<` note above) — i.e. - the validator does not make it succeed, it falls through to the malformed path. - Do not emit a "bad number" message for an empty bound; let the structural error - surface as `MalformedQueryError`. -- For multi-value numeric fields after comma expansion (`tag_id`, `viewer_id`), - validate each expanded value (`tag_id:1,foo` -> `InvalidFieldValueError`). - -This path owns `InvalidFieldValueError` exclusively: messages are rich, -context-aware, and independent of Tantivy's English strings. - -### Why unknown-field detection is NOT proactive - -`_match_field_token` returns `None` for any field not in `KNOWN_FIELDS` -(`_translate.py:193`) — an unknown `foobar:hello` is intentionally left as a -`Passthrough` (the existing `http:`-misfire guard). Detecting unknown fields in -the scanner would require a separate `\w+:` pass that re-introduces exactly the -false positives that guard exists to prevent: - -- URLs: `http://example.com/a` (`http:`) -- Dotted JSON subfields, which are valid: `notes.user:alice`, - `custom_fields.invoice_no:123` (the `\w+:` regex would see `user:` / `invoice_no:`) -- Time-like literals: `12:30` (note: a bare `12:30` is already a parse failure - today — Tantivy raises `Field does not exist: '12'` — so under this design it - reshapes into `UnknownFieldError("12")` with no close match, i.e. the bare - unknown-field message. It is _not_ a clean passthrough; the point here is only - that a proactive `\w+:` pass would mis-flag it even more aggressively.) - -So unknown fields are detected in the backstop instead, where Tantivy has already -confirmed the token is a real field reference. - -### Catch-and-sanitize backstop around `index.parse_query` (`_query.py`) - -Wrap the `index.parse_query` call(s) in `parse_user_query`. Map residual Tantivy -`ValueError` messages by prefix: - -- `Field does not exist: 'X'` -> extract `X`, build `UnknownFieldError(X)` with - did-you-mean. -- `Syntax Error: ...` -> `MalformedQueryError` (run the balance-check for a - specific hint; never echo the Tantivy text). -- `Expected a valid integer: ...` -> `InvalidFieldValueError`. This backstop is - effectively _unreachable today_: every query that produces this Tantivy error - goes through a recognized numeric field token (`asn`, `page_count`, `num_notes`, - `*_id`) that `scan()` already models, so the proactive pass catches it first - (verified — no query reaches this branch without the proactive pass firing). - Its real value is forward-safety: if a future numeric field is added to the - schema but not to `NUMERIC_FIELDS`, this branch guarantees the raw - `ParseIntError { ... }` Rust struct never leaks. Keep it; the generic "expects a - number" message stands in when `field`/`value` cannot be recovered from the - Tantivy text. -- Anything unrecognized -> re-raise, preserving today's generic-500/400 path - rather than inventing a misleading message. - -Both the fuzzy and exact `parse_query` calls go through the same wrapper. - -### Single source of truth for fields - -`KNOWN_FIELDS` (`_translate.py:63`) is the canonical set of field names a user may -validly scope on; it already includes the v2 aliases (`type`, `path`, `type_id`, -`path_id`) that `translate_query` rewrites to real schema names. Use it (with -aliases optionally excluded from the _suggestion_ pool to avoid suggesting a -deprecated alias) for both validation and did-you-mean. - -Add a drift-guard test asserting `KNOWN_FIELDS` minus the alias set is a subset of -the schema field names produced by `build_schema()`, so the two definitions cannot -silently diverge as the schema evolves. The backend-only Whoosh names (`owner`, -`has_*`, `is_shared`, `custom_field_count`) are correctly excluded from both. - -## Testing - -New dedicated test file (per project convention), e.g. -`src/documents/tests/test_search_error_shapes.py`: - -- One case per error family asserting the `SearchQueryError` subclass and a - user-safe message (no paths, no Rust structs, no verbatim Tantivy echo). -- `UnknownFieldError`: typo yields a did-you-mean suggestion; a no-close-match - name (e.g. `has_tags`) yields the bare message. -- `InvalidFieldValueError`: `asn:notanumber`, `page_count:[foo TO bar]`, and a - bad multi-value `tag_id:1,foo`. -- **`>`/`<` working case**: `page_count:>5` / `asn:<10` / `page_count:>=5` parse - successfully and are NOT raised as errors (guards the numeric validator's - operator allowance). -- **Open numeric range**: `asn:[1 TO]` / `asn:[TO 10]` surface as - `MalformedQueryError` (Tantivy `Syntax Error`, brackets balanced) and the hint - is the generic one, NOT a false "unbalanced brackets" claim. -- **Dotted JSON non-regression**: `notes.user:alice`, - `custom_fields.name:invoice` are not flagged as unknown fields. -- **URL behavior**: a query containing `http://...` is unchanged from today's - behavior — Tantivy treats `http` as a field, so it still 400s; under this design - the message becomes the clearer `UnknownFieldError('http')` (no close match) and - the proactive numeric pass does not touch it. This is a clarity gain, not a - regression (it already 400'd generically). Out of scope to make URL substrings - searchable. -- **Message-prefix pin**: a test that asserts the exact Tantivy prefixes - (`Field does not exist:`, `Syntax Error:`, `Expected a valid integer:`) the - backstop depends on, so a `tantivy-py` upgrade that changes them fails loudly - instead of silently regressing to the generic 400. -- **Drift guard**: `KNOWN_FIELDS` (minus aliases) ⊆ `build_schema()` field names. -- View-level: each subclass surfaces as HTTP 400 with `{"query": [msg]}` through - the existing handler. - -## Risks / notes - -- The backstop depends on Tantivy's error _message-string prefixes_, which are - brittle across `tantivy-py` upgrades. The pin test above is the mitigation. -- Keep all messages safe to surface: they may echo user input but must never - include internal paths, stack details, or Rust error structs. -- The balance-check for `MalformedQueryError` is a heuristic for _hints_ only; its - failure mode is the clean generic message, never a wrong-but-confident one. - -## Out of scope - -- Frontend rendering of the structured `{"query": [...]}` 400 (the inline search - error UI). Only relevant if the messages should render differently from the - current generic banner; the current banner already displays the message. -- Adding or changing `>`/`<` semantics. They work; this spec only ensures the - numeric validator does not break them. diff --git a/docs/superpowers/specs/2026-06-15-search-query-translation-design.md b/docs/superpowers/specs/2026-06-15-search-query-translation-design.md deleted file mode 100644 index 06fa4c681..000000000 --- a/docs/superpowers/specs/2026-06-15-search-query-translation-design.md +++ /dev/null @@ -1,139 +0,0 @@ -# Whoosh→Tantivy Advanced-Query Translation — Design - -**Date:** 2026-06-15 -**Branch base:** `dev` (Phase 1 implemented on `fix/search-query-translation`) -**Status:** Phase 1 implemented; Phase 2 pending an upstream `tantivy-py` release (see below) - -## Problem - -The Tantivy search migration changed the advanced-query (`?query=`) syntax contract. -A class of queries that worked under the old Whoosh backend now return an opaque -**HTTP 400**, because the old query string is handed to `tantivy.Index.parse_query` -which rejects forms Whoosh accepted. There is no fallback: a parse error propagates -through `documents/search/_backend.py` and is caught by the generic handler in -`documents/views.py` → `HttpResponseBadRequest`, with the real error only in logs. - -Affected query forms (all verified against real Whoosh and real Tantivy): - -- Bare dates on a date field: `created:2020`, `created:202003`. -- Bracketed absolute/partial/ISO ranges: `created:[20200101 TO 20201231]`, - `created:[2020-01-01 TO 2020-12-31]`, `created:[202001 TO 202006]`. -- Open-ended ranges: `created:[2020 to]`, `created:[to 2020]`. -- Relative ranges: `added:[-1 week to now]`, `added:[now-7d TO now]`. -- Comma-joined clauses and value lists: `created:[r],added:[r]`, `tag:foo,bar`, - the malformed `title:x,created:[…]`. -- Renamed fields: `type:`/`path:` (Whoosh names) vs `document_type:`/`storage_path:` - (Tantivy names). -- Invalid dates (`created:202023`) — Whoosh matched nothing (NullQuery); Tantivy 400s. - -The original compatibility layer was a stack of order-dependent regex substitutions, -which had no structural awareness (could not tell a comma inside `[...]` from a clause -separator) and required a new regex per form. This design replaces it. - -## Approach - -A structural, context-aware translation pass that intercepts only the forms Tantivy -parses differently (dates, commas, renamed fields) and passes everything else -(booleans, grouping, phrases, wildcards) straight through to Tantivy's own parser. - -Pipeline (`documents/search/_translate.py`): - -``` -parse_user_query(raw, tz) - → translate_query(raw, tz) # wrapped in a safety net: on any - → scan(raw) depth-aware tokenizer (quotes / [] depth) - → resolve_commas value-list (tag/tag_id/viewer_id) vs clause separator - → _render date tokens → translate_scalar / translate_range, - field aliasing, comma → AND - → operator normalization (spaced/trailing -/+ cleanup) - → index.parse_query(translated, …) # exception → fall back to raw query -``` - -Date math lives in `documents/search/_dates.py` (no Django deps). The two date-field -semantics are preserved: `created` is date-only (UTC-midnight boundaries); -`added`/`modified` are datetimes (local-tz-midnight → UTC). - -### Verified compatibility contract (from running both engines) - -- Comma between clauses = AND, both sides preserved; comma within a `KEYWORD(commas=True)` - field value = value list. Multi-value fields are exactly `{tag, tag_id, viewer_id}`. -- Invalid/unparsable dates → a no-match clause (never a 400), matching Whoosh's NullQuery. -- Field renames to alias: `type`→`document_type`, `type_id`→`document_type_id`, - `path`→`storage_path`, `path_id`→`storage_path_id`. Both old and new names work. -- Partial-date ranges floor the low bound and ceil the high bound; reversed ranges swap. - -## Phase 1 (implemented) - -Branch `fix/search-query-translation`. The full pipeline above, output as a Tantivy -query **string**, with these workarounds for things the string parser cannot express -on date fields: - -- **Open-ended ranges** use far-past / far-future string sentinels - (`0001-01-01T00:00:00Z` / `9999-12-31T23:59:59Z`). -- **No-match** (unparsable date) uses a degenerate equal-bound date range. - -Status: complete; the full `-m search` suite passes (date forms, comma clauses, field -aliasing, relative ranges, operator normalization, and the existing search tests now -validating the new pipeline). The old `_rewrite_*` regex helpers were left in place as -delegation shims during the transition. - -Landed in 15 commits on `fix/search-query-translation` (`f1b92a493`..`816a078a4`): -`_dates.py` extraction → partial-date helpers → scanner → comma resolution → -scalar/range translation → field aliasing → relative ranges + operator normalization → -`parse_user_query` wiring + delegation + tests. Key symbols in `_translate.py`: -`scan`, `resolve_commas`, `translate_scalar`, `translate_range`, `translate_query`; -constants `MULTI_VALUE_FIELDS`, `DATE_FIELDS`, `KNOWN_FIELDS`, `FIELD_ALIASES`, -`NO_MATCH`, `OPEN_LO`, `OPEN_HI`. - -## Phase 2 (pending — the thing being tracked) - -Replace the Phase-1 string workarounds with real `tantivy.Query` objects for date -clauses, which removes the sentinel/degenerate-range hacks entirely: - -1. **Open bounds** via `Query.range_query(field, FieldType.Date, lower_bound=…, -upper_bound=None)` (and vice-versa) instead of `OPEN_LO`/`OPEN_HI` sentinels. -2. **No-match** via `Query.empty_query()` instead of the degenerate range. This also - fixes the cosmetic issue that the no-match sentinel always names the `created` field. -3. **Retire the dead code**: remove the now-unused `_rewrite_*` helpers and the - `rewrite_natural_date_keywords` / `normalize_query` delegation shims in `_query.py` - (~160 lines left from the Phase-1 transition), and migrate their string-output tests - in `test_query.py` (replace the direct `_rewrite_compact_date` test with a - `translate_scalar` test). - -### Blocker - -Phase 2 is **gated on a published `tantivy-py` release**, not on any further code -contribution. In `tantivy-py 0.26.0` (our current pin: `tantivy~=0.26.0` in -`pyproject.toml`, released 2026-04-29), `range_query`/`term_query` **reject Python -`datetime` values on `Date` fields** (`Expected DateTime type for field …`), so date -Query objects cannot be built from Python. The fix is already merged on `tantivy-py` -`master` across two PRs: - -- **#655** — `feat: support unbounded range queries via None bounds`. -- **#666** — `fix: add_date loses tzinfo` (adds the `PyDateTime → tantivy DateTime` - converter and routes `range_query`/`term_query` through it). - -Both postdate the `0.26.0` wheel. - -- **Trigger:** a `tantivy-py` release `> 0.26.0` containing #655 + #666 is published to PyPI. -- **Action:** bump the `tantivy-py` pin, then do items 1–3 above. - -## Out of scope - -- Unknown-field handling: Whoosh degraded an unknown `field:` to text; Tantivy 400s - (`http://x/a,b` → `Field does not exist: 'http'`). Not a date/comma/rename issue. -- Whoosh fields with no Tantivy equivalent: `owner` (text), the `has_*` presence - booleans, `is_shared`, `custom_field_count`, `custom_fields_id`. -- `>`/`<`/`>=`/`<=` comparisons — never supported in paperless-Whoosh (no `GtLtPlugin`). - -## Reference / how to re-verify - -- Tantivy side (does a translated string parse?): build a real index via - `documents.search._schema.build_schema` + `register_tokenizers`, then - `index.parse_query(translate_query(q, tz), DEFAULT_SEARCH_FIELDS, field_boosts=…)`. -- Whoosh side (what did v2 do?): `src/documents/index.py` (the old `get_schema()` + - `MultifieldParser([...]) + DateParserPlugin(...)`) was deleted from `main` in - `aed9abe48` (#12471, 2026-04-02); check it out at `git show aed9abe48^:src/documents/index.py` - (or `git checkout aed9abe48^ -- src/documents/index.py`) and run a query through it - to get the ground-truth `Query`. -- A fuller empirical gap matrix lives in `SEARCH_TANTIVY_WHOOSH_COMPAT.md`. diff --git a/docs/superpowers/specs/2026-10-14-search-error-details-design.md b/docs/superpowers/specs/2026-10-14-search-error-details-design.md new file mode 100644 index 000000000..1df3f3682 --- /dev/null +++ b/docs/superpowers/specs/2026-10-14-search-error-details-design.md @@ -0,0 +1,355 @@ +# Structured advanced-search errors from whoosh-compat diagnostics + +**Status:** design / ready for implementation plan. +**Builds on:** advanced search parsed by whoosh-compat (#14072) and the search +field registry (#14071). Line numbers below refer to `dev` at `34a6c718f`. +**Scope:** backend. The frontend work lands on the same branch from a +co-maintainer; see "Frontend compatibility" and "Consumer notes". + +## Current behavior + +Advanced search does not use Tantivy's query-string parser. +`parse_user_query` (`documents/search/_query.py`) calls `wc.parse()` against the +`FieldRegistry` built from `PUBLIC_FIELDS`, maps any `Diagnostic`s to +`SearchQueryError` subclasses, then emits a `tantivy.Query` directly from the +AST. Probing the real `parse_user_query` on `dev` (whoosh-compat 0.3, tantivy +0.26) gives: + +| Input | Result today | +| --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------ | +| `foobar:hello`, `correspondent:bob`, `has_tags:true`, `owner_id:1`, `http://x`, `12:30` | No error. Unknown field prefixes are demoted to text and searched literally (documented in `docs/usage.md`). | +| `(invoice OR bill`, `invoice AND`, `OR invoice`, `title:"abc` | No error. The whoosh grammar is lenient and never fails on structure. | +| `asn:notanumber`, `page_count:[foo TO bar]`, `asn:1,2` | 400, `InvalidNumberQuery`. | +| `page_count:>5`, `asn:<10` | 400, `InvalidNumberQuery` (comparison operators are not whoosh syntax). | +| `asn:[1 TO]`, `asn:[TO 10]` | Works (open numeric ranges). | +| `created:notadate`, `created:[2020 TO 2021` | 400, `InvalidDateQuery`. | +| `title:[a TO b]`, `notes:*`, `asn:*x` | 400, plain `SearchQueryError` with a kind-specific message. | +| `asn:5 created:bad page_count:x` | 400, `MultipleSearchQueryErrors`, one message per bad value. | + +Every user-fixable failure already arrives as a `SearchQueryError` with a safe +message. + +## Problem + +whoosh-compat's `Diagnostic` carries more than paperless uses. Besides `kind` +and `raw_value`, each record has: + +- `field`: set on parse-time diagnostics and on every emit-time one, but + paperless only keeps it on `InvalidDateQuery`/`InvalidNumberQuery`. +- `startchar` / `endchar`: where in the query string the problem is. Set on + parse-time diagnostics and on emit-time ones (the emitter copies the node's + span, `emitters/tantivy_.py:732-733`; `_widen_leaf`, `_cjk_alternative` and + `_fuzzy_alternative` keep the original leaf span via `_leaf_span`). +- `suggestion`: replacement text for that span when exactly one rewrite of the + query would work. The library's documented host contract is to splice it in + rather than re-derive the rule: + `fixed = q[:startchar] + suggestion + q[endchar:]`. + In 0.3 the only case that carries one is an unquoted multi-word date + (`created:december 2019` suggests `"december 2019"`); `created:20231340`, + number errors and every emit-time diagnostic carry none. + +Paperless throws most of that away. `_single_diagnostic_to_error` and +`_map_emit_error` keep only a message, and both views raise +`ValidationError({"query": [msg, ...]})`. A client can tell the user _what_ is +wrong but not _where_, and cannot offer the fix the library already computed. + +Probed spans for reference: + +``` +created:december 2019 BAD_DATE 8..21 suggestion '"december 2019"' +created:december 2019 asn:x BAD_DATE 8..21 suggestion '"december 2019"' + BAD_NUMBER 26..27 suggestion None +created:20231340 BAD_DATE 8..16 suggestion None +😀 created:december 2019 BAD_DATE 10..23 (code points; 11..24 in UTF-16) +``` + +## Goals + +- Every search-query 400 also carries, per error, its field, location, the + offending text and the suggested replacement, in a typed shape a client can + rely on. +- When suggestions exist, the server returns one corrected query string, so no + client has to do span arithmetic. +- Additive only, with no new API version: the `query` key keeps exactly today's + strings. +- whoosh-compat types stay private to `_query.py`. The public payload is + paperless's own contract, not the library's enum names. + +## Non-goals + +- New suggestions. Fixes come only from `Diagnostic.suggestion`; paperless adds + no rewrite rules of its own. New library suggestions (for example for `>5`) + appear in the payload with a whoosh-compat upgrade and no paperless change. +- Changes to whoosh-compat. +- Hints for demoted unknown fields (`correspondent:bob`). `Diagnostic` is + fatal-only by library contract, and the documented paperless behavior is to + search such text literally. +- Frontend changes. They are done on the same branch by a co-maintainer (see + "Frontend compatibility" and "Consumer notes"). +- A live query-validation endpoint (see "Future work"). +- The `corrected_query: null` key on successful search responses + (`views.py:2744`), a leftover of Whoosh spelling correction. Untouched. +- An OpenAPI schema for the 400 body. The search list endpoint documents none + today. + +## Design + +### Error model (`documents/search/_errors.py`) + +`SearchQueryError` gains four keyword-only, optional attributes: + +```python +class SearchQueryError(ValueError): + def __init__( + self, + message: str, + *, + field: str | None = None, + start: int | None = None, + end: int | None = None, + suggestion: str | None = None, + ) -> None: + self.field = field + self.start = start + self.end = end + self.suggestion = suggestion + super().__init__(message) +``` + +`SearchQueryError("msg")` keeps working. `InvalidDateQuery` and +`InvalidNumberQuery` keep their positional `(field, value)` signature, pass +`field` up to the base, and accept and forward `start`, `end` and `suggestion`. +`QueryTooLongError` and `MultipleSearchQueryErrors` leave all four `None`: the +first has no span, and the second holds sub-errors that each carry their own. + +Messages are unchanged byte for byte. The fix is never folded into `str(e)`. + +### Diagnostic plumbing (`documents/search/_query.py`) + +- `_single_diagnostic_to_error(d)` passes `field=str(d.field)` (when set), + `start=d.startchar`, `end=d.endchar` and `suggestion=d.suggestion` to + whichever error it builds, including the plain `SearchQueryError` branches + (TOO_DEEP, PATTERN_ON_*, SINGLE_CHAR_BRACKET_RANGE) and the unmapped + fallback. +- `_map_emit_error(e)` does the same for the 400 it returns. Emit-time + diagnostics never carry a suggestion in 0.3 (the emitter's `_fail` takes + none); it is copied anyway so a future one flows through. The paths that + re-raise (INTERNAL, and MISCONFIGURED other than EXISTS_REQUIRES_FAST) are + unchanged. + +`field` is the canonical dotted name `str(FieldRef)` produces, as messages +already use (`type:` reports `document_type`, `notes.user:` reports +`notes.user`). + +### Detail helper (`documents/search/_errors.py`) + +Next to `search_query_error_messages`, and exported from `documents.search`: + +```python +def search_query_error_details( + e: SearchQueryError, + query: str, +) -> tuple[list[dict[str, object]], str | None]: + """(errors, suggested_query) for a SearchQueryError raised parsing ``query``.""" +``` + +- `errors` has one entry per leaf error, flattening `MultipleSearchQueryErrors` + in the same order as the messages helper: + + ``` + {"message": str, "field": str | None, "start": int | None, + "end": int | None, "text": str | None, "suggestion": str | None} + ``` + + `start`/`end` are code-point offsets into the normalized query (see + "Which string the spans index"). `text` is `query[start:end]`, so a client + never has to slice by offset itself; JavaScript strings index in UTF-16 units + and would get astral characters (emoji, CJK Extension B) wrong. `text` is + `None` when the error has no span or the span falls outside `query`. + +- `suggested_query` applies every non-`None` suggestion to `query` right to + left, so earlier spans stay valid. It is `None` when no error has a + suggestion, or when any suggestion lacks a span, has a span outside `query`, + or overlaps another suggestion's span. A wrong fix is worse than none. +- Errors without a suggestion do not block the others: in + `created:december 2019 asn:x` the date is quoted and `asn:x` is left as typed. + +`query` must be the exact string that was parsed. See the next section. + +### Which string the spans index + +`wc.parse` has one call site (`_query.py`, in `parse_user_query`), reached only +through `TantivyBackend._parse_query`. In every search mode the only string it +parses is the `query` request parameter: `_get_tantivy_query_and_mode` +(`views.py:324`) uses it as the search itself in `QUERY` mode and as +`filter_query` alongside `text`/`title_search`. The simple modes never produce +diagnostics. The backend NFC-normalizes before parsing (`_backend.py:485`), so +spans index `normalize_search_text(str(params["query"]))`, written with `str()` +to match `_get_tantivy_query_and_mode` (bulk `filters` values are not guaranteed +to be strings). The views pass that string to the helper. For the NFC input +browsers normally send, this is identical to what the user typed. + +### Response and views (`documents/views.py`) + +A search-query 400 becomes: + +```json +{ + "query": [ + "Invalid date value 'december 2019' for field 'created'.", + "Invalid numeric value 'x' for field 'asn'." + ], + "query_errors": [ + { + "message": "Invalid date value 'december 2019' for field 'created'.", + "field": "created", + "start": 8, + "end": 21, + "text": "december 2019", + "suggestion": "\"december 2019\"" + }, + { + "message": "Invalid numeric value 'x' for field 'asn'.", + "field": "asn", + "start": 26, + "end": 27, + "text": "x", + "suggestion": null + } + ], + "suggested_query": "created:\"december 2019\" asn:x" +} +``` + +- `query`: unchanged. +- `query_errors`: always present on a search-query 400. +- `suggested_query`: string or `null`, always present. +- `QueryTooLongError` (raised in `_get_tantivy_query_and_mode` before parsing, + for an over-long `text`, `title_search` or `query`): one entry with `field`, + `start`, `end`, `text` and `suggestion` all `null`, and + `suggested_query: null`. +- A `SearchQueryError` on the `more_like_id` path (both try blocks catch it + there too) gets the same shape with `query` treated as `""`, so every entry + has `text: null`. + +DRF's `ValidationError` cannot carry this. `_get_error_details` runs every leaf +through `force_str` (`rest_framework/exceptions.py:39`), so `start: 8` would +serialize as `"8"` and `None` as `"None"`. Add a small `APIException` subclass: + +```python +class SearchQueryValidationError(APIException): + status_code = status.HTTP_400_BAD_REQUEST + + def __init__(self, e: SearchQueryError, query: str) -> None: + errors, suggested = search_query_error_details(e, query) + detail: dict[str, object] = { + "query": search_query_error_messages(e), + "query_errors": errors, + "suggested_query": suggested, + } + # Assigned directly, not via super().__init__(detail), so DRF does not + # stringify the ints and nulls. + self.detail = detail +``` + +DRF's default `exception_handler` returns a dict `detail` as the response body +unchanged, and paperless configures no custom `EXCEPTION_HANDLER`. The `query` +list serializes to the same JSON as today's `ValidationError` (`ErrorDetail` is +a `str` subclass). Where the class lives (`documents/views.py` beside its two +users, or a small module the views import) is a plan-level choice. + +Both call sites switch to it, each a one-line change with no signature changes: + +- the document search list endpoint (`views.py:2771`); +- `_get_search_document_ids` in `DocumentSelectionMixin` (`views.py:2930`), + reached through `_resolve_document_ids` by bulk edit, bulk download and the + other selection endpoints. It raises from inside a helper, which is why this + is an exception rather than a returned `Response`. It already has `filters`, + the params dict to read `query` from. + +### Frontend compatibility + +The current frontend does not read `query` specifically. Its error handler +(`document-list-view.service.ts:391-403`) loops over every key of a 400 body and +prints `key: value[0]`. Against the new body it would print +`query_errors: [object Object]` and the first character of `suggested_query`, +and on `suggested_query: null` it throws a `TypeError` inside the handler, so +the banner never appears. + +The backend change is therefore not merged on its own. The branch is shared +with a co-maintainer who adapts that handler (at minimum, stop treating the two +new keys as generic field errors) and builds the UI from "Consumer notes" +before the branch merges. + +## Consumer notes (for the frontend, not implemented here) + +What the payload enables, so the UI work can be scheduled with its owner: + +- With `MultipleSearchQueryErrors` only the first message reaches the user + today (`fieldError[0]`). The UI can list every `query_errors[].message`. +- Show the offending part with `query_errors[].text`. Do not slice the query + with `start`/`end` in JavaScript: they are code-point offsets and JS strings + index in UTF-16 units. +- When `suggested_query` is set, offer "Search for `` + instead", which replaces the `FILTER_FULLTEXT_QUERY` rule value and calls + `setFilterRules(rules, true)`. +- Global search (`/api/search/`) runs `SearchMode.TEXT` (`views.py:3829`), + never produces diagnostics, and is unaffected. + +## Testing + +Pytest-style, grouped in classes, with GIVEN/WHEN/THEN docstrings and +`pytest.param` ids, marked `search` (and `django_db` where needed). + +- **Helper unit tests** (new `src/documents/tests/search/test_error_details.py`), + built from hand-constructed errors: + - single error with a suggestion: entry fields, `text`, and spliced + `suggested_query`; + - two suggestions in one query: both applied, right to left; + - one fixable and one unfixable error: only the fixable span changes; + - missing span, out-of-range span, overlapping spans: `suggested_query` is + `None` and out-of-range `text` is `None`; + - no suggestions: `suggested_query` is `None`; + - non-ASCII before the span (an astral character): `text` is the right + substring; + - `QueryTooLongError`: one entry, all-null location; + - `MultipleSearchQueryErrors` order matches `search_query_error_messages`. +- **Plumbing** (extend `src/documents/tests/search/test_error_routing.py`): + - `_single_diagnostic_to_error` and `_map_emit_error` copy `field`, + `startchar`, `endchar` and `suggestion` onto the error for each branch; + - end to end through `parse_user_query` with the `query_index` fixture: + `created:december 2019 asn:x` yields the spans and single suggestion probed + above, and `title:[a TO b]` yields an emit-time error with `field` and a + span. These pin the whoosh-compat contracts paperless depends on, so an + upgrade that changes them fails loudly. +- **API** (extend `src/documents/tests/test_api_search_errors.py`, which already + covers the list endpoint and the selection endpoints pytest-style): + - list endpoint: 400 with `query` identical to today, `query_errors` entries + with integer (not string) `start`/`end`, `text`, JSON `null`s, and the + expected `suggested_query`; + - a query with no possible fix: `suggested_query` is `null`; + - `query` sent alongside `text=`: spans index the `query` param; + - over-long query: all-null entry; + - a selection endpoint (bulk edit): same shape. + +## Risks / notes + +- Span correctness depends on passing the exact parsed string. The helper's + bounds check and the end-to-end tests guard against drift; a mismatch yields + `text: null` / `suggested_query: null`, never a corrupted query. +- `suggestion` and the emit-time span are whoosh-compat contracts (`Diagnostic` + docstring, README host contract). The end-to-end tests are the upgrade + tripwire. +- Until the frontend half lands on the branch, the document list's search error + banner is broken there (see "Frontend compatibility"). Expected on a shared + branch; it must not merge in that state. +- Third-party clients that iterate every key of a 400 body the way paperless's + own frontend does would see the new keys. Clients that read `query` are + unaffected. Note the new keys in the release notes. + +## Future work + +- A live `/api/search/validate/` endpoint so the filter editor can mark spans + and offer fixes while the user types. +- More suggestions in whoosh-compat (for example comparison operators to range + syntax), which this payload surfaces without paperless changes.