Skip to content

Commit 9cfc5a7

Browse files
committed
optimize review result
1 parent f0e4f76 commit 9cfc5a7

2 files changed

Lines changed: 36 additions & 123 deletions

File tree

src/bot.ts

Lines changed: 19 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -204,20 +204,29 @@ export const robot = (app: Probot) => {
204204
);
205205
continue;
206206
}
207-
208207
try {
209-
// Split patch into individual hunks (by @@ markers)
210-
const hunks = splitPatchIntoHunks(patch);
208+
const res = await chat?.codeReview(patch);
209+
// res can be a single review or an array of reviews (one for each hunk)
210+
const reviews = Array.isArray(res) ? res : [res];
211211

212-
for (const hunk of hunks) {
213-
const res = await chat?.codeReview(hunk.content);
214-
if (!res.lgtm && !!res.review_comment) {
215-
const { line, side } = computeLineAndSideFromPatch(hunk.content);
212+
for (const review of reviews) {
213+
if (!review.lgtm && !!review.review_comment) {
214+
let line: number | undefined;
215+
let side: 'LEFT' | 'RIGHT' = 'RIGHT';
216+
217+
// Extract line number from hunk header if available
218+
if (review.hunk_header) {
219+
const hunkMatch = review.hunk_header.match(/@@\s+-\d+(?:,\d+)?\s+\+(\d+)(?:,(\d+))?\s+@@/);
220+
if (hunkMatch) {
221+
line = parseInt(hunkMatch[1], 10);
222+
}
223+
}
224+
216225
ress.push({
217226
path: file.filename,
218-
body: res.review_comment,
219-
line: hunk.newStart + line - 1,
220-
side,
227+
body: review.review_comment,
228+
line: line,
229+
side: side,
221230
})
222231
}
223232
}
@@ -265,112 +274,4 @@ const matchPatterns = (patterns: string[], path: string) => {
265274
}
266275
}
267276
})
268-
}
269-
270-
const computeLineAndSideFromPatch = (patch: string | undefined) => {
271-
// Returns first suitable line number and side for a review comment.
272-
// Preference order: first added line ('+' -> RIGHT), then context (' ' -> RIGHT), then deletion ('-' -> LEFT).
273-
if (!patch) return { line: 1, side: 'RIGHT' as 'RIGHT' | 'LEFT' };
274-
275-
const lines = patch.split('\n');
276-
let currentOld = 0;
277-
let currentNew = 0;
278-
279-
// helper to parse the hunk header like: @@ -oldStart,oldCount +newStart,newCount @@
280-
const parseHunkHeader = (hdr: string) => {
281-
const m = hdr.match(/@@\s+-(\d+)(?:,(\d+))?\s+\+(\d+)(?:,(\d+))?\s+@@/);
282-
if (!m) return null;
283-
const oldStart = parseInt(m[1], 10);
284-
const newStart = parseInt(m[3], 10);
285-
return { oldStart, newStart };
286-
};
287-
288-
// track first matches
289-
let firstAdded: number | null = null;
290-
let firstContext: number | null = null;
291-
let firstDeleted: number | null = null;
292-
293-
for (let i = 0; i < lines.length; i++) {
294-
const line = lines[i];
295-
if (line.startsWith('@@')) {
296-
const parsed = parseHunkHeader(line);
297-
if (parsed) {
298-
currentOld = parsed.oldStart;
299-
currentNew = parsed.newStart;
300-
}
301-
continue;
302-
}
303-
304-
if (line.startsWith('+')) {
305-
// addition: belongs to new file
306-
if (firstAdded === null) firstAdded = currentNew;
307-
currentNew++;
308-
continue;
309-
}
310-
311-
if (line.startsWith('-')) {
312-
// deletion: belongs to old file
313-
if (firstDeleted === null) firstDeleted = currentOld;
314-
currentOld++;
315-
continue;
316-
}
317-
318-
// context line (starts with space or other)
319-
if (line.startsWith(' ') || line.length === 0) {
320-
if (firstContext === null) firstContext = currentNew;
321-
currentOld++;
322-
currentNew++;
323-
continue;
324-
}
325-
}
326-
327-
if (firstAdded !== null) return { line: firstAdded, side: 'RIGHT' as 'RIGHT' | 'LEFT' };
328-
if (firstContext !== null) return { line: firstContext, side: 'RIGHT' as 'RIGHT' | 'LEFT' };
329-
if (firstDeleted !== null) return { line: firstDeleted, side: 'LEFT' as 'RIGHT' | 'LEFT' };
330-
331-
return { line: 1, side: 'RIGHT' as 'RIGHT' | 'LEFT' };
332-
}
333-
334-
const splitPatchIntoHunks = (patch: string): Array<{ header: string; newStart: number; content: string }> => {
335-
// Split patch into individual hunks by @@ markers
336-
const hunks: Array<{ header: string; newStart: number; content: string }> = [];
337-
const lines = patch.split('\n');
338-
let currentHunk: string[] = [];
339-
let currentHeader = '';
340-
let currentNewStart = 0;
341-
342-
const parseHunkHeader = (hdr: string) => {
343-
const m = hdr.match(/@@\s+-\d+(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s+@@/);
344-
if (!m) return 0;
345-
return parseInt(m[1], 10);
346-
};
347-
348-
for (const line of lines) {
349-
if (line.startsWith('@@')) {
350-
// Save previous hunk if it exists
351-
if (currentHunk.length > 0) {
352-
hunks.push({
353-
header: currentHeader,
354-
newStart: currentNewStart,
355-
content: currentHunk.join('\n'),
356-
});
357-
}
358-
currentHeader = line;
359-
currentNewStart = parseHunkHeader(line);
360-
currentHunk = [line];
361-
} else if (currentHunk.length > 0) {
362-
currentHunk.push(line);
363-
}
364-
}
365-
366-
// Save the last hunk
367-
if (currentHunk.length > 0) {
368-
hunks.push({
369-
header: currentHeader,
370-
newStart: currentNewStart,
371-
content: currentHunk.join('\n'),
372-
});
373-
}
374-
375-
return hunks;
376277
}

src/chat.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,23 @@ export class Chat {
3838

3939
const jsonFormatRequirement = '\nProvide your feedback in a strict JSON format with the following structure:\n' +
4040
'{\n' +
41-
' "lgtm": boolean, // true if the code looks good to merge, false if there are concerns\n' +
42-
' "review_comment": string // Your detailed review comments. You can use markdown syntax in this string, but the overall response must be a valid JSON\n' +
41+
' "reviews": [\n' +
42+
' {\n' +
43+
' "hunk_header": string, // The @@ hunk header (e.g., "@@ -10,5 +10,7 @@"), optional\n' +
44+
' "lgtm": boolean, // true if this hunk looks good, false if there are concerns\n' +
45+
' "review_comment": string // Your detailed review comments for this hunk. Can use markdown syntax. Empty string if lgtm is true.\n' +
46+
' }\n' +
47+
' ]\n' +
4348
'}\n' +
44-
'Ensure your response is a valid JSON object.\n';
49+
'Review each hunk (marked by @@) separately and provide feedback for hunks that need improvement.\n' +
50+
'Ensure your response is a valid JSON object with a reviews array.\n';
4551

4652
return `${userPrompt}${jsonFormatRequirement} ${answerLanguage}:
4753
${patch}
4854
`;
4955
};
5056

51-
public codeReview = async (patch: string): Promise<{ lgtm: boolean, review_comment: string }> => {
57+
public codeReview = async (patch: string): Promise<Array<{ lgtm: boolean, review_comment: string, hunk_header?: string }> | { lgtm: boolean, review_comment: string, hunk_header?: string }> => {
5258
if (!patch) {
5359
return {
5460
lgtm: true,
@@ -80,10 +86,16 @@ export class Chat {
8086
if (res.choices.length) {
8187
try {
8288
const json = JSON.parse(res.choices[0].message.content || "");
83-
return json
89+
// If response has a 'reviews' array, return it directly
90+
if (json.reviews && Array.isArray(json.reviews)) {
91+
return json.reviews;
92+
}
93+
// Otherwise, treat as a single review response
94+
return json;
8495
} catch (e) {
8596
return {
8697
lgtm: false,
98+
hunk_header: patch.split('\n')[0].startsWith('@@') ? patch.split('\n')[0] : undefined,
8799
review_comment: res.choices[0].message.content || ""
88100
}
89101
}

0 commit comments

Comments
 (0)