From ef09d0b645aa41eebb92372f205fab5754ebe43a Mon Sep 17 00:00:00 2001 From: samuelburnham <45365069+samuelburnham@users.noreply.github.com> Date: Wed, 1 Jul 2026 19:39:13 +0000 Subject: [PATCH 1/3] ci: rename to bencher-thresholds-reset and ack the command on open PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match the workflow filename to the `!bencher-thresholds-reset` command it handles. The old `bench-thresholds-reset.yml` name didn't match the command, which made it easy to typo as `!benchmark-thresholds-reset` — a miss that silently reset nothing at merge. Keep the `bencher` spelling (not `benchmark`) so it can't collide with bench-pr.yml's `!benchmark` trigger. Add an `ack` job: when a maintainer comments the command on a still-open PR, reply confirming the baseline reset is queued for the merge commit, or warn if the workload is unknown/missing — immediate feedback instead of silence until merge. Bot-guarded so its own reply can't re-trigger it. The reset still runs only on merge (pull_request.merged == true), never on a non-merged close. --- .github/actions/bencher-track/action.yml | 2 +- ...reset.yml => bencher-thresholds-reset.yml} | 47 +++++++++++++++++-- 2 files changed, 45 insertions(+), 4 deletions(-) rename .github/workflows/{bench-thresholds-reset.yml => bencher-thresholds-reset.yml} (58%) diff --git a/.github/actions/bencher-track/action.yml b/.github/actions/bencher-track/action.yml index d37d179d..30c580c3 100644 --- a/.github/actions/bencher-track/action.yml +++ b/.github/actions/bencher-track/action.yml @@ -4,7 +4,7 @@ description: >- to the data points recorded since the workload's `baseline-reset-` tag, so a toolchain bump (or any permanent shift) is recalibrated by moving that one tag — via the !bencher-thresholds-reset PR comment or the - bench-thresholds-reset workflow. The calling job must check out with + bencher-thresholds-reset workflow. The calling job must check out with fetch-depth: 0 and fetch-tags: true (the tag is read here). inputs: diff --git a/.github/workflows/bench-thresholds-reset.yml b/.github/workflows/bencher-thresholds-reset.yml similarity index 58% rename from .github/workflows/bench-thresholds-reset.yml rename to .github/workflows/bencher-thresholds-reset.yml index 4f17e558..2f13007a 100644 --- a/.github/workflows/bench-thresholds-reset.yml +++ b/.github/workflows/bencher-thresholds-reset.yml @@ -8,12 +8,17 @@ name: Bencher thresholds reset # - Merged PR carrying a maintainer comment `!bencher-thresholds-reset `: # fires automatically on merge, anchored at the merge commit — no need to # race a manual run afterward. A bare command (no workload) is ignored. +# When the command is first commented on the still-open PR, the `ack` job +# replies to confirm the reset is queued for merge (and warns on an +# unknown/missing workload), so a typo is caught immediately instead of +# silently doing nothing at merge time. # - workflow_dispatch: reset the chosen workload at a given commit (default # HEAD); does not read PR contents. For ad-hoc or bootstrap resets. # -# pull_request_target runs in the base-repo context with a write token (works -# for fork PRs too). Safe here because the job runs no PR-provided code — only -# `gh api` calls against the trusted event payload. +# pull_request_target / issue_comment both run in the base-repo context with a +# write token (works for fork PRs too). Safe here because the jobs run no +# PR-provided code — only `gh api` calls against the trusted event payload, with +# the comment body used purely as data (never evaluated). on: workflow_dispatch: inputs: @@ -27,6 +32,8 @@ on: required: false pull_request_target: types: [closed] + issue_comment: + types: [created] permissions: contents: write @@ -85,3 +92,37 @@ jobs: [ "$EVENT" = pull_request_target ] && gh pr comment "$PR" --repo "$REPO" \ --body "♻️ Baseline reset to \`${sha:0:7}\` for:$done_list" true + + # Immediate acknowledgment: when a maintainer comments the reset command on a + # still-open PR, reply that the named baseline(s) will be reset on merge — or + # warn if the workload is unknown/missing. The `user.type != 'Bot'` guard keeps + # this reply (which may echo the command) from re-triggering the job. + ack: + if: github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.issue.state == 'open' && github.event.comment.user.type != 'Bot' && contains(github.event.comment.body, '!bencher-thresholds-reset') && (github.event.comment.author_association == 'OWNER' || github.event.comment.author_association == 'MEMBER' || github.event.comment.author_association == 'COLLABORATOR') + runs-on: ubuntu-latest + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR: ${{ github.event.issue.number }} + BODY: ${{ github.event.comment.body }} + steps: + - run: | + valid="ix-compile aiur" + # Same parse as the merge path: workload name(s) after the command, + # lowercased; `all` expands to every workload. + workloads=$(printf '%s' "$BODY" \ + | grep -oiE '!bencher-thresholds-reset[[:space:]]+[a-z0-9 -]+' \ + | sed -E 's/^!bencher-thresholds-reset[[:space:]]+//I' \ + | tr '[:upper:] ' '[:lower:]\n' | sort -u) + echo "$workloads" | grep -qx all && workloads="$valid" + ok="" + for w in $workloads; do + case " $valid " in *" $w "*) ok="$ok $w" ;; *) ;; esac + done + if [ -n "$ok" ]; then + gh pr comment "$PR" --repo "$REPO" \ + --body "♻️ Baseline reset queued for:$ok — will anchor to the merge commit when this PR merges." + else + gh pr comment "$PR" --repo "$REPO" \ + --body "⚠️ Reset command matched no known workload (expected \`ix-compile\`, \`aiur\`, or \`all\`). Nothing will reset on merge." + fi From 2d6108e182bdcd47a0bcfd88c81e534d6a44ad81 Mon Sep 17 00:00:00 2001 From: samuelburnham <45365069+samuelburnham@users.noreply.github.com> Date: Wed, 1 Jul 2026 19:54:52 +0000 Subject: [PATCH 2/3] ci: carry queued reset as a PR label instead of re-parsing on merge The ack job (comment time) is now the single place that parses and validates the !bencher-thresholds-reset command; it records each valid workload as a baseline-reset: label. The merge-time reset job reads those labels straight from the event payload instead of re-scanning and re-parsing PR comments, so the interpret logic lives in exactly one place. The label is also human-visible and can be removed to cancel a queued reset. Adds issues: write so ack can create/apply the label. --- .../workflows/bencher-thresholds-reset.yml | 51 ++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/.github/workflows/bencher-thresholds-reset.yml b/.github/workflows/bencher-thresholds-reset.yml index 2f13007a..b8671f6e 100644 --- a/.github/workflows/bencher-thresholds-reset.yml +++ b/.github/workflows/bencher-thresholds-reset.yml @@ -5,20 +5,29 @@ name: Bencher thresholds reset # Other workloads are unaffected; historical data stays in Bencher, only the # alert baseline is re-anchored. Two ways to trigger: # -# - Merged PR carrying a maintainer comment `!bencher-thresholds-reset `: -# fires automatically on merge, anchored at the merge commit — no need to -# race a manual run afterward. A bare command (no workload) is ignored. -# When the command is first commented on the still-open PR, the `ack` job -# replies to confirm the reset is queued for merge (and warns on an -# unknown/missing workload), so a typo is caught immediately instead of -# silently doing nothing at merge time. +# - Maintainer comment `!bencher-thresholds-reset ` on a PR: +# the `ack` job (issue_comment) is the single place that parses/validates the +# command. It records each valid workload as a `baseline-reset:` +# PR label and replies to confirm (or warns on an unknown/missing workload, +# catching a typo immediately). On merge, the `reset` job simply reads those +# labels from the event payload — no re-parsing — and anchors the tags at the +# merge commit. The label is the shared flag between the two events, and is +# human-visible/cancelable (remove the label to cancel a queued reset). +# - Manual label (no comment needed): the merge step honors any +# `baseline-reset:` label on the PR, whatever added it — so a +# Triage+ collaborator can queue a reset by applying the label directly, and +# cancel by removing it before merge. Naming convention: one label per +# workload, `baseline-reset:` where is one of the valid +# workloads (currently `ix-compile`, `aiur`); there is no `all` label — the +# comment path expands `all` into the per-workload labels. Labeling requires +# Triage+, so PR authors from forks cannot self-queue a reset. # - workflow_dispatch: reset the chosen workload at a given commit (default # HEAD); does not read PR contents. For ad-hoc or bootstrap resets. # # pull_request_target / issue_comment both run in the base-repo context with a # write token (works for fork PRs too). Safe here because the jobs run no -# PR-provided code — only `gh api` calls against the trusted event payload, with -# the comment body used purely as data (never evaluated). +# PR-provided code — only `gh` calls against the trusted event payload, with the +# comment body used purely as data (never evaluated). on: workflow_dispatch: inputs: @@ -38,6 +47,7 @@ on: permissions: contents: write pull-requests: write + issues: write # ack creates/applies the baseline-reset: label jobs: reset: @@ -60,16 +70,13 @@ jobs: sha="${INPUT_SHA:-$HEAD_SHA}" [ "$INPUT_WORKLOAD" = all ] && workloads="$valid" || workloads="$INPUT_WORKLOAD" else - # Reset workloads named in maintainer `!bencher-thresholds-reset` - # comments, anchored at the merge commit. Arg required, so a bare - # command matches nothing and is ignored. + # Reset the workloads the `ack` job recorded as `baseline-reset:*` + # labels (already parsed/validated at comment time), anchored at the + # merge commit. Read straight from the event payload — no re-parsing. sha="$MERGE_SHA" - workloads=$(gh api "repos/$REPO/issues/$PR/comments" --paginate \ - --jq '.[] | select(.author_association | IN("OWNER","MEMBER","COLLABORATOR")) | .body' \ - | grep -oiE '!bencher-thresholds-reset[[:space:]]+[a-z0-9 -]+' \ - | sed -E 's/^!bencher-thresholds-reset[[:space:]]+//I' \ - | tr '[:upper:] ' '[:lower:]\n' | sort -u) - echo "$workloads" | grep -qx all && workloads="$valid" + workloads=$(jq -r '.pull_request.labels[].name + | select(startswith("baseline-reset:")) | ltrimstr("baseline-reset:")' \ + "$GITHUB_EVENT_PATH") fi done_list="" @@ -120,6 +127,14 @@ jobs: case " $valid " in *" $w "*) ok="$ok $w" ;; *) ;; esac done if [ -n "$ok" ]; then + # Record each workload as a label — the flag the merge-time `reset` + # job reads. Create the label on first use (idempotent), then apply. + for w in $ok; do + label="baseline-reset:$w" + gh label create "$label" --repo "$REPO" --color BFD4F2 \ + --description "bencher baseline reset queued on merge" 2>/dev/null || true + gh pr edit "$PR" --repo "$REPO" --add-label "$label" + done gh pr comment "$PR" --repo "$REPO" \ --body "♻️ Baseline reset queued for:$ok — will anchor to the merge commit when this PR merges." else From 00027d24af6c7d718d27922c4bf209d5566fc110 Mon Sep 17 00:00:00 2001 From: samuelburnham <45365069+samuelburnham@users.noreply.github.com> Date: Wed, 1 Jul 2026 20:02:28 +0000 Subject: [PATCH 3/3] ci: unify tag and label under bencher-thresholds-reset; add an `all` label Rename the moved git tag baseline-reset- -> bencher-thresholds-reset- (and its reader in bencher-track/action.yml) so the command, workflow, label, and tag all share one name. Make `all` a first-class label: ack applies bencher-thresholds-reset:all verbatim, and the merge job is the single place that expands an `all` label into every workload. --- .github/actions/bencher-track/action.yml | 8 ++-- .github/workflows/bench-main.yml | 4 +- .../workflows/bencher-thresholds-reset.yml | 39 ++++++++++--------- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/.github/actions/bencher-track/action.yml b/.github/actions/bencher-track/action.yml index 30c580c3..498f19af 100644 --- a/.github/actions/bencher-track/action.yml +++ b/.github/actions/bencher-track/action.yml @@ -1,7 +1,7 @@ name: Track benchmarks on bencher description: >- Run `bencher run` for the `ix` project. Every measure's baseline is windowed - to the data points recorded since the workload's `baseline-reset-` + to the data points recorded since the workload's `bencher-thresholds-reset-` tag, so a toolchain bump (or any permanent shift) is recalibrated by moving that one tag — via the !bencher-thresholds-reset PR comment or the bencher-thresholds-reset workflow. The calling job must check out with @@ -12,7 +12,7 @@ inputs: description: Bencher testbed slug. required: true workload: - description: Workload key for the `baseline-reset-` tag (e.g. ix-compile, aiur). + description: Workload key for the `bencher-thresholds-reset-` tag (e.g. ix-compile, aiur). required: true file: description: Bencher Metric Format JSON file to upload. @@ -45,7 +45,7 @@ runs: THRESHOLDS: ${{ inputs.thresholds }} run: | # Baseline window: data points for this benchmark since the workload's - # `baseline-reset-` tag (moved on a toolchain bump or any + # `bencher-thresholds-reset-` tag (moved on a toolchain bump or any # permanent shift). The same window applies to every measure. We count # actual data points, not commits, so the baseline never reaches past # the anchor (a failed run produces no metric, so a commit count would @@ -54,7 +54,7 @@ runs: # project is public, so no token is needed. With no tag yet, fall back # to Bencher's 64-point window. sample=64 - if reset_epoch=$(git log -1 --format=%ct "baseline-reset-$WORKLOAD" 2>/dev/null); then + if reset_epoch=$(git log -1 --format=%ct "bencher-thresholds-reset-$WORKLOAD" 2>/dev/null); then bench=$(jq -r '[keys[]][0]' "$FILE") sample=$(curl -fsS "https://api.bencher.dev/v0/projects/ix/reports?branch=${GITHUB_REF_NAME}&testbed=${TESTBED}&start_time=$((reset_epoch * 1000))&per_page=255" \ | jq --arg b "$bench" '[.[] | select([.results[]?[]?.benchmark.name] | index($b))] | length') diff --git a/.github/workflows/bench-main.yml b/.github/workflows/bench-main.yml index 9b645594..957b51b3 100644 --- a/.github/workflows/bench-main.yml +++ b/.github/workflows/bench-main.yml @@ -81,7 +81,7 @@ jobs: - uses: actions/checkout@v6 with: fetch-depth: 0 - fetch-tags: true # bencher-track reads the baseline-reset tag + fetch-tags: true # bencher-track reads the bencher-thresholds-reset tag - uses: actions/cache/restore@v5 with: path: ~/.local/bin @@ -182,7 +182,7 @@ jobs: - uses: actions/checkout@v6 with: fetch-depth: 0 # full history for the baseline-anchor lookup - fetch-tags: true # bencher-track reads the baseline-reset tag + fetch-tags: true # bencher-track reads the bencher-thresholds-reset tag - uses: actions/cache/restore@v5 with: path: ~/.local/bin diff --git a/.github/workflows/bencher-thresholds-reset.yml b/.github/workflows/bencher-thresholds-reset.yml index b8671f6e..37a0213c 100644 --- a/.github/workflows/bencher-thresholds-reset.yml +++ b/.github/workflows/bencher-thresholds-reset.yml @@ -1,26 +1,27 @@ name: Bencher thresholds reset -# Move `baseline-reset-` tag(s) so bencher-track re-windows those +# Move `bencher-thresholds-reset-` tag(s) so bencher-track re-windows those # measures from a chosen commit — after a toolchain bump or any permanent shift. # Other workloads are unaffected; historical data stays in Bencher, only the # alert baseline is re-anchored. Two ways to trigger: # # - Maintainer comment `!bencher-thresholds-reset ` on a PR: # the `ack` job (issue_comment) is the single place that parses/validates the -# command. It records each valid workload as a `baseline-reset:` +# command. It records each valid workload as a `bencher-thresholds-reset:` # PR label and replies to confirm (or warns on an unknown/missing workload, # catching a typo immediately). On merge, the `reset` job simply reads those # labels from the event payload — no re-parsing — and anchors the tags at the # merge commit. The label is the shared flag between the two events, and is # human-visible/cancelable (remove the label to cancel a queued reset). # - Manual label (no comment needed): the merge step honors any -# `baseline-reset:` label on the PR, whatever added it — so a +# `bencher-thresholds-reset:` label on the PR, whatever added it — so a # Triage+ collaborator can queue a reset by applying the label directly, and -# cancel by removing it before merge. Naming convention: one label per -# workload, `baseline-reset:` where is one of the valid -# workloads (currently `ix-compile`, `aiur`); there is no `all` label — the -# comment path expands `all` into the per-workload labels. Labeling requires -# Triage+, so PR authors from forks cannot self-queue a reset. +# cancel by removing it before merge. Naming convention: one label per token, +# `bencher-thresholds-reset:` where is `ix-compile`, `aiur`, or +# `all` (the merge step expands an `all` label into every workload). Labeling +# requires Triage+, so PR authors from forks cannot self-queue a reset. The +# label shares the command/workflow name; the git tag it moves is the same +# stem with a dash: `bencher-thresholds-reset-`. # - workflow_dispatch: reset the chosen workload at a given commit (default # HEAD); does not read PR contents. For ad-hoc or bootstrap resets. # @@ -47,7 +48,7 @@ on: permissions: contents: write pull-requests: write - issues: write # ack creates/applies the baseline-reset: label + issues: write # ack creates/applies the bencher-thresholds-reset: label jobs: reset: @@ -70,19 +71,21 @@ jobs: sha="${INPUT_SHA:-$HEAD_SHA}" [ "$INPUT_WORKLOAD" = all ] && workloads="$valid" || workloads="$INPUT_WORKLOAD" else - # Reset the workloads the `ack` job recorded as `baseline-reset:*` + # Reset the workloads the `ack` job recorded as `bencher-thresholds-reset:*` # labels (already parsed/validated at comment time), anchored at the # merge commit. Read straight from the event payload — no re-parsing. + # A `bencher-thresholds-reset:all` label expands to every workload. sha="$MERGE_SHA" workloads=$(jq -r '.pull_request.labels[].name - | select(startswith("baseline-reset:")) | ltrimstr("baseline-reset:")' \ + | select(startswith("bencher-thresholds-reset:")) | ltrimstr("bencher-thresholds-reset:")' \ "$GITHUB_EVENT_PATH") + echo "$workloads" | grep -qx all && workloads="$valid" fi done_list="" for w in $workloads; do case " $valid " in *" $w "*) ;; *) echo "skip unknown workload: $w"; continue ;; esac - tag="baseline-reset-$w" + tag="bencher-thresholds-reset-$w" echo "Anchoring $tag -> $sha" # Update the tag if it exists, else create it. (Checking first avoids # the spurious 422 a PATCH-then-POST logs on first creation.) @@ -114,23 +117,23 @@ jobs: BODY: ${{ github.event.comment.body }} steps: - run: | - valid="ix-compile aiur" - # Same parse as the merge path: workload name(s) after the command, - # lowercased; `all` expands to every workload. + # Accepted command tokens — applied verbatim as labels (incl. `all`, + # which the merge job expands into every workload). + accepted="ix-compile aiur all" + # Parse the workload token(s) after the command, lowercased. workloads=$(printf '%s' "$BODY" \ | grep -oiE '!bencher-thresholds-reset[[:space:]]+[a-z0-9 -]+' \ | sed -E 's/^!bencher-thresholds-reset[[:space:]]+//I' \ | tr '[:upper:] ' '[:lower:]\n' | sort -u) - echo "$workloads" | grep -qx all && workloads="$valid" ok="" for w in $workloads; do - case " $valid " in *" $w "*) ok="$ok $w" ;; *) ;; esac + case " $accepted " in *" $w "*) ok="$ok $w" ;; *) ;; esac done if [ -n "$ok" ]; then # Record each workload as a label — the flag the merge-time `reset` # job reads. Create the label on first use (idempotent), then apply. for w in $ok; do - label="baseline-reset:$w" + label="bencher-thresholds-reset:$w" gh label create "$label" --repo "$REPO" --color BFD4F2 \ --description "bencher baseline reset queued on merge" 2>/dev/null || true gh pr edit "$PR" --repo "$REPO" --add-label "$label"