docs(benchmarking): drop the 3% default band; a gain is outside the declared variation criterion

CHANGELOG.md

@@ -4,6 +4,17 @@ Notable changes to house-rules. Versions follow [semantic versioning](https://se
 
 ## Unreleased
 
+- `hr-benchmarking` no longer has a 3% default band for "no clear change". There is no minimum gain
+  size: a favorable difference counts as a gain when it is outside the variation criterion declared before
+  the first run (the comparison's own run-to-run spread) and the outputs are identical. When the
+  criterion cannot support a percentage, the result is kept separate and unsupported differences are
+  labeled inconclusive. A gap inside the criterion is still "no clear change", and memory and CPU
+  costs are reported beside the gain. The benefit chart script in `hr-writing-pr` now requires the
+  spec's `band` (the declared variation criterion as a fraction of the baseline, a finite
+  non-negative number) instead of defaulting to 0.03, so a spec saved without one fails with a
+  message. `hr-writing-pr`, the thread-visuals reference and the
+  `upstream-contributions` rule now point at the declared variation criterion instead of a
+  "reporting band".
 - `hr-benchmarking` now gives a default stopping rule for performance claims, in place of "no
   universal minimum". Run 3 alternating runs per arm, and add 2 more per arm only when the arms'
   ranges overlap or the gap is within the variation criterion, for a cap of 5. If 5 runs per arm

rules/modifiers/upstream-contributions.md

@@ -29,7 +29,7 @@ issues invite, such as performance fixes or a reported defect.
 5. Title a performance discussion or issue with its headline result and the
    workload it holds for, such as "saving a file takes 1.1 s with 500
    open tabs, 0.17 s with the fix". Use a figure only when it clears the
-   `hr-benchmarking` bar (outside the reporting band, variation criterion met,
+   `hr-benchmarking` bar (outside the declared variation criterion,
    repetitions stated) and keep the caveats in the body. Title a defect by what
    the person sees, not by the code involved, and add how often only when the
    evidence supports a rate. Open the body with the same headline. A qualified

rules/references/thread-visuals.md

@@ -3,8 +3,8 @@
 Use an image, chart or diagram only to show a spatial interface state, compare
 before and after, show a trend across multiple measurements, or explain a flow
 with branches. Otherwise use text, a table or a short code block. Most comments
-are text. The exception is a qualified measured result. It lies outside the reporting band
-and meets its declared variation criterion. An
+are text. The exception is a qualified measured result. It lies outside its declared
+variation criterion and the outputs are identical. An
 issue, discussion or pull request that reports one opens with a headline chart
 of it, so a reader sees the benefit before reading the body. A result with
 one row that moved by less than 10% stays text and a table.

skills/hr-benchmarking/SKILL.md

@@ -149,7 +149,7 @@ measured workload; batch compatible probes when they dominate elapsed time.
 Report the selected repetition count or stopping rule, variation statistic and
 threshold, sample count, median and range. Always give the run count and the
 range with any central value, never a bare average. Apply the preset
-criterion and treat a gap within run-to-run variation as inconclusive. Profile
+criterion and report a gap within run-to-run variation as no clear change. Profile
 separately from the runs used to claim a speed difference.
 
 When using saved evidence for a performance claim, bind archived inputs, source,
@@ -203,16 +203,15 @@ percent change or a times improvement. Use the metric's direction and label
 worsening, unchanged or inconclusive results. With a zero or missing baseline,
 show an absolute change or state that the benefit cannot be calculated.
 
-Use 3% as the default practical no-clear-change band for reported differences.
-This is a reporting convention, not a statistical margin of error. Use the
-variation statistic and threshold declared before collecting results. When that
-threshold, expressed as a percentage of the baseline, exceeds 3%, widen the
-reporting band to at least that percentage and name the statistic and threshold.
-If the variation criterion cannot support a percentage band, keep its result
-separate and label unsupported differences inconclusive. A change outside the
-band still needs qualified evidence and must meet the declared variation criterion. Set a different
-precision target before collecting results when the workload requires it;
-changing presentation does not change frozen acceptance or rejection gates.
+There is no minimum gain size. A favorable difference counts as a gain when it
+lies outside the variation criterion declared before the first run (the comparison's
+own run-to-run spread, from its A/A arm or already-known variation, as a
+percentage of the baseline) and the outputs are identical. Report a gap inside
+that criterion as no clear change, and name the statistic and threshold beside
+it. An unfavorable difference outside it is a regression; label it worsening.
+Report memory and CPU costs beside the gain. If the variation criterion cannot support a percentage, keep its result
+separate and label unsupported differences inconclusive. Changing presentation
+does not change frozen acceptance or rejection gates.
 
 Show a comparison of three or more variants or conditions as a chart beside the
 before/after table, unless every row is no clear change. Choose the chart for the
@@ -227,7 +226,7 @@ question:
 | Change with load or time         | Line chart                                 |
 
 Put units and run counts on the axes, start bars at zero, mark a gap inside the
-reporting band as no clear change, and give the chart an explicit background colour so
+variation criterion as no clear change, and give the chart an explicit background colour so
 it reads the same on a light or a dark page.
 In a pull request, the headline chart and when it may be skipped come from
 `hr-writing-pr`; this table applies to any other chart.

skills/hr-writing-pr/SKILL.md

@@ -91,13 +91,16 @@ outcomes visible; collapse long commands or supporting detail when useful.
 Required scope or approval sections stay, with only the justification and links
 the reviewer needs.
 
-A qualified measured change outside the declared no-clear-change band gets a
+A qualified measured change outside the declared variation criterion, with
+identical outputs, gets a
 headline chart directly under the opening sentence, beside the exact result
 table. Render it with `scripts/benefit-chart.py` in this skill's directory. It reads
 a JSON spec and writes a dark PNG with labeled units, zero-based bars and a
 benefit badge per row. Skip the chart only when every row is no clear change,
-or the one row moved by less than 10%. Set the spec's top-level `band` to the declared variation
-threshold when it exceeds 0.03, and name that threshold beside the chart.
+or the one row moved by less than 10%. Set the spec's top-level `band`
+(required) to the declared variation criterion as a fraction of the baseline,
+and name it beside the chart. If the criterion cannot be a fraction of the
+baseline, skip the chart and label the result inconclusive.
 
 Show the workload and material limits beside it; a synthetic benchmark is not a
 production or UI speedup. Preview a chart in a different style with the user

skills/hr-writing-pr/scripts/benefit-chart.py

@@ -21,14 +21,16 @@
 Row fields: label, before, after, unit (text after the value), lower_is_better
 (default true), words (the two words after the percentage, for a good and a bad
 change; default "lower"/"higher", or "higher"/"lower" when lower_is_better is false), and
-note (extra text under the label). A change inside band (default 0.03) is
-labeled "no clear change". Bars start at zero and share a scale per row.
+note (extra text under the label). band is required: the variation criterion
+declared before the first run, as a fraction of the baseline. A change at or
+inside it is labeled "no clear change". Bars start at zero and share a scale per row.
 
 Values must be non-negative. Prints the alt text for the image on stdout. With --table it
 also prints a Markdown table of the same values, the text version of the chart
 for readers and agents that cannot parse the image. Requires matplotlib.
 """
 import json
+import math
 import sys
 import textwrap
 
@@ -63,13 +65,28 @@ def fmt(value):
     return f"{value:,.3f}"
 
 
+def band_of(spec):
+    if "band" not in spec:
+        sys.exit('spec needs a top-level "band": the declared variation criterion as a fraction '
+                 "of the baseline, such as 0.04.")
+    try:
+        if isinstance(spec["band"], bool):
+            raise TypeError
+        band = float(spec["band"])
+    except (TypeError, ValueError):
+        band = float("nan")
+    if not math.isfinite(band) or band < 0:
+        sys.exit('spec "band" must be a finite non-negative number.')
+    return band
+
+
 def judge(row, band):
     before, after = float(row["before"]), float(row["after"])
     lower_is_better = row.get("lower_is_better", True)
     if before == 0:
         return NEUTRAL, "no baseline to compare", "no baseline to compare"
     change = (after - before) / before
-    if abs(change) < band:
+    if abs(change) <= band:
         return NEUTRAL, "no clear change", "no clear change"
     improved = change < 0 if lower_is_better else change > 0
     default_words = ("lower", "higher") if lower_is_better else ("higher", "lower")
@@ -91,7 +108,7 @@ def render(spec, out_path):
     for row in rows:
         if float(row["before"]) < 0 or float(row["after"]) < 0:
             sys.exit(f"row {row['label']!r}: negative values are not supported (bars start at zero).")
-    band = float(spec.get("band", 0.03))
+    band = band_of(spec)
     arms = spec.get("arms", ["Before", "After"])
     count = len(rows)
     headline_lines = textwrap.wrap(spec["headline"], 54)
@@ -150,7 +167,7 @@ def render(spec, out_path):
 
 
 def table(spec):
-    band = float(spec.get("band", 0.03))
+    band = band_of(spec)
     arms = spec.get("arms", ["Before", "After"])
     lines = [f"| Metric | {arms[0]} | {arms[1]} | Change |", "| --- | ---: | ---: | --- |"]
     for row in spec["rows"]:

skills/hr-writing-pr/test/benefit-chart.test.ts

@@ -26,6 +26,7 @@ describe.skipIf(!hasMatplotlib)("benefit-chart.py", () => {
   test("labels an improvement, a regression and a change inside the band", () => {
     const { result, alt, out } = chart({
       headline: "Opening a thread takes 371 ms, was 927 ms",
+      band: 0.03,
       rows: [
         { label: "Open", before: 927, after: 371, unit: "ms", words: ["faster", "slower"] },
         { label: "Memory", before: 100, after: 130, unit: "MiB" },
@@ -45,6 +46,7 @@ describe.skipIf(!hasMatplotlib)("benefit-chart.py", () => {
     const { result } = chart(
       {
         headline: "Build time",
+        band: 0.03,
         arms: ["Main", "Branch"],
         rows: [
           { label: "Build", before: 67.741, after: 46.181, unit: "s", words: ["faster", "slower"] },
@@ -60,6 +62,7 @@ describe.skipIf(!hasMatplotlib)("benefit-chart.py", () => {
   test("never rounds a near-total reduction up to 100%", () => {
     const { result, alt } = chart({
       headline: "Node read takes 0.08 ms, was 119 ms",
+      band: 0.03,
       rows: [{ label: "Read", before: 119.1, after: 0.083, unit: "ms" }],
     });
     expect(result.exitCode).toBe(0);
@@ -70,6 +73,7 @@ describe.skipIf(!hasMatplotlib)("benefit-chart.py", () => {
     const { alt } = chart(
       {
         headline: "Renders per switch: 2, was 9",
+        band: 0.03,
         note: "Lower is better.",
         rows: [{ label: "Renders", before: 9, after: 2, unit: "", note: "Dev build only." }],
       },
@@ -84,6 +88,7 @@ describe.skipIf(!hasMatplotlib)("benefit-chart.py", () => {
     const { alt } = chart(
       {
         headline: "Read | write takes 5 ms, was 9 ms",
+        band: 0.03,
         workload: "3 runs, dev build",
         rows: [{ label: "read | write", before: 9, after: 5, unit: "ms" }],
       },
@@ -94,9 +99,40 @@ describe.skipIf(!hasMatplotlib)("benefit-chart.py", () => {
     expect(alt.split("\n")[0]).toContain("Workload: 3 runs, dev build");
   });
 
+  test("requires the declared variation criterion as band", () => {
+    const { result } = chart({
+      headline: "Idle",
+      rows: [{ label: "Idle", before: 100, after: 101, unit: "ms" }],
+    });
+    expect(result.exitCode).not.toBe(0);
+    expect(result.stderr.toString()).toContain('spec needs a top-level "band"');
+  });
+
+  test("treats a change exactly at the band as no clear change", () => {
+    const { alt } = chart({
+      headline: "Idle",
+      band: 0.25,
+      rows: [{ label: "Idle", before: 100, after: 125, unit: "ms" }],
+    });
+    expect(alt).toContain("Idle: 100 ms to 125 ms, no clear change");
+  });
+
+  test("rejects a band that is negative, not a number or not finite", () => {
+    for (const band of [-0.01, "abc", null, true, "1e999"]) {
+      const { result } = chart({
+        headline: "Idle",
+        band,
+        rows: [{ label: "Idle", before: 100, after: 101, unit: "ms" }],
+      });
+      expect(result.exitCode).not.toBe(0);
+      expect(result.stderr.toString()).toContain('"band" must be a finite non-negative number');
+    }
+  });
+
   test("rejects negative values instead of drawing them", () => {
     const { result } = chart({
       headline: "Delta",
+      band: 0.03,
       rows: [{ label: "Gain", before: -100, after: -50 }],
     });
     expect(result.exitCode).not.toBe(0);
@@ -106,6 +142,7 @@ describe.skipIf(!hasMatplotlib)("benefit-chart.py", () => {
   test("reports a missing baseline instead of dividing by zero", () => {
     const { result, alt } = chart({
       headline: "Allocations",
+      band: 0.03,
       rows: [{ label: "Calls", before: 0, after: 5, unit: "" }],
     });
     expect(result.exitCode).toBe(0);