From 695654e275e794b16b67cbab88bb25a96dd07885 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 13 Nov 2025 13:40:13 -0800 Subject: [PATCH 1/9] add replay id to ordering --- src/sentry/replays/usecases/query/__init__.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/sentry/replays/usecases/query/__init__.py b/src/sentry/replays/usecases/query/__init__.py index dcab9e74dadf80..32c7bd49c881ee 100644 --- a/src/sentry/replays/usecases/query/__init__.py +++ b/src/sentry/replays/usecases/query/__init__.py @@ -460,10 +460,13 @@ def execute_query(query: Query, tenant_id: dict[str, int], referrer: str) -> Map def handle_ordering(config: dict[str, Expression], sort: str) -> list[OrderBy]: - if sort.startswith("-"): - return [OrderBy(_get_sort_column(config, sort[1:]), Direction.DESC)] - else: - return [OrderBy(_get_sort_column(config, sort), Direction.ASC)] + direction = Direction.DESC if sort.startswith("-") else Direction.ASC + column_name = sort[1:] if sort.startswith("-") else sort + + return [ + OrderBy(_get_sort_column(config, column_name), direction), + OrderBy(Column("replay_id"), direction), + ] def _get_sort_column(config: dict[str, Expression], column_name: str) -> Function: From c891ccca148d717b2115ec1f501cf7c84772f6a5 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 13 Nov 2025 19:14:45 -0800 Subject: [PATCH 2/9] add tiebreaker arg --- src/sentry/replays/usecases/query/__init__.py | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/sentry/replays/usecases/query/__init__.py b/src/sentry/replays/usecases/query/__init__.py index 32c7bd49c881ee..c07f42811ec08e 100644 --- a/src/sentry/replays/usecases/query/__init__.py +++ b/src/sentry/replays/usecases/query/__init__.py @@ -344,7 +344,11 @@ def _query_using_scalar_strategy( try: where = handle_search_filters(scalar_search_config, search_filters) - orderby = handle_ordering(agg_sort_config, sort or "-" + DEFAULT_SORT_FIELD) + orderby = handle_ordering( + agg_sort_config, + sort or "-" + DEFAULT_SORT_FIELD, + tiebreaker="replay_id", # Ensure stable sort within the same score + ) except RetryAggregated: return _query_using_aggregated_strategy( search_filters, @@ -378,7 +382,11 @@ def _query_using_aggregated_strategy( period_start: datetime, period_stop: datetime, ): - orderby = handle_ordering(agg_sort_config, sort or "-" + DEFAULT_SORT_FIELD) + orderby = handle_ordering( + agg_sort_config, + sort or "-" + DEFAULT_SORT_FIELD, + tiebreaker="replay_id", # Ensure stable sort within the same score + ) having: list[Condition] = handle_search_filters(agg_search_config, search_filters) having.append(Condition(Function("min", parameters=[Column("segment_id")]), Op.EQ, 0)) @@ -459,14 +467,16 @@ def execute_query(query: Query, tenant_id: dict[str, int], referrer: str) -> Map raise -def handle_ordering(config: dict[str, Expression], sort: str) -> list[OrderBy]: +def handle_ordering( + config: dict[str, Expression], sort: str, tiebreaker: str | None = None +) -> list[OrderBy]: direction = Direction.DESC if sort.startswith("-") else Direction.ASC - column_name = sort[1:] if sort.startswith("-") else sort + bare_orderby = sort.lstrip("-") - return [ - OrderBy(_get_sort_column(config, column_name), direction), - OrderBy(Column("replay_id"), direction), - ] + orderby = [OrderBy(_get_sort_column(config, bare_orderby), direction)] + if tiebreaker: + orderby.append(OrderBy(Column(tiebreaker), direction)) + return orderby def _get_sort_column(config: dict[str, Expression], column_name: str) -> Function: From 73dd8280ca07772abee28078407e09926ef74332 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 13 Nov 2025 19:16:37 -0800 Subject: [PATCH 3/9] rename --- src/sentry/replays/usecases/query/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/replays/usecases/query/__init__.py b/src/sentry/replays/usecases/query/__init__.py index c07f42811ec08e..5e63a5efc54320 100644 --- a/src/sentry/replays/usecases/query/__init__.py +++ b/src/sentry/replays/usecases/query/__init__.py @@ -471,9 +471,9 @@ def handle_ordering( config: dict[str, Expression], sort: str, tiebreaker: str | None = None ) -> list[OrderBy]: direction = Direction.DESC if sort.startswith("-") else Direction.ASC - bare_orderby = sort.lstrip("-") + bare_sort = sort[1:] if sort.startswith("-") else sort - orderby = [OrderBy(_get_sort_column(config, bare_orderby), direction)] + orderby = [OrderBy(_get_sort_column(config, bare_sort), direction)] if tiebreaker: orderby.append(OrderBy(Column(tiebreaker), direction)) return orderby From 0e01879ad64f020c0a6f4a7fece40a8f1af44667 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 14 Nov 2025 09:26:53 -0800 Subject: [PATCH 4/9] add test --- .../test_organization_replay_index.py | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/sentry/replays/endpoints/test_organization_replay_index.py b/tests/sentry/replays/endpoints/test_organization_replay_index.py index 05e308ee574ede..dec91a23fa301d 100644 --- a/tests/sentry/replays/endpoints/test_organization_replay_index.py +++ b/tests/sentry/replays/endpoints/test_organization_replay_index.py @@ -2442,3 +2442,38 @@ def test_viewed_by_denylist(self) -> None: response.json()["detail"]["message"] == "Viewed by search has been disabled for your project due to a data irregularity." ) + + def test_get_replays_sort_with_duplicate_timestamps(self) -> None: + """Test that replays with identical timestamps have deterministic sort order.""" + project = self.create_project(teams=[self.team]) + + base_timestamp = datetime.datetime.now(datetime.UTC) - datetime.timedelta(minutes=5) + + replay_ids = [ + "a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d", + "f9e8d7c6-b5a4-4938-8271-6a5b4c3d2e1f", + "12345678-90ab-4cde-8f12-345678901234", + "deadbeef-cafe-4bab-8e00-decafbad0000", + "87654321-0fed-4cba-8987-654321fedcba", + ] + + for replay_id in replay_ids: + self.store_replays(mock_replay(base_timestamp, project.id, replay_id, segment_id=0)) + + with self.feature(self.features): + response = self.client.get( + self.url, + {"project": project.id, "sort": "started_at", "statsPeriod": "1h"}, + ) + assert response.status_code == 200 + asc_order = [r["id"] for r in response.json()["data"]] + + response = self.client.get( + self.url, + {"project": project.id, "sort": "-started_at", "statsPeriod": "1h"}, + ) + assert response.status_code == 200 + desc_order = [r["id"] for r in response.json()["data"]] + + assert desc_order == list(reversed(asc_order)) + assert set(asc_order) == {rid.replace("-", "") for rid in replay_ids} From 3aa189cbe725ab5ae034e5f7f0f0bd3a2b9cbf65 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 14 Nov 2025 09:28:41 -0800 Subject: [PATCH 5/9] comment --- .../sentry/replays/endpoints/test_organization_replay_index.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/replays/endpoints/test_organization_replay_index.py b/tests/sentry/replays/endpoints/test_organization_replay_index.py index dec91a23fa301d..1c4ceaed6bc2b3 100644 --- a/tests/sentry/replays/endpoints/test_organization_replay_index.py +++ b/tests/sentry/replays/endpoints/test_organization_replay_index.py @@ -2444,7 +2444,7 @@ def test_viewed_by_denylist(self) -> None: ) def test_get_replays_sort_with_duplicate_timestamps(self) -> None: - """Test that replays with identical timestamps have deterministic sort order.""" + """Test that replays with identical timestamps have deterministic order when ordering by started_at.""" project = self.create_project(teams=[self.team]) base_timestamp = datetime.datetime.now(datetime.UTC) - datetime.timedelta(minutes=5) From 420e5f5a9de70a344b3331541d3d85336aafa42b Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 14 Nov 2025 09:59:09 -0800 Subject: [PATCH 6/9] fix typing --- .../test_organization_replay_index.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/sentry/replays/endpoints/test_organization_replay_index.py b/tests/sentry/replays/endpoints/test_organization_replay_index.py index 1c4ceaed6bc2b3..4a950d41339a19 100644 --- a/tests/sentry/replays/endpoints/test_organization_replay_index.py +++ b/tests/sentry/replays/endpoints/test_organization_replay_index.py @@ -2461,19 +2461,19 @@ def test_get_replays_sort_with_duplicate_timestamps(self) -> None: self.store_replays(mock_replay(base_timestamp, project.id, replay_id, segment_id=0)) with self.feature(self.features): - response = self.client.get( - self.url, - {"project": project.id, "sort": "started_at", "statsPeriod": "1h"}, + response = self.get_success_response( + self.organization.slug, + project=project.id, + sort="started_at", ) - assert response.status_code == 200 - asc_order = [r["id"] for r in response.json()["data"]] + asc_order = [r["id"] for r in response.data["data"]] - response = self.client.get( - self.url, - {"project": project.id, "sort": "-started_at", "statsPeriod": "1h"}, + response = self.get_success_response( + self.organization.slug, + project=project.id, + sort="-started_at", ) - assert response.status_code == 200 - desc_order = [r["id"] for r in response.json()["data"]] + desc_order = [r["id"] for r in response.data["data"]] assert desc_order == list(reversed(asc_order)) assert set(asc_order) == {rid.replace("-", "") for rid in replay_ids} From b8d508299c535d69151797b749b9b10ec657df05 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 14 Nov 2025 10:06:07 -0800 Subject: [PATCH 7/9] finalize test --- .../test_organization_replay_index.py | 62 ++++++++----------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/tests/sentry/replays/endpoints/test_organization_replay_index.py b/tests/sentry/replays/endpoints/test_organization_replay_index.py index 4a950d41339a19..3ae7308099448f 100644 --- a/tests/sentry/replays/endpoints/test_organization_replay_index.py +++ b/tests/sentry/replays/endpoints/test_organization_replay_index.py @@ -485,6 +485,33 @@ def test_get_replays_duration_sorted(self) -> None: assert response_data["data"][0]["id"] == replay2_id assert response_data["data"][1]["id"] == replay1_id + def test_get_replays_duration_sorted_tiebreaker(self) -> None: + """Test that replays with identical durations have deterministic order when ordering by started_at.""" + project = self.create_project(teams=[self.team]) + replay_ids = [uuid.uuid4().hex for _ in range(5)] + base_timestamp = datetime.datetime.now(datetime.UTC) - datetime.timedelta(minutes=5) + + for replay_id in replay_ids: + self.store_replays(mock_replay(base_timestamp, project.id, replay_id, segment_id=0)) + + with self.feature(self.features): + response = self.get_success_response( + self.organization.slug, + project=project.id, + sort="started_at", + ) + asc_order = [r["id"] for r in response.data["data"]] + + response = self.get_success_response( + self.organization.slug, + project=project.id, + sort="-started_at", + ) + desc_order = [r["id"] for r in response.data["data"]] + + assert desc_order == list(reversed(asc_order)) + assert set(asc_order) == {rid.replace("-", "") for rid in replay_ids} + def test_get_replays_pagination(self) -> None: """Test replays can be paginated.""" project = self.create_project(teams=[self.team]) @@ -2442,38 +2469,3 @@ def test_viewed_by_denylist(self) -> None: response.json()["detail"]["message"] == "Viewed by search has been disabled for your project due to a data irregularity." ) - - def test_get_replays_sort_with_duplicate_timestamps(self) -> None: - """Test that replays with identical timestamps have deterministic order when ordering by started_at.""" - project = self.create_project(teams=[self.team]) - - base_timestamp = datetime.datetime.now(datetime.UTC) - datetime.timedelta(minutes=5) - - replay_ids = [ - "a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d", - "f9e8d7c6-b5a4-4938-8271-6a5b4c3d2e1f", - "12345678-90ab-4cde-8f12-345678901234", - "deadbeef-cafe-4bab-8e00-decafbad0000", - "87654321-0fed-4cba-8987-654321fedcba", - ] - - for replay_id in replay_ids: - self.store_replays(mock_replay(base_timestamp, project.id, replay_id, segment_id=0)) - - with self.feature(self.features): - response = self.get_success_response( - self.organization.slug, - project=project.id, - sort="started_at", - ) - asc_order = [r["id"] for r in response.data["data"]] - - response = self.get_success_response( - self.organization.slug, - project=project.id, - sort="-started_at", - ) - desc_order = [r["id"] for r in response.data["data"]] - - assert desc_order == list(reversed(asc_order)) - assert set(asc_order) == {rid.replace("-", "") for rid in replay_ids} From 031439340ace71694a31739e9bdf4eda2cf5e9a4 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 14 Nov 2025 10:11:51 -0800 Subject: [PATCH 8/9] finalize test for real --- .../test_organization_replay_index.py | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/tests/sentry/replays/endpoints/test_organization_replay_index.py b/tests/sentry/replays/endpoints/test_organization_replay_index.py index 3ae7308099448f..1966c3686d92dc 100644 --- a/tests/sentry/replays/endpoints/test_organization_replay_index.py +++ b/tests/sentry/replays/endpoints/test_organization_replay_index.py @@ -486,31 +486,28 @@ def test_get_replays_duration_sorted(self) -> None: assert response_data["data"][1]["id"] == replay1_id def test_get_replays_duration_sorted_tiebreaker(self) -> None: - """Test that replays with identical durations have deterministic order when ordering by started_at.""" + """Test that replays with identical durations have deterministic order when ordering by duration.""" project = self.create_project(teams=[self.team]) replay_ids = [uuid.uuid4().hex for _ in range(5)] - base_timestamp = datetime.datetime.now(datetime.UTC) - datetime.timedelta(minutes=5) + + timestamp_start = datetime.datetime.now() - datetime.timedelta(seconds=10) + timestamp_end = datetime.datetime.now() - datetime.timedelta(seconds=5) for replay_id in replay_ids: - self.store_replays(mock_replay(base_timestamp, project.id, replay_id, segment_id=0)) + self.store_replays(mock_replay(timestamp_start, project.id, replay_id, segment_id=0)) + self.store_replays(mock_replay(timestamp_end, project.id, replay_id, segment_id=1)) with self.feature(self.features): - response = self.get_success_response( - self.organization.slug, - project=project.id, - sort="started_at", - ) - asc_order = [r["id"] for r in response.data["data"]] + response = self.client.get(self.url + "?orderBy=duration") + assert response.status_code == 200, response + asc_order = [r["id"] for r in response.json()["data"]] - response = self.get_success_response( - self.organization.slug, - project=project.id, - sort="-started_at", - ) - desc_order = [r["id"] for r in response.data["data"]] + response = self.client.get(self.url + "?orderBy=-duration") + assert response.status_code == 200, response + desc_order = [r["id"] for r in response.json()["data"]] assert desc_order == list(reversed(asc_order)) - assert set(asc_order) == {rid.replace("-", "") for rid in replay_ids} + assert set(asc_order) == set(replay_ids) def test_get_replays_pagination(self) -> None: """Test replays can be paginated.""" From 642a7ff9c54b7080cda5873b82aa9a42c54670db Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 14 Nov 2025 11:07:55 -0800 Subject: [PATCH 9/9] fix comments --- src/sentry/replays/usecases/query/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/replays/usecases/query/__init__.py b/src/sentry/replays/usecases/query/__init__.py index 5e63a5efc54320..fa9f469fa08e76 100644 --- a/src/sentry/replays/usecases/query/__init__.py +++ b/src/sentry/replays/usecases/query/__init__.py @@ -347,7 +347,7 @@ def _query_using_scalar_strategy( orderby = handle_ordering( agg_sort_config, sort or "-" + DEFAULT_SORT_FIELD, - tiebreaker="replay_id", # Ensure stable sort within the same score + tiebreaker="replay_id", # Ensure stable sort when ordering by column with duplicates ) except RetryAggregated: return _query_using_aggregated_strategy( @@ -385,7 +385,7 @@ def _query_using_aggregated_strategy( orderby = handle_ordering( agg_sort_config, sort or "-" + DEFAULT_SORT_FIELD, - tiebreaker="replay_id", # Ensure stable sort within the same score + tiebreaker="replay_id", # Ensure stable sort when ordering by column with duplicates ) having: list[Condition] = handle_search_filters(agg_search_config, search_filters)