fix(telemetry): block 記録を実 quality 違反に限定し infra エラー混入を除外 (順位309) - #329
Merged
Conversation
hooks-stop-quality の emit_block が stdin/parse 失敗の fail-closed block でも telemetry firing を記録していたため、WP-12 ROI 棚卸し (発火数で hook 維持を判断) の 信号が infra エラーで歪んでいた。BlockCause enum で経路を区別し、実 quality 違反 (block_on_failures) 経路のみ記録するよう限定。infra エラー経路は block decision を emit するが telemetry には記録しない (fail-open 挙動は不変)。 leak/pre-tool-validate は既に実 violation 経路限定 (run_check の実 leak 検出時のみ / validate_command hit 時のみ) のためコード確認のみで変更なし。ADR-055 § 計装スコープを amendment。todo16.md 順位309 entry と todo-summary2.md 行を消化。
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesstop-quality telemetry
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Input as stdin/HookInput
participant Gate as stop-quality
participant Telemetry as record_block_firing
Input->>Gate: 入力を読み込み・解析
alt 品質ステップ失敗
Gate->>Telemetry: QualityViolation を記録
Gate-->>Input: block decision を出力
else stdin/JSON エラー
Gate-->>Input: InfraError の block decision を出力
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし — レビュー未着のため指摘自体が存在しない) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし) 差分概要 (レビュー未着のための軽量サマリー)todo309 (block 記録を実 quality 違反に限定) の実装 PR。4 ファイル変更、+105/-41。
次のアクション
|
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/adr-055-firing-telemetry-collection.md`:
- Around line 254-267:
ADR-055の表と設計説明を、実装どおり`emit_block`経由で`BlockCause::records_firing()`を判定し、`QualityViolation`の場合のみ`emit_block_with`でtelemetryを記録する内容へ更新してください。`record_block_firing`を`block_on_failures`へ移動したという記述を削除し、`InfraError`では記録しない契約を明記してください。
In `@src/hooks-stop-quality/src/main.rs`:
- Line 421: run_quality_steps の失敗分類を更新し、品質失敗が存在する場合のみ emit_block に
BlockCause::QualityViolation を渡してください。worker thread の panic だけで品質失敗がない場合は
BlockCause::InfraError として記録し、既存の failures 処理で品質失敗と内部障害を分離してください。
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 582b5bcb-6ad2-496a-be66-dfd6d57b2f77
📒 Files selected for processing (4)
docs/adr/adr-055-firing-telemetry-collection.mddocs/todo-summary2.mddocs/todo16.mdsrc/hooks-stop-quality/src/main.rs
💤 Files with no reviewable changes (2)
- docs/todo-summary2.md
- docs/todo16.md
Comment on lines
+254
to
+267
| | hook | 修正前 | 修正後 | | ||
| |---|---|---| | ||
| | hooks-stop-quality | stdin/parse 失敗の fail-closed block でも記録 | `record_block_firing` を `emit_block` から実 quality 違反経路 (`block_on_failures`) へ移動。infra エラー経路 (`read_stdin_or_block` / `parse_hook_input_or_block`) は block decision を emit するが telemetry には記録しない | | ||
| | hooks-stop-tool-call-leak | (変更なし) | `emit_block` は `run_check` の実 leak 検出時のみ呼ばれ、transcript 読取失敗・連続数 0・上限到達は fail-open で return するため既に実 leak 限定。ADR-061 の回収層 (`prompt-recovery`) は `should_recover` 成立時のみ warn 記録 | | ||
| | hooks-pre-tool-validate | (変更なし) | `record_preset_block` は `validate_command` が hit を返す (= preset にマッチした実 violation) 経路のみ。stdin/parse 失敗は `ExitCode::FAILURE` で return し記録しない。既に preset match 限定 | | ||
|
|
||
| ### 設計: 経路を型で区別しテストで固定 | ||
|
|
||
| stop-quality に `BlockCause { QualityViolation, InfraError }` を導入し、`emit_block(reason, cause)` | ||
| が `cause.records_firing()` (QualityViolation のみ真) のときだけ telemetry に記録する。 | ||
| telemetry 記録を closure 注入した `emit_block_with` をテストの芯とし、「QualityViolation は | ||
| recorder を発火 / InfraError は発火しない」を副作用で観測して回帰ガードにする。leak / preset | ||
| は record 呼び出しが既に violation 経路にしか存在しないため、コード構造でこの不変条件が保たれる | ||
| (追加のテストは設けない)。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
ADR-055 の実装説明を最終コードに合わせてください。
表は record_block_firing を block_on_failures へ移動したと記載していますが、実装は emit_block_with 内で BlockCause::records_firing() を判定し、emit_block 経由で記録します。後段の設計記述とも不一致なので、「cause に応じて emit_block が記録する」と修正してください。
As per path instructions: ADR の適用内容は実装契約と一致させる必要があります。
🤖 Prompt for 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.
In `@docs/adr/adr-055-firing-telemetry-collection.md` around lines 254 - 267,
ADR-055の表と設計説明を、実装どおり`emit_block`経由で`BlockCause::records_firing()`を判定し、`QualityViolation`の場合のみ`emit_block_with`でtelemetryを記録する内容へ更新してください。`record_block_firing`を`block_on_failures`へ移動したという記述を削除し、`InfraError`では記録しない契約を明記してください。
Source: Path instructions
| failures.join("\n\n") | ||
| ); | ||
| emit_block(&reason); | ||
| emit_block(&reason, BlockCause::QualityViolation); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
worker thread の panic を QualityViolation として記録しないでください。
run_quality_steps は worker thread の panic も failures に追加するため、ここで常に QualityViolation を渡すと、品質違反がない内部障害まで telemetry に記録されます。品質失敗とインフラ失敗を分離し、品質失敗がある場合だけ QualityViolation、panic のみなら InfraError としてください。
🤖 Prompt for 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.
In `@src/hooks-stop-quality/src/main.rs` at line 421, run_quality_steps
の失敗分類を更新し、品質失敗が存在する場合のみ emit_block に BlockCause::QualityViolation
を渡してください。worker thread の panic だけで品質失敗がない場合は BlockCause::InfraError として記録し、既存の
failures 処理で品質失敗と内部障害を分離してください。
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)
Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)
次のアクション
|
Resolved findings: - [Minor] docs/adr/adr-055-firing-telemetry-collection.md:267 ADR-055 の実装説明を最終コードに合わせてください。 - [Major] src/hooks-stop-quality/src/main.rs:421 worker thread の panic を `QualityViolation` として記録しないでください。
aloekun
added a commit
that referenced
this pull request
Jul 30, 2026
* docs: 月次レビュー dogfood 追加アクション (A-D) 実装プラン + フィードバック採用 todo 登録 月次 ROI レビュー (ADR-062) の初回 dogfood (/monthly-review, 2026-07-30) で発見した構造的問題への 追加アクション A-D を docs/monthly-harness-roi-review-plan.md (前身は PR #333 で削除、同パスの後継 doc) として起こす: - A [Critical]: 発火 0 = 削除候補シグナルの不動作 (機構レジストリ欠如 + zero_firing_list デッドロジック) - B [High]: degraded 保守化 (PR #333) の文書波及漏れ 5 箇所の同期 - C [Medium]: rollup 確定時 snapshot が月中状態でなく確定時点で上書きされる問題 - D [Medium]: 未確定当月が promote streak に算入される問題 file:line と grep 用文言つきで自己完結記述。あわせて #329-#333 series の post-merge feedback 採用分 (A1-A4 + D1) を todo14.md + todo-summary2.md 順位 352-356 に登録。 docs-only。pnpm lint:md / lint:docs 全通。 * fix(review): apply CodeRabbit fixes for #334 Resolved findings: - [Major] docs/monthly-harness-roi-review-plan.md:10 「本ドキュメントだけで実装可能」という宣言と、未確定の実装指示が矛盾しています。 - [Minor] docs/monthly-harness-roi-review-plan.md:77 「恒久的に degraded」は条件付きの現状と一致しません。 - [Minor] docs/todo14.md:438 352番エントリの決定事項の件数を修正してください。
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
hooks-stop-quality:BlockCause { QualityViolation, InfraError }を導入し、telemetry の block firing 記録を実 quality 違反 (品質ステップ失敗) 経路に限定emit_block_withを芯に、記録有無を副作用で観測する回帰テストを 2 件追加hooks-stop-tool-call-leak/hooks-pre-tool-validateは既に実 violation 経路限定のためコード変更なしContext
.claude/feedback-reports/275.mdT1-1)。docs/monthly-harness-roi-review-plan.mdPhase 0 として先行実施run_checkの実 leak 検出時のみ、preset はvalidate_commandhit 時のみ record) と判明したため、単一変更にまとめたValidation
cargo test --workspace: 全 pass (stop-quality 42 件、新規 2 件含む)cargo clippy --workspace --all-targets -- -D warnings: 警告ゼロpnpm lint:md(117 files): 0 errorpnpm pushpre-push review: verdict=APPROVE (security-review / simplicity-review 両者、2026-07-29、1m45s)References
.claude/feedback-reports/275.mdT1-1、CodeRabbit Major)docs/monthly-harness-roi-review-plan.mdPhase 0Summary by CodeRabbit