ref(deletions): Simplify seer deletion tests (#95267)

This simplifies the tests for deletions of hashes from Seer and it also
adds a test for #95156.

Author: armenzg

src/sentry/api/helpers/group_index/delete.py

@@ -19,7 +19,7 @@
 from sentry.models.groupinbox import GroupInbox
 from sentry.models.project import Project
 from sentry.signals import issue_deleted
-from sentry.tasks.delete_seer_grouping_records import call_delete_seer_grouping_records_by_hash
+from sentry.tasks.delete_seer_grouping_records import may_schedule_task_to_delete_hashes_from_seer
 from sentry.utils.audit import create_audit_entry
 
 from . import BULK_MUTATION_LIMIT, SearchFunction
@@ -85,7 +85,7 @@ def delete_group_list(
     create_audit_entries(request, project, group_list, delete_type, transaction_id)
 
     # Tell seer to delete grouping records for these groups
-    call_delete_seer_grouping_records_by_hash(error_ids)
+    may_schedule_task_to_delete_hashes_from_seer(error_ids)
 
     # Removing GroupHash rows prevents new events from associating to the groups
     # we just deleted.

src/sentry/deletions/defaults/group.py

@@ -15,7 +15,7 @@
 from sentry.models.rulefirehistory import RuleFireHistory
 from sentry.notifications.models.notificationmessage import NotificationMessage
 from sentry.snuba.dataset import Dataset
-from sentry.tasks.delete_seer_grouping_records import call_delete_seer_grouping_records_by_hash
+from sentry.tasks.delete_seer_grouping_records import may_schedule_task_to_delete_hashes_from_seer
 from sentry.utils.snuba import bulk_snuba_queries
 
 from ..base import BaseDeletionTask, BaseRelation, ModelDeletionTask, ModelRelation
@@ -273,7 +273,7 @@ def delete_bulk(self, instance_list: Sequence[Group]) -> bool:
             except InvalidGroupTypeError:
                 pass
         # Tell seer to delete grouping records with these group hashes
-        call_delete_seer_grouping_records_by_hash(error_group_ids)
+        may_schedule_task_to_delete_hashes_from_seer(error_group_ids)
 
         self._delete_children(instance_list)
 

src/sentry/seer/similarity/grouping_records.py

@@ -85,7 +85,7 @@ def post_bulk_grouping_records(
         return {"success": False, "reason": response.reason}
 
 
-def delete_project_grouping_records(
+def call_seer_to_delete_project_grouping_records(
     project_id: int,
 ) -> bool:
     try:

src/sentry/tasks/delete_seer_grouping_records.py

@@ -6,8 +6,8 @@
 from sentry.models.group import Group
 from sentry.models.grouphash import GroupHash
 from sentry.seer.similarity.grouping_records import (
+    call_seer_to_delete_project_grouping_records,
     call_seer_to_delete_these_hashes,
-    delete_project_grouping_records,
 )
 from sentry.seer.similarity.utils import ReferrerOptions, killswitch_enabled
 from sentry.silo.base import SiloMode
@@ -69,7 +69,7 @@ def delete_seer_grouping_records_by_hash(
             delete_seer_grouping_records_by_hash.apply_async(args=[project_id, chunked_hashes, 0])
 
 
-def call_delete_seer_grouping_records_by_hash(
+def may_schedule_task_to_delete_hashes_from_seer(
     group_ids: Sequence[int],
 ) -> None:
     project = None
@@ -124,4 +124,4 @@ def call_seer_delete_project_grouping_records(
         return
 
     logger.info("calling seer delete records by project", extra={"project_id": project_id})
-    delete_project_grouping_records(project_id)
+    call_seer_to_delete_project_grouping_records(project_id)

src/sentry/tasks/embeddings_grouping/utils.py

@@ -22,7 +22,7 @@
     BulkCreateGroupingRecordsResponse,
     CreateGroupingRecordData,
     CreateGroupingRecordsRequest,
-    delete_project_grouping_records,
+    call_seer_to_delete_project_grouping_records,
     post_bulk_grouping_records,
 )
 from sentry.seer.similarity.types import (
@@ -770,7 +770,7 @@ def delete_seer_grouping_records(
         "backfill_seer_grouping_records.delete_all_seer_records",
         extra={"project_id": project_id},
     )
-    delete_project_grouping_records(project_id)
+    call_seer_to_delete_project_grouping_records(project_id)
 
     for groups in chunked(
         RangeQuerySetWrapper(

tests/sentry/tasks/test_backfill_seer_grouping_records.py

@@ -868,7 +868,7 @@ def test_multiple_batches(
         ]
         assert mock_utils_logger.info.call_args_list == expected_utils_call_args_list
 
-    @patch("sentry.tasks.embeddings_grouping.utils.delete_project_grouping_records")
+    @patch("sentry.tasks.embeddings_grouping.utils.call_seer_to_delete_project_grouping_records")
     def test_only_delete(self, mock_project_delete_grouping_records):
         """
         Test that when the only_delete flag is on, seer_similarity is deleted from the metadata
@@ -906,7 +906,7 @@ def test_only_delete(self, mock_project_delete_grouping_records):
         for group in groups:
             assert group.data["metadata"] == default_metadata
 
-    @patch("sentry.tasks.embeddings_grouping.utils.delete_project_grouping_records")
+    @patch("sentry.tasks.embeddings_grouping.utils.call_seer_to_delete_project_grouping_records")
     @patch("sentry.tasks.embeddings_grouping.utils.post_bulk_grouping_records")
     def test_cohort_only_delete(
         self, mock_post_bulk_grouping_records, mock_delete_grouping_records

tests/sentry/tasks/test_delete_seer_grouping_records.py

@@ -1,74 +1,52 @@
 from time import time
 from unittest.mock import MagicMock, patch
 
-from sentry import options
 from sentry.models.grouphash import GroupHash
 from sentry.tasks.delete_seer_grouping_records import (
-    call_delete_seer_grouping_records_by_hash,
     delete_seer_grouping_records_by_hash,
+    may_schedule_task_to_delete_hashes_from_seer,
 )
 from sentry.testutils.cases import TestCase
 from sentry.testutils.pytest.fixtures import django_db_all
 
 
 @django_db_all
 class TestDeleteSeerGroupingRecordsByHash(TestCase):
-    @patch("sentry.tasks.delete_seer_grouping_records.call_seer_to_delete_these_hashes")
-    @patch(
-        "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
-    )
-    def test_delete_seer_grouping_records_by_hash_batches(
-        self,
-        mock_delete_seer_grouping_records_by_hash_apply_async: MagicMock,
-        mock_call_seer_to_delete_these_hashes: MagicMock,
-    ) -> None:
-        """
-        Test that when delete_seer_grouping_records_by_hash is called with more hashes than the batch size, it spawns
-        another task with the end index of the previous batch.
-        """
-        batch_size = options.get("embeddings-grouping.seer.delete-record-batch-size") or 100
-        mock_call_seer_to_delete_these_hashes.return_value = True
-        project_id, hashes = 1, [str(i) for i in range(batch_size + 1)]
-        # We call it as a function and will schedule a task for the extra hash
-        delete_seer_grouping_records_by_hash(project_id, hashes, 0)
-        assert mock_delete_seer_grouping_records_by_hash_apply_async.call_args[1] == {
-            # We do not schedule the task with all the hashes, but only the extra ones
-            "args": [project_id, hashes[batch_size:], 0]
-        }
-
-    @patch(
-        "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
-    )
-    def test_call_delete_seer_grouping_records_by_hash_simple(
-        self, mock_apply_async: MagicMock
-    ) -> None:
-        """
-        Test that call_delete_seer_grouping_records_by_hash correctly collects hashes
-        and calls the deletion task with the expected parameters.
-        """
+    def setUp(self) -> None:
+        super().setUp()
+        # Needed for may_schedule_task_to_delete_hashes_from_seer to allow the task to be scheduled
         self.project.update_option("sentry:similarity_backfill_completed", int(time()))
 
+    def _setup_groups_and_hashes(self, number_of_groups: int = 5) -> tuple[list[int], list[str]]:
         group_ids, expected_hashes = [], []
-        for i in range(5):
+        for i in range(number_of_groups):
             group = self.create_group(project=self.project)
             group_ids.append(group.id)
             group_hash = GroupHash.objects.create(
                 project=self.project, hash=f"{i:032d}", group=group
             )
             expected_hashes.append(group_hash.hash)
+        return group_ids, expected_hashes
+
+    @patch(
+        "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
+    )
+    def test_simple(self, mock_apply_async: MagicMock) -> None:
+        """
+        Test that it correctly collects hashes and schedules a task.
+        """
+        group_ids, expected_hashes = self._setup_groups_and_hashes(number_of_groups=5)
 
-        call_delete_seer_grouping_records_by_hash(group_ids)
+        may_schedule_task_to_delete_hashes_from_seer(group_ids)
 
         # Verify that the task was called with the correct parameters
         mock_apply_async.assert_called_once_with(args=[self.project.id, expected_hashes, 0])
 
-    def test_call_delete_seer_grouping_records_by_hash_chunked(self) -> None:
+    def test_chunked(self) -> None:
         """
-        Test that call_delete_seer_grouping_records_by_hash chunks large numbers of hashes
+        Test that it chunks large numbers of hashes
         into separate tasks with a maximum of batch_size hashes per task.
         """
-        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
-
         batch_size = 10
         with (
             patch(
@@ -77,16 +55,9 @@ def test_call_delete_seer_grouping_records_by_hash_chunked(self) -> None:
             self.options({"embeddings-grouping.seer.delete-record-batch-size": batch_size}),
         ):
             # Create 15 group hashes to test chunking (10 + 5 with batch size of 10)
-            group_ids, expected_hashes = [], []
-            for i in range(batch_size + 5):
-                group = self.create_group(project=self.project)
-                group_ids.append(group.id)
-                group_hash = GroupHash.objects.create(
-                    project=self.project, hash=f"{i:032d}", group=group
-                )
-                expected_hashes.append(group_hash.hash)
+            group_ids, expected_hashes = self._setup_groups_and_hashes(batch_size + 5)
 
-            call_delete_seer_grouping_records_by_hash(group_ids)
+            may_schedule_task_to_delete_hashes_from_seer(group_ids)
 
             # Verify that the task was called 2 times (15 hashes / 10 per chunk = 2 chunks)
             assert mock_apply_async.call_count == 2
@@ -108,23 +79,68 @@ def test_call_delete_seer_grouping_records_by_hash_chunked(self) -> None:
     @patch(
         "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
     )
-    def test_call_delete_seer_grouping_records_by_hash_no_hashes(
-        self, mock_apply_async: MagicMock
-    ) -> None:
-        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
-
-        group_ids = []
-        for _ in range(5):
-            group = self.create_group(project=self.project)
-            group_ids.append(group.id)
-        call_delete_seer_grouping_records_by_hash(group_ids)
+    def test_group_without_hashes(self, mock_apply_async: MagicMock) -> None:
+        group = self.create_group(project=self.project)
+        may_schedule_task_to_delete_hashes_from_seer([group.id])
         mock_apply_async.assert_not_called()
 
     @patch(
         "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
     )
-    def test_call_delete_seer_grouping_records_by_hash_no_group_ids(
-        self, mock_apply_async: MagicMock
-    ) -> None:
-        call_delete_seer_grouping_records_by_hash([])
+    def test_no_group_ids(self, mock_apply_async: MagicMock) -> None:
+        """
+        Test that when no group ids are provided, the task is not scheduled.
+        """
+        may_schedule_task_to_delete_hashes_from_seer([])
         mock_apply_async.assert_not_called()
+
+    @patch(
+        "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
+    )
+    def test_called_task_with_too_many_hashes(self, mock_apply_async: MagicMock) -> None:
+        """This tests the built-in logic of spreading hashes across multiple tasks."""
+        batch_size = 5
+        with self.options({"embeddings-grouping.seer.delete-record-batch-size": batch_size}):
+            # Create 11 group hashes to test chunking (5 + 5 + 1 with batch size of 5)
+            _, expected_hashes = self._setup_groups_and_hashes(batch_size + batch_size + 1)
+            # Call function directly rather than scheduling a task
+            delete_seer_grouping_records_by_hash(self.project.id, expected_hashes, 0)
+
+            # Verify the first chunk has batch_size hashes
+            first_call_args = mock_apply_async.call_args_list[0][1]["args"]
+            assert len(first_call_args[1]) == batch_size
+            assert first_call_args[0] == self.project.id
+            first_chunk = expected_hashes[0:batch_size]
+            assert first_call_args[1] == first_chunk
+            assert first_call_args[2] == 0
+
+            # Verify the second chunk has batch_size hashes
+            second_call_args = mock_apply_async.call_args_list[1][1]["args"]
+            assert len(second_call_args[1]) == batch_size
+            assert second_call_args[0] == self.project.id
+            second_chunk = expected_hashes[batch_size : (batch_size * 2)]
+            assert second_call_args[1] == second_chunk
+            assert second_call_args[2] == 0
+
+            # Verify the third chunk has 1 hash (remainder)
+            third_call_args = mock_apply_async.call_args_list[2][1]["args"]
+            assert len(third_call_args[1]) == 1
+            assert third_call_args[0] == self.project.id
+            third_chunk = expected_hashes[(batch_size * 2) :]
+            assert third_call_args[1] == third_chunk
+            assert third_call_args[2] == 0
+
+            # Make sure the hashes add up to the expected hashes
+            assert first_chunk + second_chunk + third_chunk == expected_hashes
+
+    @patch(
+        "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
+    )
+    def test_does_not_schedule_task_if_missing_option(self, mock_apply_async: MagicMock) -> None:
+        """
+        Test that when the project option is not set, the task is not scheduled.
+        """
+        self.project.delete_option("sentry:similarity_backfill_completed")
+        group_ids, _ = self._setup_groups_and_hashes(number_of_groups=5)
+        may_schedule_task_to_delete_hashes_from_seer(group_ids)
+        assert mock_apply_async.call_count == 0

tests/snuba/api/endpoints/test_project_group_index.py

@@ -1636,7 +1636,7 @@ def test_bulk_delete_performance_issues(self):
             assert response.status_code == 204
             self.assert_groups_are_gone(groups)
 
-    @patch("sentry.api.helpers.group_index.delete.call_delete_seer_grouping_records_by_hash")
+    @patch("sentry.api.helpers.group_index.delete.may_schedule_task_to_delete_hashes_from_seer")
     @patch("sentry.utils.audit.log_service.record_audit_log")
     def test_audit_log_even_if_exception_raised(
         self, mock_record_audit_log: Mock, mock_seer_delete: Mock