diff --git a/docs/adr/adr-038-local-llm-finding-classification.md b/docs/adr/adr-038-local-llm-finding-classification.md index bc0322c..abc88ae 100644 --- a/docs/adr/adr-038-local-llm-finding-classification.md +++ b/docs/adr/adr-038-local-llm-finding-classification.md @@ -58,7 +58,7 @@ pub struct ClassifiedFinding { ### 失敗時の振る舞い (ブロックしない設計) -Ollama 不在 / timeout / parse 失敗 / invalid action は **fallback として `human_review` + `confidence=0.0` + `fallback_reason` を返す**。consumer (Claude / takt 等) は finding を失わず、後段で人間/Claude が判断できる。 +Ollama 不在 / timeout / parse 失敗 / invalid action は **fallback として `human_review` + `action_confidence=0.0` + `fallback_reason` を返す**。consumer (Claude / takt 等) は finding を失わず、後段で人間/Claude が判断できる。 これは [docs/local-llm-offload-analysis.md](../local-llm-offload-analysis.md) §6 の方針 (「失敗してもリトライ可能 / Claude が後段で検証」) と整合する。 diff --git a/docs/local-llm-offload-analysis.md b/docs/local-llm-offload-analysis.md index 6e18f12..e76013a 100644 --- a/docs/local-llm-offload-analysis.md +++ b/docs/local-llm-offload-analysis.md @@ -168,6 +168,27 @@ Claude Code のトークン消費・レートリミットを抑える目的で - **takt fix-trust shortcut (ADR-037) が機能**: round 2 push で convergence_verdict による Iter 3 短絡が動作、2 iter 7m54s で APPROVE - **monitor edge case**: `review_state: "not_found"` (CR 未処理) → findings 空 → takt approved → park スキップで終了する経路がある (post-pr-monitor 改善 follow-up 候補) +#### 2026-05-07: Phase 5 land (PR #120) — cli-pr-monitor / classifier 統合 + Finding C strict 化 + +##### 完了範囲 + +- **Phase 5 (A)**: `cli-pr-monitor` poll stage への classifier subprocess 統合 (`ClassifierConfig` / `classifier_runner.rs` / `state.classified_findings` / `enrich_with_classifier`)。default OFF (試験運用) +- **Finding C strict (B)**: `from_llm_output` で `normalized_issue` の改行・80 chars 超を検出して fallback。`NORMALIZED_ISSUE_MAX_CHARS=80` const 化。回帰テスト 3 件追加 +- **PR #120**: feat → CR review 1 round (Major 2 件) → takt auto-fix → squash merge `3b6a847a` (2026-05-07T04:04:38 JST、master) +- **CR Major 2 件対応**: try_wait deadlock 修正 (thread + mpsc::channel に書き換え) + stale guard 削除確認 (CR round 2 で resolution 承認) + +##### Phase 5 dogfood で観測した cli-pr-monitor robustness 課題 → Bundle f として登録 + +PR #120 の dogfood で post-pr-monitor の wakeup state 遷移と auto-retry path に複数の edge case を発見。本 doc の retirement 直接条件ではないが、ローカル LLM dogfood の副産物として monitor 堅牢化を進める Bundle f を [docs/todo.md](todo.md) priority table に登録 (順位 80-84): + +- **順位 80** (Tier 1, Bundle f): rate-limit auto-retry wakeup 予約ロジックの整理 — `auto_retry_enabled=true` でも park 未予約で exit する事象を観測 +- **順位 81** (Tier 1, Bundle f): CR 投稿エラー (`Failed to post review comments`) の auto-retry 拡張 — 既存 rate-limit detection をバイパスして retry 未発火 +- **順位 82** (Tier 3, Bundle f): ADR-018 update — rate-limit 以外の transient failure auto-retry 設計の明文化 +- **順位 83** (Tier 2, 独立): 複合 AND guard の各条件を独立テストで検証 +- **順位 84** (Tier 3, 独立): `code-review.md` に「early-return guard テスト分離」チェックリスト追記 + +詳細エントリは [docs/todo5.md](todo5.md) 末尾。Bundle f land は本 doc の retirement に必須ではない。本セッション 1 回限りの観測で頻度が未確定のため、**優先度は再観測を経て判断**する (新規フィードバックは頻度が確認できるまで優先しない方針)。 + ### 効果実測の現状 未測定。次回 Claude Code session で: @@ -181,25 +202,15 @@ Claude Code のトークン消費・レートリミットを抑える目的で 優先度順に列挙。各項目はそれぞれ独立 PR を想定。 -### A. ★ Phase 5: cli-pr-monitor / takt facet への classifier 統合 (Tier 1) +### A. ✅ Phase 5: cli-pr-monitor / takt facet への classifier 統合 (LANDED in PR #120, 2026-05-07) -- **目的**: 本 PR (#119) で land した classifier を実フローに乗せ、効果を実測する -- **作業**: - - cli-pr-monitor poll stage で `cli-finding-classifier.exe` を invoke (config-driven flag で ON/OFF) - - もしくは takt facet (例 `.takt/post-pr-review.yaml`) に classifier step 追加 - - hooks-config.toml に section 追加 (default OFF、experimental flag) -- **依存**: なし (本 PR で前提が整っている) -- **見積**: 1〜2 日 -- **ROI**: ★★★ (ADR-038 の試験運用 → 本採用 昇格条件 の 2 番目 = 「`cli-pr-monitor` または takt facet への統合」を満たす) -- **同時に検討**: prompt injection サニタイズ (auto_fix 経路ができたタイミングで実装、PR #119 の WARN ベース) +`cli-pr-monitor` poll stage に `cli-finding-classifier.exe` を subprocess で統合。`ClassifierConfig` (default OFF / 試験運用) + `state.classified_findings` field + `enrich_with_classifier` step を追加。詳細は §7 実装進捗ログ参照。 -### B. Finding C strict 化 (Phase 5 と同時着手推奨) +- **prompt injection サニタイズ**: 本 PR では未対応 (`auto_fix` execution 経路がまだ無いため)。提案 1 (lint screen facet) で auto_fix を実行する経路を作るタイミングで導入予定 -- **目的**: PR #119 で却下した normalized_issue の改行/長さ check を、auto_fix 経路ができたタイミングで導入 -- **作業**: `from_llm_output` で `s.lines().count() != 1 || s.chars().count() > 80` を弾いて `fallback("invalid normalized_issue from LLM")` に倒す + 回帰テスト 2 件追加 -- **依存**: Phase 5 と同 PR か直前 PR で着手 -- **見積**: 半日 -- **ROI**: ★★ (LLM 出力契約の堅牢化、Phase 5 の自動修正経路で必須) +### B. ✅ Finding C strict 化 (LANDED in PR #120, 2026-05-07) + +`from_llm_output` で `normalized_issue` の改行・80 chars 超を検出して fallback。`NORMALIZED_ISSUE_MAX_CHARS=80` const 化。回帰テスト 3 件追加。 ### C. Finding D: ADR-038 line 61 textual fix (low priority) diff --git a/docs/todo.md b/docs/todo.md index 1482c42..b89c1dc 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -69,6 +69,11 @@ | 77 | 🔧 Tier 2 | **Unix timestamp baseline での境界値テスト matrix (PR #115 T2-2 採用) ★ Bb-3 follow-up** | todo5.md | S | なし (`MAX_SAFE_WAIT_SECS = 1年` の根拠が現在の timestamp ~1.7e9 に依存する time-dependent。`now + 0` / `now + MAX_SAFE_WAIT_SECS` / `now + MAX_SAFE_WAIT_SECS + 1` の境界値で i64 overflow safety を自動検証、user-editable config boundary として今後の変更にも追随) | | 78 | 💎 Tier 3 | **ADR-038 (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 層を明文化) | | 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 を構造的に防ぐ) | +| 80 | 🚀 Tier 1 | **cli-pr-monitor: rate-limit auto-retry wakeup 予約ロジックの整理 (PR #120 T1-1 採用) ★ Bundle f** | todo5.md | M | なし (PR #120 dogfood で `auto_retry_enabled=true` でも wakeup park が予約されず exit する事象を観測、ADR-018 設計の実装落ち) | +| 81 | 🚀 Tier 1 | **cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f** | todo5.md | M | 順位 80 と同 PR (Bundle f コア層、failure detection と retry path 共通化、rate-limit と統合) | +| 82 | 💎 Tier 3 | **ADR-018 update: rate-limit 以外の transient failure auto-retry 設計の明文化 (PR #120 T3-2 採用) ★ Bundle f** | todo5.md | S | 順位 80 / 81 と同 PR (実装と仕様の整合確保、Bundle f) | +| 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`、独立並列実施可) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。 @@ -92,6 +97,8 @@ **PR #101 (Bundle a Sub-PR 1) post-merge-feedback 反映 (2026-05-03)**: 9 件の finding を頻度評価 (過去 report 横断 + 同一 PR latent 件数) して **3 件を採用**。**順位 47 (`>` vs `>=` boundary lint)** は同一ファイル内 3 関数 (parse_listed_findings / parse_new_comments / parse_findings) で同 drift が実証済 = latent 高頻度。**順位 48 (関数長 oxlint)** は #96 / #101 で繰り返し言及 = explicit 高頻度。両者とも Bundle Z #B-α と同じ「決定論的防止層」哲学で、Bundle Z Phase 1 (Rust comment lint) の land 後に並列 deploy 可能。**順位 49 (error-path test infra)** は #99 / #101 で同型 silent fallback anti-pattern が再発、Bundle a Sub-PR 2 (順位 42 / 43 / 46) と **同一 PR で land** 推奨 (cli-pr-monitor の mock infrastructure を再利用、test 二重投資なし)。残り 6 件 (Tier 1 #1, #3, #5、Tier 2 #2、Tier 3 #1, #2) は session 1 回限りの low-frequency events として不採用。 +**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) の副産物として cli-pr-monitor の堅牢化を進める位置づけで、`docs/local-llm-offload-analysis.md` §7 (実装進捗ログ) に dogfood signal として記録。 + **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 を入れると順位の合理性が見える順序になる。 **Bundle d (PR #110 post-merge-feedback、2026-05-04)**: PR #110 (Bundle "docs quality pre-write") merge 後の post-merge-feedback で 6 findings 中 3 件採用。共通テーマは「PR #110 で導入した `no-ephemeral-todo-reference` rule (順位 29 採用分) の robustness 強化 + 設計 doc / 実装の乖離 ガード」。**順位 68 (T2 self-exclusion test)** は placeholder N 戦略の機械的保護で OBS-3 (fragile naming convention) 対策、**順位 69 (T3 yaml/yml コメント)** は OBS-2 (spec-impl 乖離) 対策。順位 70 (code-review checklist) は Bundle e で land 済。残り順位 68 (test infra) と 順位 69 (config コメント) は scope 異なるため独立 PR 推奨。 diff --git a/docs/todo5.md b/docs/todo5.md index b204972..e66243c 100644 --- a/docs/todo5.md +++ b/docs/todo5.md @@ -822,3 +822,108 @@ - ルール追加自体は機械検知不可だが、本 task は **既存の retirement workflow の Step 2 を拡充するもの** (新規 rule の追加ではなく既存 rule の精緻化) なので、memory `feedback_no_unenforced_rules.md` の「強制力のないルール追加は却下」原則とは性質が異なる。retirement workflow を実行する commit/PR で `grep -E "完了|deprioritize|defer"` 等の機械検知を後付け可能 (ただし本 task の scope 外) - 3 値分類が実用的な粒度か、より細かい分類が必要か (例: `subsumed` を `merged into bundle` / `replaced by ADR` 等に分割) は実装時に dogfood で判断 +--- + +### cli-pr-monitor: rate-limit auto-retry wakeup 予約ロジックの整理 (PR #120 T1-1 採用) ★ Bundle f + +> **動機**: PR #120 dogfood で「`rate_limit_config.auto_retry_enabled = true / max_retries = 3` でも rate-limit 検出後に wakeup park が予約されず exit する」事象を観測。手動 `@coderabbitai review` + `CronCreate` 投入でリカバリ。ADR-018 で設計された auto-retry に実装落ちが疑われる。 +> +> **参照**: PR #120 dogfood 観測、`.claude/feedback-reports/120.md` Tier 1 #1 +> +> **実行優先度**: 🚀 **Tier 1** — Effort M。`cli-pr-monitor/src/stages/poll.rs::handle_rate_limit_branch` 周辺と state.rate_limit_retries 制御の整合性確認。 + +#### 作業計画 + +- [ ] `auto_retry_enabled = true` 経路で rate-limit 検出時に確実に `parked_review_recheck` (or 専用 reason) で park signal を emit する +- [ ] `rate_limit_retries` がインクリメントされない / 上限到達後の挙動を test infra で再現 +- [ ] 順位 81 / 82 と同 PR で land (Bundle f コア実装) + +#### 完了基準 + +- rate-limit 検出時に常に PARK signal が emit される (silent exit が消える) +- regression test (auto_retry_enabled=true で rate-limit 注入 → park 予約) が green + +--- + +### cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f + +> **動機**: PR #120 dogfood で CR walkthrough overlay が `Failed to post review comments` (rate-limit ではない transient failure) を表示するも `parse_rate_limit_status` が detected せず、auto-retry が発火しなかった。1 観測だが auto-retry の silent failure として機能不全。 +> +> **参照**: PR #120 walkthrough comment (16:41Z 投稿)、`.claude/feedback-reports/120.md` Tier 1 #2 +> +> **実行優先度**: 🚀 **Tier 1** — Effort M。`check-ci-coderabbit/src/main.rs` の failure detection 拡張 + cli-pr-monitor の retry path 共通化。 + +#### 作業計画 + +- [ ] `Review failed` / `Failed to post review comments` 等の transient failure pattern を detection に追加 +- [ ] rate-limit 系と統合する場合は state field を `transient_failure: Option` に一般化検討 +- [ ] 順位 80 と同 PR で land (Bundle f) + +#### 完了基準 + +- `Failed to post review comments` を含む walkthrough overlay 検出時に auto-retry が発火する +- regression test (failure pattern 注入 → auto-retry 発火) が green + +--- + +### ADR-018 update: rate-limit 以外の transient failure auto-retry 設計の明文化 (PR #120 T3-2 採用) ★ Bundle f + +> **動機**: 順位 80 / 81 で実装する設計方針 (rate-limit 以外の transient failure も auto-retry 範囲) を ADR-018 に明記しないと、将来の同型実装落ちが構造的に再発するリスク。 +> +> **参照**: PR #120 T1-1 / T1-2 観測、`.claude/feedback-reports/120.md` Tier 3 #2、[ADR-018](adr/adr-018-pr-monitor-takt-migration.md) +> +> **実行優先度**: 💎 **Tier 3** — Effort S。順位 80 / 81 と同 PR で同時更新 (実装と仕様の整合確保)。 + +#### 作業計画 + +- [ ] ADR-018 に「auto-retry の対象 transient failure pattern」セクション追加 +- [ ] rate-limit + 投稿エラー + (将来候補) wakeup 未予約を列挙 +- [ ] 順位 80 / 81 と同 PR で land + +#### 完了基準 + +- ADR-018 に対象 failure pattern が enumerable 形式で列挙される +- 順位 80 / 81 の実装が ADR-018 から逆引き可能 + +--- + +### 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 でレビュー時に参照可能になる +