ref(seer): Type deletion code + improve tests (#94818)

This adds stronger typing and we change the tests to assert that the
task gets scheduled rather than checking for logs existence.

Fixes
[ID-759](https://linear.app/getsentry/issue/ID-759/strongly-type-seer-deletion-code).

Author: armenzg

pyproject.toml

@@ -384,6 +384,7 @@ module = [
     "sentry.tasks.beacon",
     "sentry.tasks.codeowners.*",
     "sentry.tasks.commit_context",
+    "sentry.tasks.delete_seer_grouping_records",
     "sentry.tasks.embeddings_grouping.backfill_seer_grouping_records_for_project",
     "sentry.tasks.on_demand_metrics",
     "sentry.tasks.reprocessing2",
@@ -541,6 +542,7 @@ module = [
     "tests.sentry.snuba.metrics.test_metrics_query_layer.*",
     "tests.sentry.tasks.integrations.*",
     "tests.sentry.tasks.test_code_owners",
+    "tests.sentry.tasks.test_delete_seer_grouping_records",
     "tests.sentry.tasks.test_on_demand_metrics",
     "tests.sentry.types.*",
     "tests.sentry.types.test_actor",

src/sentry/seer/similarity/grouping_records.py

@@ -1,4 +1,7 @@
+from __future__ import annotations
+
 import logging
+from collections.abc import Sequence
 from typing import NotRequired, TypedDict
 
 from django.conf import settings
@@ -122,7 +125,7 @@ def delete_project_grouping_records(
         return False
 
 
-def delete_grouping_records_by_hash(project_id: int, hashes: list[str]) -> bool:
+def delete_grouping_records_by_hash(project_id: int, hashes: Sequence[str]) -> bool:
     extra = {"project_id": project_id, "hashes": hashes}
     try:
         body = {"project_id": project_id, "hash_list": hashes}

src/sentry/tasks/delete_seer_grouping_records.py

@@ -1,4 +1,5 @@
 import logging
+from collections.abc import Sequence
 from typing import Any
 
 from sentry import options
@@ -31,7 +32,7 @@
 )
 def delete_seer_grouping_records_by_hash(
     project_id: int,
-    hashes: list[str],
+    hashes: Sequence[str],
     last_deleted_index: int = 0,
     *args: Any,
     **kwargs: Any,
@@ -54,7 +55,7 @@ def delete_seer_grouping_records_by_hash(
 
 
 def call_delete_seer_grouping_records_by_hash(
-    group_ids: list[int],
+    group_ids: Sequence[int],
 ) -> None:
     project = None
     if group_ids:

tests/sentry/tasks/test_delete_seer_grouping_records.py

@@ -1,10 +1,9 @@
 from time import time
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 from sentry.models.grouphash import GroupHash
 from sentry.tasks.delete_seer_grouping_records import (
     call_delete_seer_grouping_records_by_hash,
-    call_seer_delete_project_grouping_records,
     delete_seer_grouping_records_by_hash,
 )
 from sentry.testutils.cases import TestCase
@@ -19,9 +18,9 @@ class TestDeleteSeerGroupingRecordsByHash(TestCase):
     )
     def test_delete_seer_grouping_records_by_hash_batches(
         self,
-        mock_delete_seer_grouping_records_by_hash_apply_async,
-        mock_delete_grouping_records_by_hash,
-    ):
+        mock_delete_seer_grouping_records_by_hash_apply_async: MagicMock,
+        mock_delete_grouping_records_by_hash: MagicMock,
+    ) -> None:
         """
         Test that when delete_seer_grouping_records_by_hash is called with over 20 hashes, it spawns
         another task with the end index of the previous batch.
@@ -33,50 +32,52 @@ def test_delete_seer_grouping_records_by_hash_batches(
             "args": [project_id, hashes, 100]
         }
 
-    @patch("sentry.tasks.delete_seer_grouping_records.logger")
-    def test_call_delete_seer_grouping_records_by_hash_simple(self, mock_logger):
+    @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.
+        """
         self.project.update_option("sentry:similarity_backfill_completed", int(time()))
 
-        group_ids, hashes = [], []
+        group_ids, expected_hashes = [], []
         for i in range(5):
             group = self.create_group(project=self.project)
             group_ids.append(group.id)
             group_hash = GroupHash.objects.create(
                 project=self.project, hash=str(i) * 32, group_id=group.id
             )
-            hashes.append(group_hash.hash)
+            expected_hashes.append(group_hash.hash)
+
         call_delete_seer_grouping_records_by_hash(group_ids)
-        mock_logger.info.assert_called_with(
-            "calling seer record deletion by hash",
-            extra={"project_id": self.project.id, "hashes": hashes},
-        )
 
-    @patch("sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash")
-    @patch("sentry.tasks.delete_seer_grouping_records.logger")
+        # Verify that the task was called with the correct parameters
+        mock_apply_async.assert_called_once_with(args=[self.project.id, expected_hashes, 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_no_hashes(
-        self, mock_logger, mock_delete_seer_grouping_records_by_hash
-    ):
+        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)
-        mock_logger.info.assert_called_with(
-            "calling seer record deletion by hash",
-            extra={"project_id": self.project.id, "hashes": []},
-        )
-        mock_delete_seer_grouping_records_by_hash.assert_not_called()
+        mock_apply_async.assert_not_called()
 
-    @patch("sentry.tasks.delete_seer_grouping_records.logger")
-    def test_call_delete_seer_grouping_records_by_hash_no_group_ids(self, mock_logger):
+    @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([])
-        mock_logger.info.assert_not_called()
-
-    @patch("sentry.tasks.delete_seer_grouping_records.delete_project_grouping_records")
-    def test_call_delete_project_and_delete_grouping_records(
-        self, mock_delete_project_grouping_records
-    ):
-        call_seer_delete_project_grouping_records(self.project.id)
-        mock_delete_project_grouping_records.assert_called_once()
+        mock_apply_async.assert_not_called()