feat(hooks): comment-lint hook scope を変更行に限定 (順位 50 + 順位 51/52 follow-ups)#104
Merged
Conversation
PR #103 push の実観測で takt pre-push-review が 6-iter outlier (22m 50s) を発生させ、 うち iter 3+4 の ~10 分が wasted (stale review-diff.txt による false positive 'persists')。 supervisor の live Read で打開されたが、構造的制約として残存。 post-merge-feedback の Tier 3 #1 (snapshot semantics の ADR 化) は順位 51 land で obsolete になるため skip、Tier 3 #2 (fix scope clarity) は coding-style.md 既存ルールと 重複のため skip。機構で塞ぐ実装層対策のみ採用 (Tier 1, Effort M)。 todo.md table に順位 51 を追加し、todo5.md に詳細エントリ (動機・設計決定 2 案・ 作業計画・完了基準) を記載。
Edit tool の new_string 出現位置から変更行 range を導出し、その範囲外の pre-existing violations を flag しないよう scope を絞る。 変更前は `hooks-post-tool-comment-lint-rust` がファイル全体を scan する 設計のため、PR #102 セッション中に poll.rs / monitor.rs / feedback.rs / main.rs への 1 行追加レベルの edit でも pre-existing 20 violations が 毎回 flag され、hook 出力 12.6KB の繰り返しで token 浪費 + scope creep への暗黙圧力が発生していた (Tier 1 #1)。 実装方針 (Approach 3 / advisor 推奨): - Edit (non-empty new_string): post-edit source 内で new_string の出現を 全件検索 → 各 match の行 range の union を filter として使用 - Edit (empty new_string): 純削除につき lint skip - Write / MultiEdit / 不明 tool: フィルタなし (whole-file lint) - new_string が見つからない (line ending mismatch 等): 安全側に倒し whole-file lint にフォールバック MultiEdit は v1 では未対応 (whole-file lint にフォールバック)。利用頻度 が低く no-regression な現状維持で十分なため、follow-up とする。 検証: - 60 unit tests pass (cargo test) - cargo clippy -- -D warnings clean - 手動統合テスト: pre-existing comments を含むファイルへの Edit (line 3 のみ変更) → line 3 のみ flag、line 1/4 の pre-existing は除外 ✓ - 純削除 (new_string="") → 出力なし (lint skip) ✓ - Write tool → whole-file (3 violations) ✓ - 多行 new_string → 範囲が複数行に展開され正しく filter ✓
順位 50 (comment-lint hook の scope を変更行に限定) を実装完了 (前 commit) に 伴い、todo.md 表と todo5.md 詳細エントリを削除。v1 では MultiEdit が whole-file fallback (no-regression) で残るため、Tier 3 follow-up として 順位 52 を新規登録。
|
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レビュー差分運用で ChangesComment-Lint Hook の変更行フィルタリング実装
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks-post-tool-comment-lint-rust/src/main.rs`:
- Around line 214-221: The filter currently only checks the comment node's start
line (via node.start_position()/start.row) which misses edits inside multi-line
block comments; update the check to compute both start and end lines (use
node.start_position() and node.end_position(), convert to 1-based with +1), then
replace the single-line call to line_in_ranges with an intersection check
between the node line range and the provided ranges (e.g., create or call a
helper like line_range_intersects_ranges(start_line..=end_line, ranges) and if
it returns false continue). Ensure you reference the existing symbols
node.start_position(), node.end_position(), line_filter and line_in_ranges (or
replace line_in_ranges usage) so the code skips only when the entire node range
is outside the filter ranges.
- Around line 149-152: The loop in locate_string_line_ranges advances
search_start by a raw +1 byte which can land inside a UTF-8 codepoint and cause
a panic when slicing; change the advancement to skip to the next valid character
boundary instead of a single byte. After you compute the byte index of the match
(absolute), determine the UTF‑8 length of the next character (e.g. via
chars().next().map(|c| c.len_utf8()) on the substring starting at absolute) and
set search_start = absolute + that length (or handle end-of-string safely);
update the code paths that use search_start, remaining and needle so slices
always use valid UTF‑8 boundaries.
🪄 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: 99b16e9e-5abe-40ad-9777-818a348a5b6f
📒 Files selected for processing (3)
docs/todo.mddocs/todo5.mdsrc/hooks-post-tool-comment-lint-rust/src/main.rs
Resolved findings: - [Critical] src/hooks-post-tool-comment-lint-rust/src/main.rs:152 `locate_string_line_ranges` 関数が UTF-8 文字境界を破壊し、パニックを引き起こす可能性があります - [Minor] src/hooks-post-tool-comment-lint-rust/src/main.rs:221 複数行 block comment が変更範囲内でも検出漏れします
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
aloekun
added a commit
that referenced
this pull request
May 3, 2026
* docs(todo): PR #104 post-merge-feedback の採用分 + Bundle b (CR operation 安定化) を追加 - 順位 53: rate-limit retry の CronCreate 化 (Bundle b PR-1, Tier 1) — 47 min rate-limit auto-retry 不能の致命点解消 - 順位 54: review 完了待ちの CronCreate 化 + observer 廃止 (Bundle b PR-2, Tier 2) - 順位 55: config 拡張 + SessionStart catch-up (Bundle b PR-3, Tier 3) - 順位 56: comment-lint hook test 拡充 (PR #104 T2-1+T2-2 bundle, Tier 2) — UTF-8 multi-byte + block comment boundary の回帰テスト体系化 PR #104 post-merge-feedback の Tier 2-3/4 と Tier 3 全件は採用見送り (overlap / 過剰一般化リスク / 重複ルール)。 * feat(hooks): 関数長スケーリング検出 lint 追加 (順位 48 / PR #101 T1-4) CLAUDE.md `coding-style.md` 50 行ガイドラインを決定論的に維持する PostToolUse lint を `hooks-post-tool-comment-lint-rust` に追加。touch-trigger ratchet 方式で 既存の 50 行超過関数 (40 個) は変更行に触れた瞬間にだけ flag、grandfather される。 実装: - 新 violation type `RUST_FUNCTION_TOO_LONG` (severity: error) - 閾値 `MAX_FUNCTION_LINES = 50` - `find_function_length_violations`: `compute_metrics` の関数 length を使い、 > 50 行 かつ 関数 body の行範囲が `line_filter` と overlap する関数のみ flag - 順位 50 の `compute_changed_lines` ロジックを再利用 (Edit / Write / MultiEdit の挙動と整合) - empty filter (Edit 純削除) は lint skip、None filter (Write / MultiEdit) は whole-file lint - 既存 `line_in_ranges` を削除 (`span_overlaps_ranges` で代替) - `main` を 64 行 → 39 行に refactor (`extract_file_path` / `collect_all_violations` / `emit_violations_feedback` に分割)、本 lint の 自己適用 Pivot 経緯: - 当初計画は oxlint 自作 rule + .oxlintrc.json + src/oxlint-rules/ だったが、 oxlint は JS/TS 専用 + 自作 rule 未対応のため structurally impossible - Rust 限定 + 既存 Rust hook 拡張で代替 (ユーザー承認、TS/JS は保留) - ロールアウト戦略は Ratchet (touch-trigger、既存 40 個 grandfather) を採用 検証: - cargo test: 73 passed (新 10 件: function length 系) - cargo clippy --all-targets -- -D warnings: clean - 自己適用: main.rs の 50 行超関数は `find_violations` (90 行 / 既存 / 触らず) と `function_too_long_violation` (32 行 / 新規 / OK)、main は 39 行に refactor - 統合テスト: main.rs 内 main() を Edit する hook input → RUST_FUNCTION_TOO_LONG 発火を確認 * docs(todo): 順位 48 完了に伴い削除 順位 48 (関数長スケーリング検出 lint) を実装完了 (前 commit) に伴い、todo.md 表と todo5.md 詳細エントリを削除。 * fix(review): apply CodeRabbit fixes for #105 Resolved findings: - [Major] src/hooks-post-tool-comment-lint-rust/src/main.rs:299 touch-trigger 判定が「body overlap」仕様と一致していません。 - [Minor] src/hooks-post-tool-comment-lint-rust/src/main.rs:551 統合後の違反件数に全体上限が効いていません。
7 tasks
aloekun
added a commit
that referenced
this pull request
May 5, 2026
…le b PR-1, 順位 53) (#113) * feat(cli-pr-monitor): rate-limit retry を CronCreate park モデルに移行 (Bundle b PR-1, 順位 53) PR #104 で 47 分 rate-limit を実観測した致命点 (max_duration_secs=600s cap で auto-retry できず action_required 通知 → ユーザー手動介入が必要) への対策。 cli-pr-monitor が同プロセス内で std::thread::sleep する設計を廃止し、reset 時刻を state.next_wakeup_at_unix に保存して PARK signal を stdout に emit して exit するモデルに切り替える。Claude Code 側が PARK signal を読み CronCreate (durable: true) で wakeup を予約、reset 後に cli-pr-monitor.exe --monitor-only が再 invoke されて @coderabbitai review が自動投稿される。 ADR-030 (post-merge-feedback) で実証済の責務分離パターン (機械的 Rust + Claude Code 定期チェックの分離) を CR rate-limit に適用する 3 例目。CronCreate には 時間制約がない (60 分 cap は別ツール ScheduleWakeup の制約) ため、47 分でも 90 分でも cron で任意時刻予約可能。durable: true で .claude/scheduled_tasks.json に永続化され session 跨ぎを native サポート。 主な変更: - PrMonitorState に next_wakeup_at_unix / wakeup_reason を追加 - handle_rate_limit_retry を RateLimitOutcome enum 返却に refactor (Posted / Parked { wakeup_at_unix } / Failed) - reset 時刻が未来 → Parked → state 更新 + PARK signal emit + parked_rate_limit action で early return - reset 時刻が過去 → 従来通り即時 @coderabbitai review 投稿 (Posted) - max_duration_secs > sleep_secs の予算チェック撤廃 (sleep 自体が廃止) - print_report を parked verdict 対応に拡張 - 副次的 refactor (touch-trigger ratchet 適用): run_poll_loop を PollContext + run_one_iteration 等 11 helper に分割、print_report を compute_verdict + print_findings_table に分割 PARK signal フォーマット (stdout): [PR_MONITOR_PARK] pr / repo / reset_at_unix / reset_at_iso_utc / wait_total_seconds / retry_count / max_retries / exe / cwd + CronCreate({...}) 呼び出しテンプレート (cron 式は Claude Code が local TZ で計算、durable: true / recurring: false / prompt にコマンドを埋め込み) [/PR_MONITOR_PARK] 既知の制約 (PR description / Bb-2/Bb-3 で対応予定): - 単一 state file 設計は維持 (複数 PR 並行時の collision は Bb-2/Bb-3 で扱う) - session 終了時の wakeup catch-up は Bb-3 (SessionStart hook 拡張) で対応 - 過渡期: Bb-1 land 後 Bb-3 land 前は AI 離席中の wakeup スキップが残る (durable: true で再起動時 fire-on-idle するため致命的ではない) 設計根拠は docs/coderabbit-monitoring-efficiency.md の「着手決定 (2026-05-05)」 セクションを参照。 Tests: - 125 passed / 0 failed (cargo test -p cli-pr-monitor) - 新規テスト 6 件: state serde (3) + RateLimitOutcome 経路 (2) + format_park_signal (2) - cargo clippy -D warnings clean * fix(review): apply CodeRabbit fixes for #113 Resolved findings: - [Major] src/cli-pr-monitor/src/stages/poll.rs:351 state 永続化に失敗しても PARK を発行すると durable な再 park ループになります - [Major] src/cli-pr-monitor/src/stages/poll.rs:495 CronCreate prompt のコマンドは path をクォートしないと Windows で壊れます - [Major] src/cli-pr-monitor/src/stages/poll.rs:515 One-shot cron は ISO 8601 形式で秒精度をサポートするため、5 フィールド cron への変換を避ける
5 tasks
aloekun
added a commit
that referenced
this pull request
May 13, 2026
… test (#151) * feat(hooks): Phase D D-5 — comment-lint test 拡充 + MAX_CUSTOM_VIOLATIONS outer/inner break test (順位 56 + 119) ## D-5 実装 (順位 56 + 119 bundle) ### 順位 56: comment-lint hook test 拡充 (PR #104 T2-1+T2-2 bundle) `src/hooks-post-tool-comment-lint-rust/src/main.rs`: - UTF-8 multi-byte test 5 パターン追加: 漢字 + ASCII 混合 / 漢字単独 / emoji / BMP 外 文字 (𝕊) / 結合文字 (e + U+0301) - Block comment boundary test 6 パターン追加: 複数行 (start/end/middle 被覆) + 単行 block comment (exact / starts-at / ends-at) で `span_overlaps_ranges` の inclusive-bounds 区間交差判定を体系化 - 既存 1 パターンずつのテストは保持 (regression 防止) ### 副産物: `byte_offset_to_line` char-boundary panic fix UTF-8 漢字単独パターン test 着手時に発見した production bug を修正: - 旧: `source[..clamped].bytes()` は `clamped` が char boundary でない場合 panic - 新: `source.as_bytes()[..clamped].iter()` で byte slice 経由に変更 (char boundary 不要) - 影響: needle 末尾が multi-byte 文字の場合 (例: 「漢字のみのコメント」を Edit/Write の new_string として渡す現実シナリオ) で hook が panic していたのを修正 - direct unit test `byte_offset_to_line_handles_mid_multibyte_offset` を追加して bug location を pinpoint ### 順位 119: MAX_CUSTOM_VIOLATIONS outer/inner loop break scope explicit test `src/hooks-post-tool-linter/src/main.rs`: - `run_custom_rules_outer_break_skips_subsequent_rules`: rule_a 単独で 21 件 → cap で 20 件、rule_b は呼ばれない (全 violation が "RULE_A") を assert - `run_custom_rules_inner_cap_after_partial_first_rule`: rule_a 19 件 + rule_b 1 件 = 20 件 (rule_b の inner break が mid-rule で発火) を type 分布で検証 - PR #148 で発見した `run_custom_rules` refactor の outer/inner loop break scope を test net で固定化 ## test 結果 - hooks-post-tool-comment-lint-rust: 85 passed (前 84 + 新 1 direct unit + 11 integration) - hooks-post-tool-linter: 111 passed (前 109 + 新 2) - 合計 196 tests pass ## docs 整理 - docs/todo7.md: 順位 56 entry 削除 - docs/todo8.md: 順位 119 entry 削除 - docs/todo-summary.md: 順位 56 / 119 行削除 - docs/local-llm-offload-analysis.md: D-5 row を ✅ 着手済 に更新、bug fix を副産物として明記 ## 並行: PR #150 post-merge-feedback 登録 (本セッション前から @ に含まれていた変更) - docs/todo8.md: 順位 120 (rule comment + ADR-007 case study) / 順位 121 (4 fields 個別 fixture test) / 順位 122 (development-workflow.md Step 0 stale entry 確認 step) 登録 - docs/todo-summary.md: 3 行追加 ## Phase D 累積状況 D-3 (PR #148) + D-4 (PR #150) + D-5 (本 PR) = 3 PR 完了、Phase E 着手要件 (5 PR 累積) まであと 2 PR (D-6 + D-7)。本 PR の dogfood は push 時に `LINT_SCREEN_ENABLED=true` で opt-in 観測予定。 * docs(analysis): Phase D D-5 dogfood outcome を記録 (lint_screen 4 件目の data point)
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
Summary by CodeRabbit
Bug Fixes
Documentation