Skip to content

test(cli-pr-monitor): Bundle B — early-return guard 単独検証 test + code-review.md checklist 追記 (順位 83 + 84)#168

Merged
aloekun merged 1 commit into
masterfrom
bundle-b-early-return-guard-tests
May 21, 2026
Merged

test(cli-pr-monitor): Bundle B — early-return guard 単独検証 test + code-review.md checklist 追記 (順位 83 + 84)#168
aloekun merged 1 commit into
masterfrom
bundle-b-early-return-guard-tests

Conversation

@aloekun

@aloekun aloekun commented May 21, 2026

Copy link
Copy Markdown
Owner

Summary

Bundle B (順位 83 + 84): PR #120 W-001 follow-up として cli-pr-monitor の enrich_with_classifier 早期 return guard 検証テストを責務分離 + global review checklist 追記。

  • 順位 83 (Tier 2 / S): enrich_with_classifier の OR 結合された 2 つの early-return guard (!config.enabled / state.findings.is_empty()) を 1 件ずつ単独検証 する test variant を追加。検証対象 field state.classified_findings を sentinel で pre-populate することで「mutation が発生していない」を明示的に assert (空のままだと早期 return 由来か他経路由来か判別できない PR feat(cli-pr-monitor): cli-finding-classifier 統合 + Finding C strict (ADR-038 Phase 5) #120 W-001 の根本対策)
  • 順位 84 (Tier 3 / XS): ~/.claude/rules/common/code-review.md の review checklist に「複合 AND/OR の early-return guard を持つ関数のテストは各条件を独立 variant で検証」を追記。由来 (PR feat(cli-pr-monitor): cli-finding-classifier 統合 + Finding C strict (ADR-038 Phase 5) #120 W-001) を補足として併記
  • docs/todo5.md / docs/todo-summary.md から順位 83+84 を削除、Bundle f narrative を「f-2/f-3 land 済」に更新

Bundle f sub-PR f-2 + f-3 統合 land。Bundle f コア (f-1 = 順位 80/81/82) は systemic 性未確認のため defer 継続。

Test plan

  • cargo test --bin cli-pr-monitor stages::poll: 17/17 passed (新規 2 + 既存 15)
  • 新 test enrich_with_classifier_skips_when_disabled (sentinel ベース書き換え): pass
  • 新 test enrich_with_classifier_skips_when_findings_empty: pass
  • markdownlint-cli2 on global rule + todo edits: 0 errors
  • hooks-post-tool-linter Bundle Z #B-α (Rust 非 doc コメント禁止): block なし (doc comment /// 経由で codify)
  • takt pre-push-review: APPROVED (1 iteration)

補足

  • enrich_with_classifier の OR-guard 短絡評価:
    • Variant 1 (disabled=true, findings 非空): !enabled = true で短絡発火、is_empty() 評価せず = 左 arm 単独検証
    • Variant 2 (enabled=true, findings 空): !enabled = falseis_empty() = true 評価して fire = 右 arm 単独検証
  • ClassifierConfig::default()enabled = false を返すため、Variant 2 では ClassifierConfig { enabled: true, ..default() } を明示

Summary by CodeRabbit

リリースノート

  • ドキュメント

    • 推奨実行順序サマリーを更新しました
    • 完了済みのタスク情報を削除しました
  • テスト

    • テストケースを拡張し、検証の精度を向上させました

Review Change Stack

…rd の単独検証テスト + code-review.md checklist 追記 (順位 83 + 84)

PR #120 W-001 follow-up。`enrich_with_classifier` の OR 結合された
2 つの early-return guard (`!config.enabled` / `state.findings.is_empty()`)
を 1 件ずつ単独検証する test variant を追加し、責務分離を機械強制する。

順位 83 (T2 / S):
- 既存 `enrich_with_classifier_skips_when_disabled` を sentinel ベースに
  書き換え (空の `classified_findings` ではなく ClassifiedFinding sentinel
  で pre-populate → 早期 return しなかった場合の代入を mutation として
  検出)。`!config.enabled` guard 単独検証。
- 新 `enrich_with_classifier_skips_when_findings_empty` 追加。`enabled=true`
  + `findings` 空で `findings.is_empty()` guard 単独検証。
- precondition assert で 2 guard が同時発火しない setup を明示化。

順位 84 (T3 / XS):
- ~/.claude/rules/common/code-review.md の review checklist に
  「複合 AND/OR の early-return guard を持つ関数のテストは各条件を
  独立 variant で検証」を追記。由来 (PR #120 W-001) を補足説明として
  併記し、次回 PR レビュー時の参照点に。

検証:
- cli-pr-monitor stages::poll tests: 17/17 passed (新規 2 件 + 既存 15 件)
- markdownlint: 0 errors

Bundle f sub-PR f-2 + f-3 統合 land。Bundle f コア (f-1 = 順位 80/81/82)
は systemic 性未確認のため defer 継続。
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fc686c9f-6970-440f-8031-9121c5703906

📥 Commits

Reviewing files that changed from the base of the PR and between 4fad2be and aae2f05.

📒 Files selected for processing (3)
  • docs/todo-summary.md
  • docs/todo5.md
  • src/cli-pr-monitor/src/stages/poll.rs
💤 Files with no reviewable changes (1)
  • docs/todo5.md

📝 Walkthrough

Walkthrough

PR #120 の関連実装(早期リターンガード分離検証)を完成させるため、enrich_with_classifier テストを sentinel 駆動の検証パターンに置き換え、関連ドキュメントタスク(todo5.md、todo-summary.md)をクリーンアップしました。

Changes

早期リターンガード検証テスト拡張とタスク完了

Layer / File(s) Summary
enrich_with_classifier 早期リターンガード検証テスト拡張
src/cli-pr-monitor/src/stages/poll.rs
enrich_with_classifier_skips_when_disabled テストは ClassifiedFinding sentinel を事前注入し、!config.enabled 早期 return で mutation が完全停止することを assert_eq! で確認するよう変更。新規テスト enrich_with_classifier_skips_when_findings_empty を追加し、enabled=true かつ findings.is_empty() 条件下で 2 番目の早期 return が働き、sentinel が保持されることを検証します。
完了タスクのドキュメント削除・更新
docs/todo5.md, docs/todo-summary.md
todo5.md から「複合 AND guard 独立テスト」と「code-review.md チェックリスト追記」タスク 2 セクションを削除。todo-summary.md では該当タスク行を削除し、Bundle f 説明で Sub-PR f-2/f-3 が 2026-05-21 land 済(Bundle B)である旨を記載して構成を更新。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • aloekun/claude-code-hook-test#120: Main PR は PR #120 で追加された classify_findings の早期リターンガード動作に対応するテスト検証の完成を反映しており、同一ファイル・同一関数領域の拡張検証です。
  • aloekun/claude-code-hook-test#159: src/cli-pr-monitor/src/stages/poll.rs の enrich_with_classifier 周辺で、本 PR はガード動作検証テストを拡張し、Retrieved PR は write_state 失敗時ログ出力へ変更しており、同一ファイル・同一関数領域の変更でコードレベルの関連があります。
  • aloekun/claude-code-hook-test#133: PR #133 は todo/todo-summary ワークフローの分割・再リンク docs 専用 PR であり、本 PR の docs/todo-summary.md、docs/todo5.md タスク項目編集と重複領域がありますが、本 PR は Rust テスト変更も含みます。
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PRタイトルは、early-return guard の単独検証テストと code-review.md チェックリスト追記という、PR の主要な目的を明確に反映している。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aloekun aloekun merged commit 08a9ce3 into master May 21, 2026
1 check passed
@aloekun aloekun deleted the bundle-b-early-return-guard-tests branch May 21, 2026 09:57
aloekun added a commit that referenced this pull request May 22, 2026
…139 land) (#169)

* docs(todo): PR #168 post-merge-feedback 反映 — 順位 139 (ADR-NNN Test Isolation Patterns for Multi-Condition Guards) 新規追加

PR #168 post-merge-feedback の Tier 3 #2 採用結果として、複合 guard test の
test isolation pattern を project-level ADR で codify する task を順位 139
として登録。ADR 番号は順位 135 codified placeholder policy に従い land 時 PR
で確定する運用 (現状 ADR-NNN placeholder、PR #120 W-001 + PR #168 の 2 PR
横断 Frequency Medium で採用判断)。

* docs(adr): ADR-041 Test Isolation Patterns for Multi-Condition Guards (順位 139 land) — 順位 78 番号 placeholder 化

* fix(docs): CR finding 対応 — 順位 139 作業計画 checklist 整合 (PR description link/要約 完了マーク)
aloekun added a commit that referenced this pull request May 24, 2026
…171)

* docs(todo): 順位 142 新規追加 — PR #170 T3-#1 採用 (ADR-041 補強: State Preservation Invariant pattern)

PR #170 post-merge-feedback の Tier 3 #1 採用結果として、ADR-041
(Test Isolation Patterns for Multi-Condition Guards) に新 section
"State Preservation Invariant" (once-set-never-overwritten) を
追記する task を 順位 142 として登録。

PR #168/169/170 で write-once 不変式 (state.fix_push_time.or_else(...)
形式) のテストカバレッジ漏れが連続観測されたことが trigger。ADR-041 既存
section は Multi-Condition Guards (early-return) のみで、write-once 不変式は
別 pattern class として未収録だったため補強する。順位 141 で takt-fix が
自動追加した 3 件の preservation test (poll.rs / monitor.rs) を参照実装と
して cite し、3 点セット test pattern (既存値あり / 新値提供 / preservation
確認) を明文化する。

* test(hooks-linter): UTF-8 multi-byte content での line 算出 defensive test 追加 (順位 125 採用)

PR #151 (T2-#1) で hooks-post-tool-comment-lint-rust の byte_offset_to_line を multi-byte
boundary test 拡充で production bug を発見した経験を横展開。hooks-post-tool-linter の
build_violation_json (L500) は content[..m.start()] による str slicing を行うが、
regex::Match::start() が char boundary を保証するため panic 安全。

本 test は line 算出 (.bytes().filter(|b| *b == b'\n').count() + 1) が multi-byte content
(日本語/emoji/結合文字) でも正しく動作することを empirical に seal する。将来 char_indices()
ベースへの書き換え等で line off-by-one regression が入った場合に検知する。

string-processing 系の hooks のうち他 (pre-tool-validate / session-start / stop-quality /
user-prompt-feedback-recovery / stop-feedback-dispatch) は as_bytes() を write 用途で
使用するのみで offset 操作を持たないため横展開 scope 外。

* test(hooks-comment-lint-rust): collect_all_violations の MAX_VIOLATIONS truncate contract test 追加 (順位 57 採用)

collect_all_violations (L551) は find_violations と find_function_length_violations の両 source を extend した後に truncate(MAX_VIOLATIONS) で合計を cap する設計。既存 cap test (max_violations_capped / function_length_violations_capped) は各 source 単体の cap のみ検証し、合算 truncate の contract が機械強制されていなかった gap を補填。

将来の lint 追加時に L562 の truncate(MAX_VIOLATIONS) を削除した regression (= 合計が MAX を超える) を catch する explicit 安全網。

fixture: 長 function (55 行 > MAX_FUNCTION_LINES=50) 1 件 + 非 doc コメント 25 行 → find_function_length_violations 1 件 + find_violations が cap で 20 件 → 合計 21 件 → truncate 後 20 件に cap される contract を assert。

* docs(todo): 順位 91 [lint_screen] config parse test を既実装発見で削除 (memory feedback_verify_task_not_already_done.md 適用)

PR #132 (Phase c MVP land = 2026-05-09) 直後の post-merge-feedback で entry 化された
順位 91 だが、本セッション着手前の jj log + 既存 test 確認で src/cli-push-runner/src/config.rs
の test module L325-481 に既に 5 件の lint_screen parse test が存在することを発見:

  - config_parses_with_lint_screen_section_full_fields (L326): 全 7 field deserialize
  - config_parses_with_lint_screen_section_minimal_only_enabled (L370): enabled=false でも Some 構築
  - config_lint_screen_section_absent_yields_none (L402): section 不在 → None
  - config_lint_screen_numeric_defaults_resolve_via_constants (L441): default 値 fallback
  - config_lint_screen_string_defaults_resolve_via_constants (L456): default 値 fallback

entry の作業計画 4 項目はすべてカバー済 (silent field rename 防止 / 全 field deserialize /
section 不在時 None / default 値検証)。post-merge-feedback で採用された後、別 PR で
先行 land 済だった可能性が高い。

memory rule feedback_verify_task_not_already_done.md (todo 着手前に既実装検証 → stale
entry を削除に再目的化) の正典的 dogfood 例として価値あり。Bundle i のペアタスク 順位 92
(scale-aware eval fixtures) は別タスクとして残し、bundle 表記は historical note 化。

- docs/todo6.md: 順位 91 entry (L13-39) 削除
- docs/todo-summary.md: 順位 91 行削除 + 順位 92 行を独立タスクとして修正
aloekun added a commit that referenced this pull request May 26, 2026
…t rule⑪ + 順位 142 ADR-041 補強 (#176)

* docs(todo): PR #175 post-merge-feedback 採用 1 件を todo9.md / summary table に登録

PR #175 (Bundle 2) post-merge-feedback で採用判定された Tier 1 #1 を docs/todo9.md
新規エントリとして追加し、docs/todo-summary.md table に行追加。

- 順位 159 (T1 #1): jj template 内で脆弱な `description.first_line()` パターン
  を lint で禁止する custom lint rule (rule ⑪) 追加 (PR #175 Minor finding 由来
  の ad-hoc fix を構造化予防に格上げ、`empty` keyword 利用促進、Effort XS、
  対象 ext = toml/yaml/md)

* feat(cli-push-runner): Bundle 3 — 順位 5 _tmp_* pattern 追加 + 順位 159 lint rule⑪ + 順位 142 ADR-041 補強

順位 5 (PR #88 T1-2) AI 生成一時スクリプト pattern の pre-push 検出を実装。
Bundle 1 で確定した補完アプローチ (config-driven patterns extension) を採用、
`_tmp_*` pattern を `scratch_file_warning` stage の patterns に追加することで
PR scope 外 scratch file の混入を構造的に防止する範囲を拡張。

## 実装 (順位 5)

- push-runner-config.toml: patterns に `_tmp_*` 追加、ADR-007 との責務分担を
  コメントで明示 (本 stage = pre-push @ commit 検査 / ADR-007 = edit 時 text 検査)
- src/cli-push-runner/src/stages/scratch_file_warning.rs: module doc に Bundle 3
  完了状況と ADR-007 関係を追記、_tmp_* pattern の 3 件 test 追加
  (detects_tmp_prefix_pattern / combined patterns / does_not_match_underscore_only)

## 実装 (順位 159)

- .claude/custom-lint-rules.toml: rule⑪ (no-jj-template-first-line) 追加。
  jj template の `description` の `first_line` メソッド使用を error severity
  で禁止、`empty` keyword 利用を促進。
- extensions: toml / yaml / md (jj template が記述されうる主要 file 形式)
- Self-exclusion: message / why / example / コメント内で pattern 文字列を連続
  記述しない設計で self-trigger を回避 (rule⑥ no-ephemeral-todo-reference と
  同パターン)
- src/hooks-post-tool-linter/src/main.rs: rule⑪ test 5 件追加
  (toml / yaml positive+negative + md positive)
- rule_test_coverage_check meta validation 通過

## 実装 (順位 142)

- docs/adr/adr-041-test-isolation-patterns.md: § 関連 pattern: State Preservation
  Invariant (write-once 不変式) を追加。Multi-Condition Guards (本体) との別
  pattern class として codify、PR #168/169/170 連続観測の write-once field
  (state.fix_push_time 等) 設計 + 3 variant 直交 test pattern (既存値 preservation /
  新値 provision / 書き換え不可) を明文化。参照実装 = poll.rs の
  finalize_*_preserves_existing_fix_push_time + monitor.rs の
  resume_returns_fix_push_time_from_state_when_set を cite。

## Tests

- cargo test --manifest-path src/cli-push-runner/Cargo.toml: 136 passed (+3)
- cargo test --manifest-path src/hooks-post-tool-linter/Cargo.toml no_jj_template_first_line: 5 passed (新規)
- cargo test --manifest-path src/hooks-post-tool-linter/Cargo.toml rule_test_coverage_check: 1 passed (meta validation)
- pnpm build:cli-push-runner / pnpm build:hooks-post-tool-linter: release profile build success

* docs(todo): Bundle 3 完了に伴い 順位 5 / 159 / 142 を削除
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant