Skip to content

feat: 週次レビュー Phase C — /weekly-review skill + SessionStart hook reminder + facet P-1 fix (ADR-031) - #184

Merged
aloekun merged 6 commits into
masterfrom
feat-weekly-review-phase-c
May 29, 2026
Merged

feat: 週次レビュー Phase C — /weekly-review skill + SessionStart hook reminder + facet P-1 fix (ADR-031)#184
aloekun merged 6 commits into
masterfrom
feat-weekly-review-phase-c

Conversation

@aloekun

@aloekun aloekun commented May 29, 2026

Copy link
Copy Markdown
Owner

Summary

ADR-031 (週次プロジェクト全体レビューパイプライン) の Phase C 本体 = /weekly-review skill + SessionStart hook reminder + facet P-1 fix を実装。Phase B (PR #182) で land した takt workflow weekly-review をオーケストレートする skill と、~7 日経過 / failed marker 残存時に additionalContext で promote する SessionStart hook reminder を bundle。

加えて、本セッション (#181#182#183 → 本 PR の chain) で蓄積された 3 件の post-merge-feedback 採用 (Bundle CR-RL todo / S01 todo / Bundle DG-RULES todo) + handoff doc carry-forward を同 PR に bundle (ユーザー判断 2026-05-29「既存コミット同 1 PR 許容」)。

含まれる commits (5 件)

1. docs(todo): PR #183 post-merge-feedback Bundle DG-RULES 採用 3 件 (順位 170-172)

PR #183 post-merge-feedback で採用された 3 件 (本セッションの dogfood の rule codify):

2. docs(todo): 順位 173 (S01) 追加 — PR #182 dry-run combine_output dead-code

PR #182 Phase B dry-run の S01 finding (5 crate に同一 8 行 combine_output 関数が #[allow(dead_code)] で重複) を lib-runner-utils (or 既存 lib-*) に extract する task として todo 化。

3. feat(phase-c): /weekly-review skill + SessionStart hook reminder + facet P-1 fix

本 PR の中核 commit。3 component を bundle:

Component A: skill /weekly-review (~/.claude/skills/weekly-review/SKILL.md、global)

  • Phase 0: pending JSON 先読み + failed marker 検出 (resume / new run 判別)
  • Phase 1: pnpm exec takt -w weekly-review 起動 + pre-state 書込
  • Phase 2: Report Directory → .claude/weekly-reviews/<date>.md + pending JSON 構築
  • Phase 3: AskUserQuestion (multiSelect, severity-grouped) で採否一括選択
  • Phase 4: 採用 finding を docs/todo.md 「## 現在進行中」配下に新セクション展開 (ADR-031 § todo.md 反映ルール テンプレート準拠)
  • Phase 5: pending JSON consume + last-run timestamp 書込 + failed marker cleanup
  • Phase 6: 完了サマリー表示
  • best-effort 失敗ポリシー (ADR-031 § 失敗ポリシー): .failed marker で次セッション SessionStart promote

Component B: SessionStart hook の weekly_review_reminder sub-feature

  • src/hooks-session-start/src/main.rsWeeklyReviewReminderConfig 追加
  • 2 nudge 経路:
    1. .claude/weekly-review-last-run.json の mtime > reminder_threshold_days (default 7)
    2. .claude/weekly-reviews/*.md.failed marker 1 件以上残存 (前回失敗 resume promote)
  • 両方該当時は 1 nudge にまとめて出力
  • 関数 50 行制約に対応するため build_*_lines helper を分離 (compute_weekly_review_reminder_nudge を 30 行に圧縮)
  • 6 新規 unit test 追加 (disabled / 失敗 marker 列挙 / staleness 発火 / failed 発火 / config parse / dir 不在)
  • .claude/hooks-config.toml[session_start.weekly_review_reminder] section 追加 (enabled = true で opt-in、ADR-039 experimental pattern)

Component C: aggregate-weekly.md の report_path documentation drift (P-1) 解消

4. docs(handoff): Phase B handoff retire + Phase C 以降向け 新規 handoff 作成

ユーザー判断記録 (2026-05-29)

質問 回答
Q1: MVP scope A (skill + hook + facet fix 3 component 同 PR で land)
Q2: failed marker recovery A (含める、ADR-031 § 失敗ポリシー 完全実装)
Q3: Bundle CR-RL 先行 land A (Phase C 先行、Bundle CR-RL = 順位 167-169 は別 PR で carry-forward)

Phase D / E (carry-forward)

  • Phase D: e2e 検証 (skill 実起動 → findings 採否 → todo 追記の flow 実 PR 内で dogfood)
  • Phase E: 試験運用 dogfood (1-2 週運用 + ADR-031 ステータス本採用化)
  • 詳細は docs/handoff-weekly-review-phase-c-onwards.md § 4 参照、docs/todo-summary.md 順位 8 で trackable

Pre-push 検証結果

  • Quality gate (lint / test / build / rust-test): PASS
  • pre-push-review (2m 9s):
    • simplicity-review: APPROVED (docs-only の 3 変更で実行コードへの変更なし、blocking anomaly なし — ただし scope は @ commit = handoff swap のみ。Phase C 実装 commit は scope 外、CodeRabbit が PR 全体 diff を review)
    • security-review: APPROVED

Note: pre-push reviewer の commit scope について

pre-push reviewer は @ commit のみ review する設計のため、本 PR の Phase C 実装 commit (@- = feat(phase-c)) は scope 外でした。CodeRabbit が PR 全体 diff を review することでカバーされますが、multi-commit chain で @ を docs 系 commit に置く構造的副作用として post-merge-feedback で記録予定。

Test plan

  • pnpm push で quality_gate + pre-push-review が全 PASS
  • hooks-session-start cargo test (新規 6 tests + 既存 48 tests = 54 tests 全 pass)
  • cargo clippy -p hooks-session-start clean (no warnings)
  • pnpm build:all + .claude/hooks-session-start.exe deploy 確認
  • skill /weekly-review が ~/.claude/skills/ に登録され session 内で available
  • CodeRabbit が Phase C 実装 + docs 変更を full review
  • CI green 確認後 merge 判断
  • (post-merge) skill /weekly-review を実際に呼出して e2e dogfood (Phase D)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • 新機能

    • 週次レビューの自動リマインダーを導入。既定では無効で、7日経過で通知が出るほか、失敗マーカー検出時にも再開候補を提示します。
  • ドキュメント

    • Phase B のハンドオフ資料を削除し、Phase C以降への引き継ぎ計画ドキュメントを新規追加しました。
    • レポート生成手順やTODO一覧を整理・追記しました。

Review Change Stack

aloekun added 4 commits May 29, 2026 15:37
…RULES)

採用: PR #183 post-merge-feedback Tier 3 #1/#2/#3 (2026-05-29 ユーザー承認):
- 順位 170 (T3-#1): git-workflow.md § Multi-PR chaining 拡張 — 1 PR 内 multi-commit + intent 明記
  Frequency High (PR #119/#120/#121 + #183 の 4 観測)、~/.claude/ global 編集、派生プロジェクトへ自動波及
- 順位 171 (T3-#2): docs-governance.md に Operational vs Pointer reference 区別 section 追加
  本 PR (#183) A01 修正で実適用した判定ロジックを codify、Bundle DG-RULES
- 順位 172 (T3-#3): CR ephemeral artifact Nitpick の統一 skip 基準を新 memory に codify
  本 PR で実施した Nitpick skip 判断 codify、Bundle DG-RULES、本リポジトリ専用

171 + 172 は Bundle DG-RULES として同 PR land 推奨 (docs/rule + memory の 2 層補強)。

メタ評価: 3 採用候補すべてが本セッション (#181-#183 chain) の dogfood で実体観測された
行動パターンの codify — Phase B 設計の self-reinforcing detection loop が機能した実例。

実装時の前提:
- 170/171 は ~/.claude/ global 編集のため feedback_global_config_backup 適用必須
- 172 は ~/.claude/projects/.../memory/ 編集、本リポジトリ専用 (派生展開なし)
…cet P-1 fix (ADR-031 Phase C)

ADR-031 Phase C を実装。Phase B (PR #182) で land した takt workflow weekly-review を
オーケストレートする skill と、~7 日経過 / failed marker 残存時に additionalContext で
promote する SessionStart hook reminder + PR #182 pre-push P-1 finding (facet report_path
drift) の修正を bundle。

Component A: skill /weekly-review (本リポジトリ内では非追跡、~/.claude/skills/weekly-review/SKILL.md として配置)
  - Phase 0: pending JSON 先読み + failed marker 検出 (resume / new run 判別)
  - Phase 1: pnpm exec takt -w weekly-review 起動 + pre-state 書込
  - Phase 2: Report Directory → .claude/weekly-reviews/<date>.md + pending JSON 構築
  - Phase 3: AskUserQuestion (multiSelect, severity-grouped) で採否一括選択
  - Phase 4: 採用 finding を docs/todo.md 「## 現在進行中」配下に新セクション展開
  - Phase 5: pending JSON consume + last-run timestamp 書込 + failed marker cleanup
  - Phase 6: 完了サマリー表示
  - best-effort 失敗ポリシー (ADR-031 §): .failed marker で次セッション SessionStart promote

Component B: SessionStart hook の weekly_review_reminder sub-feature
  - src/hooks-session-start/src/main.rs に WeeklyReviewReminderConfig 追加
  - 2 nudge 経路: (1) last-run mtime > threshold_days (default 7)、(2) failed marker 残存
  - 両方該当時は 1 nudge にまとめて出力
  - 関数 50 行制約に対応するため build_*_lines helper を分離 (compute_weekly_review_reminder_nudge を 30 行に圧縮)
  - 6 新規 unit test (disabled / 失敗 marker 列挙 / staleness 発火 / failed 発火 / config parse / dir 不在)
  - .claude/hooks-config.toml に [session_start.weekly_review_reminder] section 追加 (enabled = true で opt-in、ADR-039 experimental pattern)

Component C: aggregate-weekly.md の report_path documentation drift (P-1) 解消
  - PR #182 pre-push simplicity reviewer Anomaly 1 (Medium / non-blocking) を Phase C 着手時に対応
  - findings JSON の report_path が指す .claude/weekly-reviews/<date>.md は Phase C skill が
    copy する canonical location である旨を facet instruction で明示
  - skill 未実装 (= Phase B のみ稼働) 時は dead pointer になるが Phase C land 後に realize される
    forward-pointing 記述として位置付けを明文化

ユーザー判断 (2026-05-29):
  - Q1 MVP scope: A (skill + hook + facet fix 3 component を同 PR で land)
  - Q2 failed marker recovery: A (含める、ADR-031 § 失敗ポリシー 完全実装)
  - Q3 Bundle CR-RL 先行: A (Phase C 先行、Bundle CR-RL = 順位 167-169 は別 PR)

Phase D (e2e 検証) / Phase E (試験運用 dogfood) は別 PR で carry-forward、docs/handoff-weekly-review-phase-c-onwards.md § 4 で trackable。
Phase B (PR #182) land により handoff-rank-8-weekly-review-phase-b.md の retirement
条件 (本 doc § 11) を満たしたため、Phase C 以降向けの carry-forward doc を
新規作成し、本 doc を物理削除する。

新ファイル: docs/handoff-weekly-review-phase-c-onwards.md
- Phase B 完了状況の summary table (carry-forward 用)
- 7 観点責務 mapping (Phase C/D/E でも適用)
- Phase C/D/E 工程計画 (旧 doc § 4 から carry-forward)
- 重要な設計判断 (旧 doc § 5 + 本セッションでの実証 update)
- Phase C 着手時の memory rule (旧 doc § 7 carry-forward)
- Phase C 着手前提として Bundle CR-RL (順位 167-169) land 推奨

旧ファイル削除: docs/handoff-rank-8-weekly-review-phase-b.md
- 永続価値 (7 観点 mapping、ユーザー判断記録、参照リソース) は新 doc に移管
- 残タスク (Phase C/D/E) は todo-summary.md 順位 8 で trackable + 新 doc § 4
- 永続参照リンクは grep -rn 'handoff-rank-8' で本 doc 自身のみ確認
- docs-governance.md § Retirement Workflow 4 step に整合
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2bb15a09-139a-4a4a-b678-ce8949e007b9

📥 Commits

Reviewing files that changed from the base of the PR and between 30f19f5 and a4f432a.

📒 Files selected for processing (2)
  • .claude/hooks-config.toml
  • docs/handoff-weekly-review-phase-c-onwards.md
✅ Files skipped from review due to trivial changes (1)
  • docs/handoff-weekly-review-phase-c-onwards.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • .claude/hooks-config.toml

📝 Walkthrough

Walkthrough

SessionStart hook に ADR-031 Phase C の weekly-review reminder 制御を実装。設定で有効化・閾値・failed-marker 検出を指定し、last-run ファイルの経過日数と .md.failed の有無に基づいて staleness/failed-marker nudge を生成・統合して session 出力に注入。Phase C 以降のハンドオフ資料と follow-up TODO を追加。

Changes

ADR-031 Phase C weekly-review reminder 起動制御

Layer / File(s) Summary
Weekly-review reminder 設定と定数定義
.claude/hooks-config.toml, src/hooks-session-start/src/main.rs
[session_start.weekly_review_reminder] 設定テーブルを追加し、enabled/reminder_threshold_days/failed_marker_check_enabled を定義。Rust 側で対応する config フィールドと参照パス・デフォルト閾値を定数化。
Weekly-review reminder nudge 生成ロジック
src/hooks-session-start/src/main.rs
.claude/weekly-review-last-run.json の mtime を参照して staleness 判定し、.claude/weekly-reviews/*.md.failed を列挙して failed-marker 判定を行い、文言を組み立てて統合する compute_weekly_review_reminder_nudge 等を追加。emit_session_start_output に注入する処理を追加。
Weekly-review reminder テスト体系
src/hooks-session-start/src/main.rs
設定無効時・failed マーカー列挙・last-run 未存在・staleness 判定・hooks-config パースを検証するユニットテストを追加。
Phase C 以降実装ハンドオフ計画
docs/handoff-weekly-review-phase-c-onwards.md, .takt/facets/instructions/aggregate-weekly.md
Phase C(skill /weekly-review 起動)〜 Phase E(dogfood)までの移管計画、設計判断、参照リソース、retirement 条件を追加。aggregate-weekly.md に report_path の forward-pointing 方針を明記。
Follow-up task エントリと TODO 更新
docs/todo-summary.md, docs/todo9.md
推奨実行順序サマリーに PR #183/#182 由来の task 170〜173 を追加し、todo9 に git-workflow 拡張・docs-governance 区別・ephemeral memory・combine_output 抽出等の TODO を記録。

Sequence Diagram(s)

sequenceDiagram
  participant SessionStart
  participant HooksConfig
  participant FS as FileSystem
  participant EmitOutput

  SessionStart->>HooksConfig: hooks-config 読込 (weekly_review_reminder)
  alt enabled = true
    SessionStart->>FS: .claude/weekly-review-last-run.json mtime 取得
    FS-->>SessionStart: mtime/存在情報
    SessionStart->>FS: .claude/weekly-reviews/*.md.failed 列挙
    FS-->>SessionStart: failed マーカー一覧
    SessionStart->>EmitOutput: nudge 文言を生成・統合
  end
  SessionStart-->>EmitOutput: additionalContext に注入
Loading

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PRタイトルは週次レビューPhase C実装(ADR-031)という主要な変更を正確に示しており、/weekly-reviewスキル、SessionStartフック、ドキュメント修正を包括的に要約している。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/hooks-session-start/src/main.rs`:
- Around line 505-513: The current weekly_review_days_since_last_run(repo_root:
&Path) returns Option<u64> and conflates "file missing" and "read error" (IO
failure) into None; change it to return Result<Option<u64>, std::io::Error>
(Ok(None) = missing file, Ok(Some(days)) = success, Err(e) = read/permission
error) by propagating IO errors from std::fs::metadata and
metadata.modified().elapsed() with ? and mapping the missing-file case to
Ok(None); then update callers that currently use unwrap_or(true) / treat None as
staleness to instead treat Err(_) as “fail-open” (do not trigger nudge) and only
trigger staleness when Ok(Some(days)) exceeds threshold. Ensure references to
weekly_review_days_since_last_run and caller logic are updated accordingly.
🪄 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: 3f75f905-331e-4302-b734-aa17fd3208ac

📥 Commits

Reviewing files that changed from the base of the PR and between 193b876 and fed3e44.

📒 Files selected for processing (7)
  • .claude/hooks-config.toml
  • .takt/facets/instructions/aggregate-weekly.md
  • docs/handoff-rank-8-weekly-review-phase-b.md
  • docs/handoff-weekly-review-phase-c-onwards.md
  • docs/todo-summary.md
  • docs/todo9.md
  • src/hooks-session-start/src/main.rs
💤 Files with no reviewable changes (1)
  • docs/handoff-rank-8-weekly-review-phase-b.md

Comment thread src/hooks-session-start/src/main.rs Outdated
…修正 (Missing vs Unreadable 分離) for #184

CodeRabbit Major (auto-fix mandate per memory feedback_review_severity_auto_fix):
src/hooks-session-start/src/main.rs:513 weekly_review_days_since_last_run の
失敗時 Option<u64>::None 経由で fail-open 方針と逆の挙動 (mtime 取得失敗が
staleness ヒット扱い、誤 nudge を毎回生成)。

修正:
- enum WeeklyLastRunState { Missing, ElapsedDays(u64), Unreadable } 導入で
  3 状態を明示分離
- Missing (= 未実行 / 初回) → reminder 発火 (初回利用ナビ)
- ElapsedDays(d) → d >= threshold で発火
- Unreadable (= 権限エラー等) → reminder 抑制 (fail-open、ユーザーを誤通知で煩わせない)

test 追加 (memory feedback_test_dry_antipattern 適用、各 variant 独立):
- weekly_review_staleness_hits_for_missing_state
- weekly_review_staleness_hits_for_elapsed_above_threshold
- weekly_review_staleness_skips_for_elapsed_below_threshold
- weekly_review_staleness_skips_for_unreadable_state (CR finding の fail-open 回帰防止)

検証: cargo test -p hooks-session-start で 58 tests pass (4 new + 54 existing)、
cargo clippy -p hooks-session-start --release -- -D warnings clean。
@aloekun

aloekun commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

fix commit 30f19f51 で M-1 Major (weekly_review_last_run_state enum で fail-open 分離) を修正し、4 unit test を追加しました。ユーザーがマージ前の最終確認として PR 全体の再 review を要望しています。再 review をお願いします。

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown

@aloekun 30f19f51 での M-1 修正(weekly_review_last_run_state enum による fail-open 分離)と 4 unit test の追加を確認しました。PR 全体の再 review を開始します。

(ू•‧̫•ू⑅)

✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 @.claude/hooks-config.toml:
- Line 36: The weekly_review_reminder experimental feature is currently enabled
by default; change its enabled flag to false to follow the opt-in
experimental-feature pattern (ADR-039). Locate the weekly_review_reminder block
in .claude/hooks-config.toml and set enabled = false, and ensure the block
includes the expected opt-in/killswitch/bounded-lifetime metadata (e.g., fields
named enabled, kill_switch, expiry or similar) so the feature remains off by
default and can be explicitly turned on when opted in.

In `@docs/handoff-weekly-review-phase-c-onwards.md`:
- Around line 55-56: SessionStart hook のリマインダ条件は「前回実行から 7 日経過」のみ記載されていますが、実装契約上は
.claude/weekly-reviews/*.md.failed を検出した場合も nudge(promote)を出す仕様になっていますので、docs
の該当箇所(記載中の "SessionStart hook で前回実行から 7 日経過なら promote" 文)に「または
.claude/weekly-reviews/*.md.failed 検出時にも promote(nudge)を行う」旨を追記し、参照先 ADR-031 §
採否フロー(pending 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: edb0543f-b865-4bd3-9bae-721bd69e9cdb

📥 Commits

Reviewing files that changed from the base of the PR and between 193b876 and 30f19f5.

📒 Files selected for processing (7)
  • .claude/hooks-config.toml
  • .takt/facets/instructions/aggregate-weekly.md
  • docs/handoff-rank-8-weekly-review-phase-b.md
  • docs/handoff-weekly-review-phase-c-onwards.md
  • docs/todo-summary.md
  • docs/todo9.md
  • src/hooks-session-start/src/main.rs
💤 Files with no reviewable changes (1)
  • docs/handoff-rank-8-weekly-review-phase-b.md

Comment thread .claude/hooks-config.toml Outdated
Comment thread docs/handoff-weekly-review-phase-c-onwards.md Outdated
CodeRabbit re-review (commit 30f19f5 = 前 fix commit) で post された 2 件を修正:

M-2 (Major, ADR-039 違反): weekly_review_reminder の default を OFF に変更
  - .claude/hooks-config.toml line 36: enabled = true → enabled = false
  - 理由: ADR-039 experimental feature standard pattern (config opt-in 契約) に整合
  - opt-in = repo config で明示 enable する運用、source の default は OFF
  - memory feedback_review_severity_auto_fix 適用 (Major 無条件自動修正)

N-1 (Minor, docs 正確性): handoff doc に failed marker 経路を追記
  - docs/handoff-weekly-review-phase-c-onwards.md line 55-56
  - SessionStart reminder の 2 経路 (7 日経過 + failed marker 検出) を明示記述
  - 実装契約と docs の整合を確保、Phase C 以降の仕様理解ずれを構造的に防止

検証: cargo test -p hooks-session-start 全 58 tests pass (binary 変更なし、config のみ)、
markdownlint clean。
@aloekun

aloekun commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@aloekun
aloekun merged commit 3f00f87 into master May 29, 2026
1 check passed
@aloekun
aloekun deleted the feat-weekly-review-phase-c branch May 29, 2026 11:13
aloekun added a commit that referenced this pull request May 29, 2026
…9) + 174 (#185)

* docs(todo): PR #184 post-merge-feedback 採用 1 件 (順位 174、T3-2 = ADR-039 checklist 拡張)

採用: PR #184 post-merge-feedback Tier 3 #2 (2026-05-29 ユーザー承認):
- 順位 174 (T3-#2): ADR-039 experimental feature lifecycle checklist 拡張
  - 新規 experimental feature 追加時の self-review checklist を ADR-039 に追加
  - 4 点整合確認: config schema ↔ feature flag default OFF ↔ docs / config example ↔ test coverage
  - PR #184 M-2 (weekly_review_reminder 'enabled = true' 違反) の self-review 捕捉化が目的
  - 本リポジトリ内 ADR 編集のみ、派生プロジェクトには波及しない (ADR-039 自体は global pattern として参照される構造のため、文書編集は本リポジトリで完結)

不採用 (2026-05-29 ユーザー判断記録):
- T3-1 (Feedback adoption pattern ADR 新設): 不採用。analyzer の Severity/Frequency/Effort rubric は推奨でしかなく user 判断と完全一致しないため、整合性ある判定基準を rule 化するのは構造的に困難。memory feedback_no_unenforced_rules の延長線上にある原則
  → T3-1 と T3-2 の対比 = discretionary judgment (T3-1) vs mechanical 4 点 checklist (T3-2) の境界判別の実例
  → 採用判断自体が「rule 化可能な finding」と「discretionary 依存 finding」を区別する meta-judgment

却下推奨 (analyzer 既判定): T1-1 / T1-2 / T2-2 / T3-3 (NLP 必要 / 過剰一般化 / unenforced rule)
様子見: T2-1 (ADR-039 compliance Rust test、dogfood 2+ 観測後)

メタ評価: 本セッション (#181#182#183#184) で計 17 件の採用候補が抽出され、Phase B 設計の self-reinforcing detection loop が full session 単位で機能した実例。本 commit は最終 PR #184 採用分の完結。

* feat(rate-limit): check-ci-coderabbit の rate-limit detection を CR 新フォーマット対応に拡張 (順位 167 採用、T1-#1)

PR #182 セッションで実体観測した CR rate-limit detection の silent regression を修正:

旧実装 (= silent regression の原因):
- src/check-ci-coderabbit/src/main.rs:251 RATE_LIMIT_MARKER = 'Rate limit exceeded' 単一固定
- is_rate_limit_comment() が現行 CR の新フォーマットを検出できず常に false 返却
- extract_wait_time() が 'Please wait N minutes and M seconds' のみ対応
- 結果: cli-pr-monitor の RateLimitOutcome::Parked 経路 (ADR-018 設計) が完全無効化、
  通常 review_recheck 経路で 5 分単位 polling に fallback (PR #182 で 30+ 分浪費を実観測)

修正:
- RATE_LIMIT_MARKER (const &str) → RATE_LIMIT_MARKERS (const &[&str]) を multi-variant 配列化
  - 旧 marker: 'Rate limit exceeded' (backward compat)
  - 新 marker: 'rate limited by coderabbit.ai' (HTML マーカー、PR #182/#184 で実観測)
- is_rate_limit_comment() を multi-variant check に refactor
- extract_wait_time() に新フォーマット regex を追加
  - 新 format: 'More reviews will be available in N minutes and S seconds'
  - 旧 format も backward compat 維持

注: ADR-039 experimental feature pattern は本タスクには不適用 (rate-limit detection は
ADR-018 で既に決定済の standard feature であり experimental ではない)。

Bundle CR-RL の機械強制層。test 層 (順位 168) と docs 層 (順位 169) は後続 commit。

* test(rate-limit): CR 新フォーマット fixture を 3 variant 追加 (順位 168 採用、T2-#1)

順位 167 と pair の test 層。RATE_LIMIT_MARKERS 配列化 + extract_new_format_wait_time の
backward compat 保証 + silent regression 防止用 regression gate を確立。

追加 fixture (旧 6 fixture は backward compat のため維持):
- 新 format 標準形: HTML マーカー + Review limit reached + N minutes and S seconds
- 新 format 短縮形: HTML マーカー + N minutes only
- 旧 + 新混在: 同一 PR に 2 種類の rate-limit comment が時系列で混在するケース

memory feedback_test_dry_antipattern 適用 (各 fixture 独立 setup、共通 helper 化しない)。

* docs(adr): ADR-034 / ADR-018 に CR rate-limit format evolution + 検出ロジック更新手順を codify (順位 169 採用、T3-#1)

Bundle CR-RL の docs/ADR 層。順位 167 (機械強制層) + 168 (test 層) と同 PR で land。

ADR-034 追加 section: 既知 CR rate-limit format 一覧 + 検出 logic 更新手順
  - 既知 format (発見時期昇順): 旧 'Rate limit exceeded' + 新 'rate limited by coderabbit.ai'
  - format 変更時の更新手順 (6 step: 観測 → grep → marker 配列 append → regex 追加 → fixture 追加 → ADR-034 update)
  - HTML マーカー優先の理由 (heading 文言や本文より stable な可能性)

ADR-018 lines 185-186 更新:
  - 旧 marker 単一前提の description を「順位 167 で multi-variant 対応済、詳細は ADR-034 参照」に書き換え
  - rate-limit 経路の設計根拠は ADR-018 維持、format detection 詳細は ADR-034 に集約

将来 CR が format 変更時に同じ silent regression を回避する永続 reference 層を確立。
aloekun added a commit that referenced this pull request May 29, 2026
…+ todo10.md 新設 (#186)

PR #185 post-merge-feedback 採用 (2026-05-29 ユーザー承認):
- 順位 175 (T1-#1): cargo clippy を stop_quality ステップに追加 (Rust lint structural gap 補填)
- 順位 176 (T2-#4): check-ci-coderabbit format extraction 関数への variant fixture 追加

user 追加要望 (2026-05-29):
- 順位 177: PostToolUse hook — Edit/Write したファイルのサイズ閾値超過検出 + 分割促進

Phase D dogfood 採用 (2026-05-30 ユーザー承認、週次レビュー 2026-05-30 実行):
- 順位 178 (週次レビュー S02): state.rs behavioral invariant test を ADR-041 pattern で追加
- 順位 179 (週次レビュー S03): rate-limit retry decision boundary test を rstest parameterized で追加
- 順位 180 (週次レビュー C01): lib-report-formatter に markdown pipe / newline escape を追加 (prompt injection 防御)
- 順位 181 (Phase D dogfood D-A): aggregate-weekly facet の findings.json 出力を raw JSON にする (facet output bug fix)
- 順位 182 (Phase D dogfood D-B): /weekly-review skill に重複検出 (簡易 grep + 3 択 AskUserQuestion) を Phase 4 で追加

Phase D dogfood 実行サマリー (handoff doc § 4 e2e 検証):
- takt workflow weekly-review 起動 → 22m 52s 完走 → 12 findings 検出 (high 1 / medium 5 / low 6)
- Phase C skill flow を実観測:
  - Phase 0 (pending file + failed marker 検出): clean state
  - Phase 1 (takt 起動): pre-state 書込 → background 実行
  - Phase 2 (成果物 copy): .takt/runs/ → .claude/weekly-reviews/2026-05-30.md + pending JSON 構築
  - Phase 3 (採否選択): user 承認経由で 5 件採用 (S02/S03/C01/D-A/D-B)
  - Phase 4 (todo 展開): 順位 178-182 として docs/todo10.md に追加 (順位 N system pattern 採用、ADR-031 「週次レビュー採用 (date)」section ではなく既存 priority table integration 重視)
  - Phase 5 (state file 更新): pending JSON consume + last-run timestamp 書込
- 観測した skill 改善点 (= 順位 181 / 182 として codify):
  - findings.json が  フェンス wrap で出力 (facet 出力 bug、skill 内手動 strip workaround)
  - WR-2026-05-30-S05 が既存 順位 173 と完全重複 (重複検出 MVP 未実装の実証)

不採用 / 様子見 (analyzer 推奨を user 承認、2026-05-30):
- 週次レビュー S01 / C02 / C03: 様子見 (large refactor / overflow practical mitigated / future-only risk)
- 週次レビュー S04 / S05 / S06 / S07 / S08 / A01: 却下 (YAGNI / 重複 / 既存 comment で intent 明確 / serde idiomatic / docs trivial drift)

todo10.md 状態:
- 17KB → 34KB (5 entries 追加後)、50KB threshold まで余裕
- 既存 todo.md / todo2.md 〜 todo9.md は引き続き有効、相互独立

メタ評価: 本セッション (#181#182#183#184#185) 5 PR chain + Phase D dogfood で計 28 件の todo 採用 (post-merge-feedback 22 件 + user 主導 1 件 + 週次レビュー dogfood 5 件)、Phase B 設計の self-reinforcing detection loop が **full session 単位での dogfood cycle complete** を実証。本 commit は Phase D 完了 + Phase E (試験運用 1-2 週 dogfood + ADR-031 本採用化 + handoff retire) に carry-forward する状態。
aloekun added a commit that referenced this pull request Jul 21, 2026
…#311)

* docs(plan): WP-15 追補 — 監視 fail-open 修正のゼロ再構築方針 (旧 PR #309 全破棄)

旧 PR #309 (fix/monitor-fail-open-signals) は、4 コミット目の初版が本番 config で
一度も実行されない誤修正 + 実エントリポイントを迂回して pass するテストであり、
takt の High REJECT → fix step の自動書き直しを経た合成物となった。続修より
ゼロ再構築が速いとのユーザー判断 (2026-07-20) に基づき、再利用なしの全破棄を
決定。健全に見えるコミットも含めて引き継がない (中途半端な状態の引き継ぎと、
それによる実装の制約を排除するため)。

追補には、新実装が単独セッションで着手できるよう以下を自己完結で記録した:
- 破棄対象と手順 (PR close / branch 削除 / local abandon — 本コミット時点で未実施)
- 要件 R1-R5 (What のみ。実装方式は新実装の裁量、旧コードは参照しない)
- コード実読で検証済みの根本原因チェーン 6 段 (再調査不要)
- 検証要件 (両 OS 全スイート / incident 実データでの checker 単体実測 /
  E2E カバレッジの正直な申告 / 経路同一性を確認してから実測を主張する規律)
- 旧作業の教訓 (本番経路での発火をテストで固定・実エントリポイント・実データ fixture)

本コミットはドキュメント更新のみ。#309 の close・破棄・新実装には着手していない。
lint:docs / lint:md pass。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(check-ci): CR rate-limit 書式の第3世代対応と未知書式 fallback (WP-15 追補 R2)

CodeRabbit が rate-limit comment の待機時間書式を 3 度目に変更 (PR #309、
2026-07-20 実観測) し、`parse_rate_limit` が None を返して rate-limit 検知が
沈黙した。ADR-034 が予告していた再発事案 (PR #182/#184 に次ぐ 2 度目の
書式変更起因 regression)。

- 第 3 世代書式 `**Next review available in:** **57 minutes**` の抽出を追加。
  ラベルと数値の間に markdown 強調と `:` が挟まるため区切りを `[:*\s]*` で
  吸収し、CR が強調記法を変えても壊れないようにした。
- **書式追随を前提にしない fail-closed 化**: marker (`rate limited by
  coderabbit.ai`) が一致したのに待機時間をどの既知書式でも読めない場合、
  従来は None = 「rate-limit ではない」に倒れていた。marker 一致を制限の
  根拠として採用し、待機時間だけを既定 30 分で埋める方式に変更。既定値が
  実 reset より短ければ wakeup 後に再検出されて再 park されるだけで、
  retry は max_retries で有界。
- 既定値適用時は checker が stderr に警告 (cli-pr-monitor がログ転送) し、
  「30 分」を CR の申告値と誤読させない + 書式再変更の検知シグナルを兼ねる。

fixture は PR #309 の実 comment body を出典付きで埋め込み (ADR-049)。この
comment は walkthrough header marker を同一 body に併せ持つため、clean
walkthrough と誤認しない排他も併せて固定した。

ADR-034 の既知 format 一覧に第 3 世代行と fallback 方針を追記し、更新手順の
stale なファイル参照 (main.rs → markers.rs / rate_limit.rs) を修正。

* fix(check-ci): decide() に rate_limit を渡し silent success を排除 (WP-15 追補 R1/R4)

CodeRabbit がレート制限でレビューを開始できないまま、監視が「レビュー済み・
指摘なし」と報告する silent success を、判定ロジック側で塞ぐ。

## 根本原因 (2026-07-20 コード実読 + PR #307/#309 実観測)

CR はレート制限中も commit check を pass にする (外部 SaaS 挙動)。checker は
これを review_state に採用する一方、parse_rate_limit の結果は出力 JSON に
添付されるだけで decide() には渡っていなかった。結果、decide() は
「review_state = success かつ指摘ゼロ」= 完了と読み、stop_monitoring_success
を返していた。monitor 側の terminal 短絡は rate-limit branch より先に発火する
ため、park / 再 trigger 機構は一度も呼ばれない。

症状は monitor 側に出るが、原因は action の算出そのものにある。旧 PR #309 は
症状側 (monitor 2 箇所) への多層パッチで High REJECT を受けたため、本実装では
算出点である decide() に一本化した。

## 変更

- decide() / build_summary() に rate_limit を渡す。
- R1: rate-limit 検出中かつ「レビュー実施の陽性証拠」が無ければ
  continue_monitoring を返し、判断を monitor の rate-limit branch (park /
  再 trigger、既存・有界) に委ねる。has_actionable 分岐より前に置くことで、
  過去サイクル由来の未解決スレッドで action_required に抜ける穴も塞ぐ。
- R4: rate-limit を検出できなかった場合の backstop として、陽性証拠が無い限り
  stop_monitoring_success を出さない。CR が marker 文言自体を変えても silent
  success には戻らず、最悪 max_duration までの監視継続 (timed_out) に倒れる。
- build_summary は rate-limit 中に「CodeRabbit指摘なし」と断定せず
  「レート制限中 (レビュー未実施)」を出す。

## 陽性証拠の定義

review_state (commit status) は制限中でも pass になるため証拠に使わない。
push_time で絞られた「今サイクルの CR 出力そのもの」= walkthrough_clean /
actionable_comments が読めた (Some(0) 含む) / new_comments > 0 のみを採用する。
unresolved_threads は push_time で絞られず過去サイクルの残骸を含み得るので
除外した。

## 残存リスク

陽性証拠を一切残さない clean レビュー経路が CR 側に存在した場合、監視が
max_duration まで走って timed_out 報告になる。silent success より安全側だが
遅くなる。既存 fixture の範囲では walkthrough_clean か actionable_comments の
いずれかが必ず立つことを確認済み。

既存 decide/summary テスト 100 件は無改修で pass (新 gate が確立済み挙動を
乱していないことの確認)。incident 再現テストは PR #309 の実観測値から構成。

* fix(pr-monitor): 判定文が未確定要素を無視して断定しないよう修正 (WP-15 追補 R3)

監視レポートの人間向け判定文が、findings が空というだけで「問題は見つかり
ませんでした」と断定していた。PR #307/#309 では「未解決スレッド2件」を表示
しながら同一レポート内で「問題は見つかりませんでした」と結論する矛盾が実観測
されている。

findings が空であることは「見るべきものが無かった」の十分条件ではない。
レート制限でレビューが走っていない場合も、未解決スレッドが残っている場合も
空になり得る。

- rate-limit 検出中は保留判定文を出す (R1 で checker から rate_limit が
  届くようになったため、monitor 側で判別可能になった)。
- 未解決スレッドが残っている間は「問題なし」「重大な問題なし」のいずれも
  出さず、件数を添えて保留する。重大な指摘がある場合は「修正が必要」を優先。
- 判定順を「未確定 → 重大 → 未解決 → 軽微 → 問題なし」に整理し、未確定要素を
  findings の有無より先に評価する。断定文へ落ちる経路を構造的に塞ぐ形。

compute_verdict は未確定判定 (verdict_for_unsettled_review) と findings 判定
(verdict_for_findings) に分割し、「断定文はどの guard を通過して初めて出せる
のか」を関数境界で表現した (50 行ガイドラインにも整合)。

既存の verdict テスト 13 件は無改修で pass。

* docs(plan): WP-15 完了条件 (1) 達成の記録と追補の実装状況を反映 (R5)

## R5: WP-15 `完了` 条件 (1) の達成記録

PR #307 マージ時の release-binaries.yml run は build job が失敗しており
(master が赤で #308 の E2E 修正が必要だった)、成功したのは #308 マージ後の
run (commit 541adde)。この経緯も含めて記録した。

生成物は本セッションで再実測している: WSL Ubuntu 24.04 から素の curl で
tarball (9,721,643 bytes) + .sha256 を**認証なしで**取得 → sha256sum -c 一致
→ 展開して 16 バイナリ + BUILD_INFO 確認 → release バイナリそのもので hooks
実発火 (pre-tool-validate が破壊的削除コマンドを exit 2 でブロックし無害な
echo を exit 0 で通す / session-start が additionalContext JSON を出力)。

これで WP-15 ④ の「public リポジトリの Release asset は素の HTTPS で取得
できるため gh CLI 認証は不要」という設計判断が実 URL・実 asset で裏づけられた。
旧 PR #309 の docs コミットの主張を引き写すのではなく、自分で再実測した結果を
記録している。

## 追補の実装状況

R1〜R4 の実装内容、破棄の実施結果、検証の実測値を反映。E2E カバレッジは
正直に申告した: 担保できたのは全 gate のユニット検証 / checker 実エントリ
ポイントの実データ実走 / 修正前後の差分実測の 3 点で、cli-pr-monitor 側の
統合経路 (park → PARK signal) と wakeup → 再 trigger 経路は、CR レート制限が
本セッション中に自然発生しなかったため未実測である旨を明記した。

なお本ファイルは master 時点で既に 59,798 bytes と file_size_check の 50KB
閾値を超過しており (non-blocking 警告)、本変更で 73,781 bytes になった。
分割は § 9 の退役手順で本ファイルごと削除する前提のため見送る。

* docs(check-ci): wait_time_parsed の doc が実装と食い違う記述を修正

セルフレビュー (pre-push-review simplicity facet) の指摘。

`wait_time_parsed` の doc は「監視側はこれを見て『実測』と『既定値』を区別
して報告する」と書いていたが、実装では monitor 側への配線を見送っており
(cli-pr-monitor の RateLimitState は本 field を持たない)、記述と実装が
食い違っていた。既定値適用を運用者に伝える経路は実際には checker の stderr
警告 (monitor がログ転送) が担っている。

doc を実態に合わせ、あわせて「なぜ typed 化を見送ったか」(全 struct literal の
更新コストに対し得られるのが park summary の文言精度という副次的利得)と
「値がどこから参照できるか」(monitor が保持する checker の生 JSON) を明記した。

なお本指摘は、PR 全体 (master..@) を対象にセルフレビューを再実行して初めて
検出された。push 時のパイプラインは [diff] stage が tip コミットのみを
レビュアーに渡すため (push-runner-config.toml の `command = "jj diff -r @"`)、
models.rs を含む祖先コミットがレビュー対象外だった。この構造的欠陥は
docs/todo-summary2.md 順位 288 (Tier 1、Severity High で 3 連続再発) として
既知で、本 PR で 4 回目の再発となった。修正は独立 PR で行う。

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant