Skip to content

fix(review): prevent duplicate comment replies on re-review#1725

Merged
atoomic merged 2 commits into
Anantys-oss:mainfrom
Koan-Bot:koan.atoomic/fix-review-reply-loop
Jun 4, 2026
Merged

fix(review): prevent duplicate comment replies on re-review#1725
atoomic merged 2 commits into
Anantys-oss:mainfrom
Koan-Bot:koan.atoomic/fix-review-reply-loop

Conversation

@Koan-Bot

@Koan-Bot Koan-Bot commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

What

Stop the bot from posting duplicate replies to the same PR comments on re-review.

Why

After each push/rebase, review_requested notifications trigger a new /review with a fresh SHA-based dedup key. Each review re-replies to all human issue comments because _fetch_issue_comments had no self-reply detection for flat (non-threaded) issue comments. This created a feedback loop: review → rebase → new SHA → re-review → same replies posted again.

Example: PR #1697 received 6 identical replies to the same comment; PR #1724 received 3 rounds of duplicate reviews within 30 minutes.

How

  1. Issue comment self-reply detection (review_runner.py): New _exclude_replied_issue_comments() detects bot replies by matching the > @user: text... quote pattern against human comments. _fetch_issue_comments now collects bot comments and filters already-replied human comments before returning the "repliable" list.

  2. In-progress mission dedup (github_command_handler.py): _try_assignment_notification now checks both Pending AND In Progress sections of missions.md, preventing re-queuing while a review is still running.

  3. Review cooldown infrastructure (github_notification_tracker.py): Added is_review_on_cooldown() / set_review_cooldown() helpers for future use. Not currently wired — the self-reply detection alone breaks the visible symptom.

Testing

  • 8 new tests covering self-reply detection, cooldown tracker, and integration with _fetch_issue_comments
  • Full suite: 15320 passed (1 pre-existing unrelated failure)

🤖 Generated with Claude Code


Quality Report

Changes: 5 files changed, 209 insertions(+), 11 deletions(-)

Code scan: clean

Tests: failed (1 failed, 403
tests)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

@Koan-Bot

Koan-Bot commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

@Koan-Bot review

1 similar comment
@atoomic

atoomic commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

@Koan-Bot review

@atoomic atoomic self-assigned this Jun 4, 2026
@atoomic atoomic marked this pull request as ready for review June 4, 2026 04:53
@Koan-Bot

Koan-Bot commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

@atoomic: @Koan-Bot review

Reviewed the PR. The self-reply detection approach in _exclude_replied_issue_comments is well-designed — matching the > @user: text... quote pattern is the right strategy for flat issue comments that lack in_reply_to_id.

One thing worth tightening: the prefix-matching after stripping ... from truncated quotes could produce false positives when two human comments from the same user share a common first-line prefix. See the inline comment on review_runner.py line 148 for details.

The in-progress dedup fix in github_command_handler.py is solid (and could be slightly simplified by using parse_sections alone instead of list_pending + parse_sections). The cooldown infrastructure is clean but currently unwired — I'd suggest either connecting it or splitting it into its own PR.

Overall: merge-ready with the prefix-matching caveat as a follow-up consideration.

@Koan-Bot

Koan-Bot commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

PR Review — fix(review): prevent duplicate comment replies on re-review

Solid fix for a real user-facing problem (duplicate review replies). The core self-reply detection logic is correct, well-tested, and addresses the root cause cleanly.

  • The _exclude_replied_issue_comments approach is sound — matching the > @user: text... quote pattern against flat issue comments is the right way to detect self-replies without threading support.
  • The in-progress dedup extension in github_command_handler.py is a good belt-and-suspenders fix, correctly using parse_sections with the canonical "in_progress" key.
  • The prefix-matching after ... stripping (warning above) is the one spot that could produce false positives on comments sharing a common first-line prefix — worth tightening but not blocking given how rare that scenario is in practice.
  • The cooldown infrastructure is clean but currently dead code — consider either wiring it or deferring it to a follow-up PR.
  • Test coverage is thorough: 8 new tests covering the main paths, edge cases, and integration with _fetch_issue_comments.

🟡 Important

1. Cooldown infra is dead code (unused) (`koan/app/github_notification_tracker.py`, L162-179)

The PR description says this cooldown infrastructure is "not currently wired." Dead code that ships untested-in-production tends to bit-rot or mislead future readers into thinking it's already active.

Two options:

  • Wire it up in this PR (even if the self-reply detection is sufficient alone, it's a belt-and-suspenders defense).
  • Remove it and add it in the PR that actually uses it.

If you keep it, add a brief # TODO: wire into ... comment pointing to the integration site so a future reader knows the plan.

def is_review_on_cooldown(instance_dir: str, owner: str, repo: str, pr_number: str) -> bool:
2. Stripping `...` creates over-broad prefix matching (`koan/app/review_runner.py`, L148)

When a bot reply quotes > @alice: Some text..., the code strips the trailing ... and then matches any human comment whose first line .startswith("some text"). This means a human comment starting with "Some text is wrong" would also match, even if the bot actually replied to a different comment starting with "Some text is fine".

For comments whose first lines share a common prefix this produces false-positive filtering — the bot would silently skip an unreplied human comment because a different comment happened to share the same prefix.

Safer fix: after stripping ..., also record the quote length, and compare equality of the first N characters of the human comment body (not startswith on the trimmed prefix). Or store the full original text alongside the truncated version and try exact match first, falling back to prefix match only when ... was present.

if text.endswith("..."):
    text = text[:-3].rstrip()
if text:
    replied_prefixes.append((user, text.lower()))

Checklist

  • No hardcoded secrets
  • No command/shell injection
  • Input validation at boundaries
  • No mutable default arguments
  • No unsafe eval/exec
  • Resource management (with statements)
  • Error handling not swallowing errors silently
  • No unbounded collections
  • Test isolation
  • No dead code shipped without integration path — warning #1 (cooldown infra)

Silent Failure Analysis

🟡 **MEDIUM** — swallowed exception (`koan/app/github_notification_tracker.py:176-177`)

Risk: If persisting the review cooldown fails silently, the re-review feedback loop this feature is designed to prevent will occur with no diagnostic trace — unlike the identical pattern 30 lines above which at least has an explanatory comment.

        locked_json_modify(_threads_path(instance_dir), _update)
    except OSError:
        pass

Fix: Add log.warning("Failed to set review cooldown for %s/%s#%s", owner, repo, pr_number) in the except block so operators can diagnose when the protection silently degrades, matching the comment convention of the existing handler above.


Automated review by Kōan (Claude · model claude-opus-4-6)4ac80f1

@atoomic

atoomic commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

@Koan-Bot rebase

Koan-Bot and others added 2 commits June 3, 2026 23:16
Two fixes for the regression where reviews post duplicate comments:

1. Issue comment self-reply detection: _fetch_issue_comments now collects
   bot replies and excludes human comments the bot already replied to.
   Bot replies use "> @user: text..." format — match that quote pattern
   against human comments to detect prior replies. This is the primary
   fix for the visible symptom (repeated identical replies).

2. In-progress dedup in _try_assignment_notification: the existing
   pending-mission check now also scans In Progress, preventing
   re-queuing while a review is still running.

Also adds review cooldown infrastructure to the notification tracker
(not yet wired — available for future use if needed).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Koan-Bot

Koan-Bot commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Rebase with requested adjustments

Branch koan.atoomic/fix-review-reply-loop was rebased onto main and review feedback was applied.

Changes applied

  • All three review items addressed:
  • Prefix matching tightened (review_runner.py): Non-truncated bot quotes now require exact first-line match; prefix match only applies when ... was present. Tracks was_truncated flag per quote. Added test test_exact_match_no_false_positive_on_prefix to verify two comments sharing a prefix aren't conflated.
  • TODO added to cooldown infra (github_notification_tracker.py): Added # TODO: wire into github_command_handler or loop_manager in set_review_cooldown docstring per reviewer request to mark the integration path.
  • Logging on swallowed exception (github_notification_tracker.py): Replaced bare pass in set_review_cooldown's except OSError with log.warning(...) so operators can diagnose silent cooldown failures. Added import logging and log = logging.getLogger(__name__).

Stats

5 files changed, 232 insertions(+), 11 deletions(-)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=Commit 3a0b6cf fixed self-reply loops for threaded review comments via _filter_threads, but this PR)
  • Rebased koan.atoomic/fix-review-reply-loop onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan.atoomic/fix-review-reply-loop to origin
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@Koan-Bot Koan-Bot force-pushed the koan.atoomic/fix-review-reply-loop branch from 4ac80f1 to 64a073a Compare June 4, 2026 05:17
@atoomic atoomic merged commit 3637253 into Anantys-oss:main Jun 4, 2026
5 checks passed
@Koan-Bot Koan-Bot deleted the koan.atoomic/fix-review-reply-loop branch June 4, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants