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[] { 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,