Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/bench-canonical.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,39 @@
sys.exit(0 if overall.value == 'pass' else (1 if overall.value == 'fail' else 0))
"

- name: Compute badge text
# Renders the one-line badge from the merged JSON. Uses
# `benchmarks.badge` (unit-tested) so the workflow stays free
# of count logic. Output is stashed in $GITHUB_OUTPUT for the
# next step. Skips when bench errored — leaves the previous
# badge in place rather than rewriting to a stale 0/N line.
id: badge
if: steps.bench.outputs.out != ''
run: |
set -euo pipefail
text=$(uv run python -m benchmarks.badge "${{ steps.bench.outputs.out }}")
echo "text=${text}" >> "$GITHUB_OUTPUT"
echo "computed: ${text}"

- name: Sync README into bench-canonical-results worktree + rewrite badge
# Issue #477: badge in README on the bench-canonical-results
# branch is auto-rewritten on each cron run. main is not
# touched (operator cherry-picks if they want main current).
# README is copied from the main checkout each run so the
# bench-canonical-results branch never carries a stale README.
if: steps.badge.outputs.text != ''
run: |
set -euo pipefail
cp README.md .bench-results-branch/README.md
new="${{ steps.badge.outputs.text }}" awk '
BEGIN { in_block=0 }
/<!-- bench-canonical-badge:start -->/ { print; print ENVIRON["new"]; in_block=1; next }
/<!-- bench-canonical-badge:end -->/ { print; in_block=0; next }
in_block { next }
{ print }
' .bench-results-branch/README.md > .bench-results-branch/README.md.new
mv .bench-results-branch/README.md.new .bench-results-branch/README.md
Comment on lines +138 to +148

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 suggestion (security): Guard against shell interpretation of special characters in the badge text when passing it through the new env var.

Using new="${{ steps.badge.outputs.text }}" awk ... assumes the badge text never includes shell‑significant characters ($, backticks, backslashes, quotes, etc.). A future change to the badge text could cause the shell to reinterpret it and break this step. To harden this, consider passing the value to awk without letting the shell re‑parse it—for example, write the text to a temp file that awk reads, or use a safely quoted printf and env NEW_TEXT="..." awk ... pattern.

Suggested change
run: |
set -euo pipefail
cp README.md .bench-results-branch/README.md
new="${{ steps.badge.outputs.text }}" awk '
BEGIN { in_block=0 }
/<!-- bench-canonical-badge:start -->/ { print; print ENVIRON["new"]; in_block=1; next }
/<!-- bench-canonical-badge:end -->/ { print; in_block=0; next }
in_block { next }
{ print }
' .bench-results-branch/README.md > .bench-results-branch/README.md.new
mv .bench-results-branch/README.md.new .bench-results-branch/README.md
run: |
set -euo pipefail
cp README.md .bench-results-branch/README.md
printf '%s\n' "${{ steps.badge.outputs.text }}" > .bench-results-branch/new_badge.txt
awk -v badge_file=".bench-results-branch/new_badge.txt" '
BEGIN { in_block=0 }
/<!-- bench-canonical-badge:start -->/ {
print
while ((getline line < badge_file) > 0) {
print line
}
close(badge_file)
in_block=1
next
}
/<!-- bench-canonical-badge:end -->/ { print; in_block=0; next }
in_block { next }
{ print }
' .bench-results-branch/README.md > .bench-results-branch/README.md.new
mv .bench-results-branch/README.md.new .bench-results-branch/README.md

Comment on lines +138 to +148

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when badge markers are missing/unbalanced.

Line 143–Line 146 can silently truncate the README tail if the end marker is absent. Add an explicit marker-count guard before rewrite.

Suggested hardening
       - name: Sync README into bench-canonical-results worktree + rewrite badge
         if: steps.badge.outputs.text != ''
         run: |
           set -euo pipefail
           cp README.md .bench-results-branch/README.md
+          start_count=$(grep -c '<!-- bench-canonical-badge:start -->' .bench-results-branch/README.md || true)
+          end_count=$(grep -c '<!-- bench-canonical-badge:end -->' .bench-results-branch/README.md || true)
+          if [ "$start_count" -ne 1 ] || [ "$end_count" -ne 1 ]; then
+            echo "::error::README badge markers must exist exactly once (start=$start_count end=$end_count)"
+            exit 1
+          fi
           new="${{ steps.badge.outputs.text }}" awk '
             BEGIN { in_block=0 }
             /<!-- bench-canonical-badge:start -->/ { print; print ENVIRON["new"]; in_block=1; next }
             /<!-- bench-canonical-badge:end -->/   { print; in_block=0; next }
             in_block { next }
             { print }
           ' .bench-results-branch/README.md > .bench-results-branch/README.md.new
           mv .bench-results-branch/README.md.new .bench-results-branch/README.md
🧰 Tools
🪛 GitHub Check: zizmor

[notice] 141-141:
code injection via template expansion

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/bench-canonical.yml around lines 138 - 148, The AWK
rewrite silently drops content if the end marker is missing; before replacing
.bench-results-branch/README.md, add a guard that counts occurrences of the
start and end markers (e.g., using grep -c on "<!-- bench-canonical-badge:start
-->" and "<!-- bench-canonical-badge:end -->") and exit non‑zero if the counts
are not exactly as expected (e.g., one start and one end or matching counts),
aborting the job with a clear error; implement this check in the same run block
immediately before the AWK transform so the script fails fast instead of
producing a truncated README.


- name: Commit + push to bench-canonical-results
env:
GIT_AUTHOR_NAME: aelfrice-bench-bot
Expand Down
59 changes: 59 additions & 0 deletions benchmarks/badge.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
"""Compute the README reproducibility-badge text from a canonical bench JSON.

The nightly `bench-canonical` cron writes a merged report under
`benchmarks/results/v2.0.0-cron-<date>.json`. This module reads that
report and produces the one-line badge text that the workflow splices
between the `<!-- bench-canonical-badge:start -->` and
`<!-- bench-canonical-badge:end -->` markers in README.md.

Issue: #477.
"""
from __future__ import annotations

import argparse
import json
import sys
from datetime import datetime, timezone
from pathlib import Path
from typing import Mapping


def _count_invocations(headline_cut: Mapping[str, list]) -> int:
return sum(len(v) for v in headline_cut.values())


def _count_ok(results: Mapping[str, Mapping[str, Mapping]]) -> int:
ok = 0
for by_sub in results.values():
for entry in by_sub.values():
if isinstance(entry, Mapping) and entry.get("_status") == "ok":
ok += 1
return ok


def compute_badge_text(report_path: Path, *, today: str | None = None) -> str:
"""Render the badge line from the report at *report_path*.

`today` defaults to UTC `YYYY-MM-DD`; the parameter exists so tests
can pin a date.
"""
data = json.loads(Path(report_path).read_text())
total = _count_invocations(data["headline_cut"])
ok = _count_ok(data["results"])
if today is None:
today = datetime.now(timezone.utc).strftime("%Y-%m-%d")
icon = "✅" if ok == total and total > 0 else "⚠️"
return f"reproducibility: {icon} {ok}/{total} ok · last run {today}"


def main(argv: list[str] | None = None) -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("report", type=Path, help="path to merged canonical JSON")
parser.add_argument("--today", default=None, help="override UTC date (YYYY-MM-DD)")
args = parser.parse_args(argv)
sys.stdout.write(compute_badge_text(args.report, today=args.today) + "\n")
return 0


if __name__ == "__main__":
raise SystemExit(main())
118 changes: 118 additions & 0 deletions tests/test_benchmarks_badge.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
"""Tests for benchmarks.badge — README reproducibility-badge text formatter.

Issue: #477.
"""
from __future__ import annotations

import json
from pathlib import Path

import pytest

from benchmarks import badge


def _write_report(
tmp_path: Path,
*,
headline_cut: dict,
results: dict,
) -> Path:
p = tmp_path / "report.json"
p.write_text(
json.dumps(
{
"label": "test",
"headline_cut": headline_cut,
"metric_overrides": {},
"results": results,
}
)
)
return p


def test_all_ok_renders_check_icon(tmp_path):
report = _write_report(
tmp_path,
headline_cut={"a": [{"sub_key": None}], "b": [{"sub_key": None}]},
results={
"a": {"_": {"_status": "ok"}},
"b": {"_": {"_status": "ok"}},
},
)
text = badge.compute_badge_text(report, today="2026-05-08")
assert text == "reproducibility: ✅ 2/2 ok · last run 2026-05-08"


def test_partial_renders_warn_icon(tmp_path):
report = _write_report(
tmp_path,
headline_cut={"a": [{"sub_key": None}], "b": [{"sub_key": None}]},
results={
"a": {"_": {"_status": "ok"}},
"b": {"_": {"_status": "error"}},
},
)
text = badge.compute_badge_text(report, today="2026-05-08")
assert text == "reproducibility: ⚠️ 1/2 ok · last run 2026-05-08"


def test_total_counts_subkeys_not_adapters(tmp_path):
"""An adapter with N parametrised invocations contributes N to total."""
report = _write_report(
tmp_path,
headline_cut={
"mab": [{"sub_key": "x"}, {"sub_key": "y"}, {"sub_key": "z"}],
"amabench": [{"sub_key": None}],
},
results={
"mab": {"x": {"_status": "ok"}, "y": {"_status": "ok"}, "z": {"_status": "ok"}},
"amabench": {"_": {"_status": "ok"}},
},
)
text = badge.compute_badge_text(report, today="2026-05-08")
assert "4/4" in text


def test_skipped_counts_as_not_ok(tmp_path):
"""Per #479, skipped_data_missing is distinct from ok; it does not count."""
report = _write_report(
tmp_path,
headline_cut={"a": [{"sub_key": None}], "b": [{"sub_key": None}]},
results={
"a": {"_": {"_status": "ok"}},
"b": {"_": {"_status": "skipped_data_missing"}},
},
)
text = badge.compute_badge_text(report, today="2026-05-08")
assert "1/2 ok" in text
assert text.startswith("reproducibility: ⚠️")


def test_canonical_v200_partial(tmp_path):
"""Sanity: today's checked-in canonical reports 6/11."""
canonical = Path(__file__).parent.parent / "benchmarks" / "results" / "v2.0.0.json"
if not canonical.exists():
pytest.skip("canonical baseline not present")
text = badge.compute_badge_text(canonical, today="2026-05-08")
assert "6/11 ok" in text


def test_zero_total_does_not_render_check(tmp_path):
"""Empty headline_cut shouldn't produce the all-green icon."""
report = _write_report(tmp_path, headline_cut={}, results={})
text = badge.compute_badge_text(report, today="2026-05-08")
assert text.startswith("reproducibility: ⚠️ 0/0 ok")


def test_today_defaults_to_utc(tmp_path):
report = _write_report(
tmp_path,
headline_cut={"a": [{"sub_key": None}]},
results={"a": {"_": {"_status": "ok"}}},
)
text = badge.compute_badge_text(report)
# YYYY-MM-DD shape; don't pin the actual day.
suffix = text.rsplit("last run ", 1)[1]
assert len(suffix) == 10 and suffix[4] == "-" and suffix[7] == "-"
Loading