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
Conversation
…, 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 reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe 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. ChangesBot review classification
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to 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
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
| re.IGNORECASE, | ||
| ) | ||
| CODERABBIT_FAILURE_RE = re.compile( | ||
| r"failure by coderabbit\.ai|Review failed", |
There was a problem hiding this comment.
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.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash:free · Input: 103.1K · Output: 27.7K · Cached: 1M |
FF #2900: bot-review-gate ack fix — real-body fixtures, replay guard, fenced red, trailer
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.
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)
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
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