Skip to content

Commit 883c390

Browse files
matejclaude
andauthored
Improve PR comments with GitHub suggestions (#6)
## Summary - Remove "Tool: ClaudeCode AI Review" line from comments - Add support for GitHub suggestion blocks when a code fix is available - Add `suggestion` field to finding schema in all prompts When the AI provides a `suggestion` field with exact replacement code, it will now render as a GitHub suggestion block that can be committed directly from the PR. ## Test plan - [x] Python tests pass (177) - [ ] Bun tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent 2706d17 commit 883c390

File tree

3 files changed

+177
-10
lines changed

3 files changed

+177
-10
lines changed

claudecode/prompts.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ def get_unified_review_prompt(
180180
"description": "What is wrong and where it happens",
181181
"impact": "Concrete impact or failure mode (use exploit scenario for security issues)",
182182
"recommendation": "Actionable fix or mitigation",
183+
"suggestion": "Exact replacement code (optional). Can be multi-line. Must replace lines from suggestion_start_line to suggestion_end_line.",
184+
"suggestion_start_line": 42,
185+
"suggestion_end_line": 44,
183186
"confidence": 0.95
184187
}}
185188
],
@@ -192,6 +195,12 @@ def get_unified_review_prompt(
192195
}}
193196
}}
194197
198+
SUGGESTION GUIDELINES:
199+
- Only include `suggestion` if you can provide exact, working replacement code
200+
- For single-line fixes: set suggestion_start_line = suggestion_end_line = the line number
201+
- For multi-line fixes: set the range of lines being replaced
202+
- The suggestion replaces all lines from suggestion_start_line to suggestion_end_line (inclusive)
203+
195204
SEVERITY GUIDELINES:
196205
- **HIGH**: Likely production bug, data loss, significant regression, or directly exploitable security vulnerability
197206
- **MEDIUM**: Real issue with limited scope or specific triggering conditions
@@ -343,6 +352,9 @@ def get_code_review_prompt(
343352
"description": "What is wrong and where it happens",
344353
"impact": "Concrete impact or failure mode",
345354
"recommendation": "Actionable fix or mitigation",
355+
"suggestion": "Exact replacement code (optional). Can be multi-line. Must replace lines from suggestion_start_line to suggestion_end_line.",
356+
"suggestion_start_line": 42,
357+
"suggestion_end_line": 44,
346358
"confidence": 0.95
347359
}}
348360
],
@@ -355,6 +367,12 @@ def get_code_review_prompt(
355367
}}
356368
}}
357369
370+
SUGGESTION GUIDELINES:
371+
- Only include `suggestion` if you can provide exact, working replacement code
372+
- For single-line fixes: set suggestion_start_line = suggestion_end_line = the line number
373+
- For multi-line fixes: set the range of lines being replaced
374+
- The suggestion replaces all lines from suggestion_start_line to suggestion_end_line (inclusive)
375+
358376
SEVERITY GUIDELINES:
359377
- **HIGH**: Likely production bug, data loss, or significant regression
360378
- **MEDIUM**: Real issue with limited scope or specific triggering conditions
@@ -506,6 +524,9 @@ def get_security_review_prompt(
506524
"description": "What is wrong and where it happens",
507525
"impact": "Exploit scenario or concrete impact",
508526
"recommendation": "Actionable fix or mitigation",
527+
"suggestion": "Exact replacement code (optional). Can be multi-line. Must replace lines from suggestion_start_line to suggestion_end_line.",
528+
"suggestion_start_line": 42,
529+
"suggestion_end_line": 44,
509530
"confidence": 0.95
510531
}}
511532
],
@@ -518,6 +539,12 @@ def get_security_review_prompt(
518539
}}
519540
}}
520541
542+
SUGGESTION GUIDELINES:
543+
- Only include `suggestion` if you can provide exact, working replacement code
544+
- For single-line fixes: set suggestion_start_line = suggestion_end_line = the line number
545+
- For multi-line fixes: set the range of lines being replaced
546+
- The suggestion replaces all lines from suggestion_start_line to suggestion_end_line (inclusive)
547+
521548
SEVERITY GUIDELINES:
522549
- **HIGH**: Directly exploitable vulnerabilities leading to RCE, data breach, or authentication bypass
523550
- **MEDIUM**: Vulnerabilities requiring specific conditions but with significant impact

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

Lines changed: 125 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,9 +488,133 @@ describe('comment-pr-findings.js', () => {
488488
expect(comment.body).toContain('🤖 **Code Review Finding:');
489489
expect(comment.body).toContain('**Severity:** HIGH');
490490
expect(comment.body).toContain('**Category:** security');
491-
expect(comment.body).toContain('**Tool:** ClaudeCode AI Review');
492491
expect(consoleLogSpy).toHaveBeenCalledWith('Created review with 1 inline comments');
493492
});
493+
494+
test('should include GitHub suggestion block when suggestion is provided', async () => {
495+
const mockFindings = [{
496+
file: 'test.py',
497+
line: 10,
498+
description: 'Unsafe pickle usage',
499+
severity: 'HIGH',
500+
category: 'security',
501+
recommendation: 'Use json.loads instead',
502+
suggestion: 'data = json.loads(user_input)'
503+
}];
504+
505+
const mockPrFiles = [{
506+
filename: 'test.py',
507+
patch: '@@ -10,1 +10,1 @@'
508+
}];
509+
510+
readFileSyncSpy.mockImplementation((path) => {
511+
if (path.includes('github-event.json')) {
512+
return JSON.stringify({
513+
pull_request: { number: 123, head: { sha: 'abc123' } }
514+
});
515+
}
516+
if (path === 'findings.json') {
517+
return JSON.stringify(mockFindings);
518+
}
519+
});
520+
521+
let capturedReviewData;
522+
spawnSyncSpy.mockImplementation((cmd, args, options) => {
523+
if (cmd === 'gh' && args.includes('api')) {
524+
const endpoint = args[1];
525+
const method = args[args.indexOf('--method') + 1] || 'GET';
526+
527+
if (endpoint.includes('/pulls/123/files')) {
528+
return { status: 0, stdout: JSON.stringify(mockPrFiles), stderr: '' };
529+
}
530+
if (endpoint.includes('/pulls/123/comments') && method === 'GET') {
531+
return { status: 0, stdout: '[]', stderr: '' };
532+
}
533+
if (endpoint.includes('/pulls/123/reviews') && method === 'POST') {
534+
if (options && options.input) {
535+
capturedReviewData = JSON.parse(options.input);
536+
}
537+
return { status: 0, stdout: '{}', stderr: '' };
538+
}
539+
return { status: 0, stdout: '{}', stderr: '' };
540+
}
541+
return { status: 0, stdout: '{}', stderr: '' };
542+
});
543+
544+
await import('./comment-pr-findings.js');
545+
546+
expect(capturedReviewData).toBeDefined();
547+
expect(capturedReviewData.comments).toHaveLength(1);
548+
549+
const comment = capturedReviewData.comments[0];
550+
expect(comment.body).toContain('```suggestion');
551+
expect(comment.body).toContain('data = json.loads(user_input)');
552+
expect(comment.body).toContain('```');
553+
});
554+
555+
test('should include start_line for multi-line suggestions', async () => {
556+
const mockFindings = [{
557+
file: 'test.py',
558+
line: 12,
559+
description: 'Unsafe database query',
560+
severity: 'HIGH',
561+
category: 'security',
562+
recommendation: 'Use parameterized queries',
563+
suggestion: 'cursor.execute(\n "SELECT * FROM users WHERE id = ?",\n (user_id,)\n)',
564+
suggestion_start_line: 10,
565+
suggestion_end_line: 12
566+
}];
567+
568+
const mockPrFiles = [{
569+
filename: 'test.py',
570+
patch: '@@ -10,3 +10,3 @@'
571+
}];
572+
573+
readFileSyncSpy.mockImplementation((path) => {
574+
if (path.includes('github-event.json')) {
575+
return JSON.stringify({
576+
pull_request: { number: 123, head: { sha: 'abc123' } }
577+
});
578+
}
579+
if (path === 'findings.json') {
580+
return JSON.stringify(mockFindings);
581+
}
582+
});
583+
584+
let capturedReviewData;
585+
spawnSyncSpy.mockImplementation((cmd, args, options) => {
586+
if (cmd === 'gh' && args.includes('api')) {
587+
const endpoint = args[1];
588+
const method = args[args.indexOf('--method') + 1] || 'GET';
589+
590+
if (endpoint.includes('/pulls/123/files')) {
591+
return { status: 0, stdout: JSON.stringify(mockPrFiles), stderr: '' };
592+
}
593+
if (endpoint.includes('/pulls/123/comments') && method === 'GET') {
594+
return { status: 0, stdout: '[]', stderr: '' };
595+
}
596+
if (endpoint.includes('/pulls/123/reviews') && method === 'POST') {
597+
if (options && options.input) {
598+
capturedReviewData = JSON.parse(options.input);
599+
}
600+
return { status: 0, stdout: '{}', stderr: '' };
601+
}
602+
return { status: 0, stdout: '{}', stderr: '' };
603+
}
604+
return { status: 0, stdout: '{}', stderr: '' };
605+
});
606+
607+
await import('./comment-pr-findings.js');
608+
609+
expect(capturedReviewData).toBeDefined();
610+
expect(capturedReviewData.comments).toHaveLength(1);
611+
612+
const comment = capturedReviewData.comments[0];
613+
expect(comment.body).toContain('```suggestion');
614+
expect(comment.start_line).toBe(10);
615+
expect(comment.line).toBe(12);
616+
expect(comment.start_side).toBe('RIGHT');
617+
});
494618
});
495619

496620
describe('Error Handling', () => {

scripts/comment-pr-findings.js

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,41 +133,57 @@ async function run() {
133133
const title = finding.title || message;
134134
const severity = finding.severity || 'HIGH';
135135
const category = finding.category || 'review_issue';
136-
136+
137137
// Check if this file is part of the PR diff
138138
if (!fileMap[file]) {
139139
console.log(`File ${file} not in PR diff, skipping`);
140140
continue;
141141
}
142-
142+
143143
// Build the comment body
144144
let commentBody = `🤖 **Code Review Finding: ${title}**\n\n`;
145145
commentBody += `**Severity:** ${severity}\n`;
146146
commentBody += `**Category:** ${category}\n`;
147-
commentBody += `**Tool:** ClaudeCode AI Review\n`;
148-
147+
149148
const extraMetadata = (finding.extra && finding.extra.metadata) || {};
150149

151150
// Add impact/exploit scenario if available
152151
if (finding.impact || finding.exploit_scenario || extraMetadata.impact || extraMetadata.exploit_scenario) {
153152
const impact = finding.impact || finding.exploit_scenario || extraMetadata.impact || extraMetadata.exploit_scenario;
154153
commentBody += `\n**Impact:** ${impact}\n`;
155154
}
156-
155+
157156
// Add recommendation if available
158-
if (finding.recommendation || extraMetadata.recommendation) {
159-
const recommendation = finding.recommendation || extraMetadata.recommendation;
157+
const recommendation = finding.recommendation || extraMetadata.recommendation;
158+
if (recommendation) {
160159
commentBody += `\n**Recommendation:** ${recommendation}\n`;
161160
}
162-
161+
162+
// Add GitHub suggestion block if a code suggestion is available
163+
const suggestion = finding.suggestion || extraMetadata.suggestion;
164+
if (suggestion) {
165+
commentBody += `\n\`\`\`suggestion\n${suggestion}\n\`\`\`\n`;
166+
}
167+
163168
// Prepare the review comment
164169
const reviewComment = {
165170
path: file,
166171
line: line,
167172
side: 'RIGHT',
168173
body: commentBody
169174
};
170-
175+
176+
// Handle multi-line suggestions by adding start_line
177+
const suggestionStartLine = finding.suggestion_start_line || extraMetadata.suggestion_start_line;
178+
const suggestionEndLine = finding.suggestion_end_line || extraMetadata.suggestion_end_line;
179+
180+
if (suggestion && suggestionStartLine && suggestionEndLine && suggestionStartLine !== suggestionEndLine) {
181+
// Multi-line suggestion: start_line is the first line, line is the last line
182+
reviewComment.start_line = suggestionStartLine;
183+
reviewComment.line = suggestionEndLine;
184+
reviewComment.start_side = 'RIGHT';
185+
}
186+
171187
reviewComments.push(reviewComment);
172188
}
173189

0 commit comments

Comments
 (0)