Skip to content

bot-review-gate ack fix take 2: positively recognize walkthrough-with-Run-ID as real; blacklist acks and failure notices only (supersedes PR #2525 / tsk-mk5lit) - #2900

Merged
jaylfc merged 1 commit into
devfrom
exec/tsk-qhqkdh
Sep 8, 2026

Conversation

@jaylfc

@jaylfc jaylfc commented Sep 7, 2026 •

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): bot-review-gate ack fix take 2: positively recognize walkthrough-with-Run-ID as real; blacklist acks and failure notices only (supersedes PR #2525 / tsk-mk5lit)

Autonomous build of board card tsk-qhqkdh.

  • add is_coderabbit_walkthrough() that classifies an auto-summary issue
    comment as a real review iff it carries a Run ID and at least one signal
    (quota-decrement line, no-actionable phrase, or Files-processed list)
  • remove auto-summary marker from is_coderabbit_scaffolding blacklist
  • add is_coderabbit_failure_notice() and blacklist only ack + failure notice
  • keep rate-limit stub detection unchanged
  • run scaffolding check after APPROVED/CHANGES_REQUESTED in is_real_item()
  • fold the two stub branches into one accurate message
  • red-proof with real-body fixtures: walkthrough control, ack-only, and
    failure-notice-only tests; replay guard from merged-PR shapes

Files:
.../tsk-qhqkdh-bot-review-gate-ack-fix-take2.md | 3 +
scripts/check_bot_review.py | 213 ++++++++++++---------
tests/scripts/test_check_bot_review.py | 210 ++++++++++++++------
3 files changed, 276 insertions(+), 150 deletions(-)

Summary by CodeRabbit

  • Bug Fixes
    • Valid CodeRabbit walkthrough reviews containing a Run ID and review signals are now recognized as real reviews instead of scaffolding or stubs.
    • CodeRabbit acknowledgement replies and failure notices are classified appropriately, improving review-gate results.
    • Review-gate failure messages now use clearer, more consistent stub terminology.
    • Auto-summary comments are no longer automatically treated as scaffolding when they contain decisive review information.

The six test symbols below exercised the classification path this PR replaces; the dev-vs-PR replay over 20 merged PRs (with GH_TOKEN) showed 0 verdict flips, so their removal is intentional.

Removes-Intentionally: tests/scripts/test_check_bot_review.py:TestClassify.test_rate_limit_only_message_does_not_mention_scaffolding, tests/scripts/test_check_bot_review.py:TestClassify.test_scaffolding_only_message_does_not_mention_rate_limit, tests/scripts/test_check_bot_review.py:TestClassifyZeroFindingControls.test_findings_shape_alone_is_not_a_zero_finding_pass, tests/scripts/test_check_bot_review.py:TestDetectorIsolation.test_auto_summary_body_rejected, tests/scripts/test_check_bot_review.py:TestDetectorIsolation.test_neutering_auto_summary_loses_only_its_protection, tests/scripts/test_check_bot_review.py:TestIsRealItem.test_scaffolding_bodied_review_is_not_real_whatever_the_state

…, blacklist acks and failure notices only

- add is_coderabbit_walkthrough() that classifies an auto-summary issue
  comment as a real review iff it carries a Run ID and at least one signal
  (quota-decrement line, no-actionable phrase, or Files-processed list)
- remove auto-summary marker from is_coderabbit_scaffolding blacklist
- add is_coderabbit_failure_notice() and blacklist only ack + failure notice
- keep rate-limit stub detection unchanged
- run scaffolding check after APPROVED/CHANGES_REQUESTED in is_real_item()
- fold the two stub branches into one accurate message
- red-proof with real-body fixtures: walkthrough control, ack-only, and
  failure-notice-only tests; replay guard from merged-PR shapes
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@gitar-bot

gitar-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@coderabbitai

coderabbitai Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: df1513b6-c864-45e7-b46f-f2ca89952702

📥 Commits

Reviewing files that changed from the base of the PR and between a58090f and b2cc40e.

📒 Files selected for processing (3)
  • changelog.d/tsk-qhqkdh-bot-review-gate-ack-fix-take2.md
  • scripts/check_bot_review.py
  • tests/scripts/test_check_bot_review.py

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The bot review guard now recognizes valid CodeRabbit walkthrough comments by their Run ID and review signals. It classifies acknowledgement replies and failure notices as stubs, and updates related verdicts, waiver text, and tests.

Changes

Bot review classification

Layer / File(s) Summary
Review classification rules
scripts/check_bot_review.py, tests/scripts/test_check_bot_review.py
Walkthrough comments require an auto-summary marker, Run ID, and review signal. Acknowledgement and failure notices remain scaffolding. Decisive review states count as real reviews.
Stub verdicts and messaging
scripts/check_bot_review.py, tests/scripts/test_check_bot_review.py
Rate-limit output and scaffolding use a shared stub verdict. Waiver messages name rate-limit stubs or scaffolding. Tests cover mixed stubs and updated messages.
Contract documentation and changelog
scripts/check_bot_review.py, changelog.d/tsk-qhqkdh-bot-review-gate-ack-fix-take2.md
Documentation and the changelog describe the revised classification and ordering rules.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to b2cc4

The bot review gate now accepts qualifying walkthrough reviews while continuing to reject acknowledgement, failure, and rate-limit stubs. The updated behavior and verdict messaging have targeted test coverage, with no remaining merge-blocking risk identified.

Sequence Diagram(s)

sequenceDiagram
  participant ReviewItem
  participant Detector
  participant Classifier
  ReviewItem->>Detector: provide review state and body
  Detector->>Classifier: return walkthrough, acknowledgement, or failure result
  Classifier->>ReviewItem: classify as real review or stub
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.79% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 43 functions across 2 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: it recognizes walkthrough comments with a Run ID as real reviews and limits blacklisting to acknowledgments and failure notices. It is longer than neces…
Full details: Docstring Coverage

Explanation

Docstring coverage is 62.79% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 43 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch exec/tsk-qhqkdh

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

re.IGNORECASE,
)
CODERABBIT_FAILURE_RE = re.compile(
r"failure by coderabbit\.ai|Review failed",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: CODERABBIT_FAILURE_RE pattern uses Review failed which is too broad and could match genuine review text (e.g. "This review failed to catch X"), causing false positives where real CodeRabbit reviews are incorrectly classified as failure notices.

Consider making the pattern more specific, e.g. Review failed by coderabbit\.ai|CodeRabbit review failed.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
WARNING 1
Issue Details (click to expand)

WARNING

File Line Issue
scripts/check_bot_review.py 88 CODERABBIT_FAILURE_RE pattern Review failed is too broad and could match genuine review text, causing false positives where real CodeRabbit reviews are incorrectly classified as failure notices.
Files Reviewed (3 files)
  • scripts/check_bot_review.py - 1 issue
  • tests/scripts/test_check_bot_review.py
  • changelog.d/tsk-qhqkdh-bot-review-gate-ack-fix-take2.md

Fix these issues in Kilo Cloud


Reviewed by step-3.7-flash:free · Input: 103.1K · Output: 27.7K · Cached: 1M

@jaylfc jaylfc added the gate-integrity-allow Reviewed exception: allows a PR past the gate-integrity check label Sep 8, 2026
jaylfc added a commit that referenced this pull request Sep 8, 2026
FF #2900: bot-review-gate ack fix — real-body fixtures, replay guard, fenced red, trailer
@jaylfc
jaylfc merged commit b2cc40e into dev Sep 8, 2026
47 of 51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate-integrity-allow Reviewed exception: allows a PR past the gate-integrity check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant