diff --git a/pkg/types/ruletypes/listable_rule.go b/pkg/types/ruletypes/listable_rule.go index 49693c05b3..40930cf20b 100644 --- a/pkg/types/ruletypes/listable_rule.go +++ b/pkg/types/ruletypes/listable_rule.go @@ -95,14 +95,21 @@ var severityDisplayRank = map[string]int{ // SortListableRules sorts in place. Severity ranks the well-known values and // falls back to a lexical compare between custom ones; name compares -// case-insensitively. +// case-insensitively. Ties break on name then id (always ascending, so pages +// stay stable across requests) with order applied to the primary key only. func SortListableRules(rules []*ListableRule, sortBy ListSort, order ListOrder) { direction := 1 if order == ListOrderDesc { direction = -1 } slices.SortStableFunc(rules, func(a, b *ListableRule) int { - return direction * compareListableRules(a, b, sortBy) + if c := direction * compareListableRules(a, b, sortBy); c != 0 { + return c + } + if c := strings.Compare(strings.ToLower(a.AlertName), strings.ToLower(b.AlertName)); c != 0 { + return c + } + return strings.Compare(a.Id, b.Id) }) } diff --git a/pkg/types/ruletypes/listable_rule_test.go b/pkg/types/ruletypes/listable_rule_test.go index 80874f3e46..406190a3d1 100644 --- a/pkg/types/ruletypes/listable_rule_test.go +++ b/pkg/types/ruletypes/listable_rule_test.go @@ -89,6 +89,30 @@ func TestSortListableRules(t *testing.T) { order: ListOrderDesc, wantNames: []string{"new", "old"}, }, + { + name: "state desc ties break on name asc", + rules: []*ListableRule{ + listableRule("banana", StateFiring, "", base), + listableRule("zebra", StateDisabled, "", base), + listableRule("Apple", StateFiring, "", base), + listableRule("cherry", StateFiring, "", base), + }, + sortBy: ListSortState, + order: ListOrderDesc, + wantNames: []string{"Apple", "banana", "cherry", "zebra"}, + }, + { + name: "state asc flips buckets but tiebreak stays name asc", + rules: []*ListableRule{ + listableRule("banana", StateFiring, "", base), + listableRule("zebra", StateDisabled, "", base), + listableRule("Apple", StateFiring, "", base), + listableRule("cherry", StateFiring, "", base), + }, + sortBy: ListSortState, + order: ListOrderAsc, + wantNames: []string{"zebra", "Apple", "banana", "cherry"}, + }, } for _, tc := range testCases { @@ -99,6 +123,23 @@ func TestSortListableRules(t *testing.T) { } } +func TestSortListableRulesIdTiebreak(t *testing.T) { + base := time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC) + + for _, order := range []ListOrder{ListOrderAsc, ListOrderDesc} { + t.Run(order.StringValue(), func(t *testing.T) { + older := listableRule("dup", StateFiring, "", base) + older.Id = "01aaa" + newer := listableRule("dup", StateFiring, "", base) + newer.Id = "01bbb" + + rules := []*ListableRule{newer, older} + SortListableRules(rules, ListSortState, order) + assert.Equal(t, []string{"01aaa", "01bbb"}, []string{rules[0].Id, rules[1].Id}) + }) + } +} + func TestNewLabelPairsFromRawJSON(t *testing.T) { pairs := NewLabelPairsFromRawJSON([]string{ `{"team":"infra","severity":"critical"}`, diff --git a/tests/integration/tests/alerts/06_list_rules_v3.py b/tests/integration/tests/alerts/06_list_rules_v3.py index 910ceeb48f..0ca9e149e2 100644 --- a/tests/integration/tests/alerts/06_list_rules_v3.py +++ b/tests/integration/tests/alerts/06_list_rules_v3.py @@ -368,7 +368,8 @@ def test_sorting( "prom uptime probe", ] - # state display priority: inactive (rank 1) outranks disabled (rank 0) + # state display priority: inactive (rank 1) outranks disabled (rank 0); + # the four inactive rules tie on state and must break on name asc response = requests.get( signoz.self.host_configs["8080"].get(BASE_URL), params={"sort": "state", "order": "desc"}, @@ -376,9 +377,15 @@ def test_sorting( timeout=5, ) assert response.status_code == HTTPStatus.OK - names = [rule["alert"] for rule in response.json()["data"]["rules"]] - assert names[-1] == "checkout conversion drop", "disabled rule must sort last on state desc" + assert [rule["alert"] for rule in response.json()["data"]["rules"]] == [ + "infra cpu saturation", + "payment gateway errors", + "payment latency high", + "prom uptime probe", + "checkout conversion drop", + ] + # asc flips the state buckets but the name tiebreak stays ascending response = requests.get( signoz.self.host_configs["8080"].get(BASE_URL), params={"sort": "state", "order": "asc"}, @@ -386,10 +393,16 @@ def test_sorting( timeout=5, ) assert response.status_code == HTTPStatus.OK - assert response.json()["data"]["rules"][0]["alert"] == "checkout conversion drop" + assert [rule["alert"] for rule in response.json()["data"]["rules"]] == [ + "checkout conversion drop", + "infra cpu saturation", + "payment gateway errors", + "payment latency high", + "prom uptime probe", + ] # severity: known ranks first (critical > warning), then custom values - # lexically, then rules without severity + # lexically, then rules without severity tie and break on name asc response = requests.get( signoz.self.host_configs["8080"].get(BASE_URL), params={"sort": "severity", "order": "desc"}, @@ -397,9 +410,13 @@ def test_sorting( timeout=5, ) assert response.status_code == HTTPStatus.OK - names = [rule["alert"] for rule in response.json()["data"]["rules"]] - assert names[:3] == ["payment latency high", "payment gateway errors", "checkout conversion drop"] - assert set(names[3:]) == {"infra cpu saturation", "prom uptime probe"} + assert [rule["alert"] for rule in response.json()["data"]["rules"]] == [ + "payment latency high", + "payment gateway errors", + "checkout conversion drop", + "infra cpu saturation", + "prom uptime probe", + ] for order in ("asc", "desc"): response = requests.get( @@ -443,6 +460,27 @@ def test_pagination( assert len(flattened) == len(set(flattened)), "pages must be disjoint" assert set(flattened) == {r["alert"] for r in SEED_RULES} + # state sort is almost all ties (four inactive rules); the name/id tiebreak + # must keep the pages disjoint and in the same order on every request + tie_pages = [] + for offset in (0, 2, 4): + response = requests.get( + signoz.self.host_configs["8080"].get(BASE_URL), + params={"sort": "state", "order": "desc", "limit": 2, "offset": offset}, + headers={"Authorization": f"Bearer {token}"}, + timeout=5, + ) + assert response.status_code == HTTPStatus.OK + tie_pages.append([rule["alert"] for rule in response.json()["data"]["rules"]]) + + assert [name for page in tie_pages for name in page] == [ + "infra cpu saturation", + "payment gateway errors", + "payment latency high", + "prom uptime probe", + "checkout conversion drop", + ], "tied rows must not shuffle between page requests" + # a past-the-end offset returns an empty page but keeps the real total response = requests.get( signoz.self.host_configs["8080"].get(BASE_URL),