Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/adr/adr-019-coderabbit-review-hybrid-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,20 @@ CodeRabbit は「**既にレビュー済みのコミットは再レビューし
- **決定論層 (機構)**: 監視の明示トリガー (`src/cli-pr-monitor/src/stages/review_trigger.rs`) は投稿前に `head_already_reviewed(pr, repo)` で「現 HEAD がいずれかの CodeRabbit review の `commit_id` と一致するか」を gh 照会し、一致 (= レビュー済み) なら `@coderabbitai review` を skip する。判定不能 (gh 照会失敗 / repo 未確定) は fail-open で投稿し、再レビュー欠落を招かない (確証がある `Some(true)` のときだけ skip)。
- **運用層 (規律)**: 手動で `@coderabbitai review` を投げる場合も**同一 HEAD に再投稿しない**。2026-07-05 セッションで、fix の手動 push 後に同一 HEAD へ複数回 `@coderabbitai review` を投稿し (CodeRabbit は毎回「already reviewed, nothing to do」を返すだけ)、レート枠を無駄消費した実例に由来する。新規コミット (別 SHA) への 1 回の明示トリガーは意図した消費であり抑止対象ではない (新しい修正はレビューされるべき)。

#### 判定ソースの 2 系統化と checker の head SHA 解決 (2026-07-06 追記、順位258 / WP フィードバック採用)

WP-05 の再トリガー抑止ガードは HEAD レビュー済み判定を **reviews API 単独** (CodeRabbit review の `commit_id` 照合) で実装していた。しかし CodeRabbit は「**指摘ゼロ**」で完了した (再) レビューでは **formal review object を提出せず、commit status のみで完了を通知する** (PR #247 実測: `repos/{repo}/commits/{sha}/status` に context `CodeRabbit` / state `success` / description `Review completed`)。reviews API 単独ではこの完了を検知できず、2 つの実害が出ていた:

1. **再トリガー抑止の穴 (助言層)**: 指摘ゼロでレビュー済みの HEAD を reviews API 単独では未レビューと誤判定し、fail-open で `@coderabbitai review` を再投稿してレート枠を無駄消費し得た。
2. **監視 park ループの非終了 (決定論層)**: checker (`check-ci-coderabbit`) の完了検知は元々 commit status (`repos/{repo}/commits/{sha}/statuses`) を照会していたが、head SHA を**無指定 `gh pr view` (cwd branch auto-detection)** で取得していたため、monitor 実行コンテキスト (jj workspace で cwd branch が PR branch と食い違う / `fatal: not a git repository`) で SHA 取得に失敗 → commit status が `not_found` に倒れ、指摘ゼロ完了で review_recheck が `max_review_rechecks` まで park した (PR #247 実測: recheck 0→2 で完了検知できず)。指摘ありのレビューは reviews API 由来の actionable>0 で `action_required` に落ちて止まるため、**指摘ゼロだけがハングする**症状だった。

対応 (順位258):

- **助言層 (`src/cli-pr-monitor/src/stages/review_trigger.rs`)**: `head_already_reviewed()` の判定ソースを **reviews API + commit status の 2 系統**にし、`combine_reviewed` で fail-open 合成する (いずれかが確証 `Some(true)` なら skip、両方判定不能 `None` なら投稿)。gh 照会失敗・JSON parse 不能はいずれも `None` に倒し、確証がある `Some(true)` のときだけ skip する設計は不変 (ADR-043 の fail-open 原則を維持)。純関数 `parse_commit_status_reviewed` / `combine_reviewed` に分離し、fail-open 反転 (`None` → 投稿) を含む三値分岐をユニットテストで固定。
- **決定論層 (`src/check-ci-coderabbit/src/main.rs`)**: `get_head_sha()` を **monitor が checker に渡す解決済み repo/PR で `repos/{repo}/pulls/{pr}` の `.head.sha` を照会**する形に変更し、auto-detection 依存を排除。commit status success を正しい SHA で読めるようにして park ループを終了させる。真因は commit status の parse ではなく **SHA 取得経路**にあったため、`parse_coderabbit_status` は変更せず、PR #247 実測の 6 件 status list (最新 "Review completed" success) を回帰テストに固定した。

結果として、reviews API 単独では拾えない「指摘ゼロ完了」を **commit status で補完する 2 系統構成**になり、助言層 (再トリガー抑止) と決定論層 (park ループ終了) の双方でギャップが解消される。

#### 受け入れ基準 (dogfood)

rate 解除待ちの発生が 1 回/日未満になること。導入後の実績で確認する。未達なら `auto_pause` 値 / トリガー条件を調整、または `enabled = false` (フル手動トリガー) への切替を再検討する。
Expand Down
1 change: 0 additions & 1 deletion docs/todo-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@
| 255 | 💎 Tier 3 | **ADR-040 の実測値を新 GPU (RTX PRO 5000 48GB) で再 calibration (ADR-046 WP-01 スパイクで陳腐化を観測)** | todo13.md | S | なし (ADR-038/040 が前提とする RTX 3070 8GB は RTX PRO 5000 Blackwell 48GB に更新済み。27-31B Q4 モデルが 100% GPU で動き VRAM が制約でなくなったため、ADR-040 の VRAM/latency trade-off 表と「VRAM scarcity → model swap 制約」framing が陳腐化。ADR-046 で mistral:7b / gemma4 / qwen3-coder の VRAM・latency を実測済 → ADR-040 amendment に反映、num_ctx 選定 flow の memory 軸を latency 軸へ再重み付け) |
| 256 | ⏳ Tier 5 | **classifier FP 検出強化プロンプトで格上げ候補を再評価 (WP-04 見送りの follow-up、ADR-038 amendment 由来)** | todo13.md | M | なし (WP-04 実測で全候補が FP 検出未達 = 能力限界か `classify.txt` の mistral 向け tune 不適合かが未分離。FP 検出強化プロンプト版で qwen3-coder:30b 等を再測し、能力限界と確認できれば恒久見送り、プロンプト不適合なら該当モデル + 専用プロンプトで格上げ。eval 手法・gold セットは scratchpad WP-04 資産を再利用。materially better な新モデル出現時も再評価トリガー) |
| 257 | ⏳ Tier 5 | **push pipeline の `cargo test` を cargo-nextest 化 (WP-05 で Stop hook には無効と判明、push 側 follow-up)** | todo13.md | S-M | なし (WP-05 実測: Stop hook は cargo test 不在で nextest 非適用、真因は逐次実行→並列化で解決済。ただし push pipeline (cli-push-runner quality_gate) の `cargo test -- --ignored` は実測 ~80s で nextest 高速化の余地あり。ツール依存追加 = ADR-017 pinning + 派生プロジェクト配布のコスト、push が Stop より低頻度な点を踏まえた費用対効果を評価。doctest は nextest 非実行のため `cargo test --doc` 併走が必要) |
| 258 | 🔧 Tier 2 | **CodeRabbit「指摘ゼロ再レビュー」検知ギャップ解消 — commit status を「レビュー済み」判定に追加 + fail-open 分岐テスト (PR #247 post-merge-feedback T2-2 + 2026-07-05 セッション実測採用)** | todo13.md | S-M | なし (CodeRabbit は指摘ゼロの再レビューで formal review object を提出せず summary comment 更新 + commit status のみで完了通知するため、reviews API の commit_id 照合だけの `head_already_reviewed` は該当 HEAD を未レビュー扱い (fail-open) して `@coderabbitai review` を再投稿し得る = ADR-019 再トリガー抑止ガードが quota を守れないケース。review_recheck も完了検知できず max_rechecks まで park (PR #247 実測)。commits/{sha}/status の context CodeRabbit / state success を第 2 判定ソースに追加、判定純関数化 + 三値テスト (fail-open None → 投稿の反転テスト含む、順位 162 と同型)、ADR-019 amendment) |
| 259 | 🔧 Tier 2 | **quality_gate の clippy を `--all-targets --all-features` 化 — test コード lint gap 解消 (PR #247 post-merge-feedback T2-1 採用)** | todo13.md | S | なし (現行 `cargo clippy --workspace` は lib/bin のみで #[cfg(test)] / integration test が lint されず、PR #247 で useless_format が cargo test 段階まで顕在化し手戻り。実測 2026-07-05: 追加コストはウォーム +1〜3s で rust-lint-test group (~80s) では誤差、--all-features は全 crate [features] 未定義で現時点 no-op。既存違反 1 件 (cli-merge-pipeline feedback/takt.rs の assertions_on_constants) の const assert 化クリーンアップ + templates 同期 + ADR-015 amendment を含む) |
| 260 | 💎 Tier 3 | **ADR-038 × ADR-043 の「accuracy 向上 ≠ 安全性維持」tension を cross-reference で明文化 (PR #245 post-merge-feedback T3-1 採用)** | todo13.md | XS | なし (WP-04 実測で qwen3-coder は accuracy +0.06 でも human_review 1 件を auto_fix に誤分類 = 有害な自動修正リスク。downstream 安全性優先で mistral:7b 維持と判断した根拠を ADR 相互参照で恒久化しないと将来の model evaluation で同 tension が再発。順位 261/262 と 1 docs PR bundle 推奨) |
| 261 | 💎 Tier 3 | **spike 見送り (negative result) の永続化 convention を明文化 (PR #245 post-merge-feedback T3-2 採用)** | todo13.md | XS | なし (WP-01 = ADR-046 却下 + 順位 255、WP-04 = ADR-038 amendment + 順位 256 の 2 例で「見送り → ADR 結論記録 + 計画状態更新 + follow-up の Tier 5 todo 化」3 点セットが確立済み。convention 明文化で 3 例目以降を誘導。順位 260/262 と 1 docs PR bundle 推奨) |
Expand Down
33 changes: 0 additions & 33 deletions docs/todo13.md
Original file line number Diff line number Diff line change
Expand Up @@ -772,39 +772,6 @@

---

### CodeRabbit「指摘ゼロ再レビュー」検知ギャップ解消 — commit status を「レビュー済み」判定に追加 + fail-open 分岐テスト (PR #247 post-merge-feedback T2-2 + 2026-07-05 セッション実測採用)

> **動機**: 2026-07-05 の PR #247 運用で実測した検知ギャップ。CodeRabbit は「指摘ゼロ」で完了した (再) レビューでは **formal review object (REST `pulls/{pr}/reviews`) を提出しない**。完了通知は (a) summary comment の in-place 更新 (「No actionable comments were generated in the recent review」+ 対象 commit range 記載) と (b) **commit status** (`repos/{owner}/{repo}/commits/{sha}/status` の `statuses[]` に context `CodeRabbit` / state `success` / description `Review completed`) のみ。この結果 2 つの実害が出る:
>
> 1. `head_already_reviewed()` (review_trigger.rs) は reviews API の `commit_id` 照合**だけ**で判定するため、指摘ゼロでレビュー済みの HEAD を `Some(false)` (未レビュー) と誤判定 → fail-open で `@coderabbitai review` を再投稿し得る。CodeRabbit は incremental 仕様で「already reviewed」を返すだけなので、**ADR-019 再トリガー抑止ガードが守るはずのレート枠 (WP-03) をこのケースでは守れない**。
> 2. cli-pr-monitor の review_recheck もレビュー完了を検知できず、max_rechecks (3) まで park を繰り返して人手対応になる (PR #247 で recheck 0→2 まで完了検知できないことを実測。新 HEAD の完了は summary comment 更新 + commit status success のみで通知され、reviews API には旧 HEAD の review しか存在しなかった)。
>
> **参照**: `src/cli-pr-monitor/src/stages/review_trigger.rs` (`head_already_reviewed` / `is_head_in_reviewed`)、`src/cli-pr-monitor/src/stages/poll/review_recheck.rs`、ADR-019 § 再トリガー抑止ガード (2026-07-05 追記)、ADR-022 (責務分離)、ADR-043 (fail-open は助言層に適用)。commit status の実測形状: `gh api` で `state: success` / `statuses[].context: "CodeRabbit"` / `statuses[].description: "Review completed"` (PR #247 の merge 前 HEAD で確認)。
>
> **実行優先度**: 🔧 Tier 2 — Effort S-M。ガードの本来目的 (quota 保護) の完成 + park ループ解消で監視の自律性が上がる。

#### 作業計画

- [ ] `head_already_reviewed()` に第 2 判定ソースを追加: `gh api repos/{repo}/commits/{head_sha}/status` を照会し、`statuses[]` に context `CodeRabbit` かつ state `success` があれば「レビュー済み」。既存の reviews API 照合と OR で `Some(true)` に倒す
- [ ] fail-open 原則を維持: gh 照会失敗・parse 不能は従来どおり `None` (= 投稿続行)。確証がある場合のみ skip (ADR-019 の設計思想を変えない)
- [ ] 判定ロジックを純関数に切り出し (I/O と分離)、三値 (`Some(true)` = skip / `Some(false)` = 投稿 / `None` = 投稿) の分岐テストを追加。**特に fail-open 分岐 (`None` → 投稿) の反転テスト** (PR #247 post-merge-feedback T2-2 採用分。順位 162 = hooks-stop-quality の `Option::None` path テストと同型パターン)。commit status JSON の parse も純関数化してテスト
- [ ] review_recheck のレビュー完了検知にも同じ commit status シグナルを追加し、指摘ゼロ完了で park ループが終了するようにする (ADR-022 の責務分離に注意: 判定ヘルパーは共有し、stage 間の状態は共有しない)
- [ ] ADR-019 § 再トリガー抑止ガードを amendment (判定ソースが reviews API 単独 → reviews API + commit status の 2 系統になる旨と実測根拠)
- [ ] `pnpm build:cli-pr-monitor` で exe 更新 (ビルドには Git for Windows usr/bin の cp が PATH に必要)
- [ ] 本 entry 削除 + todo-summary.md 行削除

#### 完了基準

- 指摘ゼロで再レビュー済みの HEAD への `@coderabbitai review` 再投稿が skip されること (commit status 由来の `Some(true)`)。
- review_recheck が指摘ゼロ完了を検知して park ループを終了すること。
- fail-open 分岐 (`None` → 投稿) を含む三値判定のユニットテストが存在すること。

#### 詰まっている箇所

- なし (2026-07-05 の実測ログ・commit status の実データ形状あり)。

---

### quality_gate の clippy を `--all-targets --all-features` 化 — test コード lint gap 解消 (PR #247 post-merge-feedback T2-1 採用)

> **動機**: 現行 `cargo clippy --workspace -- -D warnings` は lib/bin ターゲットのみを `cfg(test)` なしで検査するため、`#[cfg(test)]` ユニットテスト・integration test のコードが一切 lint されない。PR #247 で `useless_format` が cargo test 段階まで顕在化せず手戻りが発生した。`--all-targets` (= `--lib --bins --tests --benches --examples`) でテストコードも clippy 対象になる。
Expand Down
24 changes: 19 additions & 5 deletions src/check-ci-coderabbit/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,22 @@ fn get_current_branch() -> Result<String, String> {
}
}

fn get_head_sha() -> Result<String, String> {
run_gh(&["pr", "view", "--json", "headRefOid", "-q", ".headRefOid"])
/// 解決済み repo/PR から head SHA を取得する (順位258 harm #2 fix)。
///
/// 旧実装は無指定 `gh pr view` で cwd の branch auto-detection に依存していたが、
/// monitor は auto-detection が不安定なため checker に `--repo`/`--pr` を明示的に渡す。
/// jj workspace 等で cwd の branch が PR branch と食い違うと `gh pr view` は
/// `fatal: not a git repository` 等で SHA を取得できず、`fetch_coderabbit_commit_state`
/// が `not_found` に倒れて「指摘ゼロで commit status success 完了」の park ループが
/// 終了しなくなる (PR #247 実測)。解決済み repo/PR で `repos/{repo}/pulls/{pr}` の
/// `.head.sha` を直接照会して確実化する。
fn get_head_sha(repo: &str, pr: u64) -> Result<String, String> {
run_gh(&[
"api",
&format!("repos/{}/pulls/{}", repo, pr),
"--jq",
".head.sha",
])
}

// ─── 入力値検証 ───
Expand All @@ -224,7 +238,7 @@ fn run_check(args: CliArgs) -> CheckResult {
};

let ci = fetch_ci(&get_current_branch().unwrap_or_default());
let cr_state = fetch_coderabbit_commit_state(&repo);
let cr_state = fetch_coderabbit_commit_state(&repo, pr);

let comments_json = fetch_issue_comments_json(&repo, pr);
let new_comments = parse_new_comments(&comments_json, &args.push_time);
Expand Down Expand Up @@ -339,8 +353,8 @@ fn fetch_ci(branch: &str) -> CiStatus {
}
}

fn fetch_coderabbit_commit_state(repo: &str) -> String {
let head_sha = get_head_sha().unwrap_or_default();
fn fetch_coderabbit_commit_state(repo: &str, pr: u64) -> String {
let head_sha = get_head_sha(repo, pr).unwrap_or_default();
if head_sha.is_empty() || !is_valid_sha(&head_sha) {
return "not_found".to_string();
}
Expand Down
22 changes: 22 additions & 0 deletions src/check-ci-coderabbit/src/parsers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,28 @@ mod tests {
);
}

/// 順位258 harm #2 回帰: PR #247 head の実測 commit statuses (reverse-chronological,
/// pending/success 混在の 6 件) を与え、`.first()` = 最新 "Review completed" success を
/// 返すことを固定する。正しい head SHA さえ `fetch_coderabbit_commit_state` に渡れば
/// この list から "success" を読み取り `decide()` が park ループを止められることを示す
/// (harm #2 の真因は SHA 取得経路 `get_head_sha` であり、parse 側ではないことの根拠)。
#[test]
fn cr_status_pr247_real_shape_picks_latest_success() {
let json = r#"[
{"context": "CodeRabbit", "description": "Review completed", "state": "success"},
{"context": "CodeRabbit", "description": "Review in progress", "state": "pending"},
{"context": "CodeRabbit", "description": "Review completed", "state": "success"},
{"context": "CodeRabbit", "description": "Review in progress", "state": "pending"},
{"context": "CodeRabbit", "description": "Review skipped: incremental reviews are disabled", "state": "success"},
{"context": "CodeRabbit", "description": "Review queued", "state": "pending"}
]"#;
assert_eq!(
parse_coderabbit_status(json),
"success",
"指摘ゼロ完了の実測 status list 先頭は success。正しい SHA を渡せば完了検知できる"
);
}

#[test]
fn walkthrough_clean_detected_when_marker_present_with_header() {
let json = r#"[
Expand Down
Loading