From db11865e2901f51c9514770def8faacf796846c8 Mon Sep 17 00:00:00 2001 From: Matt Parrett Date: Sun, 9 Aug 2026 10:25:10 -0700 Subject: [PATCH 1/2] ci(perf-repeat): post the shadow-check report as a sticky PR comment The repeat A/B lane writes a job summary and uploads its evidence, but has no comment step, so a 65-minute run leaves nothing on the PR. On #706 the results had to be pulled out of the run artifact by hand to be read at all -- which means in practice the shadow check's output is only seen by whoever remembers to go looking. Mirrors perf-pr.yml's existing comment job rather than inventing a second pattern: separate job so the elevated token is scoped away from the one that builds PR code, sticky upsert by marker, continue-on-error for fork PRs whose token is read-only. Two deliberate choices: - The marker is `perf-pr-repeat-report`, distinct from perf-pr.yml's. Both lanes run on the same PR during the shadow phase and comparing them is the point, so neither may overwrite the other. - Gated on bench success. The bench job exits nonzero when families are not comparable across cycles; posting a delta table from a measurement already known to be broken is worse than posting nothing, and the summary still carries it. The comment body is the same text as the job summary, plus a link to the run for the per-snapshot evidence artifact. actionlint clean. Co-Authored-By: Claude Opus 5 --- .github/workflows/perf-pr-repeat.yml | 77 ++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/.github/workflows/perf-pr-repeat.yml b/.github/workflows/perf-pr-repeat.yml index 9176dfc8a..0e5c63132 100644 --- a/.github/workflows/perf-pr-repeat.yml +++ b/.github/workflows/perf-pr-repeat.yml @@ -54,6 +54,8 @@ jobs: # for a noisier runner, where the memory-bound families (and so the whole # suite) run slower. Right-size from observed runtime during the shadow phase. timeout-minutes: 90 + outputs: + pr: ${{ steps.refs.outputs.pr }} steps: - uses: actions/checkout@v4 with: @@ -86,8 +88,11 @@ jobs: git fetch --no-tags origin "$base_ref" "refs/pull/${PR}/head:pr-head" head_sha="$(git rev-parse pr-head)" base_sha="$(git merge-base "origin/${base_ref}" pr-head)" - echo "base=${base_sha}" >> "$GITHUB_OUTPUT" - echo "head=${head_sha}" >> "$GITHUB_OUTPUT" + { + echo "pr=${PR}" + echo "base=${base_sha}" + echo "head=${head_sha}" + } >> "$GITHUB_OUTPUT" echo "Base (merge-base): ${base_sha}" echo "Head: ${head_sha}" @@ -118,13 +123,20 @@ jobs: --base ../wt-base --head ../wt-head \ --n "$N" --budgets "$BUDGETS" --confirm-k 2 \ --out "${RUNNER_TEMP}/ab-out" | tee "${RUNNER_TEMP}/ab-report.txt" || rc=$? + # One body, two destinations: the job summary (always) and the PR + # comment artifact (consumed by the comment job below). { echo '## Repeat A/B (variance-reduced) — informational shadow check' echo echo '```' cat "${RUNNER_TEMP}/ab-report.txt" echo '```' - } >> "$GITHUB_STEP_SUMMARY" + echo + echo "Per-snapshot evidence — raw samples, cycle order, provenance — is in the \`perf-pr-repeat\` artifact on [this run](${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID})." + echo + echo '_Informational only. This check never gates the PR — the workflow header says why._' + } > "${RUNNER_TEMP}/comment.md" + cat "${RUNNER_TEMP}/comment.md" >> "$GITHUB_STEP_SUMMARY" exit "$rc" # Retain the full evidence trail (every base/head snapshot with raw samples, @@ -138,3 +150,62 @@ jobs: name: perf-pr-repeat path: ${{ runner.temp }}/ab-out/ if-no-files-found: warn + + - name: Upload PR comment body + if: always() + uses: actions/upload-artifact@v4 + with: + name: perf-pr-repeat-comment + path: ${{ runner.temp }}/comment.md + if-no-files-found: warn + + # Mirrors perf-pr.yml's comment job. Separate job so the elevated token is + # scoped away from the benchmark, which checks out and builds PR code. + # + # Gated on success deliberately: the bench job exits nonzero when families + # are not comparable across cycles, and posting a delta table built from a + # measurement we already know is broken would be worse than posting nothing. + # The job summary still carries the report in that case. + comment: + needs: bench + if: needs.bench.result == 'success' + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + issues: write + pull-requests: write + steps: + - name: Download PR comment artifact + uses: actions/download-artifact@v4 + with: + name: perf-pr-repeat-comment + path: ${{ runner.temp }}/perf-pr-repeat-comment + + # Sticky comment, upserted by marker. The marker differs from + # perf-pr.yml's on purpose: during the shadow phase both lanes run on the + # same PR and the whole point is comparing them, so neither may overwrite + # the other. Fork PRs get a read-only token, so this cannot post there — + # continue-on-error keeps the workflow green and the summary still has it. + - name: Upsert PR comment + continue-on-error: true + uses: actions/github-script@v7 + env: + PR: ${{ needs.bench.outputs.pr }} + COMMENT_PATH: ${{ runner.temp }}/perf-pr-repeat-comment/comment.md + with: + script: | + const fs = require('fs'); + const marker = ''; + const body = marker + '\n' + fs.readFileSync(process.env.COMMENT_PATH, 'utf8'); + const {owner, repo} = context.repo; + // Resolved in the refs step so it works for both pull_request and + // workflow_dispatch (where there's no pull_request in the payload). + const issue_number = Number(process.env.PR); + const {data: comments} = await github.rest.issues.listComments({owner, repo, issue_number}); + const existing = comments.find(c => c.body && c.body.includes(marker)); + if (existing) { + await github.rest.issues.updateComment({owner, repo, comment_id: existing.id, body}); + } else { + await github.rest.issues.createComment({owner, repo, issue_number, body}); + } From 8071361e7472c25ee65530e689c36f63f35e8d88 Mon Sep 17 00:00:00 2001 From: Matt Parrett Date: Sun, 9 Aug 2026 15:56:07 -0700 Subject: [PATCH 2/2] ci(perf-repeat): trim the PR comment to the aggregate The self-test on this PR posted the whole report: 120 lines, 9.6 KB, with seven literal `::group::` markers. Those are Actions log-folding directives and render as text in a comment body, so the useful part -- the family table and the would-gate verdict -- sat under a hundred lines of per-cycle bench output. The comment now starts after the last `::endgroup::`, which is where the aggregate begins: 36 lines, 1.8 KB, opening on the provenance header. The job summary is unchanged and still carries the per-cycle logs, and the comment points at it. Falls back to the full report if no group markers are found, so a format change or an early driver death posts something rather than nothing. Measured against the report this PR's own previous run produced. Co-Authored-By: Claude Opus 5 --- .github/workflows/perf-pr-repeat.yml | 29 +++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/.github/workflows/perf-pr-repeat.yml b/.github/workflows/perf-pr-repeat.yml index 0e5c63132..f385430d9 100644 --- a/.github/workflows/perf-pr-repeat.yml +++ b/.github/workflows/perf-pr-repeat.yml @@ -123,20 +123,39 @@ jobs: --base ../wt-base --head ../wt-head \ --n "$N" --budgets "$BUDGETS" --confirm-k 2 \ --out "${RUNNER_TEMP}/ab-out" | tee "${RUNNER_TEMP}/ab-report.txt" || rc=$? - # One body, two destinations: the job summary (always) and the PR - # comment artifact (consumed by the comment job below). + # The report is N ::group:: blocks of per-cycle bench output followed + # by the aggregate table. The job summary takes all of it. The PR + # comment takes the aggregate only: the per-cycle logs run to ~100 + # lines, and ::group:: is Actions log-folding syntax that renders as + # literal text in a comment body. + rpt="${RUNNER_TEMP}/ab-report.txt" + last_group="$(grep -n '::endgroup::' "$rpt" | tail -1 | cut -d: -f1 || true)" + if [ -n "$last_group" ]; then + tail -n +"$((last_group + 1))" "$rpt" > "${RUNNER_TEMP}/ab-summary.txt" + else + # No groups found — format changed, or the driver died before the + # first cycle. Fall back to the whole report rather than post an + # empty comment. + cp "$rpt" "${RUNNER_TEMP}/ab-summary.txt" + fi + { + echo '## Repeat A/B (variance-reduced) — informational shadow check' + echo + echo '```' + cat "$rpt" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" { echo '## Repeat A/B (variance-reduced) — informational shadow check' echo echo '```' - cat "${RUNNER_TEMP}/ab-report.txt" + cat "${RUNNER_TEMP}/ab-summary.txt" echo '```' echo - echo "Per-snapshot evidence — raw samples, cycle order, provenance — is in the \`perf-pr-repeat\` artifact on [this run](${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID})." + echo "Per-cycle logs are in the job summary. Per-snapshot evidence — raw samples, cycle order, provenance — is in the \`perf-pr-repeat\` artifact on [this run](${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID})." echo echo '_Informational only. This check never gates the PR — the workflow header says why._' } > "${RUNNER_TEMP}/comment.md" - cat "${RUNNER_TEMP}/comment.md" >> "$GITHUB_STEP_SUMMARY" exit "$rc" # Retain the full evidence trail (every base/head snapshot with raw samples,