refactor(takt-review): review policy の anomaly 整合 (push パイプライン改善 T10) - #287
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthrough
Changesreview-anomaly ポリシーの導入
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし — レビュー指摘自体がまだ 0 件) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし) 軽量サマリー (レビュー指摘 0 件のため)
次のアクション
|
6716c1b to
3af0cc0
Compare
…t policy で shadow (push パイプライン改善 T10) pre-push の review step は takt builtin の policy: review (8,083 bytes / 185 行) を 注入していた。内容はチェックリスト型 (「DRY 違反 / TODO / any 型 は無条件 REJECT」 「Boy Scout: 変更ファイル内の既存問題も blocking」「1 件でもあれば REJECT」) で、 ADR-036 / ADR-027 が確立した anomaly-only 設計と正面から矛盾する。 instruction が「チェックリストで列挙するな」と言い policy が「このリストは無条件 REJECT」と言う状態だった。実害: run 20260715-185649 の simplicity REJECT は builtin の 「DRY 違反 = 無条件 REJECT」を根拠に ~7 分の fix iteration を誘発した。 新名称 policy review-anomaly (5,080 bytes / 112 行、-37%) を project facet に新設し shadow する。同名 shadow を避けたのは blast radius 限定のため — post-pr (2) / weekly (7) / post-merge (4) の計 13 step は review モデルが異なり対象外。 撤去: 無条件 REJECT チェックリスト 16 項目 / Boy Scout / 「1 件でも REJECT」。 維持: 「何が blocking か」ではなく「finding をどう立証・追跡するか」の規律 (Fact-check / file:line / finding_id 追跡・reopen 条件・ID 不変性)。finding_id 機構は refute-finding.md / fix.md / output-contracts が依存するため撤去不可 (ADR-048)。 REJECT 基準は各 step の instruction facet へ委譲する。 計画の対象ファイルが誤っていた: 方針は pre-push-review.yaml を指すが、同じ計画の T4 が refute_enabled = true にしたため実際に走るのは pre-push-review-refute.yaml。 計画どおり前者だけ直せば効果ゼロで、しかも review は普通に流れるため気付けない。 両 workflow を変更して解消し、あわせて ADR-047 の kill-switch を引いても T10 が 暗黙 revert されない直交性を確保した。 適用範囲は計画の「reviewer 2 step」から pre-push の review 系 4 step に拡大 (ユーザー承認済み)。verify / supervise でも矛盾が実在した — refute-finding.md 「確信が持てなければ reject」↔ policy「DRY 違反は無条件 REJECT」、supervise.md 「blocking が解決していれば push 可」↔ policy「1 件でもあれば REJECT」。 silent degrade を実測で潰した: facet 名が未解決だと takt はリテラル文字列に degrade する (ADR-048 の実事故) が、review は普通に流れるため成功と区別できない。 - takt catalog policies → review-anomaly ... [project] で解決 - takt prompt pre-push-review-refute → 新 policy 本文が展開、builtin marker (REJECT without exception / Boy Scout / Use of `any` type) は 0 件 - takt prompt post-pr-review → builtin 維持 = blast radius が pre-push 内 (control) takt prompt の exit 1 は未変更 workflow でも同一に出る既存挙動。 あわせて review-simplicity.md の lint-screen 参照 15 行を削除 ([lint_screen] enabled = false で対象ファイル常時不在 = 恒常デッドウェイト)。削除で消費側が不在に なるため、enabled = true にしても誰も読まない (silent no-op) ことを生成側の [lint_screen] コメントに明記した (ADR-051 の規律)。 dogfood run 1-2/5 を ADR-056 の観測記録テーブルに記録 (以降の run も同テーブルが単一 source。本計画は T99 で削除されるため判定用の記録を残さない)。両 run とも reviewers 2 本が APPROVE / 1 iteration / checklist 型 REJECT ゼロ。run 2 の reviewer は非ブロッキング warning 2 件を出し、うち 1 件は実際の事実誤り (lint-screen 削除を「14 行」と記載したが jj diff --stat の実測は 15 行) で、指摘を受けて 3 箇所を修正した — checklist 型ノイズを 1 件も出さずに事実誤りだけを検出しており、本 ADR の Negative「checklist 撤去で真の問題を 拾えなくなる」への反証データになる。ただし両 run とも docs-only 相当の diff のため baseline 203s (コード diff) とは直接比較できず、判定にはコード diff の run が要る。 受け入れ基準「5 run で simplicity execute 203s → 150s 以下」は 1 PR では検証不能の ため、新規 ADR-056 の bounded lifetime (判定期限 2026-07-31、ADR-047 と同期) に 引き継ぐ。効果の帰属は T4 refute と期間が重なるため、fix iteration 減は複合効果。 Rust 変更ゼロのため exe 再ビルド不要。検証: pnpm lint / lint:docs 通過 / pre-push review verdict=APPROVE。 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3af0cc0 to
27e1fcc
Compare
|
@coderabbitai review |
1 similar comment
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし — レビュー指摘自体が 0 件) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし) 軽量サマリー (レビュー指摘 0 件のため)
次のアクション
|
|
Tip For best results, initiate chat on the files or code changes.
The distinction between a successful check and the lack of substantive incremental-review output is noted; if new findings are posted later, the PR should be reassessed then. |
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし — レビュー指摘自体が 0 件) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし) 軽量サマリー (レビュー指摘 0 件のため)
次のアクション
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし — レビュー指摘自体が 0 件) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし) 軽量サマリー (レビュー指摘 0 件のため)
次のアクション
|
|
Tip For best results, initiate chat on the files or code changes.
今回の review サイクルが完了して findings または判定が投稿された場合は、その内容に基づいて再評価します。 |
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし — レビュー指摘自体が 0 件) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし) 軽量サマリー (レビュー指摘 0 件のため)
次のアクション
|
… パイプライン改善 T11) (#288) * docs(todo): CodeRabbit 監視まわりの不具合 5 件を backlog 化 (PR #287 調査、順位 318-322) PR #287 で CodeRabbit の挙動が通常と異なったため調査し、実測で裏付けた不具合を 順位 318-322 として記録する。いずれも「壊れていないように見えるが実は機能していない」 = silent failure / false-green の系統。 順位 318 (Tier 1): CR rate-limit 第3 format 未対応 + silent 化。 marker `rate limited by coderabbit.ai` は一致するが、wait-time 文言が `Next review available in: N minutes` に変わっており old/new どちらの regex にも 不一致。parse_rate_limit は extract_wait_time(body)? で None を返して静かに終了し、 「rate-limit 無し」と区別が付かない。ADR-034 の「HTML マーカーは UI 文言より stable」 という予測自体は当たっていた (marker 安定 / 文言変化) が、regex 側の脆弱性が 残っていた。旧→新→第3 で同一クラス 3 世代目。ADR-034 の troubleshooting は 「marker が常時 false」を症状として想定しており本件を発見できない。 順位 319 (Tier 1): pr-monitor.yml バックストップの重複ガードが構造的トートロジー。 skip 条件が「前回分析以降に新しいコメントが無い」だが、起動トリガー自体が coderabbitai[bot] の issue_comment のため、発火時点で必ず新コメントが存在し skip 条件は永久に成立しない。実測 5 件投稿 (うち 2 件は CR の ack のみに反応、 1 件はマージ後)。13:18 の投稿は本文で「レビュー実体の追加は無し」と自認しつつ 投稿しており、ガードが「新情報の価値」でなく「コメントの有無」を見ている証拠。 ガードが LLM prompt (助言層) にあることが原因で、決定論層 (`if:`) へ移すべき。 順位 320 (Tier 2): CR status check は実レビュー有無に関わらず pass。 skip も rate-limit も完了も一律 pass で、緑は「レビュー済み」を意味しない。 加えて CR はコメント本文を in-place 更新するため check の summary 文字列が stale になる (本件では `Review skipped: ...` 表示のまま実態は `Review limit reached`)。診断の決定打は本文の `Configuration used` (Organization UI = レビュー未開始の症状 / Path: .coderabbit.yaml = 実行された証拠)。 順位 321 (Tier 2): WP-03 クォータ設計の前提 stale + レビュー欠落穴。 .coderabbit.yaml 冒頭は「無料枠 3〜4 レビュー/時」前提だが実際は Pro + adaptive per-developer limit。ADR-040 の GPU 前提が stale だった件と同型。 WP-03 は PR あたりの削減はできても developer 単位の rolling window 枯渇に 効かない (#276-#287 の 12 PR / 約 24h が引き金と示唆)。また auto_incremental_review: false と「初回レビュー処理中の push」の組合せで 新 head が未レビューのまま残る穴があり、手動トリガーが規約依存になっている。 順位 322 (Tier 1): post-merge-feedback が repo root に scratch script を残す。 PR #287 マージ直後に analyze_transcript.py が生成され、jj auto-snapshot で 本コミットに混入する寸前だった (commit 前の jj status で発見)。 scratch_file_warning の patterns = ["__*", "_tmp_*"] に一致せず素通りする。 PR #85 と同一クラスだが、対策が deny-list (pattern 列挙) のため AI が付ける 新しい命名を先回りできないという構造的限界が露呈した。実物は削除せず scratchpad に退避 (回帰テストの fixture 候補)。 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(push-runner): docs-only 判定で rust gate を skip する決定論 routing (push パイプライン改善 T11) PR 範囲 (`<base>..@`) が docs-only (ADR-035 path 基準) のとき、diff で結果が 変わり得ない quality_gate の rust-lint-test group (実測 ~50s = gate 律速) を quality_gate 前に決定論的に skip する。 - takt (AI レビュー) と JS 系 (pnpm lint:docs) は skip しない: path から 「Rust テスト結果不変」は演繹できるが「レビュー不要」は演繹できない (docs の cross-ref / trust boundary / 事実は誤り得る。ADR-035 §適用 criteria) - ADR-035 path 基準を新 crate lib-docs-policy に集約し、cli-pr-monitor の 重複実装 (関数 + テスト 7 本) を撤去して単一実装化 - ADR-039 3 点セット: [docs_only_routing] default OFF / env DOCS_ONLY_ROUTING_DISABLE=1 kill-switch / 本 repo enabled=true で dogfood - fail-closed (ADR-043): jj 失敗 / 除外パス混入 / 判定不能はフル実行に倒す - 新規 ADR-057 (試験運用、判定期限 2026-08-15) 回帰テスト: lib-docs-policy 8 + docs_only_routing stage 9 + quality_gate skip 3 (対照付き) + config 4。配布 exe で docs-only=skip / code=full / kill-switch / disabled の 4 scenario を実 jj repo で before/after 確認。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pr-monitor.yml バックストップが CodeRabbit の投稿 (ack / rate-limit 通知含む) ごとに 「🤖 PR Monitor 分析」コメントを再投稿していた問題を修正 (PR #287 で 5 件、#304 で 3 件、 #307 で 5 件実観測)。原因は重複ガードが LLM prompt 内 (助言層) にしかなく、トリガー事象 (新規コメントの存在) 自身が prompt の skip 条件を無効化するトートロジーだったこと。 - jobs.analyze.if: に決定論ガードを追加 (ADR-042: 助言層 -> 決定論層): - issue_comment は CR walkthrough/summary マーカーを含み、かつ rate-limit placeholder でないもののみ起動する positive allowlist。ack / rate-limit 通知 / command invocation を一括除外 (denylist より確実)。マーカーは live PR #304/#307 の生 body で実検証。 - CLOSED/MERGED PR では起動しない (issue.state / pull_request.state == 'open')。 - prompt 手順 2 を「新規コメントの有無」から「分析価値のある新情報の有無」へ書換え、 ack/rate-limit/自身の分析コメントは新情報に数えない旨を明示。決定論層の二層目に降格。 - 先頭設計メモに経緯を記録。 検証: YAML parse + paren balance を node で確認、pnpm lint:docs OK。 残 (post-merge): workflow_dispatch スモーク + 実 PR dogfood 確認 (todo17.md 順位 319)。 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
policy: review(8,083 bytes / 185 行) を、pre-push 限定の新 policyreview-anomaly(5,080 bytes / 112 行、-37%) で shadow するREJECT 基準を各 step の instruction facet (ADR-036 の anomaly 設計) に委譲する
finding_id追跡・reopen 条件・ID 不変性は維持 (refute-finding/fix/output-contracts が依存するため撤去不可、ADR-048)
post-merge (4) の計 13 step は現状維持
review-simplicity.mdの lint-screen 参照 14 行 (恒常デッドウェイト) を削除し、消費側不在を生成側 config のコメントに記録 (ADR-051)
Context
Why: builtin の
policy: reviewはチェックリスト型 (「DRY 違反 / TODO /any型 は無条件 REJECT」「Boy Scout: 変更ファイル内の既存問題も blocking」「1 件でもあれば REJECT」)
で、ADR-036 / ADR-027 が確立した anomaly-only 設計と正面から矛盾していた。instruction が
「チェックリストで列挙するな」と言い policy が「このリストは無条件 REJECT」と言う状態。
実害: run
20260715-185649の simplicity REJECT は builtin の「DRY 違反 = 無条件 REJECT」を根拠に ~7 分の fix iteration を誘発した。
Trigger:
docs/push-pipeline-fix-plan.mdの T10 (execute 短縮の本丸)。Scope decision:
pre-push-review.yamlを指すが、同じ計画のT4 (PR chore(push-runner-config): refute facet の dogfood を開始 (push T4) #281) が
refute_enabled = trueにしたため実際に走るのはpre-push-review-refute.yaml。計画どおり前者だけ直せば効果ゼロで、しかも review は普通に流れるため気付けない。両 workflow を変更し、ADR-047 の kill-switch を引いても
T10 が暗黙 revert されない直交性を確保した。
見落としており、そこでも矛盾が実在した (
refute-finding.md「確信が持てなければ reject」↔ policy「DRY 違反は無条件 REJECT」、
supervise.md「blocking が解決していれば push 可」↔ policy「1 件でもあれば REJECT」)。
post-merge の 13 step を巻き込む (review モデルが異なり効果測定が成立しない)。
simplicity (203s) と並列で wall-clock の律速ではない (T10 方針 4)。
Validation
takt catalog policies→review-anomaly ... [project]で解決。silent degrade なし(facet 名が未解決だと takt はリテラル文字列に degrade し、review は普通に流れるため
成功と区別できない = ADR-048 の実事故。「ファイルを置いた」で終わらせず実測した)
takt prompt pre-push-review-refute→ 新 policy 本文が展開、builtin marker(
REJECT without exception/Boy Scout/Use of `any` type) は 0 件takt prompt post-pr-review→ builtin# Review Policyが従来どおり注入 =blast radius が pre-push 内に留まることを control で確認
pnpm pushpre-push review: verdict=APPROVE (simplicity + security とも、1 iteration / takt 175.2s / パイプライン全体 234s、2026-07-17 21:30 JST)
execute 平均 203s → 150s 以下」は 1 PR では検証不能。ADR-056 の bounded lifetime
(判定期限 2026-07-31、ADR-047 と同期) に引き継ぐ
References
docs/push-pipeline-fix-plan.mdT10 (§3 表 / §5 詳細 / §8 判定記録に実施結果を追記)Summary by CodeRabbit
レビュー改善
ドキュメント