ci(benchmarks): surface the gate table as a sticky PR comment

.github/workflows/_checks.yml

@@ -25,20 +25,6 @@ jobs:
       - uses: astral-sh/setup-uv@v8.2.0
       - run: just docs-build
 
-  bench:
-    runs-on: ubuntu-latest
-    timeout-minutes: 20
-    steps:
-      - uses: actions/checkout@v6
-      - uses: extractions/setup-just@v4
-      - uses: astral-sh/setup-uv@v8.2.0
-      # uv.lock is git-ignored (repo convention), so a clean checkout lacks it.
-      # `just bench-check` builds the compose `application` image, whose Dockerfile
-      # does `COPY uv.lock` + `uv sync --frozen`; regenerate the lock first so that
-      # build has it. (The pytest job runs `uv sync` directly and needs no compose.)
-      - run: uv lock
-      - run: just bench-check
-
   pytest:
     runs-on: ubuntu-latest
     strategy:

.github/workflows/benchmarks.yml

@@ -0,0 +1,55 @@
+name: benchmarks
+
+on:
+  push:
+    branches: [main]
+  pull_request: {}
+
+permissions:
+  contents: read
+  pull-requests: write
+
+concurrency:
+  group: benchmarks-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  bench:
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+    steps:
+      - uses: actions/checkout@v6
+      - uses: extractions/setup-just@v4
+      - uses: astral-sh/setup-uv@v8.2.0
+      # uv.lock is git-ignored (repo convention), so a clean checkout lacks it.
+      # `just bench-report` builds the compose `application` image, whose Dockerfile
+      # does `COPY uv.lock` + `uv sync --frozen`; regenerate the lock first.
+      - run: uv lock
+      - name: Run the gate and capture the markdown report
+        shell: bash
+        run: |
+          # `docker compose run` leaks buildkit image-build output onto stdout in
+          # CI, so a naive capture mixes it into the comment. The report always
+          # begins with the `## Benchmark gate` sentinel and the container prints
+          # it last, so slice from that heading to EOF -- independent of how
+          # compose routes its own noise. Own the exit code manually: `set +e`
+          # (shell: bash runs with -e) so a failing gate still lets us post the
+          # report, then re-exit with the gate's status so the build still fails.
+          set +e
+          just bench-report > "$RUNNER_TEMP/raw.log"
+          status=$?
+          sed -n '/^## Benchmark gate/,$p' "$RUNNER_TEMP/raw.log" \
+            | tee "$GITHUB_STEP_SUMMARY" "$RUNNER_TEMP/bench-report.md"
+          exit "$status"
+      - name: Comment the report on the PR
+        # always(): comment even when the gate failed -- that is when it matters most.
+        # continue-on-error: fork PRs get a read-only token and cannot comment; the
+        # gate above already decided the build, and the run Summary still has the table.
+        if: always() && github.event_name == 'pull_request'
+        continue-on-error: true
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          gh pr comment "${{ github.event.pull_request.number }}" \
+            --edit-last --create-if-none \
+            --body-file "$RUNNER_TEMP/bench-report.md"

Justfile

@@ -52,6 +52,10 @@ bench *args: down && down
 bench-check: down && down
     docker compose run application uv run python -m benchmarks check
 
+# Gate + emit the markdown report to stdout (CI PR comment). -T keeps stdout clean.
+bench-report: down && down
+    docker compose run -T application uv run python -m benchmarks check --markdown
+
 # Serve docs at http://127.0.0.1:8000 with hot-reload on save.
 docs-serve:
     uvx --with-requirements docs/requirements.txt mkdocs serve

benchmarks/__main__.py

@@ -12,7 +12,7 @@
 import sys
 
 from benchmarks.config import DEFAULT_DSN, RunConfig
-from benchmarks.report import compare, format_table, to_baseline
+from benchmarks.report import compare, format_markdown, format_table, to_baseline
 from benchmarks.workload import RunResult, make_engine, run_consumer, run_producer
 
 
@@ -66,7 +66,7 @@ async def _run_sweep(dsn: str, messages: int, repeats: int) -> list[RunResult]:
     return results
 
 
-def _check(dsn: str) -> int:
+def _check(dsn: str, *, markdown: bool = False) -> int:
     """Run the sweep at the baseline's message count and gate the counters."""
     if not BASELINE_PATH.exists():
         sys.stdout.write(f"no baseline at {BASELINE_PATH}; run `just bench --write-baseline` first\n")
@@ -77,8 +77,12 @@ def _check(dsn: str) -> int:
     # so repeats=1 -- repeating them only burns CI time.
     messages = int(next(iter(baseline["runs"].values()))["messages"])
     results = asyncio.run(_run_sweep(dsn, messages, repeats=1))
-    sys.stdout.write(format_table(results) + "\n")
     failures = compare(to_baseline(results), baseline)
+    if markdown:
+        # Python owns the whole PR-comment body; the workflow just posts stdout.
+        sys.stdout.write(format_markdown(results, failures) + "\n")
+        return 1 if failures else 0
+    sys.stdout.write(format_table(results) + "\n")
     if failures:
         sys.stdout.write("\nBENCHMARK GATE FAILED:\n")
         for failure in failures:
@@ -106,12 +110,13 @@ def main() -> int:
     # uses 1: the gated counters are deterministic, so repeating them only burns CI time.
     parser.add_argument("--repeats", type=int, default=3)
     parser.add_argument("--write-baseline", action="store_true")
+    parser.add_argument("--markdown", action="store_true")
     args = parser.parse_args()
 
     dsn = os.environ.get("POSTGRES_DSN", DEFAULT_DSN)
 
     if args.command == "check":
-        return _check(dsn)
+        return _check(dsn, markdown=args.markdown)
     return _run(dsn, args.messages, args.repeats, write_baseline=args.write_baseline)
 
 

benchmarks/report.py

@@ -127,6 +127,37 @@ def format_table(results: list[RunResult]) -> str:
     return "\n".join(lines)
 
 
+def format_markdown(results: list[RunResult], failures: list[str]) -> str:
+    """Render the sweep as a GitHub-flavored Markdown comment body.
+
+    Heading + verdict + table + a condensed gated-vs-informational footnote.
+    ``failures`` is ``compare()``'s output: empty means the gate passed. This
+    function only formats -- the exit code is the caller's, so the comment can be
+    posted on both pass and fail.
+    """
+    verdict = "✅ gate passed" if not failures else "❌ gate FAILED"
+    lines = ["## Benchmark gate", "", verdict, ""]
+    if failures:
+        lines.extend(f"- {failure}" for failure in failures)
+        lines.append("")
+    lines.append("| scenario | msg/s | delete/msg | WALrec/msg | WALB/msg | fpi | upd | del | dead_tup |")
+    lines.append("| --- | --: | --: | --: | --: | --: | --: | --: | --: |")
+    for r in results:
+        m = normalize(r)
+        lines.append(
+            f"| {_key(r)} | {m['msgs_per_second']:.0f} | {m['delete_calls_per_msg']:.3f} "
+            f"| {m['wal_records_per_msg']:.2f} | {m['wal_bytes_per_msg']:.0f} | {m['wal_fpi']:.0f} "
+            f"| {m['tup_upd']:.0f} | {m['tup_del']:.0f} | {m['dead_tup']:.0f} |",
+        )
+    lines.append("")
+    lines.append(
+        "_Gated (fails the build): `delete_calls` + tuple counters (upd/del/ins) + the "
+        "producer's `insert_calls`/`select_calls`, exact; `wal_records` within a 10% band. "
+        "msg/s, WAL bytes and total calls are informational (timing/FPI noise)._",
+    )
+    return "\n".join(lines)
+
+
 def compare(current: dict[str, typing.Any], baseline: dict[str, typing.Any]) -> list[str]:
     """Diff current raw totals against the baseline. Empty list means pass.
 

planning/changes/2026-07-17.03-benchmark-pr-comment.md

@@ -0,0 +1,195 @@
+---
+summary: Post the existing `bench-check` sweep table as a sticky PR comment (and run Summary). A new `benchmarks check --markdown` mode owns the comment body; a scoped `benchmarks.yml` workflow captures it via stdout and posts with `gh pr comment`. The counter gate is unchanged; the bench job moves out of `_checks.yml` so `pull-requests: write` stays scoped.
+---
+
+# Design: Surface the benchmark gate as a PR comment
+
+## Summary
+
+The benchmark gate (`just bench-check`) already computes a per-message sweep
+table on every PR, but its output dies in the CI log where no one reads it. This
+change surfaces that table as a **sticky PR comment** and on the run's Summary
+page, without touching the gate itself. Python gains a `benchmarks check
+--markdown` mode that renders the table plus a pass/fail verdict as a Markdown
+document; a new, permission-scoped `benchmarks.yml` workflow captures that
+document from stdout and posts it with `gh pr comment`. No third-party actions,
+no new metrics, no timing trend.
+
+## Motivation
+
+The prompt was "modern-di comments its benchmark results on PRs — can we do the
+same?" modern-di's mechanism
+(`benchmark-action/github-action-benchmark@v1`) does **not** transfer: it speaks
+pytest-benchmark *timing* JSON, keeps a weak cached-from-`main` baseline, and
+alerts on a 150% wall-clock threshold — exactly the timing-gate model
+[the harness design](2026-07-14.01-benchmark-harness.md) rejected as "a flake
+generator." Our gate is the stronger design: deterministic Postgres counters
+against a committed, reviewed `baseline.json`.
+
+So only the **PR comment** is worth copying, not the machinery behind it. Today
+`bench-check`'s `format_table` output — the sweep a reviewer would actually want
+when weighing a perf change — is visible only to whoever expands the CI log.
+
+## Design
+
+### `benchmarks check --markdown`
+
+`report.py` gains `format_markdown(results, failures)` beside the existing
+`format_table`. `__main__.py`'s `check` command takes a `--markdown` flag; when
+set, `_check` writes a Markdown document to stdout **instead of** the plain
+table, and returns the **same** exit code (0 pass / 1 fail) — the gate is
+untouched. The document:
+
+- an `## Benchmark gate` heading;
+- a verdict line — `✅ gate passed` or `❌ gate FAILED`, followed by the failure
+  bullets `compare()` already produces;
+- the sweep reflowed as a GitHub Markdown table (same columns as `format_table`);
+- a condensed gated-vs-informational footnote.
+
+Python owns the whole comment body, so it is unit-testable alongside the existing
+`format_table` test and the YAML never builds strings.
+
+### `benchmarks.yml`
+
+A new top-level workflow — **not** an edit to `_checks.yml`. `_checks.yml` is a
+reusable workflow `uses:`-called from `ci.yml`; granting it `pull-requests:
+write` would widen the token for every job in that file (lint, docs, pytest). A
+separate workflow scopes the write token to the one job that needs it (this is
+why modern-di is structured the same way).
+
+```yaml
+name: benchmarks
+on:
+  push: { branches: [main] }
+  pull_request: {}
+permissions:
+  contents: read
+  pull-requests: write
+jobs:
+  bench:
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+    steps:
+      - uses: actions/checkout@v6
+      - uses: extractions/setup-just@v4
+      - uses: astral-sh/setup-uv@v8.2.0
+      - run: uv lock                 # uv.lock is git-ignored; compose build needs it
+      - name: Run gate, capture markdown
+        shell: bash
+        run: |
+          set +e                       # own the exit code; -e would skip the post
+          just bench-report > "$RUNNER_TEMP/raw.log"
+          status=$?                    # the gate's real result
+          sed -n '/^## Benchmark gate/,$p' "$RUNNER_TEMP/raw.log" \
+            | tee "$GITHUB_STEP_SUMMARY" "$RUNNER_TEMP/bench-report.md"
+          exit "$status"               # re-fail the build on drift
+      - name: Comment on PR
+        if: always() && github.event_name == 'pull_request'
+        continue-on-error: true      # fork PRs get a read-only token; degrade quietly
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          gh pr comment "${{ github.event.pull_request.number }}" \
+            --edit-last --create-if-none \
+            --body-file "$RUNNER_TEMP/bench-report.md"
+```
+
+The `bench` job is **deleted from `_checks.yml`** and lands here (its `uv lock`
+step and 20-minute timeout carry over verbatim) so it does not run twice. `main`
+is unprotected with no required status checks, so moving the job breaks nothing.
+
+Two hazards, both hit on the first live PR and both handled above:
+
+1. **Compose leaks build noise onto stdout.** `-T` suppresses the TTY but does
+   **not** separate `docker compose run`'s buildkit image-build output, which in
+   CI lands on stdout — the first live comment was ~120 lines of `#8 extracting …`
+   before the table. So the capture does not trust stdout hygiene: it slices the
+   report from its `## Benchmark gate` sentinel to EOF. The container prints the
+   report last and nothing follows it on stdout (compose's post-`down` writes to
+   stderr), so heading-to-EOF is exactly the Markdown, independent of how compose
+   routes its own noise.
+2. **`GITHUB_STEP_SUMMARY` is per-step.** It is a **different path in every
+   step**, so a comment step reading it directly would find its own empty summary
+   and `gh pr comment` would fail on a blank body (silently, under
+   `continue-on-error`). The sliced report is teed to two sinks:
+   `$GITHUB_STEP_SUMMARY` (the run's Summary page, free and visible even on `push`
+   and fork PRs where no comment posts) and `$RUNNER_TEMP/bench-report.md`, the
+   file the comment step reads. `RUNNER_TEMP` persists across steps within the job
+   and sits outside the checked-out tree, so it leaves no stray workspace file.
+
+The exit code is owned by hand — `set +e`, capture the gate's status from the
+redirect, re-`exit` it after teeing — because piping through `sed`/`tee` would
+otherwise mask a failing gate, and because a failing gate must still post its
+`❌` report (the slice runs regardless) before failing the build.
+`--edit-last --create-if-none` makes the comment sticky (all flags verified
+present in the runner's preinstalled gh 2.96.0). A top-level `concurrency` group
+(`benchmarks-${{ github.head_ref || github.run_id }}`, `cancel-in-progress`)
+stops rapid PR pushes from racing two `--edit-last` posts.
+
+### `bench-report` recipe
+
+```
+# Gate + emit the markdown report to stdout (CI PR comment). -T keeps stdout clean.
+bench-report: down && down
+    docker compose run -T application uv run python -m benchmarks check --markdown
+```
+
+`-T` disables the pseudo-TTY so the Markdown carries no terminal control
+characters. It does **not** by itself guarantee clean stdout — see hazard 1
+above — which is why the workflow slices the report rather than trusting the
+recipe's stdout wholesale. `benchmarks check` still returns 1 on drift, and the
+workflow re-exits that status, so the gate keeps biting. `bench-check` stays
+as-is for local dev; `bench-report` is the CI entry point.
+
+**Capture, not a written file.** An earlier idea wrote the Markdown to a file in
+the `.:/code` bind mount. Rejected: the Dockerfile runs as `USER runner`, whose
+uid differs from the Linux runner's workspace owner, so that write can hit
+permission-denied in CI (it works on macOS only because Docker Desktop's FUSE
+mount ignores uid/gid). Capturing the container's stdout sidesteps the uid
+problem; the sentinel slice then handles compose's stdout noise.
+
+## Non-goals
+
+- **No `benchmark-action` / timing trend.** Wall-clock stays ungated; the design
+  doc's reasoning holds.
+- **No new metrics or gate change.** Same `EXACT_KEYS` / `TOLERANT_KEYS`, same
+  `baseline.json`, same exit codes.
+- **No `pull_request_target`.** It would let fork PRs comment but runs untrusted
+  PR code with a write token. With 0 forks and 40/40 same-repo PRs, the fork case
+  is theoretical; the comment step `continue-on-error`s and the Summary page
+  still shows the table.
+- **No weekly scheduled run.** `benchmarks.yml` triggers only on `push`-to-main
+  and `pull_request`, not on `scheduled.yml`'s Monday cron. Moving `bench` out of
+  the reusable `_checks.yml` (to scope the write token) means the weekly
+  dependency-drift check no longer runs the counter gate — a dep bump that
+  regresses the counters is caught on the next PR, not on Monday. Accepted
+  deliberately as the cost of the scoped token; re-add a `schedule` trigger here
+  if drift regressions start slipping through.
+
+## Testing
+
+- **Unit** (`tests/test_benchmarks.py`, no Postgres): `format_markdown` renders a
+  passing run with `✅` and a GitHub table; a failing run renders `❌` plus the
+  failure bullets. Beside the existing `test_format_table_*`.
+- **Local integration**: `just bench-report` prints clean Markdown to stdout and
+  exits 0 against the committed baseline.
+- **Gate still bites**: reuse the harness doc's method (extra `SELECT 1` per
+  message) → `bench-report` exits non-zero and the Markdown carries `❌`; revert.
+- `--cov-fail-under=100` undisturbed: `benchmarks/*` stays in `[tool.coverage.run]
+  omit`; new tests import the pure functions like the existing ones.
+- **Live comment post**: validated on this feature's own PR (#148). The first
+  attempt surfaced hazard 1 (compose build noise on stdout); after the sentinel
+  slice the sticky comment renders the clean table.
+
+## Risk
+
+- **Compose build noise on stdout** (materialized) — `docker compose run` puts
+  buildkit output on stdout in CI, so a naive capture posts a wall of build logs.
+  Mitigated by slicing from the `## Benchmark gate` sentinel to EOF, which is
+  independent of compose's stream routing.
+- **Masking the gate exit** (medium × high) — piping through `sed`/`tee`, or
+  Actions' default `bash -e`, would drop the gate's non-zero status. Mitigated by
+  owning the exit code by hand (`set +e`, capture from the stdout redirect,
+  re-`exit`) and asserted by the gate-still-bites test.
+- **Comment step failing the build on fork PRs** (low × low today) — mitigated by
+  `continue-on-error` + the Summary-page fallback.