Compare commits

...

3 Commits

5 changed files with 36 additions and 207 deletions

View File

@ -251,15 +251,22 @@ struct TaskUpdateTool {
store: TaskStore,
}
const CREATE_DESCRIPTION: &str = "Create a session-lifetime task. Input only `subject` and \
const CREATE_DESCRIPTION: &str = "Create a session-lifetime task for short-term current-work \
tracking, not project management. Tasks are user-visible real-time status. Use this whenever you \
set a goal and work through steps, including implementation. Input only `subject` and \
`description`; `taskid` is assigned automatically and initial `status` is `pending`.";
const LIST_DESCRIPTION: &str = "List every session-lifetime task, including completed and \
deleted entries. Takes an empty object as input.";
const GET_DESCRIPTION: &str = "Get one session-lifetime task by `taskid`. Returns an error if \
the task does not exist.";
const UPDATE_DESCRIPTION: &str = "Update an existing session-lifetime task. Provide `taskid` and \
at least one of `status`, `subject`, or `description`. `status` must be one of `pending`, \
`inprogress`, `completed`, or `deleted`; deletion is logical (`status = deleted`).";
deleted entries. Tasks are user-visible real-time status for short-term current-work tracking. \
Takes an empty object as input.";
const GET_DESCRIPTION: &str = "Get one session-lifetime task by `taskid`. Tasks are \
user-visible real-time status for short-term current-work tracking. Returns an error if the task \
does not exist.";
const UPDATE_DESCRIPTION: &str = "Update an existing session-lifetime task before moving to the \
next step. Tasks are user-visible real-time status; when working through steps, keep status \
current with `pending`, `inprogress`, `completed`, or `deleted`. Provide `taskid` and at least \
one of `status`, `subject`, or `description`; deletion is logical (`status = deleted`). If an \
unexpected problem blocks progress, do not force the next step: leave the task as-is, summarize \
the problem to the user, and end the turn.";
#[async_trait]
impl Tool for TaskCreateTool {

View File

@ -1,83 +0,0 @@
# 注入される system message をワーカー履歴に永続化する
## 背景
現状、`Method::Notify` および `Method::PodEvent``TurnEnded` / `Errored` / `ShutDown` / `ScopeSubDelegated`)は、親 Pod 側で `NotifyBuffer` に積まれ、`PodInterceptor::pre_llm_request` で **その1リクエスト限り**のsystem messageとして注入される`crates/pod/src/ipc/interceptor.rs:147-159`)。
しかし `Worker::run``let mut request_context = self.history.clone();` してから interceptor を呼ぶ(`crates/llm-worker/src/worker.rs:862, 913`ため、interceptor が push した notification は clone 側にしか乗らない。一方、その後の LLM 応答(`assistant_items`)は `crates/llm-worker/src/worker.rs:946`**本体の `self.history`** に extend される。
結果、履歴上は「文脈ゼロから Pod がいきなり `ReadPodOutput` を呼んだ」「何の前触れもなく `child Pod が落ちた件について調べる』と発言した」などの状態になる。次のリクエスト時点で既に LLM 自身の自己一貫性が壊れており、コンパクション・再起動を待たずして破綻している。
同じ問題は `tickets/session-todo-reminder.md` で予定している `<system-reminder>` 注入にも存在する。当初「reminder は同じ内容を繰り返し注入する可能性があるから履歴を汚さない」という方針を取っていたが、これは category error だった:
- **キャッシュ破壊**: 揮発で last user message を mutate する設計だと、worker.history 側は元のままなので、毎回 reminder の有無/内容差分で「実際に LLM へ送る user message の content」が変動する。Anthropic prompt cache は anchor までしか効かないため、anchor 直後の生成が毎回 cache miss になる
- **LLM の自己一貫性**: turn N で reminder を見て `TaskUpdate` を叩いた → turn N+1 では reminder が消えて、自分の `TaskUpdate` 呼び出しだけ残る。Notify と全く同じ因果切断
- **resume 時の不整合**: ロードした history からは reminder が完全消失している状態で再開する
- **「繰り返し注入で履歴肥大」の前提も弱い**: cooldown 設計上 reminder は idle 期間に1回 + 反応で counter リセット。連発は元々しない。仮に複数回出ても、それぞれが「その時点での active Task の snapshot」として履歴に並ぶのは因果として正しい
つまり「LLM に投げた system message は、その時点で history に commit する」が原則で、Notify / PodEvent / `<system-reminder>` を一律にこの原則に揃える。
## 方針
- LLM リクエスト直前に注入される system message は、`request_context: &mut Vec<Item>` の clone 側ではなく **worker 本体の `history` 側** に append する
- `NotifyBuffer`**「次のリクエスト直前で `worker.history` に append するキュー」** として再定義する
- 永続化(`history.json`)は worker.history 経由で自動的についてくる(`PodSharedState::history_json` → `RuntimeDir::write_history`
- 対象は現状の `Method::Notify` / `Method::PodEvent` に加え、session-todo-reminder で予定している `<system-reminder>` 注入も含む(後者は前者と同じ NotifyBuffer に乗せるか別キューを立てるかは実装裁量。重要なのは「history に commit される」点)
- `notify_wrapper` の文言(`[Notification] ... not a blocking request`)はそのまま履歴に残してよい。後から見ても「これは ambient 通知だった」と分かる方が望ましい
- `<system-reminder>` も同様、タグ込みのまま history に残す(タグ形式 `<system-reminder>...</system-reminder>` の規約自体は維持)
## 要件
### Notify / PodEvent 経路の挙動変更
- `NotifyBuffer::drain` 由来の Item は `request_context` ではなく `worker.history` に append される
- append は **次の LLM リクエスト直前** に1回だけ起きる複数 notify が貯まっていれば順序を保って複数 Item として並ぶ)
- append 後、`history.json` への永続化が通常の history mutation と同じパスで起きる
- 永続化された Item は次回 resume 時にそのまま履歴の一部として読み戻される
### `<system-reminder>` 注入経路session-todo-reminder の前提変更)
- `<system-reminder>` ブロックを「直近 user message を mutate して append」する設計を撤回し、**新規 system message Item として `worker.history` に append** する形に変更する
- ライフサイクルは Notify / PodEvent と同じ: 注入条件を満たした時点で history に commit、`history.json` に永続化、resume 後も読み戻される
- session-todo-reminder.md 側の「履歴を汚さない」「`get_history` / セッションログには現れない」「last user message を mutate」記述は本ticketの方針で上書きする
### 注入レーンの統一
- 「LLM リクエスト直前に注入される system message」は一律 history レーンに乗せる、と `crates/pod/src/ipc/notify_buffer.rs` のモジュールdocに明記する
- 「揮発history を汚さない)レーン」の概念は廃する。将来 reminder 系を追加する際も同じ原則に従う
- 命名・配置を見直す必要があれば実装内で判断してよい(例: `NotifyBuffer``PendingSystemMessages` 等に改称、reminder 用の別キューを作る等。本ticketは挙動の正しさが最優先で、抽象の作り方は実装者裁量
### 既存テスト・ドキュメントの更新
- `crates/pod/src/ipc/interceptor.rs``pre_llm_request_drains_pending_notifies_into_context` 系テストは、`request_context` ではなく `worker.history` への反映を検証する形に書き換える
- `crates/pod/tests/pod_events_test.rs``PodEvent` 受信後に history に対応 Item が現れることを E2E に近い粒度で確認するケースを追加する
- 既存の「揮発レーン」前提のコメント(`crates/pod/src/ipc/notify_buffer.rs:5` の `(never into the Worker's persistent history)` 等)を新方針に合わせて書き換える
- `tickets/session-todo-reminder.md` の方針記述を本ticketの完了に合わせて更新するまたは本ticket完了時点で先行修正してよい
- `TODO.md` 末尾の「タグ形式と『履歴を汚さない』原則は session-todo で先行確立」記述から後者を撤回する
## 完了条件
- 親 Pod が `Method::Notify` または `Method::PodEvent` を受信すると、その後の最初の LLM リクエスト直前に対応 system message が **`worker.history` に append** され、リクエストにも含まれる
- 同じ Item が `history.json` に書かれており、`Pod::resume` 後に履歴の一部として読み戻される
- LLM が notification に反応して取った行動tool call / 応答)と、そのトリガーとなった notification Item が、履歴上で因果順に並んでいる
- session-todo-reminder で導入される `<system-reminder>` 注入も同じく history append として実装されるまたは、実装順次第で本ticketは Notify / PodEvent 側だけ完了させ、session-todo-reminder 実装時にこの原則に従う形でもよい。後者の場合は session-todo-reminder.md 側の方針記述を本ticket完了時に更新済みであることが必須
- 単体テストで上記が確認できる
## 範囲外
- `notify_wrapper` の文言・phrasing の見直し
- TUI 側の Notify / PodEvent 表示(`Event::PodEvent` 経路は既存通り)
- compaction 時の notify Item の扱い(通常 Item と同じく compaction 対象になればよい。特別扱いは不要)
- `<system-reminder>` 注入機構の汎用化(`TODO.md` の既存項目。本ticketは個別実装の方針統一だけ扱う
## 参照
- 設計指針: `CLAUDE.md`
- 関連: `crates/pod/src/ipc/notify_buffer.rs`、`crates/pod/src/ipc/interceptor.rs`、`crates/pod/src/ipc/event.rs`、`crates/pod/src/controller.rs`(受信ハンドリング)、`crates/llm-worker/src/worker.rs:862, 913, 946`clone する側)
- 方針反転対象: `tickets/session-todo-reminder.md`「履歴を汚さない」前提を本ticketで撤回
## Review
- 状態: Approve
- レビュー詳細: [./notify-history-persist.review.md](./notify-history-persist.review.md)
- 対象コミット: `e804577 feat: notify-history-persist実装`
- 日付: 2026-05-03

View File

@ -1,47 +0,0 @@
# Review: 注入される system message をワーカー履歴に永続化する
対象コミット: `e804577 feat: notify-history-persist実装`
## 前提・要件の確認
### Notify / PodEvent 経路の挙動変更
- **`NotifyBuffer::drain` 由来の Item を `worker.history` に append**: 達成。`crates/llm-worker/src/worker.rs:859-867` で turn loop の先頭、per-request clone の前に `interceptor.pending_history_appends().await``self.history.extend(...)` で本体側に commit する。`crates/pod/src/ipc/interceptor.rs:127-145` で旧 `pre_llm_request` の注入ロジックを新メソッドに移送。
- **次の LLM リクエスト直前に1回だけ、複数 notify は順序保持で並ぶ**: 達成。`drain()` は破壊的、ループ各 iteration で1回呼ばれる。`pending_history_appends_drains_buffer_into_items` テストで複数 entry が順序保持で返ること、再呼出で空が返ることを検証。
- **`history.json` への永続化**: 達成。`worker.history` への通常の mutation と同じ経路(既存の `PodSharedState::history_json``RuntimeDir::write_history`)に乗る。実装上の追加配線は不要で、設計意図通り。
- **resume 時に読み戻される**: 達成(同上、既存パスを通る)。
### `<system-reminder>` 注入経路session-todo-reminder の前提変更)
- 本コミットでは reminder 実装そのものは扱わず、`tickets/session-todo.md` の方針記述のみ書き換え。`pending_history_appends` のドキュメント (`crates/llm-worker/src/interceptor.rs:121-148`) が新規 system message を history に commit する責務であることを明示し、reminder もこのレーンに乗ることが構造上保証される。チケットが許容する後者パターンsession-todo-reminder 実装時に同原則に従う)を取っており、整合済み。
### 注入レーンの統一
- **`notify_buffer.rs` モジュール doc**: 達成。`crates/pod/src/ipc/notify_buffer.rs:1-19` で「single lane for system messages produced by Pod state」「no transient, history-skipping lane」を明記し、`tickets/notify-history-persist.md` と `AGENTS.md` を参照。
- **trait レベルの規約**: `crates/llm-worker/src/interceptor.rs:121-148``pending_history_appends``pre_llm_request` の役割を doc で対比。`pre_llm_request` を「決定的・履歴非依存変換」のみに使い、外部 input は前者に乗せる旨を明記。CLAUDE.md / AGENTS.md の「LLM コンテキストの加工原則」の体現として適切。
### 既存テスト・ドキュメント更新
- `pre_llm_request_drains_pending_notifies_into_context``pending_history_appends_drains_buffer_into_items` にリネームし、新レーン側の挙動検証に置換。
- `pre_llm_request_skips_notification_injection_when_yielding``pre_llm_request_does_not_touch_pending_notifies` に変更。新方針上、yield 経路で buffer を保持する責務は不要history に既に commit 済みであれば resume 後に再取得不要)なので、合理的な置換。
- `controller_test.rs``notify_while_idle...` および `pod_event_turn_ended_while_idle...``handle.shared_state.history()` への assertion を追加し、request 側だけでなく history 側にも反映されることを検証。
- `tickets/session-todo.md` / `TODO.md` 末尾の「履歴を汚さない」記述を撤回・上書き済み。
## アーキテクチャ・スコープ
- **責務分離**: `Interceptor` trait に default 実装付きの新メソッドを1個追加し、Worker が turn loop で1回呼ぶだけ。llm-worker 層は「外部 input の中身」を知らず、低レベル基盤として hook されるだけ、という layer 区分を維持している。
- **抽象の規模**: 新規概念は `pending_history_appends` 1個のみ。NotifyBuffer の改名や別キュー新設には踏み込まず、ticket の「実装裁量」のうち最小コストの選択(既存 buffer をそのまま流用、レーンの解釈を doc で更新)を取った。歪みなし。
- **挙動の対称性**: 旧来 yield 時に buffer を保持していた設計が消えるが、新原則では「外部 input は受信時点で history 化されるべき」なので、turn loop 先頭での無条件 append は正しい。compaction 後 resume では buffer は既に空・履歴に entry あり、という状態に揃うのが意図通り。
- **LLM コンテキストの加工原則との整合**: 本ticketは原則そのものの適用であり、doc 側でも `pre_llm_request``pending_history_appends` の責務境界を明示している。原則に正面から沿った変更で、コードベースを歪めるどころか、これまでの破綻を正している。
## 指摘事項
### Blocking
- なし。
### Non-blocking / Follow-up
- **ticket が名指しした test ファイル位置との乖離**: ticket 要件に「`crates/pod/tests/pod_events_test.rs` で `PodEvent` 受信後に history に対応 Item が現れることを E2E に近い粒度で確認するケースを追加する」と明記されているが、本コミットは既存 `controller_test.rs::pod_event_turn_ended_while_idle_auto_starts_turn_and_injects_system_message` に history assertion を追加する形で代替している。機能カバレッジは同等以上(既存テストを重複させない方が健全)だが、文字通りの要件には合わない。今後の維持で問題は出ない見込みだが、必要なら ticket 側を「カバレッジは controller_test.rs 側で確保済み」と注記しておくと将来の読み手が混乱しない。
- **compaction yield 経路の挙動が暗黙化**: 旧 `pre_llm_request_skips_notification_injection_when_yielding` は「yield 中は buffer に積み残す」を保証していた。新設計では「turn loop 冒頭で必ず history に commit、yield しても history は残る」だが、これを直接検証するテストcompaction 発火 → resume 後に history へ notify が残ること)は追加されていない。既存 compaction テストが history 側で間接的に保護してくれる範囲だが、回帰検出力としては薄い。session-todo-reminder で reminder を載せる際に同種の検証を入れる前提なら follow-up で十分。
### Nits
- `crates/pod/src/ipc/interceptor.rs:14-22` の use 順序: `tracing::warn`line 20`llm_worker::interceptor::...`line 16-19`llm_worker::tool::ToolOutput`line 21の間に入り、`llm_worker::` グループが分断されている。`tracing::warn` を `tracing::info` の隣line 22に置けば従来のグルーピングが保てる。`cargo fmt` がこれを揃えているなら無視可。
## 判断
**Approve完了可** — 要件は実質すべて満たされており、CLAUDE.md / AGENTS.md の「LLM コンテキストの加工原則」を体現する模範的な変更。ticket が名指しした test ファイル位置との微妙な不一致は機能カバレッジ等価のため非ブロッキング。

View File

@ -2,7 +2,7 @@
## 背景
`tickets/session-todo.md` で導入する Task ツール群があっても、LLM はそれを使わずに作業を続け得る。ツールを呼ばないまま会話が長引くと、
`tickets/session-todo.md` で導入した Task ツール群があっても、LLM はそれを使わずに作業を続け得る。ツールを呼ばないまま会話が長引くと、
- 開始した作業の `inprogress` がずっと放置されたままになる
- 「やったつもり」になって `completed` への更新を忘れる
@ -10,58 +10,51 @@
OpenCode の todo は専用の注意機構を持たない(汎用 reminder 経由)。一方 Claude Code は `task_reminder` を「N リクエスト無アクティビティで初めて発火するナッジ型」として実装しており、毎リクエスト押し戻しはしない(`/home/hare/.local/share/claude/versions/2.x` の `du_` / `cu_` 関数、閾値 `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }`)。
Insomnia でも同方針を採り、active Task が残っているのに `TaskCreate` / `TaskUpdate` が一定リクエスト呼ばれていない場合に限り、`<system-reminder>` で揮発的に思い出させる。「やったつもり」抑止と、トークン浪費・LLM の自律性侵害のバランスを取るため、毎リクエスト押し戻しはしない。
Insomnia でも同方針を採り、active Task が残っているのに `TaskCreate` / `TaskUpdate` が一定リクエスト呼ばれていない場合に限り、`<system-reminder>` Item を 1件 history に append する。「やったつもり」抑止と、トークン浪費・LLM の自律性侵害のバランスを取るため、毎リクエスト押し戻しはしない。
## 前提
- `tickets/session-todo.md` の TaskStore と `TaskCreate` / `TaskUpdate` / `TaskList` / `TaskGet` ツールが利用可能であること
- `pre_llm_request` 相当のフックを Pod が持つこと無ければ本ticket で導入
- `tickets/session-todo.md` の TaskStore と `TaskCreate` / `TaskUpdate` / `TaskList` / `TaskGet` ツールが利用可能
- `Interceptor::pending_history_appends` レーンが利用可能(`tickets/notify-history-persist.md` で導入済み
## 方針
- **`pre_llm_request` Interceptor として実装**。直近の user message に `<system-reminder>` ブロックを揮発的に append するだけ。履歴・ログには載せない
- **system-reminder 注入の汎用化はやらない**。利用者が Task 1機構しかない段階で抽象を立てないCLAUDE.md「概念の追加は不在が問題になってから」。ただし「タグ形式は `<system-reminder>...</system-reminder>` で揃える」「履歴は汚さない」の2点は本実装で確立し、将来の追加機構が同じ規約に乗れるようにする
- **`pending_history_appends` で実装**。発火時に `<system-reminder>` ブロックを含む新規 system message Item を返し、Worker が `worker.history` に append する。Notify / PodEvent と同じレーンで永続化・resume・compaction が自動で揃う
- **揮発的注入は採らない**`AGENTS.md` 「LLM コンテキストの加工原則」で禁止。history に commit せずに context を変えると、resume 時に LLM の発言の根拠が再現できなくなる)
- **system-reminder 注入機構の汎用化はやらない**。利用者が Task 1機構しかない段階で抽象を立てない`AGENTS.md`「概念の追加は不在が問題になってから」)。タグ形式 `<system-reminder>...</system-reminder>` の規約は本実装で踏襲する
- **発火はナッジ型**。N リクエスト無アクティビティで初めて発火し、cooldown も持つ
## 要件
### Interceptor
- `pre_llm_request``Vec<Item>` を受け取り、以下の AND を満たした場合のみ発動
- active Task`pending` または `inprogress`が1件以上存在する
- 直近 N リクエスト (暫定 N=8) `TaskCreate` / `TaskUpdate` のいずれも呼ばれていない
- 前回 reminder 注入から M リクエスト (暫定 M=8) 以上経過
- ここで言う「リクエスト」は **LLM への1回の推論呼び出し (= assistant 応答1回)** の単位で数える。ユーザー発火単位ではない。1ユーザー発火内で tool ループが回れば、tool_result を受けて発火する次のリクエストもそれぞれ1としてカウントする
- カウント対象はメインスレッドの assistant 応答に限る。サブエージェント / sidechain の assistant 応答は除外する
- カウンタは Pod 側の session-lifetime 状態として保持する(`requests_since_last_task_management` / `requests_since_last_reminder`。resume 時は履歴の逆走査で再計算するか 0 リセットで再開する。どちらでも「初回ナッジが最大 N リクエスト遅れる」だけで挙動として致命ではない
- 発動時、直近の user message の contentまたは content[最終 text part])の末尾に `<system-reminder>` ブロックを append し、現在の active Task リストを `taskid` / `status` / `subject` を含む簡潔な形式で列挙する。`description` は長大化を避けるため省略してよい
- 履歴 (`Worker` の保持する `Vec<Item>`) は変更しない。リクエスト送信時の Vec のみ加工する
- active Task が空の場合は何も差し込まない(忘却防止が目的なので、思い出させる対象が無いなら不要)
- `pending_history_appends` で以下を **AND** で満たす場合のみ発動し、`<system-reminder>` ブロックを含む `Item::system_message` を 1件返す。条件外なら空 `Vec<Item>` を返す
- active Task`pending` または `inprogress`)が 1件以上存在する
- 直近 N リクエスト(暫定 N=8`TaskCreate` / `TaskUpdate` のいずれも呼ばれていない
- 前回 reminder Item の append から M リクエスト(暫定 M=8以上経過
- ここで言う「リクエスト」は LLM への 1回の推論呼び出しassistant 応答 1回の単位。1ユーザー発火内で tool ループが回れば、`tool_result` を受けて発火する次のリクエストもそれぞれ 1としてカウントする
- カウンタは Pod 側の session-lifetime 状態として保持する(`requests_since_last_task_management` / `requests_since_last_reminder`。resume 時は worker.history の逆走査で再計算するか 0 リセットで再開する。後者でも「初回ナッジが最大 N リクエスト遅れる」だけで挙動として致命ではない
- 返す Item の本文は `<system-reminder>` で囲み、現在の active Task を `taskid` / `status` / `subject` を含む簡潔な形式で列挙する。`description` は長大化を避けるため省略してよい
- active Task が空の場合は何も append しない(思い出させる対象が無いなら不要)
## 完了条件
- 直近 N リクエスト連続で `TaskCreate` / `TaskUpdate` が呼ばれず、かつ active Task が残っている場合に限り、`pre_llm_request` で `<system-reminder>` が直近 user message に append される
- 直近 N リクエスト連続で `TaskCreate` / `TaskUpdate` が呼ばれず、かつ active Task が残っている場合に限り、`pending_history_appends` が `<system-reminder>` を含む `Item::system_message` を 1件返す
- 返された Item が `worker.history` に append され、その後のリクエスト・`history.json`・resume 後の `get_history` でも同じ Item が見える(揮発レーンは持たない)
- `TaskCreate` / `TaskUpdate` のいずれかが呼ばれるとカウンタがリセットされ、再び N リクエスト経過するまでは reminder が出ない
- reminder が一度出たあとは、cooldown M リクエストが経過するまで再注入されない
- active Task が0件の場合は reminder が出ない
- system-reminder の注入は揮発的で、`get_history` / セッションログには現れない
- 単体テストで Interceptor の発火条件リクエスト回数閾値、active 0件、cooldown、サブエージェント除外がカバーされる
- active Task が 0件の場合は reminder が出ない
- 単体テストで Interceptor の発火条件リクエスト回数閾値、active 0件、cooldownがカバーされる
## 範囲外
- inprogress 滞留検出 / 多重 inprogress 検出など、状態異常ベースの追加トリガ(必要になれば別チケットで追加)
- system-reminder 注入機構の汎用化(`TODO.md` に立項済み、別途検討)
- `TaskCreate` / `TaskUpdate` の戻り値に active Task 全件を埋め込む強化(必要に応じて Tool ticket 側で対応)
- サブエージェント / sidechain での独自 reminder 発火main Pod の interceptor から動く構造のため自然に対象外)
## 参照
- 設計指針: `CLAUDE.md`(最小の構造化 / 概念の追加は不在が問題になってから
- 前提: `tickets/session-todo.md`Tool 群と TaskStore
- 設計指針: `AGENTS.md`LLM コンテキストの加工原則。揮発的注入は禁止、history に append してから commit する
- 前提: `tickets/session-todo.md`Tool 群と TaskStore、`tickets/notify-history-persist.md``pending_history_appends` レーン)
- 参考実装: Claude Code の `task_reminder``du_` / `cu_` 関数、閾値 `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }`
## Review
- 状態: Approve (spec 段階)
- レビュー詳細: [./session-todo-reminder.review.md](./session-todo-reminder.review.md)
- 日付: 2026-05-03
- 補足: 実装着手前に Non-blocking で挙げた 4 点 (TaskCreate/Update のカウンタリセット契機 / active 取得経路 / reminder 本文 fmt / resume 時のカウンタ扱い) を spec に追記してから実装するとブレが減る。実装完了時に完了条件を再確認。

View File

@ -1,41 +0,0 @@
# Review: セッション内 Task ツールの注意機構
対象: `tickets/session-todo-reminder.md` (`28fe1da` で新規作成)。実装は未着手 (本レビュー時点でコード変更なし、worktree 内 grep で `task_reminder` / `requests_since_last_task` 等いずれもヒットせず)。
本レビューは spec 単独レビュー (実装が始まった時点で完了条件を再確認)。
## 前提・要件の確認 (spec の妥当性)
- 「`pre_llm_request` で `Vec<Item>` を受けて直近 user message に `<system-reminder>` を append する」は、`crates/pod/src/ipc/interceptor.rs` の既存 `pre_llm_request` レーンに自然に乗る。Worker history を変更せず、リクエスト送信用 `Vec<Item>` のみ加工する方針は、既存の `pending_notifies.drain()` と同じパターンで先例あり (`interceptor.rs:147-159`)。spec として実装可能性は高い。
- TaskStore からの active 件数取得には A の `TaskStore::list()` だけで足り、追加 API 不要。
- カウンタ (`requests_since_last_task_management` / `requests_since_last_reminder`) を Pod の session-lifetime 状態として持つ方針は、既存 `tool_calls_this_turn: AtomicUsize` (`interceptor.rs`) と同じ素地に追加できる。pre_tool_call で `TaskCreate` / `TaskUpdate` を観測したら片方をリセット、`pre_llm_request` で発火条件を見て両方を増やす、という骨格になる想定。spec の粒度は問題ない。
- メインスレッド限定 / sub-agent 除外: 主要 worker と spawn pod / compact worker を分離している現実装では「メインの `PodInterceptor` だけがカウンタを持つ」で達成できる。compact worker 側は別 Worker / 別 Interceptor。spec の前提は現実装と一致。
- resume 時のカウンタ復元: 「履歴の逆走査で再計算 or 0 リセット」のどちらでも実害無しと spec 側で割り切られているのは妥当。0 リセットの方が大幅にシンプルで、初回ナッジが最大 N 遅れるだけ。実装時はこちらを推奨したい (spec 側は両許容で問題なし)。
- 「揮発的 = 履歴を汚さない」「タグ形式 `<system-reminder>...</system-reminder>` で揃える」「2 件目の利用者が出た時に汎用化を検討」の 3 点を明文化していて、A と B の隙間に「汎用 reminder 機構」の中途半端な抽象が立たないようにしている。CLAUDE.md の「概念の追加は不在が問題になってから」と整合。
## アーキテクチャ・スコープ
- B は A に対する後続チケットとして適切に分離されている。前提依存 (TaskStore + Task* tools) を `## 前提` に明記しており、A 完了後に着手するという順序付けも自然。
- 範囲外 (inprogress 滞留 / 多重 inprogress / 注入機構汎用化 / Tool 戻り値の active 全件埋め込み) が列挙されており、本チケットで肥大化しないラインが引けている。
- 「Pod 側に session-lifetime カウンタを増やす」程度の侵襲で、既存の `crates/pod/src/ipc/interceptor.rs` 内に追加実装が収まる規模感。クレート構成や層境界に新しい歪みは生じない見込み。
- 「N=8 / M=8 暫定」と数値を明示しているのは判断しやすい。Claude Code が `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }` という参照値を持つことも書かれており、後で詰めやすい。
## 指摘事項
### Non-blocking / Follow-up (spec 段階の論点)
- **「リクエスト = LLM への 1 回の推論呼び出し」のカウント単位が、現 `Interceptor``pre_llm_request` 1 呼び出しと一致するか確認が必要。** 現実装の `pre_llm_request` は context 構築直前に必ず通るレーンで、tool ループ内の続行 LLM 呼び出しでも毎回呼ばれる。実装時に「pre_llm_request 突入時刻 = LLM へのリクエスト 1 件」が成立しているかを軽く検証するテストを1本入れてほしい (tool ループ中の発火 cadence のため)。
- **TaskCreate / TaskUpdate のカウンタリセット契機。** 現状 spec は「呼ばれていない」と書かれているのみ。実装ではどのフックで観測するかの選択 (`pre_tool_call` か `post_tool_call` か) を決めて spec に追記しておくと実装ブレが減る。`pre_tool_call` で名前判定 + リセット、で十分。
- **active Task の取り出し経路。** spec 上は明示されていないが、Interceptor が TaskStore handle を保持する形になる (Pod 経由で渡す)。`PodInterceptor::new` 等のコンストラクタに `Option<TaskStore>` を増やす想定で良いと思うが、実装時に `Tracker` の例 (`attach_tracker`) と同様、Pod から Interceptor への手渡しを統一してほしい。
- **resume 時の発火タイミング。** 0 リセットを採るなら、resume 直後の最初のリクエストでは出ない。これは spec 上「最大 N 遅れるだけ」と許容済み。実装時は単にこの選択を README/コメントに残してほしい。
- **`<system-reminder>` の本文。** spec は「taskid / status / subject を簡潔に列挙」と書かれているが、A 側の `render_snapshot` を流用するか、reminder 専用に別関数を切るかが未決定。短さを優先するなら専用 fmt が望ましい (description を必ず落とすため)。実装方針を spec に1行追記しておくと安全。
- **テスト範囲。** spec の完了条件にある「リクエスト回数閾値 / active 0件 / cooldown / サブエージェント除外」は単体テストでカバー可能。実装時はそれぞれ独立ケースで書いてほしい。
### Nits
- 数値 N / M が「暫定 N=8」「暫定 M=8」と明記されているが、変更可能性についてのフォローアップ条項 (例: 運用後に閾値を見直す) があると親切。
- Claude Code 側の参照値 `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }` は spec に書かれているので、Insomnia 側 8 を採る理由 (短めにして気付かせやすくする等) を一文添えると将来 tuning しやすい。
## 判断
**Approve (spec 段階)** — 前提・要件・範囲外の切り分けが妥当で、A の実装規約 (TaskStore handle、揮発的注入、タグ形式) に乗る形で実装可能。アーキテクチャ的歪みも生じない見込み。実装着手前に上記 Non-blocking 4 点 (カウンタリセット契機 / active 取得経路 / 本文 fmt / resume 時のリセット選択) を spec に1〜2行追記してから実装に入ると、実装中の判断ブレが減る。実装完了時に完了条件を再確認する必要がある (本レビューは spec 妥当性のみの判定)。