feat(session-start): nudge の telemetry 統合 (ADR-055 / PR-N3) - #301
Merged
Conversation
SessionStart hook の 5 nudge (weekly_review_reminder / pr_monitor_catchup / reaper / staleness / workspace_stale) の発火を lib-telemetry に warn として記録し、 ROI 棚卸し (ADR-055 WP-12) の観測基盤 firings-*.jsonl に載せる。記録失敗・opt-in OFF は lib-telemetry 内部で握りつぶす fail-open。 - Cargo.toml: lib-telemetry 依存を追加 - main.rs: record_nudge_firing ヘルパー + 5 発火点への配線。 emit_session_start_output が 50 行上限 (touch-trigger ratchet) を超えたため append_pr_monitor_catchup_nudge / append_cwd_nudges に責務分割 (挙動不変) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
初版で除外していた session-start nudge を firing 計装対象に加える。除外根拠 「decision 語彙が block/warn の 2 値で nudge は乗らない」を撤回し、warn は「発火の重み」軸 (jj-op-verify の非 block warn と同性質) なので nudge に整合すると整理した。 - 除外リストから session-start reminder を外し amendment へのポインタに更新 - Amendment (2026-07-19) セクション追加: 撤回根拠 / 計装 id 5 種 / ADR-059 動機 / 残る除外 - 関連 ADR に ADR-059 を追記 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
session-start nudge の telemetry 統合 (PR-N3) の作業記録を追記。3 分割コミット / ADR-055 除外根拠の撤回判断 / cargo test 93 passed / clippy クリーン / pnpm build:all 成功 / デプロイ exe 駆動での firings-*.jsonl E2E 確認 (pr_monitor_catchup + weekly_review_reminder の 2 発火行) と残タスク (削除条件 4 の land 後目視確認) を記録。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughsession-start nudge 5種を ChangesSession-start nudge telemetry
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant SessionStartHook
participant lib_telemetry
participant firings_jsonl
SessionStartHook->>lib_telemetry: record_nudge_firing(nudge_id, session_id)
lib_telemetry->>firings_jsonl: write warn firing
lib_telemetry-->>SessionStartHook: return without blocking hook output
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 バックストップ)
レビュー指摘が現時点で 1 件も無いため、CI 状態と diff 概要のみの軽量サマリーとする。 Diff 概要PR-N3: session-start nudge 群 (5 種) を ADR-055 telemetry 計装に追加する変更。
Applicable Findings (Critical / High / Major)(該当なし) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし — レビュー指摘自体がまだ存在しない) 次のアクション
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks-session-start/src/main.rs (1)
150-170: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value将来の拡張に備えた防衛的なリファクタリングの提案
現在の実装では機能的に問題ありませんが、独立した副作用（各 nudge の評価と context への追記）を直列に並べる関数において、
?演算子による早期リターン（151行目や164行目など）を使用すると、将来的に別の nudge をこの関数の末尾に追加した際、手前の設定（session_startやweekly_config）がNoneだと後続の処理全体が意図せずスキップされる罠になります。将来の拡張に備え、
?演算子を使わずにif letで明示的なブロックスコープに閉じる記述をお勧めします。🛠️ 提案するリファクタリング
- let hooks_config = read_hooks_config(cwd); - let session_start = hooks_config.session_start.as_ref()?; - if let Some(staleness_config) = session_start.staleness.as_ref() { - if let Some(staleness_nudge) = compute_staleness_nudge(cwd, staleness_config) { - context.push_str("\n\n"); - context.push_str(&staleness_nudge); - record_nudge_firing("staleness", session_id); - } - if let Some(stale_nudge) = compute_workspace_stale_nudge(staleness_config) { - context.push_str("\n\n"); - context.push_str(&stale_nudge); - record_nudge_firing("workspace_stale", session_id); - } - } - let weekly_config = session_start.weekly_review_reminder.as_ref()?; - let weekly_nudge = compute_weekly_review_reminder_nudge(cwd, weekly_config, now_unix)?; - context.push_str("\n\n"); - context.push_str(&weekly_nudge.additional_context); - record_nudge_firing("weekly_review_reminder", session_id); - weekly_nudge.system_message + let hooks_config = read_hooks_config(cwd); + if let Some(session_start) = hooks_config.session_start.as_ref() { + if let Some(staleness_config) = session_start.staleness.as_ref() { + if let Some(staleness_nudge) = compute_staleness_nudge(cwd, staleness_config) { + context.push_str("\n\n"); + context.push_str(&staleness_nudge); + record_nudge_firing("staleness", session_id); + } + if let Some(stale_nudge) = compute_workspace_stale_nudge(staleness_config) { + context.push_str("\n\n"); + context.push_str(&stale_nudge); + record_nudge_firing("workspace_stale", session_id); + } + } + if let Some(weekly_config) = session_start.weekly_review_reminder.as_ref() { + if let Some(weekly_nudge) = compute_weekly_review_reminder_nudge(cwd, weekly_config, now_unix) { + context.push_str("\n\n"); + context.push_str(&weekly_nudge.additional_context); + record_nudge_firing("weekly_review_reminder", session_id); + return weekly_nudge.system_message; + } + } + } + None🤖 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-session-start/src/main.rs` around lines 150 - 170, Refactor the session-start nudge flow to avoid `?`-based early returns for `session_start` and `weekly_review_reminder`. Use explicit `if let` blocks around the staleness processing and weekly reminder processing so missing configuration skips only that nudge while allowing later independent nudges to run; preserve the existing context updates and firing records.
🤖 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.
Nitpick comments:
In `@src/hooks-session-start/src/main.rs`:
- Around line 150-170: Refactor the session-start nudge flow to avoid `?`-based
early returns for `session_start` and `weekly_review_reminder`. Use explicit `if
let` blocks around the staleness processing and weekly reminder processing so
missing configuration skips only that nudge while allowing later independent
nudges to run; preserve the existing context updates and firing records.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31a651f4-998d-4922-91ed-46c40830b004
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
docs/adr/adr-055-firing-telemetry-collection.mddocs/weekly-review-notification-plan.mdsrc/hooks-session-start/Cargo.tomlsrc/hooks-session-start/src/main.rs
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし) Applicable Findings (Medium 以下)
Filtered (not applicable)(該当なし — fitness filter の定義済みカテゴリ（platform scope / intentional design / docs-only mismatch / sensitive-file / scope mismatch / false positive）のいずれにも該当せず、severity はレビュアー申告どおり維持) 次のアクション
|
3 tasks
aloekun
added a commit
that referenced
this pull request
Jul 19, 2026
* docs(todo): PR-N1〜N3 post-merge feedback の採用候補を todo 登録 (#288 昇格 + 329-332 新設) PR #299/#300/#301 の post-merge feedback から採用候補 5 件を系統別に登録: - #288 昇格 (系統 A): pre-push review の diff スコープ漏れが 3 連続再発 (Severity High)。 post-merge 全 run 集約に加え push-runner [diff] stage の tip-only 範囲修正を統合し Tier2→Tier1 - 329 (系統 B): 新規 ADR 起案時の「判断根拠 × 既存 ADR 定義」矛盾チェックリスト - 330 (系統 B): 行動要求 nudge の 2 チャネル返却 + 多義的戻り値 struct 化 convention - 331 (系統 C): systemMessage 含む JSON 出力の exe-spawn E2E テスト - 332 (系統 D): pnpm build:all の Windows cp.exe PATH 自動化 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(todo): PR #302 CodeRabbit 指摘を反映 (#288 fail-closed 祖先検証 / schema 移行 / Windows 検出 / ADR 意図変更) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
## 問題 `[diff] command` に `jj diff -r @` が直書きされており、AI レビュアーには **tip コミットの diff しか渡っていなかった**。祖先コミットは pre-push のセルフレビューを 一度も経ずに merge される。 同じパイプライン内で `pr_size_check` と `docs_only_routing` は `<base>..@` (PR 範囲) を見ており、`[diff]` だけが非対称だった。実際 PR #311 では 695 行の PR に対して 37 行だけがレビュー対象になり、レビュアーは渡された 37 行を見て正しく「docs-only」と 判定していた。**レビュアー側からは「渡された diff が PR 全体か」を検証できない**ため、 この誤りは誰にも検知されない。 docs/todo-summary2.md 順位 288 として既知で、Severity High で PR #268/#300/#301 に 続き #311 が 4 回目の再発。 ## 変更 - **範囲の真実源を 1 箇所に**: top-level `default_branch` を新設し、`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が `Config::resolve_base_branch` 経由で同じ値を使う。従来は section ごとに独立した `default_branch` を持ち、 「値を同期する義務」を config コメントで課していた (docs_only_routing.rs の doc に 明記されていた) が、義務はコード上の不変条件ではないため非対称を許していた。 section 側は後方互換の override として残す (派生プロジェクトの既存 config 対策)。 - **config から revset を排除**: `[diff] command` は `{{PR_RANGE}}` プレースホルダを 使う。push-runner が `<base>..@` に展開するため、狭い範囲を書く余地が無くなる。 - **範囲カバレッジ検査 (fail-closed)**: 生成した diff が PR 範囲の全変更ファイルを 含むか `jj diff --summary` と突き合わせ、不足があれば exit 5 で中断する。config の 書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるため、未更新の派生プロジェクト config も捕まえる。summary 取得失敗・`diff --git` ヘッダ不在 (= 収録ファイルを特定 できない) も「網羅している」に倒さずエラーにする (ADR-043)。 - **`--git` 形式へ切替 (順位 264)**: 範囲検査がヘッダを読む要件に加え、jj 既定形式は 色を落とすと `+`/`-` が消えて LLM レビュアーが削除を追加と誤読する (PR #256 で simplicity-review が todo 25 行の削除を追加と誤読し false positive REJECT、約 19 分 浪費)。 - `templates/push-runner-config.toml` も同時修正 (deploy:hooks で派生配布されるため)。 ## 実測で見つけた副次バグ 範囲 summary の取得を当初 shell 経由 (`[diff] command` と同じ経路) で実装したが、 実シェルで叩いたところ **cmd.exe はクォートを除去せず jj に渡す**ため `Revision '"<base>..@"' doesn't exist` で必ず失敗した。sh は除去するので Linux だけ 通る = Windows の全 push が fail-closed で止まる形。ユニットテストは summary 取得を 注入していたため検出できなかった。direct args 呼び出し (docs_only_routing の既存 `run_jj_diff_summary`) を共有する形に修正し、doc に理由を残した。両 stage は 「PR 範囲の変更ファイル一覧」という同一の問いを扱うため、別実装にすると本 PR が 排除した非対称を再導入することになる。 ## 検証 cargo test --workspace 全 pass / clippy clean。範囲検査は incident 形状 (PR 2 ファイル 変更に対し diff は tip の 1 ファイルのみ) の再現テストで固定し、fail-closed 経路 (summary 取得失敗 / ヘッダ不在) と過剰検知しない経路 (空 PR 範囲 / Windows パス 区切りの正規化) も併せて固定した。base branch 解決は 3 stage が同一範囲に解決される ことと override の優先順位を machine-enforce している。
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
) * fix(push-runner): AI レビュー対象 diff を PR 全体に修正し範囲を機械検査する (順位 288/264) ## 問題 `[diff] command` に `jj diff -r @` が直書きされており、AI レビュアーには **tip コミットの diff しか渡っていなかった**。祖先コミットは pre-push のセルフレビューを 一度も経ずに merge される。 同じパイプライン内で `pr_size_check` と `docs_only_routing` は `<base>..@` (PR 範囲) を見ており、`[diff]` だけが非対称だった。実際 PR #311 では 695 行の PR に対して 37 行だけがレビュー対象になり、レビュアーは渡された 37 行を見て正しく「docs-only」と 判定していた。**レビュアー側からは「渡された diff が PR 全体か」を検証できない**ため、 この誤りは誰にも検知されない。 docs/todo-summary2.md 順位 288 として既知で、Severity High で PR #268/#300/#301 に 続き #311 が 4 回目の再発。 ## 変更 - **範囲の真実源を 1 箇所に**: top-level `default_branch` を新設し、`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が `Config::resolve_base_branch` 経由で同じ値を使う。従来は section ごとに独立した `default_branch` を持ち、 「値を同期する義務」を config コメントで課していた (docs_only_routing.rs の doc に 明記されていた) が、義務はコード上の不変条件ではないため非対称を許していた。 section 側は後方互換の override として残す (派生プロジェクトの既存 config 対策)。 - **config から revset を排除**: `[diff] command` は `{{PR_RANGE}}` プレースホルダを 使う。push-runner が `<base>..@` に展開するため、狭い範囲を書く余地が無くなる。 - **範囲カバレッジ検査 (fail-closed)**: 生成した diff が PR 範囲の全変更ファイルを 含むか `jj diff --summary` と突き合わせ、不足があれば exit 5 で中断する。config の 書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるため、未更新の派生プロジェクト config も捕まえる。summary 取得失敗・`diff --git` ヘッダ不在 (= 収録ファイルを特定 できない) も「網羅している」に倒さずエラーにする (ADR-043)。 - **`--git` 形式へ切替 (順位 264)**: 範囲検査がヘッダを読む要件に加え、jj 既定形式は 色を落とすと `+`/`-` が消えて LLM レビュアーが削除を追加と誤読する (PR #256 で simplicity-review が todo 25 行の削除を追加と誤読し false positive REJECT、約 19 分 浪費)。 - `templates/push-runner-config.toml` も同時修正 (deploy:hooks で派生配布されるため)。 ## 実測で見つけた副次バグ 範囲 summary の取得を当初 shell 経由 (`[diff] command` と同じ経路) で実装したが、 実シェルで叩いたところ **cmd.exe はクォートを除去せず jj に渡す**ため `Revision '"<base>..@"' doesn't exist` で必ず失敗した。sh は除去するので Linux だけ 通る = Windows の全 push が fail-closed で止まる形。ユニットテストは summary 取得を 注入していたため検出できなかった。direct args 呼び出し (docs_only_routing の既存 `run_jj_diff_summary`) を共有する形に修正し、doc に理由を残した。両 stage は 「PR 範囲の変更ファイル一覧」という同一の問いを扱うため、別実装にすると本 PR が 排除した非対称を再導入することになる。 ## 検証 cargo test --workspace 全 pass / clippy clean。範囲検査は incident 形状 (PR 2 ファイル 変更に対し diff は tip の 1 ファイルのみ) の再現テストで固定し、fail-closed 経路 (summary 取得失敗 / ヘッダ不在) と過剰検知しない経路 (空 PR 範囲 / Windows パス 区切りの正規化) も併せて固定した。base branch 解決は 3 stage が同一範囲に解決される ことと override の優先順位を machine-enforce している。 * docs: ADR-027 に「diff 局所 = 観点の限定であって範囲の限定ではない」を明記 (順位 288/264) ## ADR-027 amendment ADR-027 が狭めたのは reviewer が使う criteria (cross-file 探索を要求しない) であり、レビュー対象に含めるコミットの範囲ではなかった。この 2 つが混同され、 `[diff] command` が tip コミット限定のまま運用されて 4 回の再発を招いたため、 射程を明文化した。 「レビュー対象は PR 範囲全体」と決定した根拠も併記: - 速度は理由にならない。同一 PR でレビュー対象を 37 行 → 1011 行 (27 倍) に 広げても 4m32s → 4m43s の +11 秒。ADR-027 の速度改善は arch-review facet の 除去 (219-270s/iter) によるもので、範囲縮小は寄与していなかった。 - 範囲が狭いことによる見落としはレビュアー側から検知できない (渡された diff が PR 全体かを検証する手段が無い)。 - CodeRabbit backstop はセルフレビューを省く理由にならない。独立した層として併用する。 ## todo 更新 - 順位 264 (`--git` 切替): 完了につきエントリと table 行を削除。 - 順位 288: `[diff]` 範囲修正の部分のみ完了として記録。**残タスク** (post-merge feedback の全 run 集約、bookmark_check.rs の祖先未レビュー穴の検証) は明示して 残す。エントリ全体を消すと未着手部分が失われるため削除しない。 * fix(push-runner): pre-push 範囲検査の欠陥修正 + CodeRabbit 指摘5件対応 (#313)
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
warnとして記録し、ROI 棚卸し (ADR-055 WP-12) の観測基盤 firings-*.jsonl に載せるrecord_nudge_firingヘルパー追加 + 5 発火点への配線。配線で 50 行上限 (touch-trigger ratchet) を超えたemit_session_start_outputをappend_pr_monitor_catchup_nudge/append_cwd_nudgesに責務分割 (挙動不変)warnは「発火の重み」軸 (jj-op-verify の非 block warn と同性質) で nudge に整合、と整理する Amendment を追加Context
weekly-review reminder (ADR-031) が約 4 週間ユーザーに気付かれず発火し続けた incident (ADR-059 / PR-N1) を受け、「どの nudge が実際に発火したか」を観測できる基盤が必要になった。本 PR は weekly-review 通知可視化改善計画 (PR-N1〜N3) の最終段 PR-N3 で、観測層のみを追加する。
ADR-059 の systemMessage 可視化は weekly reminder 限定で dogfood し、行動要求系 nudge への段階展開の採否を発火実績で判定する (期限 2026-08-16)。本計装がその観測データを供給する。表示ノイズがゼロのため、systemMessage と違い全 nudge 一括で記録する。
コミットはレビューしやすさ優先で 3 分割 (feat 配線 / ADR-055 追記 / 計画書作業記録)。stop-feedback-dispatch / user-prompt-feedback-recovery の計装は本 PR スコープ外 (各 hook を触る PR で個別に実施)。
Validation
cargo test -p hooks-session-start: 93 pass (観測層追加は挙動不変のため新規テストなし、telemetry 本体は lib-telemetry の 20+ テストが担保)cargo clippy --workspace --all-targets --all-features -- -D warnings: クリーンpnpm build:all: 成功 (全 crate release + exe 配布)pr_monitor_catchup/weekly_review_reminderの発火行 (hook=hooks-session-start / kind=hook / decision=warn) が append されることを確認pnpm pushpre-push-review: verdict=APPROVE (security / simplicity 両者、2026-07-19)/review-local(qwen3-coder:30b, 2 runs): 11 findings 全て誤検知/非該当と検証し rejectedReferences
Summary by CodeRabbit
新機能
ドキュメント