Skip to content

Commit 456fa94

Browse files
author
Gavin Williams
committed
Address coderabbit review comments
1 parent 82fb218 commit 456fa94

File tree

7 files changed

+72
-73
lines changed

7 files changed

+72
-73
lines changed

‎packages/web/src/app/(app)/agents/page.tsx‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,11 @@ export default async function AgentsPage() {
2525
<div className="flex flex-col items-center overflow-hidden min-h-screen">
2626
<NavigationMenu />
2727
<div className="w-full max-w-6xl px-4 mt-12 mb-24">
28-
<div
29-
className={
30-
agents.length === 1
31-
? "flex justify-center items-center min-h-[60vh]"
32-
: "grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-10"
33-
}
34-
>
28+
<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-10">
3529
{agents.map((agent) => (
3630
<div
3731
key={agent.id}
38-
className={
39-
agents.length === 1
40-
? "relative flex flex-col items-center border border-border rounded-2xl p-8 bg-card shadow-xl w-full max-w-xl"
41-
: "relative flex flex-col items-center border border-border rounded-2xl p-8 bg-card shadow-xl"
42-
}
32+
className="relative flex flex-col items-center border border-border rounded-2xl p-8 bg-card shadow-xl"
4333
>
4434
{/* Name and description */}
4535
<div className="flex flex-col items-center w-full">

‎packages/web/src/app/api/(server)/webhook/route.ts‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -217,12 +217,16 @@ export const POST = async (request: NextRequest) => {
217217
return Response.json({ status: 'ok' });
218218
}
219219

220-
await processGitLabMergeRequest(
221-
gitlabClient,
222-
body.project.id,
223-
body,
224-
env.GITLAB_REVIEW_AGENT_HOST,
225-
);
220+
try {
221+
await processGitLabMergeRequest(
222+
gitlabClient,
223+
body.project.id,
224+
body,
225+
env.GITLAB_REVIEW_AGENT_HOST,
226+
);
227+
} catch (error) {
228+
logger.error(`Error in processGitLabMergeRequest for project ${body.project.id} (${gitlabEvent}):`, error);
229+
}
226230
}
227231

228232
if (isGitLabNoteEvent(gitlabEvent, body)) {
@@ -243,12 +247,16 @@ export const POST = async (request: NextRequest) => {
243247
project: body.project,
244248
};
245249

246-
await processGitLabMergeRequest(
247-
gitlabClient,
248-
body.project.id,
249-
mrPayload,
250-
env.GITLAB_REVIEW_AGENT_HOST,
251-
);
250+
try {
251+
await processGitLabMergeRequest(
252+
gitlabClient,
253+
body.project.id,
254+
mrPayload,
255+
env.GITLAB_REVIEW_AGENT_HOST,
256+
);
257+
} catch (error) {
258+
logger.error(`Error in processGitLabMergeRequest for project ${body.project.id} (${gitlabEvent}):`, error);
259+
}
252260
}
253261
}
254262
}

‎packages/web/src/features/agents/review-agent/app.ts‎

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -23,28 +23,34 @@ const rules = [
2323

2424
const logger = createLogger('review-agent');
2525

26+
function getReviewAgentLogPath(identifier: string): string | undefined {
27+
if (!env.REVIEW_AGENT_LOGGING_ENABLED) {
28+
return undefined;
29+
}
30+
31+
const reviewAgentLogDir = path.join(env.DATA_CACHE_DIR, "review-agent");
32+
if (!fs.existsSync(reviewAgentLogDir)) {
33+
fs.mkdirSync(reviewAgentLogDir, { recursive: true });
34+
}
35+
36+
const timestamp = new Date().toLocaleString('en-US', {
37+
year: 'numeric',
38+
month: '2-digit',
39+
day: '2-digit',
40+
hour: '2-digit',
41+
minute: '2-digit',
42+
second: '2-digit',
43+
hour12: false
44+
}).replace(/(\d+)\/(\d+)\/(\d+), (\d+):(\d+):(\d+)/, '$3_$1_$2_$4_$5_$6');
45+
const logPath = path.join(reviewAgentLogDir, `review-agent-${identifier}-${timestamp}.log`);
46+
logger.info(`Review agent logging to ${logPath}`);
47+
return logPath;
48+
}
49+
2650
export async function processGitHubPullRequest(octokit: Octokit, pullRequest: GitHubPullRequest) {
2751
logger.info(`Received a pull request event for #${pullRequest.number}`);
2852

29-
let reviewAgentLogPath: string | undefined;
30-
if (env.REVIEW_AGENT_LOGGING_ENABLED) {
31-
const reviewAgentLogDir = path.join(env.DATA_CACHE_DIR, "review-agent");
32-
if (!fs.existsSync(reviewAgentLogDir)) {
33-
fs.mkdirSync(reviewAgentLogDir, { recursive: true });
34-
}
35-
36-
const timestamp = new Date().toLocaleString('en-US', {
37-
year: 'numeric',
38-
month: '2-digit',
39-
day: '2-digit',
40-
hour: '2-digit',
41-
minute: '2-digit',
42-
second: '2-digit',
43-
hour12: false
44-
}).replace(/(\d+)\/(\d+)\/(\d+), (\d+):(\d+):(\d+)/, '$3_$1_$2_$4_$5_$6');
45-
reviewAgentLogPath = path.join(reviewAgentLogDir, `review-agent-${pullRequest.number}-${timestamp}.log`);
46-
logger.info(`Review agent logging to ${reviewAgentLogPath}`);
47-
}
53+
const reviewAgentLogPath = getReviewAgentLogPath(String(pullRequest.number));
4854

4955
const prPayload = await githubPrParser(octokit, pullRequest);
5056
const fileDiffReviews = await generatePrReviews(reviewAgentLogPath, prPayload, rules);
@@ -59,25 +65,7 @@ export async function processGitLabMergeRequest(
5965
) {
6066
logger.info(`Received a merge request event for !${mrPayload.object_attributes.iid}`);
6167

62-
let reviewAgentLogPath: string | undefined;
63-
if (env.REVIEW_AGENT_LOGGING_ENABLED) {
64-
const reviewAgentLogDir = path.join(env.DATA_CACHE_DIR, "review-agent");
65-
if (!fs.existsSync(reviewAgentLogDir)) {
66-
fs.mkdirSync(reviewAgentLogDir, { recursive: true });
67-
}
68-
69-
const timestamp = new Date().toLocaleString('en-US', {
70-
year: 'numeric',
71-
month: '2-digit',
72-
day: '2-digit',
73-
hour: '2-digit',
74-
minute: '2-digit',
75-
second: '2-digit',
76-
hour12: false
77-
}).replace(/(\d+)\/(\d+)\/(\d+), (\d+):(\d+):(\d+)/, '$3_$1_$2_$4_$5_$6');
78-
reviewAgentLogPath = path.join(reviewAgentLogDir, `review-agent-mr-${mrPayload.object_attributes.iid}-${timestamp}.log`);
79-
logger.info(`Review agent logging to ${reviewAgentLogPath}`);
80-
}
68+
const reviewAgentLogPath = getReviewAgentLogPath(`mr-${mrPayload.object_attributes.iid}`);
8169

8270
const prPayload = await gitlabMrParser(gitlabClient, mrPayload, hostDomain);
8371
const fileDiffReviews = await generatePrReviews(reviewAgentLogPath, prPayload, rules);

‎packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,31 @@ import { createLogger } from "@sourcebot/shared";
77

88
const logger = createLogger('fetch-file-content');
99

10+
type Org = Awaited<ReturnType<typeof __unsafePrisma.org.findUnique>>;
11+
let cachedOrg: Org | undefined;
12+
13+
const getOrg = async (): Promise<NonNullable<Org>> => {
14+
if (!cachedOrg) {
15+
cachedOrg = await __unsafePrisma.org.findUnique({
16+
where: { id: SINGLE_TENANT_ORG_ID },
17+
});
18+
}
19+
if (!cachedOrg) {
20+
throw new Error("Organization not found");
21+
}
22+
return cachedOrg;
23+
};
24+
1025
export const fetchFileContent = async (pr_payload: sourcebot_pr_payload, filename: string): Promise<sourcebot_context> => {
1126
logger.debug("Executing fetch_file_content");
1227

13-
const org = await __unsafePrisma.org.findUnique({
14-
where: { id: SINGLE_TENANT_ORG_ID },
15-
});
16-
if (!org) {
17-
throw new Error("Organization not found");
18-
}
28+
const org = await getOrg();
1929

2030
const repoPath = pr_payload.hostDomain + "/" + pr_payload.owner + "/" + pr_payload.repo;
2131
const fileSourceRequest = {
2232
path: filename,
2333
repo: repoPath,
34+
ref: pr_payload.head_sha,
2435
};
2536
logger.debug(JSON.stringify(fileSourceRequest, null, 2));
2637

‎packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ describe('githubPushPrReviews', () => {
135135

136136
await expect(
137137
githubPushPrReviews(octokit, MOCK_PAYLOAD, SINGLE_REVIEW),
138-
).resolves.not.toThrow();
138+
).resolves.toBeUndefined();
139139
});
140140

141141
test('does nothing when file_diff_reviews is empty', async () => {

‎packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,9 @@ export const gitlabMrParser = async (
9696
repo: repoName,
9797
file_diffs: filteredSourcebotFileDiffs,
9898
number: mrIid,
99-
head_sha: mr.sha,
100-
diff_refs: mr.diff_refs as { base_sha: string; head_sha: string; start_sha: string },
99+
head_sha: mr.sha ?? "",
100+
diff_refs: mr.diff_refs != null
101+
? mr.diff_refs as { base_sha: string; head_sha: string; start_sha: string }
102+
: undefined,
101103
};
102104
};

‎packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ export const invokeDiffReviewLlm = async (reviewAgentLogPath: string | undefined
3434
try {
3535
const result = await generateText({
3636
model,
37-
system: prompt,
38-
prompt: "Review the code changes.",
37+
system: "You are a code review assistant. Respond only with valid JSON matching the expected schema.",
38+
prompt,
3939
providerOptions,
4040
temperature,
4141
});

0 commit comments

Comments
 (0)