Skip to content

feat: batch inline comments into a single PR review submission #1147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hashwnath wants to merge 1 commit into
base: main
Choose a base branch
from
+77 −7
Open

feat: batch inline comments into a single PR review submission #1147

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 77 additions & 7 deletions src/entrypoints/post-buffered-inline-comments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,71 @@ async function postComment(
}
}

/**
* Submit all comments as a single GitHub Pull Request Review. This produces
* one notification instead of N separate notifications for each comment.
* Falls back to posting individual comments if the batch review fails (e.g.
Copy link
Copy Markdown

@suramkavyasree suramkavyasree Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ HIGH: Missing type hints for public function parameters and return value

The function postCommentsAsReview lacks type hints for its parameters and return value.

Why this matters: Type hints improve code readability and help with static analysis, making it easier to understand and maintain the code.

Suggested fix:

Add type hints for all parameters and the return value of the function.

Copy link
Copy Markdown

@suramkavyasree suramkavyasree Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 LOW: Missing Google-style docstring for public function

The function postCommentsAsReview lacks a Google-style docstring with Args, Returns, and Raises sections.

Why this matters: Comprehensive docstrings provide clear documentation for function usage, expected inputs, outputs, and potential exceptions.

Suggested fix:

Add a Google-style docstring with Args, Returns, and Raises sections.

* when a comment targets an invalid path or line).
*/
async function postCommentsAsReview(
octokit: ReturnType<typeof createOctokit>["rest"],
owner: string,
repo: string,
pull_number: number,
headSha: string,
comments: BufferedComment[],
): Promise<number> {
const reviewComments = comments.map((c) => {
const comment: {
path: string;
body: string;
side?: "LEFT" | "RIGHT";
line?: number;
start_line?: number;
start_side?: "LEFT" | "RIGHT";
} = {
path: c.path,
body: c.body,
side: c.side || "RIGHT",
Copy link
Copy Markdown

@suramkavyasree suramkavyasree Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ HIGH: Use of console.log instead of structlog logger

The code uses console.log for logging, which is not suitable for production environments.

Why this matters: Using a structured logger like structlog provides better logging capabilities, such as log levels, structured data, and better integration with logging systems.

Suggested fix:

Replace console.log with structlog logger.

};
if (c.startLine) {
comment.start_line = c.startLine;
Copy link
Copy Markdown

@suramkavyasree suramkavyasree Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ HIGH: Bare except used in catch block

The catch block uses a bare exception handling pattern, which can swallow all exceptions, including system-exiting ones.

Why this matters: Using a bare except can hide bugs and make debugging difficult. It's important to catch specific exceptions to handle known error conditions.

Suggested fix:

Catch specific exceptions instead of using a bare except.

comment.start_side = c.side || "RIGHT";
comment.line = c.line;
} else {
comment.line = c.line;
}
return comment;
});

try {
await octokit.rest.pulls.createReview({
owner,
repo,
pull_number,
commit_id: headSha,
event: "COMMENT",
comments: reviewComments,
});
console.log(
` submitted ${comments.length} comment(s) as a single PR review`,
);
return comments.length;
} catch (e) {
console.log(
` batch review failed (${e instanceof Error ? e.message : String(e)}), falling back to individual comments`,
);
let posted = 0;
for (const c of comments) {
if (await postComment(octokit, owner, repo, pull_number, headSha, c)) {
console.log(` posted ${c.path}:${c.line}`);
posted++;
}
}
return posted;
}
}

async function main() {
let raw: string;
try {
Expand Down Expand Up @@ -217,13 +282,18 @@ async function main() {
const headSha = pr.data.head.sha;

console.log(`Posting ${toPost.length} classified-as-real comment(s)`);
let posted = 0;
for (const c of toPost) {
if (await postComment(octokit, owner, repo, pull_number, headSha, c)) {
console.log(` posted ${c.path}:${c.line}`);
posted++;
}
}

// Batch all comments into a single PR review submission. This produces
// one notification and a cohesive review in GitHub's timeline instead of
// separate per-comment notifications.
const posted = await postCommentsAsReview(
octokit,
owner,
repo,
pull_number,
headSha,
toPost,
);
console.log(`Posted ${posted}/${toPost.length}`);
}

Expand Down