Skip to content

Commit c9af121

Browse files
karlclementkarlclement
committed
Merge branch 'karl/general-review'
2 parents 86e4b19 + a694124 commit c9af121

File tree

2 files changed

+75
-81
lines changed

2 files changed

+75
-81
lines changed

‎src/github/process-review.ts‎

Lines changed: 54 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { getConfig } from "../config.js";
22
import { reviewDiff } from "../review/reviewer.js";
33
import { GitHubClient } from "./client.js";
4-
import { CHECK_STATUS, CHECK_CONCLUSION, PRContext, GitHubPullRequestEvent } from "./types.js";
4+
import { CHECK_STATUS, CHECK_CONCLUSION, GitHubPullRequestEvent } from "./types.js";
55

66
export async function processReview(
77
jobId: string,
@@ -78,75 +78,70 @@ export async function processReview(
7878

7979
console.log(`Retrieved diff content (${diffContent.length} chars)`);
8080

81-
// Create structured PR context object
82-
const prContext: PRContext = {
83-
owner,
84-
repo,
85-
pr_number: prNumber,
86-
repository_id: repositoryId,
87-
commit_sha: commitSha,
88-
pr_url: prUrl,
89-
repository_full_name: payload.repository.full_name,
90-
};
81+
// Context for agent to find PR
82+
const prContent = `Repository: ${payload.repository.full_name}\nPR Number: ${prNumber}\nCommit SHA: ${commitSha}\nPR URL: ${prUrl}`;
9183

9284
console.log(`Calling reviewDiff() for job ${jobId}`);
93-
const reviewResult = await reviewDiff(diffContent, prContext, installationId);
85+
const reviewResult = await reviewDiff(diffContent, prContent, installationId.toString(), {
86+
GITHUB_INSTALLATION_ID: installationId.toString(),
87+
GITHUB_OWNER: owner,
88+
GITHUB_REPO: repo,
89+
GITHUB_PR_NUMBER: prNumber.toString(),
90+
GITHUB_APP_ID: process.env.GITHUB_APP_ID || '',
91+
GITHUB_APP_PRIVATE_KEY_PATH: process.env.GITHUB_APP_PRIVATE_KEY_PATH || '',
92+
GITHUB_APP_CWD: process.env.GITHUB_APP_CWD || '',
93+
});
9494
console.log(`Review completed for job ${jobId}`);
9595

9696
// Read collected comments from file
9797
const fs = await import('fs');
98-
const commentsFilePath = reviewResult.commentsFilePath;
98+
const commentsContent = reviewResult.commentsContent;
9999

100-
if (fs.existsSync(commentsFilePath)) {
100+
if (commentsContent) {
101101
try {
102-
const fileContent = fs.readFileSync(commentsFilePath, 'utf8').trim();
103-
console.log(`Found comments file with ${fileContent.length} characters`);
104-
105-
if (fileContent) {
106-
const commentLines = fileContent.split('\n');
107-
const comments = commentLines.map(line => JSON.parse(line));
108-
109-
const inlineComments = comments.filter(c => c.type === 'inline');
110-
const generalComments = comments.filter(c => c.type === 'general');
111-
112-
// TODO(sayans): GitHub API allows <= 30 comments per review, so we need to add splitting logic if there are > 30 comments
113-
114-
console.log(`Collected ${inlineComments.length} inline comments and ${generalComments.length} general comments`);
115-
116-
// Create review summary from general comments
117-
let reviewSummary = generalComments.length > 0
118-
? generalComments.map(c => c.message).join('\n\n')
119-
: 'Code review completed.';
120-
121-
// Append Amp thread URL if available
122-
if (reviewResult.threadId && config.amp.server_url) {
123-
const threadUrl = `${config.amp.server_url}/threads/${reviewResult.threadId}`;
124-
reviewSummary += `\n\n[View this review on Amp](${threadUrl})`;
125-
}
126-
127-
// Post aggregated review
128-
console.log('Posting aggregated PR review...');
129-
await githubClient.createPRReview(
130-
owner,
131-
repo,
132-
prNumber,
133-
reviewSummary,
134-
'COMMENT',
135-
inlineComments.map(comment => ({
136-
path: comment.path,
137-
line: comment.line,
138-
body: comment.suggested_fix
139-
? `${comment.message}\n\n\`\`\`suggestion\n${comment.suggested_fix}\n\`\`\``
140-
: comment.message
141-
}))
142-
);
143-
console.log('PR review posted successfully');
144-
} else {
145-
console.log('No comments collected, skipping review creation');
102+
console.log(`Found comments file with ${commentsContent.length} characters`);
103+
104+
const commentLines = commentsContent.split('\n');
105+
const comments = commentLines.map(line => JSON.parse(line));
106+
107+
const inlineComments = comments.filter(c => c.type === 'inline');
108+
const generalComments = comments.filter(c => c.type === 'general');
109+
110+
// TODO(sayans): GitHub API allows <= 30 comments per review, so we need to add splitting logic if there are > 30 comments
111+
112+
console.log(`Collected ${inlineComments.length} inline comments and ${generalComments.length} general comments`);
113+
114+
// Create review summary from general comments
115+
let reviewSummary = generalComments.length > 0
116+
? generalComments.map(c => c.message).join('\n\n')
117+
: 'Code review completed.';
118+
119+
// Append Amp thread URL if available
120+
if (reviewResult.threadId && config.amp.server_url) {
121+
const threadUrl = `${config.amp.server_url}/threads/${reviewResult.threadId}`;
122+
reviewSummary += `\n\n[View this review on Amp](${threadUrl})`;
146123
}
147124

125+
// Post aggregated review
126+
console.log('Posting aggregated PR review...');
127+
await githubClient.createPRReview(
128+
owner,
129+
repo,
130+
prNumber,
131+
reviewSummary,
132+
'COMMENT',
133+
inlineComments.map(comment => ({
134+
path: comment.path,
135+
line: comment.line,
136+
body: comment.suggested_fix
137+
? `${comment.message}\n\n\`\`\`suggestion\n${comment.suggested_fix}\n\`\`\``
138+
: comment.message
139+
}))
140+
);
141+
console.log('PR review posted successfully');
142+
148143
// Clean up the comments file
149-
fs.unlinkSync(commentsFilePath);
144+
fs.unlinkSync(reviewResult.commentsFilePath);
150145
console.log('Cleaned up comments file');
151146

152147
} catch (error) {

‎src/review/reviewer.ts‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import { tmpdir } from 'os';
22
import { join } from 'path';
3-
import { mkdirSync } from 'fs';
3+
import { existsSync, mkdirSync, readFileSync } from 'fs';
44
import { v4 as uuidv4 } from 'uuid';
55
import { execute } from '@sourcegraph/amp-sdk';
66
import { Config, getConfig } from "../config.js";
7-
import { PRContext } from "../github/types.js";
8-
97

108
export const reviewDiff = async (
119
diffContent: string,
12-
prContext: PRContext,
13-
installationId: number
10+
prContent: string,
11+
id: string,
12+
env: NodeJS.ProcessEnv = {}
1413
) => {
1514

1615
// Get config
@@ -23,35 +22,26 @@ export const reviewDiff = async (
2322
try {
2423
// Create prompt content
2524
const ampConfig = config.amp;
26-
27-
// Format PR context for prompt
28-
const prDetailsContent = `Repository: ${prContext.repository_full_name}\nPR Number: ${prContext.pr_number}\nCommit SHA: ${prContext.commit_sha}\nPR URL: ${prContext.pr_url}`;
29-
25+
3026
const promptContent = ampConfig.prompt_template
31-
.replace(/__PR_DETAILS_CONTENT__/g, prDetailsContent)
27+
.replace(/__PR_DETAILS_CONTENT__/g, prContent)
3228
.replace(/__DIFF_CONTENT__/g, diffContent);
3329

3430
// Generate unique filename for comment collection
35-
const commentsFileName = `comments-${installationId}.jsonl`;
31+
const commentsFileName = `comments-${id}.jsonl`;
3632
const commentsFilePath = join(tempDir, commentsFileName);
3733

3834
// Set up environment variables for toolbox
3935
const toolboxPath = join(process.cwd(), 'dist/toolbox');
4036
const toolboxEnv = {
37+
...env,
4138
COMMENTS_FILE: commentsFilePath,
42-
43-
GITHUB_INSTALLATION_ID: installationId.toString(),
44-
GITHUB_OWNER: prContext.owner,
45-
GITHUB_REPO: prContext.repo,
46-
GITHUB_PR_NUMBER: prContext.pr_number.toString(),
47-
GITHUB_APP_ID: process.env.GITHUB_APP_ID || '',
48-
GITHUB_APP_PRIVATE_KEY_PATH: process.env.GITHUB_APP_PRIVATE_KEY_PATH || '',
49-
GITHUB_APP_CWD: process.env.GITHUB_APP_CWD || '',
5039
};
5140

5241
// Execute Amp review using SDK
5342
let threadId: string | undefined;
5443
let result: string | undefined;
44+
let commentsContent: string | undefined;
5545

5646
console.log('[Amp] Starting thread...');
5747
for await (const message of execute({
@@ -80,9 +70,18 @@ export const reviewDiff = async (
8070
// Check if threadId and result are defined
8171
if (!threadId || !result) throw new Error('Amp review failed');
8272

83-
return { success: true, threadId, result, commentsFilePath };
73+
// Read comments file with separate error handling
74+
try {
75+
if (existsSync(commentsFilePath)) {
76+
commentsContent = readFileSync(commentsFilePath, 'utf-8').trim();
77+
}
78+
} catch (fileError) {
79+
console.error(`Error reading comments file: ${fileError}`);
80+
}
81+
82+
return { success: true, threadId, result, commentsFilePath, commentsContent };
8483
} catch (error) {
85-
console.error(`Error starting thread: ${error}`);
86-
throw new Error(`Failed to start thread: ${error}`);
84+
console.error(`Error executing review: ${error}`);
85+
throw new Error(`Failed to execute review: ${error}`);
8786
}
8887
}

0 commit comments

Comments
 (0)