From f4d6cf4c42ecc77b5e989034c217d294db7bd6b6 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Tue, 4 Aug 2026 15:11:39 +0000 Subject: [PATCH] Comment on benchmarks that exceed the 1 ms iteration budget The benchmarking guide asks that a single benchmark iteration stay under 1 ms, but nothing enforced it, so new benchmarks landed at 23.8 ms and 123.4 ms and were caught only by a maintainer reading CodSpeed's table by hand. CodSpeed already measures per-iteration time and publishes it in its sticky PR comment. Read that comment rather than measuring anything: an `issue_comment` job parses CodSpeed's report when it is posted or edited, and comments listing the benchmarks over budget. Nothing is rebuilt and nothing is re-run. The trade is scope, not accuracy, and the rendered comment says so: only benchmarks CodSpeed reports as new or changed are listed, and CodSpeed truncates its own table at 20 rows. Benchmarks this PR made faster are not flagged for being over budget. Report only, never a failing check. The all-clear text updates an existing complaint but never opens a new comment, so a PR that is within budget stays quiet. Signed-off-by: Joe Isaacs Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FZJAp4wVgwTVhLyErCWFZn --- .github/workflows/bench-budget.yml | 101 ++++++++ .github/workflows/ci.yml | 18 ++ docs/developer-guide/benchmarking.md | 21 +- scripts/check-bench-budget.py | 295 +++++++++++++++++++++++ scripts/tests/test_check_bench_budget.py | 280 +++++++++++++++++++++ 5 files changed, 714 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/bench-budget.yml create mode 100644 scripts/check-bench-budget.py create mode 100644 scripts/tests/test_check_bench_budget.py diff --git a/.github/workflows/bench-budget.yml b/.github/workflows/bench-budget.yml new file mode 100644 index 00000000000..2fe01da5379 --- /dev/null +++ b/.github/workflows/bench-budget.yml @@ -0,0 +1,101 @@ +name: Benchmark Budget + +# Enforces the "keep per-iteration execution time under 1 ms" rule from +# docs/developer-guide/benchmarking.md, reported as a comment rather than a failing check. +# +# CodSpeed already measures per-iteration time and publishes it in its own sticky PR +# comment, so this workflow reads that comment instead of building or running anything. +# The trade is scope, not accuracy: CodSpeed only reports benchmarks a PR added or +# changed, so an untouched benchmark that is already over budget is not caught here. +# +# `issue_comment` workflows always run from the default branch, so edits to this file (or +# to the script) only take effect once merged. Use the `workflow_dispatch` entry point to +# try a change against a real PR's report before merging it. + +on: + issue_comment: + types: [created, edited] + workflow_dispatch: + inputs: + pr-number: + description: "Pull request to re-check against the budget" + required: true + type: string + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.issue.number || inputs.pr-number }} + cancel-in-progress: true + +jobs: + bench-budget: + name: "Check benchmark iteration budget" + # CodSpeed edits one sticky comment per PR, so this fires on every update to its + # report. Anyone can post a comment containing the marker; pinning the author is what + # makes the parsed table trustworthy. + if: >- + github.event_name == 'workflow_dispatch' + || (github.event.issue.pull_request != null + && github.event.comment.user.login == 'codspeed-hq[bot]' + && contains(github.event.comment.body, '__CODSPEED_PERFORMANCE_REPORT_COMMENT__')) + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + issues: read + pull-requests: write + env: + PR_NUMBER: ${{ github.event.issue.number || inputs.pr-number }} + steps: + # On `issue_comment` this checks out the default branch, never the pull request's + # head. That is deliberate: the job holds a `pull-requests: write` token, and must + # run this repository's script rather than a version a fork could edit. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + + # The comment body is untrusted input, so it reaches the script through the + # environment and never through shell interpolation. + - name: Read CodSpeed report from the triggering comment + if: github.event_name != 'workflow_dispatch' + env: + COMMENT_BODY: ${{ github.event.comment.body }} + run: printenv COMMENT_BODY > codspeed-comment.md + + - name: Fetch CodSpeed report for the requested pull request + if: github.event_name == 'workflow_dispatch' + env: + GH_TOKEN: ${{ github.token }} + run: | + set -Eeuo pipefail + gh api "repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" --paginate \ + --jq '[.[] | select(.user.login == "codspeed-hq[bot]") | .body] | last' \ + > codspeed-comment.md + test -s codspeed-comment.md + + - name: Check against the budget + id: check + run: | + python3 scripts/check-bench-budget.py \ + --comment-file codspeed-comment.md \ + --output comment.md + cat comment.md >> "$GITHUB_STEP_SUMMARY" + + - name: Report over-budget benchmarks + if: steps.check.outputs.violations != '0' + uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3 + with: + file-path: comment.md + comment-tag: bench-budget + pr-number: ${{ env.PR_NUMBER }} + + # Nothing is over budget: update an existing complaint to say so, but never open a + # new all-clear comment on every PR that touches a benchmark. + - name: Clear a resolved report + if: steps.check.outputs.violations == '0' + uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3 + with: + file-path: comment.md + comment-tag: bench-budget + pr-number: ${{ env.PR_NUMBER }} + create-if-not-exists: false diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 86fa85fd4d6..a3a09b4ac48 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -166,6 +166,24 @@ jobs: uv run --no-project --with pytest --with xxhash \ pytest scripts/tests/test_measurement_id.py + # Covers scripts/check-bench-budget.py, which parses CodSpeed's PR comment. It runs from + # the `issue_comment` workflow where a parsing bug surfaces as a wrong comment on someone + # else's PR rather than as a red check, so the parser is tested here instead. + bench-budget-script: + name: "Benchmark budget script" + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + # sync: false — pure-stdlib script, so skip the ~6 min vortex-data extension build. + - uses: spiraldb/actions/.github/actions/setup-uv@a746510eafaa926484c354541cfc49b2ec06cc63 # 0.18.6 + with: + sync: false + - name: Pytest - benchmark budget check + run: | + uv run --no-project --with pytest \ + pytest scripts/tests/test_check_bench_budget.py + python-cuda-test: name: "Python CUDA (test)" if: github.repository == 'vortex-data/vortex' diff --git a/docs/developer-guide/benchmarking.md b/docs/developer-guide/benchmarking.md index 9a35437b764..f1839125cc3 100644 --- a/docs/developer-guide/benchmarking.md +++ b/docs/developer-guide/benchmarking.md @@ -132,7 +132,26 @@ gate it with `#[cfg(not(codspeed))]` if it genuinely cannot be made to fit. The number to check against the budget is the per-iteration time, not the time the whole benchmark binary takes. CodSpeed reports exactly that: its performance report on a pull request lists the per-iteration time under `HEAD` for every benchmark the pull request adds -or changes, so check any new benchmark there before merging. +or changes. + +CI reads that report on every pull request, and comments rather than failing the build. The +`bench-budget` job in `.github/workflows/bench-budget.yml` pulls the per-iteration times out +of CodSpeed's report and comments listing any benchmark over the budget. Nothing is rebuilt +or re-run: the numbers are the ones CodSpeed already published. + +Two limits follow from using that report as the source: + +- Only benchmarks CodSpeed reports as **new or changed** are checked, because those are the + only ones it lists. A benchmark you did not touch is not re-checked, so the budget is + enforced going forward rather than retroactively. +- CodSpeed truncates its table at 20 rows. When it does, the comment says so rather than + implying the rest were checked — open the full report in CodSpeed to see them. + +To check a report by hand, for example while iterating on the check itself: + +```bash +python3 scripts/check-bench-budget.py --comment-file codspeed-comment.md --output comment.md +``` ### Gate CodSpeed-incompatible benchmarks diff --git a/scripts/check-bench-budget.py b/scripts/check-bench-budget.py new file mode 100644 index 00000000000..7225d05734f --- /dev/null +++ b/scripts/check-bench-budget.py @@ -0,0 +1,295 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright the Vortex contributors + +"""Flag benchmarks that exceed the per-iteration budget, using CodSpeed's own PR comment. + +``docs/developer-guide/benchmarking.md`` asks that a single benchmark iteration stay under +1 ms. CodSpeed already measures and publishes exactly that number: its sticky PR comment +lists every benchmark the pull request added or changed, with the per-iteration time under +``HEAD``. This script reads that comment and re-reports the rows that blow the budget, so +nobody has to eyeball a 20-row table of microsecond values to notice a 123 ms benchmark. + +Nothing is rebuilt and nothing is re-run: the input is a comment body, so the check costs +one API-free job and a few milliseconds. + +Two consequences of that trade fall out of the source data, and the rendered report says +both out loud: + +* Only benchmarks CodSpeed reports as new or changed appear in the comment. An untouched + benchmark that was already over budget is invisible here. +* CodSpeed truncates its table to the first 20 rows, so a pull request that adds many slow + benchmarks can hide some of them behind the truncation marker. +""" + +from __future__ import annotations + +import argparse +import os +import re +import sys +from dataclasses import dataclass +from pathlib import Path +from urllib.parse import parse_qs, unquote, urlparse + +DEFAULT_MAX_NS = 1_000_000 +"""One millisecond, per the benchmarking guide.""" + +CODSPEED_MARKER = "__CODSPEED_PERFORMANCE_REPORT_COMMENT__" +"""Hidden marker CodSpeed puts at the top of the comment it owns.""" + +GUIDE_URL = ( + "https://github.com/vortex-data/vortex/blob/develop/docs/developer-guide/benchmarking.md" + "#keep-per-iteration-execution-time-under-1-ms" +) + +MAX_ROWS = 20 +"""Cap the rendered table. CodSpeed itself never reports more rows than this.""" + +TRUNCATION_MARKER = "Only the first" +"""Substring of CodSpeed's own "only the first N benchmarks are displayed" note.""" + +UNITS_NS = {"ns": 1, "us": 1_000, "µs": 1_000, "μs": 1_000, "ms": 1_000_000, "s": 1_000_000_000} +"""Divan/CodSpeed duration suffixes. Both micro sign variants appear in the wild.""" + +STATUS_BY_EMOJI = { + "🆕": "new", + "⚡": "improved", + "❌": "regressed", + "⚠️": "regressed", +} + +FLAGGED_BY_DEFAULT = frozenset({"new", "regressed", "changed"}) +"""A benchmark this pull request made *faster* is not a reason to open a complaint.""" + +DURATION_RE = re.compile(r"^([0-9][0-9,]*(?:\.[0-9]+)?)\s*(ns|us|µs|μs|ms|s)$") +NAME_RE = re.compile(r"``\s*(.+?)\s*``", re.DOTALL) +SEPARATOR_RE = re.compile(r"^\|[\s\-:|]+\|$") + + +@dataclass(frozen=True) +class Benchmark: + """One row of CodSpeed's "Performance Changes" table.""" + + uri: str + """Fully qualified benchmark URI, e.g. ``vortex-geo/benches/x.rs::contains::points``.""" + + name: str + """Short display name, as CodSpeed renders it.""" + + mode: str + """CodSpeed instrument that produced the number, e.g. ``Simulation``.""" + + status: str + """One of ``new``, ``improved``, ``regressed``, or ``changed``.""" + + head_ns: float + """Per-iteration time on the pull request's head commit, in nanoseconds.""" + + +def parse_duration(text: str) -> float | None: + """Parse a CodSpeed duration such as ``1,182.5 µs`` into nanoseconds. + + Returns ``None`` for anything that is not a duration, including the ``N/A`` CodSpeed + prints for the base side of a new benchmark. + """ + match = DURATION_RE.match(text.strip()) + if match is None: + return None + return float(match.group(1).replace(",", "")) * UNITS_NS[match.group(2)] + + +def format_duration(nanos: float) -> str: + """Render a nanosecond count using the same units as CodSpeed's own output.""" + for limit, unit, scale in ( + (1_000, "ns", 1), + (1_000_000, "µs", 1_000), + (1_000_000_000, "ms", 1_000_000), + ): + if nanos < limit: + return f"{nanos / scale:.4g} {unit}" + return f"{nanos / 1_000_000_000:.4g} s" + + +def split_row(line: str) -> list[str]: + """Split a Markdown table row into its cells.""" + stripped = line.strip() + if not stripped.startswith("|"): + return [] + return [cell.strip() for cell in stripped.strip("|").split("|")] + + +def parse_benchmark_cell(cell: str) -> tuple[str, str]: + """Pull ``(uri, name)`` out of the linked benchmark cell. + + The link target carries a percent-encoded ``uri`` query parameter, which is the only + unambiguous identifier -- the visible label omits the file, so two benchmarks in + different crates can share it. Falls back to the label when the link is missing. + """ + name_match = NAME_RE.search(cell) + name = name_match.group(1) if name_match else cell + uri = "" + link_match = re.search(r"\]\((https?://[^)\s]+)\)", cell) + if link_match: + query = parse_qs(urlparse(link_match.group(1)).query) + if query.get("uri"): + uri = unquote(query["uri"][0]) + return uri or name, name + + +def parse_comment(body: str) -> tuple[list[Benchmark], bool]: + """Parse CodSpeed's comment into benchmarks plus whether its table was truncated. + + Rows CodSpeed emits that carry no measurement -- the header, the ``| --- |`` + separator, and the ``| ... |`` truncation row -- are skipped rather than guessed at. + """ + benchmarks = [] + for line in body.splitlines(): + cells = split_row(line) + # marker, mode, benchmark, base, head, efficiency + if len(cells) != 6 or SEPARATOR_RE.match(line.strip()): + continue + head_ns = parse_duration(cells[4]) + if head_ns is None: + continue + uri, name = parse_benchmark_cell(cells[2]) + status = next( + (status for emoji, status in STATUS_BY_EMOJI.items() if emoji in cells[0]), + "changed", + ) + benchmarks.append(Benchmark(uri=uri, name=name, mode=cells[1] or "unknown", status=status, head_ns=head_ns)) + + return benchmarks, TRUNCATION_MARKER in body + + +def over_budget(benchmarks: list[Benchmark], max_ns: int, include_improved: bool = False) -> list[Benchmark]: + """Select the benchmarks that exceed the budget, slowest first.""" + flagged = FLAGGED_BY_DEFAULT | ({"improved"} if include_improved else set()) + violations = [b for b in benchmarks if b.head_ns > max_ns and b.status in flagged] + return sorted(violations, key=lambda b: b.head_ns, reverse=True) + + +def render_report(violations: list[Benchmark], reported: int, max_ns: int, truncated: bool) -> str: + """Render the body of the sticky comment.""" + budget = format_duration(max_ns) + lines = ["## ⏱️ Benchmark iteration budget", ""] + + if not violations: + lines += [ + f"`✅ {reported}` benchmark(s) changed by this PR are within the **{budget}** per-iteration budget.", + ] + else: + lines += [ + f"`⚠️ {len(violations)}` of the `{reported}` benchmark(s) changed by this PR " + f"exceed the **{budget}** per-iteration budget.", + "", + "| Benchmark | Per-iteration | Over budget | |", + "| --- | --- | --- | --- |", + ] + for violation in violations[:MAX_ROWS]: + marker = "🆕" if violation.status == "new" else "❌" + lines.append( + f"| `{violation.uri}` | {format_duration(violation.head_ns)} " + f"| {violation.head_ns / max_ns:.1f}× | {marker} |" + ) + lines += [ + "", + "Each iteration of a benchmarked closure should finish in under " + f"{budget}. CodSpeed's simulation instrument runs each benchmark exactly once, " + "so a slow iteration spends CI time without buying extra signal. Shrink the " + "input size, or gate the benchmark with `#[cfg(not(codspeed))]`. See " + f"[the benchmarking guide]({GUIDE_URL}).", + ] + + lines += [ + "", + "
How this is measured", + "", + "These numbers are CodSpeed's own, read straight from its performance report on " + "this PR -- nothing is rebuilt or re-run here.", + "", + "That means only benchmarks CodSpeed reports as **new or changed** are checked. A " + "benchmark this PR did not touch is not listed in its report, so an existing " + "benchmark that is already over budget will not show up here.", + ] + if truncated: + lines += [ + "", + "> ⚠️ _CodSpeed truncated its own table, so this PR may change more benchmarks " + "than were checked. [Open the full report in CodSpeed]" + "(https://app.codspeed.io/vortex-data/vortex) to see the rest._", + ] + lines += ["", "
"] + return "\n".join(lines) + "\n" + + +def read_comment(args: argparse.Namespace) -> str: + if args.comment_file is not None: + return args.comment_file.read_text() + body = os.environ.get(args.comment_env) + if body is None: + raise SystemExit(f"environment variable {args.comment_env} is not set") + return body + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + source = parser.add_mutually_exclusive_group() + source.add_argument("--comment-file", type=Path, help="file holding the CodSpeed comment body") + source.add_argument( + "--comment-env", + default="CODSPEED_COMMENT_BODY", + help="environment variable holding the CodSpeed comment body", + ) + parser.add_argument( + "--max-ns", + type=int, + default=DEFAULT_MAX_NS, + help=f"per-iteration budget in nanoseconds (default: {DEFAULT_MAX_NS})", + ) + parser.add_argument( + "--include-improved", + action="store_true", + help="also flag over-budget benchmarks that this PR made faster", + ) + parser.add_argument("--output", type=Path, required=True, help="path to write the comment to") + parser.add_argument( + "--github-output", + type=Path, + default=os.environ.get("GITHUB_OUTPUT"), + help="path to append the `violations` step output to", + ) + parser.add_argument( + "--fail-on-violation", + action="store_true", + help="exit non-zero when a benchmark is over budget (default: report only)", + ) + args = parser.parse_args() + + body = read_comment(args) + if CODSPEED_MARKER not in body: + print(f"comment does not contain {CODSPEED_MARKER}; refusing to parse", file=sys.stderr) + return 1 + + benchmarks, truncated = parse_comment(body) + violations = over_budget(benchmarks, args.max_ns, args.include_improved) + + args.output.parent.mkdir(parents=True, exist_ok=True) + args.output.write_text(render_report(violations, len(benchmarks), args.max_ns, truncated)) + + if args.github_output is not None: + with Path(args.github_output).open("a") as f: + f.write(f"violations={len(violations)}\n") + + print( + f"{len(benchmarks)} benchmark(s) reported by CodSpeed, " + f"{len(violations)} over the {format_duration(args.max_ns)} budget" + ) + for violation in violations: + print(f" {format_duration(violation.head_ns):>10} {violation.uri}") + + return 1 if violations and args.fail_on_violation else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/test_check_bench_budget.py b/scripts/tests/test_check_bench_budget.py new file mode 100644 index 00000000000..28dc1f59be9 --- /dev/null +++ b/scripts/tests/test_check_bench_budget.py @@ -0,0 +1,280 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright the Vortex contributors + +"""Tests for `scripts/check-bench-budget.py`. + +The fixtures below are trimmed copies of real CodSpeed comments on this repository, so the +parser is tested against the markup CodSpeed actually posts rather than an idealised +version of it. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +_SPEC = importlib.util.spec_from_file_location( + "check_bench_budget", Path(__file__).parent.parent / "check-bench-budget.py" +) +assert _SPEC is not None and _SPEC.loader is not None +budget = importlib.util.module_from_spec(_SPEC) +sys.modules["check_bench_budget"] = budget +_SPEC.loader.exec_module(budget) + + +def _row(marker: str, name: str, uri: str, base: str, head: str, efficiency: str) -> str: + link = f"https://app.codspeed.io/vortex-data/vortex/branches/x?uri={uri}&runnerMode=Simulation" + return f"| {marker} | Simulation | [`` {name} ``]({link}) | {base} | {head} | {efficiency} |" + + +NEW_BENCHMARKS_COMMENT = "\n".join( + [ + "", + "## Merging this PR will **not alter performance**", + "", + "`✅ 1885` untouched benchmarks ", + "`🆕 75` new benchmarks ", + "", + "### Performance Changes", + "", + "| | Mode | Benchmark | `BASE` | `HEAD` | Efficiency |", + "| --- | ---- | --------- | ------ | ------ | ---------- |", + _row( + "🆕", + "inline[4096]", + "vortex-array%2Fbenches%2Fbyte_length.rs%3A%3Ainline%5B4096%5D", + "N/A", + "61.8 µs", + "N/A", + ), + _row( + "🆕", + "like_per_row_distinct_patterns", + "vortex-array%2Fbenches%2Flike.rs%3A%3Alike_per_row_distinct_patterns", + "N/A", + "1.1 ms", + "N/A", + ), + _row( + "🆕", + "column_x_column_polygons", + "vortex-geo%2Fbenches%2Fbinary_predicates.rs%3A%3Acontains%3A%3Acolumn_x_column_polygons", + "N/A", + "23.8 ms", + "N/A", + ), + _row( + "🆕", + "constant_x_polygons_overlapping", + "vortex-geo%2Fbenches%2Fbinary_predicates.rs%3A%3Acontains%3A%3Aconstant_x_polygons_overlapping", + "N/A", + "123.4 ms", + "N/A", + ), + "| ... | ... | ... | ... | ... | ... |", + "", + "> :information_source: _Only the first 20 benchmarks are displayed._", + "", + "Comparing a (9755708) with develop (c2288dc)", + ] +) + +IMPROVED_COMMENT = "\n".join( + [ + "", + "## Merging this PR will **improve performance by 27.76%**", + "", + "`⚡ 2` improved benchmarks ", + "`✅ 1840` untouched benchmarks ", + "", + "### Performance Changes", + "", + "| | Mode | Benchmark | `BASE` | `HEAD` | Efficiency |", + "| --- | ---- | --------- | ------ | ------ | ---------- |", + _row( + "⚡", + "take_map[(0.1, 1.0)]", + "vortex-array%2Fbenches%2Ftake_patches.rs%3A%3Atake_map", + "2.2 ms", + "1.6 ms", + "+34.9%", + ), + _row( + "⚡", + "take_map[(0.1, 0.5)]", + "vortex-array%2Fbenches%2Ftake_patches.rs%3A%3Atake_map2", + "1,182.5 µs", + "977.3 µs", + "+20.99%", + ), + ] +) + + +@pytest.mark.parametrize( + ("text", "expected"), + [ + ("61.8 µs", 61_800.0), + ("61.8 μs", 61_800.0), # micro sign vs greek mu + ("61.8 us", 61_800.0), + ("1.1 ms", 1_100_000.0), + ("1,182.5 µs", 1_182_500.0), + ("794 ns", 794.0), + ("1.5 s", 1_500_000_000.0), + ("N/A", None), + ("", None), + ("...", None), + ("+34.9%", None), + ], +) +def test_parse_duration(text: str, expected: float | None) -> None: + assert budget.parse_duration(text) == expected + + +@pytest.mark.parametrize( + ("nanos", "expected"), + [(794, "794 ns"), (61_800, "61.8 µs"), (23_800_000, "23.8 ms"), (1_500_000_000, "1.5 s")], +) +def test_format_duration(nanos: float, expected: str) -> None: + assert budget.format_duration(nanos) == expected + + +def test_parse_comment_reads_new_benchmarks() -> None: + benchmarks, truncated = budget.parse_comment(NEW_BENCHMARKS_COMMENT) + + assert truncated + assert [b.name for b in benchmarks] == [ + "inline[4096]", + "like_per_row_distinct_patterns", + "column_x_column_polygons", + "constant_x_polygons_overlapping", + ] + assert {b.status for b in benchmarks} == {"new"} + assert benchmarks[0].mode == "Simulation" + # The URI is percent-decoded, and identifies the file the benchmark lives in. + assert benchmarks[2].uri == ("vortex-geo/benches/binary_predicates.rs::contains::column_x_column_polygons") + assert benchmarks[3].head_ns == 123_400_000 + + +def test_parse_comment_skips_header_separator_and_truncation_rows() -> None: + benchmarks, _ = budget.parse_comment(NEW_BENCHMARKS_COMMENT) + assert len(benchmarks) == NEW_BENCHMARKS_COMMENT.count("app.codspeed.io") + + +def test_parse_comment_without_a_table() -> None: + body = "\nNo performance changes.\n" + assert budget.parse_comment(body) == ([], False) + + +def test_over_budget_flags_only_slow_benchmarks() -> None: + benchmarks, _ = budget.parse_comment(NEW_BENCHMARKS_COMMENT) + violations = budget.over_budget(benchmarks, budget.DEFAULT_MAX_NS) + + # Sorted slowest first; the 61.8 µs benchmark is comfortably inside the budget. + assert [b.name for b in violations] == [ + "constant_x_polygons_overlapping", + "column_x_column_polygons", + "like_per_row_distinct_patterns", + ] + + +def test_over_budget_ignores_improvements_by_default() -> None: + benchmarks, _ = budget.parse_comment(IMPROVED_COMMENT) + + assert budget.over_budget(benchmarks, budget.DEFAULT_MAX_NS) == [] + # Only the 1.6 ms row is over budget; its 977.3 µs sibling is inside it either way. + included = budget.over_budget(benchmarks, budget.DEFAULT_MAX_NS, include_improved=True) + assert [b.name for b in included] == ["take_map[(0.1, 1.0)]"] + + +def test_over_budget_honours_a_custom_budget() -> None: + benchmarks, _ = budget.parse_comment(NEW_BENCHMARKS_COMMENT) + violations = budget.over_budget(benchmarks, 50_000_000) + assert [b.name for b in violations] == ["constant_x_polygons_overlapping"] + + +def test_render_report_lists_violations() -> None: + benchmarks, truncated = budget.parse_comment(NEW_BENCHMARKS_COMMENT) + violations = budget.over_budget(benchmarks, budget.DEFAULT_MAX_NS) + report = budget.render_report(violations, len(benchmarks), budget.DEFAULT_MAX_NS, truncated) + + assert "`⚠️ 3` of the `4` benchmark(s)" in report + assert "123.4 ms" in report + assert "123.4×" in report + assert "vortex-geo/benches/binary_predicates.rs::contains::constant_x_polygons_overlapping" in report + assert "#[cfg(not(codspeed))]" in report + assert "CodSpeed truncated its own table" in report + # A benchmark inside the budget is never named. + assert "inline[4096]" not in report + + +def test_render_report_when_everything_is_within_budget() -> None: + benchmarks, truncated = budget.parse_comment(IMPROVED_COMMENT) + report = budget.render_report([], len(benchmarks), budget.DEFAULT_MAX_NS, truncated) + + assert "`✅ 2` benchmark(s)" in report + assert "| Benchmark |" not in report + assert "CodSpeed truncated its own table" not in report + + +def test_main_writes_comment_and_step_output(tmp_path: Path) -> None: + comment = tmp_path / "codspeed.md" + comment.write_text(NEW_BENCHMARKS_COMMENT) + output, step_output = tmp_path / "comment.md", tmp_path / "github_output" + + argv = [ + "check-bench-budget.py", + "--comment-file", + str(comment), + "--output", + str(output), + "--github-output", + str(step_output), + ] + with pytest.MonkeyPatch.context() as patch: + patch.setattr(sys, "argv", argv) + assert budget.main() == 0 + + assert "123.4 ms" in output.read_text() + assert step_output.read_text() == "violations=3\n" + + +def test_main_reads_the_comment_from_the_environment(tmp_path: Path) -> None: + output = tmp_path / "comment.md" + argv = ["check-bench-budget.py", "--output", str(output), "--github-output", str(tmp_path / "o")] + with pytest.MonkeyPatch.context() as patch: + patch.setattr(sys, "argv", argv) + patch.setenv("CODSPEED_COMMENT_BODY", IMPROVED_COMMENT) + assert budget.main() == 0 + + assert "within the **1 ms** per-iteration budget" in output.read_text() + + +def test_main_rejects_a_comment_that_is_not_codspeeds(tmp_path: Path) -> None: + output = tmp_path / "comment.md" + argv = ["check-bench-budget.py", "--output", str(output), "--github-output", str(tmp_path / "o")] + with pytest.MonkeyPatch.context() as patch: + patch.setattr(sys, "argv", argv) + patch.setenv("CODSPEED_COMMENT_BODY", "This benchmark has a too long runtime") + assert budget.main() == 1 + + assert not output.exists() + + +def test_main_can_fail_the_job(tmp_path: Path) -> None: + output = tmp_path / "comment.md" + argv = [ + "check-bench-budget.py", + "--output", + str(output), + "--github-output", + str(tmp_path / "o"), + "--fail-on-violation", + ] + with pytest.MonkeyPatch.context() as patch: + patch.setattr(sys, "argv", argv) + patch.setenv("CODSPEED_COMMENT_BODY", NEW_BENCHMARKS_COMMENT) + assert budget.main() == 1