fix(cli-pr-monitor,check-ci-coderabbit): CodeRabbit 指摘ゼロ完了の検知ギャップ解消 (順位258) - #249
Merged
Merged
Conversation
…順位258) CodeRabbit は指摘ゼロで完了した (再) レビューでは formal review object を提出せず commit status のみで完了通知する。reviews API 単独では拾えず 2 つの gap があった: 1. 助言層 (review_trigger): head_already_reviewed が reviews API のみで判定し、 指摘ゼロでレビュー済みの HEAD へ @coderabbitai review を再投稿しレート枠を浪費し得た。 reviews API + commit status の 2 系統を combine_reviewed で fail-open 合成する。 純関数 parse_commit_status_reviewed / combine_reviewed に分離し三値テスト (None→投稿の 反転テスト含む) を追加。 2. 決定論層 (check-ci-coderabbit): 完了検知は元々 commit status を照会していたが、head SHA を無指定 gh pr view (cwd auto-detection) で取得しており jj workspace 等で SHA 取得に失敗 → not_found に倒れ指摘ゼロ完了の park ループが終了しなかった (PR #247 実測)。get_head_sha を monitor が渡す解決済み repo/PR での repos/{repo}/pulls/{pr} 照会に変更。真因は parse で なく SHA 取得経路のため parse_coderabbit_status は不変、PR #247 実測 status を回帰テスト化。 ADR-019 § 再トリガー抑止ガードを amendment。todo 順位258 を消化。
📝 WalkthroughWalkthroughCodeRabbitの再レビュー済み判定ロジックを、reviews APIとcommit statusの2系統からなる三値合成方式に拡張。checker側のhead SHA取得方法をpull request直参照に修正。関連ドキュメント（ADR、TODOリスト）を更新し、対応する回帰テストとユニットテストを追加した。 Changes再トリガー抑止ガードの二重判定化
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli-pr-monitor/src/stages/review_trigger.rs (1)
83-95: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
gh pr viewに--repoを渡して cwd 依存を外してください。
head_already_reviewedはrepoを受け取っているのに、この呼び出しだけgh pr view $pr_str --json headRefOidになっています。jjワークスペースなどで repo 推測が外れるとここで空結果になり、以降の判定が丸ごとスキップされます。--repoを明示してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli-pr-monitor/src/stages/review_trigger.rs` around lines 83 - 95, The head_already_reviewed function builds a gh pr view command without using the repo argument, so it still depends on the current working directory for repo selection. Update the run_gh_quiet invocation in head_already_reviewed to pass the repo explicitly with --repo using the existing repo parameter, so the headRefOid lookup works reliably even when cwd-based repo inference fails.
🧹 Nitpick comments (1)
src/cli-pr-monitor/src/stages/review_trigger.rs (1)
129-143: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCodeRabbit context のマッチング方式が check-ci-coderabbit 側と異なる。
本関数（
eq_ignore_ascii_case("CodeRabbit")による完全一致）とsrc/check-ci-coderabbit/src/parsers.rsのparse_coderabbit_status（contains("CodeRabbit")による大文字小文字区別ありの部分一致）とで、CodeRabbit ステータス判定の意味論が異なります。現状のfixtureでは両者とも "CodeRabbit" 完全一致なので問題は顕在化しませんが、本PRの目的は2つの検出層（advisory層/deterministic層）のギャップを埋めることなので、判定ロジック自体が揃っていないと将来的なcontext文字列の変化時にドリフトするリスクがあります。共通の判定ヘルパーとして抽出するか、少なくとも同じマッチング方式（大文字小文字無視の完全一致、または部分一致）に揃えることを推奨します。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli-pr-monitor/src/stages/review_trigger.rs` around lines 129 - 143, The CodeRabbit context matching in parse_commit_status_reviewed is inconsistent with parse_coderabbit_status, so align both status checks to the same semantics. Update the matching logic in review_trigger::parse_commit_status_reviewed to use the same approach as check-ci-coderabbit/src/parsers.rs, or extract a shared helper used by both parse_commit_status_reviewed and parse_coderabbit_status so the context comparison stays consistent across advisory and deterministic paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/cli-pr-monitor/src/stages/review_trigger.rs`:
- Around line 83-95: The head_already_reviewed function builds a gh pr view
command without using the repo argument, so it still depends on the current
working directory for repo selection. Update the run_gh_quiet invocation in
head_already_reviewed to pass the repo explicitly with --repo using the existing
repo parameter, so the headRefOid lookup works reliably even when cwd-based repo
inference fails.
---
Nitpick comments:
In `@src/cli-pr-monitor/src/stages/review_trigger.rs`:
- Around line 129-143: The CodeRabbit context matching in
parse_commit_status_reviewed is inconsistent with parse_coderabbit_status, so
align both status checks to the same semantics. Update the matching logic in
review_trigger::parse_commit_status_reviewed to use the same approach as
check-ci-coderabbit/src/parsers.rs, or extract a shared helper used by both
parse_commit_status_reviewed and parse_coderabbit_status so the context
comparison stays consistent across advisory and deterministic paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 984b97bf-2b97-499a-91df-d8ecc143efde
📒 Files selected for processing (6)
docs/adr/adr-019-coderabbit-review-hybrid-policy.mddocs/todo-summary.mddocs/todo13.mdsrc/check-ci-coderabbit/src/main.rssrc/check-ci-coderabbit/src/parsers.rssrc/cli-pr-monitor/src/stages/review_trigger.rs
💤 Files with no reviewable changes (2)
- docs/todo-summary.md
- docs/todo13.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
CodeRabbit は「指摘ゼロ」で完了した (再) レビューでは formal review object (REST
pulls/{pr}/reviews) を提出せず、commit status のみで完了を通知する (contextCodeRabbit/ statesuccess/ descriptionReview completed)。reviews API 単独に依存していた 2 箇所でこの完了を検知できず実害が出ていた。順位258 (WP フィードバック採用) の対応。実害と修正
1. 助言層: 再トリガー抑止ガードの穴 (
cli-pr-monitorreview_trigger.rs)head_already_reviewed()は reviews API のcommit_id照合のみで判定していたため、指摘ゼロでレビュー済みの HEAD を未レビューと誤判定し、fail-open で@coderabbitai reviewを再投稿してレート枠を浪費し得た (ADR-019 再トリガー抑止ガードが quota を守れないケース)。combine_reviewedで fail-open 合成 (いずれか確証Some(true)なら skip、両方判定不能Noneなら投稿)parse_commit_status_reviewed/combine_reviewedに分離し、三値分岐 (fail-openNone→投稿の反転テスト含む、順位 162 と同型) をユニットテスト化2. 決定論層: park ループの非終了 (
check-ci-coderabbit)完了検知は元々 commit status を照会していたが、head SHA を無指定
gh pr view(cwd branch auto-detection) で取得しており、jj workspace 等で cwd branch が PR branch と食い違うと SHA 取得に失敗 →not_foundに倒れ、指摘ゼロ完了で review_recheck が max_review_rechecks まで park していた (PR #247 実測: recheck 0→2 で完了検知できず)。指摘ありのレビューは actionable>0 でaction_requiredに落ちて止まるため、指摘ゼロだけがハングする症状だった。get_head_sha()を monitor が渡す解決済み repo/PR でのrepos/{repo}/pulls/{pr}の.head.sha照会に変更 (auto-detection 依存を排除)parse_coderabbit_statusは不変。PR fix(cli-pr-monitor): 既レビュー済み HEAD への Add head review deduplication for Prevent duplicate @coderabbitai review posts for same HEAD review review 再トリガーを抑止 (WP-05 follow-up) #247 実測の 6 件 status list (最新 "Review completed" success) を回帰テストに固定テスト
cli-pr-monitor240 tests /check-ci-coderabbit94 tests — 全 green--all-targetsclean (テストコード含む)ドキュメント
Summary by CodeRabbit
Bug Fixes
Documentation