From 74ab8970fa444c2a4fb4cf96e087737dde153419 Mon Sep 17 00:00:00 2001
From: jojosenthusiast <sweetenedbread@gmail.com>
Date: Mon, 29 Jun 2026 11:41:22 -0600
Subject: [PATCH] fix(audit): tiebreak audit check sort by id for deterministic
 order

The audit viewer sorts checks by status then area, but ties on
(status, area) fell through to insertion order. The audit skill writes
the ledger in whatever sequence it happens to resolve checks, so two
runs with identical findings rendered in different positions and the
list appeared to rotate between runs.

Add id as a final tiebreaker in both `sortChecks` and
`groupChecksByArea` so render order is a pure function of the ledger
contents. Adds a regression test covering both call sites.

Scoped fix: the audit logic for `capture-event-names-static` and
`capture-uses-proxy` lives in the external context-mill skill and is
out of scope for this repo.

Refs: PostHog/wizard#736
---
 .../AuditChecksViewer/__tests__/sort.test.ts  | 54 +++++++++++++++++++
 .../screens/audit/AuditChecksViewer/sort.ts   | 19 +++++--
 2 files changed, 68 insertions(+), 5 deletions(-)
 create mode 100644 src/ui/tui/screens/audit/AuditChecksViewer/__tests__/sort.test.ts

diff --git a/src/ui/tui/screens/audit/AuditChecksViewer/__tests__/sort.test.ts b/src/ui/tui/screens/audit/AuditChecksViewer/__tests__/sort.test.ts
new file mode 100644
index 00000000..9489d42e
--- /dev/null
+++ b/src/ui/tui/screens/audit/AuditChecksViewer/__tests__/sort.test.ts
@@ -0,0 +1,54 @@
+import { describe, expect, it } from 'vitest';
+import type { AuditCheck } from '@lib/programs/audit/types';
+import { groupChecksByArea, sortChecks } from '../sort.js';
+
+/**
+ * Regression for PostHog/wizard#736 — audit findings rotate between runs
+ * because the viewer's sort had no tiebreaker for checks that shared a
+ * (status, area) pair. The skill writes the ledger in whatever order it
+ * happens to resolve checks, so identical findings rendered in different
+ * positions on different runs. The contract here: render order is a pure
+ * function of the ledger contents, never of the write order.
+ */
+describe('audit check ordering — deterministic regardless of input order', () => {
+  const a: AuditCheck = {
+    id: 'capture-event-names-static',
+    area: 'Event Capture',
+    label: 'Event names are static',
+    status: 'error',
+  };
+  const b: AuditCheck = {
+    id: 'capture-uses-proxy',
+    area: 'Event Capture',
+    label: 'Captures route through a reverse proxy',
+    status: 'error',
+  };
+
+  it('sortChecks orders same-status, same-area checks by id', () => {
+    const forward = sortChecks([a, b]).map((c) => c.id);
+    const reversed = sortChecks([b, a]).map((c) => c.id);
+    expect(forward).toEqual(reversed);
+    expect(forward).toEqual([
+      'capture-event-names-static',
+      'capture-uses-proxy',
+    ]);
+  });
+
+  it('groupChecksByArea orders within-area same-status checks by id', () => {
+    const eventCapture = (checks: AuditCheck[]) => {
+      const group = groupChecksByArea(checks).find(
+        (g) => g.area === 'Event Capture',
+      );
+      if (!group) throw new Error('Event Capture group missing');
+      return group.checks.map((c) => c.id);
+    };
+
+    const forward = eventCapture([a, b]);
+    const reversed = eventCapture([b, a]);
+    expect(forward).toEqual(reversed);
+    expect(forward).toEqual([
+      'capture-event-names-static',
+      'capture-uses-proxy',
+    ]);
+  });
+});
diff --git a/src/ui/tui/screens/audit/AuditChecksViewer/sort.ts b/src/ui/tui/screens/audit/AuditChecksViewer/sort.ts
index 28f615f8..147fa7b8 100644
--- a/src/ui/tui/screens/audit/AuditChecksViewer/sort.ts
+++ b/src/ui/tui/screens/audit/AuditChecksViewer/sort.ts
@@ -28,12 +28,17 @@ function areaRank(area: string): number {
   return idx === -1 ? AREA_ORDER.length : idx;
 }
 
-/** Issues at the top (error → warning → suggestion), then passes, then pending todos. */
+/** Issues at the top (error → warning → suggestion), then passes, then pending todos.
+ *  Ties broken by `id` so two ledgers with identical contents always render in
+ *  identical order, regardless of the agent's write sequence
+ *  (PostHog/wizard#736 — non-determinism). */
 export function sortChecks(checks: ReadonlyArray<AuditCheck>): AuditCheck[] {
   return [...checks].sort((a, b) => {
     const da = STATUS_ORDER[a.status] - STATUS_ORDER[b.status];
     if (da !== 0) return da;
-    return a.area.localeCompare(b.area);
+    const dArea = a.area.localeCompare(b.area);
+    if (dArea !== 0) return dArea;
+    return a.id.localeCompare(b.id);
   });
 }
 
@@ -55,9 +60,13 @@ export function groupChecksByArea(
   }
   const groups: AreaGroup[] = [];
   for (const [area, areaChecks] of byArea) {
-    const sorted = [...areaChecks].sort(
-      (a, b) => STATUS_ORDER[a.status] - STATUS_ORDER[b.status],
-    );
+    const sorted = [...areaChecks].sort((a, b) => {
+      const ds = STATUS_ORDER[a.status] - STATUS_ORDER[b.status];
+      if (ds !== 0) return ds;
+      // Tiebreak by id so identical ledgers render in identical order,
+      // regardless of the agent's write sequence (#736).
+      return a.id.localeCompare(b.id);
+    });
     const resolved = sorted.filter((c) => c.status !== 'pending').length;
     groups.push({
       area,