From 800addb7533edb3b82da6156e95144c9f71f9711 Mon Sep 17 00:00:00 2001 From: FrameAutomata Date: Wed, 26 Aug 2026 17:43:16 -0500 Subject: [PATCH] refactor: give sortEndpointStats a strict comparator `sortEndpointStats` produced its descending order by negating the ascending comparison. For two rows that tie on the sort key that returns true for both (i, j) and (j, i), which is not the strict weak ordering sort.Slice documents a requirement for, so the result is formally unspecified. Descending is now the same comparison with the operands swapped. This is hygiene, not a bug fix. I could not make the old comparator misbehave: 2400 trials over slices of 8 to 20000 elements, 1 to 100 distinct values and three input patterns produced zero misordered results and zero lost elements, so Go's pdqsort tolerates the `>=` comparator in practice today. What the change buys is not depending on that tolerance. No test, because none can distinguish the two versions -- any behaviour a test could asserts holds for the old comparator too, which is what the probe above measured. The sibling sort in task.repository.go already used the strict form; this brings the two endpoint repositories in line with it. Co-Authored-By: Claude Opus 5 (1M context) --- .../telemetry/duckdb/endpoint.repository.go | 26 ++++++++++--------- .../telemetry/sqlite/endpoint.repository.go | 26 ++++++++++--------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/backend/app/repositories/telemetry/duckdb/endpoint.repository.go b/backend/app/repositories/telemetry/duckdb/endpoint.repository.go index f93ea8a6..7357a015 100644 --- a/backend/app/repositories/telemetry/duckdb/endpoint.repository.go +++ b/backend/app/repositories/telemetry/duckdb/endpoint.repository.go @@ -823,27 +823,29 @@ func sortEndpointStats(stats []models.EndpointStats, orderBy string, sortDirecti desc := sortDirection != "asc" sort.Slice(stats, func(i, j int) bool { - var less bool + // Descending is the ascending comparison with the operands swapped. + // Negating the ascending result instead would return true for both + // (i, j) and (j, i) whenever two rows tie, which is not the strict + // weak ordering sort.Slice documents a requirement for. + if desc { + i, j = j, i + } switch orderBy { case "count": - less = stats[i].Count < stats[j].Count + return stats[i].Count < stats[j].Count case "p50_duration": - less = stats[i].P50Duration < stats[j].P50Duration + return stats[i].P50Duration < stats[j].P50Duration case "p95_duration": - less = stats[i].P95Duration < stats[j].P95Duration + return stats[i].P95Duration < stats[j].P95Duration case "p99_duration": - less = stats[i].P99Duration < stats[j].P99Duration + return stats[i].P99Duration < stats[j].P99Duration case "avg_duration": - less = stats[i].AvgDuration < stats[j].AvgDuration + return stats[i].AvgDuration < stats[j].AvgDuration case "last_seen": - less = stats[i].LastSeen.Before(stats[j].LastSeen) + return stats[i].LastSeen.Before(stats[j].LastSeen) default: - less = stats[i].Impact < stats[j].Impact - } - if desc { - return !less + return stats[i].Impact < stats[j].Impact } - return less }) } diff --git a/backend/app/repositories/telemetry/sqlite/endpoint.repository.go b/backend/app/repositories/telemetry/sqlite/endpoint.repository.go index 05474e3e..41577084 100644 --- a/backend/app/repositories/telemetry/sqlite/endpoint.repository.go +++ b/backend/app/repositories/telemetry/sqlite/endpoint.repository.go @@ -856,27 +856,29 @@ func sortEndpointStats(stats []models.EndpointStats, orderBy string, sortDirecti desc := sortDirection != "asc" sort.Slice(stats, func(i, j int) bool { - var less bool + // Descending is the ascending comparison with the operands swapped. + // Negating the ascending result instead would return true for both + // (i, j) and (j, i) whenever two rows tie, which is not the strict + // weak ordering sort.Slice documents a requirement for. + if desc { + i, j = j, i + } switch orderBy { case "count": - less = stats[i].Count < stats[j].Count + return stats[i].Count < stats[j].Count case "p50_duration": - less = stats[i].P50Duration < stats[j].P50Duration + return stats[i].P50Duration < stats[j].P50Duration case "p95_duration": - less = stats[i].P95Duration < stats[j].P95Duration + return stats[i].P95Duration < stats[j].P95Duration case "p99_duration": - less = stats[i].P99Duration < stats[j].P99Duration + return stats[i].P99Duration < stats[j].P99Duration case "avg_duration": - less = stats[i].AvgDuration < stats[j].AvgDuration + return stats[i].AvgDuration < stats[j].AvgDuration case "last_seen": - less = stats[i].LastSeen.Before(stats[j].LastSeen) + return stats[i].LastSeen.Before(stats[j].LastSeen) default: - less = stats[i].Impact < stats[j].Impact - } - if desc { - return !less + return stats[i].Impact < stats[j].Impact } - return less }) }