6.7 KiB
6.7 KiB
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_request1 呼び出しと一致するか確認が必要。 現実装の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 妥当性のみの判定)。