[NRT-771] Guard the batched AH-DB reads; trim restated comments

The diff threading and the media batching were both perf claims resting on
inspection alone, so pin the two that a future edit could silently undo:

- `test_ah_db_note_readers_agree_about_deleted_notes` pins the equivalence the
  batched read rides on: `note_data` and `notes_data_for_anki_nids` must keep
  agreeing about deleted rows, or the outgoing media-name set changes silently.
- `test_bulk_submit_does_not_read_the_ah_db_per_note` asserts the AnkiHub-DB
  read count for a bulk submit does not grow with the note count. Asserting
  non-scaling rather than exact numbers keeps it robust to unrelated changes in
  the submit path. Checked against the pre-batching implementation, where it
  fails, so it is not a vacuous guard.

A wall-clock gate would not catch either regression: re-adding thousands of
queries still fits inside the existing 2s ceiling on CI hardware.

Also: "DELETE carries no field/tag content" was stated five times in this file;
drop the one the `if change_type != DELETE` below it already says. And correct
the perf test's claim that 500 notes "matches the per-bulk-suggestion cap" —
the cap is 2000, and the ceiling is calibrated to 500.

Author: RisingOrange

ankihub/main/suggestions.py

@@ -151,9 +151,7 @@ class BulkSuggestionFilters:
 
     @classmethod
     def none_selected(cls) -> "BulkSuggestionFilters":
-        """Select nothing, for a submit that carries no field or tag choices (DELETE).
-        Distinct from passing no filters at all, which ships everything.
-        """
+        """Distinct from passing no filters at all, which ships everything."""
         return cls(fields_to_include_by_mid={})
 
     def for_mid(self, mid: NotetypeId) -> PerNoteFilters:
@@ -215,8 +213,8 @@ def compute_note_diffs(notes: Sequence[Note]) -> Dict[NoteId, NoteDiff]:
         )
 
         if ah_note is None:
-            # New-note candidate: no AH baseline, so "what would ship" is every non-empty
-            # field (new-note suggestions never carry empty fields) and all current tags.
+            # New-note suggestions never carry empty fields, so for a note with no AH
+            # baseline "what would ship" is exactly its non-empty fields.
             changed_fields = [f for f in cur.fields if f.value]
             added_tags = list(cur.tags or [])
             removed_tags: List[str] = []
@@ -794,7 +792,6 @@ def _change_note_suggestion(
     removed_tags: List[str] = []
     fields_that_changed: List[Field] = []
 
-    # DELETE carries no field/tag content, so it ships an empty suggestion.
     if change_type != SuggestionType.DELETE:
         fields_that_changed = _apply_field_allowlist(diff.changed_fields, filters.fields_to_include)
         added_tags = _apply_tag_allowlist(diff.added_tags, filters.tags_to_add)

tests/addon/performance/test_suggestion_dialog.py

@@ -30,15 +30,17 @@ def test_bulk_suggestion_dialog_open_diff_pipeline(
     profile: Profile,
 ):
     """Measures the per-note work that runs synchronously on the UI thread
-    when the user opens the bulk Suggest-a-change dialog at the 500-note cap.
+    when the user opens the bulk Suggest-a-change dialog.
     Exercises `compute_note_diffs` once and feeds its result through the
     bulk-suggestible gate and the media check. The widget's `_populate` is
     explicitly excluded — it's cheap per-note filtering off already-computed
     diffs — and isn't covered by this test.
     """
     with anki_session_with_addon_data.profile_loaded():
         mw = anki_session_with_addon_data.mw
-        notes_amount = 500  # matches the per-bulk-suggestion cap
+        # A quarter of the 2000-note bulk cap (BULK_SUGGESTION_LIMIT), to keep the
+        # test quick; the ceiling below is calibrated to this size, not to the cap.
+        notes_amount = 500
 
         ankihub_did = next_deterministic_uuid()
         importer = AnkiHubImporter()

tests/addon/test_integration.py

@@ -2513,6 +2513,78 @@ def test_bulk_new_notes_dont_ship_field_empty_on_that_note(
             # The note with an empty Back ships only Front — no empty field.
             assert [f.name for f in sent_by_nid[note_without_back.id].fields] == ["Front"]
 
+    def test_ah_db_note_readers_agree_about_deleted_notes(
+        self,
+        anki_session_with_addon_data: AnkiSession,
+        install_ah_deck: InstallAHDeck,
+        import_ah_note: ImportAHNote,
+    ):
+        """The media step reads note data through the batched `notes_data_for_anki_nids`
+        rather than a per-note `note_data` loop. That swap is only sound while the two
+        agree about deleted rows; if they diverge the outgoing media-name set changes
+        silently, so pin the agreement here.
+        """
+        with anki_session_with_addon_data.profile_loaded():
+            ah_did = install_ah_deck()
+            note_info = import_ah_note(ah_did=ah_did)
+            nid = ankihub_db.anki_nid_for_ankihub_nid(note_info.ah_nid)
+
+            assert ankihub_db.note_data(nid) is not None
+            assert [note.anki_nid for note in ankihub_db.notes_data_for_anki_nids([nid])] == [nid]
+
+            AnkiHubNote.update(last_update_type=SuggestionType.DELETE.value[0]).where(
+                AnkiHubNote.anki_note_id == nid
+            ).execute()
+
+            assert ankihub_db.note_data(nid) is None
+            assert ankihub_db.notes_data_for_anki_nids([nid]) == []
+
+    def test_bulk_submit_does_not_read_the_ah_db_per_note(
+        self,
+        anki_session_with_addon_data: AnkiSession,
+        mocker: MockerFixture,
+        install_ah_deck: InstallAHDeck,
+        next_deterministic_uuid: Callable[[], uuid.UUID],
+        import_ah_note_type: ImportAHNoteType,
+        add_anki_note: AddAnkiNote,
+    ):
+        """Submitting reads the AnkiHub DB per batch and per note type, never per note.
+        Asserting the counts don't grow with the note count rather than pinning exact
+        numbers keeps this robust to unrelated changes in the submit path. At the
+        2000-note bulk cap a reintroduced per-note read costs thousands of queries, and
+        no other test would notice.
+        """
+        with anki_session_with_addon_data.profile_loaded():
+            ah_did = install_ah_deck()
+            note_type = import_ah_note_type(ah_did=ah_did)
+            mocker.patch.object(AnkiHubClient, "create_suggestions_in_bulk", return_value={})
+            note_data_spy = mocker.spy(ankihub_db, "note_data")
+            note_type_dict_spy = mocker.spy(ankihub_db, "note_type_dict")
+
+            def ah_db_reads_for_submitting(note_count: int) -> Tuple[int, int]:
+                notes = []
+                for i in range(note_count):
+                    note = add_anki_note(note_type=note_type)
+                    note["Front"] = f"front_{len(notes)}_{i}"
+                    aqt.mw.col.update_note(note)
+                    notes.append(note)
+
+                mocker.patch("uuid.uuid4", side_effect=[next_deterministic_uuid() for _ in range(note_count)])
+                note_data_spy.reset_mock()
+                note_type_dict_spy.reset_mock()
+
+                suggest_notes_in_bulk(
+                    ankihub_did=ah_did,
+                    notes=notes,
+                    auto_accept=False,
+                    change_type=SuggestionType.NEW_CONTENT,
+                    comment="test",
+                    media_upload_cb=mocker.stub(),
+                )
+                return note_data_spy.call_count, note_type_dict_spy.call_count
+
+            assert ah_db_reads_for_submitting(2) == ah_db_reads_for_submitting(6)
+
     @pytest.mark.parametrize(
         "note_has_changes, note_is_marked_as_deleted",
         [