From 10aeaf8c111eb73e5851a5066dbbc4273b738dd9 Mon Sep 17 00:00:00 2001 From: David Blass Date: Sat, 9 May 2026 00:30:51 +0000 Subject: [PATCH] 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 * address review: drop stale file ref in dedupe comment; soften tool description * remove comment.test.ts --------- Co-authored-by: Cursor Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> --- mcp/comment.ts | 70 +++++++++++++++++++++++++++++++++++++++++++++++++- mcp/server.ts | 9 +++++++ modes.ts | 2 +- 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/mcp/comment.ts b/mcp/comment.ts index 14a6abf..12c118a 100644 --- a/mcp/comment.ts +++ b/mcp/comment.ts @@ -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) { return tool({ name: "reply_to_review_comment", 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, execute: execute(async ({ pull_number, comment_id, 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({ owner: ctx.repo.owner, repo: ctx.repo.name, @@ -420,6 +480,14 @@ export function ReplyToReviewCommentTool(ctx: ToolContext) { // a substantive write happened (used by reportErrorToComment / handleAgentResult) 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 { success: true, commentId: result.data.id, diff --git a/mcp/server.ts b/mcp/server.ts index 32dce5f..bc75cf0 100644 --- a/mcp/server.ts +++ b/mcp/server.ts @@ -131,6 +131,15 @@ export interface ToolState { nodeId: string; 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?: { status: "not_started" | "in_progress" | "completed" | "failed"; promise: Promise | undefined; diff --git a/modes.ts b/modes.ts index e74d860..e0f7775 100644 --- a/modes.ts +++ b/modes.ts @@ -147,7 +147,7 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, 5. Finalize: - 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")}\` - call \`${t("report_progress")}\` with a brief summary (or the exact push error if push failed)`, },