Skip to content

Commit 43564f6

Browse files
ci: Fix !bencher-thresholds-reset PR comment (#466)
* ci: rename to bencher-thresholds-reset and ack the command on open PRs 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. * 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:<workload> 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. * ci: unify tag and label under bencher-thresholds-reset; add an `all` label Rename the moved git tag baseline-reset-<workload> -> bencher-thresholds-reset-<workload> (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.
1 parent 305fa9b commit 43564f6

4 files changed

Lines changed: 153 additions & 94 deletions

File tree

.github/actions/bencher-track/action.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
name: Track benchmarks on bencher
22
description: >-
33
Run `bencher run` for the `ix` project. Every measure's baseline is windowed
4-
to the data points recorded since the workload's `baseline-reset-<workload>`
4+
to the data points recorded since the workload's `bencher-thresholds-reset-<workload>`
55
tag, so a toolchain bump (or any permanent shift) is recalibrated by moving
66
that one tag — via the !bencher-thresholds-reset PR comment or the
7-
bench-thresholds-reset workflow. The calling job must check out with
7+
bencher-thresholds-reset workflow. The calling job must check out with
88
fetch-depth: 0 and fetch-tags: true (the tag is read here).
99
1010
inputs:
1111
testbed:
1212
description: Bencher testbed slug.
1313
required: true
1414
workload:
15-
description: Workload key for the `baseline-reset-<workload>` tag (e.g. ix-compile, aiur).
15+
description: Workload key for the `bencher-thresholds-reset-<workload>` tag (e.g. ix-compile, aiur).
1616
required: true
1717
file:
1818
description: Bencher Metric Format JSON file to upload.
@@ -45,7 +45,7 @@ runs:
4545
THRESHOLDS: ${{ inputs.thresholds }}
4646
run: |
4747
# Baseline window: data points for this benchmark since the workload's
48-
# `baseline-reset-<workload>` tag (moved on a toolchain bump or any
48+
# `bencher-thresholds-reset-<workload>` tag (moved on a toolchain bump or any
4949
# permanent shift). The same window applies to every measure. We count
5050
# actual data points, not commits, so the baseline never reaches past
5151
# the anchor (a failed run produces no metric, so a commit count would
@@ -54,7 +54,7 @@ runs:
5454
# project is public, so no token is needed. With no tag yet, fall back
5555
# to Bencher's 64-point window.
5656
sample=64
57-
if reset_epoch=$(git log -1 --format=%ct "baseline-reset-$WORKLOAD" 2>/dev/null); then
57+
if reset_epoch=$(git log -1 --format=%ct "bencher-thresholds-reset-$WORKLOAD" 2>/dev/null); then
5858
bench=$(jq -r '[keys[]][0]' "$FILE")
5959
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" \
6060
| jq --arg b "$bench" '[.[] | select([.results[]?[]?.benchmark.name] | index($b))] | length')

.github/workflows/bench-main.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ jobs:
8181
- uses: actions/checkout@v6
8282
with:
8383
fetch-depth: 0
84-
fetch-tags: true # bencher-track reads the baseline-reset tag
84+
fetch-tags: true # bencher-track reads the bencher-thresholds-reset tag
8585
- uses: actions/cache/restore@v5
8686
with:
8787
path: ~/.local/bin
@@ -182,7 +182,7 @@ jobs:
182182
- uses: actions/checkout@v6
183183
with:
184184
fetch-depth: 0 # full history for the baseline-anchor lookup
185-
fetch-tags: true # bencher-track reads the baseline-reset tag
185+
fetch-tags: true # bencher-track reads the bencher-thresholds-reset tag
186186
- uses: actions/cache/restore@v5
187187
with:
188188
path: ~/.local/bin

.github/workflows/bench-thresholds-reset.yml

Lines changed: 0 additions & 87 deletions
This file was deleted.
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
name: Bencher thresholds reset
2+
3+
# Move `bencher-thresholds-reset-<workload>` tag(s) so bencher-track re-windows those
4+
# measures from a chosen commit — after a toolchain bump or any permanent shift.
5+
# Other workloads are unaffected; historical data stays in Bencher, only the
6+
# alert baseline is re-anchored. Two ways to trigger:
7+
#
8+
# - Maintainer comment `!bencher-thresholds-reset <workload|all>` on a PR:
9+
# the `ack` job (issue_comment) is the single place that parses/validates the
10+
# command. It records each valid workload as a `bencher-thresholds-reset:<workload>`
11+
# PR label and replies to confirm (or warns on an unknown/missing workload,
12+
# catching a typo immediately). On merge, the `reset` job simply reads those
13+
# labels from the event payload — no re-parsing — and anchors the tags at the
14+
# merge commit. The label is the shared flag between the two events, and is
15+
# human-visible/cancelable (remove the label to cancel a queued reset).
16+
# - Manual label (no comment needed): the merge step honors any
17+
# `bencher-thresholds-reset:<token>` label on the PR, whatever added it — so a
18+
# Triage+ collaborator can queue a reset by applying the label directly, and
19+
# cancel by removing it before merge. Naming convention: one label per token,
20+
# `bencher-thresholds-reset:<token>` where <token> is `ix-compile`, `aiur`, or
21+
# `all` (the merge step expands an `all` label into every workload). Labeling
22+
# requires Triage+, so PR authors from forks cannot self-queue a reset. The
23+
# label shares the command/workflow name; the git tag it moves is the same
24+
# stem with a dash: `bencher-thresholds-reset-<workload>`.
25+
# - workflow_dispatch: reset the chosen workload at a given commit (default
26+
# HEAD); does not read PR contents. For ad-hoc or bootstrap resets.
27+
#
28+
# pull_request_target / issue_comment both run in the base-repo context with a
29+
# write token (works for fork PRs too). Safe here because the jobs run no
30+
# PR-provided code — only `gh` calls against the trusted event payload, with the
31+
# comment body used purely as data (never evaluated).
32+
on:
33+
workflow_dispatch:
34+
inputs:
35+
workload:
36+
description: Workload baseline to reset
37+
required: true
38+
type: choice
39+
options: [ix-compile, aiur, all]
40+
sha:
41+
description: "Commit to anchor to (default: HEAD)"
42+
required: false
43+
pull_request_target:
44+
types: [closed]
45+
issue_comment:
46+
types: [created]
47+
48+
permissions:
49+
contents: write
50+
pull-requests: write
51+
issues: write # ack creates/applies the bencher-thresholds-reset:<workload> label
52+
53+
jobs:
54+
reset:
55+
if: github.event_name == 'workflow_dispatch' || github.event.pull_request.merged == true
56+
runs-on: ubuntu-latest
57+
env:
58+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
59+
REPO: ${{ github.repository }}
60+
EVENT: ${{ github.event_name }}
61+
INPUT_WORKLOAD: ${{ inputs.workload }}
62+
INPUT_SHA: ${{ inputs.sha }}
63+
HEAD_SHA: ${{ github.sha }}
64+
PR: ${{ github.event.pull_request.number }}
65+
MERGE_SHA: ${{ github.event.pull_request.merge_commit_sha }}
66+
steps:
67+
- run: |
68+
valid="ix-compile aiur"
69+
if [ "$EVENT" = workflow_dispatch ]; then
70+
# Reset the chosen workload(s) at the given commit; no PR scan.
71+
sha="${INPUT_SHA:-$HEAD_SHA}"
72+
[ "$INPUT_WORKLOAD" = all ] && workloads="$valid" || workloads="$INPUT_WORKLOAD"
73+
else
74+
# Reset the workloads the `ack` job recorded as `bencher-thresholds-reset:*`
75+
# labels (already parsed/validated at comment time), anchored at the
76+
# merge commit. Read straight from the event payload — no re-parsing.
77+
# A `bencher-thresholds-reset:all` label expands to every workload.
78+
sha="$MERGE_SHA"
79+
workloads=$(jq -r '.pull_request.labels[].name
80+
| select(startswith("bencher-thresholds-reset:")) | ltrimstr("bencher-thresholds-reset:")' \
81+
"$GITHUB_EVENT_PATH")
82+
echo "$workloads" | grep -qx all && workloads="$valid"
83+
fi
84+
85+
done_list=""
86+
for w in $workloads; do
87+
case " $valid " in *" $w "*) ;; *) echo "skip unknown workload: $w"; continue ;; esac
88+
tag="bencher-thresholds-reset-$w"
89+
echo "Anchoring $tag -> $sha"
90+
# Update the tag if it exists, else create it. (Checking first avoids
91+
# the spurious 422 a PATCH-then-POST logs on first creation.)
92+
if gh api --silent "repos/$REPO/git/refs/tags/$tag" 2>/dev/null; then
93+
gh api --silent -X PATCH "repos/$REPO/git/refs/tags/$tag" -f sha="$sha" -F force=true
94+
else
95+
gh api --silent -X POST "repos/$REPO/git/refs" -f ref="refs/tags/$tag" -f sha="$sha"
96+
fi
97+
done_list="$done_list $w"
98+
done
99+
[ -z "$done_list" ] && { echo "Nothing to reset."; exit 0; }
100+
101+
# Confirm on the PR for the merge-triggered path.
102+
[ "$EVENT" = pull_request_target ] && gh pr comment "$PR" --repo "$REPO" \
103+
--body "♻️ Baseline reset to \`${sha:0:7}\` for:$done_list"
104+
true
105+
106+
# Immediate acknowledgment: when a maintainer comments the reset command on a
107+
# still-open PR, reply that the named baseline(s) will be reset on merge — or
108+
# warn if the workload is unknown/missing. The `user.type != 'Bot'` guard keeps
109+
# this reply (which may echo the command) from re-triggering the job.
110+
ack:
111+
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')
112+
runs-on: ubuntu-latest
113+
env:
114+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
115+
REPO: ${{ github.repository }}
116+
PR: ${{ github.event.issue.number }}
117+
BODY: ${{ github.event.comment.body }}
118+
steps:
119+
- run: |
120+
# Accepted command tokens — applied verbatim as labels (incl. `all`,
121+
# which the merge job expands into every workload).
122+
accepted="ix-compile aiur all"
123+
# Parse the workload token(s) after the command, lowercased.
124+
workloads=$(printf '%s' "$BODY" \
125+
| grep -oiE '!bencher-thresholds-reset[[:space:]]+[a-z0-9 -]+' \
126+
| sed -E 's/^!bencher-thresholds-reset[[:space:]]+//I' \
127+
| tr '[:upper:] ' '[:lower:]\n' | sort -u)
128+
ok=""
129+
for w in $workloads; do
130+
case " $accepted " in *" $w "*) ok="$ok $w" ;; *) ;; esac
131+
done
132+
if [ -n "$ok" ]; then
133+
# Record each workload as a label — the flag the merge-time `reset`
134+
# job reads. Create the label on first use (idempotent), then apply.
135+
for w in $ok; do
136+
label="bencher-thresholds-reset:$w"
137+
gh label create "$label" --repo "$REPO" --color BFD4F2 \
138+
--description "bencher baseline reset queued on merge" 2>/dev/null || true
139+
gh pr edit "$PR" --repo "$REPO" --add-label "$label"
140+
done
141+
gh pr comment "$PR" --repo "$REPO" \
142+
--body "♻️ Baseline reset queued for:$ok — will anchor to the merge commit when this PR merges."
143+
else
144+
gh pr comment "$PR" --repo "$REPO" \
145+
--body "⚠️ Reset command matched no known workload (expected \`ix-compile\`, \`aiur\`, or \`all\`). Nothing will reset on merge."
146+
fi

0 commit comments

Comments
 (0)