Report runtime boundary increases a PR has already accepted as notices

.github/workflows/runtime-boundary.yml

@@ -3,7 +3,8 @@
 # (src, src-js, resources, package/src, tests). Two modes:
 #   - pull_request: compares the merge commit with its first parent (the base
 #     branch tip) and fails only for a Deno.* API that src/deno_ral replaces;
-#     other increases are warnings plus a maintainer block in the job summary.
+#     other increases are warnings plus a maintainer block in the job summary,
+#     or notices once the PR's own baseline accepts them.
 #   - push (main) and workflow_dispatch: fails when a count rises above
 #     tools/runtime-boundary-baseline.json.
 # See dev-docs/runtime-migration/02-runtime-boundary-check.md.

tests/unit/tools/check-runtime-boundary.test.ts

@@ -10,7 +10,8 @@
 
 import { unitTest } from "../../test.ts";
 import { assert, assertEquals, assertRejects, assertThrows } from "../../asserts.ts";
-import { join } from "../../../src/deno_ral/path.ts";
+import { join, resolve } from "../../../src/deno_ral/path.ts";
+import { quartoDevBinCmd } from "../../quarto-cmd.ts";
 import { ensureDirSync, safeRemoveSync } from "../../../src/deno_ral/fs.ts";
 import { execProcess } from "../../../src/core/process.ts";
 import { createTempContext } from "../../../src/core/temp.ts";
@@ -661,6 +662,76 @@ unitTest("check-runtime-boundary - baselineAt reads the baseline committed at th
   }
 });
 
+// deno-lint-ignore require-await
+unitTest("check-runtime-boundary - renderDelta accepts only counts the PR itself raised", async () => {
+  const report = {
+    deltas: { "tests": { denoCalls: 1, stdImportFiles: 1 } },
+    findings: [
+      { scope: "tests", file: "tests/unit/a.test.ts", kind: "deno" as const, name: "cwd", delta: 1 },
+      { scope: "tests", file: "tests/unit/c.test.ts", kind: "std" as const, name: "testing/asserts", delta: 1, suggestion: "tests/asserts.ts" },
+    ],
+  };
+  const current = { "tests": { denoCalls: 830, stdImportFiles: 178 } };
+  // main sits below its baseline for calls; the PR raises only std-import files.
+  const base = { "tests": { denoCalls: 832, stdImportFiles: 177 } };
+  const rendered = renderDelta(report, current, base, { "tests": { denoCalls: 832, stdImportFiles: 178 } });
+  assertEquals(rendered.accepted.map((a) => a.message), [
+    "tests: tests/unit/c.test.ts now imports the standard library directly (testing/asserts); accepted by this PR's tools/runtime-boundary-baseline.json.",
+  ]);
+  assertEquals(rendered.warnings.map((w) => w.file), ["tests/unit/a.test.ts"]);
+  assert(rendered.summary.includes("- tests std-import files: baseline 177 → 178"));
+  assert(!rendered.summary.includes(`- tests ${DENO}* calls`));
+  assert(rendered.summary.includes("### Maintainer action before merging"));
+});
+
+/** Runs the script in PR mode on `root`, as CI does, and returns its stdout. */
+async function runAgainst(root: string, rev: string): Promise<string> {
+  const result = await execProcess({
+    cmd: resolve(quartoDevBinCmd()),
+    args: [
+      "run", "--dev", resolve("..", "tools", "check-runtime-boundary.ts"),
+      "--root", root, "--against", rev,
+    ],
+    env: { GITHUB_ACTIONS: "true", GITHUB_STEP_SUMMARY: "" },
+    stdout: "piped",
+    stderr: "piped",
+  });
+  assert(result.success, `check-runtime-boundary failed: ${result.stdout}\n${result.stderr}`);
+  return result.stdout ?? "";
+}
+
+unitTest("check-runtime-boundary - PR mode: an increase the PR's baseline already accepts is a notice, not a warning", async () => {
+  const temp = createTempContext();
+  try {
+    const root = temp.createDir();
+    makeRepo(root);
+    writeFixture(root, "tools/runtime-boundary-baseline.json", JSON.stringify(kExpected));
+    await git(root, "init");
+    await git(root, "add", ".");
+    await git(root, "-c", "user.name=t", "-c", "user.email=t@example.com", "-c", "commit.gpgsign=false", "commit", "-m", "base");
+
+    writeFixture(root, "tests/j.test.ts", `${DENO}cwd();\n`);
+    const raised = { ...kExpected, "tests": { denoCalls: 2, stdImportFiles: 2 } };
+    writeFixture(root, "tools/runtime-boundary-baseline.json", JSON.stringify(raised));
+
+    const out = await runAgainst(root, "HEAD");
+    assert(!out.includes("::warning"), out);
+    assert(out.includes("::notice file=tests/j.test.ts,line=1::tests: +1 " + DENO + "cwd in tests/j.test.ts; accepted by this PR's tools/runtime-boundary-baseline.json"), out);
+    assert(out.includes("### Accepted in this PR"), out);
+    assert(out.includes(`- tests ${DENO}* calls: baseline 1 → 2`), out);
+    assert(!out.includes("### Maintainer action before merging"), out);
+
+    // A baseline raised for a smaller increase than the PR makes leaves the warnings.
+    writeFixture(root, "tests/k.test.ts", `${DENO}cwd();\n`);
+    const stale = await runAgainst(root, "HEAD");
+    assert(stale.includes("::warning file=tests/j.test.ts"), stale);
+    assert(stale.includes("### Maintainer action before merging"), stale);
+    assert(!stale.includes("### Accepted in this PR"), stale);
+  } finally {
+    temp.cleanup();
+  }
+});
+
 // deno-lint-ignore require-await
 unitTest("check-runtime-boundary - a covered API in browser-shared code is a warning, not an error", async () => {
   const report = diffChanges([

tools/check-runtime-boundary.ts

@@ -8,7 +8,8 @@
  * standard library directly instead of through src/deno_ral/. Two modes:
  * without flags it fails when any count exceeds the committed baseline (main);
  * with --against <rev> it compares the working tree with <rev> and fails only
- * for a Deno.* API that src/deno_ral already replaces (pull requests).
+ * for a Deno.* API that src/deno_ral already replaces (pull requests); other
+ * increases are warnings until the working tree's baseline accepts them.
  *
  * Usage (from the repository root):
  *   quarto run --dev tools/check-runtime-boundary.ts
@@ -501,6 +502,7 @@ export interface Annotation {
 export interface Rendered {
   errors: Annotation[];
   warnings: Annotation[];
+  accepted: Annotation[]; // increases the PR's own baseline already covers
   notices: string[];
   summary: string; // markdown for GITHUB_STEP_SUMMARY
   failed: boolean; // errors.length > 0
@@ -514,18 +516,30 @@ const kAcceptSentence = kAcceptClause[0].toUpperCase() + kAcceptClause.slice(1);
 
 const signed = (n: number) => n > 0 ? `+${n}` : `${n}`;
 
-/** `current` is measure() of the working tree (the merge result on CI), so `current - deltas` is the base. */
+/** `current` is measure() of the working tree (the merge result on CI), so `current - deltas` is the base.
+ * `baseline` is the base revision's baseline, `merged` the working tree's: a count the PR raised
+ * in `merged` and does not exceed is accepted, the state the push check on main will see. */
 export function renderDelta(
   report: DeltaReport,
   current: Baseline,
   baseline: Baseline,
+  merged: Baseline = baseline,
 ): Rendered {
   const errors: Annotation[] = [];
   const warnings: Annotation[] = [];
+  const accepted: Annotation[] = [];
   const notices: string[] = [];
   const changes: string[] = [];
   const warnedScopes = new Set<string>();
 
+  const isAccepted = (scope: string, field: keyof BoundaryCounts) => {
+    const raised = merged[scope]?.[field];
+    return raised !== undefined &&
+      raised > (baseline[scope]?.[field] ?? 0) &&
+      (current[scope]?.[field] ?? 0) <= raised;
+  };
+  const acceptedSuffix = `; accepted by this PR's ${kBaselineRelPath}.`;
+
   for (const finding of report.findings) {
     const annotation = (message: string): Annotation => {
       const result: Annotation = { message, file: finding.file };
@@ -542,6 +556,14 @@ export function renderDelta(
       );
       continue;
     }
+    if (isAccepted(finding.scope, finding.kind === "deno" ? "denoCalls" : "stdImportFiles")) {
+      accepted.push(annotation(
+        finding.kind === "deno"
+          ? `${finding.scope}: +${finding.delta} Deno.${finding.name} in ${finding.file}${acceptedSuffix}`
+          : `${finding.scope}: ${finding.file} now imports the standard library directly (${finding.name})${acceptedSuffix}`,
+      ));
+      continue;
+    }
     warnedScopes.add(finding.scope);
     if (finding.kind === "deno") {
       warnings.push(annotation(
@@ -591,6 +613,18 @@ export function renderDelta(
   if (errors.length > 0) {
     summary.push("### What to change", "", ...changes, "");
   }
+  const acceptedRows = Object.entries(report.deltas).flatMap(([scope, d]) =>
+    kFields
+      .filter((field) => d[field] > 0 && isAccepted(scope, field))
+      .map((field) =>
+        `- ${scope} ${kFieldLabels[field]}: baseline ${
+          baseline[scope]?.[field] ?? 0
+        } → ${merged[scope][field]}`
+      )
+  );
+  if (acceptedRows.length > 0) {
+    summary.push("### Accepted in this PR", "", ...acceptedRows, "");
+  }
   if (warnings.length > 0) {
     const expected = Object.entries(current)
       .filter(([scope]) => warnedScopes.has(scope))
@@ -610,6 +644,7 @@ export function renderDelta(
   return {
     errors,
     warnings,
+    accepted,
     notices,
     summary: summary.join("\n"),
     failed: errors.length > 0,
@@ -838,10 +873,13 @@ if (import.meta.main) {
     } catch (e) {
       usage(`cannot use ${kBaselineRelPath} at ${against}: ${(e as Error).message}`);
     }
-    const rendered = renderDelta(diffChanges(changes), current, baseBaseline);
+    const rendered = renderDelta(diffChanges(changes), current, baseBaseline, baseline);
     for (const message of rendered.notices) {
       notice(message);
     }
+    for (const { message, file, line } of rendered.accepted) {
+      notice(message, { file, line });
+    }
     for (const { message, file, line } of rendered.warnings) {
       warning(message, { file, line });
     }