docs/test: PR #194 follow-up bundle — fail-closed ADR + jj test/revset patterns + experimental feature checklist - #195
Merged
Merged
Conversation
|
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)
📝 WalkthroughWalkthroughPR ChangesFail-Closed 設計パターンと Revset 原則の確立・検証
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/cli-pr-monitor/src/fix_commit.rs`:
- Around line 720-721: The test helper assert_descriptions_absent_in_pr_range
currently hardcodes the range "master..@" which breaks tests that set
default_branch="main"; modify assert_descriptions_absent_in_pr_range to accept a
branch/range parameter (or a default_branch argument) and use that when
constructing the git range instead of "master..@", then update the call in
integration_sweep_respects_alternative_default_branch to pass "main..@" (or
construct the range from the test's default_branch) so the assertion checks the
intended main..@ range.
🪄 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: 5d0b3eb7-7d81-4f29-b480-906fe2a39d83
📒 Files selected for processing (6)
CLAUDE.mddocs/adr/adr-021-jj-change-detection-principles.mddocs/adr/adr-039-experimental-feature-standard-pattern.mddocs/adr/adr-043-security-gates-fail-closed.mdsrc/cli-pr-monitor/src/fix_commit.rssrc/hooks-pre-tool-validate/src/main.rs
…引数化 (CR Major PR #195)
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
aloekun
added a commit
that referenced
this pull request
Jun 5, 2026
PR #195 で test helper 3 関数のうち 2 関数が `"master..@"` hardcode → CR Major 指摘で default_branch 引数化済 (commit 9663dd6)、残 1 関数 (`count_empty_in_pr_range`) は pre-push F-1 で non-blocking 観測。手動 grep / reviewer 判断で塞ぐと再発するため、 決定論的防止層 (ADR-007) として機械検出する。 - Pattern: `master\.\.@` (jj revset 固有 token、FP リスクは Bundle Z #B-α で 非 doc comments 既に禁止のため軽微) - extensions = ["rs"] のみ、main.rs source は format! 経由で literal を runtime 組立し self-exclusion 維持 (`grep -n 'master\.\.@' src/hooks-post-tool-linter/src/main.rs` = 0 hit) - Test 4 件: simple hardcode / empty() filter scope / parameterized format skip / other branch literal skip - TOML meta field `[rules.test_coverage.main_ext_tests.rs]` 宣言、 順位 137 land 済の rule_test_coverage_check が機械強制 順位 190 (count_empty_in_pr_range parameterize) と同 PR で land 推奨 = 機械検出が auto-fix promotion 候補を提示し、順位 190 で受け修正、dogfood 循環を 1 PR で成立。 Bundle 195-FB Layer 1 (mechanical)
aloekun
added a commit
that referenced
this pull request
Jun 5, 2026
…化 (PR #195 T2-#1) PR #195 で test helper 3 関数のうち 2 関数 (`assert_descriptions_absent_in_pr_range` / `assert_descriptions_present_in_pr_range`) は `default_branch` 引数化済 (commit 9663dd6) だが、`count_empty_in_pr_range` のみ `master..@` 引数化漏れ = API cohesion 違反。 順位 189 lint rule (rule⑫ no-hardcoded-jj-revset-range) が機械検出した hardcode を受けて修正、 companion helper group 3 関数の API signature を `(repo_dir, default_branch, ...)` で整合化。 - `count_empty_in_pr_range(repo_dir, default_branch: &str)` に signature 変更 - caller (`integration_sweep_empty_commits_abandons_multiple_in_range`) は `"master"` 引数で更新 - `integration_sweep_respects_alternative_default_branch` に `count_empty_in_pr_range(repo_dir, "main")` sanity check 追加 (alternative branch variant が helper レベルで保護) - doc comments (line 621, 644) は `<default_branch>..@` placeholder に書き換え (rule⑫ pattern `master\.\.@` の FP を構造的に排除) - `grep -n 'master\.\.@' src/` = 0 hit (clean baseline) 検証: - cargo test --bin cli-pr-monitor fix_commit: 11 unit tests pass - cargo test --bin cli-pr-monitor 'fix_commit::tests::integration_sweep' -- --ignored --test-threads=1: 5 integration tests pass - 順位 189 lint rule が production code 上で fire 0 件 Bundle 195-FB Layer 2 (実装)
aloekun
added a commit
that referenced
this pull request
Jun 5, 2026
Bundle 195-FB (PR #195 post-merge-feedback 4 件) の 4 layer が land 完了したため todo10.md / todo-summary.md から 順位 189-192 entries を削除。 Layer: - 189 (T1): rule⑫ no-hardcoded-jj-revset-range 追加 (commit uoxuyxrp、本 PR 内) - 190 (T2): count_empty_in_pr_range default_branch 引数化 (commit qntnzyxt、本 PR 内) - 191 (T3): non-blocking finding 留置基準を code-review.md に追記 (~/.claude/、本 PR 外) - 192 (T3): Companion Helper API Cohesion section + ADR-041 cross-ref (~/.claude/、本 PR 外) global rule 編集 (191, 192) は ~/.claude/rules/common/{code-review,patterns,testing}.md への edit のため本リポジトリの diff には含まれないが、 feedback_global_config_backup per ~/.claude-backups/20260605-131309-bundle-195-fb/ で snapshot を取得済。 派生プロジェクト (techbook-ledger / auto-review-fix-vc) に自動波及。 memory feedback_todo_no_history per: 完了タスクは ADR/仕組みに反映後、todo.md から削除 (履歴は git log で追跡可能)。 Bundle 195-FB Layer 4 (cleanup)
6 tasks
aloekun
added a commit
that referenced
this pull request
Jun 5, 2026
* docs(todo): PR #195 post-merge-feedback 採用 4 件登録 (順位 189-192) * feat(lint): rule⑫ no-hardcoded-jj-revset-range 追加 (PR #195 T1-#1) PR #195 で test helper 3 関数のうち 2 関数が `"master..@"` hardcode → CR Major 指摘で default_branch 引数化済 (commit 9663dd6)、残 1 関数 (`count_empty_in_pr_range`) は pre-push F-1 で non-blocking 観測。手動 grep / reviewer 判断で塞ぐと再発するため、 決定論的防止層 (ADR-007) として機械検出する。 - Pattern: `master\.\.@` (jj revset 固有 token、FP リスクは Bundle Z #B-α で 非 doc comments 既に禁止のため軽微) - extensions = ["rs"] のみ、main.rs source は format! 経由で literal を runtime 組立し self-exclusion 維持 (`grep -n 'master\.\.@' src/hooks-post-tool-linter/src/main.rs` = 0 hit) - Test 4 件: simple hardcode / empty() filter scope / parameterized format skip / other branch literal skip - TOML meta field `[rules.test_coverage.main_ext_tests.rs]` 宣言、 順位 137 land 済の rule_test_coverage_check が機械強制 順位 190 (count_empty_in_pr_range parameterize) と同 PR で land 推奨 = 機械検出が auto-fix promotion 候補を提示し、順位 190 で受け修正、dogfood 循環を 1 PR で成立。 Bundle 195-FB Layer 1 (mechanical) * refactor(cli-pr-monitor): count_empty_in_pr_range を default_branch 引数化 (PR #195 T2-#1) PR #195 で test helper 3 関数のうち 2 関数 (`assert_descriptions_absent_in_pr_range` / `assert_descriptions_present_in_pr_range`) は `default_branch` 引数化済 (commit 9663dd6) だが、`count_empty_in_pr_range` のみ `master..@` 引数化漏れ = API cohesion 違反。 順位 189 lint rule (rule⑫ no-hardcoded-jj-revset-range) が機械検出した hardcode を受けて修正、 companion helper group 3 関数の API signature を `(repo_dir, default_branch, ...)` で整合化。 - `count_empty_in_pr_range(repo_dir, default_branch: &str)` に signature 変更 - caller (`integration_sweep_empty_commits_abandons_multiple_in_range`) は `"master"` 引数で更新 - `integration_sweep_respects_alternative_default_branch` に `count_empty_in_pr_range(repo_dir, "main")` sanity check 追加 (alternative branch variant が helper レベルで保護) - doc comments (line 621, 644) は `<default_branch>..@` placeholder に書き換え (rule⑫ pattern `master\.\.@` の FP を構造的に排除) - `grep -n 'master\.\.@' src/` = 0 hit (clean baseline) 検証: - cargo test --bin cli-pr-monitor fix_commit: 11 unit tests pass - cargo test --bin cli-pr-monitor 'fix_commit::tests::integration_sweep' -- --ignored --test-threads=1: 5 integration tests pass - 順位 189 lint rule が production code 上で fire 0 件 Bundle 195-FB Layer 2 (実装) * docs(todo): Bundle 195-FB 完了で 順位 189-192 削除 + global rule edit 同伴 Bundle 195-FB (PR #195 post-merge-feedback 4 件) の 4 layer が land 完了したため todo10.md / todo-summary.md から 順位 189-192 entries を削除。 Layer: - 189 (T1): rule⑫ no-hardcoded-jj-revset-range 追加 (commit uoxuyxrp、本 PR 内) - 190 (T2): count_empty_in_pr_range default_branch 引数化 (commit qntnzyxt、本 PR 内) - 191 (T3): non-blocking finding 留置基準を code-review.md に追記 (~/.claude/、本 PR 外) - 192 (T3): Companion Helper API Cohesion section + ADR-041 cross-ref (~/.claude/、本 PR 外) global rule 編集 (191, 192) は ~/.claude/rules/common/{code-review,patterns,testing}.md への edit のため本リポジトリの diff には含まれないが、 feedback_global_config_backup per ~/.claude-backups/20260605-131309-bundle-195-fb/ で snapshot を取得済。 派生プロジェクト (techbook-ledger / auto-review-fix-vc) に自動波及。 memory feedback_todo_no_history per: 完了タスクは ADR/仕組みに反映後、todo.md から削除 (履歴は git log で追跡可能)。 Bundle 195-FB Layer 4 (cleanup)
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #194 (
feat(hooks): merge 前 mechanical gate 強化) の post-merge-feedback で採用された 6 件 (順位 183-188) を 1 PR に bundle して land する。behind?fail-open bug と sweep revset の scope 過剰問題に対し、回帰防止網となる test を追加 (本リポ scope)~/.claude/rules/common/{testing,patterns}.md+ ADR-021/039 に codify (派生プロジェクトへ自動波及)Option::?vsis_none_orの semantic mismatch)変更内容 (commit ごと)
build_todo_staleness_message(None, ...)の fail-closed test 2 件追加 (PR #194 T2-#1)~/.claude/rules/common/testing.md§ "jj 操作コードの integration test pattern" +fix_commit.rs内//!companion doc (PR #194 T2-#3)~/.claude/rules/common/patterns.md§ Experimental Feature 設計時の参照必須 (PR #194 T3-#1)統計
~/.claude/rules/common/{testing,patterns}.mdを 2 ファイル更新 (派生プロジェクトに波及)試験運用判断基準
docs/adr/adr-043-security-gates-fail-closed.md§ 試験運用判断基準)Test plan
cargo test --workspace全 pass (unit test)cargo test --workspace -- --ignored --test-threads=15 件の integration_sweep_* passStatus: all("approved")参照
dfad56ff— takt-fix iteration のbehind?→is_none_or修正is_none_orイディオム~/.claude/rules/common/testing.md§ jj 操作コードの integration test pattern (本 PR で追加) — count NG / description-based assert OK pattern~/.claude/rules/common/patterns.md§ Experimental Feature 設計時の参照必須 (本 PR で追加) — 3 軸の設計時確認.claude/feedback-reports/194.md— post-merge-feedback 採用根拠Summary by CodeRabbit
Documentation
Tests