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
4 changes: 1 addition & 3 deletions docs/todo-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@
| 78 | 💎 Tier 3 | **ADR-041 (Rust timestamp arithmetic safety) + CLAUDE.md security 拡充 (PR #115 T3-1 採用) ★ Bb-3 follow-up** | todo5.md | S | なし (config が user-editable system boundary のとき `sanitize()` 値域検証を必須化し dependent arithmetic に `// SAFETY: <sanitize-fn> により上限保証` コメントを要求するパターンを ADR + CLAUDE.md に codify、Rust 固有の checked_add + MAX_SAFE capping + time-dependent test の 3 層を明文化。2026-05-16 に entry 登録時の旧予約番号 ADR-038 が Local LLM finding classification で占有済と判明し ADR-041 に振り直し) |
| 79 | 💎 Tier 3 | **`docs-governance.md` § Retirement Workflow に「残タスクの lifecycle 整合」要件明記 (PR #117 T3-1 採用)** | todo5.md | XS | なし (PR #117 で順位 15 を Bb-3 で吸収済として削除した際、現 Step 2「残タスクを priority table に登録」が priority table から除外するケース = 完了/deprioritize/defer を未定義だった実証。除外時の commit/PR で 3 値のいずれかを明示する要件を追加して将来の同型 ambiguity を構造的に防ぐ) |
| 81 | 🚀 Tier 1 | **cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f (defer)** | todo5.md | M | 1 観測のみで systemic 性未確認 (§A-2 P-5 PR で defer 判断、ADR-018 §追記 2026-05-08 で re-trigger 条件 = 2 件以上の同型観測を規定) |
| 83 | 🔧 Tier 2 | **cli-pr-monitor: 複合 AND guard の各条件を独立テストで検証 (PR #120 T2-1 採用)** | todo5.md | S | なし (PR #120 W-001、`enrich_with_classifier_skips_when_disabled` の test setup で複合条件分離不全) |
| 84 | 💎 Tier 3 | **グローバルルール: code-review.md に「early-return guard テスト分離」チェックリスト追記 (PR #120 T3-1 採用)** | todo5.md | XS | なし (順位 83 の知見を global rule に codify、`~/.claude/rules/common/code-review.md`、独立並列実施可) |
| 91 | 🔧 Tier 2 | **`[lint_screen]` config parse テスト (PR #132 T2-#4 採用) ★ Bundle i** | todo6.md | S | なし (PR #132 で追加した push-runner-config.toml の `[lint_screen]` section に対する toml::from_str テスト、CodeRabbit nitpick 起点、silent field rename 防止) |
| 92 | 🔧 Tier 2 | **scale-aware eval fixtures (200+ 行) — Phase d 投入前の必須 infrastructure (PR #132 T2-#5 採用) ★ Bundle i** | todo6.md | M | 順位 91 と同 PR 推奨 (Bundle i コア、PR #132 smoke で観測した mistral:7b 大規模 diff JSON 不完全 (`missing field 'screen_decision'`) を fixture 化、Phase d 着手前の改善 ループ reference point 確保) |
| 93 | 💎 Tier 3 | **`coding-style.md` Cross-File Reference Lifecycle に partial fix 例を追記 (PR #132 T3-#8 採用)** | todo6.md | XS | なし (PR #94 / #111 / #132 で反復した「変更差分外ファイルへの partial fix 再発」パターンを anti-pattern 例として codify、独立並列実施可) |
Expand Down Expand Up @@ -99,7 +97,7 @@

**Bundle g (PR #121 post-merge-feedback、monitor verdict logic + session pattern codify、2026-05-07)** ✅ 完了: 4 件採用 (Tier 1 #85、Tier 2 #86、Tier 3 #87/#88) を 2 PR で land。**g-1 (順位 85 + 86、Rust 実装 + verdict transition matrix tests) は PR #125 で land 済** (`compute_verdict` の review_state == "not_found" / "pending" guard + 12 verdict transition tests in `src/cli-pr-monitor/src/stages/monitor.rs`)。**g-2 (順位 87 + 88、global rule 追記)** は Bundle h と同 PR (#139) で land 済。Bundle f との関係: Bundle f は retry logic (rate-limit + 投稿エラー)、Bundle g は verdict logic (review_state 評価) で別軸、両者 land で post-pr-monitor の robustness が retry/verdict/state 全方向で堅牢化された状態。

**Bundle f (PR #120 post-merge-feedback、cli-pr-monitor robustness、2026-05-07)**: PR #120 (ADR-038 Phase 5: cli-finding-classifier 統合) の dogfood で post-pr-monitor の wakeup state 遷移に複数の edge case を観測。**5 件採用** (Tier 1 #80/#81、Tier 3 #82、Tier 2 #83、Tier 3 #84) で **3 層対策**: (1) 実装層 = 順位 80 / 81 (rate-limit + CR 投稿エラーの auto-retry path 整理) + 順位 82 (ADR-018 設計明文化、同 PR 推奨)、(2) test 層 = 順位 83 (複合 guard の独立 variant test)、(3) ガイド層 = 順位 84 (code-review.md checklist 追記、独立並列可)。**Sub-PR 分割推奨**: f-1 (順位 80 + 81 + 82、cli-pr-monitor + ADR、Effort M+M+S、Bundle f コア) / f-2 (順位 83、test 拡充、Effort S、独立) / f-3 (順位 84、global rule、Effort XS、独立)。Bundle f はローカル LLM dogfood (ADR-038 採用、2026-05-15) の副産物として cli-pr-monitor の堅牢化を進める位置づけ。
**Bundle f (PR #120 post-merge-feedback、cli-pr-monitor robustness、2026-05-07)**: PR #120 (ADR-038 Phase 5: cli-finding-classifier 統合) の dogfood で post-pr-monitor の wakeup state 遷移に複数の edge case を観測。**5 件採用** (Tier 1 #80/#81、Tier 3 #82、Tier 2 #83、Tier 3 #84) で **3 層対策**: (1) 実装層 = 順位 80 / 81 (rate-limit + CR 投稿エラーの auto-retry path 整理) + 順位 82 (ADR-018 設計明文化、同 PR 推奨)、(2) test 層 = 順位 83 (複合 guard の独立 variant test)、(3) ガイド層 = 順位 84 (code-review.md checklist 追記、独立並列可)。**Sub-PR 分割推奨**: f-1 (順位 80 + 81 + 82、cli-pr-monitor + ADR、Effort M+M+S、Bundle f コア) / **f-2 (順位 83、test 拡充) + f-3 (順位 84、global rule) は 2026-05-21 land 済 (Bundle B)**。Bundle f はローカル LLM dogfood (ADR-038 採用、2026-05-15) の副産物として cli-pr-monitor の堅牢化を進める位置づけ。

**Bundle c (PR #109 post-merge-feedback 堅牢化、2026-05-04)**: PR #109 で post-merge-feedback workflow が SIGPIPE で silent 中断され `.failed` marker 未生成という ADR-030 仕様違反が実証された。5 件採用 (Tier 1 #63/#64/#65 + Tier 3 #66/#67) で **3 層防御** を構築: (1) 事前防止 = 順位 65 (exe + `--help` を PreToolUse block) + 順位 66 (グローバルルールの subprocess pipe truncate 禁止)、(2) in-process recovery = 順位 63 (Drop guard / signal trap で abrupt 終了時の `.failed` marker 保証)、(3) out-of-process backstop = 順位 64 (`meta.json status=running` 5-15 分放置 reaper)。順位 67 (ADR-030 spec 拡張) は実装と同 PR で仕様/実装の整合性確保。**Sub-PR 分割推奨**: c-1 (順位 63 + 64 + 67、Rust 実装 + ADR、Effort M+M+XS、コア層) / c-2 (順位 65 + 66、hook + global rule、Effort S+XS、trigger 防止層)。c-1 と c-2 は独立に land 可能だが c-1 land 後の dogfood で recovery 機構を実証してから c-2 を入れると順位の合理性が見える順序になる。

Expand Down
44 changes: 0 additions & 44 deletions docs/todo5.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,47 +121,3 @@

---

### cli-pr-monitor: 複合 AND guard の各条件を独立テストで検証 (PR #120 T2-1 採用)

> **動機**: PR #120 で `enrich_with_classifier_skips_when_disabled` テストが `enabled=false` と `classified_findings.is_empty()` を同時に真にする setup で書かれており、`enabled=false` パスを純粋に分離できなかった。複合 guard は今後も発生しうるため独立 variant test で各条件を分離する。
>
> **参照**: PR #120 W-001、`.claude/feedback-reports/120.md` Tier 2 #1
>
> **実行優先度**: 🔧 **Tier 2** — Effort S。`cli-pr-monitor/src/stages/poll.rs::tests` の `enrich_with_classifier_*` テスト群拡充。

#### 作業計画

- [ ] `enabled=false` 単独 variant (findings 非空、classified_findings 非空) を追加
- [ ] `findings.is_empty()` 単独 variant も同様に分離
- [ ] 既存テストとの責務分担をコメントで明示

#### 完了基準

- 各 early-return 条件を単独で検証する test variant が存在
- 1 つの条件を変更したとき該当 variant のみ落ちる (= 責務分離が機械的に確認可能)

---

### グローバルルール: code-review.md に「early-return guard テスト分離」チェックリスト追記 (PR #120 T3-1 採用)

> **動機**: PR #120 W-001 (複合 AND guard テストの責務混在) の知見を `~/.claude/rules/common/code-review.md` の review checklist に codify、次回 PR レビューで活用。
>
> **参照**: PR #120 W-001、`.claude/feedback-reports/120.md` Tier 3 #1、`~/.claude/rules/common/code-review.md`
>
> **実行優先度**: 💎 **Tier 3** — Effort XS。1 行追記。順位 83 と独立に並列実施可。

#### 作業計画

- [ ] `~/.claude/rules/common/code-review.md` の review checklist セクションに 1 行追記:
- 「複合 AND の early-return guard を持つ関数のテストは、各条件を独立 variant で検証すること」
- [ ] 派生プロジェクト deploy には影響なし (global rule のみ)

#### 完了基準

- code-review.md にチェックリスト項目が追加される
- 次回複合 guard を持つ関数を含む PR でレビュー時に参照可能になる

---

---

72 changes: 66 additions & 6 deletions src/cli-pr-monitor/src/stages/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1414,13 +1414,17 @@ mod tests {
);
}

/// `enabled = false` のとき `enrich_with_classifier` は early return し
/// `classified_findings` を変更しない。
/// PR #120 W-001 follow-up (順位 83): `enrich_with_classifier` の `!config.enabled`
/// guard を **単独で** 検証する。`findings` を非空 (= `findings.is_empty()` guard
/// 不発)、`enabled = false` (= 本 guard 発火) にして 2 つの OR guard を直交させる。
///
/// `classified_findings` を空のまま渡すことで `!config.enabled` ガードのみを純粋に分離する。
/// 検証対象 field `state.classified_findings` を sentinel で pre-populate し、
/// 早期 return しなかった場合の代入 (`state.classified_findings = classified;`)
/// を sentinel 消失として検出する設計。空のまま渡すと「不変=空」が早期 return
/// 由来か他経路由来か判別できないため sentinel 必須。
#[test]
fn enrich_with_classifier_skips_when_disabled() {
use lib_report_formatter::Finding;
use crate::classifier_runner::ClassifiedFinding;

let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into());
state.findings = vec![Finding {
Expand All @@ -1431,11 +1435,67 @@ mod tests {
suggestion: "fix".into(),
source: "coderabbit".into(),
}];
let sentinel = ClassifiedFinding {
finding: Finding {
severity: "Minor".into(),
file: "sentinel.rs".into(),
line: "1".into(),
issue: "sentinel".into(),
suggestion: "must not be overwritten".into(),
source: "test".into(),
},
action: "auto_fix".into(),
action_confidence: 0.99,
normalized_issue: None,
fallback_reason: None,
};
state.classified_findings = vec![sentinel.clone()];
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"
);
}

/// `state.findings.is_empty()` guard (`enrich_with_classifier` 2 番目の早期 return)
/// を単独で検証する。`enabled = true` (明示、= `!config.enabled` guard 不発)、
/// `findings` 空 (= 本 guard 発火) にして他条件と直交させる。
#[test]
fn enrich_with_classifier_skips_when_findings_empty() {
use crate::classifier_runner::ClassifiedFinding;

let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into());
assert!(
state.classified_findings.is_empty(),
"disabled guard should prevent classification from running"
state.findings.is_empty(),
"test precondition: findings must be empty so `!enabled` guard stays unfired"
);
let sentinel = ClassifiedFinding {
finding: Finding {
severity: "Minor".into(),
file: "sentinel.rs".into(),
line: "1".into(),
issue: "sentinel".into(),
suggestion: "must not be overwritten".into(),
source: "test".into(),
},
action: "auto_fix".into(),
action_confidence: 0.99,
normalized_issue: None,
fallback_reason: None,
};
state.classified_findings = vec![sentinel.clone()];
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"
);
}

Expand Down