From ec86546582177641b001efeb9851920b2ca4bba7 Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 14:03:23 +1100 Subject: [PATCH 1/8] fix(delta): report status transitions, not placeholder scores, for checks that did not run (#107) A check that did not run (skipped, unavailable, Pro-only) carries a placeholder score of 100. delta.ts and trend.ts subtracted it from real scores, so installing a tool read as "lint: 100 -> 72 (-28)" and removing one as "+36". - delta.ts / trend.ts: when either side is skipped, unavailable or absent, emit no numeric delta; set a status transition instead ("lint: unavailable -> 72"). Run state reads `status`, falling back to details.unavailable/comingSoon/skipped for older reports. - PR comment, CLI markdown, delta markdown and the HTML actions page / trend banner list status changes separately from score changes. - analyzer snapshots omit `score` for skipped/unavailable checks. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/analyzer-snapshot.test.ts | 15 +++++ src/analyzer-snapshot.ts | 26 +++++--- src/cli.ts | 11 +-- src/delta.test.ts | 85 ++++++++++++++++++++++- src/delta.ts | 122 ++++++++++++++++++++++++++++++++-- src/pr-comment.test.ts | 36 ++++++++++ src/pr-comment.ts | 13 +--- src/report/pages.test.ts | 29 ++++++++ src/report/pages.ts | 11 ++- src/trend.test.ts | 34 ++++++++++ src/trend.ts | 14 +++- 11 files changed, 358 insertions(+), 38 deletions(-) diff --git a/src/analyzer-snapshot.test.ts b/src/analyzer-snapshot.test.ts index 5180424..41a542a 100644 --- a/src/analyzer-snapshot.test.ts +++ b/src/analyzer-snapshot.test.ts @@ -85,3 +85,18 @@ 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"); + }); +}); diff --git a/src/analyzer-snapshot.ts b/src/analyzer-snapshot.ts index 4644a94..b9fbe3e 100644 --- a/src/analyzer-snapshot.ts +++ b/src/analyzer-snapshot.ts @@ -3,17 +3,25 @@ import type { AnalyzerMetric, AnalyzerSnapshot, CheckResult, Issue } from "./typ 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); it is + // not a measurement, so the snapshot has no score key at all (#107). + const score = !NOT_RUN.has(status) && Number.isFinite(Number(check.score)) ? Number(check.score) : undefined; + return { + analyzerId: check.name, + status, + ...(score === undefined ? {} : { score }), + findingCount: check.issues.length, + severityCounts: severityCounts(check.issues), + metrics: normalizedMetrics(check), + durationMs: check.duration, + }; + }); } +const NOT_RUN = new Set(["skipped", "unavailable"]); + function checkStatus(check: CheckResult): AnalyzerSnapshot["status"] { const details = check.details as Record; const status = details.status; diff --git a/src/cli.ts b/src/cli.ts index ab01e3a..a16f352 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -11,7 +11,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 { postPRComment } from "./pr-comment.js"; import { generatePages } from "./report/html.js"; @@ -184,14 +184,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`; diff --git a/src/delta.test.ts b/src/delta.test.ts index f25e961..3a40119 100644 --- a/src/delta.test.ts +++ b/src/delta.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { computeDelta, formatDeltaMarkdown } from "./delta.js"; +import { checkRunState, computeDelta, formatCheckChangeBullets, formatDeltaMarkdown, formatTransition } from "./delta.js"; import type { VibeReport } from "./types.js"; function makeReport(overrides: Partial = {}): VibeReport { @@ -114,3 +114,86 @@ 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("ran"); + 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)"); + }); +}); diff --git a/src/delta.ts b/src/delta.ts index d6a976e..185fcbf 100644 --- a/src/delta.ts +++ b/src/delta.ts @@ -5,7 +5,66 @@ */ import { readIssueFingerprint } from "./issue-fingerprint.js"; -import type { Issue, VibeReport } from "./types.js"; +import type { CheckResult, Issue, VibeReport } from "./types.js"; + +/** + * Whether a check produced a real score. A check that did not run carries a + * placeholder `score: 100` (core.ts), which must never be subtracted from a + * real score: "lint: 100 โ†’ 72" would read as a regression, and the reverse + * (a tool uninstalled) as an improvement (#107). + */ +export type CheckRunState = "ran" | "skipped" | "unavailable" | "absent"; + +/** One side of a check comparison. `score` is set only when the check ran. */ +export interface CheckSide { + state: CheckRunState; + score?: number; +} + +/** A check whose run state changed between two scans, e.g. lint: unavailable โ†’ 72. */ +export interface StatusTransition { + before: CheckSide; + after: CheckSide; +} + +/** + * Run state of a check. Reads `status` (the CLI writes it; it survives schema + * 0.5.0 via `.passthrough()`); older reports without it fall back to the + * details flags, in the order core.ts uses (`unavailable`/`comingSoon` + * before `skipped`, a "runner error:" reason counting as a run). + */ +export function checkRunState(check: CheckResult | undefined): CheckRunState { + if (!check) return "absent"; + const status = (check as CheckResult & { status?: unknown }).status; + if (status === "skipped" || status === "unavailable") return status; + if (typeof status === "string" && status) return "ran"; + const details = (check.details ?? {}) as Record; + const reason = typeof details.reason === "string" ? details.reason : ""; + if (reason.startsWith("runner error:")) return "ran"; + if (details.unavailable || details.comingSoon) return "unavailable"; + if (details.skipped) return "skipped"; + return "ran"; +} + +export function checkSide(check: CheckResult | undefined): CheckSide { + const state = checkRunState(check); + return state === "ran" && check ? { state, score: check.score } : { state }; +} + +/** + * Compare two sides of one check: a numeric delta when both ran, otherwise a + * status transition (or nothing, when the not-run state is unchanged). + */ +export function compareCheckSides(before: CheckSide, after: CheckSide): { delta: number; transition?: StatusTransition } { + if (before.state === "ran" && after.state === "ran") return { delta: (after.score ?? 0) - (before.score ?? 0) }; + if (before.state === after.state) return { delta: 0 }; + return { delta: 0, transition: { before, after } }; +} + +export function formatCheckSide(side: CheckSide): string { + if (side.state === "ran") return String(side.score); + return side.state === "absent" ? "not present" : side.state; +} export interface DeltaIssue { check: string; @@ -21,7 +80,10 @@ export interface CheckDelta { label: string; before: number; after: number; + /** Score change; 0 whenever either side did not run (see `transition`). */ delta: number; + /** Set instead of a numeric delta when the check's run state changed. */ + transition?: StatusTransition; fixed: DeltaIssue[]; introduced: DeltaIssue[]; } @@ -52,6 +114,7 @@ export function computeDelta(before: VibeReport, after: VibeReport): ScanDelta { for (const afterCheck of after.checks) { const beforeCheck = before.checks.find((c) => c.name === afterCheck.name); const beforeScore = beforeCheck?.score ?? 0; + const { delta: scoreChange, transition } = compareCheckSides(checkSide(beforeCheck), checkSide(afterCheck)); // Build multiset of issue keys for before and after const beforeKeys = new Map(); @@ -118,7 +181,8 @@ export function computeDelta(before: VibeReport, after: VibeReport): ScanDelta { label: afterCheck.name, before: beforeScore, after: afterCheck.score, - delta: afterCheck.score - beforeScore, + delta: scoreChange, + ...(transition ? { transition } : {}), fixed, introduced, }); @@ -128,12 +192,49 @@ export function computeDelta(before: VibeReport, after: VibeReport): ScanDelta { before: { score: before.score, grade: before.grade, timestamp: before.timestamp, issueCount: beforeIssueCount }, after: { score: after.score, grade: after.grade, timestamp: after.timestamp, issueCount: afterIssueCount }, scoreDelta: after.score - before.score, - checks: checks.filter((c) => c.delta !== 0 || c.fixed.length > 0 || c.introduced.length > 0), + checks: checks.filter((c) => c.delta !== 0 || c.transition || c.fixed.length > 0 || c.introduced.length > 0), fixed: allFixed, introduced: allIntroduced, }; } +/** Checks whose score changed, both sides having run; biggest gain first. */ +export function scoreChanges(delta: ScanDelta): CheckDelta[] { + return delta.checks.filter((c) => c.delta !== 0).sort((a, b) => b.delta - a.delta); +} + +/** Checks whose run state changed (ran โ†” skipped/unavailable/absent). */ +export function statusTransitions(delta: ScanDelta): (CheckDelta & { transition: StatusTransition })[] { + return delta.checks.filter((c): c is CheckDelta & { transition: StatusTransition } => c.transition !== undefined); +} + +export function formatTransition(t: StatusTransition): string { + return `${formatCheckSide(t.before)} โ†’ ${formatCheckSide(t.after)}`; +} + +/** + * Markdown bullet list of per-check changes, as the PR comment and the CLI + * summary print it: score changes, then status transitions separately. + */ +export function formatCheckChangeBullets(delta: ScanDelta, limit: number): string { + let md = ""; + const changed = scoreChanges(delta); + if (changed.length > 0) { + for (const c of changed.slice(0, limit)) { + const a = c.delta > 0 ? "+" : ""; + md += `- ${c.delta > 0 ? "โœ…" : "โš ๏ธ"} ${c.name}: ${c.before} โ†’ ${c.after} (${a}${c.delta})\n`; + } + md += "\n"; + } + const transitions = statusTransitions(delta); + if (transitions.length > 0) { + md += "**Status changes** (not scored):\n"; + for (const c of transitions.slice(0, limit)) md += `- ${c.name}: ${formatTransition(c.transition)}\n`; + md += "\n"; + } + return md; +} + /** Format a delta as a markdown report. */ export function formatDeltaMarkdown(delta: ScanDelta): string { const arrow = delta.scoreDelta > 0 ? "+" : ""; @@ -145,17 +246,28 @@ export function formatDeltaMarkdown(delta: ScanDelta): string { md += `| **Issues** | ${delta.before.issueCount} | ${delta.after.issueCount} | ${delta.fixed.length} fixed, ${delta.introduced.length} new |\n\n`; // Per-check changes - const changed = delta.checks.filter((c) => c.delta !== 0); + const changed = scoreChanges(delta); if (changed.length > 0) { md += `## Check Changes\n\n`; md += `| Check | Before | After | Delta |\n|---|---|---|---|\n`; - for (const c of changed.sort((a, b) => b.delta - a.delta)) { + for (const c of changed) { const a = c.delta > 0 ? "+" : ""; md += `| ${c.name} | ${c.before} | ${c.after} | ${a}${c.delta} |\n`; } md += "\n"; } + // Checks that started or stopped running: a placeholder score is not a score. + const transitions = statusTransitions(delta); + if (transitions.length > 0) { + md += `## Status Changes\n\n`; + md += `| Check | Before | After |\n|---|---|---|\n`; + for (const c of transitions) { + md += `| ${c.name} | ${formatCheckSide(c.transition.before)} | ${formatCheckSide(c.transition.after)} |\n`; + } + md += "\n"; + } + // Fixed issues if (delta.fixed.length > 0) { md += `## Fixed (${delta.fixed.length})\n\n`; diff --git a/src/pr-comment.test.ts b/src/pr-comment.test.ts index 54290c4..98358f9 100644 --- a/src/pr-comment.test.ts +++ b/src/pr-comment.test.ts @@ -25,3 +25,39 @@ describe("pr-comment module", () => { expect(result).toBe(false); }); }); + +describe("PR comment body (#107)", () => { + const base = { + version: "0.56.0", + timestamp: "2026-10-05T00:00:00Z", + score: 80, + grade: "B" as const, + meta: { cwd: "/tmp", node: "v22", duration: 100, stack: {} as any, repoUrl: null, branch: "main" }, + }; + const unavailable = { + name: "lint", + status: "unavailable", + score: 100, + grade: "A" as const, + details: { skipped: true, unavailable: true, status: "unavailable" }, + issues: [], + duration: 1, + }; + const scored = { name: "lint", status: "failed", score: 72, grade: "C" as const, details: { status: "failed" }, issues: [], duration: 1 }; + + it("lists lint unavailable โ†’ 72 as a status change, not a 100 โ†’ 72 regression", async () => { + const { buildCommentBody } = await import("./pr-comment.js"); + const body = buildCommentBody({ ...base, checks: [scored] }, null, { ...base, checks: [unavailable] }); + expect(body).toContain("lint: unavailable โ†’ 72"); + expect(body).not.toContain("100 โ†’ 72"); + expect(body).not.toContain("(-28)"); + }); + + it("does not show removing the tool as a +N improvement", async () => { + const { buildCommentBody } = await import("./pr-comment.js"); + const body = buildCommentBody({ ...base, checks: [unavailable] }, null, { ...base, checks: [{ ...scored, score: 64 }] }); + expect(body).toContain("lint: 64 โ†’ unavailable"); + expect(body).not.toContain("+36"); + expect(body).not.toContain("64 โ†’ 100"); + }); +}); diff --git a/src/pr-comment.ts b/src/pr-comment.ts index 0002a8d..53db173 100644 --- a/src/pr-comment.ts +++ b/src/pr-comment.ts @@ -2,7 +2,7 @@ import { execSync } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; -import { computeDelta } from "./delta.js"; +import { computeDelta, formatCheckChangeBullets } from "./delta.js"; import type { TrendDelta } from "./trend.js"; import type { VibeReport } from "./types.js"; @@ -74,7 +74,7 @@ function detectPR(cwd: string): PRInfo | null { return null; } -function buildCommentBody(report: VibeReport, trend: TrendDelta | null, prevReport?: VibeReport): string { +export function buildCommentBody(report: VibeReport, trend: TrendDelta | null, prevReport?: VibeReport): string { const grade = report.grade; const score = report.score; const gradeEmoji = grade === "A" ? "๐ŸŸข" : grade === "B" ? "๐ŸŸก" : grade === "C" ? "๐ŸŸ " : "๐Ÿ”ด"; @@ -89,14 +89,7 @@ function buildCommentBody(report: VibeReport, trend: TrendDelta | null, prevRepo if (delta.introduced.length > 0) body += ` ยท ${delta.introduced.length} new`; body += "\n\n"; - 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, 6)) { - const a = c.delta > 0 ? "+" : ""; - body += `- ${c.delta > 0 ? "โœ…" : "โš ๏ธ"} ${c.name}: ${c.before} โ†’ ${c.after} (${a}${c.delta})\n`; - } - body += "\n"; - } + body += formatCheckChangeBullets(delta, 6); } else if (trend) { const arrow = trend.scoreDelta > 0 ? "๐Ÿ“ˆ" : trend.scoreDelta < 0 ? "๐Ÿ“‰" : "โžก๏ธ"; body += `${arrow} **${trend.scoreDelta > 0 ? "+" : ""}${trend.scoreDelta}** vs previous`; diff --git a/src/report/pages.test.ts b/src/report/pages.test.ts index 2d3b32f..8c31149 100644 --- a/src/report/pages.test.ts +++ b/src/report/pages.test.ts @@ -225,3 +225,32 @@ describe("report generation", () => { expect(index).toContain("Feature Map"); }); }); + +describe("actions page delta (#107)", () => { + const unavailable: CheckResult = { + name: "lint", + score: 100, + grade: "A", + details: { skipped: true, unavailable: true, status: "unavailable" }, + issues: [], + duration: 1, + }; + const scored: CheckResult = { name: "lint", score: 72, grade: "C", details: { status: "failed" }, issues: [], duration: 1 }; + (unavailable as CheckResult & { status: string }).status = "unavailable"; + (scored as CheckResult & { status: string }).status = "failed"; + + it("shows the status transition and no 100 โ†’ 72 score change", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const html = generatePages(makeReport(dir, [scored]), undefined, makeReport(dir, [unavailable])).get("actions.html")!; + expect(html).toContain("Status changes:"); + expect(html).toContain("lint: unavailable โ†’ 72"); + expect(html).not.toContain("lint -28"); + }); + + it("does not show removing the tool as an improvement", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const html = generatePages(makeReport(dir, [unavailable]), undefined, makeReport(dir, [{ ...scored, score: 64 }])).get("actions.html")!; + expect(html).toContain("lint: 64 โ†’ unavailable"); + expect(html).not.toContain("lint +36"); + }); +}); diff --git a/src/report/pages.ts b/src/report/pages.ts index e38f7f9..a898815 100644 --- a/src/report/pages.ts +++ b/src/report/pages.ts @@ -4,7 +4,7 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { type CheckMeta, getCheckMeta } from "../check-meta.js"; import { suggestFix } from "../commands/shared.js"; -import type { ScanDelta } from "../delta.js"; +import { formatTransition, type ScanDelta, scoreChanges, statusTransitions } from "../delta.js"; import { buildCoverageMapInput, generateCoverageMap } from "../diagrams/coverage.js"; import { loadHistory } from "../history.js"; import { @@ -1001,7 +1001,7 @@ export function actionsPage(allChecks: CheckResult[], fl: FL, linter: string, de const dArrow = delta.scoreDelta > 0 ? "▲" : delta.scoreDelta < 0 ? "▼" : "▬"; // Per-check changes - const changed = delta.checks.filter((c) => c.delta !== 0).sort((a, b) => b.delta - a.delta); + const changed = scoreChanges(delta); const checkDeltas = changed .slice(0, 10) .map((c) => { @@ -1009,6 +1009,12 @@ export function actionsPage(allChecks: CheckResult[], fl: FL, linter: string, de return `${c.name} ${c.delta > 0 ? "+" : ""}${c.delta}`; }) .join(""); + // Checks that started or stopped running โ€” listed apart from score changes, + // because a not-run check's placeholder 100 is not a score (#107). + const statusChips = statusTransitions(delta) + .slice(0, 10) + .map((c) => `${e(c.name)}: ${e(formatTransition(c.transition))}`) + .join(""); // Fixed issues list let fixedList = ""; @@ -1047,6 +1053,7 @@ export function actionsPage(allChecks: CheckResult[], fl: FL, linter: string, de ${delta.after.issueCount} remaining ${checkDeltas ? `
${checkDeltas}
` : ""} + ${statusChips ? `
Status changes: ${statusChips}
` : ""} ${fixedList}${newList} `; } diff --git a/src/trend.test.ts b/src/trend.test.ts index 59f43e6..de273ab 100644 --- a/src/trend.test.ts +++ b/src/trend.test.ts @@ -106,6 +106,40 @@ describe("computeTrend", () => { }); }); +describe("computeTrend not-run checks (#107)", () => { + it("emits a status transition instead of a numeric delta when either side did not run", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-trend-")); + const prev = makeReport(70, [{ name: "lint", score: 100, issues: 0 }]); + prev.checks[0].details = { skipped: true, unavailable: true, status: "unavailable" }; + writeFileSync(join(dir, "report.json"), JSON.stringify(prev)); + const curr = makeReport(70, [{ name: "lint", score: 72, issues: 0 }]); + curr.checks[0].details = { status: "failed" }; + + const trend = computeTrend(curr, dir)!; + const lint = trend.checkDeltas.find((d) => d.name === "lint")!; + expect(lint.delta).toBe(0); + expect(lint.transition).toEqual({ before: { state: "unavailable" }, after: { state: "ran", score: 72 } }); + + const html = trendHTML(trend); + expect(html).toContain("lint: unavailable โ†’ 72"); + expect(html).not.toContain("lint -28"); + }); + + it("does not report a removed tool as +N", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-trend-")); + const prev = makeReport(70, [{ name: "lint", score: 64, issues: 0 }]); + writeFileSync(join(dir, "report.json"), JSON.stringify(prev)); + const curr = makeReport(70, [{ name: "lint", score: 100, issues: 0 }]); + curr.checks[0].details = { skipped: true, comingSoon: true }; + + const trend = computeTrend(curr, dir)!; + expect(trend.checkDeltas[0].delta).toBe(0); + const html = trendHTML(trend); + expect(html).toContain("lint: 64 โ†’ unavailable"); + expect(html).not.toContain("+36"); + }); +}); + describe("formatTrend", () => { it("formats improvement", () => { const out = formatTrend({ diff --git a/src/trend.ts b/src/trend.ts index 13359cd..80557e9 100644 --- a/src/trend.ts +++ b/src/trend.ts @@ -2,12 +2,14 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; +import { checkSide, compareCheckSides, formatTransition, type StatusTransition } from "./delta.js"; import { type IssueSnapshot, issueSnapshot, readIssueFingerprint } from "./issue-fingerprint.js"; import type { VibeReport } from "./types.js"; export interface TrendDelta { scoreDelta: number; // positive = improved - checkDeltas: { name: string; prev: number; curr: number; delta: number }[]; + /** `delta` is 0 and `transition` is set when either side did not run (#107). */ + checkDeltas: { name: string; prev: number; curr: number; delta: number; transition?: StatusTransition }[]; newIssues: number; fixedIssues: number; introduced?: IssueSnapshot[]; @@ -34,11 +36,13 @@ export function computeTrend(report: VibeReport, outputDir: string): TrendDelta for (const curr of report.checks) { const prevCheck = prev.checks.find((c) => c.name === curr.name); const prevScore = prevCheck?.score ?? 0; + const { delta, transition } = compareCheckSides(checkSide(prevCheck), checkSide(curr)); checkDeltas.push({ name: curr.name, prev: prevScore, curr: curr.score, - delta: curr.score - prevScore, + delta, + ...(transition ? { transition } : {}), }); } @@ -107,10 +111,16 @@ export function trendHTML(trend: TrendDelta): string { const c = d.delta > 0 ? "var(--pass)" : "var(--fail)"; deltas += `${d.name} ${d.delta > 0 ? "+" : ""}${d.delta}`; } + let transitions = ""; + for (const d of trend.checkDeltas) { + if (!d.transition) continue; + transitions += `${d.name}: ${formatTransition(d.transition)}`; + } return `
${arrow}${trend.scoreDelta > 0 ? "+" : ""}${trend.scoreDelta} ptsvs ${trend.prevTimestamp.split("T")[0]}
${trend.fixedIssues > 0 ? `${trend.fixedIssues} fixed` : ""}${trend.newIssues > 0 ? `${trend.newIssues} new` : ""}
${deltas ? `
${deltas}
` : ""} +${transitions ? `
${transitions}
` : ""}
`; } From e15fbcd131563b02ac34d15e2cf4c9ca4ebcd25f Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 19:18:16 +1100 Subject: [PATCH 2/8] fix(delta): cover checks that disappeared and drop placeholder scores from CheckDelta (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit computeDelta iterated only the newer report's checks, so a check present only in the older scan (tool removed, check dropped or renamed) produced no "72 โ†’ not present" transition and its issues were missing from the per-check fixed list. It now walks the union of check names. CheckDelta.before/after are now null for a side that did not run or was absent, instead of the placeholder 100, so a JSON consumer printing before โ†’ after cannot reproduce the misleading score. No consumer in app, mcp or github-app reads CheckDelta; in this repo only score changes (both sides ran) print before/after. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/delta.test.ts | 26 ++++++++++ src/delta.ts | 123 ++++++++++++++++++++++------------------------ 2 files changed, 84 insertions(+), 65 deletions(-) diff --git a/src/delta.test.ts b/src/delta.test.ts index 3a40119..0d3222b 100644 --- a/src/delta.test.ts +++ b/src/delta.test.ts @@ -196,4 +196,30 @@ describe("not-run checks (#107)", () => { 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 counts 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(2); + expect(delta.fixed.map((f) => f.check)).toEqual(["lint", "lint"]); + expect(formatCheckChangeBullets(delta, 8)).toContain("lint: 72 โ†’ not present"); + expect(formatDeltaMarkdown(delta)).toContain("## Fixed (2)"); + }); + + 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'); + }); }); diff --git a/src/delta.ts b/src/delta.ts index 185fcbf..9ec6e63 100644 --- a/src/delta.ts +++ b/src/delta.ts @@ -78,8 +78,10 @@ export interface DeltaIssue { export interface CheckDelta { name: string; label: string; - before: number; - after: number; + /** Score before; null when the check did not run or was absent (see `transition`). */ + before: number | null; + /** Score after; null when the check did not run or was absent (see `transition`). */ + after: number | null; /** Score change; 0 whenever either side did not run (see `transition`). */ delta: number; /** Set instead of a numeric delta when the check's run state changed. */ @@ -102,6 +104,38 @@ function issueKey(check: string, iss: Issue): string { return readIssueFingerprint(check, iss); } +type IssueMultiset = Map; + +function issueMultiset(name: string, check: CheckResult | undefined): IssueMultiset { + const out: IssueMultiset = new Map(); + for (const iss of check?.issues ?? []) { + const key = issueKey(name, iss); + const entry = out.get(key); + if (entry) entry.count++; + else out.set(key, { count: 1, issue: iss }); + } + return out; +} + +/** Issues in `from` beyond their count in `minus`, as DeltaIssues. */ +function multisetDifference(name: string, from: IssueMultiset, minus: IssueMultiset): DeltaIssue[] { + const out: DeltaIssue[] = []; + for (const [key, entry] of from) { + const diff = entry.count - (minus.get(key)?.count ?? 0); + for (let i = 0; i < diff; i++) { + out.push({ + check: name, + severity: entry.issue.severity, + message: entry.issue.message, + file: typeof entry.issue.file === "string" ? entry.issue.file : undefined, + line: entry.issue.line, + rule: entry.issue.rule, + }); + } + } + return out; +} + /** Compute a structured delta between two scan reports. */ export function computeDelta(before: VibeReport, after: VibeReport): ScanDelta { const beforeIssueCount = before.checks.reduce((s, c) => s + c.issues.length, 0); @@ -111,76 +145,35 @@ export function computeDelta(before: VibeReport, after: VibeReport): ScanDelta { const allFixed: DeltaIssue[] = []; const allIntroduced: DeltaIssue[] = []; - for (const afterCheck of after.checks) { - const beforeCheck = before.checks.find((c) => c.name === afterCheck.name); - const beforeScore = beforeCheck?.score ?? 0; - const { delta: scoreChange, transition } = compareCheckSides(checkSide(beforeCheck), checkSide(afterCheck)); + // Union of check names: a check present only in `before` (tool removed, + // check dropped or renamed) still gets a "72 โ†’ not present" transition and + // its issues count as fixed. + const names = [...after.checks.map((c) => c.name)]; + for (const c of before.checks) if (!names.includes(c.name)) names.push(c.name); - // Build multiset of issue keys for before and after - const beforeKeys = new Map(); - const afterKeys = new Map(); - - if (beforeCheck) { - for (const iss of beforeCheck.issues) { - const key = issueKey(afterCheck.name, iss); - const entry = beforeKeys.get(key); - if (entry) entry.count++; - else beforeKeys.set(key, { count: 1, issue: iss }); - } - } - for (const iss of afterCheck.issues) { - const key = issueKey(afterCheck.name, iss); - const entry = afterKeys.get(key); - if (entry) entry.count++; - else afterKeys.set(key, { count: 1, issue: iss }); - } + for (const name of names) { + const beforeCheck = before.checks.find((c) => c.name === name); + const afterCheck = after.checks.find((c) => c.name === name); + const beforeSide = checkSide(beforeCheck); + const afterSide = checkSide(afterCheck); + const { delta: scoreChange, transition } = compareCheckSides(beforeSide, afterSide); - const fixed: DeltaIssue[] = []; - const introduced: DeltaIssue[] = []; + // Build multiset of issue keys for before and after + const beforeKeys = issueMultiset(name, beforeCheck); + const afterKeys = issueMultiset(name, afterCheck); // Fixed: in before but not in after (or count decreased) - for (const [key, bEntry] of beforeKeys) { - const aEntry = afterKeys.get(key); - const aCount = aEntry?.count ?? 0; - const diff = bEntry.count - aCount; - for (let i = 0; i < diff; i++) { - const di: DeltaIssue = { - check: afterCheck.name, - severity: bEntry.issue.severity, - message: bEntry.issue.message, - file: typeof bEntry.issue.file === "string" ? bEntry.issue.file : undefined, - line: bEntry.issue.line, - rule: bEntry.issue.rule, - }; - fixed.push(di); - allFixed.push(di); - } - } - + const fixed = multisetDifference(name, beforeKeys, afterKeys); // Introduced: in after but not in before (or count increased) - for (const [key, aEntry] of afterKeys) { - const bEntry = beforeKeys.get(key); - const bCount = bEntry?.count ?? 0; - const diff = aEntry.count - bCount; - for (let i = 0; i < diff; i++) { - const di: DeltaIssue = { - check: afterCheck.name, - severity: aEntry.issue.severity, - message: aEntry.issue.message, - file: typeof aEntry.issue.file === "string" ? aEntry.issue.file : undefined, - line: aEntry.issue.line, - rule: aEntry.issue.rule, - }; - introduced.push(di); - allIntroduced.push(di); - } - } + const introduced = multisetDifference(name, afterKeys, beforeKeys); + allFixed.push(...fixed); + allIntroduced.push(...introduced); checks.push({ - name: afterCheck.name, - label: afterCheck.name, - before: beforeScore, - after: afterCheck.score, + name, + label: name, + before: beforeSide.score ?? null, + after: afterSide.score ?? null, delta: scoreChange, ...(transition ? { transition } : {}), fixed, From ea5deefe512011810ff86134c2ed9667538dcb10 Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 19:18:17 +1100 Subject: [PATCH 3/8] refactor(trend): match computeDelta for dropped checks; remove unused trendHTML (#107) computeTrend now walks the union of check names and carries null instead of the placeholder score for a not-run side, like computeDelta. trendHTML had no caller since the HTML report moved its "since last scan" banner to the actions page, which already renders score changes and status transitions from computeDelta. Its transition chips were dead code, so it is removed rather than wired in a second time. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/trend.test.ts | 40 ++++++++++++++++------------------------ src/trend.ts | 47 +++++++++++++---------------------------------- 2 files changed, 29 insertions(+), 58 deletions(-) diff --git a/src/trend.test.ts b/src/trend.test.ts index de273ab..efe87aa 100644 --- a/src/trend.test.ts +++ b/src/trend.test.ts @@ -2,7 +2,8 @@ import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { computeTrend, formatTrend, trendHTML } from "./trend.js"; +import { formatTransition } from "./delta.js"; +import { computeTrend, formatTrend } from "./trend.js"; import type { VibeReport } from "./types.js"; function makeReport(score: number, checks: { name: string; score: number; issues: number }[]): VibeReport { @@ -120,9 +121,8 @@ describe("computeTrend not-run checks (#107)", () => { expect(lint.delta).toBe(0); expect(lint.transition).toEqual({ before: { state: "unavailable" }, after: { state: "ran", score: 72 } }); - const html = trendHTML(trend); - expect(html).toContain("lint: unavailable โ†’ 72"); - expect(html).not.toContain("lint -28"); + expect(lint).toMatchObject({ prev: null, curr: 72 }); + expect(formatTransition(lint.transition!)).toBe("unavailable โ†’ 72"); }); it("does not report a removed tool as +N", () => { @@ -133,10 +133,18 @@ describe("computeTrend not-run checks (#107)", () => { curr.checks[0].details = { skipped: true, comingSoon: true }; const trend = computeTrend(curr, dir)!; - expect(trend.checkDeltas[0].delta).toBe(0); - const html = trendHTML(trend); - expect(html).toContain("lint: 64 โ†’ unavailable"); - expect(html).not.toContain("+36"); + expect(trend.checkDeltas[0]).toMatchObject({ prev: 64, curr: null, delta: 0 }); + expect(formatTransition(trend.checkDeltas[0].transition!)).toBe("64 โ†’ unavailable"); + }); + + it("reports a check that disappeared as a transition", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-trend-")); + writeFileSync(join(dir, "report.json"), JSON.stringify(makeReport(70, [{ name: "lint", score: 72, issues: 2 }]))); + const trend = computeTrend(makeReport(70, []), dir)!; + expect(trend.checkDeltas).toHaveLength(1); + expect(trend.checkDeltas[0]).toMatchObject({ name: "lint", prev: 72, curr: null, delta: 0 }); + expect(formatTransition(trend.checkDeltas[0].transition!)).toBe("72 โ†’ not present"); + expect(trend.fixedIssues).toBe(2); }); }); @@ -176,19 +184,3 @@ describe("formatTrend", () => { expect(out).toMatch(/[โ–โ–‚โ–ƒโ–„โ–…โ–†โ–‡โ–ˆ]/); }); }); - -describe("trendHTML", () => { - it("generates valid HTML", () => { - const html = trendHTML({ - scoreDelta: 5, - checkDeltas: [{ name: "lint", prev: 70, curr: 75, delta: 5 }], - newIssues: 0, - fixedIssues: 2, - prevTimestamp: "2026-05-29T00:00:00Z", - }); - expect(html).toContain("+5 pts"); - expect(html).toContain("2 fixed"); - expect(html).toContain("lint +5"); - expect(html).toContain("var(--pass)"); - }); -}); diff --git a/src/trend.ts b/src/trend.ts index 80557e9..fda2e4c 100644 --- a/src/trend.ts +++ b/src/trend.ts @@ -2,14 +2,15 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; -import { checkSide, compareCheckSides, formatTransition, type StatusTransition } from "./delta.js"; +import { checkSide, compareCheckSides, type StatusTransition } from "./delta.js"; import { type IssueSnapshot, issueSnapshot, readIssueFingerprint } from "./issue-fingerprint.js"; import type { VibeReport } from "./types.js"; export interface TrendDelta { scoreDelta: number; // positive = improved /** `delta` is 0 and `transition` is set when either side did not run (#107). */ - checkDeltas: { name: string; prev: number; curr: number; delta: number; transition?: StatusTransition }[]; + /** `prev`/`curr` are null for a side that did not run or was absent. */ + checkDeltas: { name: string; prev: number | null; curr: number | null; delta: number; transition?: StatusTransition }[]; newIssues: number; fixedIssues: number; introduced?: IssueSnapshot[]; @@ -33,14 +34,17 @@ export function computeTrend(report: VibeReport, outputDir: string): TrendDelta const scoreDelta = report.score - prev.score; const checkDeltas: TrendDelta["checkDeltas"] = []; - for (const curr of report.checks) { - const prevCheck = prev.checks.find((c) => c.name === curr.name); - const prevScore = prevCheck?.score ?? 0; - const { delta, transition } = compareCheckSides(checkSide(prevCheck), checkSide(curr)); + // Union of names, so a check that disappeared reads "72 โ†’ not present". + const names = report.checks.map((c) => c.name); + for (const c of prev.checks) if (!names.includes(c.name)) names.push(c.name); + for (const name of names) { + const prevSide = checkSide(prev.checks.find((c) => c.name === name)); + const currSide = checkSide(report.checks.find((c) => c.name === name)); + const { delta, transition } = compareCheckSides(prevSide, currSide); checkDeltas.push({ - name: curr.name, - prev: prevScore, - curr: curr.score, + name, + prev: prevSide.score ?? null, + curr: currSide.score ?? null, delta, ...(transition ? { transition } : {}), }); @@ -99,28 +103,3 @@ function terminalSparkline(values: number[]): string { }) .join(""); } - -/** Render trend HTML for the report. */ -export function trendHTML(trend: TrendDelta): string { - const arrow = trend.scoreDelta > 0 ? "▲" : trend.scoreDelta < 0 ? "▼" : "▬"; - const color = trend.scoreDelta > 0 ? "var(--pass)" : trend.scoreDelta < 0 ? "var(--fail)" : "var(--muted)"; - - let deltas = ""; - for (const d of trend.checkDeltas) { - if (d.delta === 0) continue; - const c = d.delta > 0 ? "var(--pass)" : "var(--fail)"; - deltas += `${d.name} ${d.delta > 0 ? "+" : ""}${d.delta}`; - } - let transitions = ""; - for (const d of trend.checkDeltas) { - if (!d.transition) continue; - transitions += `${d.name}: ${formatTransition(d.transition)}`; - } - - return `
-
${arrow}${trend.scoreDelta > 0 ? "+" : ""}${trend.scoreDelta} ptsvs ${trend.prevTimestamp.split("T")[0]}
-
${trend.fixedIssues > 0 ? `${trend.fixedIssues} fixed` : ""}${trend.newIssues > 0 ? `${trend.newIssues} new` : ""}
-${deltas ? `
${deltas}
` : ""} -${transitions ? `
${transitions}
` : ""} -
`; -} From 26811b2aca89d96eac2a7fb6ea2cf66875a6460d Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 19:26:09 +1100 Subject: [PATCH 4/8] fix(report): stop plotting and diffing placeholder scores on the Trends page (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit loadHistory put every check's raw score into checkScores, so a check that did not run contributed its placeholder 100: uninstalling a linter showed as "lint 64 โ†’ 100 +36" in the first โ†’ latest table and the per-check card plotted the 100. - history.ts: checkScores holds only checks that ran; a new checkStates map records each check's run state, read with checkRunState (status, else the details flags that history snapshots already keep). - Trends page: per-check charts plot only the scans where the check ran (a not-run scan is a gap) and the card shows the current state instead of a score when the check is not running. The first โ†’ latest table shows a numeric delta only when the check ran at both ends, otherwise a status row such as "lint 64 โ†’ unavailable" or "structure 80 โ†’ not present". The terminal monitor's Trends screen reads the same checkScores, so it stops plotting the placeholder too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/history.test.ts | 29 +++++++++++ src/history.ts | 38 +++++++++----- src/report/pages.test.ts | 70 ++++++++++++++++++++++++++ src/report/pages.ts | 106 +++++++++++++++++++++++++-------------- src/report/styles.ts | 2 + 5 files changed, 196 insertions(+), 49 deletions(-) diff --git a/src/history.test.ts b/src/history.test.ts index e8307e5..039423b 100644 --- a/src/history.test.ts +++ b/src/history.test.ts @@ -178,6 +178,35 @@ describe("loadHistory", () => { expect(entries[0].checkScores.has("testing")).toBe(false); }); + it("keeps not-run checks out of checkScores and records their run state (#107)", () => { + writeFileSync( + join(tmp, "2026-05-15T11-00-00.json"), + JSON.stringify({ + timestamp: "2026-05-15T11:00:00.000Z", + score: 80, + checks: [ + { name: "lint", score: 100, details: { skipped: true, unavailable: true, status: "unavailable" } }, + { name: "ai-review", score: 100, details: { comingSoon: true } }, + { name: "deps", score: 100, details: { skipped: true } }, + { name: "types", score: 0, details: { skipped: true, reason: "runner error: boom" } }, + { name: "structure", score: 90, details: {} }, + ], + }), + ); + const [entry] = loadHistory(tmp); + expect([...entry.checkScores]).toEqual([ + ["types", 0], + ["structure", 90], + ]); + expect(Object.fromEntries(entry.checkStates)).toEqual({ + lint: "unavailable", + "ai-review": "unavailable", + deps: "skipped", + types: "ran", + structure: "ran", + }); + }); + it("limits to last 30 entries", () => { for (let i = 0; i < 40; i++) { const day = String(i + 1).padStart(2, "0"); diff --git a/src/history.ts b/src/history.ts index 302e761..2436608 100644 --- a/src/history.ts +++ b/src/history.ts @@ -2,13 +2,17 @@ import { existsSync, readdirSync, readFileSync } from "node:fs"; import { join } from "node:path"; +import { type CheckRunState, checkRunState } from "./delta.js"; import type { IssueSnapshot } from "./issue-fingerprint.js"; import type { AnalyzerSnapshot, VibeReport } from "./types.js"; export interface HistoryEntry { timestamp: string; score: number; + /** Scores of checks that ran. A not-run check's placeholder 100 is left out (#107). */ checkScores: Map; + /** Run state of every check in the snapshot (from `status`, else the details flags). */ + checkStates: Map>; issues: IssueSnapshot[]; analyzerSnapshots: AnalyzerSnapshot[]; } @@ -27,20 +31,11 @@ export function loadHistory(historyDir: string): HistoryEntry[] { const raw: VibeReport = JSON.parse(readFileSync(join(historyDir, file), "utf-8")); if (raw.score === null || raw.score === undefined || !Number.isFinite(Number(raw.score)) || !raw.checks) continue; const checkScores = new Map(); + const checkStates: HistoryEntry["checkStates"] = new Map(); const issues: IssueSnapshot[] = []; const analyzerSnapshots = normalizeAnalyzerSnapshots(raw.meta?.analyzerSnapshots); - for (const c of raw.checks) { - if (c.score !== null && c.score !== undefined && Number.isFinite(Number(c.score))) { - checkScores.set(c.name, Number(c.score)); - } - if (Array.isArray((c as { issues?: unknown }).issues)) { - for (const issue of (c as { issues: unknown[] }).issues) { - const snapshot = normalizeHistoryIssue(c.name, issue); - if (snapshot) issues.push(snapshot); - } - } - } - entries.push({ timestamp: raw.timestamp, score: Number(raw.score), checkScores, issues, analyzerSnapshots }); + for (const c of raw.checks) readHistoryCheck(c, checkScores, checkStates, issues); + entries.push({ timestamp: raw.timestamp, score: Number(raw.score), checkScores, checkStates, issues, analyzerSnapshots }); } catch { // skip corrupt files } @@ -49,6 +44,25 @@ export function loadHistory(historyDir: string): HistoryEntry[] { return entries.slice(-30); } +function readHistoryCheck( + c: VibeReport["checks"][number], + checkScores: HistoryEntry["checkScores"], + checkStates: HistoryEntry["checkStates"], + issues: IssueSnapshot[], +): void { + const state = checkRunState(c); + if (state !== "absent") checkStates.set(c.name, state); + if (state === "ran" && c.score !== null && c.score !== undefined && Number.isFinite(Number(c.score))) { + checkScores.set(c.name, Number(c.score)); + } + if (Array.isArray((c as { issues?: unknown }).issues)) { + for (const issue of (c as { issues: unknown[] }).issues) { + const snapshot = normalizeHistoryIssue(c.name, issue); + if (snapshot) issues.push(snapshot); + } + } +} + function normalizeAnalyzerSnapshots(raw: unknown): AnalyzerSnapshot[] { if (!Array.isArray(raw)) return []; return raw.flatMap((item) => { diff --git a/src/report/pages.test.ts b/src/report/pages.test.ts index 8c31149..2b84767 100644 --- a/src/report/pages.test.ts +++ b/src/report/pages.test.ts @@ -2,6 +2,7 @@ import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; +import { buildReportHistorySnapshot } from "../report-contract.js"; import type { CheckResult, VibeReport } from "../types.js"; // We test categoryPage indirectly through generatePages @@ -254,3 +255,72 @@ describe("actions page delta (#107)", () => { expect(html).not.toContain("lint +36"); }); }); + +describe("trends page with not-run checks (#107)", () => { + function writeHistory(dir: string, entries: { timestamp: string; checks: CheckResult[] }[]): string { + const historyDir = join(dir, "history"); + mkdirSync(historyDir, { recursive: true }); + for (const { timestamp, checks } of entries) { + const report = { ...makeReport(dir, checks), timestamp }; + writeFileSync(join(historyDir, `${timestamp}.json`), JSON.stringify(buildReportHistorySnapshot(report))); + } + return historyDir; + } + const lint = (score: number): CheckResult => ({ + name: "lint", + score, + grade: "C", + details: { status: "failed" }, + issues: [], + duration: 1, + }); + const lintUnavailable: CheckResult = { + name: "lint", + score: 100, + grade: "A", + details: { skipped: true, unavailable: true, status: "unavailable" }, + issues: [], + duration: 1, + }; + const structure = (score: number): CheckResult => ({ name: "structure", score, grade: "B", details: {}, issues: [], duration: 1 }); + const trendRows = (html: string) => + [...html.matchAll(/
(.*?)<\/div>/g)].map((m) => + m[1] + .replace(/<[^>]+>/g, " ") + .replace(/\s+/g, " ") + .trim(), + ); + + it("shows uninstalling a tool as a transition, not +36", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const historyDir = writeHistory(dir, [ + { timestamp: "2026-10-01T00:00:00.000Z", checks: [lint(64), structure(80)] }, + { timestamp: "2026-10-02T00:00:00.000Z", checks: [lint(70), structure(84)] }, + { timestamp: "2026-10-03T00:00:00.000Z", checks: [lintUnavailable, structure(86)] }, + ]); + const r2 = makeReport(dir, [lintUnavailable, structure(86)]); + const html = generatePages(r2, historyDir).get("trends.html")!; + const rows = trendRows(html); + expect(rows).toContain("structure 80 โ†’ 86 +6"); + expect(rows).toContain("lint 64 โ†’ unavailable status"); + expect(html).not.toContain("+36"); + expect(rows.join("\n")).not.toMatch(/lint.*100/); + // The lint card plots only the scans where lint ran and labels its current state. + expect(html).toMatch(/lint<\/span>unavailable<\/span>/); + }); + + it("shows installing a tool, and a dropped check, as transitions (legacy details flags)", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const legacySkipped: CheckResult = { name: "lint", score: 100, grade: "A", details: { skipped: true }, issues: [], duration: 1 }; + const historyDir = writeHistory(dir, [ + { timestamp: "2026-10-01T00:00:00.000Z", checks: [legacySkipped, structure(80)] }, + { timestamp: "2026-10-02T00:00:00.000Z", checks: [lint(72)] }, + { timestamp: "2026-10-03T00:00:00.000Z", checks: [lint(75)] }, + ]); + const html = generatePages(makeReport(dir, [lint(75)]), historyDir).get("trends.html")!; + const rows = trendRows(html); + expect(rows).toContain("lint skipped โ†’ 75 status"); + expect(rows).toContain("structure 80 โ†’ not present status"); + expect(html).not.toContain("-25"); + }); +}); diff --git a/src/report/pages.ts b/src/report/pages.ts index a898815..813f442 100644 --- a/src/report/pages.ts +++ b/src/report/pages.ts @@ -4,9 +4,17 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { type CheckMeta, getCheckMeta } from "../check-meta.js"; import { suggestFix } from "../commands/shared.js"; -import { formatTransition, type ScanDelta, scoreChanges, statusTransitions } from "../delta.js"; +import { + type CheckSide, + compareCheckSides, + formatCheckSide, + formatTransition, + type ScanDelta, + scoreChanges, + statusTransitions, +} from "../delta.js"; import { buildCoverageMapInput, generateCoverageMap } from "../diagrams/coverage.js"; -import { loadHistory } from "../history.js"; +import { type HistoryEntry, loadHistory } from "../history.js"; import { generateArchSVG, generateDSM, @@ -698,59 +706,74 @@ export function trendsPage(historyDir: string | undefined): string { { width: 700, height: 150 }, ); - // Collect all check names from latest entry + // Check names from the latest entry, plus any the first entry had (so a + // dropped check reads "72 โ†’ not present" instead of vanishing). + const first = history[0]; const latest = history[history.length - 1]; - const checkNames = [...latest.checkScores.keys()]; + const checkNames = [...latest.checkStates.keys()]; + for (const name of first.checkStates.keys()) if (!checkNames.includes(name)) checkNames.push(name); - // Per-check mini charts + // Per-check mini charts. Only scans where the check ran are plotted: a + // not-run check's placeholder 100 is not a score, so it is a gap (#107). const checkCharts = checkNames .map((name) => { - const data = history.map((h) => ({ score: h.checkScores.get(name) ?? 0, timestamp: h.timestamp })).filter((d) => d.score > 0); + const data = history.flatMap((h) => { + const score = h.checkScores.get(name); + return score === undefined ? [] : [{ score, timestamp: h.timestamp }]; + }); if (data.length < 2) return ""; - const current = data[data.length - 1].score; - const prev = data[data.length - 2].score; - const delta = current - prev; - const deltaStr = - delta > 0 - ? `+${delta}` - : delta < 0 - ? `${delta}` - : `=`; - const color = current >= 90 ? "var(--pass)" : current >= 75 ? "#84cc16" : current >= 60 ? "var(--warn)" : "var(--fail)"; - + const latestSide = historySide(latest, name); const chart = buildTimeline(data, { width: 300, height: 60 }); + let headline: string; + if (latestSide.state === "ran") { + const current = latestSide.score ?? 0; + const delta = current - data[data.length - 2].score; + const deltaStr = + delta > 0 + ? `+${delta}` + : delta < 0 + ? `${delta}` + : `=`; + const color = current >= 90 ? "var(--pass)" : current >= 75 ? "#84cc16" : current >= 60 ? "var(--warn)" : "var(--fail)"; + headline = `${current}${deltaStr}`; + } else { + headline = `${e(formatCheckSide(latestSide))}`; + } return `
-
${e(name)}${current}${deltaStr}
+
${e(name)}${headline}
${chart}
`; }) .filter(Boolean) .join(""); - // Score delta table - const deltaRows = checkNames - .map((name) => { - const scores = history.map((h) => h.checkScores.get(name) ?? 0); - const first = scores.find((s) => s > 0) ?? 0; - const last = scores[scores.length - 1]; - const delta = last - first; - if (first === 0 && last === 0) return ""; - return { name, first, last, delta }; - }) - .filter(Boolean) - .sort((a, b) => (b as { delta: number }).delta - (a as { delta: number }).delta) as { - name: string; - first: number; - last: number; - delta: number; - }[]; + // First โ†’ latest: a numeric delta only when the check ran at both ends; + // otherwise a status transition such as "64 โ†’ unavailable". + type DeltaRow = { name: string; before: CheckSide; after: CheckSide; delta: number; transition: boolean }; + const deltaRows: DeltaRow[] = []; + for (const name of checkNames) { + const firstEntry = history.find((h) => h.checkStates.has(name)); + if (!firstEntry) continue; + const before = historySide(firstEntry, name); + const after = historySide(latest, name); + const { delta, transition } = compareCheckSides(before, after); + if (before.state !== "ran" && !transition) continue; // never ran: nothing to show + deltaRows.push({ name, before, after, delta, transition: !!transition }); + } + deltaRows.sort((a, b) => Number(a.transition) - Number(b.transition) || b.delta - a.delta); const deltaTable = deltaRows .map((r) => { - const clr = r.delta > 0 ? "var(--pass)" : r.delta < 0 ? "var(--fail)" : "var(--muted)"; - return `
${e(r.name)}${r.first}\u2192${r.last}${r.delta > 0 ? "+" : ""}${r.delta}
`; + const cell = (side: CheckSide) => + side.state === "ran" + ? `${side.score}` + : `${e(formatCheckSide(side))}`; + const tail = r.transition + ? `status` + : `${r.delta > 0 ? "+" : ""}${r.delta}`; + return `
${e(r.name)}${cell(r.before)}\u2192${cell(r.after)}${tail}
`; }) .join(""); @@ -768,6 +791,15 @@ export function trendsPage(historyDir: string | undefined): string {
${checkCharts}
`; } +/** One check's side in a history entry: its score when it ran, else its run state. */ +function historySide(entry: HistoryEntry, name: string): CheckSide { + const state = entry.checkStates.get(name); + if (!state) return { state: "absent" }; + if (state !== "ran") return { state }; + const score = entry.checkScores.get(name); + return score === undefined ? { state: "absent" } : { state, score }; +} + // โ”€โ”€ Feature Map (Pro) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ export function featureMapPage(deadPatternsCheck: CheckResult | undefined, fl: FL): string { diff --git a/src/report/styles.ts b/src/report/styles.ts index 4c4a04d..b21c7bf 100644 --- a/src/report/styles.ts +++ b/src/report/styles.ts @@ -226,6 +226,8 @@ h3{font-size:0.85rem;color:var(--muted);text-transform:uppercase;letter-spacing: .trend-row-val{width:2rem;text-align:center;color:var(--muted)} .trend-row-arrow{color:var(--muted);font-size:0.6rem} .trend-row-delta{width:2.5rem;text-align:right;font-weight:700} +.trend-row-state{width:auto;min-width:2rem;font-weight:400;font-style:italic} +.trend-status{font-size:0.75rem;font-style:italic} .footer{text-align:center;color:var(--muted);font-size:0.58rem;margin-top:2rem;padding:0.8rem 0;border-top:1px solid var(--border)} .footer a{color:var(--muted)} From 5a0df8682a5f4b661ff17bb5b8a532a8872c9310 Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 19:47:46 +1100 Subject: [PATCH 5/8] fix(delta): show a crashed runner as a status transition, not a 0 score (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A check whose runner threw ("runner error:" reason, core.ts) counts as having run, with status failed, but its 0/F is a placeholder. Deltas and trends subtracted it: "lint 72 โ†’ 0, -72". - checkRunState gains a "runner-error" state, detected from the reason; an explicit skipped/unavailable status still wins over it. It reads `status`, then `details.status` (what history snapshots keep), then the legacy details flags. - computeDelta / computeTrend report a transition with before/after (or prev/curr) null for the crashed side: "72 โ†’ failed (runner error)" and "failed (runner error) โ†’ 72". PR comment, markdown and the actions page render it through the shared formatter. - Trends page: loadHistory leaves the crashed scan out of checkScores, so the per-check chart has a gap there, and the first โ†’ latest table shows the transition. - Analyzer snapshots omit the placeholder score for a crashed runner too; status stays "failed". - `status` is read as unknown and any value other than skipped/unavailable counts as ran, so this compiles against schema 0.6.0's open status type as well as 0.5.0. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/analyzer-snapshot.test.ts | 10 ++++ src/analyzer-snapshot.ts | 11 +++-- src/delta.test.ts | 65 ++++++++++++++++++++++++- src/delta.ts | 36 +++++++++----- src/history.test.ts | 7 +-- src/history.ts | 4 +- src/pr-comment.test.ts | 26 ++++++++++ src/report/pages.test.ts | 92 ++++++++++++++++++++++++++++++++++- src/trend.test.ts | 27 ++++++++++ src/trend.ts | 6 ++- 10 files changed, 256 insertions(+), 28 deletions(-) diff --git a/src/analyzer-snapshot.test.ts b/src/analyzer-snapshot.test.ts index 41a542a..713c720 100644 --- a/src/analyzer-snapshot.test.ts +++ b/src/analyzer-snapshot.test.ts @@ -100,3 +100,13 @@ describe("buildAnalyzerSnapshots not-run checks (#107)", () => { 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"); + }); +}); diff --git a/src/analyzer-snapshot.ts b/src/analyzer-snapshot.ts index b9fbe3e..657a6ef 100644 --- a/src/analyzer-snapshot.ts +++ b/src/analyzer-snapshot.ts @@ -1,3 +1,4 @@ +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"]); @@ -5,9 +6,11 @@ const DETAIL_DENYLIST = new Set(["assessment", "containerSvg", "graph", "reason" export function buildAnalyzerSnapshots(checks: CheckResult[]): AnalyzerSnapshot[] { return checks.map((check) => { const status = checkStatus(check); - // A check that did not run carries a placeholder 100 (core.ts); it is - // not a measurement, so the snapshot has no score key at all (#107). - const score = !NOT_RUN.has(status) && Number.isFinite(Number(check.score)) ? Number(check.score) : undefined; + // 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, @@ -20,8 +23,6 @@ export function buildAnalyzerSnapshots(checks: CheckResult[]): AnalyzerSnapshot[ }); } -const NOT_RUN = new Set(["skipped", "unavailable"]); - function checkStatus(check: CheckResult): AnalyzerSnapshot["status"] { const details = check.details as Record; const status = details.status; diff --git a/src/delta.test.ts b/src/delta.test.ts index 0d3222b..31b4842 100644 --- a/src/delta.test.ts +++ b/src/delta.test.ts @@ -177,7 +177,7 @@ describe("not-run checks (#107)", () => { }; expect(checkRunState(legacyComingSoon)).toBe("unavailable"); expect(checkRunState(legacySkipped)).toBe("skipped"); - expect(checkRunState(legacyRunnerError)).toBe("ran"); + expect(checkRunState(legacyRunnerError)).toBe("runner-error"); expect(checkRunState(undefined)).toBe("absent"); const delta = computeDelta(makeReport({ checks: [legacySkipped] }), makeReport({ checks: [lintScored] })); @@ -223,3 +223,66 @@ describe("not-run checks (#107)", () => { 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"); + }); +}); diff --git a/src/delta.ts b/src/delta.ts index 9ec6e63..f549ab7 100644 --- a/src/delta.ts +++ b/src/delta.ts @@ -12,8 +12,12 @@ import type { CheckResult, Issue, VibeReport } from "./types.js"; * placeholder `score: 100` (core.ts), which must never be subtracted from a * real score: "lint: 100 โ†’ 72" would read as a regression, and the reverse * (a tool uninstalled) as an improvement (#107). + * + * `runner-error`: the check ran and its runner crashed. It counts as having + * run, with status `failed`, but its `score: 0` / grade F is a placeholder + * too, so it is compared as a state, never as a number. */ -export type CheckRunState = "ran" | "skipped" | "unavailable" | "absent"; +export type CheckRunState = "ran" | "runner-error" | "skipped" | "unavailable" | "absent"; /** One side of a check comparison. `score` is set only when the check ran. */ export interface CheckSide { @@ -29,18 +33,24 @@ export interface StatusTransition { /** * Run state of a check. Reads `status` (the CLI writes it; it survives schema - * 0.5.0 via `.passthrough()`); older reports without it fall back to the - * details flags, in the order core.ts uses (`unavailable`/`comingSoon` - * before `skipped`, a "runner error:" reason counting as a run). + * 0.5.0 via `.passthrough()`), else `details.status` (what history snapshots + * keep). Reports without either fall back to the details flags, in the order + * core.ts uses (`unavailable`/`comingSoon` before `skipped`). + * + * A "runner error:" reason is a crashed runner (core.ts reports it as + * `failed`); an explicit `skipped`/`unavailable` status still wins over it. + * `status` is an open vocabulary (schema 0.6.0): any other value counts as ran. */ export function checkRunState(check: CheckResult | undefined): CheckRunState { if (!check) return "absent"; - const status = (check as CheckResult & { status?: unknown }).status; - if (status === "skipped" || status === "unavailable") return status; - if (typeof status === "string" && status) return "ran"; const details = (check.details ?? {}) as Record; + const topStatus: unknown = (check as { status?: unknown }).status; + const status: unknown = typeof topStatus === "string" && topStatus ? topStatus : details.status; + if (status === "skipped") return "skipped"; + if (status === "unavailable") return "unavailable"; const reason = typeof details.reason === "string" ? details.reason : ""; - if (reason.startsWith("runner error:")) return "ran"; + if (reason.startsWith("runner error:")) return "runner-error"; + if (typeof status === "string" && status) return "ran"; if (details.unavailable || details.comingSoon) return "unavailable"; if (details.skipped) return "skipped"; return "ran"; @@ -61,8 +71,10 @@ export function compareCheckSides(before: CheckSide, after: CheckSide): { delta: return { delta: 0, transition: { before, after } }; } +/** "72", "failed (runner error)", "unavailable", "skipped" or "not present". */ export function formatCheckSide(side: CheckSide): string { if (side.state === "ran") return String(side.score); + if (side.state === "runner-error") return "failed (runner error)"; return side.state === "absent" ? "not present" : side.state; } @@ -78,11 +90,11 @@ export interface DeltaIssue { export interface CheckDelta { name: string; label: string; - /** Score before; null when the check did not run or was absent (see `transition`). */ + /** Score before; null when the check did not run, its runner crashed, or it was absent (see `transition`). */ before: number | null; - /** Score after; null when the check did not run or was absent (see `transition`). */ + /** Score after; null when the check did not run, its runner crashed, or it was absent (see `transition`). */ after: number | null; - /** Score change; 0 whenever either side did not run (see `transition`). */ + /** Score change; 0 whenever either side did not run or crashed (see `transition`). */ delta: number; /** Set instead of a numeric delta when the check's run state changed. */ transition?: StatusTransition; @@ -196,7 +208,7 @@ export function scoreChanges(delta: ScanDelta): CheckDelta[] { return delta.checks.filter((c) => c.delta !== 0).sort((a, b) => b.delta - a.delta); } -/** Checks whose run state changed (ran โ†” skipped/unavailable/absent). */ +/** Checks whose run state changed (ran โ†” runner-error/skipped/unavailable/absent). */ export function statusTransitions(delta: ScanDelta): (CheckDelta & { transition: StatusTransition })[] { return delta.checks.filter((c): c is CheckDelta & { transition: StatusTransition } => c.transition !== undefined); } diff --git a/src/history.test.ts b/src/history.test.ts index 039423b..fc1cdd8 100644 --- a/src/history.test.ts +++ b/src/history.test.ts @@ -194,15 +194,12 @@ describe("loadHistory", () => { }), ); const [entry] = loadHistory(tmp); - expect([...entry.checkScores]).toEqual([ - ["types", 0], - ["structure", 90], - ]); + expect([...entry.checkScores]).toEqual([["structure", 90]]); expect(Object.fromEntries(entry.checkStates)).toEqual({ lint: "unavailable", "ai-review": "unavailable", deps: "skipped", - types: "ran", + types: "runner-error", structure: "ran", }); }); diff --git a/src/history.ts b/src/history.ts index 2436608..876f84a 100644 --- a/src/history.ts +++ b/src/history.ts @@ -9,9 +9,9 @@ import type { AnalyzerSnapshot, VibeReport } from "./types.js"; export interface HistoryEntry { timestamp: string; score: number; - /** Scores of checks that ran. A not-run check's placeholder 100 is left out (#107). */ + /** Scores of checks that ran. A not-run check's placeholder 100, and a crashed runner's 0, are left out (#107). */ checkScores: Map; - /** Run state of every check in the snapshot (from `status`, else the details flags). */ + /** Run state of every check in the snapshot (from `status` / `details.status`, the "runner error:" reason, else the details flags). */ checkStates: Map>; issues: IssueSnapshot[]; analyzerSnapshots: AnalyzerSnapshot[]; diff --git a/src/pr-comment.test.ts b/src/pr-comment.test.ts index 98358f9..9cab56d 100644 --- a/src/pr-comment.test.ts +++ b/src/pr-comment.test.ts @@ -60,4 +60,30 @@ describe("PR comment body (#107)", () => { expect(body).not.toContain("+36"); expect(body).not.toContain("64 โ†’ 100"); }); + + const crashed = { + name: "lint", + status: "failed", + score: 0, + grade: "F" as const, + details: { skipped: true, status: "failed", reason: "runner error: boom" }, + issues: [], + duration: 0, + }; + + it("shows 72 โ†’ runner error as a status change, not a -72 regression", async () => { + const { buildCommentBody } = await import("./pr-comment.js"); + const body = buildCommentBody({ ...base, checks: [crashed] }, null, { ...base, checks: [scored] }); + expect(body).toContain("lint: 72 โ†’ failed (runner error)"); + expect(body).not.toContain("72 โ†’ 0"); + expect(body).not.toContain("-72"); + }); + + it("shows runner error โ†’ 72 as a status change, not a +72 improvement", async () => { + const { buildCommentBody } = await import("./pr-comment.js"); + const body = buildCommentBody({ ...base, checks: [scored] }, null, { ...base, checks: [crashed] }); + expect(body).toContain("lint: failed (runner error) โ†’ 72"); + expect(body).not.toContain("0 โ†’ 72"); + expect(body).not.toContain("+72"); + }); }); diff --git a/src/report/pages.test.ts b/src/report/pages.test.ts index 2b84767..07ec3a7 100644 --- a/src/report/pages.test.ts +++ b/src/report/pages.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -324,3 +324,93 @@ describe("trends page with not-run checks (#107)", () => { expect(html).not.toContain("-25"); }); }); + +describe("crashed runner on the actions and trends pages (#107)", () => { + // core.ts's stub for a runner that threw: status failed, a placeholder 0/F. + const crashed: CheckResult = { + name: "lint", + score: 0, + grade: "F", + details: { skipped: true, status: "failed", reason: "runner error: boom" }, + issues: [], + duration: 0, + }; + (crashed as CheckResult & { status: string }).status = "failed"; + const lint = (score: number): CheckResult => ({ + name: "lint", + score, + grade: "C", + details: { status: "failed" }, + issues: [], + duration: 1, + }); + const structure = (score: number): CheckResult => ({ name: "structure", score, grade: "B", details: {}, issues: [], duration: 1 }); + const trendRows = (html: string) => + [...html.matchAll(/
(.*?)<\/div>/g)].map((m) => + m[1] + .replace(/<[^>]+>/g, " ") + .replace(/\s+/g, " ") + .trim(), + ); + function writeHistory(dir: string, entries: { timestamp: string; checks: CheckResult[] }[]): string { + const historyDir = join(dir, "history"); + mkdirSync(historyDir, { recursive: true }); + for (const { timestamp, checks } of entries) { + const report = { ...makeReport(dir, checks), timestamp }; + writeFileSync(join(historyDir, `${timestamp}.json`), JSON.stringify(buildReportHistorySnapshot(report))); + } + return historyDir; + } + + it("actions page: 72 โ†’ runner error is a status change, not lint -72", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const html = generatePages(makeReport(dir, [crashed]), undefined, makeReport(dir, [lint(72)])).get("actions.html")!; + expect(html).toContain("lint: 72 โ†’ failed (runner error)"); + expect(html).not.toContain("lint -72"); + }); + + it("actions page: runner error โ†’ 72 is a status change, not lint +72", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const html = generatePages(makeReport(dir, [lint(72)]), undefined, makeReport(dir, [crashed])).get("actions.html")!; + expect(html).toContain("lint: failed (runner error) โ†’ 72"); + expect(html).not.toContain("lint +72"); + }); + + it("trends page: a crashed scan is a gap in the chart and a transition in the table", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const historyDir = writeHistory(dir, [ + { timestamp: "2026-10-01T00:00:00.000Z", checks: [lint(64), structure(80)] }, + { timestamp: "2026-10-02T00:00:00.000Z", checks: [lint(72), structure(84)] }, + { timestamp: "2026-10-03T00:00:00.000Z", checks: [crashed, structure(86)] }, + ]); + // The snapshot keeps the crash only in details (no top-level status). + const snapshot = JSON.parse(readFileSync(join(historyDir, "2026-10-03T00:00:00.000Z.json"), "utf-8")); + expect(snapshot.checks[0]).not.toHaveProperty("status"); + expect(snapshot.checks[0].details.reason).toBe("runner error: boom"); + + const html = generatePages(makeReport(dir, [crashed, structure(86)]), historyDir).get("trends.html")!; + const rows = trendRows(html); + expect(rows).toContain("lint 64 โ†’ failed (runner error) status"); + expect(rows).toContain("structure 80 โ†’ 86 +6"); + expect(html).not.toContain("-64"); + expect(html).not.toContain("-72"); + // The lint card plots only the two scans that produced a score, and labels the crash. + expect(html).toMatch(/lint<\/span>failed \(runner error\)<\/span>/); + const lintCard = html.slice(html.indexOf('lint')).split('
')[0]; + expect(lintCard).toContain("2026-10-01 โ€” 64"); + expect(lintCard).toContain("2026-10-02 โ€” 72"); + expect(lintCard).not.toContain("2026-10-03"); + }); + + it("trends page: runner error โ†’ 72 is a transition, not +72", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const historyDir = writeHistory(dir, [ + { timestamp: "2026-10-01T00:00:00.000Z", checks: [crashed] }, + { timestamp: "2026-10-02T00:00:00.000Z", checks: [lint(70)] }, + { timestamp: "2026-10-03T00:00:00.000Z", checks: [lint(72)] }, + ]); + const html = generatePages(makeReport(dir, [lint(72)]), historyDir).get("trends.html")!; + expect(trendRows(html)).toContain("lint failed (runner error) โ†’ 72 status"); + expect(html).not.toContain("+72"); + }); +}); diff --git a/src/trend.test.ts b/src/trend.test.ts index efe87aa..b971c35 100644 --- a/src/trend.test.ts +++ b/src/trend.test.ts @@ -148,6 +148,33 @@ describe("computeTrend not-run checks (#107)", () => { }); }); +describe("computeTrend crashed runner (#107)", () => { + const crash = { skipped: true, status: "failed", reason: "runner error: boom" }; + + it("72 โ†’ runner error is a transition, not -72", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-trend-")); + writeFileSync(join(dir, "report.json"), JSON.stringify(makeReport(70, [{ name: "lint", score: 72, issues: 0 }]))); + const curr = makeReport(60, [{ name: "lint", score: 0, issues: 0 }]); + curr.checks[0].details = crash; + + const lint = computeTrend(curr, dir)!.checkDeltas.find((d) => d.name === "lint")!; + expect(lint).toMatchObject({ prev: 72, curr: 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)"); + }); + + it("runner error โ†’ 72 is a transition, not +72", () => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-trend-")); + const prev = makeReport(60, [{ name: "lint", score: 0, issues: 0 }]); + prev.checks[0].details = crash; + writeFileSync(join(dir, "report.json"), JSON.stringify(prev)); + + const lint = computeTrend(makeReport(70, [{ name: "lint", score: 72, issues: 0 }]), dir)!.checkDeltas[0]; + expect(lint).toMatchObject({ prev: null, curr: 72, delta: 0 }); + expect(formatTransition(lint.transition!)).toBe("failed (runner error) โ†’ 72"); + }); +}); + describe("formatTrend", () => { it("formats improvement", () => { const out = formatTrend({ diff --git a/src/trend.ts b/src/trend.ts index fda2e4c..4e202a3 100644 --- a/src/trend.ts +++ b/src/trend.ts @@ -8,8 +8,10 @@ import type { VibeReport } from "./types.js"; export interface TrendDelta { scoreDelta: number; // positive = improved - /** `delta` is 0 and `transition` is set when either side did not run (#107). */ - /** `prev`/`curr` are null for a side that did not run or was absent. */ + /** + * `delta` is 0 and `transition` is set when either side did not run or its + * runner crashed (#107). `prev`/`curr` are null for such a side, or an absent one. + */ checkDeltas: { name: string; prev: number | null; curr: number | null; delta: number; transition?: StatusTransition }[]; newIssues: number; fixedIssues: number; From 390a5ad45e167589e23d887eb681b486e4f6e74b Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 20:11:01 +1100 Subject: [PATCH 6/8] fix(delta): do not count issues of a check that did not run as fixed or new (#107) A check's issues are now diffed only when it ran in both scans. When either side crashed (runner error), was skipped or unavailable, or is absent, the change is reported as the status transition alone, so a lint crash, an uninstalled tool or a dropped check no longer reads as "3 fixed" (or, in reverse, "3 new") in computeDelta, computeTrend, the PR comment, the delta markdown or the actions page. This reverses the earlier choice in this PR that counted a dropped check's issues as fixed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/delta.test.ts | 73 +++++++++++++++++++++++++++++++++++++--- src/delta.ts | 36 ++++++++++++++------ src/pr-comment.test.ts | 46 +++++++++++++++++++++++++ src/report/pages.test.ts | 36 ++++++++++++++++++++ src/trend.test.ts | 37 +++++++++++++++++++- src/trend.ts | 12 ++++--- 6 files changed, 221 insertions(+), 19 deletions(-) diff --git a/src/delta.test.ts b/src/delta.test.ts index 31b4842..6955ed0 100644 --- a/src/delta.test.ts +++ b/src/delta.test.ts @@ -197,7 +197,7 @@ describe("not-run checks (#107)", () => { expect(formatCheckChangeBullets(delta, 8)).toContain("โœ… lint: 60 โ†’ 72 (+12)"); }); - it("reports a check that disappeared as a transition and counts its issues as fixed", () => { + it("reports a check that disappeared as a transition and does not count its issues as fixed", () => { const lintWithIssues = { ...lintScored, issues: [ @@ -209,10 +209,10 @@ describe("not-run checks (#107)", () => { 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(2); - expect(delta.fixed.map((f) => f.check)).toEqual(["lint", "lint"]); + expect(lint.fixed).toHaveLength(0); + expect(delta.fixed).toHaveLength(0); expect(formatCheckChangeBullets(delta, 8)).toContain("lint: 72 โ†’ not present"); - expect(formatDeltaMarkdown(delta)).toContain("## Fixed (2)"); + expect(formatDeltaMarkdown(delta)).not.toContain("## Fixed"); }); it("carries null, not the placeholder 100, for a side that did not run", () => { @@ -286,3 +286,68 @@ describe("crashed runner (#107)", () => { 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); + }); +}); diff --git a/src/delta.ts b/src/delta.ts index f549ab7..981c83f 100644 --- a/src/delta.ts +++ b/src/delta.ts @@ -71,6 +71,16 @@ export function compareCheckSides(before: CheckSide, after: CheckSide): { delta: return { delta: 0, transition: { before, after } }; } +/** + * Whether a check's issues can be diffed between two scans: only when it ran + * on both sides. A crashed, skipped, unavailable or absent side has no issue + * list to compare, so diffing against it would report every issue on the + * other side as fixed (or new) when nothing changed in the code (#107). + */ +export function issuesComparable(before: CheckSide, after: CheckSide): boolean { + return before.state === "ran" && after.state === "ran"; +} + /** "72", "failed (runner error)", "unavailable", "skipped" or "not present". */ export function formatCheckSide(side: CheckSide): string { if (side.state === "ran") return String(side.score); @@ -98,7 +108,9 @@ export interface CheckDelta { delta: number; /** Set instead of a numeric delta when the check's run state changed. */ transition?: StatusTransition; + /** Issues no longer reported. Always empty when either side did not run, crashed or is absent. */ fixed: DeltaIssue[]; + /** Issues newly reported. Always empty when either side did not run, crashed or is absent. */ introduced: DeltaIssue[]; } @@ -158,8 +170,8 @@ export function computeDelta(before: VibeReport, after: VibeReport): ScanDelta { const allIntroduced: DeltaIssue[] = []; // Union of check names: a check present only in `before` (tool removed, - // check dropped or renamed) still gets a "72 โ†’ not present" transition and - // its issues count as fixed. + // check dropped or renamed) still gets a "72 โ†’ not present" transition. + // Its issues are not counted as fixed: the check stopped looking. const names = [...after.checks.map((c) => c.name)]; for (const c of before.checks) if (!names.includes(c.name)) names.push(c.name); @@ -170,14 +182,18 @@ export function computeDelta(before: VibeReport, after: VibeReport): ScanDelta { const afterSide = checkSide(afterCheck); const { delta: scoreChange, transition } = compareCheckSides(beforeSide, afterSide); - // Build multiset of issue keys for before and after - const beforeKeys = issueMultiset(name, beforeCheck); - const afterKeys = issueMultiset(name, afterCheck); - - // Fixed: in before but not in after (or count decreased) - const fixed = multisetDifference(name, beforeKeys, afterKeys); - // Introduced: in after but not in before (or count increased) - const introduced = multisetDifference(name, afterKeys, beforeKeys); + // Issues are diffed only when the check ran on both sides; otherwise + // the change is the status transition, not N fixed / N new. + let fixed: DeltaIssue[] = []; + let introduced: DeltaIssue[] = []; + if (issuesComparable(beforeSide, afterSide)) { + const beforeKeys = issueMultiset(name, beforeCheck); + const afterKeys = issueMultiset(name, afterCheck); + // Fixed: in before but not in after (or count decreased) + fixed = multisetDifference(name, beforeKeys, afterKeys); + // Introduced: in after but not in before (or count increased) + introduced = multisetDifference(name, afterKeys, beforeKeys); + } allFixed.push(...fixed); allIntroduced.push(...introduced); diff --git a/src/pr-comment.test.ts b/src/pr-comment.test.ts index 9cab56d..937514e 100644 --- a/src/pr-comment.test.ts +++ b/src/pr-comment.test.ts @@ -87,3 +87,49 @@ describe("PR comment body (#107)", () => { expect(body).not.toContain("+72"); }); }); + +describe("PR comment does not count issues of a check that did not run on one side (#107)", () => { + const base = { + version: "0.56.0", + timestamp: "2026-10-05T00:00:00Z", + score: 80, + grade: "B" as const, + meta: { cwd: "/tmp", node: "v22", duration: 100, stack: {} as any, repoUrl: null, branch: "main" }, + }; + const issues = ["a", "b", "c"].map((f) => ({ severity: "warning" as const, message: `issue in ${f}`, file: `src/${f}.ts`, rule: "r" })); + const ran = { name: "lint", status: "failed", score: 72, grade: "C" as const, details: { status: "failed" }, issues, 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}`, prev: [ran], curr: other ? [other] : [] }, + { title: `${kind} โ†’ ran (3 issues)`, prev: other ? [other] : [], curr: [ran] }, + ]); + + it.each(cases)("$title: no fixed / new count in the headline", async ({ prev, curr }) => { + const { buildCommentBody } = await import("./pr-comment.js"); + const body = buildCommentBody({ ...base, checks: curr }, null, { ...base, checks: prev }); + const headline = body.split("\n").find((l) => l.includes("vs previous"))!; + expect(headline).toBeDefined(); + expect(headline).not.toMatch(/fixed|new/); + expect(body).toContain("**Status changes** (not scored):"); + }); +}); diff --git a/src/report/pages.test.ts b/src/report/pages.test.ts index 07ec3a7..f9a1955 100644 --- a/src/report/pages.test.ts +++ b/src/report/pages.test.ts @@ -414,3 +414,39 @@ describe("crashed runner on the actions and trends pages (#107)", () => { expect(html).not.toContain("+72"); }); }); + +describe("actions page does not count issues of a check that did not run on one side (#107)", () => { + const issues = ["a", "b", "c"].map((f) => ({ severity: "warning" as const, message: `issue in ${f}`, file: `src/${f}.ts`, rule: "r" })); + const ran: CheckResult = { name: "lint", score: 72, grade: "C", details: { status: "failed" }, issues, duration: 1 }; + const crashed: CheckResult = { + name: "lint", + score: 0, + grade: "F", + details: { skipped: true, status: "failed", reason: "runner error: boom" }, + issues: [], + duration: 0, + }; + const unavailable: CheckResult = { + name: "lint", + score: 100, + grade: "A", + details: { skipped: true, unavailable: true, status: "unavailable" }, + issues: [], + duration: 1, + }; + const cases = Object.entries({ crashed, unavailable, dropped: undefined }).flatMap(([kind, other]) => [ + { title: `ran (3 issues) โ†’ ${kind}`, prev: [ran], curr: other ? [other] : [] }, + { title: `${kind} โ†’ ran (3 issues)`, prev: other ? [other] : [], curr: [ran] }, + ]); + + it.each(cases)("$title: 0 fixed, 0 new", ({ prev, curr }) => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-report-")); + const html = generatePages(makeReport(dir, curr), undefined, makeReport(dir, prev)).get("actions.html")!; + const banner = html.slice(html.indexOf('
')); + expect(banner).toContain('0 fixed'); + expect(banner).toContain('0 new'); + expect(banner).not.toContain("Fixed ("); + expect(banner).not.toContain("New ("); + expect(banner).toContain("Status changes:"); + }); +}); diff --git a/src/trend.test.ts b/src/trend.test.ts index b971c35..8f5c6b7 100644 --- a/src/trend.test.ts +++ b/src/trend.test.ts @@ -144,7 +144,7 @@ describe("computeTrend not-run checks (#107)", () => { expect(trend.checkDeltas).toHaveLength(1); expect(trend.checkDeltas[0]).toMatchObject({ name: "lint", prev: 72, curr: null, delta: 0 }); expect(formatTransition(trend.checkDeltas[0].transition!)).toBe("72 โ†’ not present"); - expect(trend.fixedIssues).toBe(2); + expect(trend.fixedIssues).toBe(0); }); }); @@ -175,6 +175,41 @@ describe("computeTrend crashed runner (#107)", () => { }); }); +describe("computeTrend does not diff issues of a check that did not run on one side (#107)", () => { + const crashed = { skipped: true, status: "failed", reason: "runner error: boom" }; + const unavailable = { skipped: true, unavailable: true, status: "unavailable" }; + const notRun = (details: Record | null): VibeReport => { + if (!details) return makeReport(70, [{ name: "structure", score: 80, issues: 0 }]); + const r = makeReport(70, [ + { name: "lint", score: 0, issues: 0 }, + { name: "structure", score: 80, issues: 0 }, + ]); + r.checks[0].details = details; + return r; + }; + const ran = () => + makeReport(70, [ + { name: "lint", score: 72, issues: 3 }, + { name: "structure", score: 80, issues: 0 }, + ]); + const cases = Object.entries({ crashed, unavailable, dropped: null }).flatMap(([kind, details]) => [ + { title: `ran (3 issues) โ†’ ${kind}`, prev: ran, curr: () => notRun(details) }, + { title: `${kind} โ†’ ran (3 issues)`, prev: () => notRun(details), curr: ran }, + ]); + + it.each(cases)("$title: 0 fixed, 0 new", ({ prev, curr }) => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-trend-")); + writeFileSync(join(dir, "report.json"), JSON.stringify(prev())); + const trend = computeTrend(curr(), dir)!; + expect(trend.fixedIssues).toBe(0); + expect(trend.newIssues).toBe(0); + expect(trend.fixed).toEqual([]); + expect(trend.introduced).toEqual([]); + expect(trend.checkDeltas.find((d) => d.name === "lint")!.transition).toBeDefined(); + expect(formatTrend(trend)).not.toMatch(/fixed|new/); + }); +}); + describe("formatTrend", () => { it("formats improvement", () => { const out = formatTrend({ diff --git a/src/trend.ts b/src/trend.ts index 4e202a3..e6cb7b5 100644 --- a/src/trend.ts +++ b/src/trend.ts @@ -2,7 +2,7 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; -import { checkSide, compareCheckSides, type StatusTransition } from "./delta.js"; +import { checkSide, compareCheckSides, issuesComparable, type StatusTransition } from "./delta.js"; import { type IssueSnapshot, issueSnapshot, readIssueFingerprint } from "./issue-fingerprint.js"; import type { VibeReport } from "./types.js"; @@ -37,11 +37,14 @@ export function computeTrend(report: VibeReport, outputDir: string): TrendDelta const checkDeltas: TrendDelta["checkDeltas"] = []; // Union of names, so a check that disappeared reads "72 โ†’ not present". + // Issues are compared only for checks that ran in both scans (#107). + const comparable = new Set(); const names = report.checks.map((c) => c.name); for (const c of prev.checks) if (!names.includes(c.name)) names.push(c.name); for (const name of names) { const prevSide = checkSide(prev.checks.find((c) => c.name === name)); const currSide = checkSide(report.checks.find((c) => c.name === name)); + if (issuesComparable(prevSide, currSide)) comparable.add(name); const { delta, transition } = compareCheckSides(prevSide, currSide); checkDeltas.push({ name, @@ -52,8 +55,8 @@ export function computeTrend(report: VibeReport, outputDir: string): TrendDelta }); } - const prevIssueMap = issueMap(prev); - const currIssueMap = issueMap(report); + const prevIssueMap = issueMap(prev, comparable); + const currIssueMap = issueMap(report, comparable); const introduced = [...currIssueMap.entries()].filter(([fp]) => !prevIssueMap.has(fp)).map(([, issue]) => issue); const fixed = [...prevIssueMap.entries()].filter(([fp]) => !currIssueMap.has(fp)).map(([, issue]) => issue); const newIssues = introduced.length; @@ -62,9 +65,10 @@ export function computeTrend(report: VibeReport, outputDir: string): TrendDelta return { scoreDelta, checkDeltas, newIssues, fixedIssues, introduced, fixed, prevTimestamp: prev.timestamp }; } -function issueMap(report: VibeReport): Map { +function issueMap(report: VibeReport, comparable: Set): Map { const out = new Map(); for (const check of report.checks) { + if (!comparable.has(check.name)) continue; for (const issue of check.issues) { const fp = readIssueFingerprint(check.name, issue); out.set(fp, issueSnapshot(check.name, issue)); From b474422b0f9b402b7e66e620a0348f7170b122e4 Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 20:12:50 +1100 Subject: [PATCH 7/8] fix(report): show the full runner-error reason on a single check's Trends card (#107) Transition and delta lines keep the short "failed (runner error)" token, which compares states and keeps local paths out of PR comments. Where one check is shown on its own, the Trends-page card, the crash reason is the useful detail, so it now reads e.g. "failed (runner error: boom)", truncated to 80 chars. History entries keep each crashed check's reason for this. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/delta.test.ts | 17 ++++++++++++++++- src/delta.ts | 12 ++++++++++++ src/history.ts | 21 ++++++++++++++++----- src/report/pages.test.ts | 7 +++++-- src/report/pages.ts | 4 +++- 5 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/delta.test.ts b/src/delta.test.ts index 6955ed0..5d83bb4 100644 --- a/src/delta.test.ts +++ b/src/delta.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { checkRunState, computeDelta, formatCheckChangeBullets, formatDeltaMarkdown, formatTransition } from "./delta.js"; +import { checkRunState, computeDelta, formatCheckChangeBullets, formatCheckState, formatDeltaMarkdown, formatTransition } from "./delta.js"; import type { VibeReport } from "./types.js"; function makeReport(overrides: Partial = {}): VibeReport { @@ -351,3 +351,18 @@ describe("issues of a check that did not run on one side are not diffed (#107)", 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"); + }); +}); diff --git a/src/delta.ts b/src/delta.ts index 981c83f..43f888a 100644 --- a/src/delta.ts +++ b/src/delta.ts @@ -88,6 +88,18 @@ export function formatCheckSide(side: CheckSide): string { return side.state === "absent" ? "not present" : side.state; } +/** + * One check's state shown on its own (e.g. a Trends-page card), where the + * crash reason is the useful detail: "failed (runner error: eslint exited 2)". + * Transition and delta lines keep the short `formatCheckSide` token: the + * reason varies between runs and can carry local paths. + */ +export function formatCheckState(side: CheckSide, reason?: string): string { + if (side.state !== "runner-error" || !reason?.startsWith("runner error:")) return formatCheckSide(side); + const text = reason.length > 80 ? `${reason.slice(0, 79)}โ€ฆ` : reason; + return `failed (${text})`; +} + export interface DeltaIssue { check: string; severity: Issue["severity"]; diff --git a/src/history.ts b/src/history.ts index 876f84a..74a2583 100644 --- a/src/history.ts +++ b/src/history.ts @@ -13,6 +13,8 @@ export interface HistoryEntry { checkScores: Map; /** Run state of every check in the snapshot (from `status` / `details.status`, the "runner error:" reason, else the details flags). */ checkStates: Map>; + /** The "runner error: โ€ฆ" reason of each check whose runner crashed. */ + checkReasons: Map; issues: IssueSnapshot[]; analyzerSnapshots: AnalyzerSnapshot[]; } @@ -32,10 +34,19 @@ export function loadHistory(historyDir: string): HistoryEntry[] { if (raw.score === null || raw.score === undefined || !Number.isFinite(Number(raw.score)) || !raw.checks) continue; const checkScores = new Map(); const checkStates: HistoryEntry["checkStates"] = new Map(); + const checkReasons = new Map(); const issues: IssueSnapshot[] = []; const analyzerSnapshots = normalizeAnalyzerSnapshots(raw.meta?.analyzerSnapshots); - for (const c of raw.checks) readHistoryCheck(c, checkScores, checkStates, issues); - entries.push({ timestamp: raw.timestamp, score: Number(raw.score), checkScores, checkStates, issues, analyzerSnapshots }); + for (const c of raw.checks) readHistoryCheck(c, { checkScores, checkStates, checkReasons, issues }); + entries.push({ + timestamp: raw.timestamp, + score: Number(raw.score), + checkScores, + checkStates, + checkReasons, + issues, + analyzerSnapshots, + }); } catch { // skip corrupt files } @@ -46,12 +57,12 @@ export function loadHistory(historyDir: string): HistoryEntry[] { function readHistoryCheck( c: VibeReport["checks"][number], - checkScores: HistoryEntry["checkScores"], - checkStates: HistoryEntry["checkStates"], - issues: IssueSnapshot[], + { checkScores, checkStates, checkReasons, issues }: Pick, ): void { const state = checkRunState(c); if (state !== "absent") checkStates.set(c.name, state); + const reason = (c.details as Record | undefined)?.reason; + if (state === "runner-error" && typeof reason === "string") checkReasons.set(c.name, reason); if (state === "ran" && c.score !== null && c.score !== undefined && Number.isFinite(Number(c.score))) { checkScores.set(c.name, Number(c.score)); } diff --git a/src/report/pages.test.ts b/src/report/pages.test.ts index f9a1955..a637438 100644 --- a/src/report/pages.test.ts +++ b/src/report/pages.test.ts @@ -394,8 +394,11 @@ describe("crashed runner on the actions and trends pages (#107)", () => { expect(rows).toContain("structure 80 โ†’ 86 +6"); expect(html).not.toContain("-64"); expect(html).not.toContain("-72"); - // The lint card plots only the two scans that produced a score, and labels the crash. - expect(html).toMatch(/lint<\/span>failed \(runner error\)<\/span>/); + // The lint card shows one check on its own, so it carries the full crash reason; + // the first โ†’ latest row above keeps the short transition token. + expect(html).toMatch(/lint<\/span>failed \(runner error: boom\)<\/span>/); + expect(rows.join("\n")).not.toContain("boom"); + // The lint card plots only the two scans that produced a score. const lintCard = html.slice(html.indexOf('lint')).split('
')[0]; expect(lintCard).toContain("2026-10-01 โ€” 64"); expect(lintCard).toContain("2026-10-02 โ€” 72"); diff --git a/src/report/pages.ts b/src/report/pages.ts index 813f442..917c7cb 100644 --- a/src/report/pages.ts +++ b/src/report/pages.ts @@ -8,6 +8,7 @@ import { type CheckSide, compareCheckSides, formatCheckSide, + formatCheckState, formatTransition, type ScanDelta, scoreChanges, @@ -738,7 +739,8 @@ export function trendsPage(historyDir: string | undefined): string { const color = current >= 90 ? "var(--pass)" : current >= 75 ? "#84cc16" : current >= 60 ? "var(--warn)" : "var(--fail)"; headline = `${current}${deltaStr}`; } else { - headline = `${e(formatCheckSide(latestSide))}`; + // A single check's state on its own: show the crash reason in full. + headline = `${e(formatCheckState(latestSide, latest.checkReasons.get(name)))}`; } return `
From 9ffeb242607bf7fa4845531da809c7e2f74ea13f Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 20:14:15 +1100 Subject: [PATCH 8/8] docs(changelog): note not-run check deltas and the nullable CheckDelta scores (#107) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- CHANGELOG.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88a9ea7..9b72bfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,31 @@ # Changelog +## 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. + ## 0.56.0 (2026-08-18) ### A linter that cannot read your language no longer grades it