Skip to content

Commit c1dac8a

Browse files
jcputneyjcputney
committed
Move PR benchmark commenting to separate workflow
Extracts the logic for commenting benchmark results on pull requests from benchmark.yml into a new workflow benchmark-comment.yml. This improves workflow organization and ensures benchmark comments are only posted for completed PR runs.
1 parent dd54cd5 commit c1dac8a

2 files changed

Lines changed: 100 additions & 68 deletions

File tree

‎.github/workflows/benchmark-comment.yml‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
name: Benchmarks PR Comment
2+
3+
on:
4+
workflow_run:
5+
workflows: ["Benchmarks"]
6+
types: [completed]
7+
8+
jobs:
9+
comment:
10+
if: ${{ github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion != 'cancelled' && github.event.workflow_run.conclusion != 'skipped' && github.event.workflow_run.pull_requests[0] != null }}
11+
runs-on: ubuntu-latest
12+
permissions:
13+
contents: read
14+
issues: write
15+
pull-requests: write
16+
steps:
17+
- name: Download benchmark artifact
18+
uses: actions/download-artifact@v4
19+
with:
20+
name: benchmark-results
21+
path: benchmark
22+
run-id: ${{ github.event.workflow_run.id }}
23+
github-token: ${{ secrets.GITHUB_TOKEN }}
24+
if-no-files-found: warn
25+
26+
- name: Comment PR with benchmark results
27+
uses: actions/github-script@v8
28+
with:
29+
script: |
30+
const fs = require('fs');
31+
const path = require('path');
32+
33+
const pr = context.payload.workflow_run.pull_requests[0];
34+
if (!pr) {
35+
core.info('No associated pull request; skipping comment.');
36+
return;
37+
}
38+
39+
const resultsPath = path.join(process.cwd(), 'benchmark', 'benchmark-results.json');
40+
if (!fs.existsSync(resultsPath)) {
41+
core.info('benchmark-results.json not found in artifact; skipping comment.');
42+
return;
43+
}
44+
45+
const results = JSON.parse(fs.readFileSync(resultsPath, 'utf8'));
46+
const runUrl = context.payload.workflow_run.html_url;
47+
48+
let comment = '## 📊 Benchmark Results\\n\\n';
49+
comment += `Triggered by [workflow run](${runUrl}).\\n\\n`;
50+
comment += '| Benchmark | Score | Error | Unit |\\n';
51+
comment += '|-----------|-------|-------|------|\\n';
52+
53+
let hasRegression = false;
54+
55+
results.forEach(result => {
56+
const score = result.primaryMetric.score;
57+
const error = result.primaryMetric.scoreError;
58+
const unit = result.primaryMetric.scoreUnit;
59+
const benchmark = result.benchmark.split('.').pop();
60+
61+
const emoji = score > 50 ? '🔴' : '🟢';
62+
if (score > 50) hasRegression = true;
63+
64+
comment += `| ${emoji} ${benchmark} | ${score.toFixed(3)} | ±${error.toFixed(3)} | ${unit} |\\n`;
65+
});
66+
67+
if (hasRegression) {
68+
comment += '\\n⚠️ **Performance regression detected**: One or more benchmarks exceeded the 50ms threshold.\\n';
69+
} else {
70+
comment += '\\n✅ **All benchmarks passed** performance thresholds.\\n';
71+
}
72+
73+
const { owner, repo } = context.repo;
74+
const issue_number = pr.number;
75+
76+
const { data: comments } = await github.rest.issues.listComments({
77+
owner,
78+
repo,
79+
issue_number,
80+
});
81+
82+
const existing = comments.find(comment =>
83+
comment.user.type === 'Bot' && comment.body.includes('📊 Benchmark Results')
84+
);
85+
86+
if (existing) {
87+
await github.rest.issues.updateComment({
88+
owner,
89+
repo,
90+
comment_id: existing.id,
91+
body: comment,
92+
});
93+
} else {
94+
await github.rest.issues.createComment({
95+
owner,
96+
repo,
97+
issue_number,
98+
body: comment,
99+
});
100+
}

‎.github/workflows/benchmark.yml‎

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ jobs:
1010
benchmark:
1111
permissions:
1212
contents: read
13-
issues: write
14-
pull-requests: write
1513
runs-on: ubuntu-latest
1614

1715
steps:
@@ -73,69 +71,3 @@ jobs:
7371
name: benchmark-results
7472
path: benchmark-results.json
7573
retention-days: 30
76-
77-
- name: Comment PR with benchmark results
78-
if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository
79-
uses: actions/github-script@v8
80-
with:
81-
script: |
82-
const fs = require('fs');
83-
84-
if (!fs.existsSync('benchmark-results.json')) {
85-
console.log('No benchmark results found');
86-
return;
87-
}
88-
89-
const results = JSON.parse(fs.readFileSync('benchmark-results.json', 'utf8'));
90-
91-
let comment = '## 📊 Benchmark Results\n\n';
92-
comment += '| Benchmark | Score | Error | Unit |\n';
93-
comment += '|-----------|-------|-------|------|\n';
94-
95-
let hasRegression = false;
96-
97-
results.forEach(result => {
98-
const score = result.primaryMetric.score;
99-
const error = result.primaryMetric.scoreError;
100-
const unit = result.primaryMetric.scoreUnit;
101-
const benchmark = result.benchmark.split('.').pop();
102-
103-
// Check if score exceeds threshold
104-
const emoji = score > 50 ? '🔴' : '🟢';
105-
if (score > 50) hasRegression = true;
106-
107-
comment += `| ${emoji} ${benchmark} | ${score.toFixed(3)} | ±${error.toFixed(3)} | ${unit} |\n`;
108-
});
109-
110-
if (hasRegression) {
111-
comment += '\n⚠️ **Performance regression detected**: One or more benchmarks exceeded the 50ms threshold.\n';
112-
} else {
113-
comment += '\n✅ **All benchmarks passed** performance thresholds.\n';
114-
}
115-
116-
// Find and update or create comment
117-
const { data: comments } = await github.rest.issues.listComments({
118-
owner: context.repo.owner,
119-
repo: context.repo.repo,
120-
issue_number: context.issue.number,
121-
});
122-
123-
const botComment = comments.find(comment =>
124-
comment.user.type === 'Bot' && comment.body.includes('📊 Benchmark Results')
125-
);
126-
127-
if (botComment) {
128-
await github.rest.issues.updateComment({
129-
owner: context.repo.owner,
130-
repo: context.repo.repo,
131-
comment_id: botComment.id,
132-
body: comment
133-
});
134-
} else {
135-
await github.rest.issues.createComment({
136-
owner: context.repo.owner,
137-
repo: context.repo.repo,
138-
issue_number: context.issue.number,
139-
body: comment
140-
});
141-
}

0 commit comments

Comments
 (0)