diff --git a/docs/todo-summary.md b/docs/todo-summary.md index 135a27b..06a7876 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -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: により上限保証` コメントを要求するパターンを 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、独立並列実施可) | @@ -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 を入れると順位の合理性が見える順序になる。 diff --git a/docs/todo5.md b/docs/todo5.md index c9afa93..e731c9a 100644 --- a/docs/todo5.md +++ b/docs/todo5.md @@ -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 でレビュー時に参照可能になる - ---- - ---- - diff --git a/src/cli-pr-monitor/src/stages/poll.rs b/src/cli-pr-monitor/src/stages/poll.rs index a2a329e..b00c59c 100644 --- a/src/cli-pr-monitor/src/stages/poll.rs +++ b/src/cli-pr-monitor/src/stages/poll.rs @@ -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 { @@ -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" ); }