diff --git a/.takt/facets/instructions/analyze-coderabbit.md b/.takt/facets/instructions/analyze-coderabbit.md index e494fca0..e92dd278 100644 --- a/.takt/facets/instructions/analyze-coderabbit.md +++ b/.takt/facets/instructions/analyze-coderabbit.md @@ -8,6 +8,7 @@ Read `.takt/review-comments.json`. This file contains the output from `check-ci- - `findings`: Array of structured findings (severity, file, line, issue, suggestion, source) - `action`: Terminal action from the monitor ("action_required", "stop_monitoring_success", etc.) - `summary`: Human-readable summary +- `docs_only`: Boolean ADR-035 verdict for the **whole PR**, decided deterministically by `cli-pr-monitor` from GitHub's PR-wide file list (never re-derive it from a diff) ## Task @@ -19,7 +20,7 @@ Read `.takt/review-comments.json`. This file contains the output from `check-ci- CodeRabbit sometimes raises findings that are not applicable to this project. Before classifying severity, evaluate each finding against the project context: 1. Read `CLAUDE.md` to understand the project's architecture decisions and constraints -2. **Determine if the PR is docs-only** under [ADR-035](../../../docs/adr/adr-035-doc-evaluation-policy.md): inspect the diff in `.takt/review-diff.txt`. The PR is docs-only when **all** changed files are `docs/**` / `*.md` / source-code doc comments / yaml comment-only, **and** no executable code logic changes. Excluded paths (`.takt/facets/instructions/**`, `.claude/**`, `.takt/workflows/**.yaml` structural changes, `docs/claude-code-web-tasks.md` = the nightly loop's task ledger per ADR-072) disqualify docs-only treatment even when the file extension is `.md`/`.yaml` +2. **Read the docs-only verdict** for [ADR-035](../../../docs/adr/adr-035-doc-evaluation-policy.md) from the `docs_only` field of `.takt/review-comments.json`. It is a boolean already decided by the deterministic layer (`cli-pr-monitor` classifies GitHub's PR-wide file list through `lib-docs-policy`, the single implementation of ADR-035's path criteria). **Do not derive it yourself, and do not read `.takt/review-diff.txt`** — that file is a pre-push artifact that can hold a tip-only diff or a leftover from a different PR, which is exactly how a code-bearing PR got misclassified as docs-only (PR #227). If `docs_only` is absent, treat it as `false` (an old exe wrote the file; fail-closed = apply findings normally) 3. For each finding, check: - **Platform scope**: This project targets Windows only. Findings about cross-platform compatibility (e.g., `.exe` hardcoding) are NOT applicable -- downgrade to `Info` - **Intentional design**: Check if the finding contradicts an ADR decision. If so, mark as `not_applicable` diff --git a/.takt/facets/instructions/fix.md b/.takt/facets/instructions/fix.md index 6a2e4ec5..dbe3ea8e 100644 --- a/.takt/facets/instructions/fix.md +++ b/.takt/facets/instructions/fix.md @@ -83,7 +83,12 @@ New files (post-only) and files absent from `@-` are reported `metrics_check: sk After completing all fixes (Edit/Write operations) AND before emitting the `convergence_verdict` line, refresh `.takt/review-diff.txt` so the next reviewer iteration (if `convergence_verdict: partial`) sees the post-fix state: - jj diff -r @ > .takt/review-diff.txt + jj diff --git -r 'trunk()..@' > .takt/review-diff.txt + +Both parts of that command are load-bearing — it must reproduce the scope and format `cli-push-runner` used when it first wrote the file, not a narrower one: + +- **`trunk()..@` (the whole PR), not `-r @`.** `-r @` writes the tip commit only, so on a multi-commit PR the ancestors' changes vanish from the file the next iteration reads. Reviewers then judge a PR they can only partly see, and the `analyze-coderabbit` step used to misread such a diff as a docs-only PR (todo 順位 233 / PR #227). `trunk()` is a jj builtin that resolves the remote trunk bookmark, so this works unchanged in derived projects whose default branch is `main` +- **`--git` (unified diff), not jj's default format.** The default format loses `+`/`-` markers once colour is stripped, and LLM reviewers read deletions as additions (todo 順位 264) This refresh is **unconditional**: diff --git a/docs/todo-summary2.md b/docs/todo-summary2.md index eb3dfab5..1cac664d 100644 --- a/docs/todo-summary2.md +++ b/docs/todo-summary2.md @@ -16,7 +16,6 @@ | 228 | 🔧 Tier 2 | **`evaluate_rate_limit_shortcut` の cr_clean regression test (PR #224 post-merge-feedback T2-1 採用)** | todo13.md | S | なし (Fix 3 で拡張した `unresolved_threads` / `new_comments` / `actionable_comments` 3 field の clean 判定の回帰防止、None/境界ケース網羅、silent-clean 誤認の保護。2026-08-17: 対象パスを実体 `poll/rate_limit/tests.rs` へ修正 — 旧記載 `rate_limit_signal.rs` は module 分割で消滅) | | 231 | 💎 Tier 3 | **ADR-022 拡張 — pre-create cleanup flow 例 + agent fmt スコープ指針 (PR #224 post-merge-feedback T3-1 採用)** | todo13.md | S | なし (CodeRabbit が `create_fix_commit` の空 findings 設計を bug 誤判定=却下 CR#2、agent 無差別 fmt の 2 事象を ADR-022 責務分離で codify、doc-only) | | 232 | 🔧 Tier 2 | **post-merge-feedback / workflow agent の repo 作業ツリー書込禁止 + 検知安全網 (PR #224 セッション合意)** | todo13.md | S-M | なし (merge 時に analyze-session agent が repo root に throwaway script (parse_transcript.py) を残した、日常工程ゆえ累積リスク = コンテキスト汚染。(1) feedback facets に repo 書込禁止 + jq/scratch 使用を明記 (2) post_steps/Stop hook で root 新規 untracked を warning 検知 (3) gitignore は補助) | -| 233 | 🔧 Tier 2 | **post-pr-review (takt) の diff scope を PR 全体に修正 — `@` 限定による docs-only 誤判定解消 (PR #227 観測)** | todo13.md | M | なし (PR #227 で post-pr-review analyze が `@` コミット (docs のみ) の diff を見て PR を docs-only 誤判定し、CodeRabbit が PR 全体で出した finding (create_pr.rs:208) を ADR-035 docs-only filter で誤って適用外化。今回は finding も false positive (composition root) だったため実害なしだが有効 finding 見逃しリスク。根因は ADR-027 pre-push-review の `jj diff -r @` 由来 review-diff.txt 流用 or post-pr-review 独自 @ 限定 diff 生成の疑い。diff scope を PR 全体 (base..head) に修正 or 分類を CodeRabbit findings file path 基準に変更) | | 234 | 💎 Tier 3 | **memory `feedback-di-over-ambient-global-tests` に serialization primitive 例外境界 + PR #227 具体例を追記 (PR #227 post-merge-feedback T3-1 採用)** | todo13.md | XS | なし (PR #227 が memory 原則「DI over ambient global」の 2 例目 = PR #224 env_override_lock も同根、Frequency Medium。(a) `PR_MONITOR_STATE_FILE_OVERRIDE` race → `state_path: &Path` DI 解消の具体例 (b) serialization primitive `OnceLock>` は複製禁止だが通常 test helper は複製推奨という例外境界、を追記。「DI over ambient global」と「helper 複製推奨」の見かけの矛盾を解消。順位 235 と相補) | | 235 | 💎 Tier 3 | **ADR-022 に Serialization Primitive Single-Instance Rule の Appendix 追加 (PR #227 post-merge-feedback T3-2 採用)** | todo13.md | S | なし (PR #224 T2-2 共有 env_override_lock helper 抽出 + PR #227 で同根の serialization primitive 単一化問題 2 PR 観測 = Frequency Medium。`OnceLock>` 等を複製すると各々独立した Mutex になり競合排除が破壊される特殊ケースを ADR-022 Appendix で明文化、通常 helper 複製推奨 (DRY) との例外境界を codify。ADR-046 独立化 (feedback T3-3) との overlap は着手時判断、順位 234 と相補) | | 236 | 🚀 Tier 1 | **tempfile mandate + PID+ms 命名 block の custom lint (PR #229 post-merge-feedback T1-1 採用)** | todo13.md | S | なし (#227 で修正した temp file collision flaky の再導入防止。#229 で本 flaky が push pipeline の `cargo test` を 3 回ブロックした実害。custom lint で `tempfile::Builder` / `NamedTempFile` を mandate + `gh-pr-body-{PID}-{ms}` 形式の手動命名を block。順位 237 = 検出層と二層防御) | diff --git a/docs/todo13.md b/docs/todo13.md index b0ec80d0..4783dd37 100644 --- a/docs/todo13.md +++ b/docs/todo13.md @@ -162,39 +162,6 @@ --- -### post-pr-review (takt) の diff scope を PR 全体に修正 — `@` コミット限定による docs-only 誤判定の解消 (PR #227 観測) - -> **動機**: PR #227 (cli-pr-monitor flaky 修正 2 件 + docs 整理、3 commit) の post-pr monitor で、takt `post-pr-review` の analyze が PR を **docs-only と誤判定**した。実際は spvtqwor (create_pr.rs の tempfile 化) / qzpwsyzr (state path DI) の Rust 変更を含むが、analyze が見た diff は `@` コミット (`docs/todo*.md` のみ) だった。その結果、CodeRabbit が PR 全体 (`create_pr.rs:208`) を見て出した finding を ADR-035 docs-only filter で「適用外」と**誤フィルタ**した。今回は finding 自体も false positive (composition root のため DI 不要) だったため実害はなかったが、**有効な finding を見逃すリスク**がある。 -> -> **本タスクの位置づけ**: PR #227 セッション観測 (2026-06-30)、ユーザー判断で todo 登録。CodeRabbit が PR 全体 (base..head) を見るのに対し takt の判定 diff が `@` 限定で、findings と local diff scope が構造的に不整合になる点が核心。 -> -> **参照**: PR #227、`.takt/review-comments.json` (findings = `create_pr.rs:208`)、push runner ログ `[diff] 実行: jj diff -r @` / `review-diff.txt (68 行)`、ADR-027 (push-time review は `@` の simplicity 限定、architectural review は post-PR CodeRabbit に委ねる)、ADR-035 (docs-only 評価ポリシー — classify の入力 diff scope を誤ると誤適用)、cli-pr-monitor の `post-pr-review` 起動箇所 (`stages/takt.rs` 周辺)。 -> -> **実行優先度**: 🔧 **Tier 2** — Effort M。診断 (diff scope の生成箇所特定) + 修正。実害は CodeRabbit がフル PR を見るため現状限定的だが、自動フィルタの信頼性に関わる。 - -#### 設計決定 (案) - -- **(A)** post-pr-review の analyze に渡す diff を PR 全体 (`master..@` または PR base..head) に変更する。`@` 限定の pre-push-review (ADR-027) とは射程が異なる (post-PR は PR 全体を評価すべき) ことを明示。 -- **(B)** または docs-only 分類を local diff でなく **CodeRabbit findings の file path 基準** に切り替える (findings が code file を指すなら docs-only にしない)。 -- pre-push-review (ADR-027 = `@` 限定 simplicity) と diff 生成を共有しているなら、post-pr-review 専用に分離する。 - -#### 作業計画 - -- [ ] post-pr-review が docs-only 判定に使う diff の生成箇所を特定 (`review-diff.txt` 流用 or 独自生成) -- [ ] diff scope を PR 全体に修正、or 分類基準を findings file path に変更 -- [ ] dogfood: code + docs 混在 PR で docs-only 誤判定しないことを確認 -- [ ] 本 entry 削除 + todo-summary2.md 行削除 - -#### 完了基準 - -- code 変更を含む PR が post-pr-review で docs-only と誤判定されず、code file を指す CodeRabbit finding が ADR-035 filter で誤って適用外にされない。 - -#### 詰まっている箇所 - -- diff scope を PR 全体にする際、pre-push-review (ADR-027 = `@` 限定 simplicity) との設定/生成共有部分に影響しないか。post-pr-review 専用に diff 生成を分離する必要があるか。 - ---- - ### memory `feedback-di-over-ambient-global-tests` に serialization primitive 例外境界 + PR #227 具体例を追記 (PR #227 post-merge-feedback T3-1 採用) > **動機**: PR #227 (cli-pr-monitor 並列テスト flaky 修正) の post-merge-feedback で採用候補 (T3-1) として浮上。既存 memory `feedback-di-over-ambient-global-tests` の「DI over ambient global」原則が、直感的には「通常 test helper は複製推奨」原則と矛盾するように見える問題を解消する。PR #227 は本原則の 2 例目 (PR #224 の env_override_lock 関連も同根)。 diff --git a/src/cli-pr-monitor/src/stages/collect.rs b/src/cli-pr-monitor/src/stages/collect.rs index 7435cbc4..bfc864ef 100644 --- a/src/cli-pr-monitor/src/stages/collect.rs +++ b/src/cli-pr-monitor/src/stages/collect.rs @@ -1,16 +1,48 @@ +//! Collect stage — poll 結果を `.takt/review-comments.json` に書き出す。 +//! +//! ## docs-only 判定を決定論層で行う理由 (順位 233) +//! +//! 以前は post-pr-review の `analyze-coderabbit` facet が `.takt/review-diff.txt` を +//! **目視して** PR が docs-only かを分類していた。この入力は 2 通りに壊れる: +//! +//! - `.takt/facets/instructions/fix.md` の refresh が tip 限定 diff で上書きするため、 +//! fix を挟んだ iteration では祖先コミットの code 変更が消える +//! - post-pr-review は push の後に走るので、pre-push run が diff を書かなかった場合 +//! (`DiffResult::Empty` で takt skip 等) **別 PR の残骸**がそのまま残る +//! +//! どちらでも「code を含む PR」を docs-only と誤判定し、CodeRabbit が PR 全体を見て +//! 出した finding を ADR-035 filter で誤って適用外にする (PR #227 実観測)。 +//! +//! そこで判定を決定論層へ引き上げ、**GitHub が持つ PR 全体の変更ファイル一覧**を +//! 真実源にして `lib_docs_policy` で分類し、結果を JSON の `docs_only` として渡す。 +//! facet は値を読むだけで diff を分類しない。ローカルの working copy 状態に一切 +//! 依存しないため、上の 2 経路とも構造的に消える。 +//! +//! fail-closed ([ADR-043]): 一覧を取得できなければ `docs_only: false` を書く。 +//! docs-only 扱いは ADR-035 filter を緩める方向なので、取得失敗を `true` に倒すと +//! 「検証できなかった」が「docs だけだった」に化けて finding を握り潰す。 +//! +//! **一覧が全件そろっていることも検証する**: 取得した件数と PR が申告する +//! `changedFiles` が一致しなければ `false` に倒す。「取得できた」と「全部取得できた」は +//! 別物で、後者を確かめずに分類すると、見えていないファイルが source かどうかを +//! 判断できないまま docs-only 側へ倒れる。 + use std::path::Path; use crate::log::log_info; +use crate::runner::run_gh_quiet; use crate::stages::poll::PollResult; +use crate::util::PrInfo; const OUTPUT_PATH: &str = ".takt/review-comments.json"; /// PollResult を .takt/review-comments.json に書き出す /// /// instruction (analyze-coderabbit.md) が期待するフィールド: -/// action, summary, ci, coderabbit, findings -pub(crate) fn collect_findings(result: &PollResult) -> bool { - // instruction が期待するスキーマに合わせたラッパーを構築 +/// action, summary, ci, coderabbit, findings, docs_only +pub(crate) fn collect_findings(result: &PollResult, pr_info: &PrInfo) -> bool { + let docs_only = resolve_docs_only(pr_info); + let wrapper = serde_json::json!({ "action": result.action, "summary": result.summary, @@ -18,11 +50,11 @@ pub(crate) fn collect_findings(result: &PollResult) -> bool { "coderabbit": result.coderabbit, "findings": result.findings, "check_output": result.check_output, + "docs_only": docs_only, }); let output_path = Path::new(OUTPUT_PATH); - // .takt/ ディレクトリが存在しない場合は作成 if let Some(parent) = output_path.parent() { if !parent.exists() { if let Err(e) = std::fs::create_dir_all(parent) { @@ -55,3 +87,117 @@ pub(crate) fn collect_findings(result: &PollResult) -> bool { } } } + +/// PR 全体の変更ファイル一覧と、PR が申告する変更ファイル数。 +/// +/// 2 つを別々に取得して**件数の一致を要求する**のが本 struct の存在理由で、 +/// 「一覧が全件そろっている」ことを取得元の挙動に依存せず検証するための対になっている +/// ([`classify_docs_only`])。 +struct PrFileList { + paths: Vec, + expected_count: usize, +} + +/// PR 全体の変更ファイル一覧から docs-only 判定を得る (順位 233、module doc 参照)。 +fn resolve_docs_only(pr_info: &PrInfo) -> bool { + let files = fetch_pr_files(pr_info); + let docs_only = classify_docs_only(files.as_ref()); + log_info(&format!( + "[docs_only] PR 全体の path 基準判定 (ADR-035): {} ({})", + docs_only, + describe_file_list(files.as_ref()) + )); + docs_only +} + +fn describe_file_list(files: Option<&PrFileList>) -> String { + match files { + None => "ファイル一覧を取得できませんでした".to_string(), + Some(f) => format!( + "取得 {} 件 / PR 申告 {} 件", + f.paths.len(), + f.expected_count + ), + } +} + +/// 取得済みのファイル一覧を分類する。gh 実行から切り離して単体テスト可能にする。 +/// +/// **件数の一致を先に要求する** (PR #435 CodeRabbit Major): 一覧が PR の申告数に +/// 満たなければ、見えていないファイルが source かどうかを確かめる術が無い。 +/// 欠けた分を「無かった」と扱うと docs-only 側へ倒れて ADR-035 filter が有効な +/// finding を落とすため、不一致は必ず `false` にする ([ADR-043])。 +/// 取得元の上限値 (`gh pr view --json files` の 100 件、REST の 3,000 件) を +/// 定数として持たないのは、上限が変わっても本検査が成立し続けるようにするため。 +/// +/// `None` (取得失敗) と `Some(空)` (ファイルが 1 件も無い) はどちらも `false`。 +/// 前者は fail-closed、後者は `lib_docs_policy` 側の「空は docs-only ではない」と +/// 同じ規則で、判定規則を本関数に写し取らない (ADR-035 の drift 防止)。 +fn classify_docs_only(files: Option<&PrFileList>) -> bool { + let Some(files) = files else { + return false; + }; + if files.paths.len() != files.expected_count { + return false; + } + lib_docs_policy::is_docs_only_paths(files.paths.iter().map(String::as_str)) +} + +/// PR 全体の変更ファイル一覧を GitHub から取得する。 +/// +/// `jj diff` ではなく GitHub を真実源にするのは、post-pr-review 時点のローカル +/// working copy が PR の内容と一致している保証が無いため (fix step の途中経過や、 +/// 別ブランチへ移動した後の `--monitor-only` 再実行)。 +/// +/// `repo` が解決できない場合も `None` (fail-closed): endpoint を組み立てられない。 +fn fetch_pr_files(pr_info: &PrInfo) -> Option { + let pr_number = pr_info.pr_number?; + let repo = pr_info.repo.as_deref()?; + let expected_count = fetch_changed_file_count(pr_number, repo)?; + let paths = fetch_all_file_paths(pr_number, repo)?; + Some(PrFileList { + paths, + expected_count, + }) +} + +/// PR が申告する変更ファイル数 (`changedFiles`)。一覧の全件性を検証する対照値。 +fn fetch_changed_file_count(pr_number: u64, repo: &str) -> Option { + let number = pr_number.to_string(); + let raw = run_gh_quiet(&[ + "pr", + "view", + &number, + "--repo", + repo, + "--json", + "changedFiles", + "-q", + ".changedFiles", + ])?; + raw.trim().parse::().ok() +} + +/// 変更ファイルのパスを **全ページ**取得する。 +/// +/// `gh pr view --json files` を使わないのは、100 件で**無言で切り捨てる**ため +/// (実測: 185 ファイルの PR で 100 件しか返らない / cli/cli#13338)。切り捨てられた +/// 一覧をそのまま分類すると、101 件目以降の source file が見えず docs-only へ +/// 誤って倒れる。REST の files endpoint を `--paginate` で辿る。 +fn fetch_all_file_paths(pr_number: u64, repo: &str) -> Option> { + let endpoint = format!("repos/{}/pulls/{}/files", repo, pr_number); + let raw = run_gh_quiet(&["api", "--paginate", &endpoint, "--jq", ".[].filename"])?; + let paths: Vec = raw + .lines() + .map(str::trim) + .filter(|l| !l.is_empty()) + .map(str::to_string) + .collect(); + if paths.is_empty() { + return None; + } + Some(paths) +} + +#[cfg(test)] +mod tests; diff --git a/src/cli-pr-monitor/src/stages/collect/tests.rs b/src/cli-pr-monitor/src/stages/collect/tests.rs new file mode 100644 index 00000000..9e19b12e --- /dev/null +++ b/src/cli-pr-monitor/src/stages/collect/tests.rs @@ -0,0 +1,88 @@ +//! collect stage のテスト (production は ../collect.rs)。 + +use super::*; + +fn file_list(items: &[&str]) -> PrFileList { + PrFileList { + paths: items.iter().map(|s| s.to_string()).collect(), + expected_count: items.len(), + } +} + +/// 順位 233: PR が docs-only かの判定を決定論層で行う (PR #227 誤判定の incident)。 +/// +/// 旧経路は `analyze-coderabbit` facet が `.takt/review-diff.txt` を目視分類しており、 +/// その diff が tip 限定 (fix.md の refresh) や別 PR の残骸になり得たため、code を含む +/// PR を docs-only と誤判定して CodeRabbit の finding を ADR-035 filter で握り潰した。 +mod rank233_docs_only_verdict { + use super::*; + + /// incident 再現 (bad): docs と code が混在する PR は docs-only ではない。 + /// PR #227 はこの形 (docs + create_pr.rs) で誤判定された。 + #[test] + fn mixed_docs_and_code_is_not_docs_only() { + assert!(!classify_docs_only(Some(&file_list(&[ + "docs/dev-conventions.md", + "src/cli-pr-monitor/src/stages/create_pr.rs" + ])))); + } + + /// 対 (good): 本当に docs だけなら docs-only。filter を殺していない対照。 + #[test] + fn all_docs_is_docs_only() { + assert!(classify_docs_only(Some(&file_list(&[ + "docs/dev-conventions.md", + "docs/adr/adr-035-doc-evaluation-policy.md" + ])))); + } + + /// fail-closed: 一覧を取得できなかったら docs-only 扱いにしない + /// (ADR-035 filter を緩める方向なので「検証できなかった」を true に倒さない)。 + #[test] + fn unavailable_file_list_is_not_docs_only() { + assert!(!classify_docs_only(None)); + } + + /// 判定規則を本 crate に写し取らず `lib_docs_policy` に委ねていること。 + /// 除外パス (code-equivalent) の扱いが一致するかで確認する。 + #[test] + fn excluded_code_equivalent_paths_follow_lib_docs_policy() { + assert!(!classify_docs_only(Some(&file_list(&[ + ".takt/facets/instructions/fix.md" + ])))); + assert!(!classify_docs_only(Some(&file_list(&[ + "docs/claude-code-web-tasks.md" + ])))); + } + + /// PR #435 CodeRabbit Major: 一覧が切り捨てられたケース。 + /// + /// 実測: `gh pr view --json files` は 185 ファイルの PR で 100 件しか返さない + /// (cli/cli#13338)。先頭 100 件が docs、101 件目が source という PR では、 + /// 切り捨てられた一覧だけを見ると「全部 docs」に見えてしまう。 + /// 申告数との不一致を検出して `false` に倒すことを固定する。 + #[test] + fn truncated_file_list_is_not_docs_only_even_when_every_visible_path_is_docs() { + let visible: Vec = (0..100).map(|i| format!("docs/note-{i}.md")).collect(); + let truncated = PrFileList { + paths: visible.clone(), + expected_count: 101, + }; + assert!( + lib_docs_policy::is_docs_only_paths(visible.iter().map(String::as_str)), + "前提: 見えている 100 件だけなら docs-only に見える" + ); + assert!(!classify_docs_only(Some(&truncated))); + } + + /// 対 (good): 件数が一致していれば従来どおり判定する。 + /// 不一致検査が正常系まで巻き込んでいないことの対照。 + #[test] + fn complete_file_list_still_classifies_normally() { + let complete = PrFileList { + paths: (0..100).map(|i| format!("docs/note-{i}.md")).collect(), + expected_count: 100, + }; + assert!(classify_docs_only(Some(&complete))); + } +} diff --git a/src/cli-pr-monitor/src/stages/monitor.rs b/src/cli-pr-monitor/src/stages/monitor.rs index eb4ea4bd..54eff2d8 100644 --- a/src/cli-pr-monitor/src/stages/monitor.rs +++ b/src/cli-pr-monitor/src/stages/monitor.rs @@ -137,7 +137,7 @@ fn run_takt_stage( if !has_coderabbit_findings { return outcome; } - if !collect_findings(poll_result) { + if !collect_findings(poll_result, pr_info) { log_info("review-comments.json 書き出し失敗 (takt 分析をスキップ)"); return outcome; } diff --git a/src/lib-docs-policy/src/lib.rs b/src/lib-docs-policy/src/lib.rs index 42185899..e963d523 100644 --- a/src/lib-docs-policy/src/lib.rs +++ b/src/lib-docs-policy/src/lib.rs @@ -10,6 +10,10 @@ //! - `cli-pr-monitor` の auto-push 前 gate (fix diff が docs-only なら gate skip) //! - `cli-push-runner` の docs_only_routing stage (PR 範囲が docs-only なら //! docs で結果が変わり得ない quality_gate group を skip) +//! - `cli-pr-monitor` の collect stage (PR 全体が docs-only かを判定し +//! `.takt/review-comments.json` の `docs_only` として post-pr-review へ渡す。 +//! 順位 233: 以前は analyze facet が `.takt/review-diff.txt` を目視分類しており、 +//! その diff が tip 限定 / 別 PR の残骸になり得たため誤判定した) //! //! ## 本 crate が判定する範囲 (path 基準のみ) //! @@ -47,6 +51,38 @@ pub fn is_docs_only_summary(summary: &str) -> bool { saw_any } +/// 変更ファイルの**パス一覧**が docs-only か判定する (ADR-035 path 基準)。 +/// +/// [`is_docs_only_summary`] と判定規則は同一で、入力形式だけが違う: `jj diff --summary` +/// のように status を持たない source (例 `gh pr view --json files`) から呼ぶための入口。 +/// 判定を [`is_docs_only_path`] の単一実装に集約するのが目的で、呼び出し側がパスを +/// 自前で分類し始めると ADR-035 が防ごうとした drift を再生産する。 +/// +/// fail-closed: 空イテレータ (= 変更ファイルを 1 つも観測できなかった) は `false`。 +/// 「取得に失敗した」と「本当に docs だけだった」を同じ `true` に潰さないための線引きで、 +/// 呼び出し側は docs-only 向けの緩い経路ではなく通常経路へ倒れる ([ADR-043])。 +/// +/// **`is_docs_only_summary` を本関数へ委譲させない** (pre-push simplicity review の +/// 非 blocking 指摘に対する判断)。ループ制御は同形だが、空要素の扱いが違う: 本関数は +/// 空文字を「要素なし」として読み飛ばす一方、summary 版は `M ` のような**パスが欠けた +/// 行をパース不能として `false` に倒す**。委譲に置き換えると `M \nM docs/a.md` が +/// `true` に変わり、summary 版が持つ「壊れた行を見たら docs-only を主張しない」性質が +/// 緩む。共有すべきなのは判定規則 (= `is_docs_only_path`) であって、入力形式ごとの +/// 妥当性検査ではない。 +pub fn is_docs_only_paths<'a, I: IntoIterator>(paths: I) -> bool { + let mut saw_any = false; + for path in paths { + if path.trim().is_empty() { + continue; + } + saw_any = true; + if !is_docs_only_path(path) { + return false; + } + } + saw_any +} + /// 夜間ループ (ADR-072) のタスク台帳。 /// /// `docs/` 配下だが「次に何を実装するか」を決める**実行入力**であり、無人可マークの @@ -155,4 +191,60 @@ mod tests { assert!(is_docs_only_summary("M docs\\notes.md")); assert!(!is_docs_only_summary("M .takt\\facets\\instructions\\fix.md")); } + + /// 順位 233: status を持たない source (`gh pr view --json files`) からの入口。 + /// 判定規則は summary 版と同一であることを、同じ入力の対で固定する。 + mod rank233_paths_entry_point { + use super::*; + + #[test] + fn paths_accepts_all_docs_paths() { + assert!(is_docs_only_paths([ + "docs/adr/adr-001.md", + "docs/guide.md", + "README.md" + ])); + } + + #[test] + fn paths_rejects_mixed_docs_and_source() { + assert!(!is_docs_only_paths(["docs/a.md", "src/lib.rs"])); + } + + #[test] + fn paths_rejects_excluded_code_equivalent_paths() { + assert!(!is_docs_only_paths([".takt/facets/instructions/fix.md"])); + assert!(!is_docs_only_paths(["docs/claude-code-web-tasks.md"])); + } + + /// fail-closed: 1 件も観測できなかった入力を「docs だけだった」に潰さない。 + #[test] + fn paths_rejects_empty_input() { + assert!(!is_docs_only_paths(std::iter::empty())); + assert!(!is_docs_only_paths(["", " "])); + } + + /// 2 関数を統合しない理由 (`is_docs_only_paths` の doc) を実行可能にする。 + /// summary 版はパスが欠けた行を「壊れた入力」として `false` に倒す一方、 + /// paths 版は空要素を読み飛ばす — この非対称が委譲を許さない根拠なので、 + /// 委譲へ書き換えたら落ちる形で固定する。 + #[test] + fn summary_rejects_a_path_less_line_that_paths_would_skip() { + assert!(!is_docs_only_summary("M \nM docs/a.md")); + assert!(is_docs_only_paths(["", "docs/a.md"])); + } + + /// summary 版と同じ規則であることの対照 (drift 検知)。 + #[test] + fn paths_agrees_with_summary_on_the_same_files() { + assert_eq!( + is_docs_only_paths(["docs/a.md", "src/lib.rs"]), + is_docs_only_summary("M docs/a.md\nM src/lib.rs") + ); + assert_eq!( + is_docs_only_paths(["docs/a.md", "docs/b.md"]), + is_docs_only_summary("M docs/a.md\nM docs/b.md") + ); + } + } }