From a10982096fa957f894419708345f0eade43fb21c Mon Sep 17 00:00:00 2001 From: aloekun Date: Thu, 16 Apr 2026 13:32:54 +0900 Subject: [PATCH] =?UTF-8?q?docs:=20ADR-019=20(CodeRabbit=20=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E9=81=8B=E7=94=A8)=20+=20ADR-020=20?= =?UTF-8?q?(takt=20facets=20=E5=85=B1=E9=80=9A=E5=8C=96)=20+=20todo.md=20?= =?UTF-8?q?=E5=85=B7=E4=BD=93=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ADR-019: fitness filter + severity classification + hybrid re-push の3レイヤー構成を記録 - ADR-020: fix/supervise を pre-push/post-pr で共有する判定基準と drift 防止策を記録 - todo.md: マージ後フィードバックの定常化を cli-merge-pipeline の post_steps 統合として具体化 - .claude/hooks-config.toml のフルパス化 (CodeRabbit Minor 対応) --- CLAUDE.md | 2 + ...adr-019-coderabbit-review-hybrid-policy.md | 86 ++++++++++++++++++ docs/adr/adr-020-takt-facets-sharing.md | 91 +++++++++++++++++++ docs/todo.md | 11 ++- 4 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 docs/adr/adr-019-coderabbit-review-hybrid-policy.md create mode 100644 docs/adr/adr-020-takt-facets-sharing.md diff --git a/CLAUDE.md b/CLAUDE.md index 6b2bc0fe..3a59cf75 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,6 +20,8 @@ - [ADR-016: Claude Code Bash ツールでの長時間コマンド実行戦略](docs/adr/adr-016-long-running-command-strategy.md) - [ADR-017: takt バージョン固定と検証環境の維持](docs/adr/adr-017-takt-version-pinning.md) - [ADR-018: cli-pr-monitor の takt ベース移行と CronCreate 廃止](docs/adr/adr-018-pr-monitor-takt-migration.md) *(Supersedes ADR-009 の daemon + CronCreate 部分)* +- [ADR-019: CodeRabbit レビュー運用のハイブリッド構成](docs/adr/adr-019-coderabbit-review-hybrid-policy.md) +- [ADR-020: takt facets (fix/supervise) の pre-push/post-pr 共通化戦略](docs/adr/adr-020-takt-facets-sharing.md) ## Build diff --git a/docs/adr/adr-019-coderabbit-review-hybrid-policy.md b/docs/adr/adr-019-coderabbit-review-hybrid-policy.md new file mode 100644 index 00000000..52caa1f5 --- /dev/null +++ b/docs/adr/adr-019-coderabbit-review-hybrid-policy.md @@ -0,0 +1,86 @@ +# ADR-019: CodeRabbit レビュー運用のハイブリッド構成 + +## ステータス + +承認済み (2026-04-16) + +## コンテキスト + +### 問題 + +ADR-018 で cli-pr-monitor を takt ベースに移行したが、Phase 1 は「分析のみ」で、CodeRabbit 指摘への対応は依然として Claude Code への「お願いベース」だった。また CodeRabbit は以下の特性を持つ: + +1. **プラットフォーム非依存のレビュー**: 本プロジェクトは Windows 専用だが、`.exe` ハードコードなどを cross-platform 観点で Critical/Major 指摘する +2. **深刻度の過剰評価**: false positive や設計意図に反する提案を Critical として挙げることがある +3. **修正の粒度バラつき**: 1行置換で済むものから設計変更を伴うものまで混在 + +これらを無差別に自動修正しようとすると、ADR 違反や設計意図を破壊するリスクがある。一方で全指摘をユーザー判断に委ねると、takt 化の意義（deterministic な AI 連携）が薄れる。 + +### 検証で得られた知見 + +PR #41 (Phase 2 fix loop) 実装と CodeRabbit との相互作用で以下を確認: + +- **project fitness filter が有効**: `CLAUDE.md` + ADR を参照して `not_applicable` をマークすることで、Windows 非対応指摘を除外できる +- **severity 再分類で精度向上**: CodeRabbit の severity をそのまま使うのではなく、takt の analyze ステップで再評価した方が自動修正の精度が上がる +- **ハイブリッド再 push**: Critical は自動 push、Medium 以下はユーザー確認、という設定分岐で安全性と自動化のバランスが取れる + +## 決定 + +### 3 レイヤーのレビュー対応ポリシー + +```text +[Layer 1] Project Fitness Filter (takt analyze ステップ) + ├─ CLAUDE.md + ADR を読み、適用可能性を判定 + ├─ applicable / not_applicable にマーク + └─ 不適合理由をレポートに明記 + +[Layer 2] Severity Classification (takt analyze ステップ) + ├─ applicable な findings のみ対象 + ├─ Critical / High / Major → needs_fix (自動修正対象) + ├─ Medium / Minor → user_decision (ユーザー判断) + └─ Low / Info → approved (対応不要) + +[Layer 3] Hybrid Re-push Policy (Rust cli-pr-monitor) + ├─ auto_push_severity = "critical" → 常に自動 push + ├─ auto_push_severity = "major" → 常に自動 push + ├─ auto_push_severity = "none" → 常にユーザー確認 + └─ 未知値 → fail-closed (ユーザー確認) +``` + +### 設計原則 + +1. **AI の評価を Rust で二重判定しない**: Layer 2 の判定結果 (takt が fix を実行した事実) を信頼する。Rust 側は生 findings を severity 判定に使わない +2. **fail-closed をデフォルト**: 設定値が不正な場合は自動 push せず、ユーザーに判断を委ねる +3. **fitness filter は必須**: Layer 1 をスキップすると Windows 専用プロジェクトで意味のない修正が入る +4. **verdict 値の一貫性**: takt workflow YAML の `condition` 値 (`approved` / `needs_fix` / `user_decision`) と instruction の出力例を統一する。不整合は lint で検出する (ADR-020 関連) + +### CodeRabbit Learning との連携 + +CodeRabbit は自身の Learning システムで「この repo/path では cross-platform 対応は不要」といったルールを記憶する。プロジェクト側からも以下を宣言する: + +- `CLAUDE.md` に platform scope (Windows only) を明記 +- ADR で意図的な設計決定を記録 +- `.takt/facets/instructions/analyze-coderabbit.md` で fitness filter のチェック項目を明示 + +これにより CodeRabbit のレビュー自体が徐々に適合していく。 + +## 影響 + +### 採用される構成要素 + +- `.takt/facets/instructions/analyze-coderabbit.md` (Layer 1 + Layer 2) +- `.takt/workflows/post-pr-review.yaml` の `analyze` ステップ (3-way verdict 分岐) +- `pr-monitor-config.toml` の `[fix]` セクション (`auto_push_severity`) +- `src/cli-pr-monitor/src/stages/monitor.rs` の `should_auto_push()` 純粋関数 (Layer 3) + +### 避けるべきアンチパターン + +- **生 findings ベースの auto push 判定**: Layer 1 の filter を通っていない findings を severity 判定に使うと、`not_applicable` な Critical が自動 push を誤発動させる (PR #41 CodeRabbit Major 指摘) +- **byte-position slicing**: レビュー文は日本語を含むため `str[..N]` は panic する。`truncate_safe` または `chars().take(N)` を使う (ADR-007 のカスタムリンター層 custom-lint-rules.toml に検出ルールを追加) +- **お願いベースの通知**: Claude Code に「CronCreate してください」と stdout で指示するのではなく、takt の完了を Bash tool の `run_in_background` で待つ (ADR-018 で決定済み) + +## 次ステップ (スコープ外) + +- **analyze instruction の強化**: ADR を自動検索して filter ルールを動的に抽出 +- **Learning と ADR の双方向同期**: ADR を更新したら CodeRabbit Learning にも通知 +- **他ツールのレビュー統合**: Copilot review, Greptile などの別 AI レビューも同じ Layer 構成で処理 diff --git a/docs/adr/adr-020-takt-facets-sharing.md b/docs/adr/adr-020-takt-facets-sharing.md new file mode 100644 index 00000000..ee1a01ac --- /dev/null +++ b/docs/adr/adr-020-takt-facets-sharing.md @@ -0,0 +1,91 @@ +# ADR-020: takt facets (fix/supervise) の pre-push/post-pr 共通化戦略 + +## ステータス + +承認済み (2026-04-16) + +## コンテキスト + +### 問題 + +本プロジェクトには 2 つの takt workflow が存在する: + +1. **pre-push-review** (ADR-015): push 前のローカル差分レビュー +2. **post-pr-review** (ADR-018, ADR-019): PR 作成後の CodeRabbit 指摘対応 + +両者とも「レビュー → fix → supervise」の基本構造を持ち、特に **fix** と **supervise** ステップは本質的に同じ責務を担う: + +- **fix**: 検出された指摘をコード修正する +- **supervise**: 修正が妥当か上位判断する + +PR #41 (Phase 2) 実装時、初期案では post-pr-review 専用の `fix.md` / `supervise.md` を新規作成しようとしたが、pre-push-review のものとほぼ同内容になることが判明した。 + +### 重複の弊害 + +- **instruction の drift**: 片方だけ更新されると、push 前後でレビュー基準が食い違う +- **学習コストの増加**: 寄与者が workflow ごとに別々の instruction を理解する必要がある +- **保守コスト**: CodeRabbit 指摘で fix.md を改善したら両 workflow に反映する手間 + +## 決定 + +### takt facets の責務分離 + +```text +.takt/ +├── workflows/ +│ ├── pre-push-review.yaml ← 差分取得 → 分析 → fix → supervise +│ └── post-pr-review.yaml ← CodeRabbit 取得 → 分析 → fix → supervise +│ +└── facets/instructions/ + ├── review-arch.md ← pre-push 専用 (ローカル差分の arch レビュー) + ├── review-security.md ← pre-push 専用 (ローカル差分の security レビュー) + ├── analyze-coderabbit.md ← post-pr 専用 (CodeRabbit 指摘の分析 + filter) + ├── loop-monitor-reviewers-fix.md ← 両 workflow 共有 (loop 判定) + │ + ├── fix.md ← 【共有】コード修正の共通ロジック + ├── supervise.md ← 【共有】修正妥当性の上位判断 + └── fix-supervisor.md ← 【共有】supervisor 指示での再修正 +``` + +### 共有/専用の判定基準 + +| 責務 | 共有 | 理由 | +|------|------|------| +| **入力ソースの取得** | 専用 | pre-push は `jj diff`, post-pr は CodeRabbit API で入力形式が異なる | +| **プロジェクト適合性判定** | 専用 | pre-push は書いたコードへの一次レビュー, post-pr は外部 AI のフィルタリング | +| **severity 分類** | 専用 | 入力形式が違うため、抽出ロジックも異なる | +| **コード修正** | **共有** | ソースコード + findings があれば修正方針は同じ | +| **修正の妥当性判断** | **共有** | 修正後のコード評価基準は push 前後で変わらない | +| **supervisor 再修正** | **共有** | supervisor の判断ロジックも共通 | + +### 共通 instruction の設計原則 + +1. **入力ソースに非依存**: `fix.md` は「どの形式の findings が来ても対応できる」ように書く。pre-push の `architecture-review.md` + `security-review.md` と post-pr の `coderabbit-analysis.md` のどちらも読めるように記述 +2. **workflow 固有の前提を持たない**: `.takt/review-diff.txt` が存在する前提 などを書かない (存在する場合のみ参照、という書き方にする) +3. **出力フォーマットを統一**: 修正サマリは両 workflow で同じ Markdown テンプレートを使う + +### drift 防止策 + +- **差分レビュー時のチェック**: `fix.md` / `supervise.md` を変更する PR では、両 workflow で動作確認する +- **ドキュメント化**: 各 instruction ファイルの冒頭に「このファイルは {workflow A, B} で共有されている」と明記 +- **カスタム lint (ADR-020 関連)**: workflow YAML の `instruction:` 参照先と実ファイルの存在を突き合わせる lint を追加する (scope: 次ステップ) + +## 影響 + +### 適用済み (PR #41) + +- `.takt/facets/instructions/fix.md`: pre-push-review と post-pr-review の両方で使用 +- `.takt/facets/instructions/supervise.md`: 同上 +- `.takt/facets/instructions/fix-supervisor.md`: 同上 +- `.takt/facets/instructions/loop-monitor-reviewers-fix.md`: 同上 + +### 設計の副次効果 + +- **fix ロジックの改善が両方に波及**: 一度修正すれば push 前・PR 後の両方のレビュー品質が向上 +- **新規 workflow 追加時の雛形**: 「入力取得 + 分析 (専用) → fix/supervise (共有)」のパターンが確立 + +## 次ステップ (スコープ外) + +- **instruction の参照整合性 lint**: workflow YAML の `instruction:` 参照先が facets に存在するか自動チェック +- **verdict 値の整合性 lint**: workflow の `condition` 値と instruction の出力例が一致しているか自動チェック (PR #41 の Major 指摘を再発防止) +- **takt-test-vc への還元**: 共通 facets パターンを takt のサンプルリポジトリにも反映 diff --git a/docs/todo.md b/docs/todo.md index adbe2128..d0c804cd 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -27,4 +27,13 @@ ## プロセス改善 -- [ ] **マージ後フィードバックの定常化**: PR マージ後に毎回、セッションで得られた知見を整理し「ADR として記録すべきもの」「既存の仕組みに反映すべきもの」をフィードバックとして提示する。post-merge-feedback スキルの拡張、または独立したチェックリストとして運用化を検討 +- [ ] **マージ後フィードバックの定常化** (cli-merge-pipeline の post_steps 統合案): + - **現状**: `pnpm merge-pr` マージ後に毎回「ADR として記録すべきもの」「既存の仕組みに反映すべきもの」を手動で依頼している。`docs/adr/adr-014-post-merge-feedback.md` で提唱された `post-merge-feedback` スキルは存在するが、明示的トリガーでしか動かない + - **方針**: cli-merge-pipeline の `[merge_pipeline.post_steps]` に AI ステップを追加し、マージ完了後に自動で `post-merge-feedback` スキル相当の知見整理を実行する + - **作業項目**: + - [ ] `src/cli-merge-pipeline/src/main.rs` の `run_steps` の `"ai"` 分岐を現在の `SKIP` から実装に置き換える (takt 経由で skill を起動、または claude -p で起動) + - [ ] `.claude/hooks-config.toml` の `[[merge_pipeline.post_steps]]` に `type = "ai"`, `prompt = "post-merge-feedback"` を設定 + - [ ] `post-merge-feedback` スキルが PR 番号とブランチ名を受け取れるよう、cli-merge-pipeline から環境変数または引数で渡す + - [ ] マージ済みセッションの会話ログを参照する手段 (Claude Code Session ID 等) を検討 + - **依存**: ADR-016 (長時間コマンド実行) の制約を満たすため、post_steps の AI 起動も `run_in_background` 前提で設計する + - **検証環境**: takt-test-vc での試験運用後、本プロジェクトに反映