Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
- [ADR-038: ローカル LLM による CodeRabbit findings classification](docs/adr/adr-038-local-llm-finding-classification.md) *(試験運用)*
- [ADR-039: Experimental feature 標準パターン (config opt-in + kill-switch + bounded lifetime)](docs/adr/adr-039-experimental-feature-standard-pattern.md) *(試験運用)*
- [ADR-040: Local LLM Context Size と Resource Trade-off](docs/adr/adr-040-local-llm-context-size.md) *(試験運用)*
- [ADR-041: Test Isolation Patterns for Multi-Condition Guards](docs/adr/adr-041-test-isolation-patterns.md) *(試験運用)*

## Build

Expand Down
137 changes: 137 additions & 0 deletions docs/adr/adr-041-test-isolation-patterns.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# ADR-041: Test Isolation Patterns for Multi-Condition Guards

## ステータス

試験運用 (2026-05-22)

> 本 ADR は `~/.claude/rules/common/code-review.md` checklist (順位 84 land 済) を **project-level rationale + 具体実装例 + history** で補完する layer。global checklist は「複合 AND (OR) の early-return guard を持つ関数のテストは、各条件を独立 variant で検証している (1 guard だけ壊したとき 1 テストだけ落ちる構造)」と要件を 1 行で示すが、本 ADR は PR #120 W-001 の vacuous assertion 失敗事例と、PR #168 で sentinel pattern により構造的に解決した実装例を記録する。

## コンテキスト

`src/cli-pr-monitor/src/stages/poll.rs` の `enrich_with_classifier(state, config)` は OR-guard による早期 return を持つ:

```rust
fn enrich_with_classifier(state: &mut PrMonitorState, config: &ClassifierConfig) {
if !config.enabled || state.findings.is_empty() {
return;
}
// ... mutate state.classified_findings ...
}
```

PR #120 W-001 で初発見された問題: 当初のテスト `enrich_with_classifier_skips_when_disabled` は `enabled = false` + `findings` 空 (= `ClassifierConfig::default()` + `PrMonitorState::new()` の素直な組み合わせ) で `assert!(state.classified_findings.is_empty())` を assert していた。これは **責務混在の vacuous assertion**:

- 2 つの OR guard が同時発火しても test は pass
- 検証対象 field `classified_findings` を空のまま渡しているため、「早期 return 由来で空のまま」か「mutation 経路を通過したが結果として空」か判別不能
- 片方の guard を消した mutation テスト (`!config.enabled` だけ削除) を入れても、`findings.is_empty()` が代替発火して test は依然 pass = guard が壊れても落ちない

同型の責務混在は今後も 2+ 条件の OR/AND 早期 return を持つ pure function 系 test で再発しうるため、PR #168 で実装した sentinel pattern + 直交 precondition setup を project ADR として codify する。

## 決定

複合 AND/OR の early-return guard を持つ関数のテストは、以下 2 原則を満たす形で実装する。

### 原則 1: Sentinel pattern (検証対象 field の pre-populate)

検証対象 field を test setup 段階で `sentinel` 値で pre-populate し、「mutation 経路を通過した場合は sentinel が上書きされて消える」構造にする。assertion は `assert_eq!(field, vec![sentinel])` (生存確認) で行う。

空のまま渡す anti-pattern では、「不変=空」が早期 return 由来か他経路由来か判別不能。sentinel pattern により「早期 return が発火した → mutation が起きなかった → sentinel が survive した」と一意に推論可能になる。

### 原則 2: OR-guard precondition assertion (直交 setup)

`if !A || B { return; }` 形式の OR guard を test する場合、各 variant で **片方の guard だけが発火する直交 precondition** を組む:

- Variant 1 (左 arm): `A` を偽にする (= `!A` 発火) + `B` を偽にする (= `B` 不発)
- Variant 2 (右 arm): `A` を真にする (= `!A` 不発) + `B` を真にする (= `B` 発火)

片方が不発になる条件を test 内で **明示的に assert** または **コメント** で記述すると、将来 guard 順序が変わったり追加 guard が増えたりした場合に「test の前提が崩れた」ことを早期検出できる。

### 実装例 (poll.rs の `enrich_with_classifier` 2 variants)

`enrich_with_classifier_skips_when_disabled` (左 arm = `!enabled` 単独発火):

```rust
#[test]
fn enrich_with_classifier_skips_when_disabled() {
let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into());
// 右 arm 不発条件: findings 非空 (= findings.is_empty() guard が偽)
state.findings = vec![Finding { /* ... */ }];
// sentinel pattern: mutation 経路を通過すれば上書きされて消える
let sentinel = ClassifiedFinding { /* ... */ };
state.classified_findings = vec![sentinel.clone()];
// 左 arm 発火条件: enabled = false
let disabled = ClassifierConfig { enabled: false, ..ClassifierConfig::default() };

enrich_with_classifier(&mut state, &disabled);

assert_eq!(
state.classified_findings,
vec![sentinel],
"!config.enabled guard should early return before any mutation"
);
}
```

`enrich_with_classifier_skips_when_findings_empty` (右 arm = `findings.is_empty()` 単独発火):

```rust
#[test]
fn enrich_with_classifier_skips_when_findings_empty() {
let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into());
// 左 arm 不発条件: 明示 assert で precondition を文書化 + 機械検証
assert!(
state.findings.is_empty(),
"test precondition: findings must be empty so `!enabled` guard stays unfired"
);
// sentinel pattern (同上)
let sentinel = ClassifiedFinding { /* ... */ };
state.classified_findings = vec![sentinel.clone()];
// 右 arm 発火条件: enabled = true (= !enabled 不発)
let enabled = ClassifierConfig { enabled: true, ..ClassifierConfig::default() };

enrich_with_classifier(&mut state, &enabled);

assert_eq!(
state.classified_findings,
vec![sentinel],
"findings.is_empty() guard should early return before any mutation"
);
}
```

注目点:

- **左 arm test**: `findings` を 1 件で pre-populate して右 arm の発火条件 (`is_empty()`) を偽にする
- **右 arm test**: `PrMonitorState::new()` の自然な初期値 (`findings` 空) を使うが、**precondition assert** で「左 arm が確実に不発になる」ことを test 内で文書化 + 機械検証
- 両 variant とも `classified_findings` を sentinel で pre-populate し、mutation 不発を survival assert で検出

## 帰結

### Pros

- guard 順序が変わっても test は依然有効 (各 variant が独立に短絡 path を検証)
- mutation testing (cargo-mutants 等) で片方の guard を `true` にした mutant が確実に 1 つの test を落とす = test 救命率が上がる
- 将来複合 guard 関数を実装する際の reference として poll.rs テストを直接 cite できる
- `~/.claude/rules/common/code-review.md` checklist (順位 84) を project context (poll.rs 実装 + PR #120 W-001 history) で補完し、checklist 項目の意図が新規実装者に伝わる

### Cons / リスク

- test 関数が 1 → 2 に増える (boilerplate 増加)
- sentinel 用 dummy 値 (`ClassifiedFinding` の field を埋める) を毎回作る必要があるため、helper macro / fixture が将来必要になる可能性 (PR #168 post-merge-feedback Tier 2 #1 で「複合 guard test 再登場時に precondition assert helper 抽出を再評価」と 様子見扱い)
- 副作用検証を伴う test (write_state / println / file IO 等) には本 pattern は適用外で、別途副作用観測の test pattern が必要 (本 ADR の scope 外)

### 適用範囲

- ✅ **対象**: 2+ 条件の OR/AND 早期 return を持つ pure function 系 test (返り値または引数 mutation で結果を観測するもの)
- ❌ **対象外**: 副作用 (file IO / network / println) で結果を観測する test、単一 guard の test (1 条件のみの場合は vacuous 化リスクが本質的に存在しない)

### 既存資料との関係

- `~/.claude/rules/common/code-review.md` checklist (順位 84 land 済): global rule として「複合 guard の独立 variant 検証」要件を 1 行で codify。本 ADR は project-level の rationale + 実装例 + history で補完する layer
- PR #120 W-001 (cli-pr-monitor `enrich_with_classifier` 初期テスト): 本 ADR の動機事例
- PR #168 (順位 83 = sentinel pattern + 直交 precondition 実装): 本 ADR の reference 実装
- `src/cli-pr-monitor/src/stages/poll.rs` (`enrich_with_classifier_skips_when_disabled` / `_skips_when_findings_empty`): canonical 実装例 (関数 doc-comment から本 ADR へリンク可能)

## 改訂履歴

- 2026-05-22: 初版 (順位 139 採用 PR で land、PR #120 W-001 + PR #168 の 2 PR 横断観測ベース)
Loading