fix: stop double-listing region warnings; share page header

Address review round 4 (CRF-32..35):

- CRF-32: the CLI folds region/node warnings into the DERP section `warnings`,
  so the region walk listed them twice. Drop the region-warning walk (keep
  region errors, which are not aggregated) and remove the now-redundant region
  `warnings` field from the schema and shared type.
- CRF-33: add a test for the `derp.error` severity fold (the existing case set
  `netcheck_err`, short-circuiting the `|| d.error` operand).
- CRF-34: extract `pageHeader` into @repo/webview-shared/dom.ts, used by both
  vanilla webviews (the last header duplicate after the dom.ts dedup).
- CRF-35: document that section `severity` is a parse-time rollup. Kept the
  error fold inlined per earlier maintainer preference (trivial ternary).

Author: EhabY

packages/netcheck/src/health.ts

@@ -69,24 +69,15 @@ export function collectIssues(report: NetcheckReport): Issue[] {
 	}
 	for (const [key, region] of Object.entries(report.derp.regions)) {
 		const name = regionName(region, Number(key));
+		// Region warnings are already in the section list; list only errors.
 		if (region.error) {
 			errors.push({ kind: "error", message: `${name}: ${region.error}` });
-		}
-		for (const warning of region.warnings ?? []) {
-			warnings.push({
-				kind: "warning",
-				message: `${name}: ${warning.message}`,
-				...(warning.code ? { code: warning.code } : {}),
+		} else if (region.severity === "error") {
+			errors.push({
+				kind: "error",
+				message: `${name}: a node failed its health check`,
 			});
 		}
-		// A node can be unhealthy with no message of its own (region severity
-		// already reflects it). Synthesize one so the banner isn't alone.
-		const hasMessage = Boolean(region.error) || Boolean(region.warnings?.length);
-		if (!hasMessage && region.severity === "error") {
-			errors.push({ kind: "error", message: `${name}: a node failed its health check` });
-		} else if (!hasMessage && region.severity === "warning") {
-			warnings.push({ kind: "warning", message: `${name}: a node reported a warning` });
-		}
 	}
 	addSection(report.interfaces);
 	return [...errors, ...warnings];

packages/netcheck/src/page.ts

@@ -5,7 +5,7 @@ import {
 	type NetcheckReport,
 	type NetcheckSeverity,
 } from "@repo/shared";
-import { emptyMessage, viewJsonAction } from "@repo/webview-shared";
+import { emptyMessage, pageHeader, viewJsonAction } from "@repo/webview-shared";
 
 import { buildConnectivityItems } from "./connectivity";
 import { formatLatency, formatTriState } from "./format";
@@ -22,7 +22,10 @@ export function renderPage(
 	{ host, report }: NetcheckData,
 	onViewJson: () => void,
 ): HTMLElement[] {
-	const children = [renderHeading(host), renderBanner(report)];
+	const children = [
+		pageHeader("Network Check", host, "deployment-host"),
+		renderBanner(report),
+	];
 
 	const issues = collectIssues(report);
 	if (issues.length > 0) {
@@ -40,15 +43,6 @@ export function renderPage(
 	return children;
 }
 
-function renderHeading(host: string): HTMLElement {
-	const header = el("header", "page-header");
-	header.append(
-		el("p", "eyebrow", "Network Check"),
-		el("h1", "deployment-host", host),
-	);
-	return header;
-}
-
 function renderBanner(report: NetcheckReport): HTMLElement {
 	const severity = overallNetcheckSeverity(report);
 	const banner = el("div", `status-banner severity-${severity}`);

packages/shared/src/netcheck/types.ts

@@ -9,6 +9,7 @@ export type NetcheckSeverity = "ok" | "warning" | "error";
 
 /** Health fields shared by the DERP and interfaces sections of the report. */
 export interface NetcheckSectionHealth {
+	/** Parse-time rollup: errors (and, for DERP, probe/region severity) folded in. */
 	severity: NetcheckSeverity;
 	warnings: Array<{ code: string; message: string }>;
 	error?: string | null;
@@ -24,7 +25,6 @@ interface NetcheckNodeReport {
 export interface NetcheckRegionReport {
 	severity: NetcheckSeverity;
 	error?: string | null;
-	warnings?: Array<{ code: string; message: string }>;
 	region?: {
 		RegionID: number;
 		RegionName: string;

packages/speedtest/src/index.ts

@@ -2,6 +2,7 @@ import { SpeedtestApi, type SpeedtestResult, toError } from "@repo/shared";
 import {
 	emptyMessage,
 	errorMessage,
+	pageHeader,
 	sendCommand,
 	subscribeNotifications,
 	viewJsonAction,
@@ -54,7 +55,7 @@ function renderPage(
 	}
 
 	root.innerHTML = "";
-	root.appendChild(renderHeading(workspaceId));
+	root.appendChild(pageHeader("Speed Test", workspaceId, "workspace-id"));
 	root.appendChild(renderSummary(data));
 
 	const samples = toChartSamples(data.intervals);
@@ -70,22 +71,6 @@ function renderPage(
 	return chart.cleanup;
 }
 
-function renderHeading(workspaceId: string): HTMLElement {
-	const header = document.createElement("header");
-	header.className = "page-header";
-
-	const eyebrow = document.createElement("p");
-	eyebrow.className = "eyebrow";
-	eyebrow.textContent = "Speed Test";
-
-	const heading = document.createElement("h1");
-	heading.className = "workspace-id";
-	heading.textContent = workspaceId;
-
-	header.append(eyebrow, heading);
-	return header;
-}
-
 function renderSummary(data: SpeedtestResult): HTMLElement {
 	const summary = document.createElement("div");
 	summary.className = "summary";

packages/webview-shared/src/dom.ts

@@ -1,5 +1,23 @@
 /** DOM builders shared by the vanilla webviews. */
 
+/** A page header: an eyebrow label above a title with the given class. */
+export function pageHeader(
+	eyebrow: string,
+	title: string,
+	titleClass: string,
+): HTMLElement {
+	const header = document.createElement("header");
+	header.className = "page-header";
+	const eyebrowEl = document.createElement("p");
+	eyebrowEl.className = "eyebrow";
+	eyebrowEl.textContent = eyebrow;
+	const titleEl = document.createElement("h1");
+	titleEl.className = titleClass;
+	titleEl.textContent = title;
+	header.append(eyebrowEl, titleEl);
+	return header;
+}
+
 /** An action bar with a "View JSON" button wired to `onClick`. */
 export function viewJsonAction(onClick: () => void): HTMLElement {
 	const actions = document.createElement("div");

packages/webview-shared/src/index.ts

@@ -16,4 +16,4 @@ export {
 } from "./ipc";
 
 // DOM builders shared by the vanilla webviews.
-export { viewJsonAction, emptyMessage, errorMessage } from "./dom";
+export { pageHeader, viewJsonAction, emptyMessage, errorMessage } from "./dom";

src/webviews/netcheck/types.ts

@@ -80,7 +80,6 @@ const RegionReportSchema = z
 	.object({
 		severity: SeveritySchema,
 		error: z.string().nullish(),
-		warnings: WarningsSchema,
 		region: z
 			.object({
 				RegionID: z.number(),

test/unit/webviews/netcheck/types.test.ts

@@ -100,6 +100,17 @@ describe("parseNetcheckReport", () => {
 		expect(report.interfaces.severity).toBe("error");
 	});
 
+	it("escalates derp severity from a section error alone", () => {
+		const r = structuredClone(validReport) as {
+			derp: { severity: string; error?: string };
+		};
+		r.derp.severity = "ok";
+		r.derp.error = "DERP map unreachable";
+
+		const report = parseNetcheckReport(JSON.stringify(r));
+		expect(report.derp.severity).toBe("error");
+	});
+
 	it("throws ZodError when a required field is missing", () => {
 		const missingSeverity = structuredClone(validReport) as {
 			derp: { severity?: string };

test/webview/netcheck/health.test.ts

@@ -49,15 +49,17 @@ describe("collectIssues", () => {
 		]);
 	});
 
-	it("includes per-region errors and warnings, prefixed with the region name", () => {
+	it("lists region errors prefixed, and section warnings only once", () => {
 		const issues = collectIssues(
 			report({
 				derp: {
+					severity: "error",
+					// The CLI folds region/node warnings into the section list.
+					warnings: [{ code: "ERLY", message: "high latency" }],
 					regions: {
 						"5": {
 							severity: "error",
 							error: "region unreachable",
-							warnings: [{ code: "ERLY", message: "high latency" }],
 							region: {
 								RegionID: 5,
 								RegionName: "Tokyo",
@@ -72,7 +74,7 @@ describe("collectIssues", () => {
 
 		expect(issues).toEqual([
 			{ kind: "error", message: "Tokyo: region unreachable" },
-			{ kind: "warning", code: "ERLY", message: "Tokyo: high latency" },
+			{ kind: "warning", code: "ERLY", message: "high latency" },
 		]);
 	});
 
@@ -83,7 +85,11 @@ describe("collectIssues", () => {
 					regions: {
 						"3": {
 							severity: "error",
-							region: { RegionID: 3, RegionName: "Frankfurt", EmbeddedRelay: true },
+							region: {
+								RegionID: 3,
+								RegionName: "Frankfurt",
+								EmbeddedRelay: true,
+							},
 							node_reports: [
 								{
 									can_exchange_messages: false,