From 82da5784476bc627ec898dd73cb7ba599e306b8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Wed, 5 Aug 2026 11:12:28 +1200 Subject: [PATCH] feat(incidents): the timeline leads with what is worst, not what is newest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The incident timeline was ordered by time alone, so an operator opening a page mid-incident had to read a chronology to find the failure — and a warning filed a minute ago sat above a failure that had been going for an hour. Issues are now ordered by the severity of their effective result, ties broken by most recent, with notes below every issue. Effective, not observed: a check an operator capped at warning ranks as a warning here too, matching what the rest of the incident workflow acts on. The rank comes from CHECK_RESULT_ORDER, already the UI's severity vocabulary, and 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 — where an unknown lands if you index it directly. Co-Authored-By: Claude Opus 5 (1M context) --- .workhorse/specs/monitoring/incidents.md | 4 + private-web/e2e/incident-ordering.spec.ts | 129 ++++++++++++++++++++++ private-web/e2e/seed.ts | 26 +++++ private-web/src/routes/IncidentDetail.tsx | 35 +++++- 4 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 private-web/e2e/incident-ordering.spec.ts diff --git a/.workhorse/specs/monitoring/incidents.md b/.workhorse/specs/monitoring/incidents.md index 337be083d..9c2fa8589 100644 --- a/.workhorse/specs/monitoring/incidents.md +++ b/.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. diff --git a/private-web/e2e/incident-ordering.spec.ts b/private-web/e2e/incident-ordering.spec.ts new file mode 100644 index 000000000..f39d005ca --- /dev/null +++ b/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[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 { + 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); + }); +}); diff --git a/private-web/e2e/seed.ts b/private-web/e2e/seed.ts index a9c392b9e..05d1a1a76 100644 --- a/private-web/e2e/seed.ts +++ b/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; diff --git a/private-web/src/routes/IncidentDetail.tsx b/private-web/src/routes/IncidentDetail.tsx index 32a0b4330..9266a6f84 100644 --- a/private-web/src/routes/IncidentDetail.tsx +++ b/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 No timeline entries yet.;