fix(delta): report status transitions, not placeholder scores, for checks that did not run (#107)

CHANGELOG.md

@@ -2,6 +2,30 @@
 
 ## Unreleased
 
+### A check that did not run is no longer scored, diffed or counted
+A not-run check carries a placeholder `score: 100` and a crashed runner a
+placeholder `0`/F. Deltas and trends subtracted them from real scores, so
+installing a linter read as `lint: 100 → 72 (-28)`, uninstalling it as `+36`,
+and a crash as `-72` (#107).
+- **Changed**: when either side of a comparison did not run (skipped,
+  unavailable), its runner crashed, or the check is absent, the delta, the
+  trend, the PR comment, the delta markdown and the Actions and Trends pages
+  show a status transition, e.g. `lint: unavailable → 72` or
+  `72 → failed (runner error)`, instead of a numeric delta. The Trends page
+  plots only the scans in which a check ran; a single check's card shows the
+  full crash reason.
+- **Changed**: issues of such a check are not compared, so a crash, an
+  uninstalled tool or a dropped check no longer reports its issues as fixed
+  (or, the other way round, as new).
+
+**Note for consumers** of `@vibecodeqa/cli/core`:
+- `ScanDelta.checks[].before` / `.after` (`CheckDelta`) are now
+  `number | null`: `null` for a side that did not run, crashed, or is absent;
+  the check then carries a `transition` and `delta: 0`. TypeScript callers
+  that treat them as `number` need a null check.
+- Internally (not exported from `core`), `TrendDelta.checkDeltas[].prev` /
+  `.curr` change the same way, and the unused `trendHTML` is removed.
+
 ### Issue fingerprints v2 — a finding keeps its identity when its number moves
 Fingerprints hashed check + rule + path + message, so a message carrying a
 measurement ("AdminLayout: 65 lines (max 60)") re-identified the finding every

src/analyzer-snapshot.test.ts

@@ -85,3 +85,28 @@ describe("buildAnalyzerSnapshots", () => {
 		});
 	});
 });
+
+describe("buildAnalyzerSnapshots not-run checks (#107)", () => {
+	it("omits score for skipped and unavailable checks", () => {
+		const snaps = buildAnalyzerSnapshots([
+			check({ name: "lint", score: 100, details: { skipped: true, unavailable: true, status: "unavailable" } }),
+			check({ name: "react", score: 100, details: { skipped: true, status: "skipped" } }),
+			check({ name: "ai-review", score: 100, details: { comingSoon: true } }),
+			check({ name: "types", score: 88, details: { status: "passed" } }),
+		]);
+		expect(snaps.map((s) => s.status)).toEqual(["unavailable", "skipped", "unavailable", "passed"]);
+		for (const s of snaps.slice(0, 3)) expect(s).not.toHaveProperty("score");
+		expect(snaps[3].score).toBe(88);
+		expect(JSON.parse(JSON.stringify(snaps[0]))).not.toHaveProperty("score");
+	});
+});
+
+describe("buildAnalyzerSnapshots crashed runner (#107)", () => {
+	it("keeps status failed but omits the placeholder 0", () => {
+		const [snap] = buildAnalyzerSnapshots([
+			check({ name: "lint", score: 0, details: { skipped: true, status: "failed", reason: "runner error: boom" } }),
+		]);
+		expect(snap.status).toBe("failed");
+		expect(snap).not.toHaveProperty("score");
+	});
+});

src/analyzer-snapshot.ts

@@ -1,17 +1,26 @@
+import { checkSide } from "./delta.js";
 import type { AnalyzerMetric, AnalyzerSnapshot, CheckResult, Issue } from "./types.js";
 
 const DETAIL_DENYLIST = new Set(["assessment", "containerSvg", "graph", "reason", "skipped", "status", "toolRuns"]);
 
 export function buildAnalyzerSnapshots(checks: CheckResult[]): AnalyzerSnapshot[] {
-	return checks.map((check) => ({
-		analyzerId: check.name,
-		status: checkStatus(check),
-		score: Number.isFinite(Number(check.score)) ? Number(check.score) : undefined,
-		findingCount: check.issues.length,
-		severityCounts: severityCounts(check.issues),
-		metrics: normalizedMetrics(check),
-		durationMs: check.duration,
-	}));
+	return checks.map((check) => {
+		const status = checkStatus(check);
+		// A check that did not run carries a placeholder 100 (core.ts), and a
+		// crashed runner a placeholder 0 (its status stays "failed"); neither is
+		// a measurement, so the snapshot has no score key at all (#107).
+		const ran = checkSide(check).score;
+		const score = ran !== undefined && Number.isFinite(Number(ran)) ? Number(ran) : undefined;
+		return {
+			analyzerId: check.name,
+			status,
+			...(score === undefined ? {} : { score }),
+			findingCount: check.issues.length,
+			severityCounts: severityCounts(check.issues),
+			metrics: normalizedMetrics(check),
+			durationMs: check.duration,
+		};
+	});
 }
 
 function checkStatus(check: CheckResult): AnalyzerSnapshot["status"] {

src/cli.ts

@@ -12,7 +12,7 @@ import { runInit } from "./commands/init.js";
 import { validateCwd } from "./commands/shared.js";
 import { loadConfig } from "./config.js";
 import { scan } from "./core.js";
-import { computeDelta } from "./delta.js";
+import { computeDelta, formatCheckChangeBullets } from "./delta.js";
 import { detectStack, detectWorkspace } from "./detect.js";
 import { resolveFailUnder } from "./fail-under.js";
 import { isPartialScan } from "./history.js";
@@ -187,14 +187,7 @@ function generateMarkdown(report: VibeReport, trend: TrendDelta | null, prevRepo
 		md += "\n\n";
 
 		// Per-check changes
-		const changed = delta.checks.filter((c) => c.delta !== 0).sort((a, b) => b.delta - a.delta);
-		if (changed.length > 0) {
-			for (const c of changed.slice(0, 8)) {
-				const a = c.delta > 0 ? "+" : "";
-				md += `- ${c.delta > 0 ? "✅" : "⚠️"} ${c.name}: ${c.before} → ${c.after} (${a}${c.delta})\n`;
-			}
-			md += "\n";
-		}
+		md += formatCheckChangeBullets(delta, 8);
 	} else if (trend) {
 		const arrow = trend.scoreDelta > 0 ? "📈" : trend.scoreDelta < 0 ? "📉" : "➡️";
 		md += `${arrow} **${trend.scoreDelta > 0 ? "+" : ""}${trend.scoreDelta}** vs previous`;

src/delta.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { computeDelta, formatDeltaMarkdown } from "./delta.js";
+import { checkRunState, computeDelta, formatCheckChangeBullets, formatCheckState, formatDeltaMarkdown, formatTransition } from "./delta.js";
 import type { VibeReport } from "./types.js";
 
 function makeReport(overrides: Partial<VibeReport> = {}): VibeReport {
@@ -114,3 +114,255 @@ describe("formatDeltaMarkdown", () => {
 		expect(md).toContain("+10");
 	});
 });
+
+describe("not-run checks (#107)", () => {
+	const lintUnavailable = {
+		name: "lint",
+		status: "unavailable",
+		score: 100,
+		grade: "A" as const,
+		details: { skipped: true, unavailable: true, status: "unavailable", reason: "Dart SDK not installed" },
+		issues: [],
+		duration: 1,
+	};
+	const lintScored = {
+		name: "lint",
+		status: "failed",
+		score: 72,
+		grade: "C" as const,
+		details: { status: "failed" },
+		issues: [],
+		duration: 1,
+	};
+
+	it("reports a status transition, not a 100 → 72 delta, when lint was unavailable before", () => {
+		const delta = computeDelta(makeReport({ checks: [lintUnavailable] }), makeReport({ checks: [lintScored] }));
+		const lint = delta.checks.find((c) => c.name === "lint")!;
+		expect(lint.delta).toBe(0);
+		expect(lint.transition).toEqual({ before: { state: "unavailable" }, after: { state: "ran", score: 72 } });
+		expect(formatTransition(lint.transition!)).toBe("unavailable → 72");
+
+		const bullets = formatCheckChangeBullets(delta, 8);
+		expect(bullets).toContain("lint: unavailable → 72");
+		expect(bullets).not.toContain("100 → 72");
+		expect(bullets).not.toMatch(/\(-28\)/);
+
+		const md = formatDeltaMarkdown(delta);
+		expect(md).toContain("## Status Changes");
+		expect(md).toContain("| lint | unavailable | 72 |");
+		expect(md).not.toContain("## Check Changes");
+		expect(md).not.toContain("100");
+	});
+
+	it("does not report removing a tool as an improvement", () => {
+		const scored64 = { ...lintScored, score: 64, grade: "D" as const };
+		const delta = computeDelta(makeReport({ checks: [scored64] }), makeReport({ checks: [lintUnavailable] }));
+		const bullets = formatCheckChangeBullets(delta, 8);
+		expect(bullets).toContain("lint: 64 → unavailable");
+		expect(bullets).not.toMatch(/\+\d/);
+		expect(bullets).not.toContain("✅");
+		expect(formatDeltaMarkdown(delta)).not.toMatch(/\+36/);
+	});
+
+	it("falls back to details flags for reports without a status field", () => {
+		const legacyComingSoon = { name: "ai-review", score: 100, grade: "A" as const, details: { comingSoon: true }, issues: [], duration: 1 };
+		const legacySkipped = { name: "lint", score: 100, grade: "A" as const, details: { skipped: true }, issues: [], duration: 1 };
+		const legacyRunnerError = {
+			name: "types",
+			score: 0,
+			grade: "F" as const,
+			details: { skipped: true, reason: "runner error: boom" },
+			issues: [],
+			duration: 1,
+		};
+		expect(checkRunState(legacyComingSoon)).toBe("unavailable");
+		expect(checkRunState(legacySkipped)).toBe("skipped");
+		expect(checkRunState(legacyRunnerError)).toBe("runner-error");
+		expect(checkRunState(undefined)).toBe("absent");
+
+		const delta = computeDelta(makeReport({ checks: [legacySkipped] }), makeReport({ checks: [lintScored] }));
+		expect(delta.checks[0]).toMatchObject({ delta: 0, transition: { before: { state: "skipped" }, after: { state: "ran", score: 72 } } });
+	});
+
+	it("reports nothing when a check stays skipped", () => {
+		const skipped = { ...lintUnavailable, status: "skipped", details: { skipped: true, status: "skipped" } };
+		const delta = computeDelta(makeReport({ checks: [skipped] }), makeReport({ checks: [skipped] }));
+		expect(delta.checks).toHaveLength(0);
+	});
+
+	it("keeps numeric deltas when both sides ran", () => {
+		const delta = computeDelta(makeReport({ checks: [{ ...lintScored, score: 60 }] }), makeReport({ checks: [lintScored] }));
+		expect(delta.checks[0]).toMatchObject({ delta: 12 });
+		expect(delta.checks[0].transition).toBeUndefined();
+		expect(formatCheckChangeBullets(delta, 8)).toContain("✅ lint: 60 → 72 (+12)");
+	});
+
+	it("reports a check that disappeared as a transition and does not count its issues as fixed", () => {
+		const lintWithIssues = {
+			...lintScored,
+			issues: [
+				{ severity: "error" as const, message: "unused var", file: "src/a.ts", rule: "no-unused" },
+				{ severity: "warning" as const, message: "prefer const", file: "src/b.ts", rule: "prefer-const" },
+			],
+		};
+		const delta = computeDelta(makeReport({ checks: [lintWithIssues] }), makeReport({ checks: [] }));
+		const lint = delta.checks.find((c) => c.name === "lint")!;
+		expect(lint).toMatchObject({ before: 72, after: null, delta: 0 });
+		expect(lint.transition).toEqual({ before: { state: "ran", score: 72 }, after: { state: "absent" } });
+		expect(lint.fixed).toHaveLength(0);
+		expect(delta.fixed).toHaveLength(0);
+		expect(formatCheckChangeBullets(delta, 8)).toContain("lint: 72 → not present");
+		expect(formatDeltaMarkdown(delta)).not.toContain("## Fixed");
+	});
+
+	it("carries null, not the placeholder 100, for a side that did not run", () => {
+		const delta = computeDelta(makeReport({ checks: [lintUnavailable] }), makeReport({ checks: [lintScored] }));
+		expect(delta.checks[0]).toMatchObject({ before: null, after: 72 });
+		const back = computeDelta(makeReport({ checks: [lintScored] }), makeReport({ checks: [lintUnavailable] }));
+		expect(back.checks[0]).toMatchObject({ before: 72, after: null });
+		expect(JSON.stringify(back)).not.toContain('"after":100');
+	});
+});
+
+describe("crashed runner (#107)", () => {
+	// core.ts's stub for a runner that threw: status failed, a placeholder 0/F.
+	const lintCrashed = {
+		name: "lint",
+		status: "failed",
+		score: 0,
+		grade: "F" as const,
+		details: { skipped: true, status: "failed", reason: "runner error: eslint exited 2" },
+		issues: [],
+		duration: 0,
+	};
+	const lintScored = {
+		name: "lint",
+		status: "failed",
+		score: 72,
+		grade: "C" as const,
+		details: { status: "failed" },
+		issues: [],
+		duration: 1,
+	};
+
+	it("reports 72 → runner error as a transition, with no numeric delta", () => {
+		const delta = computeDelta(makeReport({ checks: [lintScored] }), makeReport({ checks: [lintCrashed] }));
+		const lint = delta.checks.find((c) => c.name === "lint")!;
+		expect(lint).toMatchObject({ before: 72, after: null, delta: 0 });
+		expect(lint.transition).toEqual({ before: { state: "ran", score: 72 }, after: { state: "runner-error" } });
+		expect(formatTransition(lint.transition!)).toBe("72 → failed (runner error)");
+
+		const bullets = formatCheckChangeBullets(delta, 8);
+		expect(bullets).toContain("- lint: 72 → failed (runner error)");
+		expect(bullets).not.toContain("-72");
+		expect(bullets).not.toContain("72 → 0");
+
+		const md = formatDeltaMarkdown(delta);
+		expect(md).toContain("| lint | 72 | failed (runner error) |");
+		expect(md).not.toContain("## Check Changes");
+		expect(md).not.toContain("-72");
+	});
+
+	it("reports runner error → 72 as a transition, not +72", () => {
+		const delta = computeDelta(makeReport({ checks: [lintCrashed] }), makeReport({ checks: [lintScored] }));
+		const lint = delta.checks.find((c) => c.name === "lint")!;
+		expect(lint).toMatchObject({ before: null, after: 72, delta: 0 });
+		expect(formatTransition(lint.transition!)).toBe("failed (runner error) → 72");
+		expect(formatCheckChangeBullets(delta, 8)).not.toContain("+72");
+		expect(formatDeltaMarkdown(delta)).not.toContain("+72");
+
+		// Crashed in both scans: nothing changed, nothing to report.
+		expect(computeDelta(makeReport({ checks: [lintCrashed] }), makeReport({ checks: [lintCrashed] })).checks).toHaveLength(0);
+	});
+
+	it("detects a crash from details alone (history snapshots, older reports)", () => {
+		const { status: _status, ...noTopStatus } = lintCrashed;
+		expect(checkRunState(noTopStatus)).toBe("runner-error");
+		expect(checkRunState({ ...noTopStatus, details: { skipped: true, reason: "runner error: boom" } })).toBe("runner-error");
+		// An explicit skipped/unavailable status still wins over the reason.
+		expect(checkRunState({ ...lintCrashed, status: "unavailable" })).toBe("unavailable");
+		// `status` is an open vocabulary (schema 0.6.0): an unknown value counts as ran.
+		expect(checkRunState({ ...lintScored, status: "timeout" })).toBe("ran");
+		expect(checkRunState({ ...lintScored, status: "timeout", details: { status: "timeout" } })).toBe("ran");
+	});
+});
+
+describe("issues of a check that did not run on one side are not diffed (#107)", () => {
+	const threeIssues = [
+		{ severity: "error" as const, message: "unused var", file: "src/a.ts", rule: "no-unused" },
+		{ severity: "warning" as const, message: "prefer const", file: "src/b.ts", rule: "prefer-const" },
+		{ severity: "warning" as const, message: "no any", file: "src/c.ts", rule: "no-any" },
+	];
+	const lintRan = {
+		name: "lint",
+		status: "failed",
+		score: 72,
+		grade: "C" as const,
+		details: { status: "failed" },
+		issues: threeIssues,
+		duration: 1,
+	};
+	const notRun = {
+		crashed: {
+			name: "lint",
+			status: "failed",
+			score: 0,
+			grade: "F" as const,
+			details: { skipped: true, status: "failed", reason: "runner error: boom" },
+			issues: [],
+			duration: 0,
+		},
+		unavailable: {
+			name: "lint",
+			status: "unavailable",
+			score: 100,
+			grade: "A" as const,
+			details: { skipped: true, unavailable: true, status: "unavailable" },
+			issues: [],
+			duration: 1,
+		},
+		dropped: undefined,
+	};
+	const cases = Object.entries(notRun).flatMap(([kind, other]) => [
+		{ title: `ran (3 issues) → ${kind}`, before: [lintRan], after: other ? [other] : [] },
+		{ title: `${kind} → ran (3 issues)`, before: other ? [other] : [], after: [lintRan] },
+	]);
+
+	it.each(cases)("$title: 0 fixed, 0 new; only the status transition", ({ before, after }) => {
+		const delta = computeDelta(makeReport({ checks: before }), makeReport({ checks: after }));
+		expect(delta.fixed).toHaveLength(0);
+		expect(delta.introduced).toHaveLength(0);
+		const lint = delta.checks.find((c) => c.name === "lint")!;
+		expect(lint.transition).toBeDefined();
+		expect(lint.fixed).toHaveLength(0);
+		expect(lint.introduced).toHaveLength(0);
+
+		const md = formatDeltaMarkdown(delta);
+		expect(md).toContain("| 0 fixed, 0 new |");
+		expect(md).not.toContain("## Fixed");
+		expect(md).not.toContain("## New Issues");
+		expect(md).not.toMatch(/\b3 (fixed|new)\b/);
+		expect(formatCheckChangeBullets(delta, 8)).not.toMatch(/fixed|new/);
+	});
+
+	it("still diffs issues when the check ran on both sides", () => {
+		const delta = computeDelta(makeReport({ checks: [lintRan] }), makeReport({ checks: [{ ...lintRan, issues: threeIssues.slice(1) }] }));
+		expect(delta.fixed).toHaveLength(1);
+		expect(delta.introduced).toHaveLength(0);
+	});
+});
+
+describe("formatCheckState (#107)", () => {
+	it("shows the full crash reason for a single check, truncated to 80 chars", () => {
+		expect(formatCheckState({ state: "runner-error" }, "runner error: eslint exited 2")).toBe("failed (runner error: eslint exited 2)");
+		const long = `runner error: ${"x".repeat(200)}`;
+		const out = formatCheckState({ state: "runner-error" }, long);
+		expect(out).toBe(`failed (${long.slice(0, 79)}…)`);
+	});
+
+	it("falls back to the short token without a reason, and for other states", () => {
+		expect(formatCheckState({ state: "runner-error" })).toBe("failed (runner error)");
+		expect(formatCheckState({ state: "unavailable" }, "Dart SDK not installed")).toBe("unavailable");
+		expect(formatCheckState({ state: "ran", score: 72 })).toBe("72");
+	});
+});