feat(observability-map): add --routes flag and a PR-comment renderer for CI

1stvamp · 1stvamp · commit 82fac3eb9c75 · 2026-08-01T14:24:46.000+01:00
diff --git a/internal-packages/observability-map/README.md b/internal-packages/observability-map/README.md
@@ -20,6 +20,13 @@ suppresses. The single-route mode takes either the route path the report prints
 the file name (`api.v1.token.ts`). An exact match wins over the routes it is a prefix of, and an
 ambiguous prefix warns and names the alternatives rather than silently picking one.
 
+## CI
+
+A PR that touches `apps/webapp/app/routes` or this package gets a sticky comment scanning head
+against the PR's merge base, with the score, what changed, and the current fix list. It is
+report-only: nothing here fails the build or blocks a merge, and the gate stays deferred until a
+later phase decides to add one. See `.github/workflows/observability-map.yml`.
+
 ## What 17 means
 
 It is the mean score of the 412 entry points that had at least one applicable check, where an
diff --git a/internal-packages/observability-map/src/cli.ts b/internal-packages/observability-map/src/cli.ts
@@ -1,4 +1,4 @@
-import { existsSync, writeFileSync } from "node:fs";
+import { existsSync, statSync, writeFileSync } from "node:fs";
 import { dirname, resolve } from "node:path";
 import { fileURLToPath } from "node:url";
 import type { EntryPoint } from "./types.js";
@@ -63,7 +63,25 @@ export function main(argv: string[], io: Io = processIo): number {
   const target = args.find((a) => !a.startsWith("--"));
 
   const repoRoot = findRepoRoot(dirname(fileURLToPath(import.meta.url)));
-  const routesDir = resolve(repoRoot, DEFAULT_ROUTES);
+  const routesFlag = args.find((a) => a.startsWith("--routes="));
+
+  let routesDir: string;
+  if (routesFlag) {
+    routesDir = resolve(process.cwd(), routesFlag.slice("--routes=".length));
+    let isDir = false;
+    try {
+      isDir = statSync(routesDir).isDirectory();
+    } catch {
+      isDir = false;
+    }
+    if (!isDir) {
+      io.err(`--routes: not a readable directory: ${routesDir}\n`);
+      return 1;
+    }
+  } else {
+    routesDir = resolve(repoRoot, DEFAULT_ROUTES);
+  }
+
   const { entryPoints, parseFailures } = scanDirectory(routesDir);
 
   if (target) {
diff --git a/internal-packages/observability-map/src/report/prComment.ts b/internal-packages/observability-map/src/report/prComment.ts
@@ -0,0 +1,179 @@
+import type { MapReport, ScoredEntry } from "../score.js";
+import { auditLine, contextLine, contextOnly, scoredFailures } from "./terminal.js";
+
+/** First line of every comment this job posts, so the upsert step can find its own comment again. */
+export const MARKER = "<!-- observability-map-report -->";
+
+const MAX_CHANGED_ROWS = 15;
+
+const failingIds = (e: ScoredEntry) => e.checks.filter((c) => c.status === "fail").map((c) => c.id);
+
+function scoreLine(head: MapReport, base: MapReport | null): string {
+  const headline =
+    head.global === null
+      ? `not measured over ${head.measured} measured of ${head.entries.length} entry points`
+      : `**${head.global}/100** over ${head.measured} measured of ${head.entries.length} entry points`;
+
+  if (!base) return headline;
+  if (base.global === null || head.global === null) return `${headline} (base not measured)`;
+
+  const diff = head.global - base.global;
+  const comparison =
+    diff === 0
+      ? `(base ${base.global}, no change)`
+      : diff > 0
+        ? `(base ${base.global}, up ${diff})`
+        : `(base ${base.global}, down ${-diff})`;
+  return `${headline} ${comparison}`;
+}
+
+type ChangedRow = {
+  routePath: string;
+  sensitive: boolean;
+  baseScore: number | "new";
+  headScore: number;
+  nowFailing: string[];
+  /**
+   * How much the entry got worse, used to sort the table. A new entry has no base score to
+   * subtract from, so it is scored against a perfect 100: a new entry landing at 60 sorts the
+   * same as an existing one that dropped 40 points, which is the ordering "what needs fixing
+   * first" implies.
+   */
+  drop: number;
+};
+
+function changedRows(head: MapReport, base: MapReport): { rows: ChangedRow[]; removed: number } {
+  const baseByFile = new Map(base.entries.map((e) => [e.fileName, e]));
+  const headFiles = new Set(head.entries.map((e) => e.fileName));
+
+  const rows: ChangedRow[] = [];
+  for (const h of head.entries) {
+    const b = baseByFile.get(h.fileName);
+    if (!b) {
+      rows.push({
+        routePath: h.routePath,
+        sensitive: h.sensitive,
+        baseScore: "new",
+        headScore: h.score,
+        nowFailing: failingIds(h),
+        drop: 100 - h.score,
+      });
+      continue;
+    }
+    if (b.score === h.score) continue;
+    const baseFailing = new Set(failingIds(b));
+    rows.push({
+      routePath: h.routePath,
+      sensitive: h.sensitive,
+      baseScore: b.score,
+      headScore: h.score,
+      nowFailing: failingIds(h).filter((id) => !baseFailing.has(id)),
+      drop: b.score - h.score,
+    });
+  }
+
+  rows.sort(
+    (a, b) =>
+      Number(b.sensitive) - Number(a.sensitive) ||
+      b.drop - a.drop ||
+      a.routePath.localeCompare(b.routePath)
+  );
+
+  const removed = base.entries.filter((e) => !headFiles.has(e.fileName)).length;
+  return { rows, removed };
+}
+
+function whatChangedSection(head: MapReport, base: MapReport | null): string[] {
+  const lines = ["**What this PR changed**"];
+
+  if (!base) {
+    lines.push("Base comparison unavailable.");
+    return lines;
+  }
+
+  const { rows, removed } = changedRows(head, base);
+
+  if (rows.length === 0 && removed === 0) {
+    lines.push("No entry point this PR touches changed its score.");
+    return lines;
+  }
+
+  if (rows.length > 0) {
+    lines.push("");
+    lines.push("| route | base | head | now failing |");
+    lines.push("| --- | --- | --- | --- |");
+    for (const row of rows.slice(0, MAX_CHANGED_ROWS)) {
+      lines.push(
+        `| ${row.routePath} | ${row.baseScore} | ${row.headScore} | ${row.nowFailing.join(", ")} |`
+      );
+    }
+    if (rows.length > MAX_CHANGED_ROWS) {
+      lines.push("");
+      lines.push(`and ${rows.length - MAX_CHANGED_ROWS} more`);
+    }
+  }
+
+  if (removed > 0) {
+    lines.push("");
+    lines.push(`${removed} entries removed`);
+  }
+
+  return lines;
+}
+
+function fixFirstSection(head: MapReport): string[] {
+  const lines = ["FIX FIRST"];
+  const worst = head.entries
+    .filter((e) => scoredFailures(e).length > 0 && !contextOnly(e))
+    .sort(
+      (a, b) =>
+        Number(b.sensitive) - Number(a.sensitive) ||
+        a.score - b.score ||
+        a.fileName.localeCompare(b.fileName)
+    );
+
+  for (const e of worst.slice(0, 3)) {
+    const marks = e.sensitive ? " (sensitive)" : "";
+    lines.push(
+      `- ${e.routePath}${marks} - ${scoredFailures(e)
+        .map((c) => c.id)
+        .join(", ")}`
+    );
+  }
+  return lines;
+}
+
+/**
+ * Pure function, no I/O: `head` and `base` are already-built reports. Matches entries across the
+ * two by `fileName`, the same identifier `renderJson` carries.
+ */
+export function renderPrComment(head: MapReport, base: MapReport | null): string {
+  const lines = [MARKER, "", "## Observability map", "", scoreLine(head, base), ""];
+
+  lines.push(...whatChangedSection(head, base), "");
+  lines.push(...fixFirstSection(head), "");
+
+  const audit = auditLine(head);
+  if (audit) lines.push(audit);
+  const context = contextLine(head);
+  if (context) lines.push(context);
+  if (audit || context) lines.push("");
+
+  lines.push(
+    "Report only, nothing here gates the merge. The rules and their reasons: " +
+      "internal-packages/observability-map/README.md."
+  );
+
+  const headFailures = head.parseFailures.length;
+  const baseFailures = base?.parseFailures.length ?? 0;
+  if (headFailures > 0 || baseFailures > 0) {
+    const parts: string[] = [];
+    if (headFailures > 0) parts.push(`${headFailures} at head`);
+    if (baseFailures > 0) parts.push(`${baseFailures} at base`);
+    lines.push(
+      `Warning: parse failures (${parts.join(", ")}) are excluded from the score, shrinking the denominator.`
+    );
+  }
+
+  return lines.join("\n");
+}
diff --git a/internal-packages/observability-map/src/report/prCommentCli.ts b/internal-packages/observability-map/src/report/prCommentCli.ts
@@ -0,0 +1,39 @@
+import { readFileSync } from "node:fs";
+import { resolve } from "node:path";
+import { fileURLToPath } from "node:url";
+import type { MapReport } from "../score.js";
+import { renderPrComment } from "./prComment.js";
+
+/** Where output goes. Injectable so tests can read it without spawning a process. */
+export type Io = { out: (s: string) => void; err: (s: string) => void };
+
+const processIo: Io = {
+  out: (s) => process.stdout.write(s),
+  err: (s) => process.stderr.write(s),
+};
+
+/** `-` or a missing second arg means no base: the CI job falls back to this when the base scan
+ * itself failed, so the comment still renders rather than the job going red. */
+export function main(argv: string[], io: Io = processIo): number {
+  const args = argv.slice(2);
+  const headPath = args[0];
+  const basePath = args[1];
+
+  if (!headPath) {
+    io.err("usage: prCommentCli.ts <head.json> [base.json|-]\n");
+    return 1;
+  }
+
+  const head = JSON.parse(readFileSync(headPath, "utf8")) as MapReport;
+  const base: MapReport | null =
+    !basePath || basePath === "-" ? null : JSON.parse(readFileSync(basePath, "utf8"));
+
+  io.out(renderPrComment(head, base));
+  io.out("\n");
+  return 0;
+}
+
+// Only when run as a program. Importing the module, which the tests do, must not read a file.
+const invokedDirectly =
+  process.argv[1] !== undefined && resolve(process.argv[1]) === fileURLToPath(import.meta.url);
+if (invokedDirectly) process.exitCode = main(process.argv);
diff --git a/internal-packages/observability-map/src/report/terminal.ts b/internal-packages/observability-map/src/report/terminal.ts
@@ -17,7 +17,7 @@ const gauge = (score: number | null) => {
  * of the fixable, route-specific gaps the list exists to surface. That gap is reported once, as
  * `AUDIT`, below.
  */
-const scoredFailures = (e: ScoredEntry) =>
+export const scoredFailures = (e: ScoredEntry) =>
   e.checks.filter((c) => SCORED_CHECK_IDS.includes(c.id) && c.status === "fail");
 
 /**
@@ -27,11 +27,40 @@ const scoredFailures = (e: ScoredEntry) =>
  * An entry that fails something else as well stays in the list with all of its findings, so a
  * route like `/account/tokens` still shows the request-context gap alongside the rest.
  */
-const contextOnly = (e: ScoredEntry) => {
+export const contextOnly = (e: ScoredEntry) => {
   const failures = scoredFailures(e);
   return failures.length === 1 && failures[0]!.id === "request-context";
 };
 
+/** The AUDIT figure, shared with `prComment.ts` so both renderers say the same thing. Null when
+ * there is nothing to report, i.e. no sensitive mutation exists. */
+export function auditLine(report: MapReport): string | null {
+  const { sensitiveMutations, withAudit } = report.auditGap;
+  if (sensitiveMutations === 0) return null;
+  // The closing sentence is a claim about the codebase, so it is only made when the figure in
+  // front of it supports it. It was printed unconditionally, including next to a non-zero count.
+  const gap =
+    withAudit === 0
+      ? " No audit helper exists in the webapp."
+      : ` ${sensitiveMutations - withAudit} without one.`;
+  return `AUDIT   ${withAudit} of ${sensitiveMutations} sensitive mutations record an actor.${gap}`;
+}
+
+/** The CONTEXT figure, shared with `prComment.ts`. Null when nothing is applicable. */
+export function contextLine(report: MapReport): string | null {
+  const { applicable, naming } = report.contextGap;
+  if (applicable === 0) return null;
+  const collapsed = report.entries.filter(contextOnly);
+  const sensitive = collapsed.filter((e) => e.sensitive).length;
+  return (
+    `CONTEXT   ${naming} of ${applicable} entry points name a tenant on a failure path.` +
+    (collapsed.length > 0
+      ? ` ${collapsed.length} appear${collapsed.length === 1 ? "s" : ""} only here, ` +
+        `${sensitive} of them sensitive, in the JSON rather than the list below.`
+      : "")
+  );
+}
+
 export function renderTerminal(report: MapReport): string {
   const lines: string[] = [];
 
@@ -52,32 +81,16 @@ export function renderTerminal(report: MapReport): string {
     }/${report.sensitiveCohort.n} entry points`
   );
 
-  const { sensitiveMutations, withAudit } = report.auditGap;
-  if (sensitiveMutations > 0) {
+  const audit = auditLine(report);
+  if (audit) {
     lines.push("");
-    // The closing sentence is a claim about the codebase, so it is only made when the figure in
-    // front of it supports it. It was printed unconditionally, including next to a non-zero count.
-    const gap =
-      withAudit === 0
-        ? " No audit helper exists in the webapp."
-        : ` ${sensitiveMutations - withAudit} without one.`;
-    lines.push(
-      `AUDIT   ${withAudit} of ${sensitiveMutations} sensitive mutations record an actor.${gap}`
-    );
+    lines.push(audit);
   }
 
-  const { applicable, naming } = report.contextGap;
-  if (applicable > 0) {
-    const collapsed = report.entries.filter(contextOnly);
-    const sensitive = collapsed.filter((e) => e.sensitive).length;
+  const context = contextLine(report);
+  if (context) {
     lines.push("");
-    lines.push(
-      `CONTEXT   ${naming} of ${applicable} entry points name a tenant on a failure path.` +
-        (collapsed.length > 0
-          ? ` ${collapsed.length} appear${collapsed.length === 1 ? "s" : ""} only here, ` +
-            `${sensitive} of them sensitive, in the JSON rather than the list below.`
-          : "")
-    );
+    lines.push(context);
   }
 
   if (report.suppressions.checks > 0) {
diff --git a/internal-packages/observability-map/test/cli.test.ts b/internal-packages/observability-map/test/cli.test.ts
@@ -1,5 +1,6 @@
-import { existsSync, rmSync } from "node:fs";
-import { resolve } from "node:path";
+import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join, resolve } from "node:path";
 import { main, type Io } from "../src/cli.js";
 
 const REPORT_FILE = resolve(__dirname, "../../../observability-map.json");
@@ -69,3 +70,31 @@ describe("map", () => {
     expect(existsSync(REPORT_FILE)).toBe(false);
   });
 });
+
+describe("map --routes=<dir>", () => {
+  it("scans the directory it names instead of the repo's routes tree", () => {
+    const dir = mkdtempSync(join(tmpdir(), "obs-map-routes-"));
+    writeFileSync(
+      join(dir, "resources.only.ts"),
+      `export const loader = () => new Response("ok");`
+    );
+
+    const r = run("--routes=" + dir, "--json", "--no-write");
+
+    expect(r.code).toBe(0);
+    const parsed = JSON.parse(r.out);
+    expect(parsed.entries).toHaveLength(1);
+    expect(parsed.entries[0].fileName).toBe("resources.only.ts");
+
+    rmSync(dir, { recursive: true });
+  });
+
+  it("exits 1 with a message when the directory does not exist", () => {
+    const dir = join(tmpdir(), "obs-map-routes-does-not-exist");
+    const r = run("--routes=" + dir);
+
+    expect(r.code).toBe(1);
+    expect(r.err).toContain("not a readable directory");
+    expect(r.out).toBe("");
+  });
+});
diff --git a/internal-packages/observability-map/test/prComment.test.ts b/internal-packages/observability-map/test/prComment.test.ts
diff --git a/internal-packages/observability-map/test/prCommentCli.test.ts b/internal-packages/observability-map/test/prCommentCli.test.ts
