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
3 changes: 2 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@
- [ADR-021: jj 変更検出ロジックの設計原則](docs/adr/adr-021-jj-change-detection-principles.md)
- [ADR-022: 自動化コンポーネントの責務分離原則](docs/adr/adr-022-automation-responsibility-separation.md)
- [ADR-023: CodeRabbit false positive 対応スキル](docs/adr/adr-023-coderabbit-reject-thread-skill.md) *(試験運用)*
- [ADR-024: 共通 jj ヘルパーライブラリ](docs/adr/adr-024-shared-jj-helpers-library.md) *(試験運用)*
- [ADR-024: 共通 jj ヘルパーライブラリ](docs/adr/adr-024-shared-jj-helpers-library.md)
- [ADR-025: CwdRestore Drop guard パターン](docs/adr/adr-025-cwd-restore-drop-guard.md) *(試験運用)*
- [ADR-026: Cargo workspace による Rust パッケージ統合](docs/adr/adr-026-cargo-workspace.md)
- [ADR-027: Push-time review を simplicity に限定し architectural review は post-PR に委ねる](docs/adr/adr-027-push-review-simplicity-focus.md)
- [ADR-028: 外部可視成果物の生成コマンド (PR 作成/マージ) の実行ゲート](docs/adr/adr-028-pnpm-create-pr-gate.md)

## Automated actor boundary (ADR-022)

Expand Down
57 changes: 57 additions & 0 deletions docs/adr/adr-019-coderabbit-review-hybrid-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,63 @@ CodeRabbit は自身の Learning システムで「この repo/path では cross

これにより CodeRabbit のレビュー自体が徐々に適合していく。

### CodeRabbit 無料枠の制約 (2026-04-19 追記)

本プロジェクトは CodeRabbit の無料枠を前提に運用しており、以下の制約を受け入れる:

| 制約 | 影響 |
|---|---|
| **1 時間 3 回のレビュー上限** | 連続 PR 作成時に 3 本目以降のレビューがスキップされる |
| **public リポジトリ限定** | 本リポジトリが public である前提が崩れると CodeRabbit が利用不能 |
| **アカウント単位の制約** | fork / 別アカウント運用すると制約が分離される (逆用可能性あり) |

### レビュアー可換性の方針 (2026-04-19 追記)

CodeRabbit は便利だが、無料枠制約と将来的な仕様変更リスクを踏まえて「CodeRabbit 依存を固定化しない」ことを設計方針とする。

#### 「ハイブリッド」の定義 (再定義)

当初 ADR-019 起草時の「ハイブリッド」は「takt 分析 + CodeRabbit review」の意味合いが強かったが、本追記で以下に再定義する:

> **ハイブリッド = takt 内製レビュー + 外部 AI レビュー (plugin 可換)**

外部 AI レビューは CodeRabbit に限定せず、以下を交換可能な plugin として扱う:

- CodeRabbit (現行)
- GitHub Copilot Reviews
- Greptile
- その他 future 候補

#### Layer 構成は外部 AI 可換を前提に保つ

ADR-019 の 3 レイヤー構成は外部 AI の種類に依存しない形で設計されている:

- **Layer 1 (fitness filter)**: `.takt/facets/instructions/analyze-<tool>.md` を tool 別に用意する
- **Layer 2 (severity classification)**: `needs_fix` / `user_decision` / `approved` の 3-way verdict は tool 共通
- **Layer 3 (hybrid re-push)**: `auto_push_severity` の設定は tool 非依存

切り替え時の実装コストは「Layer 1 の analyze instruction を新 tool 用に書き起こす」程度に抑える設計を維持する。

#### 具体的な可換性確保策

- **CodeRabbit 固有の成果物に依存しない命名**: `pr-review` / `post-pr-monitor` 等、tool 名を含まない workflow 名を優先
- **analyze instruction は tool 別ファイル**: `analyze-coderabbit.md` / (将来) `analyze-copilot.md` のように分離
- **Rust 側 (cli-pr-monitor) は tool 固有 API に依存しない**: PR comments API を通じて取得できる汎用フォーマットに閉じる

### M5 (rate limit 耐性作り込み) を不採用とする論拠 (2026-04-19 追記)

無料枠制約に対する耐性機能 (例: "3 回超過後の自動 retry"、"レビュー失敗時の claude -p fallback") を作り込まない理由:

1. **レビュアーロックインの温床**: rate limit 耐性は CodeRabbit 固有挙動への依存を深め、可換性の方針と矛盾する
2. **投資対効果が薄い**: 1h 3 回制限は日常運用でまず引っかからない。連続 PR 作成の局面は設計上避けるべきケース (CodeRabbit rate limit 対策として PR-B/PR-C の push 間に 1 時間インターバル = 運用でカバー)
3. **無料枠に高度機能を期待しない**: 有償プラン契約の判断は別 ADR で行う。現時点では「制約を受け入れて公式経路を使う」のが最小コスト

代わりに以下で運用カバーする:

- PR 作成間隔の調整 (運用ルール、自動化なし)
- レビュー空振りを検出したら「そもそも push しない」「手動で claude -p review に切り替える」 (interactive 判断)
- ロックインが問題化した時点で plugin 可換設計の具体実装に着手 (本 ADR の方針に従う)

## 影響

### 採用される構成要素
Expand Down
49 changes: 49 additions & 0 deletions docs/adr/adr-021-jj-change-detection-principles.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,55 @@ pub fn decide_repush(

commit_id 取得失敗時 (`IdCaptureFailed`) は「変更なし」と同じ扱いにし、push を発動しない。「判定できないから念のため push」は致命的副作用 (元 description 上書き等) を招く。

### 原則 5: bookmark 検出は優先度付き revset + trunk filter を標準とする (2026-04-19 追加)

PR #54 / PR #55 で cli-merge-pipeline / cli-pr-monitor の bookmark 検出を標準化した。以下をプロジェクト共通の既定値とする:

```rust
const BOOKMARK_SEARCH_REVSETS: &[&str] = &["@", "@-", "@--"];
const TRUNK_BOOKMARKS: &[&str] = &["main", "master", "trunk", "develop"];
```

#### 問題の背景

`jj new` 直後は「`@` = 空コミット / bookmark は `@-` 上」という構成になる (PR #53 実測)。`@` だけを見る実装では bookmark 検出が空振りする。

単純に `@-` まで広げると、fresh checkout 直後に `@-` が trunk bookmark (`master` 等) を指してしまい、PR の head として trunk を誤検出する。

#### 検討した選択肢

| option | revset | 評価 | 結果 |
|---|---|---|---|
| A | `@` のみ | 空コミット状況で空振り (PR #53 実測の症状) | 不採用 |
| **B** | **`@`, `@-`, `@--` の近い順** | 最大 2 階層までカバー。trunk filter 併用で false hit 回避可 | **採用** |
| C | `ancestors(@, N)` 等の広い revset | N の決定が恣意的、遠い祖先の bookmark を誤検出するリスク | 不採用 |

option B + trunk filter で、以下の両方を成立させる:

- `@` 空 + bookmark が `@-` / `@--` 上にある一般的ケースをカバー
- fresh checkout で `@-` = `master` の場合に false hit しない

#### 検出ロジックの 3 層構造

```text
parse : jj bookmark list 出力のテキスト解析 (pure function)
query : revset を受けて bookmark 名リストを返す (副作用: jj プロセス)
select : BOOKMARK_SEARCH_REVSETS を近い順に走査し、
最初に非空かつ trunk filter 通過する revset の結果を返す
```

`select_from_revsets(revsets, query_fn)` はクロージャを受け取る pure function 設計にし、unit test で jj プロセスなしに network / revset priority / trunk filter を検証できる (ADR-021 原則 3 と整合)。

#### 実装箇所 (PR #54 / PR #55 時点)

| クレート | ファイル | 用途 |
|---|---|---|
| cli-merge-pipeline | `src/cli-merge-pipeline/src/main.rs` | `pnpm merge-pr` の PR detection |
| cli-pr-monitor | `src/cli-pr-monitor/src/util.rs` | `pnpm create-pr` の bookmark 検出 + `--head` 自動補完 |
| cli-push-runner | `src/cli-push-runner/src/stages/push_jj_bookmark.rs` | `pnpm push` の bookmark fallback |

3 クレートで定数・関数が重複している状態は ADR-024 の共通化対象 (PR-C / `docs/todo.md` #8 で `lib-jj-helpers` 抽出予定)。

## 影響

### 採用される構成要素
Expand Down
110 changes: 64 additions & 46 deletions docs/adr/adr-024-shared-jj-helpers-library.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# ADR-024 (仮): 共通 jj ヘルパーライブラリ
# ADR-024: 共通 jj ヘルパーライブラリ

## ステータス

試験運用 (観察開始: 2026-04-17)
本採用 (2026-04-19、試験運用期間: 2026-04-17 ~ 2026-04-19)

> 本 ADR は試験運用ステータス。正式採用は他パッケージでの使用例出現による
> 観察期間中 (2026-04-17 起点、想定 3.5 ヶ月) の早い段階で正式採用条件 (3 箇所の port 完了) を達成したため繰上げ本採用。実抽出作業は PR-C (`docs/todo.md` #8) で `src/lib-jj-helpers/` を新設して実施する

## コンテキスト

Expand All @@ -17,76 +17,94 @@ pub(crate) fn capture_commit_id() -> Option<String> { ... }
pub(crate) fn diff_is_empty(from: &str, to: &str) -> bool { ... }
```

現状は `pub(crate)` (cli-pr-monitor 内からのみ呼び出し)。正式採用時に package を跨いで共有するとなった段階で `pub` に引き上げる
これらは ADR-021 の「jj 変更検出の二段構え判定」を実装する基盤ヘルパーで、cli-pr-monitor 固有のロジックではない

これらは ADR-021 の「jj 変更検出の二段構え判定」を実装する基盤ヘルパーで、cli-pr-monitor 固有のロジックではない。将来以下のような場面で同じ関数が必要になる可能性がある:

- **cli-merge-pipeline の post_steps 実装** (ADR-013 + ADR-014): merge 後の AI ステップで「merge 後に変化があったか」を検出
- **cli-push-runner の bookmark 自動化** (feat/push-runner-auto-bookmark): push 前に @ が変わったか検出
- **その他の jj 連携 CLI**: 将来の拡張
本 ADR 試験運用時点では 1 箇所のみの使用で、早期の共通化は YAGNI 違反と判断して観察ステータスに留めていた。

### 早期の共通化はリスク

現時点で `src/lib-jj-helpers/` を新設して 2 関数を移すのは YAGNI に反する:

- 1 つの呼び出し元しか存在しない
- API 設計 (関数シグネチャ、エラー型、timeout 値、log 出力方法) の安定性が読めない
- 2 つ目の呼び出し例が出て初めて「共通部分」と「固有部分」を分離できる

## 決定 (試験運用方針)

### 観察期間

2026-04-17 ~ 2026-07-31 (約 3.5 ヶ月、ADR-023 と同期)。
## 決定 (本採用)

### 観察対象
### 正式採用条件の達成実績

- cli-pr-monitor 以外の Rust パッケージで `capture_commit_id` / `diff_is_empty` 相当の関数を使いたい場面が出現するか
- 出現時に「cli-pr-monitor の関数をそのまま呼ぶ」「コピペで再実装」「共通ライブラリ化」のどれが自然か
観察期間 2026-04-17 ~ 2026-07-31 を想定していたが、2026-04-19 時点で既に 3 箇所で同パターンが port 済みとなった:

### 正式採用条件 (2026-07-31 再評価)
| クレート | ファイル | 導入 PR |
|---|---|---|
| cli-pr-monitor | `src/cli-pr-monitor/src/util.rs` | PR #55 (2026-04-19) |
| cli-merge-pipeline | `src/cli-merge-pipeline/src/main.rs` | PR #54 (2026-04-19) |
| cli-push-runner | `src/cli-push-runner/src/stages/push_jj_bookmark.rs` | PR #50 (2026-04-18) |

| 他パッケージでの使用例 | アクション |
|----------------------|----------|
| 2 つ目の使用例が出現 | 正式採用 → `src/lib-jj-helpers/` 新設 |
| 1 つだけ (cli-pr-monitor のみ) | ADR 廃止 (cli-pr-monitor 内に留める) |
| 使用例なしだが明確な計画あり | 延長 (半年) |
試験運用方針で定めた「2 つ目の使用例出現で正式採用」を超えており、さらに ADR-021 原則 5 (bookmark 検出の優先度付き revset + trunk filter) で定数・関数が機械的に追加される見通しが立った。このままだと 4 箇所目以降も重複コピペが増える。

### 正式採用時の候補構成
### 採用する構成

```text
src/lib-jj-helpers/
├── Cargo.toml
└── src/
└── lib.rs ← capture_commit_id / diff_is_empty / その他共通 jj ラッパー
└── lib.rs
```

- 既存 `src/lib-report-formatter/` と同階層 (ADR-012 の命名規約 `lib-*`)
- 依存元パッケージは workspace の member として参照 (ADR-026 (予定) の Cargo workspace 化を前提)
公開する API の初期セット:

- ADR-021 原則 1-4 系 (変更検出):
- `capture_commit_id() -> Option<String>`
- `diff_is_empty(from: &str, to: &str) -> bool`
- ADR-021 原則 5 系 (bookmark 検出):
- 定数 `BOOKMARK_SEARCH_REVSETS = ["@", "@-", "@--"]`
- 定数 `TRUNK_BOOKMARKS = ["main", "master", "trunk", "develop"]`
- `is_trunk_bookmark(name: &str) -> bool`
- `parse_bookmark_list_output(stdout: &str) -> Vec<String>`
- `select_from_revsets(...)` (クロージャ注入型 pure function)
- `query_bookmarks_at(revset: &str) -> Vec<String>`
- `get_jj_bookmarks(stderr_mode: StderrMode) -> Vec<String>`

配置は ADR-012 の命名規約 `lib-*` に従い、ADR-026 の Cargo workspace の member として登録する。

### API 設計方針 (PR-C で確定)

呼び出し側 3 クレートで `stderr` ハンドリングと log prefix が異なるため、以下で吸収する:

- **`stderr` ハンドリングは引数化**: `enum StderrMode { Silent, Piped(LogFn) }` で `Stdio::null` 派 (cli-pr-monitor) と `Stdio::piped` + logging 派 (cli-merge-pipeline) を両立
- **`log_info` 注入**: `fn(&str)` クロージャを引数で受ける設計。各クレート固有 prefix (`[post-pr-monitor]` / `[merge-pipeline]` 等) を崩さない
- **fallback 方針**: log 注入設計で詰まった場合は「各クレート固有の薄いラッパー関数を残す」方針で進める (PR-C 段階で判断)

### 移行方針

PR-C (`docs/todo.md` #8) で以下を実施:

1. `src/lib-jj-helpers/` 新設、workspace member 登録
2. 共通定数・関数を移動し `pub` 公開
3. 呼び出し側 3 クレートを差し替え、各 `Cargo.toml` に依存追加
4. unit テストを `lib-jj-helpers` 側に集約、3 クレートの重複テスト削除
5. `cargo test --workspace` / `pnpm build:all` でグリーン確認

## 影響

### 試験運用中の運用
### 採用される構成要素

- cli-pr-monitor の `runner.rs` に `capture_commit_id` / `diff_is_empty` を持つ (現状維持)
- 他パッケージで同機能が必要になったら:
1. まず「cli-pr-monitor の関数を pub 化して参照」を試す
2. Cargo workspace 化 (ADR-026 (予定)) 後なら cross-package dependency で呼び出せる
3. 2 箇所以上で必要になったらライブラリ化を本 ADR の正式採用として検討
- `src/lib-jj-helpers/` (PR-C で新設予定)
- 3 呼び出し側クレート (`cli-pr-monitor` / `cli-merge-pipeline` / `cli-push-runner`) の `Cargo.toml` への依存追加

### 参照する他 ADR
### 避けるべきアンチパターン

- ADR-012 (src/ ディレクトリの命名規約): `lib-*` prefix に従う
- ADR-021 (jj 変更検出): 本 ADR のヘルパーが実装する原則
- ADR-026 (予定): Cargo workspace 化が先行する前提
- **4 箇所目のクレートが出現しても個別コピペで対応**: ADR-021 原則 5 の定数・関数が広がる機械的パターンでは保守性が崩壊する
- **`pub(crate)` のまま他クレートから呼び出そうとする**: workspace 依存で解決できるが、所在が不透明になり循環依存の温床になる
- **共通化で過度に汎用化する**: 既存 3 箇所の使い方を超える抽象化は YAGNI 違反。今必要な API だけを公開

## 次ステップ (試験運用中に確認すること)
### 参照する他 ADR

- cli-merge-pipeline の post_steps 実装 (現 docs/todo.md #3) が始まったときに使用を検討
- cli-push-runner の bookmark 自動化再開時も同様
- 使用パターンを 2 件観察してから共通化すれば、過度に汎用化せずに済む
- ADR-012 (src/ ディレクトリの命名規約): `lib-*` prefix に従う
- ADR-021 (jj 変更検出): 本 ADR のヘルパーが実装する原則 (原則 1-5 すべて)
- ADR-026 (Cargo workspace): workspace member として参照する前提

## 観察終了条件
## 次ステップ (スコープ外、PR-C で実施)

- 2026-07-31 時点で使用例を再評価
- 正式採用 / 延長 / 廃止 のいずれかを選択し、本 ADR の status を更新
- **PR-C (`docs/todo.md` #8)**: `src/lib-jj-helpers/` 新設と 3 クレート差し替え
- **将来の新規クレート**: jj 連携が必要になったらまず `lib-jj-helpers` を依存に追加することから始める
- **API 拡張**: `jj new` / `jj describe` / `jj bookmark` 系のラッパーは都度検討 (早期汎用化を避ける)
Loading