refactor(check-ci-coderabbit): main.rs を 7 module に分割 (PR A) - #213
Conversation
…tion length 解消) PR #212 で残置した touch-trigger ratchet 違反 (file_length / function-too-long) を解消するため check-ci-coderabbit/src/main.rs (2369 行) を 7 module に分割し、 run_check (147 行) を 8 helper 関数に decompose。 分割後: - models.rs (118): CheckResult / RateLimitInfo / CiStatus / CodeRabbitStatus / Gh* / Listed* models 集約 - markers.rs (60): RATE_LIMIT_MARKERS / WALKTHROUGH_* + 3 判定 helper - decide.rs (412): decide + build_summary + 16 tests、build_summary を 2 helper (build_summary_ci_part / build_summary_cr_part) に分解 - parsers.rs (532): parse_ci_runs / parse_coderabbit_status / parse_new_comments / parse_walkthrough_clean_marker / parse_actionable_comments / extract_actionable_count / parse_unresolved_threads + 32 tests、 parse_ci_runs 内の classification を classify_ci_overall / is_pending_run / is_failure_run に分解 - rate_limit.rs (365): parse_rate_limit + extract_*_format_wait_time + parse_iso8601_to_unix + 21 tests - findings.rs (381): parse_findings / parse_listed_findings + 抽出 helper + 16 tests、各 parse 関数を candidate/transform 2 関数に分解 - main.rs (483): entry, parse_args, run_check (37 行) + 8 fetch helper, run_list_findings, auto_detect_*, is_valid_*, parse_args test run_check decomposition (147→37 行) で抽出した helper: - resolve_repo_and_pr (Box<CheckResult> で result_large_err 回避) - build_init_error_result - fetch_ci / fetch_coderabbit_commit_state / fetch_issue_comments_json / fetch_actionable_count / fetch_unresolved_threads / fetch_findings 合計行数: 2351 (元 2369、わずかに減少)。 全ファイル 800 行以下 (file_length lint 解消)。 全関数 50 行以下 (function_length lint 解消)。 検証: - cargo build --release: clean (18.22s) - cargo test -p check-ci-coderabbit: 90 passed (refactor 前後で同数、test 同一性確認) - cargo clippy -p check-ci-coderabbit --tests -- -D warnings: clean - Box<CheckResult> で clippy::result_large_err warning 解消 scope 外 (PR B で対応): - cli-pr-monitor/src/stages/poll.rs (2090 行) の分割 途中事故: - PowerShell スクリプトで main.rs を 0 行に誤削除 → jj op restore で復旧 → module ファイル再 Write で継続 → 以降 PowerShell の in-file 編集を回避し Edit/Bash sed のみ使用
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
Changescheck-ci-coderabbit モジュール分割
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/check-ci-coderabbit/src/parsers.rs`:
- Around line 171-175: The boundary condition for comparing `submitted_at` with
`push_time` in the `after_push_time` calculation is using a strict greater-than
comparison (>) instead of greater-than-or-equal (>=), which is inconsistent with
other boundary condition checks in the codebase that check for "on or after
push_time". Change the comparison operator in the map function where
`submitted_at` is compared against `push_time` to use >= instead of >, so that
reviews submitted at exactly the push_time are included rather than excluded,
preventing `actionable_comments` from becoming None and causing incorrect
results.
- Around line 76-93: The CodeRabbit status retrieval logic in the cr_statuses
vector is using the incorrect element accessor method. Since the API response
returns statuses ordered from newest to oldest, using `.last()` on the
cr_statuses collection retrieves the oldest status, but the code should retrieve
the newest one. Change the `.last()` call to `.first()` to access the most
recent CodeRabbit status from the filtered collection instead of the oldest one.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fae700a5-5c34-4a46-b1a3-10a8a9c363b1
📒 Files selected for processing (7)
src/check-ci-coderabbit/src/decide.rssrc/check-ci-coderabbit/src/findings.rssrc/check-ci-coderabbit/src/main.rssrc/check-ci-coderabbit/src/markers.rssrc/check-ci-coderabbit/src/models.rssrc/check-ci-coderabbit/src/parsers.rssrc/check-ci-coderabbit/src/rate_limit.rs
Resolved findings: - [Major] src/check-ci-coderabbit/src/parsers.rs:93 CodeRabbitステータスの取得順序が誤っています。 - [Major] src/check-ci-coderabbit/src/parsers.rs:175 `push_time` の境界条件が1件取りこぼします。
|
@coderabbitai review |
✅ Action performedReview finished.
|
* docs(todo): PR #213 post-merge-feedback 採用 3 件 + session 派生統合を追加 (順位 212/213/214) * refactor(cli-pr-monitor): poll.rs を 3 module に分割 (PR B、partial) PR #213 (PR A) で残置した cli-pr-monitor/src/stages/poll.rs (2090 行) の file_length lint 違反を解消するため module 分割を実施。本 commit は **部分達成** で mod.rs の test ブロック分割は未完了 (1404 行で 800 行制限超過)。 分割構成: - poll.rs (2090 行) → poll/ ディレクトリ - poll/mod.rs (1404 行): PollResult, PollContext, run_poll_loop, iteration 系 helpers (run_one_iteration, invoke_checker 等), make_park_poll_result, recompute_action + 35 tests - poll/rate_limit.rs (417 行): rate-limit branch (handle_rate_limit_branch + dispatch + finalize_posted_retrigger + finalize_parked + shortcut 系 helpers + RateLimitOutcome + handle_rate_limit_retry + post_review_immediately + format_park_signal + MergeableStatus) — production only - poll/review_recheck.rs (297 行): review_recheck park (round_up_to_next_minute + compute_safe_minute + ReviewParkSignalFields + collect_review_park_fields + format_review_park_signal + finalize_initial_review_park + finalize_review_recheck_park + finalize_review_recheck_max_reached + schedule_next_review_recheck_park) — production only 検証: - cargo build -p cli-pr-monitor: clean - cargo test -p cli-pr-monitor: 203 passed, 12 ignored (refactor 前後で同数) - cargo clippy -p cli-pr-monitor --tests -- -D warnings: clean 未完了 (follow-up PR で対応): - mod.rs の test ブロック (~1000 行) を rate_limit/review_recheck.rs に分配し、 mod.rs を 800 行以下にする - 共有 test helpers (env_override_lock, unwritable_state_path, invoke_*_with_bad_path 等) の sub-module への分配 (memory feedback_test_dry_antipattern 適用で duplication 許容) 設計決定: - PollContext fields を pub(super) 化 (sub-module からの test 用 fixture 構築) - make_park_poll_result, make_action_required_result を pub(super) 化 - compute_safe_minute_for_park_signal, format_review_park_signal, schedule_next_review_recheck_park, handle_rate_limit_retry, evaluate_rate_limit_shortcut, finalize_parked, finalize_posted_retrigger, format_shortcut_signal, format_park_signal を pub(super) 化 (mod.rs tests からの参照を維持) - mod.rs の sub-module import を production / #[cfg(test)] で分離して unused warning を解消 scope 外: - PR A 同様、pr_size_check は事前に override 必要見込み (純粋 refactor のため) * fix(review): apply CodeRabbit fixes for #214 PR #214 (PR B partial) takt post-pr-review iter 2 で 5 findings 検出 → 4 fix + 1 no-change-needed で convergence_verdict: fully_resolved。 修正内容: - F1 (FIXED): rate_limit::finalize_posted_retrigger の max_retries 引数を削除し format_posted_retrigger_review_park_signal を新規追加 (rate-limit retry vs review_recheck park の semantic 分離、test 2 件追加) - F2 (FIXED): rate_limit.rs に make_action_required_result import 追加 (CR 指摘) - F3 (FIXED): review_recheck.rs の compute_safe_minute_for_park_signal と format_review_park_signal を pub(super) 化 (mod.rs tests から参照可能化) - F4 (NO_CHANGE_NEEDED): 既に対応済 - F5 (FIXED): docs/todo*.md の analyzer label を T1-1 → feedback-T1-1 に 統一 (CR 指摘の表記揺れ修正) 検証: - cargo build -p cli-pr-monitor: clean - cargo test -p cli-pr-monitor: 205 passed (refactor 前 203 + 新規 2 件) - cargo clippy clean metrics gate 例外: review_recheck.rs の +2 lines が pub(super) 化に伴う visibility 変更で metrics gate FAIL (TRUE POSITIVE GATE, ACCEPTABLE DELTA)。 takt convergence でも fully_resolved 判定済。supervisor ratification 待ち。 * fix(review): apply CodeRabbit fixes for #214 (round 2) CR 再レビューで Major 3 件追加検出 → 全件修正: F1 (FIXED): review_recheck::finalize_initial_review_park で read_state() が 既存 state を返した場合、PrMonitorState::new(...) が呼ばれず ctx の最新値 (pr/repo/started_at) が反映されない logic bug。fresh push 経路で前 push の PR/repo/started_at 情報を引き継ぐリスクがあったため、明示的に上書き処理を 追加。fix_push_time も ctx 優先 + state fallback の方向に統一。 F2 (FIXED): docs/todo10.md の順位 212 entry に Tier 分類 disambiguation を 追加。analyzer feedback report の Tier 1 #1 と project tier 列の Tier 2 が 矛盾するように見えるため、memory feedback_tier_classification の re-classification rule を明示参照する補足 paragraph を追加。 F3 (FIXED): docs/todo-summary.md の順位 212 entry に同様の Tier 分類補足を 追加。CR が「T1-1 vs Tier 2」の表記揺れと判定したため。 検証: - cargo test -p cli-pr-monitor: 205 passed - cargo clippy -p cli-pr-monitor --tests -- -D warnings: clean CR スレッド対応: takt-fix iter 1 (2m 42s、approved) では未検出の Major 3 件 だったため手動修正。
…uctive-write-block preset 追加 (順位 212) PR #213 (refactor PR A) 作業中に発生した PowerShell スクリプトでの `check-ci-coderabbit/src/main.rs` (2369 行) 0 byte 化事故の機械強制 対策。memory `feedback_no_powershell_inplace_edit` の codify と併用する mechanical defense layer として preset と dispatch を追加した。 設計: - 新 preset `powershell-destructive-write-block` (5 BlockedPattern) - `[System.IO.File]::WriteAllText|WriteAllBytes|WriteAllLines` (事故直接因) - `Out-File` (redirect 系 cmdlet) - `Set-Content -Value` (cmdlet 版書込、-Value flag 付きのみ false positive 回避) - すべての pattern に `(?i)` flag を付与 (PowerShell の .NET method 名 + cmdlet 名は case-insensitive) - Exception を path 位置に scope (= command 全体ではなく cmdlet/method 直後の 最初の引数のみ): - WriteAllText/Bytes/Lines: `\(\s*['"]__` で open paren 直後限定 - Out-File: `Out-File\s+(?:-FilePath\s+)?['"]?__` (unquoted も allow) - Set-Content: `Set-Content\s+(?:-(?:Literal)?Path\s+)?['"]?__` - Set-Content main pattern は `[^|]*-Value` で param 順非依存 (`-Value` が `-Path` より先に来る reversed order でも block) - dispatch table に `PowerShell` arm 追加 + `handle_powershell_tool` 関数 (handle_bash_tool と同形で `tool_input.command` を build_blocked_patterns 全 preset で照合) - `default_preset_names` に登録 = Config::default() で default-on 追加 test 19 件 (= 11 base + 8 edge case): base 11 件: - positive (5): WriteAllText / WriteAllBytes / WriteAllLines / Out-File / Set-Content -Value の destructive write block - negative scratch (2): `__*` prefix path への write は allow - negative legit (3): Get-ChildItem / Where-Object / Set-Content 単体 (false positive 回避) - default-on (1) edge case 8 件 (pre-push-review Warning 4 件に対応、ADR-043 fail-closed): - W-002 SEC: lowercase `.NET` method 名 (`writealltext`) でも block - W-001 SEC: `__` 文字列が value 位置にあるとき block (path 位置に scope) - W-001 SEC: ReadAllText で `__` を読んでも WriteAllText 先が prod なら block - W-001 SIMP: Set-Content の reversed parameter order (`-Value` 先) でも block - W-001 SIMP: Set-Content の reversed order でも scratch path なら allow - W-002 SIMP: unquoted scratch path (`Out-File __output.txt`) は allow - value 位置の `__` を path と誤認しない (Set-Content 版) 検証: - cargo test -p hooks-pre-tool-validate: 221 passed (= 202 既存 + 19 new) - cargo clippy -p hooks-pre-tool-validate -- -D warnings: clean - cargo test --workspace: 全テスト pass (regression なし) Note: `src/hooks-pre-tool-validate/src/main.rs` は本 PR 後 2900 行になり file_length lint (800 行ガイドライン) を超過するが、本 PR は順位 212 の mechanical defense 追加に scope を限定し、ファイル分割 refactor は別 PR (PR #213/#214 と同 pattern) で対応する。
…derabbit 境界 test (順位 212-215) (#215) * docs(todo): PR #214 post-merge-feedback T3-1 採用 (順位 215) PR #214 (refactor PR B partial) post-merge-feedback report の T3-1 を採用: `~/.claude/rules/common/coding-style.md` に "Defensive State Reset in State Machines" section を追記する todo entry を docs/todo-summary.md + docs/todo10.md に追加。 由来: PR #214 round 2 で CR Major #4 fix として `finalize_initial_review_park` 内 `read_state()` 後の `state.pr`/`state.repo`/`state.started_at` 無条件上書きが land。同型 `head_commit`/`review_recheck_count` reset と合わせて 5 field の確立 された defensive pattern を future reviewer が "redundant" と誤判定して 削除しないよう global rule で防御する。 Severity Medium / Frequency Medium / Effort S / Adoption Risk None で ユーザー承認 (2026-06-20)。pre-push:simplicity + pre-push:security の 独立 2 ソース検出。実装時は `feedback_global_config_backup` 適用必須。 * test(check-ci-coderabbit): API ordering + 境界条件テスト追加 (順位 213, 214) 順位 213 (PR #213 post-merge-feedback T2-1 採用): parse_coderabbit_status の `.first()` semantics を test 名 + doc comment + assert message で explicit 化。GitHub statuses API の reverse-chronological 返却 implicit assumption を test fixture で 固定し、refactor 時の意味喪失を機械的に検出する。 順位 214 (PR #213 post-merge-feedback T2-2 採用): parse_actionable_comments の `submitted_at >= push_time` inclusive 比較を境界で固定する 2 test を追加: - actionable_includes_review_at_exact_push_time: `==` 境界の inclusive 比較を保証 - actionable_excludes_review_before_push_time: 配列 latest 位置に sentinel (Actionable: 99, 11:00) を置き、rfind が time filter で 正しく除外することを確認 (filter 壊れた場合 99 が返り test 落ちる) 既存 rule⑦ `no-time-field-strict-greater` は変数名抽出後を catch 不可なため、test による second defense layer として機能。 memory `feedback_test_dry_antipattern.md` 適用で独立 setup。 検証: - cargo test -p check-ci-coderabbit: 96 passed (= 93 + 3 new) - 既存 actionable_filters_by_time との相補的 boundary 検証 * feat(hooks-pre-tool-validate): PowerShell dispatch + powershell-destructive-write-block preset 追加 (順位 212) PR #213 (refactor PR A) 作業中に発生した PowerShell スクリプトでの `check-ci-coderabbit/src/main.rs` (2369 行) 0 byte 化事故の機械強制 対策。memory `feedback_no_powershell_inplace_edit` の codify と併用する mechanical defense layer として preset と dispatch を追加した。 設計: - 新 preset `powershell-destructive-write-block` (5 BlockedPattern) - `[System.IO.File]::WriteAllText|WriteAllBytes|WriteAllLines` (事故直接因) - `Out-File` (redirect 系 cmdlet) - `Set-Content -Value` (cmdlet 版書込、-Value flag 付きのみ false positive 回避) - すべての pattern に `(?i)` flag を付与 (PowerShell の .NET method 名 + cmdlet 名は case-insensitive) - Exception を path 位置に scope (= command 全体ではなく cmdlet/method 直後の 最初の引数のみ): - WriteAllText/Bytes/Lines: `\(\s*['"]__` で open paren 直後限定 - Out-File: `Out-File\s+(?:-FilePath\s+)?['"]?__` (unquoted も allow) - Set-Content: `Set-Content\s+(?:-(?:Literal)?Path\s+)?['"]?__` - Set-Content main pattern は `[^|]*-Value` で param 順非依存 (`-Value` が `-Path` より先に来る reversed order でも block) - dispatch table に `PowerShell` arm 追加 + `handle_powershell_tool` 関数 (handle_bash_tool と同形で `tool_input.command` を build_blocked_patterns 全 preset で照合) - `default_preset_names` に登録 = Config::default() で default-on 追加 test 19 件 (= 11 base + 8 edge case): base 11 件: - positive (5): WriteAllText / WriteAllBytes / WriteAllLines / Out-File / Set-Content -Value の destructive write block - negative scratch (2): `__*` prefix path への write は allow - negative legit (3): Get-ChildItem / Where-Object / Set-Content 単体 (false positive 回避) - default-on (1) edge case 8 件 (pre-push-review Warning 4 件に対応、ADR-043 fail-closed): - W-002 SEC: lowercase `.NET` method 名 (`writealltext`) でも block - W-001 SEC: `__` 文字列が value 位置にあるとき block (path 位置に scope) - W-001 SEC: ReadAllText で `__` を読んでも WriteAllText 先が prod なら block - W-001 SIMP: Set-Content の reversed parameter order (`-Value` 先) でも block - W-001 SIMP: Set-Content の reversed order でも scratch path なら allow - W-002 SIMP: unquoted scratch path (`Out-File __output.txt`) は allow - value 位置の `__` を path と誤認しない (Set-Content 版) 検証: - cargo test -p hooks-pre-tool-validate: 221 passed (= 202 既存 + 19 new) - cargo clippy -p hooks-pre-tool-validate -- -D warnings: clean - cargo test --workspace: 全テスト pass (regression なし) Note: `src/hooks-pre-tool-validate/src/main.rs` は本 PR 後 2900 行になり file_length lint (800 行ガイドライン) を超過するが、本 PR は順位 212 の mechanical defense 追加に scope を限定し、ファイル分割 refactor は別 PR (PR #213/#214 と同 pattern) で対応する。 * fix(hooks-pre-tool-validate): Set-Content exception を順序非依存に対応 (PR #215 CR Minor #1) PR #215 CR review が指摘した Minor finding を対応: `Set-Content` の exception regex が main pattern と order-independence を共有していなかったため、`Set-Content -Value $x -Path "__file"` のような reversed parameter order での scratch path 指定が false positive で block されていた。 修正: 旧 exception: `(?i)\bSet-Content\b\s+(?:-(?:Literal)?Path\s+)?['"]?__` → `-Path` が Set-Content 直後にある順序のみカバー 新 exception: `(?i)\bSet-Content\b(?:\s+['"]?__|[^|]*\s-(?:Literal)?Path\s+['"]?__)` → 2-alternative: - alt 1: `Set-Content` 直後の positional `__` path - alt 2: `Set-Content` 以降のいずれかの位置の `-Path`/`-LiteralPath` 後の `__` path CR の修正案は `(?=...)` lookahead を使っていたが、Rust 標準 `regex` crate は lookahead 非対応 (compile failure) のため、同 semantics を 2-alt 構成で lookahead なしに実装。main pattern `[^|]*-Value` が既に `-Value` 存在を 保証しているため、exception 側に lookahead-based verification は不要。 test 追加: - `Set-Content -Value $x -Path "__file.txt"` (reversed order) → allow - `Set-Content -Value $x -LiteralPath "__file.txt"` (LiteralPath 変種) → allow - 既存の positional case (`Set-Content "__file.txt" -Value $x`) は維持 検証: - cargo test -p hooks-pre-tool-validate: 221 passed - cargo clippy -p hooks-pre-tool-validate -- -D warnings: clean 由来: PR #215 CR Minor #1 (discussion_r3448719154)、ADR-043 fail-closed 原則による security gate と false-positive 抑制の両立。
Summary
PR #212 (Bundle CR-CHECK) で残置した
check-ci-coderabbit/src/main.rsの touch-trigger ratchet 違反 (RUST_FILE_TOO_LONG/RUST_FUNCTION_TOO_LONG) を解消するため、純粋な module 分割 refactor を行います。run_check(147 行) を 8 helper 関数に decompose → 50 行制約満足 (順位 48 lint guideline)分割後の構成
main.rsmodels.rsmarkers.rsRATE_LIMIT_MARKERS/WALKTHROUGH_*+ 3 判定 helperdecide.rsparsers.rsrate_limit.rsfindings.rsrun_check decomposition (147→37 行)
抽出した helper 関数:
resolve_repo_and_pr(args) -> Result<(String, u64), Box<CheckResult>>—Box<CheckResult>でclippy::result_large_err回避build_init_error_result— 初期化エラー時のCheckResult構築fetch_ci(branch) -> CiStatusfetch_coderabbit_commit_state(repo) -> Stringfetch_issue_comments_json(repo, pr) -> Stringfetch_actionable_count(repo, pr, push_time) -> Option<usize>fetch_unresolved_threads(repo, pr) -> Option<usize>fetch_findings(repo, pr, push_time) -> Vec<Finding>関数長 50 行制約への追加 decomposition
decide::build_summaryをbuild_summary_ci_part/build_summary_cr_part/build_summary_cr_part_with_countsに分解parsers::parse_ci_runs内の classification logic をclassify_ci_overall/is_pending_run/is_failure_runに分解parsers::parse_new_commentsの filter closure をis_kept_new_comment名前付き関数に抽出findings::parse_findingsをis_finding_candidate+comment_to_findingに分解findings::parse_listed_findingsをcollect_resolved_root_ids+is_listed_finding_candidate+comment_to_listed_findingに分解Test plan
cargo build --releaseclean (18.22s)cargo test -p check-ci-coderabbit90 passed (refactor 前後で同数、test 同一性確認)cargo clippy -p check-ci-coderabbit --tests -- -D warningscleanBox<CheckResult>でclippy::result_large_errwarning 解消注記
PR_SIZE_CHECK_OVERRIDE=1使用について本 PR は
pnpm pushのpr_size_checkで初回 block されました (PR diff サイズがblock_threshold超過)。block message が示す option (a)(b)(c) のうち option (b)PR_SIZE_CHECK_OVERRIDE=1で再 push しています。理由:
ユーザー承認 (2026-06-19) を得て override を適用しました。
途中事故と復旧
途中、PowerShell スクリプトで main.rs を 0 行に誤削除 →
jj op restoreで復旧 → module ファイル再 Write で継続。以降 PowerShell の in-file 編集を回避し Edit / Bash sed のみ使用しました。Out of scope (PR B で対応)
cli-pr-monitor/src/stages/poll.rs(2090 行) の分割関連
Summary by CodeRabbit
Release Notes