feat(incidents): the timeline leads with what is worst, not what is newest

.workhorse/specs/monitoring/incidents.md

@@ -28,6 +28,10 @@ Lingering damps reporter flapping, not operator action: a last failure leaving t
 The membership history — which issues joined and left, and when — is kept and presented as the incident's timeline.
 An issue can leave and rejoin the same incident.
 
+The timeline leads with what is worst rather than what is newest: issues are ordered by effective result, most severe first, and issues sharing a result are ordered most recent first.
+An issue with no recorded result is ordered below every graded one.
+Notes are ordered most recent first and sit below every issue.
+
 Operator actions that change what counts (monitoring toggles, group membership changes, policy and silence changes) re-evaluate the affected issues' incident membership.
 
 Membership evaluation is asynchronous. A report records its issue state immediately; the resulting open, join, leave, or close follows within a short bounded delay rather than synchronously with the report. Membership is therefore eventually consistent with the current issue state.

private-web/e2e/incident-ordering.spec.ts

@@ -0,0 +1,129 @@
+import type { Page } from "@playwright/test";
+import { expect, test } from "./test-fixtures";
+import {
+	resetSeededTables,
+	seedIncident,
+	seedIncidentNote,
+	seedIssue,
+	seedServer,
+	seedServerGroup,
+} from "./seed";
+
+// An incident's timeline leads with what is failing. Ordering is severity
+// first, most recent second, and notes sit below every issue — an operator
+// opening a page mid-incident shouldn't have to read a chronology to find
+// the failure. Each fixture below is seeded so that ordering by time alone
+// would produce the opposite order, so a passing assertion can only mean the
+// severity rank is being applied.
+test.describe("incident timeline ordering", () => {
+	test.beforeEach(async ({ sql }) => {
+		await resetSeededTables(sql);
+	});
+
+	async function seedTimeline(sql: Parameters<typeof seedIssue>[0]) {
+		const group = await seedServerGroup(sql, { name: "ordering-group" });
+		const server = await seedServer(sql, {
+			name: "ordering-server",
+			kind: "central",
+			rank: "production",
+			groupId: group.id,
+		});
+
+		const minutesAgo = (n: number) =>
+			new Date(Date.now() - n * 60_000).toISOString();
+
+		// Assertions key off the message, which is the row's always-visible
+		// summary text. The check name only shows in the expanded provenance
+		// line, and a recovered issue renders collapsed.
+		//
+		// Deliberately inverted: the newest thing is the least severe, so
+		// time-ordering alone would put the warning above the failure.
+		const warning = await seedIssue(sql, {
+			serverId: server.id,
+			ref: "health/freshwarning",
+			severity: "warning",
+			message: "marker-warning-newest",
+		});
+		const failure = await seedIssue(sql, {
+			serverId: server.id,
+			ref: "health/oldfailure",
+			severity: "error",
+			message: "marker-failure-oldest",
+		});
+		const recovered = await seedIssue(sql, {
+			serverId: server.id,
+			ref: "health/middlerecovered",
+			active: false,
+			message: "marker-recovered-middle",
+		});
+
+		const incident = await seedIncident(sql, {
+			serverGroupId: group.id,
+			openedAt: minutesAgo(60),
+			issues: [
+				{ issueId: warning.id, joinedAt: minutesAgo(5) },
+				{ issueId: recovered.id, joinedAt: minutesAgo(30) },
+				{ issueId: failure.id, joinedAt: minutesAgo(60) },
+			],
+		});
+		// Newest entry on the page, and still below every issue.
+		await seedIncidentNote(sql, {
+			incidentId: incident.id,
+			body: "notewrittenlast",
+			createdAt: minutesAgo(1),
+		});
+		return { group, server, incident };
+	}
+
+	/** Where each marker first appears in the rendered page text. Rows render
+	 * in order, so a marker unique to one row first appears inside it, and
+	 * comparing offsets compares row positions. */
+	async function positionsOf(page: Page, markers: string[]): Promise<number[]> {
+		const body = await page.locator("body").innerText();
+		return markers.map((marker) => {
+			const at = body.indexOf(marker);
+			expect(at, `${marker} should be on the page`).toBeGreaterThan(-1);
+			return at;
+		});
+	}
+
+	test("failures sort above warnings, recoveries and notes", async ({
+		page,
+		sql,
+	}) => {
+		const { incident } = await seedTimeline(sql);
+
+		await page.goto(`/incidents/${incident.id}`);
+		await expect(page.getByText("marker-failure-oldest").first()).toBeVisible();
+
+		const [failure, warning, recovered, note] = await positionsOf(page, [
+			"marker-failure-oldest",
+			"marker-warning-newest",
+			"marker-recovered-middle",
+			"notewrittenlast",
+		]);
+
+		expect(failure, "the failure leads, despite being the oldest").toBeLessThan(
+			warning,
+		);
+		expect(warning, "the warning outranks the recovered check").toBeLessThan(
+			recovered,
+		);
+		expect(recovered, "notes sit below every issue").toBeLessThan(note);
+	});
+
+	test("the issues filter keeps the same order", async ({ page, sql }) => {
+		const { incident } = await seedTimeline(sql);
+
+		await page.goto(`/incidents/${incident.id}`);
+		await page.getByText(/^Issues \(/).click();
+		await expect(page.getByText("marker-failure-oldest").first()).toBeVisible();
+
+		const [failure, warning] = await positionsOf(page, [
+			"marker-failure-oldest",
+			"marker-warning-newest",
+		]);
+		expect(failure).toBeLessThan(warning);
+		await expect(page.getByText("notewrittenlast")).toHaveCount(0);
+	});
+});

private-web/e2e/seed.ts

@@ -629,6 +629,32 @@ export async function seedIncident(
 	return { id };
 }
 
+/** Add an operator note to an incident's timeline. */
+export async function seedIncidentNote(
+	sql: Sql,
+	opts: {
+		incidentId: string;
+		author?: string;
+		body?: string;
+		/** ISO 8601; defaults to NOW(). */
+		createdAt?: string;
+	},
+): Promise<{ id: string }> {
+	const id = randomUUID();
+	await sql.query(
+		`INSERT INTO incident_notes (id, incident_id, author, body, created_at)
+		 VALUES ($1, $2, $3, $4, COALESCE($5::timestamptz, NOW()))`,
+		[
+			id,
+			opts.incidentId,
+			opts.author ?? "operator@example.com",
+			opts.body ?? "a note",
+			opts.createdAt ?? null,
+		],
+	);
+	return { id };
+}
+
 export interface SeededVersion {
 	id: string;
 	major: number;

private-web/src/routes/IncidentDetail.tsx

@@ -30,12 +30,15 @@ import ResolverAvatar from "../components/ResolverAvatar";
 import { usePageTitle } from "../hooks/usePageTitle";
 import { humanDuration } from "../lib/humanDuration";
 import {
+	CHECK_RESULT_ORDER,
 	RESOLVED_REASONS,
 	RESOLVED_REASON_LABEL,
 	isIncidentLingering,
+	type CheckResult,
 	type IncidentIssueData,
 	type IncidentNoteData,
 	type IncidentWithIssues,
+	type IssueData,
 	type ResolvedReason,
 } from "../types";
 
@@ -404,6 +407,24 @@ type TimelineEntry =
 	| { kind: "issue"; at: number; issue: IncidentIssueData }
 	| { kind: "note"; at: number; note: IncidentNoteData };
 
+const CHECK_RESULT_RANK = new Map(
+	CHECK_RESULT_ORDER.map((result, index) => [result, index]),
+);
+
+/// Where an issue sits in the severity ranking, by its *effective* result —
+/// the graded one everything else acts on, so a check an operator capped at
+/// warning ranks as a warning here too.
+///
+/// An issue with no recorded result predates the check-state model and has no
+/// severity to rank on, so it sorts below everything graded rather than above
+/// it, which is where an unknown lands if you index it directly.
+function issueSeverityRank(issue: IssueData): number {
+	const rank = issue.effective_result
+		? CHECK_RESULT_RANK.get(issue.effective_result as CheckResult)
+		: undefined;
+	return rank ?? CHECK_RESULT_ORDER.length;
+}
+
 function Timeline({
 	issues,
 	notes,
@@ -425,7 +446,19 @@ function Timeline({
 			note: n,
 		})),
 	];
-	entries.sort((a, b) => b.at - a.at);
+	// Issues above notes, ranked by severity so whatever is failing is at the
+	// top of the incident and an operator doesn't have to read a whole
+	// chronology to find it. Ties, and notes among themselves, fall back to
+	// most recent first.
+	entries.sort((a, b) => {
+		if (a.kind !== b.kind) return a.kind === "issue" ? -1 : 1;
+		if (a.kind === "issue" && b.kind === "issue") {
+			const bySeverity =
+				issueSeverityRank(a.issue.issue) - issueSeverityRank(b.issue.issue);
+			if (bySeverity !== 0) return bySeverity;
+		}
+		return b.at - a.at;
+	});
 
 	if (entries.length === 0) {
 		return <MuiAlert severity="info">No timeline entries yet.</MuiAlert>;