Skip to content

fix(telemetry): block 記録を実 quality 違反に限定し infra エラー混入を除外 (順位309) #329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aloekun merged 2 commits into from
Jul 29, 2026
Merged

fix(telemetry): block 記録を実 quality 違反に限定し infra エラー混入を除外 (順位309) #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe493d5
fix(telemetry): block 記録を実 quality 違反に限定し infra エラー混入を除外 (順位309)
aloekun Jul 29, 2026
4f90993
fix(review): apply CodeRabbit fixes for #329
aloekun Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 43 additions & 3 deletions docs/adr/adr-055-firing-telemetry-collection.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ warm-up 後に実データで棚卸し (step 2/3) を後続 PR で行う。本 A

`decision` は「hook がツールを実際に停止したか」ではなく「発火の重み」を表す軸である。
custom rule / jj-op-verify は additionalContext の助言層で実際には block しないが、severity
に応じて block/warn を記録する。逆に stop-quality は infra エラー (stdin/parse 失敗) の
fail-closed 経路でも block を emit するため、「hook が block を emit した総数」として記録
する。file-length gate の fail-closed 経路 (jj 失敗の判定不能 block) は ROI 信号を汚さない
に応じて block/warn を記録する。stop-quality は当初 infra エラー (stdin/parse 失敗) の
fail-closed 経路でも block を emit するため「hook が block を emit した総数」として記録して
いたが、後述の Amendment (2026-07-29) でこの定義を撤回し、実 quality 違反 (品質ステップ
失敗) の block のみ記録するよう限定した (WP-12 ROI 信号から infra ノイズを除外)。
file-length gate の fail-closed 経路 (jj 失敗の判定不能 block) は当初から ROI 信号を汚さない
よう記録しない。

### 副作用注入によるテスト可能性
Expand Down Expand Up @@ -239,6 +241,44 @@ stop-feedback-dispatch / user-prompt-feedback-recovery は本 PR では計装し
これらにも当てはまるが、計装は各 hook を触る PR で個別に行う (ADR-059 段階展開に連動)。
§計装スコープ の除外リストは本 amendment に合わせて更新した。

## Amendment (2026-07-29): block 記録を実 quality 違反に限定 (順位309、WP-12 ROI 信号の精度)

初版 § 計装スコープ は stop-quality の block 記録を「hook が block を emit した総数」と
定義し、infra エラー (stdin 読込 / JSON parse 失敗) の fail-closed 経路も計上していた。
WP-12 step 2/3 (発火数で hook の維持・削除を判断する ROI 棚卸し) の観点では、この infra
エラー混入が発火数を歪め「実際に品質違反を捕捉した回数」と乖離する。CodeRabbit Major 指摘
(順位309) を受け、**block 記録を実 quality 違反 (品質ステップ失敗) パス限定に絞り込む**。

### 3 hook の状態 (計装スコープ表のうち block を emit する hook)

| hook | 修正前 | 修正後 |
|---|---|---|
| hooks-stop-quality | stdin/parse 失敗の fail-closed block でも記録 | `emit_block(reason, cause: BlockCause)` に変更し、`cause.records_firing()` (QualityViolation のみ真) の場合だけ telemetry に記録する。infra エラー経路 (`read_stdin_or_block` / `parse_hook_input_or_block`) と worker thread panic (`run_quality_steps` の join 失敗) は `InfraError` 扱いで block decision のみ emit し記録しない。実 quality 違反 (品質ステップ失敗) のみ `QualityViolation` |
| 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 経路にしか存在しないため、コード構造でこの不変条件が保たれる
(追加のテストは設けない)。
Comment on lines +254 to +267

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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


また `run_quality_steps` の各失敗は `StepFailure { message, cause }` で由来を保持し、worker
thread panic (join の `Err`) は実 quality 違反ではないため `InfraError`、ステップの実失敗は
`QualityViolation` とする。`block_on_failures` は `aggregate_block_cause` で全体の cause を
決め (1 件でも実失敗があれば `QualityViolation`、全て panic なら `InfraError`)、`emit_block`
に渡す。これにより内部障害 (panic) を品質違反として ROI 信号に誤計上しない (CodeRabbit Major 指摘)。

### 帰結

- 「発火 0 = 削除候補」の ROI 信号が infra エラーの fail-closed block で汚染されなくなり、
発火数は「hook が実際に品質違反を捕捉した回数」を表す。
- fail-open 原則は不変。telemetry 記録の有無に関わらず block decision 自体は emit するため、
infra エラー時も Claude への block 通知は従来どおり行われ、ゲート挙動は変わらない。

## 関連 ADR

- [ADR-039](adr-039-experimental-feature-standard-pattern.md) — 試験運用標準パターン (opt-in / kill-switch / bounded lifetime)
Expand Down
1 change: 0 additions & 1 deletion docs/todo-summary2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
| 306 | 💎 Tier 3 | **quality gate isolation 機構を見送り、recovery による risk acceptance とした判断の記録 (negative result) (273.md T3-5 採用)** | todo16.md | S | なし (spike 見送り convention に従い、isolation 機構を却下し recovery コストの低さ (順位304) を理由に risk acceptance した根拠を記録。recovery は isolation の代替ではなく、予防機能の欠如という残存リスクと再検討条件を明記する) |
| 307 | 🔧 Tier 2 | **WP-12 step 2: 発火テレメトリ ROI 棚卸し pre-step (発火 0 の rule/preset/hook を削除候補提示)** | todo16.md | M | なし (**着手条件 = ADR-055 収集層マージから 28 日 warm-up 後**。それ以前は全項目が発火 0 = データ無しで判定無意味。集計は Rust exe、weekly-review に file-length-watchlist 同型 facet で接続、incident 由来ルールは発火 0 でも維持推奨の区別) |
| 308 | 💎 Tier 3 | **WP-12 step 3: ADR-039 bounded lifetime 判定の発火数機械化** | todo16.md | S | 順位 307 (step 2 の集計基盤に依存)。試験運用 ADR 機構の卒業/廃止検討を発火数で自動 promote。step 3 完了で WP-12 完了 |
| 309 | 🚀 Tier 1 | **telemetry の block 記録を実 quality 違反に限定（infra エラー混入除外）(275.md T1-1 採用)** | todo16.md | M | なし (CodeRabbit Major。ADR-055 で「emit 総数」と意図的定義したが WP-12 ROI 棚卸しが infra エラー〔stdin/parse 失敗〕混入で歪むため実 violation パス限定に絞る。3 hook 横断で分割 PR 推奨、ADR-055 amendment 併記) |
| 310 | 🚀 Tier 1 | **custom-regex preset の生 regex が telemetry id に流れる privacy footgun 是正（非ブロッキング follow-up 統合）(275.md T1-2 採用)** | todo16.md | S | なし (現行 config は named preset のみで非発火だが派生プロジェクトの latent footgun。fallback を合成 id〔"custom-block"〕に正規化 + ADR-055 に config privacy 注記) |
| 311 | 🚀 Tier 1 | **逐語的関数複製（3+ コピー）を pre-push 検出する DRY lint rule (275.md T1-3 採用)** | todo16.md | M | なし (is_truthy 三重複製事案。ADR-007 regex 層に threshold 検出追加。順位 313 の fixture と抱き合わせ) |
| 312 | 🔧 Tier 2 | **`.claude/telemetry/` の per-pid×日次 partition ファイル retention/cleanup (275.md T2-1 採用)** | todo16.md | M | 順位 307 (WP-12 step 2 と同時期＝step1 マージから 28 日後 2026-08-12 頃に着手) |
Expand Down
23 changes: 0 additions & 23 deletions docs/todo16.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,29 +272,6 @@

---

### telemetry の block 記録を実 quality 違反に限定（infra エラー混入の除外）(275.md T1-1 採用)

> **動機**: CodeRabbit Major 指摘。`emit_block` / `record_*_firing` が品質違反だけでなく fail-closed の infra エラー（stdin 読込失敗 / JSON parse 失敗）でも発火を記録する。ADR-055 では「hook が block を emit した総数」として意図的にこの設計にしたが、WP-12 の ROI 棚卸し（発火数で hook 維持を判断）では infra エラー混入が発火数を歪めるため、実 quality 違反パス（`block_on_failures` 等）限定に絞り込む方が信号が正確になる。
>
> **重要**: これは ADR-055 で「意図的」と記録した判断の見直しであり、実装時は ADR-055 の該当記述（emit 総数の定義）も併せて amendment する。3 hook 横断（hooks-stop-quality / hooks-stop-tool-call-leak / hooks-pre-tool-validate）のため実装は分割 PR 推奨。stop-tool-call-leak は実 leak でのみ emit_block を呼ぶため既に実質限定されている点も確認する。
>
> **参照**: `.claude/feedback-reports/275.md` Tier 1 #1、`src/hooks-stop-quality/src/main.rs`（`emit_block` / `record_block_firing`）、[ADR-055](adr/adr-055-firing-telemetry-collection.md) § 計装スコープ、WP-12 step 2（順位 307、集計精度の前提）。
>
> **実行優先度**: 🚀 Tier 1 — Severity Medium / Effort M。

#### 作業計画

- [ ] 各 hook の記録呼び出しを実 quality 違反パス限定に移動（infra エラー経路では記録しない）。record 位置の見直し。
- [ ] [ADR-055](adr/adr-055-firing-telemetry-collection.md) の「emit 総数」定義を amendment（実 violation 限定に方針変更した根拠を記録）。
- [ ] 各 hook のユニットテストで「infra エラー経路では telemetry を記録しない」ことを検証。
- [ ] 本エントリ削除 + todo-summary2.md 行削除。

#### 完了基準

- telemetry の block 記録が実 quality 違反に限定され、infra エラー（stdin/parse 失敗）では記録されないことがテストで保証され、ADR-055 の定義も整合していること。

---

### custom-regex preset の生 regex が telemetry id に流れる privacy footgun の是正（非ブロッキング follow-up 統合）(275.md T1-2 採用)

> **動機**: PR #275 の pre-push simplicity review 非ブロッキング warning（= セッション中に検出された「非ブロッキング follow-up」）。`tag_source(name, ...)` の `name` が named preset 名でなく `blocked_patterns` の生正規表現文字列の場合、その regex テキストがそのまま telemetry の `id` フィールドに載り、ADR-055 の「コマンド本文・内容は非記録」プライバシー原則と緊張する。現行 `hooks-config.toml` は named preset のみのため**非発火**だが、派生プロジェクトが raw-regex エントリを足すと該当する latent footgun。
Expand Down
Loading