chore: some more improvements to try and get as close to original as possible
This commit is contained in:
+88
-9
@@ -181,7 +181,20 @@ export function formatDroppedCommentsNote(dropped: DroppedComment[]): string {
|
||||
|
||||
export const CreatePullRequestReview = type({
|
||||
pull_number: type.number.describe("The pull request number to review"),
|
||||
body: type.string.describe("1-2 sentence high-level summary").optional(),
|
||||
preamble: type.string.describe(
|
||||
"One sentence describing what was reviewed (e.g. 'This PR adds device management behind a feature flag'). " +
|
||||
"The server prepends '**Reviewed changes** — ' automatically. " +
|
||||
"When provided, do NOT repeat the preamble inside 'body'."
|
||||
).optional(),
|
||||
changes: type.string.array().describe(
|
||||
"Bullet list of substantive changes — neutral descriptions of what the PR added/changed/removed. " +
|
||||
"Each entry is one formatted bullet, e.g. '**Feature X** — 1 sentence description'. " +
|
||||
"These must describe CHANGES only, not findings or issues."
|
||||
).optional(),
|
||||
body: type.string.describe(
|
||||
"When 'preamble'/'changes' are used: include ONLY the metadata HTML comment and any non-anchored ### sections. " +
|
||||
"When used alone (legacy): full review body including preamble."
|
||||
).optional(),
|
||||
approved: type.boolean.describe("Set to true to submit as an approval.").optional(),
|
||||
commit_id: type.string.describe("Optional SHA of the commit being reviewed.").optional(),
|
||||
comments: type({
|
||||
@@ -200,25 +213,86 @@ export const CreatePullRequestReview = type({
|
||||
.optional(),
|
||||
});
|
||||
|
||||
/** Assemble the **Reviewed changes** preamble block from structured params. */
|
||||
function assemblePreamble(preamble: string, changes: string[]): string {
|
||||
const lines = [`**Reviewed changes** — ${preamble}`];
|
||||
if (changes.length > 0) {
|
||||
lines.push("");
|
||||
for (const change of changes) {
|
||||
lines.push(change.startsWith("- ") ? change : `- ${change}`);
|
||||
}
|
||||
}
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip body ### sections whose file paths are already covered by inline
|
||||
* comments. Prevents the model from duplicating inline findings in the body.
|
||||
*/
|
||||
function postProcessBody(
|
||||
body: string,
|
||||
validComments: ReviewCommentInput[],
|
||||
): string {
|
||||
if (validComments.length === 0) return body;
|
||||
|
||||
const commentedPaths = new Set(validComments.map((c) => c.path));
|
||||
|
||||
const sectionRegex = /^### .+$/gm;
|
||||
const matches: Array<{ index: number; heading: string }> = [];
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = sectionRegex.exec(body)) !== null) {
|
||||
matches.push({ index: m.index, heading: m[0] });
|
||||
}
|
||||
if (matches.length === 0) return body;
|
||||
|
||||
const sections = matches.map((match, i) => ({
|
||||
start: match.index,
|
||||
end: matches[i + 1]?.index ?? body.length,
|
||||
heading: match.heading,
|
||||
}));
|
||||
|
||||
const toRemove: Array<{ start: number; end: number }> = [];
|
||||
for (const section of sections) {
|
||||
const content = body.slice(section.start, section.end);
|
||||
for (const path of commentedPaths) {
|
||||
if (content.includes(path)) {
|
||||
toRemove.push({ start: section.start, end: section.end });
|
||||
log.info(`stripped duplicate body section "${section.heading.slice(0, 80)}" — already covered by inline comment on ${path}`);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (toRemove.length === 0) return body;
|
||||
|
||||
let result = body;
|
||||
for (const { start, end } of [...toRemove].reverse()) {
|
||||
result = result.slice(0, start) + result.slice(end);
|
||||
}
|
||||
return result.replace(/\n{3,}/g, "\n\n").trim();
|
||||
}
|
||||
|
||||
export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
return tool({
|
||||
name: "create_pull_request_review",
|
||||
description:
|
||||
"Submit a review for an existing pull request. " +
|
||||
"Reviews with no body AND no comments are silently skipped. " +
|
||||
"IMPORTANT: 95%+ of feedback should be in 'comments' with file paths and line numbers. " +
|
||||
"Only use 'body' for a 1-2 sentence summary. " +
|
||||
"For every actionable issue, the comment 'body' should explain the problem AND include a markdown " +
|
||||
"code block showing the corrected code. For example: " +
|
||||
"'SQL injection: id is interpolated directly into the query string.\\n\\n```ts\\nconst result = await db.query(\\'SELECT * FROM users WHERE id = $1\\', [id]);\\n```'. " +
|
||||
"PREFERRED: use 'preamble' + 'changes' for the reviewed-changes block, and 'body' for ONLY the metadata HTML comment and non-anchored ### sections. " +
|
||||
"IMPORTANT: 95%+ of feedback must be in 'comments' with file paths and line numbers — not in 'body'. " +
|
||||
"The first submission may error once with a diff-coverage nudge — retry with the same arguments. " +
|
||||
"Inline comments: 'path' must be the SOURCE FILE path (e.g. 'apps/foo/bar.ts') from the diff --git header, NOT the diffPath returned by checkout_pr. " +
|
||||
"Inline comments: 'path' must be the SOURCE FILE path (e.g. 'apps/foo/bar.ts') from the diff --git header, NOT the diffPath. " +
|
||||
"'line' must be the actual file line number from the '| newLine |' column in the formatted diff (not the TOC line range). " +
|
||||
"Inline comments can ONLY target files and lines that appear in the PR diff.",
|
||||
parameters: CreatePullRequestReview,
|
||||
execute: execute(async ({ pull_number, body, approved, commit_id, comments = [] }) => {
|
||||
execute: execute(async ({ pull_number, preamble, changes, body, approved, commit_id, comments = [] }) => {
|
||||
if (body) body = fixDoubleEscapedString(body);
|
||||
|
||||
// Assemble structured preamble if provided
|
||||
if (preamble || (changes && changes.length > 0)) {
|
||||
const preambleBlock = assemblePreamble(preamble ?? "", changes ?? []);
|
||||
body = body ? `${preambleBlock}\n\n${body}` : preambleBlock;
|
||||
}
|
||||
|
||||
ctx.toolState.issueNumber = pull_number;
|
||||
|
||||
const dup = duplicateReviewDecision({
|
||||
@@ -287,6 +361,11 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
}
|
||||
}
|
||||
|
||||
// Strip body ### sections that duplicate inline comments (#1 post-processing)
|
||||
if (body && validComments.length > 0) {
|
||||
body = postProcessBody(body, validComments);
|
||||
}
|
||||
|
||||
if (droppedComments.length > 0) {
|
||||
const note = formatDroppedCommentsNote(droppedComments);
|
||||
body = body ? body + note : note.replace(/^\n\n/, "");
|
||||
|
||||
Reference in New Issue
Block a user