refactor: address speedtest webview review feedback

Validate CLI output with Zod on the extension side so the webview trusts
typed data and stops hand-parsing JSON. Share SpeedtestResult types via
@repo/shared and flatten the message payload.

Webview cleanup: newspaper-ordered index.ts with a main() entrypoint,
em-scaled chart layout, named constants in place of magic numbers, empty
samples handled with a message, and a subscribeNotification helper that
useIpc now delegates to.

Protocol: buildApiHook and useIpc now take RequestDef, CommandDef, and
NotificationDef directly in both overloads, no casts needed.

Platform: move toError into shared with a serialize hook so the extension
keeps util.inspect output, rename createBaseWebviewConfig to
createWebviewConfig, add createReactWebviewConfig, extract
reportElapsedProgress for reuse by other long running commands, and drop
the createWebviewConfig eslint ignore.

Author: EhabY

eslint.config.mjs

@@ -17,7 +17,6 @@ export default defineConfig(
 		"**/*.d.ts",
 		"vitest.config.ts",
 		"**/vite.config*.ts",
-		"**/createWebviewConfig.ts",
 		".vscode-test/**",
 		"test/fixtures/scripts/**",
 	]),

packages/shared/src/error/toError.ts

@@ -0,0 +1,39 @@
+/** Convert any thrown value into an Error. Pass `serialize` (e.g. `util.inspect`
+ *  in Node) for richer object formatting; the default `JSON.stringify` will
+ *  throw on circular inputs and fall through to `defaultMsg`. */
+export function toError(
+	value: unknown,
+	defaultMsg?: string,
+	serialize: (value: unknown) => string = JSON.stringify,
+): Error {
+	if (value instanceof Error) {
+		return value;
+	}
+
+	if (typeof value === "string") {
+		return new Error(value);
+	}
+
+	if (
+		value !== null &&
+		typeof value === "object" &&
+		"message" in value &&
+		typeof value.message === "string"
+	) {
+		const error = new Error(value.message);
+		if ("name" in value && typeof value.name === "string") {
+			error.name = value.name;
+		}
+		return error;
+	}
+
+	if (value === null || value === undefined) {
+		return new Error(defaultMsg ?? "Unknown error");
+	}
+
+	try {
+		return new Error(serialize(value));
+	} catch {
+		return new Error(defaultMsg ?? "Non-serializable error object");
+	}
+}

packages/shared/src/index.ts

@@ -1,7 +1,18 @@
+// IPC protocol types
 export * from "./ipc/protocol";
 
+// Error utilities
+export { toError } from "./error/toError";
+
+// Tasks types, utilities, and API
 export * from "./tasks/types";
 export * from "./tasks/utils";
 export * from "./tasks/api";
 
-export { SpeedtestApi, type SpeedtestData } from "./speedtest/api";
+// Speedtest API
+export {
+	SpeedtestApi,
+	type SpeedtestData,
+	type SpeedtestInterval,
+	type SpeedtestResult,
+} from "./speedtest/api";

packages/shared/src/ipc/protocol.ts

@@ -166,26 +166,34 @@ export function buildApiHook<
 	api: Api,
 	ipc: {
 		request: <P, R>(
-			def: { method: string; _types?: { params: P; response: R } },
+			def: RequestDef<P, R>,
 			...args: P extends void ? [] : [params: P]
 		) => Promise<R>;
 		command: <P>(
-			def: { method: string; _types?: { params: P } },
+			def: CommandDef<P>,
 			...args: P extends void ? [] : [params: P]
 		) => void;
 		onNotification: <D>(
-			def: { method: string; _types?: { data: D } },
+			def: NotificationDef<D>,
 			cb: (data: D) => void,
 		) => () => void;
 	},
 ): ApiHook<Api>;
 export function buildApiHook(
-	api: Record<string, { kind: string; method: string }>,
+	api: Record<
+		string,
+		| RequestDef<unknown, unknown>
+		| CommandDef<unknown>
+		| NotificationDef<unknown>
+	>,
 	ipc: {
-		request: (def: { method: string }, params?: unknown) => Promise<unknown>;
-		command: (def: { method: string }, params?: unknown) => void;
+		request: (
+			def: RequestDef<unknown, unknown>,
+			params?: unknown,
+		) => Promise<unknown>;
+		command: (def: CommandDef<unknown>, params?: unknown) => void;
 		onNotification: (
-			def: { method: string },
+			def: NotificationDef<unknown>,
 			cb: (data: unknown) => void,
 		) => () => void;
 	},

packages/shared/src/speedtest/api.ts

@@ -1,13 +1,24 @@
 import { defineCommand, defineNotification } from "../ipc/protocol";
 
+export interface SpeedtestInterval {
+	start_time_seconds: number;
+	end_time_seconds: number;
+	throughput_mbits: number;
+}
+
+export interface SpeedtestResult {
+	overall: SpeedtestInterval;
+	intervals: SpeedtestInterval[];
+}
+
 export interface SpeedtestData {
-	json: string;
 	workspaceName: string;
+	result: SpeedtestResult;
 }
 
 export const SpeedtestApi = {
-	/** Extension pushes results to the webview */
+	/** Extension pushes parsed results to the webview */
 	data: defineNotification<SpeedtestData>("speedtest/data"),
 	/** Webview requests to open raw JSON in a text editor */
-	viewJson: defineCommand<string>("speedtest/viewJson"),
+	viewJson: defineCommand<void>("speedtest/viewJson"),
 } as const;

packages/speedtest/src/chart.ts

@@ -4,22 +4,33 @@ export interface ChartPoint {
 	label: string;
 }
 
-const MIN_TICK_SPACING_PX = 48;
+const MIN_TICK_SPACING_EM = 4;
 const Y_GRID_LINES = 5;
-/** 10% padding above the max value so the line doesn't hug the top edge. */
+/** 10% headroom above the max value so the line doesn't hug the top edge. */
 const Y_HEADROOM = 1.1;
+const DOT_RADIUS_PX = 4;
+const LINE_WIDTH_PX = 2;
+
+/** Plot padding in ems, so it scales with font size. */
+const PLOT_PAD_EM = { top: 2, right: 2, bottom: 3.5 };
+const Y_LABEL_GAP_EM = 1;
+const X_LABEL_GAP_EM = 1.5;
+const X_AXIS_TITLE_GAP_EM = 0.25;
+const Y_AXIS_TITLE_GAP_EM = 1;
 
 /** Candidate x-axis tick step sizes in seconds (1s, 2s, 5s, ..., 30m, 1h). */
 const TICK_STEP_SECONDS = [
 	1, 2, 5, 10, 15, 20, 30, 60, 120, 300, 600, 900, 1800, 3600,
 ];
 
+// Exported for tests.
 export function niceStep(raw: number): number {
 	return (
 		TICK_STEP_SECONDS.find((s) => s >= raw) ?? Math.ceil(raw / 3600) * 3600
 	);
 }
 
+// Exported for tests.
 export function formatTick(t: number, step: number): string {
 	if (step >= 3600) {
 		const h = t / 3600;
@@ -39,11 +50,8 @@ interface Theme {
 	family: string;
 }
 
-/**
- * Read VS Code theme colors from CSS custom properties on <html>. Canvas
- * pixels don't inherit CSS vars, so we re-read on each render to pick up
- * theme switches.
- */
+/** Read VS Code theme CSS vars. Canvas pixels don't inherit CSS vars, so
+ *  re-read on each render to track theme switches. */
 function readTheme(): Theme {
 	const s = getComputedStyle(document.documentElement);
 	const css = (prop: string) => s.getPropertyValue(prop).trim();
@@ -69,18 +77,21 @@ function layoutChart(
 	samples: ChartPoint[],
 	width: number,
 	height: number,
+	pxPerEm: number,
 	family: string,
 ) {
 	const maxVal = samples.reduce((m, s) => Math.max(m, s.y), 1) * Y_HEADROOM;
 	const maxX = samples.at(-1)?.x ?? 1;
-	const xRange = maxX || 1;
 	ctx.font = `1em ${family}`;
 	const yLabelWidth = ctx.measureText(maxVal.toFixed(0)).width;
 	const pad = {
-		top: 24,
-		right: 24,
-		bottom: 52,
-		left: Math.max(48, yLabelWidth + 24),
+		top: PLOT_PAD_EM.top * pxPerEm,
+		right: PLOT_PAD_EM.right * pxPerEm,
+		bottom: PLOT_PAD_EM.bottom * pxPerEm,
+		left: Math.max(
+			PLOT_PAD_EM.right * pxPerEm,
+			yLabelWidth + Y_LABEL_GAP_EM * pxPerEm,
+		),
 	};
 	const plotW = width - pad.left - pad.right;
 	const plotH = height - pad.top - pad.bottom;
@@ -90,9 +101,8 @@ function layoutChart(
 		plotH,
 		maxVal,
 		maxX,
-		xRange,
 		height,
-		tAt: (t: number) => pad.left + (t / xRange) * plotW,
+		tAt: (t: number) => pad.left + (t / maxX) * plotW,
 		yAt: (v: number) => pad.top + plotH - (v / maxVal) * plotH,
 	};
 }
@@ -103,8 +113,9 @@ function drawAxes(
 	ctx: CanvasRenderingContext2D,
 	layout: Layout,
 	theme: Theme,
+	pxPerEm: number,
 ): void {
-	const { pad, plotW, plotH, maxVal, maxX, xRange, height, tAt, yAt } = layout;
+	const { pad, plotW, plotH, maxVal, maxX, height, tAt, yAt } = layout;
 
 	ctx.strokeStyle = theme.grid;
 	ctx.lineWidth = 1;
@@ -117,7 +128,11 @@ function drawAxes(
 		ctx.moveTo(pad.left, y);
 		ctx.lineTo(pad.left + plotW, y);
 		ctx.stroke();
-		ctx.fillText(v.toFixed(0), pad.left - 12, y + 5);
+		ctx.fillText(
+			v.toFixed(0),
+			pad.left - Y_LABEL_GAP_EM * pxPerEm,
+			y + pxPerEm / 3,
+		);
 	}
 
 	ctx.strokeStyle = theme.fg;
@@ -128,16 +143,24 @@ function drawAxes(
 
 	ctx.textAlign = "center";
 	const step = niceStep(
-		xRange / Math.max(1, Math.floor(plotW / MIN_TICK_SPACING_PX)),
+		maxX / Math.max(1, Math.floor(plotW / (MIN_TICK_SPACING_EM * pxPerEm))),
 	);
 	for (let t = 0; t <= maxX; t += step) {
-		ctx.fillText(formatTick(t, step), tAt(t), height - pad.bottom + 24);
+		ctx.fillText(
+			formatTick(t, step),
+			tAt(t),
+			height - pad.bottom + X_LABEL_GAP_EM * pxPerEm,
+		);
 	}
 
 	ctx.font = `0.95em ${theme.family}`;
-	ctx.fillText("Time", pad.left + plotW / 2, height - 4);
+	ctx.fillText(
+		"Time",
+		pad.left + plotW / 2,
+		height - X_AXIS_TITLE_GAP_EM * pxPerEm,
+	);
 	ctx.save();
-	ctx.translate(14, pad.top + plotH / 2);
+	ctx.translate(Y_AXIS_TITLE_GAP_EM * pxPerEm, pad.top + plotH / 2);
 	ctx.rotate(-Math.PI / 2);
 	ctx.fillText("Mbps", 0, 0);
 	ctx.restore();
@@ -187,32 +210,31 @@ function drawSeries(
 		ctx.lineTo(tAt(samples[i].x), yAt(samples[i].y));
 	}
 	ctx.strokeStyle = theme.accent;
-	ctx.lineWidth = 2;
+	ctx.lineWidth = LINE_WIDTH_PX;
 	ctx.stroke();
 
 	return samples.map((s) => {
 		const x = tAt(s.x);
 		const y = yAt(s.y);
 		if (showDots) {
 			ctx.beginPath();
-			ctx.arc(x, y, 4, 0, Math.PI * 2);
+			ctx.arc(x, y, DOT_RADIUS_PX, 0, Math.PI * 2);
 			ctx.fillStyle = theme.accent;
 			ctx.fill();
 		}
 		return { x, y, label: s.label };
 	});
 }
 
+/** Render the speedtest chart. Caller must ensure `samples` is non-empty. */
 export function renderLineChart(
 	canvas: HTMLCanvasElement,
 	samples: ChartPoint[],
 	showDots: boolean,
 ): ChartPoint[] {
-	// Scale the backing store by DPR for crisp rendering on high-DPI
-	// displays. ctx.scale lets draw calls keep using CSS pixels.
-	const { width, height } = (
-		canvas.parentElement ?? canvas
-	).getBoundingClientRect();
+	// Scale backing store by DPR so drawing stays crisp on high-DPI screens.
+	const parent = canvas.parentElement ?? canvas;
+	const { width, height } = parent.getBoundingClientRect();
 	const dpr = window.devicePixelRatio || 1;
 	canvas.width = width * dpr;
 	canvas.height = height * dpr;
@@ -222,10 +244,16 @@ export function renderLineChart(
 	}
 	ctx.scale(dpr, dpr);
 
+	const pxPerEm = parseFloat(getComputedStyle(parent).fontSize) || 14;
 	const theme = readTheme();
-	const layout = layoutChart(ctx, samples, width, height, theme.family);
-	drawAxes(ctx, layout, theme);
-	return samples.length > 0
-		? drawSeries(ctx, samples, layout, theme, showDots)
-		: [];
+	const layout = layoutChart(
+		ctx,
+		samples,
+		width,
+		height,
+		pxPerEm,
+		theme.family,
+	);
+	drawAxes(ctx, layout, theme, pxPerEm);
+	return drawSeries(ctx, samples, layout, theme, showDots);
 }