feat(telemetry): 機構レジストリで発火 0 リストを never-fired/went-quiet に再定義 (ADR-062) - #336
Conversation
|
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:
📝 WalkthroughWalkthrough静的な機構レジストリを設定・構築し、rollup 履歴と組み合わせて発火 0 を never-fired と went-quiet に分類します。Markdown と JSON に分類情報、最終発火月、供給源失敗を出力し、関連テストと ADR を更新します。 Changes発火 0 リストの再定義
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FinishReport
participant RegistryBuilder
participant RollupHistory
participant ReportRenderer
FinishReport->>RegistryBuilder: build_registry
FinishReport->>RollupHistory: read rollup history
FinishReport->>ReportRenderer: pass registry and rollups
ReportRenderer->>ReportRenderer: classify never-fired or went-quiet
ReportRenderer-->>FinishReport: write Markdown and JSON
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 |
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし) 変更概要 (レビュー指摘 0 件のため軽量サマリー)
次のアクション
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli-telemetry-report/src/report.rs (1)
447-455: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
zero_firing_listの呼び出しが markdown / JSON で二重化しています。
format_zero_firingとzero_firing_jsonが同じ 5 引数呼び出しとmonitored_ids収集を複製しています。renderで一度算出して両者に渡す形にすると、引数追加時の片側漏れ(md と JSON の内容乖離)を構造的に防げます。任意対応で構いません。🤖 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/cli-telemetry-report/src/report.rs` around lines 447 - 455, `format_zero_firing` と `zero_firing_json` に重複している `monitored_ids` の収集および `zero_firing_list` 呼び出しを、`render` で一度だけ実行する形に変更してください。算出した一覧を両フォーマッタへ渡し、各関数内の再計算を削除して、Markdown と JSON が同じ結果を利用する構造にしてください。
🤖 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-telemetry-report/src/registry.rs`:
- Line 90: Update the hook handling around push_entries so an empty hook_ids
list is recorded in source_failures as a missing hook source instead of silently
producing zero entries. Preserve the existing entry population behavior when
hook_ids is non-empty, and use the established failure representation used by
rule and preset sources.
---
Nitpick comments:
In `@src/cli-telemetry-report/src/report.rs`:
- Around line 447-455: `format_zero_firing` と `zero_firing_json` に重複している
`monitored_ids` の収集および `zero_firing_list` 呼び出しを、`render`
で一度だけ実行する形に変更してください。算出した一覧を両フォーマッタへ渡し、各関数内の再計算を削除して、Markdown と JSON
が同じ結果を利用する構造にしてください。
🪄 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: 784f59be-c9ac-430b-abda-26ec57fed9ad
📒 Files selected for processing (7)
.claude/hooks-config.tomldocs/adr/adr-062-monthly-harness-roi-review.mdsrc/cli-telemetry-report/src/config.rssrc/cli-telemetry-report/src/incident.rssrc/cli-telemetry-report/src/main.rssrc/cli-telemetry-report/src/registry.rssrc/cli-telemetry-report/src/report.rs
| ), | ||
| } | ||
|
|
||
| push_entries(&mut entries, hook_ids.to_vec(), "hook"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
hook_ids が空のときも欠落として明示すべきです。
rule / preset は読取失敗を source_failures に積みますが、hook は config 未設定・空でも無言で 0 件になります。この場合「hook の never-fired 判定が実施できていない」ことがレポート上区別できず、モジュール doc(設計決定 1)の「読めなかった」と「id が 0 件」を区別する方針と食い違います。hook は自動列挙元が無いため、空リストは実質的に供給源欠落です。
🔧 hook 供給源欠落の明示
- push_entries(&mut entries, hook_ids.to_vec(), "hook");
+ if hook_ids.is_empty() {
+ source_failures.push(
+ "hook 供給源 ([telemetry_report.registry] hook_ids) が未設定のため hook / nudge の never-fired 判定は不能"
+ .to_string(),
+ );
+ } else {
+ push_entries(&mut entries, hook_ids.to_vec(), "hook");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| push_entries(&mut entries, hook_ids.to_vec(), "hook"); | |
| if hook_ids.is_empty() { | |
| source_failures.push( | |
| "hook 供給源 ([telemetry_report.registry] hook_ids) が未設定のため hook / nudge の never-fired 判定は不能" | |
| .to_string(), | |
| ); | |
| } else { | |
| push_entries(&mut entries, hook_ids.to_vec(), "hook"); | |
| } |
🤖 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/cli-telemetry-report/src/registry.rs` at line 90, Update the hook
handling around push_entries so an empty hook_ids list is recorded in
source_failures as a missing hook source instead of silently producing zero
entries. Preserve the existing entry population behavior when hook_ids is
non-empty, and use the established failure representation used by rule and
preset sources.
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし) Applicable Findings (Medium 以下)
Filtered (not applicable)(該当なし — 2 件とも source/config 変更で Windows-only/docs-only/sensitive-file/read-only zone のいずれにも該当せず、ADR にも矛盾しないため applicable と判定) 次のアクション
|
… A, ADR-062) 発火 0 = 削除候補の中核シグナルが実装上機能していなかった問題を解消する。従来の発火 0 リストは 窓内 rollup に現れた id しか列挙せず、rollup の id entry は発火レコードからしか作られないため (1) 発火が止まり窓外に落ちた id (went-quiet) と (2) 一度も発火していない機構 (never-fired) が どちらも不可視だった。 機構レジストリ (registry.rs) で 3 供給源から母集合を静的列挙する: - rule: custom-lint-rules.toml の全 rule id (incident.rs の読み口を read_all_rule_ids で拡張) - preset: hooks-config.toml [pre_tool_validate] blocked_patterns (= telemetry の hit.source と一致) - hook/nudge: 自動列挙元が無いため config [telemetry_report.registry] hook_ids を新設 発火 0 集合 = (レジストリ ∪ 全 rollup 履歴) − 窓内発火 id。never-fired / went-quiet (最終発火月併記) の 2 区分でレポート。degraded 実行時は (b) 全体に参考値注記、供給源読取失敗は fail-open で skip しつつ 欠落を明示 (silent fallback 排除)。JSON に provenance / last_fired_month / registry.source_failures 追加。 hook_ids の dogfood 値は各 hook の lib_telemetry::record 実装から実確認 (file-length / jj-op-verify / pr_monitor_catchup / reaper / staleness / workspace_stale / weekly_review_reminder / monthly_review_reminder / hooks-stop-quality / hooks-stop-tool-call-leak[/prompt-recovery])。 実データ検証 (improve workspace から pnpm telemetry-report、非 degraded): file-length / reaper / workspace_stale / monthly_review_reminder 等の未発火 hook と 3 本の未発火 incident rule が never-fired に 出現、leak verdict は NotMet 不変。 ADR-039 additive (section 不在でも rule/preset は自動列挙)。 ADR-062 / ADR-055
Phase A 実装 (registry.rs + 発火 0 リスト 2 区分化) の設計根拠を ADR に記録する。3 供給源 (rule/preset/hook) の id 語彙の実確認方針、発火 0 集合の新定義 (レジストリ ∪ 履歴 − 窓内発火)、 never-fired/went-quiet の 2 区分と最終発火月併記、degraded 参考値注記、供給源欠落の明示、 JSON provenance/last_fired_month を明記。§ 決定 4 の「発火 0 リスト全般で足りる」の実装充足。 ADR-062
a41111b to
1858b39
Compare
Phase E の締めくくり: 1. ADR-062 § 決定 2 amendment の fail-open 記述に、hook 供給源特有の判断を 1 文補足。 hook は自動列挙元が無く hook_ids が唯一の供給源のため、空リストも実質的な供給源欠落として 明示する (rule/preset の空 = 読める上での 0 件、hook の空 = 列挙不能)。PR #336 の CodeRabbit 指摘で実装した挙動を ADR に明文化 (実装中に生じた判断を doc コメントのみに留めない)。 2. 設計決定 1〜4 が ADR-062 amendment 群に漏れなく記載済みであることを照合済みのため、指示書 としての役目を終えた docs/monthly-harness-roi-review-plan.md を削除。 ADR-062
Phase E の締めくくり: 1. ADR-062 § 決定 2 amendment の fail-open 記述に、hook 供給源特有の判断を 1 文補足。 hook は自動列挙元が無く hook_ids が唯一の供給源のため、空リストも実質的な供給源欠落として 明示する (rule/preset の空 = 読める上での 0 件、hook の空 = 列挙不能)。PR #336 の CodeRabbit 指摘で実装した挙動を ADR に明文化 (実装中に生じた判断を doc コメントのみに留めない)。 2. 設計決定 1〜4 が ADR-062 amendment 群に漏れなく記載済みであることを照合済みのため、指示書 としての役目を終えた docs/monthly-harness-roi-review-plan.md を削除。 ADR-062
* docs(todo): 月次 ROI レビュー (PR #335-338) post-merge feedback 採用候補を todo14 に登録 #336/#337/#338 の post-merge-feedback レポートの採用候補 (6 系統 13 項目、dedup 済) を docs/todo14.md の現在進行中に 4 エントリとして登録する。ユーザー承認済み (全系統 = todo.md 登録)。 - cli-telemetry-report コード堅牢化 + 回帰テスト (系統1+2、resolve_snapshot 越境テスト等) - telemetry 時間語義・不変条件・degraded 運用の文書補強 (系統3+5) - jj workspace/bookmark semantics 文書 + pr-monitor 回帰テスト (系統4) - 開発ワークフロー規約の補強 (系統6) ADR-033 準拠 (本文に順位番号を書かず PR/Tier 参照のみ)。優先度 table (todo-summary2.md) の 行追加はユーザー判断のため本コミットでは行わない。実装は後日。 * docs(todo): CodeRabbit PR #339 指摘を反映 (checked_sub 計画化 / 検出限界の表記整合) - verdict.rs 計画を debug_assert! 単独から checked_sub ベースの明示処理 + 診断用 debug_assert! 併設に変更 (release build でも underflow 防止)。 完了基準に release-mode 判定保証の回帰テストを追加 - pr-monitor 回帰テストの bullet を「既知の false negative を明示記録、 検出改善はスコープ外」に修正し、対処案の seal 方針と表記を整合
概要
月次レビュー dogfood 追加アクション A。「発火 0 = 削除候補」の中核シグナルが実装上機能していなかった問題を解消する。
問題
従来の発火 0 リストは窓内 rollup に現れた id しか列挙せず、rollup の id entry は発火レコードからしか作られない(
count_firingsは block/warn のみ加算)ため、以下がどちらも不可視だった:(ADR-062 § 決定 4 の「MVP は 1 件 + 発火 0 リスト全般で足りる」の後半が実装で未充足だった)
変更内容
registry.rs新設): 3 供給源から母集合を静的列挙custom-lint-rules.tomlの全 rule id(incident.rsの読み口をread_all_rule_idsで拡張)hooks-config.toml [pre_tool_validate] blocked_patterns(= telemetry のhit.sourceと一致)[telemetry_report.registry] hook_idsを新設(ADR-039 additive、section 不在でも rule/preset は自動列挙)provenance/last_fired_month/registry.source_failures追加id 語彙の実確認
hook_idsの dogfood 値は各 hook のlib_telemetry::record実装から実確認(hook 名と不一致のjj-op-verify/pr_monitor_catchup/hooks-stop-tool-call-leak/prompt-recovery等を config 列挙)。pre-push simplicity レビューでも record 呼び出し箇所との完全一致が確認済み。検証
cargo test --workspace/cargo clippy --workspace --all-targets -- -D warnings全通(rust-lint-test PASS)pnpm telemetry-report、非 degraded): 未発火 hook(file-length/reaper/workspace_stale/monthly_review_reminder等)と未発火 incident rule 3 本が never-fired に出現、leak verdict は NotMet 不変ADR-062 / ADR-055
🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
改善