fix(hooks-session-start): weekly-review staleness を last_run_at ベース化 - #233
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough週次レビューの staleness 判定を Changeslast_run_at ベースの staleness 判定
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/weekly_review.rs`:
- Around line 55-65: The mtime fallback in weekly_review_last_run_state still
depends on wall-clock time instead of the injected now_unix, so update the
fallback path to pass now_unix into last_run_state_from_mtime and compute the
age from a modified timestamp converted to unix seconds rather than using
mtime.elapsed(). Keep the behavior consistent with
compute_weekly_review_reminder_nudge and the existing
last_run_state_from_content / last_run_state_from_mtime flow so the
WeeklyLastRunState result is deterministic when last_run_at is unavailable.
🪄 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: c517a5ff-7c06-412b-a07f-f9d146acc649
📒 Files selected for processing (9)
.claude/weekly-review-deferred.json.claude/weekly-review-last-run.json.claude/weekly-reviews/2026-05-30.md.claude/weekly-reviews/2026-06-01.md.gitignoredocs/adr/adr-031-weekly-review-pipeline.mddocs/todo.mdsrc/hooks-session-start/src/main.rssrc/hooks-session-start/src/weekly_review.rs
💤 Files with no reviewable changes (4)
- .claude/weekly-reviews/2026-06-01.md
- .claude/weekly-reviews/2026-05-30.md
- .claude/weekly-review-deferred.json
- .claude/weekly-review-last-run.json
aloekun
force-pushed
the
fix-weekly-review-staleness
branch
from
July 1, 2026 16:28
32b377f to
214f990
Compare
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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 `@docs/adr/adr-031-weekly-review-pipeline.md`:
- Around line 128-136: The SessionStart hook flow currently falls back to mtime
when .claude/weekly-review-last-run.json is missing last_run_at, which
reintroduces the legacy silent-fresh path. Update the behavior described around
hooks-session-start and the weekly-review-last-run state handling so missing or
unmigrated last_run_at is treated as stale instead of using mtime, and ensure
write/update paths always populate last_run_at to backfill old state files.
🪄 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: a11ecbc1-4584-4a27-b829-30a84cabc0dd
📒 Files selected for processing (9)
.claude/weekly-review-deferred.json.claude/weekly-review-last-run.json.claude/weekly-reviews/2026-05-30.md.claude/weekly-reviews/2026-06-01.md.gitignoredocs/adr/adr-031-weekly-review-pipeline.mddocs/todo.mdsrc/hooks-session-start/src/main.rssrc/hooks-session-start/src/weekly_review.rs
💤 Files with no reviewable changes (4)
- .claude/weekly-review-last-run.json
- .claude/weekly-reviews/2026-06-01.md
- .claude/weekly-reviews/2026-05-30.md
- .claude/weekly-review-deferred.json
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- docs/todo.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks-session-start/src/main.rs
- src/hooks-session-start/src/weekly_review.rs
aloekun
force-pushed
the
fix-weekly-review-staleness
branch
from
July 2, 2026 05:43
214f990 to
b140daf
Compare
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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/weekly_review.rs`:
- Line 111: The `WeeklyLastRunState::Stale` label in `weekly_review.rs` is too
specific and should cover all stale cases, not just missing `last_run_at`.
Update the string in the `match` for `WeeklyLastRunState` so it describes
stale/invalid state generally, and make sure any parse-failure or
future-timestamp paths that map to `Stale` are aligned with this broader label.
🪄 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: 5f2b29af-470c-42be-9714-9c701fae813c
📒 Files selected for processing (9)
.claude/weekly-review-deferred.json.claude/weekly-review-last-run.json.claude/weekly-reviews/2026-05-30.md.claude/weekly-reviews/2026-06-01.md.gitignoredocs/adr/adr-031-weekly-review-pipeline.mddocs/todo.mdsrc/hooks-session-start/src/main.rssrc/hooks-session-start/src/weekly_review.rs
💤 Files with no reviewable changes (4)
- .claude/weekly-reviews/2026-05-30.md
- .claude/weekly-review-last-run.json
- .claude/weekly-reviews/2026-06-01.md
- .claude/weekly-review-deferred.json
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- docs/todo.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks-session-start/src/main.rs
jj workspace の mtime リセットで reminder が発火しない silent-fresh バグを解消。 staleness 判定を `.claude/weekly-review-last-run.json` の mtime ではなく JSON 内 `last_run_at` から計算するよう変更 (欠落時のみ mtime フォールバック)。既存の reaper::parse_iso8601_to_unix + past_time::PastTime (未来値を型で reject) を流用。 背景: 状態ファイルが jj tracked かつ mtime 判定だったため、workspace 作成 (ADR-045) のたびに mtime がリセットされ、last_run_at=2026-06-01 (1か月前) でも約1日に見えて 7日閾値に届かず、週次レビューが約1か月起動しなかった。 同梱: - ADR-031 本文の mtime 記述を last_run_at ベースに修正 + 設計ノート追記 - weekly-review 状態ファイルを .gitignore + jj untrack (mtime リセット経路を根絶) - 2026-07-01 週次レビュー実行の採用反映を docs/todo.md に追記 (A01=Stop hook/push gate の lint 重複解消、A02=ADR-032 dead pointer 再検出注記) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aloekun
force-pushed
the
fix-weekly-review-staleness
branch
from
July 2, 2026 09:25
b140daf to
cdcc1c7
Compare
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
6 tasks
aloekun
added a commit
that referenced
this pull request
Jul 19, 2026
…300) * feat(lib-jj-helpers): resolve_main_workspace_root を追加 (ADR-045 状態分裂対策 / PR-N2) secondary jj workspace から canonical な (メイン) workspace root を解決する fs ヘルパーを追加。 gitignore 済み untracked 状態ファイル (weekly-review-last-run.json 等) は per-checkout で materialize され secondary workspace には存在しない問題への基盤 (ADR-045)。 - .jj/repo がディレクトリ → colocated main → 自身を返す - .jj/repo がファイル → 内容の main store パス (相対は .jj/ 基準) の 2 階層上がメイン root - .jj 不在 / 読取失敗 / 導出パス不存在 → None (caller は現 root に fail-open) resolve_git_dir と同じ layout 解釈 (相対基準・verbatim prefix 剥がし) を共有。テストは fixture + 実 jj E2E (ignored) を resolve_git_dir パターンで流用。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(session-start): weekly last-run 読込をメイン workspace root に canonical 化 (ADR-045 / PR-N2) gitignore 済み untracked の weekly-review-last-run.json は workspace ローカルで secondary workspace に存在せず、メイン側では常に「未実行」判定で reminder が永久発火していた (2026-07-19 実観測、ADR-045 状態分裂)。 - hooks-session-start に lib-jj-helpers 依存を追加 - compute_weekly_review_reminder_nudge の last-run 読込を resolve_main_workspace_root(cwd).unwrap_or(cwd) 基準に変更 (導出不能は現 root に fail-open) - failed marker / pending JSON はレビュー成果物として workspace ローカルのまま維持 (線引きを doc 明記) - doc comment の「last_run_at は workspace 不変」誤記を訂正 (値は不変だがファイル所在は workspace 依存) secondary レイアウト (.jj/repo ファイル) でメイン root の last-run を読みつつ failed marker は 現 workspace から読むことを検証する unit test を追加。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(adr): 状態ファイルの workspace 分裂を ADR-031/045 に追記 (PR-N2) weekly-review last-run が secondary workspace 側にのみ存在しメイン側で永久「未実行」発火した silent bug (2026-07-19 実観測) を ADR に定着。 - ADR-031 § トリガー方式と reminder: last-run のメイン workspace canonical 化の決定を追記。 「last_run_at は workspace 不変」誤記の是正 (値は checkout 不変だがファイル所在は workspace 依存)、 resolve_main_workspace_root による読込 canonical 化、failed marker/pending JSON は per-workspace 維持。 - ADR-045: gitignore 済み untracked 状態ファイルの workspace 分裂を silent bug class として新設。 mtime リセット (CR #233) と対になる実例として対比表を追加し、per-workspace/global の判定基準を明文化。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: weekly-review-notification-plan に PR-N2 作業記録を反映 4 分割コミット粒度・skills repo は内容編集のみ (commit/deploy は skills 側に委任)・ 検証結果 (cargo test --workspace green / hooks-session-start 93 passed / lib-jj-helpers 37+E2E 2 / clippy clean / build:all 成功 / デプロイ exe を secondary レイアウトで駆動しメイン root の last-run を読んで「18 日経過」を end-to-end 確認、対照の未実行 fail-open も確認)・ 残タスク (運用コピー・削除条件 3 = 新セッション目視・skills deploy) を追記。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
概要
週次プロジェクト全体レビュー (ADR-031) が 約 1 か月間起動していなかった問題を修正します。SessionStart hook の staleness 判定が状態ファイルの mtime を見ていたため、jj workspace (ADR-045) の checkout / materialization のたびに mtime がリセットされ、「実際は 1 か月前の実行なのに fresh」に見えて reminder が永久に発火しない silent-fresh バグでした。
根本原因
hooks-session-startの staleness 判定が.claude/weekly-review-last-run.jsonの mtime を経過日数の情報源にしていた。last_run_at=2026-06-01(1 か月前) でも mtime 上は約 1 日に見え、7 日閾値に届かず reminder が抑制 → 唯一のトリガーが機能せず週次レビューが未実行のままだった。変更内容
src/hooks-session-start/src/weekly_review.rs): staleness の第一情報源を mtime → JSON 内last_run_atに変更 (欠落時のみ mtime フォールバック)。既存のreaper::parse_iso8601_to_unix+past_time::PastTime(未来値を型で reject = silent-fresh 防止) を流用。main.rsはnow_unixを渡すよう更新。回帰テスト含め 6 テスト追加。.gitignore): weekly-review のローカル状態 (weekly-review-last-run.json/-deferred.json/-pending.json/weekly-reviews/) を.gitignoreに追加し jj から untrack。ADR-031 の「ローカル専用」設計に整合させ、mtime リセット経路そのものを根絶。docs/adr/adr-031-weekly-review-pipeline.md): mtime 記述 3 箇所をlast_run_atベースに修正 + 設計ノート追記。docs/todo.md): 修正後に実際の weekly-review を実行し、採用 finding を反映。[stop_quality]と push-runner[quality_gate]の lint/test 重複解消 (PR-W5 との整合注記付き)。テスト
cargo test -p hooks-session-start: 77 passedcompute_weekly_review_reminder_nudge_uses_last_run_at_over_fresh_mtimeで「fresh な mtime でも古い last_run_at なら発火」を保証補足
docs/file-length-enforcement-plan.mdを進めるメインセッションとは ファイル競合なし (本 PR はhooks-session-startcrate +docs/todo.mdのみ、計画書側は cli-* crate +todo-summary/13/2)。🤖 Generated with Claude Code
Summary by CodeRabbit