docs(tickets): compact-worker-occupancy-cap完了
This commit is contained in:
parent
4d61d044ec
commit
5cf8eb94c7
2
TODO.md
2
TODO.md
|
|
@ -24,7 +24,6 @@
|
|||
- セッションコンテキスト長 / ウィンドウ占有率の常時表示 → [tickets/tui-context-usage-indicator.md](tickets/tui-context-usage-indicator.md)
|
||||
- Manifest: Tool Output / File Upload 上限の分離とデフォルト緩和 → [tickets/manifest-output-upload-limits.md](tickets/manifest-output-upload-limits.md)
|
||||
- Prune: 保護境界を turn 数から末尾 token budget に置き換え → [tickets/prune-token-budget.md](tickets/prune-token-budget.md)
|
||||
- Compact worker サーキットブレーカーを占有量ベースに統一 → [tickets/compact-worker-occupancy-cap.md](tickets/compact-worker-occupancy-cap.md)
|
||||
- メモリ機構
|
||||
- 使用頻度メトリクス + Knowledge 化候補レポート → [tickets/memory-usage-metrics.md](tickets/memory-usage-metrics.md)
|
||||
- Phase 1/2 呼称を extract/consolidation に統一 → [tickets/memory-phase-naming.md](tickets/memory-phase-naming.md)
|
||||
|
|
@ -32,3 +31,4 @@
|
|||
- セッション内 Task ツールの注意機構（無アクティビティで `<system-reminder>` ナッジ） → [tickets/session-todo-reminder.md](tickets/session-todo-reminder.md)
|
||||
- ワークスペースのメモリーをLintするヘッドレスCLI
|
||||
- system-reminder 注入機構の汎用化（2件目の利用者が出た時に検討。タグ形式 `<system-reminder>...</system-reminder>` の規約は session-todo-reminder で先行確立。注入された Item は worker.history に append する方針）
|
||||
- Bashツールがファイル編集に常用されている問題をdesciptionで抑制
|
||||
|
|
|
|||
|
|
@ -1,56 +0,0 @@
|
|||
# Compact worker のサーキットブレーカーを占有量ベースに統一
|
||||
|
||||
## 背景
|
||||
|
||||
Compact worker のサーキットブレーカー (`crates/pod/src/compact/worker.rs:253-269` の `CompactWorkerInterceptor`) は `compact_worker_max_input_tokens` を `UsageEvent.input_tokens` の **累積和** で見ている。一方、`UsageEvent.input_tokens` の定義 (`crates/llm-worker/src/llm_client/event.rs:76-94`) は「送信した prompt prefix の総トークン数（占有量、キャッシュ込み）」であり、Anthropic 側でも `cache_read + cache_creation` を加算してこの規約に揃えている。
|
||||
|
||||
結果として現行の累積メトリックは、毎ターン同じ prefix をフルカウントする「context size × ターン数」相当の値を測っており:
|
||||
|
||||
- compact worker は `build_summary_prompt` (`crates/pod/src/pod.rs:2630-2657`) で reasoning / ToolCall.arguments / ToolResult.content を strip した skeleton + 探索ツールという設計なので、初回 input は元 history より大幅に小さい（数十 K 程度）。
|
||||
- それでも cache hit 含む prefix を毎ターン丸ごと足していくため、20-30K の skeleton を入力にツールを 2-3 回叩いた時点で 50K デフォルトに到達する。
|
||||
- prompt cache が 99% ヒットしていても累積値は同じだけ増えるので、コストの近似にも安全マージンの近似にもなっていない。
|
||||
|
||||
メイン Worker 側の対応するしきい値 (`compact_threshold`, `compact_request_threshold`) は `Pod::total_tokens()` (`crates/pod/src/pod.rs:146-149` → `llm_worker::token_counter::total_tokens(history, &usage_records)`) を見ており、これは `UsageRecord` 列を最新測定 + バイト按分で射影した「現在の占有量」(単一の値, 累積ではない)。Compact worker でもこの正規のカウンタに統一すべき。
|
||||
|
||||
サーキットブレーカーとして測るべき軸は二つあり、占有量カウンタは前者だけを担当する:
|
||||
|
||||
1. **占有量** (cost / window 圧迫の相当値): `total_tokens()` を流用。
|
||||
2. **ループ深さ** (短い context でツールを延々叩く暴走): `Worker::set_max_turns` (`crates/llm-worker/src/worker.rs:1369`) で別途上限を入れる。
|
||||
|
||||
## 方針
|
||||
|
||||
`CompactWorkerInterceptor` を、メインと同じ `UsageTracker` + `total_tokens` 機構に乗せ替える。累積メトリックは廃止する。ループ深さ対策として `compact_worker_max_turns` を新設し、`set_max_turns` 経由で compact worker に伝える。
|
||||
|
||||
## 要件
|
||||
|
||||
- `CompactWorkerInterceptor` を削除または書き換え、`pre_llm_request` の判定を `llm_worker::token_counter::total_tokens(worker.history(), &records).tokens > max_input_tokens` に切り替える。`input_so_far: AtomicU64` の累積パスは廃止。
|
||||
- Compact worker にも `UsageTracker` を持たせ、`pre_llm_request` で `note_request(history.len())`、`on_usage` で `record_usage` する。メイン Pod (`pod.rs:777-780`) と同じ配線パターン。
|
||||
- `compact_worker_max_input_tokens` の意味を「compact worker 側の現在占有量しきい値」に変更し、ドキュメントとデフォルト値を更新する。デフォルトは `compact_threshold` と単位が揃うため、現行 50K のままだと typical な main 側設定 (80K) に対して小さく compact 自身がそれを下回るのは妥当な範囲。実値は新セマンティクスで再評価する（要件としては「累積値ではなく占有量を測る」ことのみで固定）。
|
||||
- `CompactionConfig` に `compact_worker_max_turns: Option<u32>` を追加し、`compact()` (`pod.rs:1458-`) で `summary_worker.set_max_turns` に渡す。`None` のときは無制限（既存動作）。デフォルトは要検討（仮: `Some(20)`）。
|
||||
- 後方互換 shim は入れない。`compact_worker_max_input_tokens` はフィールド名を維持しつつセマンティクスだけ差し替えるため、旧設定値はそのまま新セマンティクスで解釈される。閾値のオーダーは大きくは変わらないので運用上の破壊的影響は小さい。
|
||||
|
||||
## 完了条件
|
||||
|
||||
- 通常の compact 実行で、cache hit 込みの prefix がフルカウントされなくなり、`build_summary_prompt` の skeleton + 数回のファイル読み程度では cap に当たらない。
|
||||
- 短い context でツールを延々呼び続ける疑似ケースで、`compact_worker_max_turns` により compact run が打ち切られる。
|
||||
- `Pod::total_tokens()` と compact worker の占有量推定で同じ算出経路 (`llm_worker::token_counter::total_tokens`) が使われている。
|
||||
|
||||
## 範囲外
|
||||
|
||||
- `compact_threshold` / `compact_request_threshold` 自体のセマンティクス・既定値変更。
|
||||
- Compact worker が更に compact をかける（meta-compact）。
|
||||
- `compact_auto_read_budget` ロジックの変更。
|
||||
- token 推定アルゴリズム自体の改善。
|
||||
|
||||
## 影響範囲
|
||||
|
||||
- `crates/pod/src/compact/worker.rs`: `CompactWorkerInterceptor` の実装、`input_so_far` 経路の削除。
|
||||
- `crates/pod/src/pod.rs`: `compact()` の compact worker 構築箇所 (`UsageTracker` 配線、`on_usage` 差し替え、`set_max_turns` 呼び出し)。
|
||||
- `crates/pod/src/compact/usage_tracker.rs`: 既存 `UsageTracker` を compact worker からも使うため、必要なら可視性 / API の調整。
|
||||
- `crates/manifest/src/lib.rs`, `crates/manifest/src/config.rs`, `crates/manifest/src/defaults.rs`: `compact_worker_max_input_tokens` のドキュメント更新、`compact_worker_max_turns` の追加とカスケード。
|
||||
- `crates/pod/src/compact/worker.rs` のユニットテスト、および compact 関連の統合テスト。
|
||||
|
||||
## Review
|
||||
- 状態: Approve with follow-up
|
||||
- レビュー詳細: [./compact-worker-occupancy-cap.review.md](./compact-worker-occupancy-cap.review.md)
|
||||
- 日付: 2026-05-11
|
||||
|
|
@ -1,41 +0,0 @@
|
|||
# Review: Compact worker のサーキットブレーカーを占有量ベースに統一
|
||||
|
||||
レビュー対象: commit `ef0cdf7` (base `d818b37`).
|
||||
|
||||
## 前提・要件の確認
|
||||
|
||||
- 要件1「`CompactWorkerInterceptor` を `total_tokens` ベースに切り替え、`input_so_far: AtomicU64` 経路を廃止」: 満たされている。`crates/pod/src/compact/worker.rs:249-274` で `usage_tracker.records()` → `llm_worker::token_counter::total_tokens(context, &records)` に置き換わっており、`AtomicU64` import / フィールドも削除されている。`crates/pod/src/pod.rs:1456` 周辺の `use std::sync::atomic` も消えている。
|
||||
- 要件2「Compact worker に `UsageTracker` を持たせ、`pre_llm_request` で `note_request`、`on_usage` で `record_usage`」: 満たされている。`pod.rs:1537-1547` で `summary_usage_tracker = Arc::new(UsageTracker::new())` を作り、`on_usage` で `record_usage(event)`、`CompactWorkerInterceptor::pre_llm_request` 内で `usage_tracker.note_request(context.len())` を呼ぶ (`compact/worker.rs:262-272`)。
|
||||
- 要件3「`compact_worker_max_input_tokens` の意味を「現在占有量しきい値」に変更し doc 更新」: 満たされている。`crates/manifest/src/lib.rs:324`、`crates/manifest/src/defaults.rs:39-43`、`docs/compaction.md:143`、`docs/manifest.toml:215` がすべて「現在占有」「累計ではない」表現に揃っている。フィールド名は維持され、後方互換 shim は入っていない (要件通り)。
|
||||
- 要件4「`compact_worker_max_turns: Option<u32>` を新設し `Worker::set_max_turns` 経由で渡す」: 満たされている。`CompactionConfig` (`lib.rs:329-332`)、`CompactionConfigPartial` (`config.rs:118-119`)、`defaults::COMPACT_WORKER_MAX_TURNS = Some(20)` (`defaults.rs:46-48`)、`pod.rs:1548-1550` で `summary_worker.set_max_turns(...)`。manifest 側の merge / TryFrom / TOML パースもテスト付きで通っている (`config.rs:960-983`, `lib.rs:521-546`)。
|
||||
- 要件5「後方互換 shim 無し」: 満たされている。フィールド名は変更しないため旧 manifest はそのまま新セマンティクスで読まれ、deprecated alias 等は導入されていない。
|
||||
- 完了条件1「cache hit 込み prefix がフルカウントされない」: 単体テスト `compact_worker_interceptor_uses_occupancy_not_cumulative_usage` (`compact/worker.rs:301-328`) で「2 回 100-token 入力 → 累積 200 でも occupancy は最新の 100 のままで cap=150 を通る」を確認。
|
||||
- 完了条件2「`compact_worker_max_turns` で打ち切られる」: `set_max_turns` のロジック自体は llm-worker 側の既存挙動 (`worker.rs:1045-1050`) に乗るのみで、本チケットでは新規ロジックを足していない。配線テスト (manifest 側のパース) は入っているが、compact 経由で実 abort する pod-level の統合テストは無し。Trivial wiring なのでブロッキングではないが、後で run-level テストを足すと安心。
|
||||
- 完了条件3「`Pod::total_tokens()` と同じ算出経路」: 満たされている。`pod.rs:148` (`llm_worker::token_counter::total_tokens(self.history(), &usage)`) と `compact/worker.rs:265` (`llm_worker::token_counter::total_tokens(context, &records)`) が同じ関数を経由する。compact 側は per-request の prune 後 `request_context` を渡すため、メイン側と完全一致ではないが、`pre_llm_request` 時点でのその後 LLM に投げる prefix 占有量という意味では妥当 (むしろ正確)。
|
||||
|
||||
## アーキテクチャ・スコープ
|
||||
|
||||
- 修正は `compact/worker.rs` + `pod.rs` の compact 構築 + manifest 1 フィールド追加に閉じており、ticket 影響範囲表と完全一致。`crates/pod/src/compact/usage_tracker.rs` への変更は「`records()` メソッドを `pub(crate)` で追加して drain せず読む」のみで、API 拡張は最小。
|
||||
- `UsageTracker` は元々「per-LLM-request 計測のペアリング・drain は Pod が persist 時に行う」モデル。compact worker は drain せず read-only で snapshot を覗くだけなので、メインの persist セマンティクスを汚さない。compact worker のループ内で記録しても drain しないままワーカーが破棄される (`pod.rs:1530-1551` で関数ローカル) ため、メインの `Pod::usage_tracker` (turn 永続化用) とは完全に独立した別インスタンスで、相互干渉しない。設計として綺麗。
|
||||
- `note_request` をメインでは `UsageTrackingHook` (`pod.rs:46-59`) として `pre_llm_request` Hook で呼んでいるのに対し、compact 側は `CompactWorkerInterceptor::pre_llm_request` 内で直接呼んでいる。Worker のコール順 (Hook → Interceptor → stream → on_usage) を踏まえると `note_request` は `record_usage` より前に呼ばれていれば意味的に等価で、両者とも同じ tick で呼ばれるため不整合は無い。ただし「メイン: Hook で `note_request`、Interceptor で別判定」「compact: Interceptor で両方」と配線パターンが分岐している点は将来読む人が混乱するかも。今のままでも動くが、補足コメントを Pod 側か Interceptor 側に入れておくと親切。
|
||||
- ループ深さは `Worker::set_max_turns` 既存機構の流用。新規ロジック無し、コードベースを歪めない選択。
|
||||
|
||||
## 指摘事項
|
||||
|
||||
### Blocking
|
||||
無し。
|
||||
|
||||
### Non-blocking / Follow-up
|
||||
- `pod.rs:1548-1550` の `if compact_worker_max_turns.is_some() { summary_worker.set_max_turns(compact_worker_max_turns); }` は不要な分岐。`set_max_turns` は `Option<u32>` を取り、`None` を渡しても初期値 `None` を上書きするだけで no-op (`crates/llm-worker/src/worker.rs:1369-1371` + `worker.rs:1181`)。無条件に `summary_worker.set_max_turns(compact_worker_max_turns);` で十分。条件付けることで「`None` だと何もしない」読者バイアスを生んでむしろ読みづらい。
|
||||
- `crates/pod/src/pod.rs:2178-2201` の `MemoryExtractWorkerInterceptor` は古い累積メトリック方式のまま残っており、コメントには `Mirror of compact::worker::CompactWorkerInterceptor;` と書かれている。今回の変更で「mirror」ではなくなったのでコメントが嘘になっている。memory extract 側のセマンティクス変更は本チケットの範囲外で正しい判断だが、コメント補正 (例: 「以前は CompactWorkerInterceptor のミラーだったが、compact 側は占有量ベースに移行した。memory extract 側は別チケットで追従予定」) を別チケットなり TODO なりで残しておきたい。
|
||||
- `compact_worker_max_turns` が compact 経由で実際に `Aborted/MaxTurnsReached` を引き起こす経路の pod-level 統合テストは含まれていない。配線そのものはトリビアルで、`set_max_turns` の振る舞いは llm-worker 側で別途テストされている前提。後追いで足すかどうかは判断に任せる。
|
||||
- デフォルト `COMPACT_WORKER_MAX_TURNS = Some(20)` は `build_summary_prompt` の skeleton + 数回ファイル読みなら十分余裕がある妥当なライン。ただし auto_read_budget (8000) と read_file の典型呼び出しサイズを考えると、深いツールループが起きる前にトークン cap が先に効きやすい設計なので、20 が上限になる場面はまずレアケース。妥当な保険値として OK。
|
||||
|
||||
### Nits
|
||||
- `compact/worker.rs:249-253` の docstring は新しいセマンティクスを正しく説明できている。良。
|
||||
- `compact/usage_tracker.rs:102-112` `records()` の docstring が「request-time circuit breakers が同じ occupancy projection を見るため」と用途を明示しており、将来の extract worker 側追従の伏線にもなっている。良。
|
||||
- ユニットテスト `compact_worker_interceptor_uses_occupancy_not_cumulative_usage` (`compact/worker.rs:301-328`) は「累積では落ちる量でも occupancy では通る」ケースをピンポイントで掴んでいて、今回の本質的なバグへの回帰防止として的確。
|
||||
|
||||
## 判断
|
||||
|
||||
Approve with follow-up — チケットの 4 要件と完了条件 1/3 はすべて根拠つきで満たされている。残るのは (a) `set_max_turns` 呼び出しの不要な分岐、(b) `MemoryExtractWorkerInterceptor` 側のコメント陳腐化と将来の追従、(c) max_turns の pod-level 統合テスト追加、いずれも非ブロッキング。マージ可。
|
||||
Loading…
Reference in New Issue
Block a user