memory-phase1の、トークンカウントの実装位置が悪い件
This commit is contained in:
parent
11bd486740
commit
a9d30e1c37
1
TODO.md
1
TODO.md
|
|
@ -19,5 +19,6 @@
|
|||
- [ ] Phase 2 consolidation → [tickets/memory-phase2-consolidation.md](tickets/memory-phase2-consolidation.md)
|
||||
- [ ] 使用頻度メトリクス + Knowledge 化候補レポート → [tickets/memory-usage-metrics.md](tickets/memory-usage-metrics.md)
|
||||
- [ ] GC（定期再評価） → [tickets/memory-gc.md](tickets/memory-gc.md)
|
||||
- [ ] session-store / llm-worker 型責務の整理 → [tickets/session-store-llm-worker-type-ownership.md](tickets/session-store-llm-worker-type-ownership.md)
|
||||
- ワークスペースのメモリーをLintするヘッドレスCLI
|
||||
- Thinking中のTUI上での表示: 内容の公開/非公開両対応
|
||||
|
|
|
|||
|
|
@ -174,6 +174,18 @@ pub(crate) fn total_tokens_impl(history: &[Item], records: &[UsageRecord]) -> To
|
|||
tokens_at(history, records, history.len(), &prefix)
|
||||
}
|
||||
|
||||
/// 任意の history index 時点でのプロンプト全長推定。
|
||||
/// `history_len == 0` で 0 を返す。delta 計算 (extract trigger 等) で
|
||||
/// `total_tokens_at(now) - total_tokens_at(pointer)` の形で使う。
|
||||
pub(crate) fn total_tokens_at_impl(
|
||||
history: &[Item],
|
||||
records: &[UsageRecord],
|
||||
history_len: usize,
|
||||
) -> TokenEstimate {
|
||||
let prefix = prefix_bytes(history);
|
||||
tokens_at(history, records, history_len.min(history.len()), &prefix)
|
||||
}
|
||||
|
||||
fn split_for_retained_impl(history: &[Item], records: &[UsageRecord], retained: u64) -> SplitPoint {
|
||||
let prefix = prefix_bytes(history);
|
||||
let current = tokens_at(history, records, history.len(), &prefix);
|
||||
|
|
@ -291,6 +303,17 @@ impl<C: LlmClient, St: Store> Pod<C, St> {
|
|||
total_tokens_impl(self.history(), &usage)
|
||||
}
|
||||
|
||||
/// 任意の history index 時点でのプロンプト全長推定。
|
||||
///
|
||||
/// `total_tokens()` と同じ accounting を任意位置で評価する版。
|
||||
/// memory phase 1 trigger が
|
||||
/// `total_tokens_at(now) - total_tokens_at(pointer)` で
|
||||
/// pointer 以降に増えたプロンプト長を測るのに使う。
|
||||
pub fn total_tokens_at(&self, history_len: usize) -> TokenEstimate {
|
||||
let usage = self.usage_history();
|
||||
total_tokens_at_impl(self.history(), &usage, history_len)
|
||||
}
|
||||
|
||||
/// 末尾から `retained` トークン以上を残すための分割位置。
|
||||
///
|
||||
/// `history[..cut.index]` が要約／破棄される側、`history[cut.index..]` が残る側。
|
||||
|
|
|
|||
|
|
@ -1231,9 +1231,10 @@ impl<C: LlmClient, St: Store> Pod<C, St> {
|
|||
// session has a fresh log with no `LogEntry::Extension` entries
|
||||
// yet, so a cold restore here would set extract_pointer to None
|
||||
// via fold_pointer. The in-memory pointer must match — otherwise
|
||||
// `cumulative_input_tokens_since(old_history_len)` filters out
|
||||
// every record in the new (shorter) history and Phase 1 stops
|
||||
// firing for the rest of the process's lifetime.
|
||||
// `tokens_added_since(old_history_len)` would treat the new
|
||||
// (shorter) history as if it had already been processed, and
|
||||
// Phase 1 would stop firing for the rest of the process's
|
||||
// lifetime.
|
||||
*self
|
||||
.extract_pointer
|
||||
.lock()
|
||||
|
|
@ -1273,18 +1274,23 @@ impl<C: LlmClient, St: Store> Pod<C, St> {
|
|||
Ok(worker.client().clone_boxed())
|
||||
}
|
||||
|
||||
/// Cumulative `input_total_tokens` of usage records added after the
|
||||
/// item-count boundary `history_len_pointer`. Used by Phase 1 trigger.
|
||||
/// pointer 以降に増えたプロンプト全長の推定。Phase 1 trigger が
|
||||
/// 閾値判定に使う。
|
||||
///
|
||||
/// `history_len_pointer == 0` means "everything so far".
|
||||
fn cumulative_input_tokens_since(&self, history_len_pointer: usize) -> u64 {
|
||||
self.usage_history
|
||||
.lock()
|
||||
.expect("usage_history poisoned")
|
||||
.iter()
|
||||
.filter(|r| r.history_len > history_len_pointer)
|
||||
.map(|r| r.input_total_tokens)
|
||||
.sum()
|
||||
/// `total_tokens_at(now) - total_tokens_at(pointer)` の差分で、
|
||||
/// compact と同じ accounting (measured / interpolated / extrapolated)
|
||||
/// に乗る。`history_len_pointer == 0` は「未抽出」扱いで現プロンプト
|
||||
/// 全長そのものが返る。
|
||||
///
|
||||
/// 素朴な `usage_history.input_total_tokens` の合計は使わない:
|
||||
/// `input_total_tokens` は **送信時の prompt prefix 全長** であって
|
||||
/// 増分ではないので、長い turn 内の連続 LLM call では super-set を
|
||||
/// 何度も足し込んでしまい実消費の数倍に膨らむ。
|
||||
fn tokens_added_since(&self, history_len_pointer: usize) -> u64 {
|
||||
let now = self.history().len();
|
||||
let total_now = self.total_tokens_at(now).tokens;
|
||||
let total_at_pointer = self.total_tokens_at(history_len_pointer).tokens;
|
||||
total_now.saturating_sub(total_at_pointer)
|
||||
}
|
||||
|
||||
/// Phase 1 (memory.extract) post-run trigger.
|
||||
|
|
@ -1362,7 +1368,7 @@ impl<C: LlmClient, St: Store> Pod<C, St> {
|
|||
.map(|p| p.processed_through_history_len)
|
||||
.unwrap_or(0);
|
||||
|
||||
let tokens_since = self.cumulative_input_tokens_since(processed_history_len);
|
||||
let tokens_since = self.tokens_added_since(processed_history_len);
|
||||
if tokens_since < threshold {
|
||||
return Ok(ExtractDecision::Skipped);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@
|
|||
### Non-blocking / Follow-up
|
||||
|
||||
- **閾値の単位（cumulative `input_total_tokens` の解釈）** — `UsageRecord.input_total_tokens` は「そのリクエスト送信時のプロンプト全長」(`session-store/src/session_log.rs:147-149`) で増分ではない。pointer 以降の records を素直に sum すると、長い 1 turn 内の連続 LLM call では各 prompt が前 prompt の super-set なので合計は実トークン消費の数倍に膨らむ。ticket / 設計の「cumulative input tokens since last pointer」をどう取るかは複数解釈あるが、現実装は最もリベラル（=最も発火しやすい）解釈になっており、頻繁発火を許容する仕様前提と整合はする。今後 threshold の運用値を tune するときに「3 turn で hit する」程度の感覚と乖離するので、doc に "billed input cumulative" と明記するか、`input_total_tokens - cache_read` の差分版や、最後の record だけを見る (= 現プロンプト長) など別解釈に切り替えるか、いずれ運用観察で決める
|
||||
- **対応済み (2026-04-28)**: `Pod::tokens_added_since(history_len_pointer)` を `Pod::total_tokens_at(now) - Pod::total_tokens_at(pointer)` の差分計算に切り替え (`crates/pod/src/pod.rs:1284-1294`)。compact 側と同じ accounting (measured/interpolated/extrapolated) に乗るので、threshold 値は「pointer 以降に追加されたプロンプト全長」と素直に解釈できる。`Pod::total_tokens_at(history_len)` を `compact::token_counter` に追加 (`pub` API、将来別チケットで llm-worker に下ろす予定)。
|
||||
- **`build_extract_input` が ToolCall 引数と ToolResult 本文を落とす** — `crates/memory/src/extract/input.rs:40-45`。compact 用 `build_summary_prompt` のミラーだが、Phase 1 の `attempts` 抽出は「何を試したか」が tool 引数（書き換えた箇所、実行コマンド）に集中することが多く、tool 名だけだと "ran read_file" レベルの粒度になる。tool 結果 summary は残るので致命ではないが、Phase 1 prompt の `attempts.action` 品質を観察してから decide。共通ヘルパー化したいなら memory crate へ持って行く整理も将来検討
|
||||
- **空 payload で pointer だけ前進する設計の妥当性** — `pod.rs:1414-1424` で `payload.is_empty()` なら staging を作らず pointer だけ進める。spec §完了条件の「空配列で書き出す または skip、どちらでもよい」に合致。一方 `LogEntry::Extension` は payload を増やし続けるので、空 turn が連続すると Extension entry が積み上がる。session 寿命なので Phase 1 の頻度なら許容範囲だが、log size 監視のときに見える要素として把握しておく
|
||||
- **`write_extracted` が呼ばれずに worker が終了したケースの取り扱い** — `pod.rs:1406-1412` で warn を出して空 payload 扱い、pointer は前進。再実行で同 range を再抽出できないので、ここで pointer を前進させる選択は LLM 不安定時に情報を取りこぼす可能性がある一方、無限ループ防止という意味では合理的。仕様上は "skip でも空配列でもよい" なので問題ないが、運用で `write_extracted` 呼び忘れが頻発するなら spec 変更（pointer 据え置きで再 trigger）が選択肢
|
||||
|
|
|
|||
103
tickets/session-store-llm-worker-type-ownership.md
Normal file
103
tickets/session-store-llm-worker-type-ownership.md
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
# session-store / llm-worker 型責務の整理
|
||||
|
||||
## 背景
|
||||
|
||||
session-store の `LogEntry` 周りで「llm-worker の概念を session-store 内に二重定義 / inline flatten している」「未使用の variants が残っている」状態が複数ある。`memory-phase1-extract` の作業中に整理対象として浮上したが、本筋とは別軸なので独立チケットに切り出す。
|
||||
|
||||
依存方向は崩さない: pod → llm-worker、pod → session-store、session-store → llm-worker は維持。
|
||||
|
||||
## 要件
|
||||
|
||||
### 1. `UsageRecord` を llm-worker に移動
|
||||
|
||||
- 現状: `crates/session-store/src/session_log.rs` 内で `pub struct UsageRecord { history_len, input_total_tokens, cache_read_tokens, cache_write_tokens, output_tokens }` を定義
|
||||
- 本性: 「ある history prefix 長で 1 リクエスト送ったときの計測スナップショット」 = LLM call に紐づく per-call measurement。永続化が本質ではない
|
||||
- 移動先: `crates/llm-worker/src/usage_record.rs` (or `llm_client/usage.rs`)。`UsageEvent` (provider stream イベント) と隣接させる
|
||||
- session-store 側: `pub use llm_worker::UsageRecord` で互換 re-export。`LogEntry::LlmUsage` は inline fields のままで良い (中身が `UsageRecord` 1 個分の field 列に対応している)
|
||||
- pod 側: import 経路だけ更新
|
||||
|
||||
### 2. `token_counter` を llm-worker に移動
|
||||
|
||||
- 現状: `crates/pod/src/compact/token_counter.rs` に `prefix_bytes`, `tokens_at`, `total_tokens_impl`, `total_tokens_at_impl`, `split_for_retained_impl`, `tool_result_content_bytes` が同居
|
||||
- consumer も増えている: 当初は compact だけだったが、memory phase 1 trigger (`Pod::tokens_added_since` → `total_tokens_at(now) - total_tokens_at(pointer)` の差分計算) でも同じ accounting を使うようになった。`compact::` 名前空間下にあるのが事実とそぐわない
|
||||
- 移動方針:
|
||||
- **汎用部分** (`prefix_bytes`, `tokens_at`, `total_tokens_impl`, `total_tokens_at_impl`) を `crates/llm-worker/src/token_counter.rs` に移す。`Item` も llm-worker、`UsageRecord` も llm-worker に来るので素材が揃う
|
||||
- **compact 専用部分** (`split_for_retained_impl`, `tool_result_content_bytes`) は pod 側に残す (compact / prune だけが consumer)
|
||||
- pod 側の `Pod::total_tokens()` / `Pod::total_tokens_at()` / `Pod::split_for_retained()` メソッドは llm-worker の関数を呼ぶ薄ラッパーに (現在は `compact::token_counter::*_impl` を呼んでいる、import 経路だけが変わる)
|
||||
- これにより phase 1 trigger と将来の usage metrics が `use llm_worker::token_counter::...` で参照できるようになり、`compact::` 経由の不自然な依存が解消される
|
||||
|
||||
### 3. `Outcome` 廃止 + `LogEntry::RunCompleted` / `RunErrored` に flat 展開
|
||||
|
||||
- 現状: `crates/session-store/src/session_log.rs` の `Outcome` enum が `WorkerResult` の 4 variants (Finished / Paused / LimitReached / Yielded) を再定義した上に `Error { message: String }` を追加した形。`LogEntry::RunOutcome { outcome: Outcome, interrupted: bool }` で wrap されてる
|
||||
- 当初設計の意図 (`docs/persistence.md` の元コミット 2026-04-05): `RunOutcome` は **「audit-only metadata、replay 分岐には使わない」** と明記されていた。後から log viewer 等の consumer ができる前提で「書く側だけ整えた」状態。現在も replay は `interrupted: bool` しか参照しない (`session_log.rs:294`)
|
||||
- 問題点: WorkerResult の 4 variants が session-store 側で二重定義されている / `Outcome` 中間層が JSON / Rust 両方で余分なネストを生む / variant 名 (`RunOutcome`) と enum 名 (`Outcome`) が重複
|
||||
- 動機: pod の `handle_worker_result` で `Result<WorkerResult, WorkerError>` を 1 record に永続化する必要がある。`WorkerError` は `ClientError` (reqwest 等) を wrap していて `Serialize` 不可能なので、エラー側は `message: String` に lossy 変換するしかない (この事情は変わらない)
|
||||
- 改修方針:
|
||||
- `llm_worker::WorkerResult` に `#[derive(Serialize, Deserialize)]` + `#[serde(rename_all = "snake_case")]` を追加
|
||||
- session-store 側の `Outcome` enum を **完全削除**
|
||||
- `LogEntry::RunOutcome` を 2 variants に分解 (audit metadata の意図は保持):
|
||||
```rust
|
||||
pub enum LogEntry {
|
||||
// ...
|
||||
|
||||
/// run() / resume() が WorkerResult で正常終了した。
|
||||
/// 当初設計どおり audit-only: replay は `interrupted` のみ反映。
|
||||
RunCompleted {
|
||||
ts: u64,
|
||||
interrupted: bool,
|
||||
result: llm_worker::WorkerResult,
|
||||
},
|
||||
|
||||
/// run() / resume() が WorkerError で終了した。
|
||||
/// WorkerError は Serialize 不可なので message のみ lossy 保持。
|
||||
/// audit-only: replay は `interrupted` のみ反映。
|
||||
RunErrored {
|
||||
ts: u64,
|
||||
interrupted: bool,
|
||||
message: String,
|
||||
},
|
||||
|
||||
// ...
|
||||
}
|
||||
```
|
||||
- `save_outcome` を `save_run_completed` / `save_run_errored` の 2 関数に分割 (or `save_run_outcome(result: &Result<WorkerResult, WorkerError>)` の helper を 1 つだけ持って内部で振り分け、どちらでも)
|
||||
- pod の `match` (`pod.rs:967` 付近) を `Ok(r) => save_run_completed(.., r) / Err(e) => save_run_errored(.., e.to_string())` に
|
||||
- `collect_state` の対応 match arm 2 つに分けるが、どちらも `state.last_run_interrupted = *interrupted` だけ
|
||||
- 既存ログ互換: variant tag が変わる (`run_outcome` → `run_completed` / `run_errored`) ので JSON 形式が変わる。v1 ログを読む経路があるなら custom deserializer か migration が必要 (実運用ログがほぼ無い前提なら破壊的変更で OK、判断はチケット着手時)
|
||||
|
||||
### 4. `LogEntry::Locked` / `LogEntry::CacheUnlocked` および周辺 API を削除
|
||||
|
||||
- 現状: variants 自体は残っているが、**書き手 (caller) が存在しない**
|
||||
- `save_cache_locked` / `save_cache_unlocked` は `pub` 公開だが session-store 外の呼び出しゼロ
|
||||
- Pod は `worker.set_cache_anchor(...)` を in-memory で操作するだけで永続化していない
|
||||
- `RestoredState.locked_prefix_len` も誰も読んでいない
|
||||
- 削除対象:
|
||||
- `LogEntry::Locked` / `LogEntry::CacheUnlocked` variants
|
||||
- `save_cache_locked` / `save_cache_unlocked` 関数 (lib.rs の re-export 含む)
|
||||
- `RestoredState.locked_prefix_len` field
|
||||
- `collect_state` の対応 match arm
|
||||
- 関連 unit test
|
||||
- 既存ログ互換: 上述の通り書き手不在なので既存ログにエントリは入っていないはず。念のため `serde(other)` 等で未知 variant を skip する救済層を入れるかは判断
|
||||
|
||||
## 範囲外
|
||||
|
||||
- `LogEntry::TurnEnd` の `usize` flatten (Worker.turn_count() の永続化) — 重複というほどではないので触らない
|
||||
- pod の cache anchor 永続化を実装する話 — 必要性が出てから別途
|
||||
- session-store の独立した一般化 (memory ドメイン以外の Extension 用途展開) — 必要が出てから別途
|
||||
|
||||
## 完了条件
|
||||
|
||||
- `UsageRecord` が llm-worker から `pub` され、session-store / pod の参照経路が更新されて workspace 全テスト pass
|
||||
- token_counter の汎用部分が llm-worker 配下にあり、pod / 将来の memory phase 1 から `use llm_worker::token_counter::...` で参照できる
|
||||
- `Outcome` enum が削除され、`LogEntry::RunCompleted { result: WorkerResult }` / `LogEntry::RunErrored { message }` の 2 variants で表現される。`WorkerResult` の 4 variants は llm-worker 単一情報源
|
||||
- `Locked` / `CacheUnlocked` 関連 variants / 関数 / fields が削除されてビルド & テストが通る
|
||||
- 既存 compact / prune / phase 1 trigger の挙動に回帰がない (token accounting の数値が変わらない、Outcome serialization の往復が成立する)
|
||||
|
||||
## 参照
|
||||
|
||||
- `crates/session-store/src/session_log.rs` (LogEntry, RestoredState, UsageRecord, Outcome)
|
||||
- `crates/session-store/src/session.rs` (save_outcome, save_cache_*, save_usage)
|
||||
- `crates/llm-worker/src/worker.rs` (WorkerResult, WorkerError, set_cache_anchor)
|
||||
- `crates/pod/src/compact/token_counter.rs` (移動元)
|
||||
- `crates/pod/src/pod.rs` (handle_worker_result の Outcome 構築箇所、Pod::total_tokens 経路)
|
||||
- `docs/persistence.md` (元設計の意図: RunOutcome は audit-only)
|
||||
Loading…
Reference in New Issue
Block a user