Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 34 additions & 0 deletions docs/adr/adr-018-pr-monitor-takt-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,37 @@ Bundle b 設計時に再分析して判明した CronCreate の特性:
- PR #114 (Bb-2): review_recheck CronCreate park + observer 撤廃
- PR #115 (Bb-3): config 拡張 + SessionStart catch-up nudge + sanitize() (CR Major #1+#2 fold-in)
- PR #116: Bb-3 post-merge-feedback 採用 3 件 (順位 76/77/78) を todo に登録

## 追記 (2026-05-08): 順位 80 — auto-retry の transient failure scope 明文化

PR #120 (cli-finding-classifier 統合) の dogfood で観測された **「rate-limit auto-retry が `RateLimitOutcome::Posted` 経路で park 予約せず silent exit する」** 事象を契機に、auto-retry の対象 transient failure pattern を本 ADR で明文化。

### 対象 transient failure の分類

| Pattern | Detection 基準 | Auto-retry 状態 | Notes |
|---|---|---|---|
| **Rate limit (待機型)** | `Rate limit exceeded` + 未来 reset_time | ✅ 実装済 (`RateLimitOutcome::Parked`) | reset まで `until_unix_secs` で park、再投稿不要 |
| **Rate limit (即時型)** | `Rate limit exceeded` + 過去 reset_time | ✅ 実装済 (`RateLimitOutcome::Posted` + 順位 80 fix) | `@coderabbitai review` 投稿後、`review_recheck_wait_secs` で park (順位 80 fix で導入、silent exit 防止) |
| **CR 投稿エラー** (`Failed to post review comments`) | walkthrough overlay の error message | ⏳ 未実装 (順位 81、1 観測のみ低頻度) | 頻度が確認できるまで実装を defer (memory: `feedback_no_unenforced_rules` 系の判断) |
| **Wakeup 未予約 fallback** | rate-limit 検出ありで polling 終端時に next_wakeup_at_unix 未設定 | ⏳ 将来候補 | 順位 80 fix で `Posted` 経路は塞がれた、他経路の同型 silent exit が再観測されたら追加 |

### 順位 80 fix の実装ポイント (PR #129 / 2026-05-08)

`finalize_posted_retrigger` (poll.rs) を以下のように変更:

- **Before**: 投稿後 `write_state` のみして `None` を返す → polling 継続 → max_duration timeout で `make_timeout_result` (silent exit)
- **After**: 投稿後 `state.action = "parked_review_recheck"` + `next_wakeup_at_unix = now + review_recheck_wait_secs` を設定し、`format_park_signal` で PARK signal を stdout 出力、`Some(park_poll_result)` を返す → CronCreate wakeup で再開

これにより、rate-limit detection 後の **全経路で必ず PARK signal が emit される** ことが構造的に保証される。

### 順位 81 (CR 投稿エラー) を defer した理由

- **Frequency**: 1 観測 (PR #120 のみ) で systemic 性が未確認
- **Risk**: detection 拡張は false positive リスク (`Failed to post review comments` が本当に transient か、permanent failure かの判別が難しい)
- **Decision**: ユーザー方針 `feedback_no_unenforced_rules` (機械検知不可なら何もしない方がマシ) と整合、**3 PR 観測** の閾値到達まで実装を defer
- **Re-trigger 条件**: 同型の error message が別 PR で 2 件以上観測されたら順位 81 を再活性化

### 関連 PR / commit (本追記)

- PR #120 (Phase 5 land): rate-limit auto-retry の Posted 経路 silent exit 観測の根拠
- 本 PR (順位 80 fix + 順位 82 ADR update + §A-2 P-4 ledger): 本 ADR 追記の land
2 changes: 1 addition & 1 deletion docs/local-llm-offload-analysis.md
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ P-1 (Bundle g-1): PR #125, merged 2026-05-07, findings: 0 (CR APPROVE no comm
P-2 (順位 47): PR #126, merged 2026-05-07, findings: 1 (Nitpick, CR review body 内 `<details>` block), agreement: 1/1 (100%, 私評価=human_review と一致), latency: 6.4s/件 (>5s 目標), fallback: 1/1 (normalized_issue length 100>80)
- 既知 gap: check-ci-coderabbit が review body の `<details>` block 内 Nitpick を抽出しない (post-pr-monitor が classifier に渡せず、手動で synthetic finding 構築して classifier 実行)
P-3 (順位 7): PR #127, merged 2026-05-08, findings: rate-limit blocked (CR が 27 min wait の rate-limit overlay で formal review 投稿不可)、classifier 未起動、計測 N/A — dogfood 不発 (但し CR rate-limit 経路が dogfood の阻害要因として観測された、§A-2 §6 注意事項 #1 を実証)
P-4 (順位 76+77): PR #___, merged ___, findings: __, agreement: __/__, token Δ: __, latency: __s/件, fallback: __/__
P-4 (順位 76+77): PR #128, merged 2026-05-08, findings: 1 (CR Nitpick: cross_module_* 命名 misleading)、手動 synthetic finding で classifier 実行 → action=human_review / action_confidence=**0.9** (P-2 の 0.0 から大幅改善、length contract pass)、latency: 6.6s/件、fallback: **0/1** (P-2 の 1/1 から改善、normalized_issue 50 chars Japanese で contract pass)、agreement: 1/1 (100%、私評価=human_review と一致)
P-5 (Bundle f-1): PR #___, merged ___, findings: __, agreement: __/__, token Δ: __, latency: __s/件, fallback: __/__

集計:
Expand Down
4 changes: 1 addition & 3 deletions docs/todo.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@
| 69 | 💎 Tier 3 | **`no-ephemeral-todo-reference` の `yaml`/`yml` extensions 追加理由をコメントで明記 (PR #110 T3-1 採用) ★ Bundle d** | todo5.md | XS | なし (rule⑥ コメント欄に 1-2 行追記、設計 doc と実装の経緯保存、git blame 不要化) |
| 78 | 💎 Tier 3 | **ADR-038 (Rust timestamp arithmetic safety) + CLAUDE.md security 拡充 (PR #115 T3-1 採用) ★ Bb-3 follow-up** | todo5.md | S | なし (config が user-editable system boundary のとき `sanitize()` 値域検証を必須化し dependent arithmetic に `// SAFETY: <sanitize-fn> により上限保証` コメントを要求するパターンを ADR + CLAUDE.md に codify、Rust 固有の checked_add + MAX_SAFE capping + time-dependent test の 3 層を明文化) |
| 79 | 💎 Tier 3 | **`docs-governance.md` § Retirement Workflow に「残タスクの lifecycle 整合」要件明記 (PR #117 T3-1 採用)** | todo5.md | XS | なし (PR #117 で順位 15 を Bb-3 で吸収済として削除した際、現 Step 2「残タスクを priority table に登録」が priority table から除外するケース = 完了/deprioritize/defer を未定義だった実証。除外時の commit/PR で 3 値のいずれかを明示する要件を追加して将来の同型 ambiguity を構造的に防ぐ) |
| 80 | 🚀 Tier 1 | **cli-pr-monitor: rate-limit auto-retry wakeup 予約ロジックの整理 (PR #120 T1-1 採用) ★ Bundle f** | todo5.md | M | なし (PR #120 dogfood で `auto_retry_enabled=true` でも wakeup park が予約されず exit する事象を観測、ADR-018 設計の実装落ち) |
| 81 | 🚀 Tier 1 | **cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f** | todo5.md | M | 順位 80 と同 PR (Bundle f コア層、failure detection と retry path 共通化、rate-limit と統合) |
| 82 | 💎 Tier 3 | **ADR-018 update: rate-limit 以外の transient failure auto-retry 設計の明文化 (PR #120 T3-2 採用) ★ Bundle f** | todo5.md | S | 順位 80 / 81 と同 PR (実装と仕様の整合確保、Bundle f) |
| 81 | 🚀 Tier 1 | **cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f (defer)** | todo5.md | M | 1 観測のみで systemic 性未確認 (§A-2 P-5 PR で defer 判断、ADR-018 §追記 2026-05-08 で re-trigger 条件 = 2 件以上の同型観測を規定) |
| 83 | 🔧 Tier 2 | **cli-pr-monitor: 複合 AND guard の各条件を独立テストで検証 (PR #120 T2-1 採用)** | todo5.md | S | なし (PR #120 W-001、`enrich_with_classifier_skips_when_disabled` の test setup で複合条件分離不全) |
| 84 | 💎 Tier 3 | **グローバルルール: code-review.md に「early-return guard テスト分離」チェックリスト追記 (PR #120 T3-1 採用)** | todo5.md | XS | なし (順位 83 の知見を global rule に codify、`~/.claude/rules/common/code-review.md`、独立並列実施可) |
| 85 | 🚀 Tier 1 | **cli-pr-monitor: monitor state machine guard 強化 (`review_state: not_found && findings: []` を pending 据置) (PR #121 T1-1 採用) ★ Bundle g** | todo5.md | S | なし (PR #119/#120/#121 で 3 PR 連続観測、Frequency Medium 閾値到達済み、Severity High = 誤 approved リスク、Bundle f #80 と関連だが verdict logic 側) |
Expand Down
55 changes: 8 additions & 47 deletions docs/todo5.md
Original file line number Diff line number Diff line change
Expand Up @@ -745,66 +745,27 @@

---

### cli-pr-monitor: rate-limit auto-retry wakeup 予約ロジックの整理 (PR #120 T1-1 採用) ★ Bundle f

> **動機**: PR #120 dogfood で「`rate_limit_config.auto_retry_enabled = true / max_retries = 3` でも rate-limit 検出後に wakeup park が予約されず exit する」事象を観測。手動 `@coderabbitai review` + `CronCreate` 投入でリカバリ。ADR-018 で設計された auto-retry に実装落ちが疑われる。
>
> **参照**: PR #120 dogfood 観測、`.claude/feedback-reports/120.md` Tier 1 #1
>
> **実行優先度**: 🚀 **Tier 1** — Effort M。`cli-pr-monitor/src/stages/poll.rs::handle_rate_limit_branch` 周辺と state.rate_limit_retries 制御の整合性確認。

#### 作業計画

- [ ] `auto_retry_enabled = true` 経路で rate-limit 検出時に確実に `parked_review_recheck` (or 専用 reason) で park signal を emit する
- [ ] `rate_limit_retries` がインクリメントされない / 上限到達後の挙動を test infra で再現
- [ ] 順位 81 / 82 と同 PR で land (Bundle f コア実装)

#### 完了基準

- rate-limit 検出時に常に PARK signal が emit される (silent exit が消える)
- regression test (auto_retry_enabled=true で rate-limit 注入 → park 予約) が green

---

### cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f
### cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f (defer)

> **動機**: PR #120 dogfood で CR walkthrough overlay が `Failed to post review comments` (rate-limit ではない transient failure) を表示するも `parse_rate_limit_status` が detected せず、auto-retry が発火しなかった。1 観測だが auto-retry の silent failure として機能不全。
>
> **参照**: PR #120 walkthrough comment (16:41Z 投稿)、`.claude/feedback-reports/120.md` Tier 1 #2
> **参照**: PR #120 walkthrough comment (16:41Z 投稿)、`.claude/feedback-reports/120.md` Tier 1 #2、[ADR-018 §追記 2026-05-08](adr/adr-018-pr-monitor-takt-migration.md)
>
> **実行優先度**: 🚀 **Tier 1 (defer)** — §A-2 P-5 PR (2026-05-08) で Defer 判定。1 観測のみで systemic 性未確認のため、ユーザー方針 `feedback_no_unenforced_rules` (機械検知不可なら何もしない方がマシ) と整合させて 3 PR 観測閾値到達まで待つ。
>
> **実行優先度**: 🚀 **Tier 1** — Effort M。`check-ci-coderabbit/src/main.rs` の failure detection 拡張 + cli-pr-monitor の retry path 共通化
> **Re-trigger 条件**: `Failed to post review comments` (またはそれに類する rate-limit 以外の CR transient failure) が他の PR で 1 件以上追加観測 (合計 2 件以上) されたら本タスクを再活性化、実装に着手

#### 作業計画
#### 作業計画 (defer 中、参考)

- [ ] `Review failed` / `Failed to post review comments` 等の transient failure pattern を detection に追加
- [ ] rate-limit 系と統合する場合は state field を `transient_failure: Option<TransientFailureKind>` に一般化検討
- [ ] 順位 80 と同 PR で land (Bundle f)
- [ ] ADR-018 §追記 2026-05-08 の「対象 transient failure 分類」表を「⏳ 未実装」→「✅ 実装済」に更新

#### 完了基準

- `Failed to post review comments` を含む walkthrough overlay 検出時に auto-retry が発火する
- regression test (failure pattern 注入 → auto-retry 発火) が green

---

### ADR-018 update: rate-limit 以外の transient failure auto-retry 設計の明文化 (PR #120 T3-2 採用) ★ Bundle f

> **動機**: 順位 80 / 81 で実装する設計方針 (rate-limit 以外の transient failure も auto-retry 範囲) を ADR-018 に明記しないと、将来の同型実装落ちが構造的に再発するリスク。
>
> **参照**: PR #120 T1-1 / T1-2 観測、`.claude/feedback-reports/120.md` Tier 3 #2、[ADR-018](adr/adr-018-pr-monitor-takt-migration.md)
>
> **実行優先度**: 💎 **Tier 3** — Effort S。順位 80 / 81 と同 PR で同時更新 (実装と仕様の整合確保)。

#### 作業計画

- [ ] ADR-018 に「auto-retry の対象 transient failure pattern」セクション追加
- [ ] rate-limit + 投稿エラー + (将来候補) wakeup 未予約を列挙
- [ ] 順位 80 / 81 と同 PR で land

#### 完了基準

- ADR-018 に対象 failure pattern が enumerable 形式で列挙される
- 順位 80 / 81 の実装が ADR-018 から逆引き可能
- ADR-018 §追記 2026-05-08 と整合

---

Expand Down
131 changes: 125 additions & 6 deletions src/cli-pr-monitor/src/stages/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,13 @@ fn run_one_iteration(ctx: &PollContext<'_>) -> Option<PollResult> {
return Some(make_terminal_result(state, result));
}

if let Some(terminal) =
handle_rate_limit_branch(&mut state, ctx.rate_limit_config, ctx.pr_info, &result)
{
if let Some(terminal) = handle_rate_limit_branch(
&mut state,
ctx.rate_limit_config,
ctx.pr_info,
ctx.review_recheck_wait_secs,
&result,
) {
return Some(terminal);
}

Expand Down Expand Up @@ -293,6 +297,7 @@ fn handle_rate_limit_branch(
state: &mut PrMonitorState,
rate_limit_config: &RateLimitConfig,
pr_info: &PrInfo,
review_recheck_wait_secs: u64,
result: &serde_json::Value,
) -> Option<PollResult> {
let rl = state.rate_limit.clone()?;
Expand All @@ -319,18 +324,33 @@ fn handle_rate_limit_branch(
return None;
}

dispatch_rate_limit_outcome(state, &rl, pr_info, rate_limit_config.max_retries, result)
dispatch_rate_limit_outcome(
state,
&rl,
pr_info,
rate_limit_config.max_retries,
review_recheck_wait_secs,
result,
)
}

fn dispatch_rate_limit_outcome(
state: &mut PrMonitorState,
rl: &crate::state::RateLimitState,
pr_info: &PrInfo,
max_retries: u32,
review_recheck_wait_secs: u64,
result: &serde_json::Value,
) -> Option<PollResult> {
match handle_rate_limit_retry(rl, state, pr_info, max_retries) {
RateLimitOutcome::Posted => finalize_posted_retrigger(state, rl, result),
RateLimitOutcome::Posted => finalize_posted_retrigger(
state,
rl,
pr_info,
review_recheck_wait_secs,
max_retries,
result,
),
RateLimitOutcome::Parked { wakeup_at_unix } => Some(finalize_parked(
state,
rl,
Expand All @@ -356,9 +376,28 @@ fn dispatch_rate_limit_outcome(
fn finalize_posted_retrigger(
state: &mut PrMonitorState,
rl: &crate::state::RateLimitState,
pr_info: &PrInfo,
review_recheck_wait_secs: u64,
max_retries: u32,
result: &serde_json::Value,
) -> Option<PollResult> {
state.rate_limit_last_retriggered_at = Some(rl.comment_event_time.clone());

let now_unix = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_secs() as i64)
.unwrap_or(0);
let park_at_unix = now_unix + review_recheck_wait_secs as i64;

state.action = "parked_review_recheck".into();
state.next_wakeup_at_unix = Some(park_at_unix);
state.wakeup_reason = Some("rate_limit_post_retrigger".into());
state.head_commit = pr_info.head_commit.clone();
state.summary = format!(
"rate-limit retrigger 後の review 完了待ちを {}s 後に予約 (順位 80 fix: silent exit 防止)",
review_recheck_wait_secs
);

if let Err(e) = write_state(state) {
log_info(&format!(
"[rate_limit] retrigger 後の state 永続化失敗、自動 retry を停止: {}",
Expand All @@ -373,7 +412,11 @@ fn finalize_posted_retrigger(
),
));
}
None

let signal = format_park_signal(state, rl, pr_info, max_retries);
println!("{}", signal);

Some(make_park_poll_result(state.clone()))
}

fn finalize_parked(
Expand Down Expand Up @@ -1429,4 +1472,80 @@ mod tests {
"sibling parity (review_recheck ↔ initial_review)"
);
}

fn setup_posted_retrigger_fixture() -> (PrMonitorState, RateLimitState, crate::util::PrInfo) {
let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into());
state.action = "continue_monitoring".into();
state.rate_limit_retries = 1;
let rl = RateLimitState {
until_unix_secs: 0,
comment_event_time: "2026-05-08T00:00:00Z".into(),
wait_minutes: 5,
wait_seconds: 0,
};
let pr_info = crate::util::PrInfo {
pr_number: Some(1),
repo: Some("o/r".into()),
push_time: Some("2026-05-01T00:00:00Z".into()),
head_commit: Some("abc1234".into()),
};
(state, rl, pr_info)
}

#[test]
fn finalize_posted_retrigger_schedules_park_after_post() {
let _guard = env_override_lock();
let tmp = tempfile::tempdir().unwrap();
let state_path = tmp.path().join("state.json");
std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &state_path);

let (mut state, rl, pr_info) = setup_posted_retrigger_fixture();
let result = finalize_posted_retrigger(&mut state, &rl, &pr_info, 300, 3, &serde_json::Value::Null);

std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE");

let park_result = result.expect("順位 80 fix: Posted 後は必ず park を返し silent exit を防ぐ");
assert_eq!(park_result.action, "parked_review_recheck");
assert_eq!(state.wakeup_reason.as_deref(), Some("rate_limit_post_retrigger"));
let now_unix = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_secs() as i64;
let wakeup = state.next_wakeup_at_unix.expect("next_wakeup_at_unix が設定される");
assert!(wakeup > now_unix && wakeup <= now_unix + 301);
assert_eq!(state.rate_limit_last_retriggered_at.as_deref(), Some("2026-05-08T00:00:00Z"));
}

#[test]
fn finalize_posted_retrigger_action_required_when_write_state_fails() {
let _guard = env_override_lock();
let bad_path = unwritable_state_path();
std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &bad_path);

let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into());
state.action = "continue_monitoring".into();
let rl = RateLimitState {
until_unix_secs: 0,
comment_event_time: "2026-05-08T00:00:00Z".into(),
wait_minutes: 5,
wait_seconds: 0,
};
let pr_info = crate::util::PrInfo {
pr_number: Some(1),
repo: Some("o/r".into()),
push_time: Some("2026-05-01T00:00:00Z".into()),
head_commit: None,
};

let result = finalize_posted_retrigger(&mut state, &rl, &pr_info, 300, 3, &serde_json::Value::Null);

std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE");

assert!(result.is_some());
assert_eq!(
result.unwrap().action,
"action_required",
"write_state 失敗時は action_required で抜ける (sibling parity with finalize_parked)"
);
}
}