From b6092c4d055a2fe59c4a233d5aa3a9931f11c7f1 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 7 Apr 2026 13:19:03 -0700 Subject: [PATCH 1/5] fix(supergroups): Filter resolved groups from Seer response, not request The status filter was pre-filtering group IDs sent to Seer, but Seer returns all group_ids per supergroup regardless of what was requested. So the response still contained resolved groups. Move the filtering to the response side - collect all group_ids Seer returns, query their actual status, and strip out non-matching ones. Also fixes the response key from "supergroups" to "data" to match the actual Seer response model (GetSupergroupsByGroupIdsResponse). The previous code was silently a no-op. Co-Authored-By: Claude Opus 4.6 --- src/sentry/seer/signed_seer_api.py | 16 ++++++ .../organization_supergroups_by_group.py | 54 +++++++++++++------ .../test_organization_supergroups_by_group.py | 51 +++++++++++------- 3 files changed, 88 insertions(+), 33 deletions(-) diff --git a/src/sentry/seer/signed_seer_api.py b/src/sentry/seer/signed_seer_api.py index f69c3bb11a0d61..a88218af5efae3 100644 --- a/src/sentry/seer/signed_seer_api.py +++ b/src/sentry/seer/signed_seer_api.py @@ -386,6 +386,22 @@ class SupergroupsGetByGroupIdsRequest(TypedDict): group_ids: list[int] +class SupergroupDetailData(TypedDict): + id: int + title: str + summary: str + error_type: str + code_area: str + group_ids: list[int] + project_ids: list[int] + created_at: str + updated_at: str + + +class SupergroupsByGroupIdsResponse(TypedDict): + data: list[SupergroupDetailData] + + class ServiceMapUpdateRequest(TypedDict): organization_id: int nodes: list[dict[str, Any]] diff --git a/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py b/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py index c3cadbddde864c..ed4e3536ab30f2 100644 --- a/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py +++ b/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py @@ -16,6 +16,7 @@ from sentry.models.organization import Organization from sentry.seer.signed_seer_api import ( SeerViewerContext, + SupergroupsByGroupIdsResponse, make_supergroups_get_by_group_ids_request, ) @@ -55,21 +56,19 @@ def get(self, request: Request, organization: Organization) -> Response: status=status_codes.HTTP_400_BAD_REQUEST, ) - group_qs = Group.objects.filter( - id__in=group_ids, - project__organization=organization, - ) - status_param = request.GET.get("status") - if status_param is not None: - if status_param not in STATUS_QUERY_CHOICES: - return Response( - {"detail": "Invalid status parameter"}, - status=status_codes.HTTP_400_BAD_REQUEST, - ) - group_qs = group_qs.filter(status=STATUS_QUERY_CHOICES[status_param]) - - valid_group_ids = set(group_qs.values_list("id", flat=True)) + if status_param is not None and status_param not in STATUS_QUERY_CHOICES: + return Response( + {"detail": "Invalid status parameter"}, + status=status_codes.HTTP_400_BAD_REQUEST, + ) + + valid_group_ids = set( + Group.objects.filter( + id__in=group_ids, + project__organization=organization, + ).values_list("id", flat=True) + ) group_ids = [gid for gid in group_ids if gid in valid_group_ids] if not group_ids: @@ -90,4 +89,29 @@ def get(self, request: Request, organization: Organization) -> Response: status=response.status, ) - return Response(orjson.loads(response.data)) + data: SupergroupsByGroupIdsResponse = orjson.loads(response.data) + + # Seer returns all group_ids per supergroup regardless of status. + # We can't filter before the Seer call because Seer expands group_ids + # to include the full supergroup membership, not just the requested IDs. + # Instead, collect every group_id from the response, check status in + # bulk, and strip out non-matching ones. + if status_param is not None: + all_response_group_ids: set[int] = set() + for sg in data.get("data", []): + all_response_group_ids.update(sg["group_ids"]) + + matching_ids = set( + Group.objects.filter( + id__in=all_response_group_ids, + project__organization=organization, + status=STATUS_QUERY_CHOICES[status_param], + ).values_list("id", flat=True) + ) + + for sg in data.get("data", []): + sg["group_ids"] = [gid for gid in sg["group_ids"] if gid in matching_ids] + # Drop supergroups that have no matching groups after filtering + data["data"] = [sg for sg in data["data"] if sg["group_ids"]] + + return Response(data) diff --git a/tests/sentry/seer/supergroups/endpoints/test_organization_supergroups_by_group.py b/tests/sentry/seer/supergroups/endpoints/test_organization_supergroups_by_group.py index 7f239d45207ef3..40db5eb60e1dd4 100644 --- a/tests/sentry/seer/supergroups/endpoints/test_organization_supergroups_by_group.py +++ b/tests/sentry/seer/supergroups/endpoints/test_organization_supergroups_by_group.py @@ -1,5 +1,6 @@ from __future__ import annotations +from typing import Any from unittest.mock import MagicMock, patch import orjson @@ -7,8 +8,10 @@ from sentry.models.group import GroupStatus from sentry.testutils.cases import APITestCase +MOCK_PATCH = "sentry.seer.supergroups.endpoints.organization_supergroups_by_group.make_supergroups_get_by_group_ids_request" -def mock_seer_response(data): + +def mock_seer_response(data: dict[str, Any]) -> MagicMock: response = MagicMock() response.status = 200 response.data = orjson.dumps(data) @@ -26,21 +29,42 @@ def setUp(self): ) self.resolved_group = self.create_group(project=self.project, status=GroupStatus.RESOLVED) - @patch( - "sentry.seer.supergroups.endpoints.organization_supergroups_by_group.make_supergroups_get_by_group_ids_request" - ) - def test_status_filter(self, mock_seer): - mock_seer.return_value = mock_seer_response({"supergroups": []}) + @patch(MOCK_PATCH) + def test_status_filter_strips_resolved_from_response(self, mock_seer): + extra_unresolved = self.create_group(project=self.project, status=GroupStatus.UNRESOLVED) + mock_seer.return_value = mock_seer_response( + { + "data": [ + { + "id": 1, + "group_ids": [ + self.unresolved_group.id, + self.resolved_group.id, + extra_unresolved.id, + ], + "title": "kept", + }, + { + "id": 2, + "group_ids": [self.resolved_group.id], + "title": "dropped", + }, + ] + } + ) with self.feature("organizations:top-issues-ui"): - self.get_success_response( + response = self.get_success_response( self.organization.slug, group_id=[self.unresolved_group.id, self.resolved_group.id], status="unresolved", ) - body = mock_seer.call_args[0][0] - assert body["group_ids"] == [self.unresolved_group.id] + assert len(response.data["data"]) == 1 + assert response.data["data"][0]["group_ids"] == [ + self.unresolved_group.id, + extra_unresolved.id, + ] def test_status_filter_invalid(self): with self.feature("organizations:top-issues-ui"): @@ -50,12 +74,3 @@ def test_status_filter_invalid(self): status="bogus", status_code=400, ) - - def test_status_filter_all_filtered_out(self): - with self.feature("organizations:top-issues-ui"): - self.get_error_response( - self.organization.slug, - group_id=[self.resolved_group.id], - status="unresolved", - status_code=404, - ) From bd2c3051200ba046a5d0d9cc2f412c8345608bad Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 7 Apr 2026 13:33:40 -0700 Subject: [PATCH 2/5] ref(supergroups): Inline mock patch path in test --- .../endpoints/test_organization_supergroups_by_group.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/sentry/seer/supergroups/endpoints/test_organization_supergroups_by_group.py b/tests/sentry/seer/supergroups/endpoints/test_organization_supergroups_by_group.py index 40db5eb60e1dd4..4419d824602847 100644 --- a/tests/sentry/seer/supergroups/endpoints/test_organization_supergroups_by_group.py +++ b/tests/sentry/seer/supergroups/endpoints/test_organization_supergroups_by_group.py @@ -8,8 +8,6 @@ from sentry.models.group import GroupStatus from sentry.testutils.cases import APITestCase -MOCK_PATCH = "sentry.seer.supergroups.endpoints.organization_supergroups_by_group.make_supergroups_get_by_group_ids_request" - def mock_seer_response(data: dict[str, Any]) -> MagicMock: response = MagicMock() @@ -29,7 +27,9 @@ def setUp(self): ) self.resolved_group = self.create_group(project=self.project, status=GroupStatus.RESOLVED) - @patch(MOCK_PATCH) + @patch( + "sentry.seer.supergroups.endpoints.organization_supergroups_by_group.make_supergroups_get_by_group_ids_request" + ) def test_status_filter_strips_resolved_from_response(self, mock_seer): extra_unresolved = self.create_group(project=self.project, status=GroupStatus.UNRESOLVED) mock_seer.return_value = mock_seer_response( From 62fb704b4ff8be1dd51922efca17b20deff9c0cd Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 7 Apr 2026 14:06:17 -0700 Subject: [PATCH 3/5] ref(supergroups): Use direct dict access consistently for Seer response The TypedDict guarantees the "data" key exists. Defensive .get() was misleading and would silently return wrong data on a malformed response instead of failing loudly. --- .../endpoints/organization_supergroups_by_group.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py b/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py index ed4e3536ab30f2..e1d990ce17a391 100644 --- a/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py +++ b/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py @@ -98,7 +98,7 @@ def get(self, request: Request, organization: Organization) -> Response: # bulk, and strip out non-matching ones. if status_param is not None: all_response_group_ids: set[int] = set() - for sg in data.get("data", []): + for sg in data["data"]: all_response_group_ids.update(sg["group_ids"]) matching_ids = set( @@ -109,7 +109,7 @@ def get(self, request: Request, organization: Organization) -> Response: ).values_list("id", flat=True) ) - for sg in data.get("data", []): + for sg in data["data"]: sg["group_ids"] = [gid for gid in sg["group_ids"] if gid in matching_ids] # Drop supergroups that have no matching groups after filtering data["data"] = [sg for sg in data["data"] if sg["group_ids"]] From 96059ab1d5fefd22e26cd62d82f360559c63c40d Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 7 Apr 2026 14:27:45 -0700 Subject: [PATCH 4/5] ref(supergroups): Early return when no status filter --- .../organization_supergroups_by_group.py | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py b/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py index e1d990ce17a391..84060c338f73ec 100644 --- a/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py +++ b/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py @@ -91,27 +91,29 @@ def get(self, request: Request, organization: Organization) -> Response: data: SupergroupsByGroupIdsResponse = orjson.loads(response.data) + if not status_param: + return Response(data) + # Seer returns all group_ids per supergroup regardless of status. # We can't filter before the Seer call because Seer expands group_ids # to include the full supergroup membership, not just the requested IDs. # Instead, collect every group_id from the response, check status in # bulk, and strip out non-matching ones. - if status_param is not None: - all_response_group_ids: set[int] = set() - for sg in data["data"]: - all_response_group_ids.update(sg["group_ids"]) - - matching_ids = set( - Group.objects.filter( - id__in=all_response_group_ids, - project__organization=organization, - status=STATUS_QUERY_CHOICES[status_param], - ).values_list("id", flat=True) - ) + all_response_group_ids: set[int] = set() + for sg in data["data"]: + all_response_group_ids.update(sg["group_ids"]) + + matching_ids = set( + Group.objects.filter( + id__in=all_response_group_ids, + project__organization=organization, + status=STATUS_QUERY_CHOICES[status_param], + ).values_list("id", flat=True) + ) - for sg in data["data"]: - sg["group_ids"] = [gid for gid in sg["group_ids"] if gid in matching_ids] - # Drop supergroups that have no matching groups after filtering - data["data"] = [sg for sg in data["data"] if sg["group_ids"]] + for sg in data["data"]: + sg["group_ids"] = [gid for gid in sg["group_ids"] if gid in matching_ids] + # Drop supergroups that have no matching groups after filtering + data["data"] = [sg for sg in data["data"] if sg["group_ids"]] return Response(data) From e9f3e4b36ada9ef43917c4aa2926c13a2f63caae Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 7 Apr 2026 14:33:02 -0700 Subject: [PATCH 5/5] ref(supergroups): Use list for collecting response group IDs --- .../endpoints/organization_supergroups_by_group.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py b/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py index 84060c338f73ec..fc2c50de7a59be 100644 --- a/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py +++ b/src/sentry/seer/supergroups/endpoints/organization_supergroups_by_group.py @@ -99,9 +99,9 @@ def get(self, request: Request, organization: Organization) -> Response: # to include the full supergroup membership, not just the requested IDs. # Instead, collect every group_id from the response, check status in # bulk, and strip out non-matching ones. - all_response_group_ids: set[int] = set() + all_response_group_ids: list[int] = [] for sg in data["data"]: - all_response_group_ids.update(sg["group_ids"]) + all_response_group_ids.extend(sg["group_ids"]) matching_ids = set( Group.objects.filter(