Say when a run could not have found anything

.github/workflows/cache-value.yml

@@ -11,7 +11,11 @@ on:
           - "2"
           - "6"
           - "10"
-        default: "6"
+          - "14"
+        # The sign-flip test needs at least five usable pairs to reach 0.05 at
+        # all, and this workflow discards stalled runners, so a nominal six can
+        # land on four and report nothing about a 4.5x effect. Ten leaves room.
+        default: "10"
       setup-java-repository:
         description: Repository containing the setup-java action
         required: true

.github/workflows/transfer-overlap.yml

@@ -21,7 +21,10 @@ on:
       baseline-ref:
         description: Git ref for the baseline arm
         required: true
-        default: v4.8.0
+        # The concurrent-restore change this scenario exists to detect landed
+        # between v5.6.0 and main. Defaulting to v4.8.0 straddled several other
+        # changes at once and measured their sum, which read as no effect.
+        default: v5.6.0
         type: string
       candidate-ref:
         description: Git ref for the candidate arm

README.md

@@ -25,6 +25,8 @@ Every job also runs one unmeasured warm-up slot first. The first setup in a job
 
 **Stalled runners are discarded.** A slot can stall on the cache service for several seconds. Which arm the stall lands on is arbitrary, so that runner contributes an arbitrarily large difference and, with ten runners, one such slot moves the mean by more than any effect being measured. Runners whose _own arm disagrees with itself_ by more than a robust threshold are dropped. That decision is made purely on within-arm spread, which has the same distribution whether or not the arms differ, so unlike filtering on the arm difference it cannot bias the result. Every report lists what it discarded and why.
 
+**A design that cannot reach significance says so.** The sign-flip test has only 2^n distinct assignments for n paired runners, so its smallest attainable p-value is 2^-n however large the effect is. At four usable runners that floor is 0.0625, above the 0.05 a verdict requires — the run cannot report a finding even for an effect it measured perfectly. Those runs are reported as `underpowered` rather than `inconclusive`, because the two call for opposite readings: `inconclusive` means the data did not show an effect, `underpowered` means the design could not have shown one. The first live run of **Cache value** measured caching as 4.5x faster, 22.4 s, and reported `inconclusive` on four surviving runners; that is what this exists to stop.
+
 Every report also publishes two guard rails:
 
 - A **noise floor**, the median spread between the two slots of the same arm on one runner. An effect smaller than this is reported as `within-noise` even when its interval excludes zero.
@@ -169,7 +171,9 @@ Network throughput, hosted-runner image changes, upstream artifact availability
 ## Local checks
 
 ```bash
-npm test
+npm test          # unit tests, plus a syntax check of the JS embedded in heredocs
 bash -n scripts/*.sh
 shellcheck scripts/*.sh
 ```
+
+`npm test` includes `scripts/check-embedded-node.mjs` because several helpers run node inline through a heredoc, and that code is a string to everything else here: prettier does not format it, `node --test` never imports it, and shellcheck sees an opaque block. A syntax error in one of them otherwise stays invisible until a runner reaches it, which costs a whole benchmark run to find out.

package.json

@@ -3,6 +3,6 @@
   "private": true,
   "type": "module",
   "scripts": {
-    "test": "node --test"
+    "test": "node --test scripts/*.test.mjs && node scripts/check-embedded-node.mjs"
   }
 }

scripts/cache-save.sh

@@ -219,7 +219,7 @@ import { createRequire } from "node:module";
 // by package name fails outright. Resolving its manifest and requiring the file
 // its `main` points at goes around the map, and keeps working whichever version
 // a given setup-java ref happens to pin.
-function loadCacheClient(require, manifest) {
+function loadCacheClient(require, callerManifest) {
   const { readFileSync } = require("node:fs");
   const { dirname, join } = require("node:path");
   let manifestPath;
@@ -229,15 +229,15 @@ function loadCacheClient(require, manifest) {
     // Some versions do not expose "./package.json" through the map either, in
     // which case the install layout is the only thing left to go on.
     manifestPath = join(
-      dirname(manifest),
+      dirname(callerManifest),
       "node_modules",
       "@actions",
       "cache",
       "package.json",
     );
   }
-  const manifest = JSON.parse(readFileSync(manifestPath, "utf8"));
-  return require(join(dirname(manifestPath), manifest.main ?? "lib/cache.js"));
+  const packageJson = JSON.parse(readFileSync(manifestPath, "utf8"));
+  return require(join(dirname(manifestPath), packageJson.main ?? "lib/cache.js"));
 }
 
 const [manifest] = process.argv.slice(2);
@@ -265,7 +265,7 @@ import { pathToFileURL } from "node:url";
 // by package name fails outright. Resolving its manifest and requiring the file
 // its `main` points at goes around the map, and keeps working whichever version
 // a given setup-java ref happens to pin.
-function loadCacheClient(require, manifest) {
+function loadCacheClient(require, callerManifest) {
   const { readFileSync } = require("node:fs");
   const { dirname, join } = require("node:path");
   let manifestPath;
@@ -275,15 +275,15 @@ function loadCacheClient(require, manifest) {
     // Some versions do not expose "./package.json" through the map either, in
     // which case the install layout is the only thing left to go on.
     manifestPath = join(
-      dirname(manifest),
+      dirname(callerManifest),
       "node_modules",
       "@actions",
       "cache",
       "package.json",
     );
   }
-  const manifest = JSON.parse(readFileSync(manifestPath, "utf8"));
-  return require(join(dirname(manifestPath), manifest.main ?? "lib/cache.js"));
+  const packageJson = JSON.parse(readFileSync(manifestPath, "utf8"));
+  return require(join(dirname(manifestPath), packageJson.main ?? "lib/cache.js"));
 }
 
 const [manifest, fixtureDir, key, resultsFile, sample, arm, slot] =

scripts/check-embedded-node.mjs

@@ -0,0 +1,77 @@
+// Syntax-check the JavaScript embedded in shell heredocs.
+//
+// Several benchmark helpers run node inline, via `node --input-type=module -
+// <<'NODE'`. That code is a string as far as every other check here is
+// concerned: prettier does not format it, `node --test` never imports it, and
+// shellcheck sees an opaque heredoc. A syntax error in one of those blocks is
+// therefore invisible until a runner reaches it, which costs a whole benchmark
+// run to discover — that is exactly how `Identifier 'manifest' has already been
+// declared` reached a live job.
+
+import { readFile, writeFile, unlink } from "node:fs/promises";
+import { execFile } from "node:child_process";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { promisify } from "node:util";
+import { glob } from "node:fs/promises";
+import { pathToFileURL } from "node:url";
+
+const run = promisify(execFile);
+const BLOCK = /<<'NODE'\n([\s\S]*?)\nNODE\n/g;
+
+export function extractBlocks(source) {
+  return [...source.matchAll(BLOCK)].map((match, index) => ({
+    index,
+    code: match[1],
+    // Line of the heredoc opener, so a failure points somewhere useful.
+    line: source.slice(0, match.index).split("\n").length,
+  }));
+}
+
+export async function checkBlock(block, file) {
+  const scratch = join(
+    tmpdir(),
+    `embedded-${process.pid}-${file.replace(/\W/g, "_")}-${block.index}.mjs`,
+  );
+  await writeFile(scratch, block.code);
+  try {
+    await run("node", ["--check", scratch]);
+    return null;
+  } catch (error) {
+    return `${file}:${block.line}: ${String(error.stderr).trim().split("\n").pop()}`;
+  } finally {
+    await unlink(scratch).catch(() => {});
+  }
+}
+
+export async function main() {
+  const files = [];
+  for await (const file of glob("scripts/*.sh")) files.push(file);
+  files.sort();
+
+  const failures = [];
+  let checked = 0;
+  for (const file of files) {
+    for (const block of extractBlocks(await readFile(file, "utf8"))) {
+      checked += 1;
+      const failure = await checkBlock(block, file);
+      if (failure) failures.push(failure);
+    }
+  }
+
+  if (failures.length > 0) {
+    throw new Error(
+      `Embedded node scripts failed to parse:\n${failures.map((failure) => `  ${failure}`).join("\n")}`,
+    );
+  }
+  console.log(
+    `${checked} embedded node scripts parse in ${files.length} files`,
+  );
+}
+
+if (
+  process.argv[1] &&
+  import.meta.url === pathToFileURL(process.argv[1]).href
+) {
+  await main();
+}

scripts/paired.mjs

@@ -27,6 +27,7 @@ import {
   pairedInterval,
   pairedPermutationTest,
   quantile,
+  significanceReachable,
   standardDeviation,
 } from "./stats.mjs";
 
@@ -214,7 +215,12 @@ export function analyzePairs(
     interval,
     pValue,
     shiftSeconds: hodgesLehmann(candidateValues, baselineValues),
-    verdict: classify(interval, { noiseFloor: floor, pValue }),
+    verdict: classify(interval, {
+      noiseFloor: floor,
+      pValue,
+      pairCount: pairs.length,
+    }),
+    significanceReachable: significanceReachable(pairs.length),
     droppedRunners: dropped,
     stallThresholdSeconds: thresholdSeconds,
     control: {
@@ -292,6 +298,7 @@ export function analyzeAgainstReference(rows, arms, reference) {
       verdict: isReference
         ? "reference"
         : classify(interval, {
+            pairCount: runners.length,
             noiseFloor: floor,
             pValue: pairedPermutationTest(differences, { seed: 50 + index }),
           }),

scripts/report-action-overhead.mjs

@@ -282,7 +282,7 @@ export function markdown(
 
   if (overall) {
     lines.push(
-      `Pooled across ${overall.pairs.length} configurations: ${seconds(overall.interval.estimate)} ${formatInterval(overall.interval)}, permutation p = ${overall.pValue.toFixed(3)}, noise floor ${seconds(overall.noiseFloorSeconds)}.`,
+      `Pooled across ${overall.pairs.length} configurations: ${formatInterval(overall.interval)}, permutation p = ${overall.pValue.toFixed(3)}, noise floor ${seconds(overall.noiseFloorSeconds)}.`,
       "",
       `The A/A control — the two baseline slots against each other, which differ by nothing — reads ${overall.control.verdict}. Anything other than \`within-noise\` or \`inconclusive\` there means the harness is measuring something it should not, and the comparison above cannot be trusted.`,
       "",

scripts/stats.mjs

@@ -7,6 +7,7 @@
 
 const BOOTSTRAP_ITERATIONS = 10000;
 const DEFAULT_CONFIDENCE = 0.95;
+const SIGNIFICANCE_LEVEL = 0.05;
 
 // Deterministic PRNG so a given set of samples always produces the same
 // interval. Reports are compared across runs and must not wobble because the
@@ -168,6 +169,20 @@ export function pairedPermutationTest(differences, options = {}) {
   return (atLeastAsExtreme + 1) / (iterations + 1);
 }
 
+// The sign-flip test has only 2^n distinct assignments for n paired
+// observations, so its smallest attainable p-value is 2^-n no matter how large
+// the effect is. At four pairs that floor is 0.0625, above the 0.05 the verdict
+// requires — so a run with four usable runners cannot report a finding even for
+// an effect it measured perfectly. That is a property of the design, not of the
+// data, and it has to be said rather than dressed up as "collect more samples".
+export function significanceReachable(pairCount, alpha = SIGNIFICANCE_LEVEL) {
+  return pairCount > 0 && Math.pow(2, -pairCount) <= alpha;
+}
+
+export function pairsNeededForSignificance(alpha = SIGNIFICANCE_LEVEL) {
+  return Math.ceil(Math.log2(1 / alpha));
+}
+
 // Holm's step-down correction controls the family-wise error rate when one
 // report makes several comparisons against the same reference.
 export function holmAdjust(pValues) {
@@ -210,10 +225,17 @@ export function classify(interval, options = {}) {
     noiseFloor = 0,
     lowerIsBetter = true,
     pValue = null,
-    alpha = 0.05,
+    alpha = SIGNIFICANCE_LEVEL,
+    pairCount = null,
   } = options;
   if (!interval) return "unknown";
   const { low, high, estimate } = interval;
+  // Reported before anything else, because when the design cannot reach alpha
+  // the p-value carries no information and "collect more samples" is the only
+  // honest reading of any verdict built on it.
+  if (pairCount !== null && !significanceReachable(pairCount, alpha)) {
+    return "underpowered";
+  }
   if (low <= 0 && high >= 0) return "inconclusive";
   if (pValue !== null && pValue >= alpha) return "inconclusive";
   if (Math.abs(estimate) < noiseFloor) return "within-noise";
@@ -224,7 +246,7 @@ export function classify(interval, options = {}) {
 // The label follows the interval's own confidence level rather than assuming
 // 95%, so a caller that widens or narrows the interval cannot end up publishing
 // a number under the wrong label.
-export function formatInterval(interval, { digits = 1, unit = "s" } = {}) {
+export function formatInterval(interval, { digits = 3, unit = "s" } = {}) {
   if (!interval) return "n/a";
   const { estimate, low, high, confidence = DEFAULT_CONFIDENCE } = interval;
   const level = Number((confidence * 100).toFixed(2));
@@ -241,6 +263,8 @@ export function describeVerdict(verdict) {
       return "No usable signal — the effect is smaller than the harness noise floor.";
     case "inconclusive":
       return "Inconclusive — the interval includes zero or the permutation test does not agree; collect more samples.";
+    case "underpowered":
+      return `Underpowered — too few paired runners for the permutation test to reach significance at all; it needs at least ${pairsNeededForSignificance()}. Any effect shown below may be real, but this run cannot establish it.`;
     default:
       return "Unknown.";
   }

scripts/stats.test.mjs

@@ -5,6 +5,7 @@ import {
   bootstrapInterval,
   classify,
   createRandom,
+  describeVerdict,
   differenceInterval,
   formatInterval,
   holmAdjust,
@@ -14,7 +15,9 @@ import {
   medianAbsoluteDeviation,
   pairedInterval,
   pairedPermutationTest,
+  pairsNeededForSignificance,
   quantile,
+  significanceReachable,
   standardDeviation,
 } from "./stats.mjs";
 
@@ -148,11 +151,36 @@ test("formatInterval labels the interval's own confidence level", () => {
   // Intervals built before `confidence` was carried still read as 95%.
   assert.equal(
     formatInterval({ estimate: 0, low: -1, high: 1 }),
-    "0.0s (95% CI -1.0 to 1.0)",
+    "0.000s (95% CI -1.000 to 1.000)",
   );
 });
 
 test("Holm adjustment controls a family of p-values", () => {
   assert.deepEqual(holmAdjust([0.01, 0.04, 0.2]), [0.03, 0.08, 0.2]);
   assert.deepEqual(holmAdjust([null, 0.01, 0.2]), [null, 0.02, 0.2]);
 });
+
+// A four-runner design cannot clear 0.05 no matter how large the effect is,
+// because the sign-flip test only has sixteen assignments to draw on. Reporting
+// that as "inconclusive" invites someone to read the number anyway.
+test("calls a design underpowered when significance is unreachable", () => {
+  assert.equal(significanceReachable(4), false);
+  assert.equal(significanceReachable(5), true);
+  assert.equal(pairsNeededForSignificance(), 5);
+
+  const decisive = { estimate: -22, low: -28, high: -18 };
+  assert.equal(
+    classify(decisive, { pValue: 0.061, pairCount: 4 }),
+    "underpowered",
+  );
+  assert.equal(
+    classify(decisive, { pValue: 0.01, pairCount: 10 }),
+    "improvement",
+  );
+});
+
+test("explains what an underpowered run can and cannot show", () => {
+  const described = describeVerdict("underpowered");
+  assert.match(described, /at least 5/);
+  assert.match(described, /may be real/);
+});