fix: clarify benchmark regression report verdicts (#2394)

## Summary

- Add an explicit base-vs-head regression verdict using comparable JMH
metadata, confidence-interval overlap, and a practical 5% threshold.
- Remove cross-method `Nx slower` rankings from the PR-head tables.
- List head-only benchmarks separately as descriptive results with no
regression verdict.

## Motivation

The [#2329 benchmark
report](#2329 (comment))
described `HistogramBenchmark.openTelemetryExponential` as “20x slower”
only because it was ranked against a different benchmark method in the
same run. Its actual base/head delta was -5.7% with overlapping
confidence intervals (within noise). This change makes that distinction
explicit and prevents the within-run ranking from being mistaken for a
regression.

## Tests

- `python3 .mise/tasks/test_generate-benchmark-summary.py`
- `python3 .mise/tasks/test_update-benchmarks.py`
- `ruff check` and `ruff format --check` on the changed Python files
- `mise run lint` (full lint is blocked only by the pre-existing README
Slack link returning HTTP 403; the pre-push scoped lint passed)

Related: [#2329](#2329).

---------

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>

Author: zeitlinger

.mise/tasks/generate_benchmark_summary.py

@@ -26,6 +26,8 @@
 from datetime import datetime, timezone
 from pathlib import Path
 
+PRACTICAL_CHANGE_THRESHOLD = 5.0
+
 
 def parse_args():
     parser = argparse.ArgumentParser(
@@ -246,7 +248,7 @@ def metric_score(result: dict) -> float | None:
     """Extract a benchmark score as a finite float."""
     try:
         score = float(result.get("primaryMetric", {}).get("score"))
-        if not math.isnan(score):
+        if math.isfinite(score):
             return score
     except (ValueError, TypeError):
         pass
@@ -261,7 +263,7 @@ def score_interval(result: dict) -> tuple[float, float] | None:
         try:
             low = float(confidence[0])
             high = float(confidence[1])
-            if not math.isnan(low) and not math.isnan(high):
+            if math.isfinite(low) and math.isfinite(high):
                 return min(low, high), max(low, high)
         except (ValueError, TypeError):
             pass
@@ -271,7 +273,7 @@ def score_interval(result: dict) -> tuple[float, float] | None:
         return None
     try:
         error = float(metric.get("scoreError"))
-        if not math.isnan(error):
+        if math.isfinite(error) and error >= 0:
             return score - error, score + error
     except (ValueError, TypeError):
         pass
@@ -285,41 +287,81 @@ def lower_is_better(result: dict) -> bool:
     return mode in {"avgt", "sample", "ss"} or unit.endswith("/op")
 
 
+def normalize_jvm_args(value) -> list:
+    """Normalize optional JMH JVM argument fields for metadata comparison."""
+    if value is None:
+        return []
+    if isinstance(value, list):
+        return value
+    return [value]
+
+
+def benchmark_metadata(result: dict) -> dict:
+    """Return metadata that must match for a base/head comparison."""
+    primary_metric = result.get("primaryMetric", {})
+    return {
+        "jmhVersion": result.get("jmhVersion"),
+        "mode": result.get("mode"),
+        "vmName": result.get("vmName"),
+        "vmVersion": result.get("vmVersion"),
+        "jvmArgs": normalize_jvm_args(result.get("jvmArgs")),
+        "jvmArgsPrepend": normalize_jvm_args(result.get("jvmArgsPrepend")),
+        "jvmArgsAppend": normalize_jvm_args(result.get("jvmArgsAppend")),
+        "threads": result.get("threads"),
+        "forks": result.get("forks"),
+        "warmupIterations": result.get("warmupIterations"),
+        "warmupTime": result.get("warmupTime"),
+        "warmupBatchSize": result.get("warmupBatchSize"),
+        "measurementIterations": result.get("measurementIterations"),
+        "measurementTime": result.get("measurementTime"),
+        "measurementBatchSize": result.get("measurementBatchSize"),
+        "jdkVersion": result.get("jdkVersion"),
+        "scoreUnit": primary_metric.get("scoreUnit"),
+        "params": result.get("params", {}),
+    }
+
+
+def comparable_metadata(head: dict, baseline: dict) -> bool:
+    """Return whether two results describe the same benchmark configuration."""
+    return benchmark_metadata(head) == benchmark_metadata(baseline)
+
+
 def comparison_status(head: dict, baseline: dict) -> str:
-    """Classify a benchmark comparison using confidence intervals."""
+    """Classify a benchmark comparison using confidence intervals and a threshold."""
     head_interval = score_interval(head)
     baseline_interval = score_interval(baseline)
     head_score = metric_score(head)
     baseline_score = metric_score(baseline)
-    if head_score is None or baseline_score is None:
-        return ""
-
-    is_lower_better = lower_is_better(head)
-    if head_interval and baseline_interval:
-        head_low, head_high = head_interval
-        baseline_low, baseline_high = baseline_interval
-        if is_lower_better:
-            if head_high < baseline_low:
-                return "faster"
-            if head_low > baseline_high:
-                return "slower"
-        else:
-            if head_low > baseline_high:
-                return "faster"
-            if head_high < baseline_low:
-                return "slower"
+    if (
+        not comparable_metadata(head, baseline)
+        or head_score is None
+        or baseline_score is None
+    ):
+        return "inconclusive"
+
+    change = performance_change(head, baseline)
+    if change is None or head_interval is None or baseline_interval is None:
+        return "inconclusive"
+
+    head_low, head_high = head_interval
+    baseline_low, baseline_high = baseline_interval
+    intervals_overlap = head_low <= baseline_high and baseline_low <= head_high
+    if intervals_overlap or abs(change) < PRACTICAL_CHANGE_THRESHOLD:
         return "within noise"
 
-    if is_lower_better:
-        return "faster" if head_score < baseline_score else "slower"
-    return "faster" if head_score > baseline_score else "slower"
+    return "meaningful improvement" if change > 0 else "meaningful regression"
 
 
 def performance_change(head: dict, baseline: dict) -> float | None:
     """Return percent performance change, with positive meaning faster."""
     head_score = metric_score(head)
     baseline_score = metric_score(baseline)
-    if head_score is None or baseline_score in (None, 0):
+    if (
+        head_score is None
+        or baseline_score is None
+        or head_score == 0
+        or baseline_score == 0
+    ):
         return None
     if lower_is_better(head):
         return (float(baseline_score) / head_score - 1) * 100
@@ -333,6 +375,27 @@ def format_change(change: float | None) -> str:
     return f"{change:+.1f}%"
 
 
+def metric_direction_note(results: list) -> str:
+    """Describe whether scores represent throughput or latency."""
+    directions = {
+        "latency" if lower_is_better(result) else "throughput" for result in results
+    }
+    if directions == {"throughput"}:
+        return (
+            "Throughput scores are higher-is-better; positive Head vs base deltas "
+            "indicate faster performance."
+        )
+    if directions == {"latency"}:
+        return (
+            "Latency scores are lower-is-better; positive Head vs base deltas "
+            "indicate faster performance."
+        )
+    return (
+        "Throughput scores are higher-is-better and latency scores are "
+        "lower-is-better; positive Head vs base deltas indicate faster performance."
+    )
+
+
 def generate_comparison_section(
     results: list,
     baseline_results: list,
@@ -356,7 +419,7 @@ def generate_comparison_section(
     md.append("")
     md.append(f"- **Head:** {format_commit_link(commit_sha, repo)}")
     md.append(f"- **Base:** {format_commit_link(baseline_sha, baseline_repo)}")
-    md.append("- **Change:** positive means the PR is faster than base.")
+    md.append(f"- **Metric direction:** {metric_direction_note(results)}")
     if comparison_note:
         md.append(f"- **Note:** {comparison_note}")
     if baseline_system_info:
@@ -373,7 +436,7 @@ def generate_comparison_section(
         md.append("")
         return md
 
-    md.append("| Benchmark | PR | Base | Change | Result |")
+    md.append("| Benchmark | PR | Base | Head vs base | Regression verdict |")
     md.append("|:----------|---:|-----:|-------:|:-------|")
 
     for name in common_names:
@@ -396,7 +459,9 @@ def generate_comparison_section(
         md.append("")
         if missing_in_base:
             missing = ", ".join(short_benchmark_name(name) for name in missing_in_base)
-            md.append(f"- Benchmarks only in PR results: {missing}")
+            md.append(
+                f"- Benchmarks only in PR results (listed separately below): {missing}"
+            )
         if missing_in_head:
             missing = ", ".join(short_benchmark_name(name) for name in missing_in_head)
             md.append(f"- Benchmarks only in base results: {missing}")
@@ -474,9 +539,25 @@ def generate_markdown(
             )
         )
 
+    # A benchmark without a base counterpart cannot receive a regression verdict.
+    # Keep it out of the comparison-oriented head table and list it separately.
+    baseline_names = {
+        b.get("benchmark", "") for b in (baseline_results or []) if b.get("benchmark")
+    }
+    if baseline_results:
+        comparable_results = [
+            b for b in results if b.get("benchmark", "") in baseline_names
+        ]
+        head_only_results = [
+            b for b in results if b.get("benchmark", "") not in baseline_names
+        ]
+    else:
+        comparable_results = results
+        head_only_results = []
+
     # Group by benchmark class
     benchmarks_by_class: dict[str, list] = {}
-    for b in results:
+    for b in comparable_results:
         name = b.get("benchmark", "")
         parts = name.rsplit(".", 1)
         if len(parts) == 2:
@@ -502,16 +583,10 @@ def generate_markdown(
             reverse=True,
         )
 
-        md.append("| Benchmark | Score | Error | Units | Within run |")
-        md.append("|:----------|------:|------:|:------|:-----------|")
-
-        best_score = (
-            sorted_benchmarks[0].get("primaryMetric", {}).get("score", 1)
-            if sorted_benchmarks
-            else 1
-        )
+        md.append("| Benchmark | Score | Error | Units |")
+        md.append("|:----------|------:|------:|:------|")
 
-        for i, b in enumerate(sorted_benchmarks):
+        for b in sorted_benchmarks:
             name = b.get("benchmark", "").split(".")[-1]
             score = b.get("primaryMetric", {}).get("score", 0)
             error = b.get("primaryMetric", {}).get("scoreError", 0)
@@ -520,23 +595,28 @@ def generate_markdown(
             score_fmt = format_score(score)
             error_fmt = format_error(error)
 
-            # Calculate relative performance as multiplier
-            try:
-                if i == 0:
-                    relative_fmt = "**fastest**"
-                else:
-                    multiplier = float(best_score) / float(score)
-                    if multiplier >= 10:
-                        relative_fmt = f"{multiplier:.0f}x slower"
-                    else:
-                        relative_fmt = f"{multiplier:.1f}x slower"
-            except (ValueError, TypeError, ZeroDivisionError):
-                relative_fmt = ""
+            md.append(f"| {name} | {score_fmt} | {error_fmt} | {unit} |")
 
+        md.append("")
+
+    if head_only_results:
+        md.append("## New benchmarks in PR head")
+        md.append("")
+        md.append(
+            "These benchmarks have no base counterpart; scores are descriptive only "
+            "and have no regression verdict."
+        )
+        md.append("")
+        md.append("| Benchmark | Score | Error | Units |")
+        md.append("|:----------|------:|------:|:------|")
+        for b in sorted(head_only_results, key=lambda x: x.get("benchmark", "")):
+            name = short_benchmark_name(b.get("benchmark", ""))
+            score = b.get("primaryMetric", {}).get("score", 0)
+            error = b.get("primaryMetric", {}).get("scoreError", 0)
+            unit = b.get("primaryMetric", {}).get("scoreUnit", "ops/s")
             md.append(
-                f"| {name} | {score_fmt} | {error_fmt} | {unit} | {relative_fmt} |"
+                f"| {name} | {format_score(score)} | {format_error(error)} | {unit} |"
             )
-
         md.append("")
 
     md.append("### Raw Results")
@@ -577,16 +657,21 @@ def generate_markdown(
 
     md.append("## Notes")
     md.append("")
-    md.append("- **Score** = Throughput in operations per second (higher is better)")
+    md.append(
+        "- **Score** = the JMH primary metric; "
+        "throughput is higher-is-better and latency is lower-is-better."
+    )
     md.append("- **Error** = 99.9% confidence interval")
     if baseline_results:
         md.append(
-            "- **Comparison with base** uses JMH confidence intervals when "
-            'available; overlapping intervals are marked "within noise".'
+            "- **Regression verdict** requires comparable benchmark metadata, "
+            "non-overlapping JMH confidence intervals, and a change of at least "
+            f"{PRACTICAL_CHANGE_THRESHOLD:.0f}%; otherwise it is marked "
+            '"within noise" or "inconclusive".'
         )
     md.append(
-        "- **Within run** compares benchmarks in the same result set, not against "
-        "the base commit."
+        "- Scores for different benchmark methods are not ranked against one another; "
+        "they may measure different workloads."
     )
     md.append("")