Skip to content

Commit f1d6781

Browse files
sayansisodiyasayansisodiya
authored
Merge pull request #3 from sourcegraph/sayans/leave-reviews
refactor: make CRA leave single review with multiple comments. Add re-review button in Checks tab
2 parents 0f8f6a5 + d8e7176 commit f1d6781

File tree

9 files changed

+392
-177
lines changed

9 files changed

+392
-177
lines changed

‎src/github/client.ts‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ export class GitHubClient {
113113
text?: string;
114114
};
115115
details_url?: string;
116+
actions?: Array<{
117+
label: string;
118+
description: string;
119+
identifier: string;
120+
}>;
116121
}
117122
): Promise<any> {
118123
const url = `${this.baseUrl}/repos/${owner}/${repo}/check-runs`;
@@ -124,6 +129,7 @@ export class GitHubClient {
124129
...(options.conclusion && { conclusion: options.conclusion }),
125130
...(options.output && { output: options.output }),
126131
...(options.details_url && { details_url: options.details_url }),
132+
...(options.actions && { actions: options.actions }),
127133
};
128134

129135
const response = await this.makeRequest(url, {
@@ -152,6 +158,11 @@ export class GitHubClient {
152158
text?: string;
153159
};
154160
details_url?: string;
161+
actions?: Array<{
162+
label: string;
163+
description: string;
164+
identifier: string;
165+
}>;
155166
}
156167
): Promise<any> {
157168
const url = `${this.baseUrl}/repos/${owner}/${repo}/check-runs/${checkRunId}`;
@@ -275,17 +286,38 @@ export class GitHubClient {
275286
}
276287

277288
async getPRComments(owner: string, repo: string, prNumber: number): Promise<any[]> {
278-
const url = `${this.baseUrl}/repos/${owner}/${repo}/issues/${prNumber}/comments`;
279-
280-
const response = await this.makeRequest(url, {
281-
method: 'GET',
282-
});
289+
// Fetch both issue comments (general discussion) and review comments (inline code comments)
290+
const [issueCommentsResponse, reviewCommentsResponse] = await Promise.all([
291+
// General PR discussion comments
292+
this.makeRequest(`${this.baseUrl}/repos/${owner}/${repo}/issues/${prNumber}/comments`, {
293+
method: 'GET',
294+
}),
295+
// Inline code review comments
296+
this.makeRequest(`${this.baseUrl}/repos/${owner}/${repo}/pulls/${prNumber}/comments`, {
297+
method: 'GET',
298+
})
299+
]);
300+
301+
if (!issueCommentsResponse.ok) {
302+
const errorText = await issueCommentsResponse.text();
303+
throw new Error(`GitHub API error (issue comments): ${issueCommentsResponse.status} ${issueCommentsResponse.statusText} - ${errorText}`);
304+
}
283305

284-
if (!response.ok) {
285-
const errorText = await response.text();
286-
throw new Error(`GitHub API error: ${response.status} ${response.statusText} - ${errorText}`);
306+
if (!reviewCommentsResponse.ok) {
307+
const errorText = await reviewCommentsResponse.text();
308+
throw new Error(`GitHub API error (review comments): ${reviewCommentsResponse.status} ${reviewCommentsResponse.statusText} - ${errorText}`);
287309
}
288310

289-
return await response.json();
311+
const issueComments = await issueCommentsResponse.json();
312+
const reviewComments = await reviewCommentsResponse.json();
313+
314+
// Combine both types with a type indicator
315+
const allComments = [
316+
...issueComments.map((comment: any) => ({ ...comment, comment_type: 'issue' })),
317+
...reviewComments.map((comment: any) => ({ ...comment, comment_type: 'review' }))
318+
];
319+
320+
// Sort by creation date
321+
return allComments.sort((a, b) => new Date(a.created_at).getTime() - new Date(b.created_at).getTime());
290322
}
291323
}

‎src/github/process-review.ts‎

Lines changed: 151 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -4,110 +4,170 @@ import { GitHubClient } from "./client.js";
44
import { CHECK_STATUS, CHECK_CONCLUSION, PRDetails, GitHubPullRequestEvent } from "./types.js";
55

66
export async function processReview(
7-
jobId: string,
8-
installationId: number,
9-
payload: GitHubPullRequestEvent
10-
): Promise<void> {
11-
const config = await getConfig();
12-
const githubClient = GitHubClient.forInstallation(config, installationId);
13-
14-
try {
15-
// Extract PR information
16-
const prNumber = payload.pull_request.number;
17-
const repositoryId = payload.repository.id;
18-
const commitSha = payload.pull_request.head.sha;
19-
const owner = payload.repository.owner.login;
20-
const repo = payload.repository.name;
21-
22-
if (!prNumber || !repositoryId || !commitSha) {
23-
const error = 'Missing required PR information';
24-
console.error(error);
25-
throw new Error(error);
26-
}
7+
jobId: string,
8+
installationId: number,
9+
payload: GitHubPullRequestEvent
10+
): Promise<void> {
11+
const config = await getConfig();
12+
const githubClient = GitHubClient.forInstallation(config, installationId);
2713

28-
// Generate PR URL
29-
const prUrl = payload.pull_request.html_url;
14+
try {
15+
// Extract PR information
16+
const prNumber = payload.pull_request.number;
17+
const repositoryId = payload.repository.id;
18+
const commitSha = payload.pull_request.head.sha;
19+
const owner = payload.repository.owner.login;
20+
const repo = payload.repository.name;
3021

31-
// Create check run
32-
const checkRun = await githubClient.createCheckRun(owner, repo, commitSha, {
33-
name: config.github.check_name,
34-
status: CHECK_STATUS.IN_PROGRESS,
35-
output: {
36-
title: 'Code Review',
37-
summary: 'Analyzing changes...',
38-
},
39-
details_url: prUrl,
40-
});
22+
if (!prNumber || !repositoryId || !commitSha) {
23+
const error = 'Missing required PR information';
24+
console.error(error);
25+
throw new Error(error);
26+
}
4127

42-
// Get diff content from GitHub API
43-
console.log('Fetching diff content from GitHub API...');
44-
const diffContent = await githubClient.getPRDiff(owner, repo, prNumber);
45-
46-
if (!diffContent) {
47-
const error = 'No diff content found';
48-
console.error(error);
49-
throw new Error(error);
50-
}
28+
// Generate PR URL
29+
const prUrl = payload.pull_request.html_url;
5130

52-
console.log(`Retrieved diff content (${diffContent.length} chars)`);
31+
// Create check run
32+
const checkRun = await githubClient.createCheckRun(owner, repo, commitSha, {
33+
name: config.github.check_name,
34+
status: CHECK_STATUS.IN_PROGRESS,
35+
output: {
36+
title: 'Code Review',
37+
summary: 'Analyzing changes...',
38+
},
39+
details_url: prUrl,
40+
});
5341

54-
// Create PR details object
55-
const prDetails: PRDetails = {
56-
pr_number: prNumber,
57-
repository_id: repositoryId,
58-
commit_sha: commitSha,
59-
pr_url: prUrl,
60-
};
42+
// Get diff content from GitHub API
43+
console.log('Fetching diff content from GitHub API...');
44+
const diffContent = await githubClient.getPRDiff(owner, repo, prNumber);
6145

62-
const prDetailsContent = `Repository: ${payload.repository.full_name}, PR Number: ${prDetails.pr_number}, Commit SHA: ${prDetails.commit_sha}, PR URL: ${prDetails.pr_url}`;
46+
if (!diffContent) {
47+
const error = 'No diff content found';
48+
console.error(error);
49+
throw new Error(error);
50+
}
6351

64-
console.log(`Calling reviewDiff() for job ${jobId}`);
65-
const reviewResult = await reviewDiff(diffContent, prDetailsContent, installationId);
66-
console.log(`Review completed for job ${jobId}`);
52+
console.log(`Retrieved diff content (${diffContent.length} chars)`);
6753

68-
// Update check run with success
69-
await githubClient.updateCheckRun(owner, repo, checkRun.id, {
70-
status: CHECK_STATUS.COMPLETED,
71-
conclusion: CHECK_CONCLUSION.SUCCESS,
72-
output: {
73-
title: 'Code Review Completed',
74-
summary: 'Code review has been completed successfully.',
75-
text: reviewResult.result || 'Review completed successfully.',
76-
},
77-
details_url: prUrl,
78-
});
54+
// Create PR details object
55+
const prDetails: PRDetails = {
56+
pr_number: prNumber,
57+
repository_id: repositoryId,
58+
commit_sha: commitSha,
59+
pr_url: prUrl,
60+
};
61+
62+
const prDetailsContent = `Repository: ${payload.repository.full_name}, PR Number: ${prDetails.pr_number}, Commit SHA: ${prDetails.commit_sha}, PR URL: ${prDetails.pr_url}`;
63+
64+
console.log(`Calling reviewDiff() for job ${jobId}`);
65+
const reviewResult = await reviewDiff(diffContent, prDetailsContent, installationId);
66+
console.log(`Review completed for job ${jobId}`);
67+
68+
// Read collected comments from file
69+
const fs = await import('fs');
70+
const commentsFilePath = reviewResult.commentsFilePath;
71+
72+
if (fs.existsSync(commentsFilePath)) {
73+
try {
74+
console.log(`📖 Reading collected comments from ${commentsFilePath}`);
75+
const fileContent = fs.readFileSync(commentsFilePath, 'utf8').trim();
76+
77+
if (fileContent) {
78+
const commentLines = fileContent.split('\n');
79+
const comments = commentLines.map(line => JSON.parse(line));
80+
81+
const inlineComments = comments.filter(c => c.type === 'inline');
82+
const generalComments = comments.filter(c => c.type === 'general');
83+
84+
// TODO(sayans): GitHub API allows <= 30 comments per review, so we need to add splitting logic if there are > 30 comments
7985

80-
} catch (error) {
81-
console.error(`Review job ${jobId} failed with exception:`, error);
82-
// Post FAILED check run for exceptions
83-
await postFailureCheckRun(githubClient, payload, error instanceof Error ? error.message : String(error));
86+
console.log(`📝 Collected ${inlineComments.length} inline comments and ${generalComments.length} general comments`);
87+
88+
// Create review summary from general comments
89+
const reviewSummary = generalComments.length > 0
90+
? generalComments.map(c => c.message).join('\n\n')
91+
: 'Code review completed.';
92+
93+
// Post aggregated review
94+
console.log('📋 Posting aggregated PR review...');
95+
await githubClient.createPRReview(
96+
owner,
97+
repo,
98+
prNumber,
99+
reviewSummary,
100+
'COMMENT',
101+
inlineComments.map(comment => ({
102+
path: comment.path,
103+
line: comment.line,
104+
body: comment.message
105+
}))
106+
);
107+
console.log('✅ PR review posted successfully');
108+
} else {
109+
console.log('📝 No comments collected, skipping review creation');
110+
}
111+
112+
// Clean up the comments file
113+
fs.unlinkSync(commentsFilePath);
114+
console.log('🗑️ Cleaned up comments file');
115+
116+
} catch (error) {
117+
console.error('❌ Failed to read or process comments file:', error);
118+
}
119+
} else {
120+
console.log('📝 No comments file found, skipping review creation');
121+
}
122+
123+
// Update check run with success (simplified since review details are now in PR review)
124+
await githubClient.updateCheckRun(owner, repo, checkRun.id, {
125+
status: CHECK_STATUS.COMPLETED,
126+
conclusion: CHECK_CONCLUSION.SUCCESS,
127+
output: {
128+
title: 'Code Review Completed',
129+
summary: 'Code review has been completed successfully. See PR conversation for details.',
130+
},
131+
details_url: prUrl,
132+
actions: [{
133+
label: 'Re-run review',
134+
description: 'Request a new code review for this PR',
135+
identifier: 're-run-review'
136+
}]
137+
});
138+
139+
} catch (error) {
140+
console.error(`Review job ${jobId} failed with exception:`, error);
141+
142+
// Post FAILED check run for exceptions
143+
await postFailureCheckRun(githubClient, payload, error instanceof Error ? error.message : String(error));
84144
}
85145
}
86146

87147
async function postFailureCheckRun(
88148
githubClient: GitHubClient,
89-
payload: GitHubPullRequestEvent,
90-
errorMessage: string
91-
): Promise<void> {
92-
const config = getConfig();
93-
94-
try {
95-
const commitSha = payload.pull_request.head.sha;
96-
const owner = payload.repository.owner.login;
97-
const repo = payload.repository.name;
98-
99-
if (commitSha && owner && repo) {
100-
await githubClient.createCheckRun(owner, repo, commitSha, {
101-
name: config.github.check_name,
102-
status: CHECK_STATUS.COMPLETED,
103-
conclusion: CHECK_CONCLUSION.FAILURE,
104-
output: {
105-
title: 'Code Review Failed',
106-
summary: `Review failed: ${errorMessage.substring(0, 100)}...`,
107-
},
108-
});
109-
}
110-
} catch (error) {
111-
console.error(`Failed to post failure check run:`, error);
149+
payload: GitHubPullRequestEvent,
150+
errorMessage: string
151+
): Promise<void> {
152+
const config = getConfig();
153+
154+
try {
155+
const commitSha = payload.pull_request.head.sha;
156+
const owner = payload.repository.owner.login;
157+
const repo = payload.repository.name;
158+
159+
if (commitSha && owner && repo) {
160+
await githubClient.createCheckRun(owner, repo, commitSha, {
161+
name: config.github.check_name,
162+
status: CHECK_STATUS.COMPLETED,
163+
conclusion: CHECK_CONCLUSION.FAILURE,
164+
output: {
165+
title: 'Code Review Failed',
166+
summary: `Review failed: ${errorMessage.substring(0, 100)}...`,
167+
},
168+
});
112169
}
170+
} catch (error) {
171+
console.error(`Failed to post failure check run:`, error);
172+
}
113173
}

0 commit comments

Comments
 (0)