ci(perf): post the instruction-budget result as one PR comment (LAB-10982)

.github/workflows/ci.yml

@@ -512,7 +512,87 @@ jobs:
       working-directory: ${{ runner.temp }}/ir/head
       run: |
         .venv/bin/python tests/performance/ir_budget.py --base ../base --shard ${{ matrix.shard }}/4 \
-          --skip serializer_arrow_usgs --skip serializer_auto_usgs
+          --skip serializer_arrow_usgs --skip serializer_auto_usgs --report "$RUNNER_TEMP/ir-report-${{ matrix.shard }}-${{ github.run_attempt }}.json"
+
+    # The verdict, PASS or FAIL, for perf-ir-report. A shard with no verdict uploads nothing.
+    - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7
+      if: always()
+      with:
+        name: perf-ir-report-${{ matrix.shard }}
+        path: ${{ runner.temp }}/ir-report-${{ matrix.shard }}-${{ github.run_attempt }}.json
+        if-no-files-found: ignore
+        retention-days: 7
+        overwrite: true  # a re-run attempt replaces its shard's report; the file name says which attempt wrote it
+
+  # The shards' verdicts as one PR comment, so a FAIL or WARN does not hide in four job logs. Two jobs keep
+  # the write token away from repository code: this one renders the comment with read-only scopes (actions:
+  # read lists the shard jobs, to link a shard that left no report), and perf-ir-comment, which never checks
+  # out the repository, posts it. Fork PRs are skipped: their token cannot write to the PR. A cancelled run
+  # (a newer push) posts nothing, so it cannot overwrite the newer run's comment.
+  perf-ir-report:
+    name: Instruction budget report
+    needs: perf-ir
+    if: ${{ !cancelled() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository }}
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    permissions:
+      contents: read
+      actions: read
+    outputs:
+      body: ${{ steps.render.outputs.body }}
+    steps:
+    - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
+      with:
+        fetch-depth: 2  # the merge commit the shards measured, and its parents
+        persist-credentials: false
+
+    - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
+      with:
+        pattern: perf-ir-report-*
+        path: ${{ runner.temp }}/reports
+        merge-multiple: true  # one file per shard, ir-report-K-ATTEMPT.json, in one directory
+
+    - name: Render the comment
+      id: render
+      env:
+        GH_TOKEN: ${{ github.token }}
+        RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+      run: |
+        gh api --paginate "repos/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID/jobs?per_page=100" \
+          --jq '.jobs[] | {name, html_url, conclusion, run_attempt}' > "$RUNNER_TEMP/jobs.jsonl"
+        delimiter="EOF_$(openssl rand -hex 16)"
+        {
+          echo "body<<$delimiter"
+          python3 tests/performance/ir_report.py --reports "$RUNNER_TEMP/reports" --shards 4 --jobs "$RUNNER_TEMP/jobs.jsonl" \
+            --head "$(git rev-parse HEAD^2)" --base "$(git rev-parse HEAD^1)" --run-url "$RUN_URL"
+          echo "$delimiter"
+        } >> "$GITHUB_OUTPUT"
+
+  perf-ir-comment:
+    name: Instruction budget comment
+    needs: perf-ir-report
+    if: ${{ !cancelled() && needs.perf-ir-report.result == 'success' }}
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    permissions:
+      pull-requests: write  # enough for a PR's issue comments
+    steps:
+    - name: Post or update the PR comment
+      env:
+        GH_TOKEN: ${{ github.token }}
+        BODY: ${{ needs.perf-ir-report.outputs.body }}
+        PR: ${{ github.event.pull_request.number }}
+      run: |
+        # The marker ir_report.py opens the body with. Only the bot's own comment is edited, whoever copies it.
+        marker='<!-- cachekit-instruction-budget -->'
+        ids=$(gh api --paginate "repos/$GITHUB_REPOSITORY/issues/$PR/comments?per_page=100" \
+          --jq ".[] | select(.user.login == \"github-actions[bot]\" and (.body | startswith(\"$marker\"))) | .id")
+        id=${ids%%$'\n'*}
+        if [[ -n "$id" ]]; then
+          jq -n --arg body "$BODY" '{body: $body}' | gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$id" --input - > /dev/null
+        else
+          jq -n --arg body "$BODY" '{body: $body}' | gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR/comments" --input - > /dev/null
+        fi
 
   # Version sync + dependency CVE scan (push to main only)
   post-merge:

CONTRIBUTING.md

@@ -324,8 +324,10 @@ valgrind against budgets committed in `tests/performance/ir_baselines.json`, so
 is visible however loaded the machine is. Budgets belong to the build that recorded them; on a
 different interpreter, extension or C library, record a baseline on `main` first. Every pull
 request also runs it in CI (the `Instruction budget` jobs, CPython 3.12), against the PR's base
-measured on the same runner rather than against the committed budgets. Method, budgets and the
-CI job: [docs/performance.md](docs/performance.md#instruction-budgets).
+measured on the same runner rather than against the committed budgets. Each pull request gets one
+comment with the result, every path's figures included, updated in place on each run; pull requests
+from forks get none, and their shard logs carry the result. Method, budgets and the CI job:
+[docs/performance.md](docs/performance.md#instruction-budgets).
 
 ### Formatting Code
 

docs/performance.md

@@ -304,13 +304,14 @@ uv run python tests/performance/ir_budget.py --path serializer_arrow_usgs --path
 
 ### In CI
 
-Every pull request runs the gate in the `Instruction budget (shard K/4)` jobs of `.github/workflows/ci.yml`, on GitHub-hosted `ubuntu-latest` runners with CPython 3.12. They do not compare against the committed budgets: those belong to the machine and interpreter build that recorded them, and a CPython 3.12 patch release alone read 2.0% to 3.6% over them with no code change. Instead each job builds two checkouts side by side, the pull request (its merge commit) and its base (the base branch tip), measures both on the same runner, and fails when a path costs 1% or more above the base (2% for the orjson round trip). That is `ir_budget.py --base DIR`, which writes nothing.
+Every pull request runs the gate in the `Instruction budget (shard K/4)` jobs of `.github/workflows/ci.yml`, on GitHub-hosted `ubuntu-latest` runners with CPython 3.12. They do not compare against the committed budgets: those belong to the machine and interpreter build that recorded them, and a CPython 3.12 patch release alone read 2.0% to 3.6% over them with no code change. Instead each job builds two checkouts side by side, the pull request (its merge commit) and its base (the base branch tip), measures both on the same runner, and fails when a path costs 1% or more above the base (2% for the orjson round trip). That is `ir_budget.py --base DIR`, which never writes the committed budgets; CI adds `--report FILE`, which writes the verdict and every path's figures as JSON.
 
 - Both checkouts must sit at absolute paths of the same length, and the gate refuses a base that does not. The measured process's argv and import paths move its heap layout: a relative workload path against an absolute one read `l1_hit` 0.3% apart on identical source; at equal-length absolute paths the same pair agreed within 0.01%.
 - The two `serializer_*_usgs` paths are skipped (`--skip`): pyarrow's parquet reader can execute an instruction valgrind does not support on some CPUs, and the run then dies with SIGILL.
 - Four runners split the remaining paths (`--shard 1/4` to `--shard 4/4`), so the check takes minutes rather than half an hour.
 - A path the base does not have yet is reported as `new` and not compared.
-- CI keeps no budgets of its own, so there is nothing to re-record for it, and it ignores the committed ones entirely. They are still ratcheted on your machine with `make perf-ir-update`. A deliberate cost increase (`--update --allow-increase`, with the reason in the PR) therefore shows as a red `Instruction budget` check: the check is not part of `CI Success`, and the reviewer accepts the increase at merge. A raised committed budget cannot waive it, because a budget diff cannot tell a deliberate increase from a re-record on another interpreter or extension build, which moved one path 8.8% with no code change.
+- Each pull request carries one comment from `github-actions[bot]` with the result of its latest completed run: the overall verdict, the head and base commits measured, the interpreter, and every path's verdict, Ir/op on both sides and delta. Each run updates the same comment in place. A shard that ends without a verdict (a crash, an environment error, a timeout) shows as "no result", linked to its job, and the verdict reads `INCOMPLETE`, never `PASS`. The `Instruction budget report` job renders it from the shards' `--report` files with `tests/performance/ir_report.py`, and `Instruction budget comment` posts it: only that job can write to the pull request, and it never checks the code out. Pull requests from forks get no comment, because their token cannot write to the pull request; their shard logs still carry the result.
+- CI keeps no budgets of its own, so there is nothing to re-record for it, and it ignores the committed ones entirely. They are still ratcheted on your machine with `make perf-ir-update`. A deliberate cost increase (`--update --allow-increase`, with the reason in the PR) therefore shows as a red `Instruction budget` check and a `FAIL` in the pull request's instruction-budget comment: the check is not part of `CI Success`, and the reviewer accepts the increase at merge. A raised committed budget cannot waive it, because a budget diff cannot tell a deliberate increase from a re-record on another interpreter or extension build, which moved one path 8.8% with no code change.
 
 To reproduce a CI verdict, check the base out beside your branch at a path of the same length, run `uv sync --group dev` in both, and run the gate from your branch:
 

tests/performance/README.md

@@ -30,7 +30,8 @@ callgrind instructions per call for each hot path, failing at +1% (the orjson ro
 against `ir_baselines.json`. Budgets belong to the build that recorded them; on another
 interpreter, extension or C library, record a baseline on `main` first. CI runs it on every PR
 with `--base`: the PR and its base are measured on the same runner and compared with each other,
-not with the committed budgets ([In CI](../../docs/performance.md#in-ci)).
+not with the committed budgets, and `ir_report.py` renders the shards' `--report` files as one PR
+comment, which CI posts ([In CI](../../docs/performance.md#in-ci)).
 See [docs/performance.md](../../docs/performance.md#instruction-budgets).
 
 **Real data.** `tests/data/` holds a pinned slice of a public dataset (provenance in its README).

tests/performance/ir_budget.py

@@ -311,31 +311,48 @@ def median(p: str) -> int:
     return {p: median(p) for p in paths}
 
 
-def compare(budgets: dict[str, int], measured: dict[str, int], against: str = "budget") -> tuple[list[str], bool]:
-    """Report lines and whether the gate passes. A path without a budget fails: record it first.
-
-    ``against`` names what the budgets are in the report: the committed ``budget``, or the ``base``
-    checkout's figures measured in the same run (``--base``).
-    """
-    lines, ok = [], True
+def judge(budgets: dict[str, int], measured: dict[str, int]) -> list[dict[str, object]]:
+    """One row per measured path: its verdict, Ir/op, budget and delta %. A path without a budget is a FAIL
+    with neither of the last two."""
+    rows: list[dict[str, object]] = []
     for path, ir in measured.items():
         budget = budgets.get(path)
         if budget is None:
-            lines.append(f"FAIL  {path:22} {ir:>9,} Ir/op  no budget for this interpreter (run --update)")
-            ok = False
+            rows.append({"path": path, "verdict": "FAIL", "ir": ir, "budget": None, "pct": None})
             continue
         pct = (ir - budget) / budget * 100
         fail_pct, warn_pct = TOLERANCE_PCT.get(path, (FAIL_PCT, WARN_PCT))
         if pct >= fail_pct:
-            verdict, ok = "FAIL", False
+            verdict = "FAIL"
         elif pct >= warn_pct:
             verdict = "WARN"
         elif pct <= -fail_pct:
             verdict = "LOWER"  # cheaper: ratchet it down with --update if the change touched this path
         else:
             verdict = "ok"
-        lines.append(f"{verdict:5} {path:22} {ir:>9,} Ir/op  {against} {budget:>9,}  {pct:+.2f}%")
-    return lines, ok
+        rows.append({"path": path, "verdict": verdict, "ir": ir, "budget": budget, "pct": round(pct, 2)})
+    return rows
+
+
+def compare(budgets: dict[str, int], measured: dict[str, int], against: str = "budget") -> tuple[list[str], bool]:
+    """Report lines and whether the gate passes. A path without a budget fails: record it first.
+
+    ``against`` names what the budgets are in the report: the committed ``budget``, or the ``base``
+    checkout's figures measured in the same run (``--base``).
+    """
+    return report_lines(judge(budgets, measured), against)
+
+
+def report_lines(rows: list[dict[str, object]], against: str) -> tuple[list[str], bool]:
+    """``judge``'s rows as report lines, and whether none of them fails."""
+    lines = []
+    for row in rows:
+        verdict, path, ir, budget = row["verdict"], row["path"], row["ir"], row["budget"]
+        if budget is None:
+            lines.append(f"{verdict:5} {path:22} {ir:>9,} Ir/op  no budget for this interpreter (run --update)")
+        else:
+            lines.append(f"{verdict:5} {path:22} {ir:>9,} Ir/op  {against} {budget:>9,}  {row['pct']:+.2f}%")
+    return lines, all(row["verdict"] != "FAIL" for row in rows)
 
 
 def ratchet(budgets: dict[str, int], measured: dict[str, int], allow_increase: bool) -> dict[str, int]:
@@ -355,9 +372,12 @@ def _positive_minutes(text: str) -> float:
     return minutes
 
 
-def _against_base(base: Path, paths: list[str], jobs: int, child_timeout_s: float) -> int:
+def _against_base(base: Path, paths: list[str], jobs: int, child_timeout_s: float, report: Path | None = None) -> int:
     """``--base``: measure the base checkout and this one on this machine, and gate this one against it.
 
+    ``report`` also writes the verdict as JSON once there is one (``--report``), for CI to post on the pull
+    request. An environment error or a crashed run writes none, so a missing report means no verdict.
+
     Both checkouts run from absolute paths of the same length. The measured process's argv and import
     paths move its heap layout: a relative workload path against an absolute one read l1_hit 0.3% apart
     on identical source. The venv interpreter path is not resolved, as that would follow its symlink.
@@ -391,10 +411,15 @@ def _against_base(base: Path, paths: list[str], jobs: int, child_timeout_s: floa
 
     measured = measure(paths, jobs, child_timeout_s, sys.executable, WORKLOAD.resolve())
     base_figures = measure(paths, jobs, child_timeout_s, str(python), workload)
-    lines, ok = compare(base_figures, measured, against="base")
+    rows = judge(base_figures, measured)
+    lines, ok = report_lines(rows, against="base")
     print(f"main-thread Ir/op on {interpreter_key()} ({platform.python_version()}), this checkout against --base {base}")
     print("\n".join(lines + [f"new   {p:22} not on the base: nothing to compare against" for p in new]))
     print("PASS" if ok else f"FAIL: a path regressed by {FAIL_PCT}% or more against the base")
+    if report is not None:  # after the verdict is printed, so a failed write cannot hide it
+        rows += [{"path": p, "verdict": "new", "ir": None, "budget": None, "pct": None} for p in new]
+        summary = {"verdict": "PASS" if ok else "FAIL", "interpreter": interpreter_key(), "python": platform.python_version()}
+        report.write_text(json.dumps(summary | {"rows": rows}, indent=2) + "\n")
     return 0 if ok else 1
 
 
@@ -446,17 +471,25 @@ def _main() -> int:
         metavar="K/N",
         help="measure only every Nth path, starting at the Kth, so N machines can split the gate",
     )
+    parser.add_argument(
+        "--report",
+        type=Path,
+        metavar="FILE",
+        help="with --base, also write the verdict and every path's figures to FILE as JSON (CI posts it on the PR)",
+    )
     args = parser.parse_args()
     if args.base and args.update:
-        parser.error("--update writes committed budgets; --base compares two checkouts and writes nothing")
+        parser.error("--update writes committed budgets; --base compares two checkouts and never writes them")
+    if args.report and not args.base:
+        parser.error("--report writes the --base verdict; pass --base DIR")
     paths = select_paths(args.path, args.skip, args.shard)
 
     if shutil.which("valgrind") is None:
         print("valgrind not found: install it (apt install valgrind) to run the instruction budget", file=sys.stderr)
         return 2
     key = interpreter_key()
     if args.base:
-        return _against_base(args.base, paths, args.jobs, args.child_timeout * 60)
+        return _against_base(args.base, paths, args.jobs, args.child_timeout * 60, args.report)
     data = json.loads(BASELINES.read_text())
     entry = data["interpreters"].get(key, {"budgets": {}})
     if entry.get("python") not in (None, platform.python_version()):