Skip to content

Commit 6b591a6

Browse files
matejclaude
andcommitted
Improve PR review summary to be concise
- Summarize issue types (e.g., "security and performance issues") - Single sentence with clear recommendation - No statistics or multiple sections Examples: - "Found 5 security and performance issues. Please address the high-severity issues before merging." - "Found 2 correctness issues. Consider addressing the suggestions in the comments." - "No issues found. Changes look good." Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 681ae06 commit 6b591a6

2 files changed

Lines changed: 33 additions & 22 deletions

File tree

scripts/comment-pr-findings.bun.test.js

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,7 @@ describe('comment-pr-findings.js', () => {
157157

158158
expect(reviewDataCaptured).toBeTruthy();
159159
expect(reviewDataCaptured.event).toBe('APPROVE');
160-
expect(reviewDataCaptured.body).toContain('Summary: No findings were reported.');
161-
expect(reviewDataCaptured.body).toContain('Assessment:');
160+
expect(reviewDataCaptured.body).toContain('No issues found');
162161
});
163162

164163
test('should process findings correctly', async () => {
@@ -231,8 +230,8 @@ describe('comment-pr-findings.js', () => {
231230
expect(reviewDataCaptured).toBeTruthy();
232231
expect(reviewDataCaptured.comments).toHaveLength(1);
233232
expect(reviewDataCaptured.event).toBe('REQUEST_CHANGES');
234-
expect(reviewDataCaptured.body).toContain('Summary: 1 finding');
235-
expect(reviewDataCaptured.body).toContain('Assessment:');
233+
expect(reviewDataCaptured.body).toContain('Found 1');
234+
expect(reviewDataCaptured.body).toContain('issue');
236235
});
237236

238237
test('should approve when findings are medium or low only', async () => {
@@ -299,8 +298,8 @@ describe('comment-pr-findings.js', () => {
299298

300299
expect(reviewDataCaptured).toBeTruthy();
301300
expect(reviewDataCaptured.event).toBe('APPROVE');
302-
expect(reviewDataCaptured.body).toContain('Summary: 2 findings');
303-
expect(reviewDataCaptured.body).toContain('HIGH: 0');
301+
expect(reviewDataCaptured.body).toContain('Found 2');
302+
expect(reviewDataCaptured.body).toContain('Consider addressing');
304303
expect(reviewDataCaptured.comments).toHaveLength(2);
305304
});
306305

@@ -350,7 +349,7 @@ describe('comment-pr-findings.js', () => {
350349

351350
expect(reviewDataCaptured).toBeTruthy();
352351
expect(reviewDataCaptured.event).toBe('REQUEST_CHANGES');
353-
expect(reviewDataCaptured.body).toContain('Summary: 1 finding');
352+
expect(reviewDataCaptured.body).toContain('Found 1');
354353
expect(reviewDataCaptured.comments).toBeUndefined();
355354
});
356355
});

scripts/comment-pr-findings.js

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -125,35 +125,47 @@ async function run() {
125125

126126
function buildReviewSummary(findings) {
127127
const total = findings.length;
128-
const files = new Set();
128+
const categories = {};
129+
129130
for (const finding of findings) {
130-
const file = finding.file || finding.path;
131-
if (file) {
132-
files.add(file);
131+
const category = finding.category || 'other';
132+
if (!categories[category]) {
133+
categories[category] = [];
133134
}
135+
categories[category].push(finding);
134136
}
135137

136138
const { high, medium, low } = countSeverities(findings);
137-
let summaryLine;
138139

139140
if (total === 0) {
140-
summaryLine = 'Summary: No findings were reported.';
141+
return 'No issues found. Changes look good.';
142+
}
143+
144+
// Build concise category summary
145+
const categoryNames = Object.keys(categories).map(c => c.toLowerCase());
146+
let issueTypes;
147+
if (categoryNames.length === 1) {
148+
issueTypes = categoryNames[0];
149+
} else if (categoryNames.length === 2) {
150+
issueTypes = categoryNames.join(' and ');
141151
} else {
142-
const fileCount = files.size;
143-
const fileText = fileCount > 0 ? ` across ${fileCount} file${fileCount === 1 ? '' : 's'}` : '';
144-
summaryLine = `Summary: ${total} finding${total === 1 ? '' : 's'}${fileText} (HIGH: ${high}, MEDIUM: ${medium}, LOW: ${low}).`;
152+
const last = categoryNames.pop();
153+
issueTypes = categoryNames.join(', ') + ', and ' + last;
145154
}
146155

147-
let assessmentLine;
156+
// Build the summary
157+
let summary = `Found ${total} ${issueTypes} issue${total === 1 ? '' : 's'}. `;
158+
159+
// Recommendation
148160
if (high > 0) {
149-
assessmentLine = 'Assessment: High-severity issues were found that should be addressed before merge.';
150-
} else if (total > 0) {
151-
assessmentLine = 'Assessment: Changes look acceptable overall; consider addressing the findings.';
161+
summary += 'Please address the high-severity issues before merging.';
162+
} else if (medium > 0) {
163+
summary += 'Consider addressing the suggestions in the comments.';
152164
} else {
153-
assessmentLine = 'Assessment: Changes look good; no high-severity issues detected.';
165+
summary += 'Minor suggestions noted in comments.';
154166
}
155167

156-
return `${summaryLine}\n${assessmentLine}`;
168+
return summary;
157169
}
158170

159171
const { high: highSeverityCount } = countSeverities(newFindings);

0 commit comments

Comments
 (0)