action: dedupe identical reply_to_review_comment calls within a session (#623)
* action: dedupe identical reply_to_review_comment calls within a session PR #610 reproduced a Kimi K2 stutter where the agent's tool_use surface showed one `pullfrog_reply_to_review_comment` call but GitHub recorded two byte-identical POSTs 3s apart, leaving a duplicate response on `action/mcp/review.ts:14`. Add `duplicateReplyDecision` (mirrors `duplicateReviewDecision`) and track per-session replies on `ToolState.reviewReplies`, keyed by parent `comment_id` + `bodyWithFooter`. Identical re-emissions short circuit with `{ skipped: true, reason }` instead of POSTing again. Body-keyed (not just id-keyed) so legitimate follow-up replies with different content still go through. Tighten `AddressReviews` step 5 to say *exactly once per comment* and note that the runtime dedupes identical bodies, so the agent has both prompt-level guidance and a server-side guarantee. Co-authored-by: Cursor <cursoragent@cursor.com> * address review: drop stale file ref in dedupe comment; soften tool description * remove comment.test.ts --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
This commit is contained in:
committed by
pullfrog[bot]
parent
85d25a6fe6
commit
10aeaf8c11
+69
-1
@@ -398,15 +398,75 @@ export const ReplyToReviewComment = type({
|
|||||||
),
|
),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* decision returned by `duplicateReplyDecision` when a session has already
|
||||||
|
* posted an identical reply to the same parent review comment.
|
||||||
|
*/
|
||||||
|
export interface DuplicateReplyDecision {
|
||||||
|
kind: "already-replied";
|
||||||
|
commentId: number;
|
||||||
|
url: string | undefined;
|
||||||
|
reason: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* decide whether a second reply_to_review_comment call in the same session
|
||||||
|
* is a duplicate of an earlier reply to the same parent comment.
|
||||||
|
*
|
||||||
|
* the agent is instructed to call reply_to_review_comment exactly once per
|
||||||
|
* parent comment per AddressReviews session, but in practice it sometimes
|
||||||
|
* emits the same call twice. PR #610 reproduced this with Kimi K2:
|
||||||
|
* identical body posted 3 seconds apart, only one tool_use event in the
|
||||||
|
* agent log. the second post is always redundant and clutters the PR thread.
|
||||||
|
*
|
||||||
|
* we key on (comment_id, bodyWithFooter) so a legitimate follow-up reply
|
||||||
|
* with different content still goes through. within a single run the
|
||||||
|
* footer is constant (workflow run + model + jobId), so byte-equal bodies
|
||||||
|
* catch the stutter without blocking real follow-ups.
|
||||||
|
*
|
||||||
|
* mirrors the shape of `duplicateReviewDecision` in mcp/review.ts.
|
||||||
|
*/
|
||||||
|
export function duplicateReplyDecision(params: {
|
||||||
|
existing: { commentId: number; url: string | undefined; bodyWithFooter: string } | undefined;
|
||||||
|
bodyWithFooter: string;
|
||||||
|
}): DuplicateReplyDecision | null {
|
||||||
|
const existing = params.existing;
|
||||||
|
if (!existing) return null;
|
||||||
|
if (existing.bodyWithFooter !== params.bodyWithFooter) return null;
|
||||||
|
return {
|
||||||
|
kind: "already-replied",
|
||||||
|
commentId: existing.commentId,
|
||||||
|
url: existing.url,
|
||||||
|
reason: `reply ${existing.commentId} with identical body was already posted in this session; ignoring duplicate call`,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
export function ReplyToReviewCommentTool(ctx: ToolContext) {
|
export function ReplyToReviewCommentTool(ctx: ToolContext) {
|
||||||
return tool({
|
return tool({
|
||||||
name: "reply_to_review_comment",
|
name: "reply_to_review_comment",
|
||||||
description:
|
description:
|
||||||
"Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). Call this for EACH comment you address in AddressReviews mode. Keep replies extremely brief (1 sentence max).",
|
"Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). Call exactly ONCE per parent comment you address in AddressReviews mode — duplicate calls with the same body are a no-op. Keep replies extremely brief (1 sentence max).",
|
||||||
parameters: ReplyToReviewComment,
|
parameters: ReplyToReviewComment,
|
||||||
execute: execute(async ({ pull_number, comment_id, body }) => {
|
execute: execute(async ({ pull_number, comment_id, body }) => {
|
||||||
const bodyWithFooter = addFooter(ctx, body);
|
const bodyWithFooter = addFooter(ctx, body);
|
||||||
|
|
||||||
|
// guard against duplicate reply submissions in the same session.
|
||||||
|
// see duplicateReplyDecision for the rationale.
|
||||||
|
const dup = duplicateReplyDecision({
|
||||||
|
existing: ctx.toolState.reviewReplies?.get(comment_id),
|
||||||
|
bodyWithFooter,
|
||||||
|
});
|
||||||
|
if (dup) {
|
||||||
|
log.info(`skipping duplicate review reply: ${dup.reason}`);
|
||||||
|
return {
|
||||||
|
success: true,
|
||||||
|
skipped: true,
|
||||||
|
reason: dup.reason,
|
||||||
|
commentId: dup.commentId,
|
||||||
|
url: dup.url,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
const result = await ctx.octokit.rest.pulls.createReplyForReviewComment({
|
const result = await ctx.octokit.rest.pulls.createReplyForReviewComment({
|
||||||
owner: ctx.repo.owner,
|
owner: ctx.repo.owner,
|
||||||
repo: ctx.repo.name,
|
repo: ctx.repo.name,
|
||||||
@@ -420,6 +480,14 @@ export function ReplyToReviewCommentTool(ctx: ToolContext) {
|
|||||||
// a substantive write happened (used by reportErrorToComment / handleAgentResult)
|
// a substantive write happened (used by reportErrorToComment / handleAgentResult)
|
||||||
ctx.toolState.wasUpdated = true;
|
ctx.toolState.wasUpdated = true;
|
||||||
|
|
||||||
|
// record this reply for in-session dedupe of subsequent identical calls.
|
||||||
|
ctx.toolState.reviewReplies ??= new Map();
|
||||||
|
ctx.toolState.reviewReplies.set(comment_id, {
|
||||||
|
commentId: result.data.id,
|
||||||
|
url: result.data.html_url,
|
||||||
|
bodyWithFooter,
|
||||||
|
});
|
||||||
|
|
||||||
return {
|
return {
|
||||||
success: true,
|
success: true,
|
||||||
commentId: result.data.id,
|
commentId: result.data.id,
|
||||||
|
|||||||
@@ -131,6 +131,15 @@ export interface ToolState {
|
|||||||
nodeId: string;
|
nodeId: string;
|
||||||
reviewedSha: string | undefined;
|
reviewedSha: string | undefined;
|
||||||
};
|
};
|
||||||
|
// dedupe key: parent review comment_id → most-recent reply written this
|
||||||
|
// session by reply_to_review_comment. used by duplicateReplyDecision to
|
||||||
|
// skip identical-body re-emissions of the same call (PR #610 root cause).
|
||||||
|
// body-keyed (not just id-keyed) so legitimate follow-up replies with
|
||||||
|
// different content still go through.
|
||||||
|
reviewReplies?: Map<
|
||||||
|
number,
|
||||||
|
{ commentId: number; url: string | undefined; bodyWithFooter: string }
|
||||||
|
>;
|
||||||
dependencyInstallation?: {
|
dependencyInstallation?: {
|
||||||
status: "not_started" | "in_progress" | "completed" | "failed";
|
status: "not_started" | "in_progress" | "completed" | "failed";
|
||||||
promise: Promise<PrepResult[]> | undefined;
|
promise: Promise<PrepResult[]> | undefined;
|
||||||
|
|||||||
@@ -147,7 +147,7 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
|
|||||||
|
|
||||||
5. Finalize:
|
5. Finalize:
|
||||||
- confirm a clean working tree, then push via \`${t("push_branch")}\` (same push/prepush guidance as Build mode in *SYSTEM*)
|
- confirm a clean working tree, then push via \`${t("push_branch")}\` (same push/prepush guidance as Build mode in *SYSTEM*)
|
||||||
- reply to each comment using \`${t("reply_to_review_comment")}\`
|
- reply to each comment **exactly once** using \`${t("reply_to_review_comment")}\` — do not re-emit the same call (the runtime dedupes identical bodies and the second call is wasted)
|
||||||
- resolve addressed threads via \`${t("resolve_review_thread")}\`
|
- resolve addressed threads via \`${t("resolve_review_thread")}\`
|
||||||
- call \`${t("report_progress")}\` with a brief summary (or the exact push error if push failed)`,
|
- call \`${t("report_progress")}\` with a brief summary (or the exact push error if push failed)`,
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user