Adds the given/when/then commenting

src/documents/tests/search/test_fields.py

@@ -5,6 +5,14 @@ from documents.search._fields import PUBLIC_FIELDS
 
 class TestPublicFields:
     def test_json_fields_have_subpaths(self) -> None:
+        """
+        GIVEN:
+            - PUBLIC_FIELDS, the canonical query-syntax field table
+        WHEN:
+            - A field declares FieldKind.JSON
+        THEN:
+            - That field also declares at least one subpath
+        """
         for field in PUBLIC_FIELDS:
             if field.kind is FieldKind.JSON:
                 assert field.subpaths, f"{field.name} is JSON but has no subpaths"

src/documents/tests/search/test_json_subpath_completeness.py

@@ -40,6 +40,15 @@ class TestJsonSubpathsAreWrittenAtIndexTime:
         self,
         backend: TantivyBackend,
     ) -> None:
+        """
+        GIVEN:
+            - A document with a Note and a CustomFieldInstance attached
+        WHEN:
+            - The document is indexed via TantivyBackend.add_or_update
+        THEN:
+            - Every subpath PUBLIC_FIELDS declares for notes/custom_fields
+              is present as a key in the document's stored JSON payload
+        """
         user = User.objects.create_user(username="completeness-user")
         field = CustomField.objects.create(
             name="Completeness Field",

src/documents/tests/search/test_keyword_pattern_literal.py

@@ -47,14 +47,30 @@ class TestKeywordPatternNormalizer:
         ],
     )
     def test_keyword_runs_are_folded_not_stemmed(self, run: str) -> None:
-        """One form, the run as typed: a KEYWORD pattern must never be widened
+        """
-        to a stem, which would return checksums that do not start with what
+        GIVEN:
-        the user typed."""
+            - The "checksum" field's registered pattern normalizer
+              (KEYWORD kind, "en" registry)
+        WHEN:
+            - A wildcard pattern run is normalized
+        THEN:
+            - The run is returned unchanged, never widened to a stem (which
+              would return checksums that do not start with what the user
+              typed)
+        """
         normalize = _normalizer(get_field_registry("en"), "checksum")
         assert normalize(run) == run
 
     def test_text_runs_still_offer_their_stem(self) -> None:
-        """A TEXT field offers the stem alongside the typed run, so a term
+        """
-        matching either one is reachable."""
+        GIVEN:
+            - The "title" field's registered pattern normalizer (TEXT kind,
+              "en" registry)
+        WHEN:
+            - A wildcard pattern run is normalized
+        THEN:
+            - Both the folded run and its stem are offered, so a term
+              matching either one is reachable
+        """
         normalize = _normalizer(get_field_registry("en"), "title")
         assert tuple(normalize("Running")) == ("running", "run")

src/documents/tests/search/test_registry.py

@@ -36,6 +36,14 @@ class TestFieldRegistry:
         self,
         registry: FieldRegistry,
     ) -> None:
+        """
+        GIVEN:
+            - The field registry built from PUBLIC_FIELDS
+        WHEN:
+            - An internal *_id column name (e.g. "tag_id") is looked up
+        THEN:
+            - The registry does not recognize it as a queryable field
+        """
         for name in (
             "tag_id",
             "owner_id",
@@ -48,12 +56,17 @@ class TestFieldRegistry:
             assert name not in registry
 
     def test_no_queryable_field_name_ends_in_id(self) -> None:
-        # The list above names the seven that were dropped; this catches the
+        """
-        # eighth. Internal *_id columns are written for permission filtering
+        GIVEN:
-        # and joins, and whoosh only exposed them as query fields by accident,
+            - PUBLIC_FIELDS, the canonical query-syntax field table
-        # so a new one reaching the query surface is a leak rather than a
+        WHEN:
-        # feature. Checked against PUBLIC_FIELDS rather than the registry so
+            - Every declared field name is inspected
-        # an internal field is caught where it is declared.
+        THEN:
+            - None of them end in "_id" (internal id columns, written for
+              permission filtering and joins, must never reach the query
+              surface; checked against PUBLIC_FIELDS rather than the
+              registry so a leak is caught where it is declared)
+        """
         leaked = [f.name for f in PUBLIC_FIELDS if f.name.endswith("_id")]
         assert not leaked, f"internal id fields reached the query surface: {leaked}"
 
@@ -61,18 +74,50 @@ class TestFieldRegistry:
         self,
         registry: FieldRegistry,
     ) -> None:
+        """
+        GIVEN:
+            - The field registry
+        WHEN:
+            - The alias "type" is resolved
+        THEN:
+            - It resolves to the canonical "document_type" field
+        """
         assert _resolve(registry, "type").spec.name == "document_type"
 
     def test_path_alias_resolves_to_storage_path(self, registry: FieldRegistry) -> None:
+        """
+        GIVEN:
+            - The field registry
+        WHEN:
+            - The alias "path" is resolved
+        THEN:
+            - It resolves to the canonical "storage_path" field
+        """
         assert _resolve(registry, "path").spec.name == "storage_path"
 
     def test_notes_json_subpaths_resolve(self, registry: FieldRegistry) -> None:
+        """
+        GIVEN:
+            - The field registry
+        WHEN:
+            - "notes.user" is resolved
+        THEN:
+            - It resolves to the "notes" field with json_path "user"
+        """
         resolved = _resolve(registry, "notes.user")
         assert resolved.spec.name == "notes"
         assert resolved.json_path == "user"
         assert resolved.is_subpath is True
 
     def test_custom_fields_json_subpaths_resolve(self, registry: FieldRegistry) -> None:
+        """
+        GIVEN:
+            - The field registry
+        WHEN:
+            - "custom_fields.name" and "custom_fields.value" are resolved
+        THEN:
+            - Both resolve without error
+        """
         for raw in ("custom_fields.name", "custom_fields.value"):
             _resolve(registry, raw)
 
@@ -80,30 +125,67 @@ class TestFieldRegistry:
         self,
         registry: FieldRegistry,
     ) -> None:
-        # An unregistered subpath is not even a valid FieldRef: make_ref
+        """
-        # returns None for a dotted name whose subpath isn't registered
+        GIVEN:
-        # (it doesn't produce a ref for resolve() to then reject).
+            - The field registry
+        WHEN:
+            - A dotted name naming an unregistered subpath ("notes.bogus")
+              is turned into a FieldRef
+        THEN:
+            - make_ref returns None (it is not even a valid ref for
+              resolve() to then reject)
+        """
         assert registry.make_ref("notes.bogus") is None
 
     def test_tag_is_comma_values(self, registry: FieldRegistry) -> None:
+        """
+        GIVEN:
+            - The field registry
+        WHEN:
+            - The "tag" field is resolved
+        THEN:
+            - It is marked comma_values=True
+        """
         assert _resolve(registry, "tag").spec.comma_values is True
 
     def test_correspondent_is_not_comma_values(self, registry: FieldRegistry) -> None:
-        # "tag" is the only field that opts in. This is only observable here:
+        """
-        # end to end the two readings of "correspondent:foo,bar" agree,
+        GIVEN:
-        # because the analyzer splits the literal value on the comma anyway,
+            - The field registry
-        # so a result-level test cannot tell a value list from literal text.
+        WHEN:
+            - The "correspondent" field is resolved
+        THEN:
+            - It is not marked comma_values ("tag" is the only field that
+              opts in; end to end the two readings of
+              "correspondent:foo,bar" agree anyway, since the analyzer
+              splits the literal value on the comma regardless, so this is
+              only observable at the registry level)
+        """
         assert _resolve(registry, "correspondent").spec.comma_values is False
 
     def test_created_is_date_kind(self, registry: FieldRegistry) -> None:
+        """
+        GIVEN:
+            - The field registry
+        WHEN:
+            - The "created" field is resolved
+        THEN:
+            - Its kind is DATE and date_only is True
+        """
         resolved = _resolve(registry, "created")
         assert resolved.spec.kind is FieldKind.DATE
         assert resolved.spec.date_only is True
 
     def test_analyzer_lowercases_and_ascii_folds(self, registry: FieldRegistry) -> None:
-        # title uses the paperless_text analyzer: simple -> remove_long ->
+        """
-        # lowercase -> ascii_fold [-> stemmer]. With no language configured
+        GIVEN:
-        # (None), no stemmer runs, so "Café" folds to the single token "cafe".
+            - The field registry with no language configured (no stemmer
+              in the analyzer chain)
+        WHEN:
+            - The "title" field's analyzer processes "Café"
+        THEN:
+            - It is lowercased and ASCII-folded to the single token "cafe"
+        """
         resolved = _resolve(registry, "title")
         assert resolved.spec.analyzer is not None
         assert resolved.spec.analyzer("Café") == ["cafe"]
@@ -112,7 +194,15 @@ class TestFieldRegistry:
         self,
         registry: FieldRegistry,
     ) -> None:
-        # checksum uses the raw tokenizer at index time (no splitting).
+        """
+        GIVEN:
+            - The field registry
+        WHEN:
+            - The "checksum" field's analyzer (raw tokenizer, no
+              splitting) processes "ABC-123"
+        THEN:
+            - It is returned unchanged as a single token
+        """
         resolved = _resolve(registry, "checksum")
         assert resolved.spec.analyzer is not None
         assert resolved.spec.analyzer("ABC-123") == ["ABC-123"]
@@ -121,10 +211,17 @@ class TestFieldRegistry:
         self,
         registry: FieldRegistry,
     ) -> None:
-        # Index terms are stemmed, so patterns offer their stem too, using the
+        """
-        # registry's own language: "Running" has to reach the indexed "run".
+        GIVEN:
-        # Without a language the index holds surface forms, so there is no
+            - A registry with no language, and a registry built for "en"
-        # second form and the run is only case/accent-folded.
+        WHEN:
+            - The "title" field's pattern normalizer processes "Running"
+        THEN:
+            - With no language, only the folded run is offered
+              ("running"), since the index holds surface forms
+            - With "en", the stem is offered too ("run"), since indexed
+              terms are stemmed and the pattern has to reach them
+        """
         resolved = _resolve(registry, "title")
         assert resolved.spec.pattern_normalizer is not None
         assert _distinct_forms(resolved.spec.pattern_normalizer("Running")) == (
@@ -139,11 +236,28 @@ class TestFieldRegistry:
         )
 
     def test_registry_is_cached_per_language(self) -> None:
+        """
+        GIVEN:
+            - Two calls to get_field_registry("en")
+        WHEN:
+            - Both calls are made
+        THEN:
+            - They return the same registry instance
+        """
         a = get_field_registry("en")
         b = get_field_registry("en")
         assert a is b
 
     def test_registry_rebuilds_on_language_change(self) -> None:
+        """
+        GIVEN:
+            - A call to get_field_registry("en") and a call to
+              get_field_registry("de")
+        WHEN:
+            - Both calls are made
+        THEN:
+            - They return different registry instances
+        """
         a = get_field_registry("en")
         b = get_field_registry("de")
         assert a is not b

src/documents/tests/search/test_schema.py

@@ -107,6 +107,15 @@ def _schema_fields(schema: tantivy.Schema) -> dict[str, dict]:
 
 class TestSchemaMatchesPublicFields:
     def test_every_public_field_is_in_the_schema(self) -> None:
+        """
+        GIVEN:
+            - PUBLIC_FIELDS and the tantivy schema built by build_schema()
+        WHEN:
+            - Every field declared in PUBLIC_FIELDS is checked against the
+              schema
+        THEN:
+            - Each one is present as a field in the built schema
+        """
         schema = build_schema()
         schema_field_names = set(_schema_fields(schema))
         for field in PUBLIC_FIELDS:
@@ -115,7 +124,16 @@ class TestSchemaMatchesPublicFields:
             )
 
     def test_asn_page_count_num_notes_are_fast_unsigned_fields(self) -> None:
-        # Spot-check kind-derived construction for the U64 fields.
+        """
+        GIVEN:
+            - A document with asn/page_count/num_notes values, indexed
+              against the schema built by build_schema()
+        WHEN:
+            - A term query on the fast "asn" field is run
+        THEN:
+            - The document is found, spot-checking kind-derived
+              construction for the U64 fields
+        """
         schema = build_schema()
         doc = tantivy.Document()
         doc.add_unsigned("id", 1)
@@ -139,21 +157,23 @@ class TestSchemaMatchesPublicFields:
 
 class TestFastFlagAgreement:
     def test_every_public_field_fast_flag_matches_the_built_schema(self) -> None:
-        # whoosh-compat's registry trusts PUBLIC_FIELDS' fast flag when resolving
+        """
-        # field:* existence checks (its FAST_FIELD strategy); a fast=True
+        GIVEN:
-        # entry whose actual tantivy column is not fast would make those
+            - PUBLIC_FIELDS and field_descriptors() (the latter is exactly
-        # searches silently match nothing at search time. Only the U64 and
+              the input build_schema()'s SchemaBuilder consumes for the
-        # DATE descriptors can carry the flag today, so this
+              `fast` kwarg on every field kind, so it pins the agreement
-        # pins the agreement for EVERY kind: a future fast=True
+              without depending on a private tantivy-py pickled
-        # TEXT/KEYWORD/JSON entry the builder silently ignores fails here
+              representation)
-        # instead of at a user's query.
+        WHEN:
-        #
+            - Every PUBLIC_FIELDS entry's fast flag is compared against
-        # field_descriptors() (not tantivy-py's __reduce__() pickling
+              field_descriptors()' fast flag for the same field
-        # internals) is used as the probe here: it is exactly the input
+        THEN:
-        # build_schema()'s SchemaBuilder consumes for the `fast` kwarg on
+            - They agree for every field, catching a fast=True
-        # every field kind, so it pins the same agreement without depending
+              PUBLIC_FIELDS entry the builder silently ignores here
-        # on a private pickled representation surviving a tantivy-py
+              instead of at a user's field:* existence query, which
-        # upgrade.
+              whoosh-compat's registry trusts PUBLIC_FIELDS' fast flag to
+              resolve
+        """
         descriptor_fast = {d.name: d.fast for d in field_descriptors()}
         for public_field in PUBLIC_FIELDS:
             assert descriptor_fast[public_field.name] == public_field.fast, (

src/documents/tests/search/test_schema_fingerprint.py

@@ -311,13 +311,32 @@ def _sentinels(index_dir: Path, **overrides: object) -> None:
 
 class TestDescriptorsDescribeTheBuiltSchema:
     def test_descriptors_match_the_pinned_field_layout(self) -> None:
+        """
+        GIVEN:
+            - PINNED_DESCRIPTORS, a frozen snapshot of the v2 on-disk field
+              layout, reproduced from build_schema()'s output as it stood
+              before the descriptor refactor
+        WHEN:
+            - field_descriptors() is called
+        THEN:
+            - It matches the pinned layout exactly, in the same order,
+              pinning that the refactor changed nothing
+        """
         assert tuple(field_descriptors()) == PINNED_DESCRIPTORS
 
     def test_built_schema_matches_the_descriptors(self) -> None:
-        """The descriptors are not a parallel description - they are the input.
+        """
-
+        GIVEN:
-        Reading the built schema back proves the loop honours every option, so
+            - The schema built by build_schema()
-        a descriptor edit cannot claim a shape the SchemaBuilder did not build.
+        WHEN:
+            - Its fields are read back via __reduce__() (schema.__reduce__(),
+              tantivy-py's pickling hook)
+        THEN:
+            - Every field's name, kind, stored/fast flags and tokenizer
+              match what field_descriptors() declared as input; the
+              descriptors are not a parallel description, they are the
+              input, so a descriptor edit cannot claim a shape the
+              SchemaBuilder did not actually build
         """
         kinds = {"text": "text", "json": "json_object", "u64": "u64", "date": "date"}
         built = [
@@ -348,6 +367,15 @@ class TestFingerprintSensitivity:
         self,
         monkeypatch: pytest.MonkeyPatch,
     ) -> None:
+        """
+        GIVEN:
+            - The current schema fingerprint
+        WHEN:
+            - A single field descriptor's "fast" option is changed, with
+              no other change
+        THEN:
+            - The fingerprint changes
+        """
         before = schema_fingerprint()
         changed = field_descriptors()
         changed[1] = changed[1]._replace(fast=True)
@@ -359,10 +387,16 @@ class TestFingerprintSensitivity:
         self,
         monkeypatch: pytest.MonkeyPatch,
     ) -> None:
-        """The original bug: same fields, different declaration order.
+        """
-
+        GIVEN:
-        A set- or dict-based fingerprint would be blind to this, and tantivy
+            - The current schema fingerprint
-        would reject every write against the existing index.
+        WHEN:
+            - Two field descriptors are swapped, with no other change (the
+              original bug: same fields, different declaration order)
+        THEN:
+            - The fingerprint changes; a set- or dict-based fingerprint
+              would be blind to this, and tantivy would reject every write
+              against the existing index
         """
         before = schema_fingerprint()
         swapped = field_descriptors()
@@ -374,11 +408,19 @@ class TestFingerprintSensitivity:
 
 class TestFingerprintIsIndependentOfTantivy:
     def test_a_tantivy_option_key_addition_would_not_move_it(self) -> None:
-        """A tantivy-py upgrade must not force a global reindex.
+        """
-
+        GIVEN:
-        Hashing schema.__reduce__() would do exactly that: the simulated new
+            - The built schema's raw field list, and the same list with a
-        option key below changes that payload for every user with no schema
+              new tantivy-internal option key added (simulating a
-        change at all.
+              tantivy-py upgrade)
+        WHEN:
+            - Both raw lists are hashed directly, and schema_fingerprint()
+              is compared against a hash of field_descriptors()
+        THEN:
+            - The raw hashes differ (hashing schema.__reduce__() would
+              force a global reindex on every tantivy-py upgrade), but
+              schema_fingerprint() is unaffected, since it hashes
+              field_descriptors(), never tantivy's own representation
         """
         fields = _schema_fields(build_schema())
         upgraded = [
@@ -392,6 +434,17 @@ class TestFingerprintIsIndependentOfTantivy:
         self,
         monkeypatch: pytest.MonkeyPatch,
     ) -> None:
+        """
+        GIVEN:
+            - tantivy.SchemaBuilder replaced with a stand-in that raises if
+              constructed
+        WHEN:
+            - build_schema() is called (and raises), then
+              schema_fingerprint() is called again
+        THEN:
+            - schema_fingerprint() still matches its earlier value,
+              proving it never consults SchemaBuilder
+        """
         before = schema_fingerprint()
 
         class _RemovedSchemaBuilder:
@@ -419,6 +472,15 @@ class TestNeedsRebuildOnFingerprint:
         index_dir: Path,
         settings: SettingsWrapper,
     ) -> None:
+        """
+        GIVEN:
+            - An index directory whose sentinel file records the current
+              schema_fingerprint()
+        WHEN:
+            - needs_rebuild() is called
+        THEN:
+            - It returns False
+        """
         settings.SEARCH_LANGUAGE = None
         _sentinels(index_dir)
 
@@ -430,9 +492,18 @@ class TestNeedsRebuildOnFingerprint:
         settings: SettingsWrapper,
         monkeypatch: pytest.MonkeyPatch,
     ) -> None:
-        """The failure this task exists to prevent: schema edited, version not
+        """
-        bumped. Without the fingerprint check, `reindex --if-needed` reports the
+        GIVEN:
-        index up to date and every write then raises."""
+            - An index directory whose sentinel matches SCHEMA_VERSION,
+              but field_descriptors() is patched to add a field the
+              fingerprint never saw (schema edited, version not bumped)
+        WHEN:
+            - needs_rebuild() is called
+        THEN:
+            - It returns True; without the fingerprint check,
+              `reindex --if-needed` would report the index up to date and
+              every subsequent write would raise
+        """
         settings.SEARCH_LANGUAGE = None
         _sentinels(index_dir)
         extended = [
@@ -456,6 +527,16 @@ class TestNeedsRebuildOnFingerprint:
         settings: SettingsWrapper,
         monkeypatch: pytest.MonkeyPatch,
     ) -> None:
+        """
+        GIVEN:
+            - An index directory whose sentinel matches the current
+              fingerprint, but field_descriptors() is patched to swap two
+              fields' order
+        WHEN:
+            - needs_rebuild() is called
+        THEN:
+            - It returns True
+        """
         settings.SEARCH_LANGUAGE = None
         _sentinels(index_dir)
         reordered = field_descriptors()
@@ -469,8 +550,16 @@ class TestNeedsRebuildOnFingerprint:
         index_dir: Path,
         settings: SettingsWrapper,
     ) -> None:
-        """No seeding: an index whose schema shape nobody recorded is rebuilt
+        """
-        rather than trusted."""
+        GIVEN:
+            - An index directory whose sentinel has no "schema_fingerprint"
+              key at all
+        WHEN:
+            - needs_rebuild() is called
+        THEN:
+            - It returns True; an index whose schema shape nobody recorded
+              is rebuilt rather than trusted
+        """
         settings.SEARCH_LANGUAGE = None
         (index_dir / ".index_settings.json").write_text(
             json.dumps({"schema_version": SCHEMA_VERSION, "language": None}),
@@ -483,6 +572,15 @@ class TestNeedsRebuildOnFingerprint:
         index_dir: Path,
         settings: SettingsWrapper,
     ) -> None:
+        """
+        GIVEN:
+            - An index directory whose sentinels are written by
+              _write_sentinels() itself
+        WHEN:
+            - needs_rebuild() is called
+        THEN:
+            - It returns False
+        """
         settings.SEARCH_LANGUAGE = "en"
         _write_sentinels(index_dir)
 

src/documents/tests/search/test_schema_version.py

@@ -123,10 +123,16 @@ class TestUpgradeFromReleasedV1Index:
         self,
         released_v1_index: Path,
     ) -> None:
-        """The current schema differs from v1's, so the sentinel must be stale.
+        """
-
+        GIVEN:
-        If this fails, `document_index reindex --if-needed` prints "Search index
+            - An index directory laid out exactly as a v3.0.x (schema
-        is up to date" and skips, leaving the mismatched index in place.
+              version 1) install would leave it
+        WHEN:
+            - needs_rebuild() is called
+        THEN:
+            - It returns True; if this fails,
+              `document_index reindex --if-needed` prints "Search index is
+              up to date" and skips, leaving the mismatched index in place
         """
         assert needs_rebuild(released_v1_index) is True
 
@@ -134,9 +140,16 @@ class TestUpgradeFromReleasedV1Index:
         self,
         released_v1_index: Path,
     ) -> None:
-        """The failure mode the version bump exists to prevent.
+        """
-
+        GIVEN:
-        This is exactly what WriteBatch.__enter__ does on every index write.
+            - A v1 index directory and the current build_schema()
+        WHEN:
+            - A new tantivy.Index is opened against that directory with
+              the current schema
+        THEN:
+            - It raises ValueError("schema does not match ..."), the
+              exact failure mode WriteBatch.__enter__ hits on every index
+              write, which the version bump exists to prevent
         """
         schema = build_schema()
         with pytest.raises(ValueError, match="schema does not match"):
@@ -146,10 +159,19 @@ class TestUpgradeFromReleasedV1Index:
         self,
         released_v1_index: Path,
     ) -> None:
-        """End to end: open_or_rebuild_index must hand back an index that the
+        """
-        write path can reopen. Before the version bump, needs_rebuild() returned
+        GIVEN:
-        False here, the stale directory survived untouched, and every subsequent
+            - A v1 index directory
-        write raised the ValueError above."""
+        WHEN:
+            - open_or_rebuild_index() is called against it
+        THEN:
+            - The directory can be reopened with the current schema
+              without raising; end to end, open_or_rebuild_index must
+              hand back an index the write path can reopen. Before the
+              version bump, needs_rebuild() returned False here, the
+              stale directory survived untouched, and every subsequent
+              write raised the ValueError from the test above
+        """
         open_or_rebuild_index(released_v1_index)
 
         tantivy.Index(build_schema(), path=str(released_v1_index))
@@ -158,8 +180,17 @@ class TestUpgradeFromReleasedV1Index:
         self,
         released_v1_index: Path,
     ) -> None:
-        """The rebuild must stamp the version it actually wrote, otherwise every
+        """
-        startup wipes and reindexes the whole corpus."""
+        GIVEN:
+            - A v1 index directory that has just been rebuilt by
+              open_or_rebuild_index()
+        WHEN:
+            - needs_rebuild() is called again
+        THEN:
+            - It returns False; the rebuild must stamp the version it
+              actually wrote, otherwise every startup wipes and reindexes
+              the whole corpus
+        """
         open_or_rebuild_index(released_v1_index)
 
         assert needs_rebuild(released_v1_index) is False