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
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
86 changes: 86 additions & 0 deletions docs/adr/adr-019-coderabbit-review-hybrid-policy.md
Original file line number Diff line number Diff line change
@@ -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 構成で処理
91 changes: 91 additions & 0 deletions docs/adr/adr-020-takt-facets-sharing.md
Original file line number Diff line number Diff line change
@@ -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 のサンプルリポジトリにも反映
11 changes: 10 additions & 1 deletion docs/todo.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 での試験運用後、本プロジェクトに反映
Comment thread
coderabbitai[bot] marked this conversation as resolved.