update: Reasoningコンテキスト管理のレビュー・対応
This commit is contained in:
parent
594671edc3
commit
75c573fed1
1
TODO.md
1
TODO.md
|
|
@ -11,6 +11,7 @@
|
|||
- llm-worker のエラー耐性
|
||||
- ストリーム途中失敗時の継続 → [tickets/llm-worker-stream-continuation.md](tickets/llm-worker-stream-continuation.md)
|
||||
- llm-worker: reasoning ブロックを history に永続化 (Anthropic signature 保持・ツール使用ループでの round-trip) → [tickets/reasoning-history-persist.md](tickets/reasoning-history-persist.md)
|
||||
- llm-worker: Anthropic projection で assistant ターン内ブロックを 1 message に束ねる → [tickets/anthropic-assistant-burst-bundling.md](tickets/anthropic-assistant-burst-bundling.md)
|
||||
- ネイティブ GUI クライアント MVP → [tickets/native-gui-mvp.md](tickets/native-gui-mvp.md)
|
||||
- TUI 拡充
|
||||
- Run 中の入力キューイング → [tickets/tui-input-queue.md](tickets/tui-input-queue.md)
|
||||
|
|
|
|||
|
|
@ -868,6 +868,88 @@ mod tests {
|
|||
assert!(collect_assistant_thinking_parts(&req).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thinking_part_lands_in_assistant_role_message() {
|
||||
// wire 構造の position 検証: thinking part は assistant role の
|
||||
// message 配列に並ぶ（user role には絶対に入らない）。
|
||||
let scheme = AnthropicScheme::new();
|
||||
let request = Request::new()
|
||||
.user("question?")
|
||||
.item(Item::reasoning("thinking inside").with_signature("SIG-A"))
|
||||
.item(Item::tool_call("c1", "tool_a", "{}"))
|
||||
.item(Item::tool_result("c1", "result"))
|
||||
.user("follow up");
|
||||
let req = scheme.build_request("claude-sonnet-4-20250514", &request, &cap_explicit());
|
||||
|
||||
// 全 thinking part が assistant role の message に存在すること
|
||||
let mut thinking_msg_indices = Vec::new();
|
||||
for (i, msg) in req.messages.iter().enumerate() {
|
||||
if let AnthropicContent::Parts(parts) = &msg.content {
|
||||
if parts
|
||||
.iter()
|
||||
.any(|p| matches!(p, AnthropicContentPart::Thinking { .. }))
|
||||
{
|
||||
assert_eq!(
|
||||
msg.role, "assistant",
|
||||
"thinking part must be in assistant role, got {} at msg {}",
|
||||
msg.role, i,
|
||||
);
|
||||
thinking_msg_indices.push(i);
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
!thinking_msg_indices.is_empty(),
|
||||
"expected at least one thinking part in messages: {:?}",
|
||||
req.messages,
|
||||
);
|
||||
|
||||
// thinking part を含む assistant message は、それに続く tool_use を含む
|
||||
// assistant message より前 (= 先頭側) に位置すること
|
||||
// (Anthropic 仕様: 同一論理ターン内で thinking → tool_use の順)
|
||||
let mut tool_use_msg_indices = Vec::new();
|
||||
for (i, msg) in req.messages.iter().enumerate() {
|
||||
if let AnthropicContent::Parts(parts) = &msg.content {
|
||||
if parts
|
||||
.iter()
|
||||
.any(|p| matches!(p, AnthropicContentPart::ToolUse { .. }))
|
||||
{
|
||||
tool_use_msg_indices.push(i);
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(!tool_use_msg_indices.is_empty(), "expected tool_use part");
|
||||
let first_thinking = thinking_msg_indices[0];
|
||||
let first_tool_use = tool_use_msg_indices[0];
|
||||
assert!(
|
||||
first_thinking <= first_tool_use,
|
||||
"thinking msg ({}) must precede tool_use msg ({})",
|
||||
first_thinking,
|
||||
first_tool_use,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redacted_thinking_part_lands_in_assistant_role_message() {
|
||||
// RedactedThinking も同様に assistant role に置かれること。
|
||||
let scheme = AnthropicScheme::new();
|
||||
let request = Request::new()
|
||||
.user("ask")
|
||||
.item(Item::reasoning("").with_encrypted_content("opaque"))
|
||||
.item(Item::tool_call("c1", "tool_a", "{}"))
|
||||
.item(Item::tool_result("c1", "ok"));
|
||||
let req = scheme.build_request("claude-sonnet-4-20250514", &request, &cap_explicit());
|
||||
for msg in &req.messages {
|
||||
if let AnthropicContent::Parts(parts) = &msg.content {
|
||||
for part in parts {
|
||||
if matches!(part, AnthropicContentPart::RedactedThinking { .. }) {
|
||||
assert_eq!(msg.role, "assistant");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_definitions_carry_no_cache_control() {
|
||||
// Tool JSON schema must serialise unchanged — no sneak-in of
|
||||
|
|
|
|||
|
|
@ -286,6 +286,48 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn round_trip_reasoning_preserves_signature() {
|
||||
// 新世代 Claude の thinking signature が history.json に永続化され、
|
||||
// resume 後の Item::Reasoning に復元されること。
|
||||
let original = Item::reasoning("inner thought").with_signature("SIG-OPUS-XYZ");
|
||||
let logged: LoggedItem = (&original).into();
|
||||
let json = serde_json::to_string(&logged).unwrap();
|
||||
// wire 形式に signature キーが乗ること（古い形式との互換のため
|
||||
// 値が None のときは省略される。Some の値は載る）
|
||||
assert!(
|
||||
json.contains("SIG-OPUS-XYZ"),
|
||||
"serialised JSON must carry signature: {json}",
|
||||
);
|
||||
let parsed: LoggedItem = serde_json::from_str(&json).unwrap();
|
||||
match Item::from(parsed) {
|
||||
Item::Reasoning {
|
||||
text, signature, ..
|
||||
} => {
|
||||
assert_eq!(text, "inner thought");
|
||||
assert_eq!(signature.as_deref(), Some("SIG-OPUS-XYZ"));
|
||||
}
|
||||
other => panic!("unexpected variant: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_reasoning_without_signature_field_deserializes() {
|
||||
// signature フィールドが無い旧形式の history.json を読み込んでも
|
||||
// None としてロードできる（後方互換性）。
|
||||
let legacy_json = r#"{"kind":"reasoning","text":"old","summary":[],"encrypted_content":null}"#;
|
||||
let parsed: LoggedItem = serde_json::from_str(legacy_json).unwrap();
|
||||
match Item::from(parsed) {
|
||||
Item::Reasoning {
|
||||
text, signature, ..
|
||||
} => {
|
||||
assert_eq!(text, "old");
|
||||
assert!(signature.is_none());
|
||||
}
|
||||
other => panic!("unexpected variant: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn round_trip_tool_result_with_content() {
|
||||
let original = Item::tool_result_with_content("call_1", "ok", "full output");
|
||||
|
|
|
|||
31
tickets/anthropic-assistant-burst-bundling.md
Normal file
31
tickets/anthropic-assistant-burst-bundling.md
Normal file
|
|
@ -0,0 +1,31 @@
|
|||
# Anthropic projection: assistant ターン内ブロックを 1 message に束ねる
|
||||
|
||||
## 背景
|
||||
|
||||
`crates/llm-worker/src/llm_client/scheme/anthropic/request.rs` の `convert_items_to_messages` は、Worker が 1 ターンで生成する `[Reasoning, assistant_message, ToolCall]` の連列を、Anthropic wire 上で **複数の隣接した assistant message** に分割している。
|
||||
|
||||
具体的には:
|
||||
- `Item::Reasoning` を `pending_assistant` に push
|
||||
- 次の `Item::Message { Role::Assistant }` が到来すると `pending_assistant` を flush し、自分自身は別 message として messages に直 push
|
||||
- 続く `Item::ToolCall` は再び `pending_assistant` に積まれ、turn 末で flush され 3 つ目の assistant message に
|
||||
|
||||
結果として 1 turn が `assistant[Thinking] / assistant[text] / assistant[tool_use]` の 3 message に展開される。
|
||||
|
||||
Anthropic Messages API は user/assistant の交互を要求し、同一論理 turn 内の thinking/text/tool_use は **1 つの assistant message の `content` 配列** に並べる仕様。新世代 Claude (Opus 4.5+/Sonnet 4.6+) で thinking signature を round-trip する際、隣接 assistant message に分かれていると signature の文脈が崩れて 400 になる懸念がある（reasoning-history-persist のレビュー指摘）。
|
||||
|
||||
なお、本バグは reasoning-history-persist で導入されたものではなく、`assistant_message` + `tool_call` の組合せで以前から存在していた pre-existing な分割。Reasoning が同じ flush 経路を継承した形。
|
||||
|
||||
## 要件
|
||||
|
||||
- 同一論理ターンに属する `Item::Reasoning` / `Item::Message(Assistant)` / `Item::ToolCall` を、Anthropic wire 上の **1 つの assistant message の `content` 配列** に束ねる
|
||||
- 順序は arrival 順 (= history 順)。Anthropic 仕様の典型は thinking → text → tool_use
|
||||
- user / system role の `Item::Message` や `Item::ToolResult` を境界として assistant burst を区切る
|
||||
- 既存の breakpoint (cache_control) 計算が壊れないこと: 各 item のオリジン index → (msg_idx, part_idx) マッピングは flush_pending 経由で記録されているので、Item::Message(Assistant) も pending を経由するように揃えれば自然に追従する
|
||||
- Single-text 専用の `AnthropicContent::Text` shorthand は assistant burst 内 1 part のみのときに限定して維持するか、簡潔さのために常に `Parts` 形式に統一するかは実装時に判断
|
||||
- 既存テスト群（`completed_turn`, `single_text_message_uses_text_shorthand_without_breakpoint`, `breakpoint_on_tool_result_head` 等）の意図を逸脱しないよう更新
|
||||
|
||||
## スコープ外
|
||||
|
||||
- モデル世代別の thinking keep/strip デフォルト分岐（reasoning-history-persist のフォローアップ候補と同じ扱い）
|
||||
- `clear_thinking_20251015` context-edit
|
||||
- prune.rs の reasoning aware 化
|
||||
|
|
@ -32,3 +32,8 @@ CLAUDE.md の「LLM コンテキスト加工原則」に照らしても、thinki
|
|||
- Anthropic `clear_thinking_20251015` context-editing 戦略の実装
|
||||
- `prune.rs` の reasoning aware 化 (古い reasoning の選択的剥離)
|
||||
- Ollama scheme の `think` パラメータ対応 (そもそも Ollama scheme 自体が未実装)
|
||||
|
||||
## Review
|
||||
- 状態: Approve with follow-up
|
||||
- レビュー詳細: [./reasoning-history-persist.review.md](./reasoning-history-persist.review.md)
|
||||
- 日付: 2026-05-04
|
||||
|
|
|
|||
64
tickets/reasoning-history-persist.review.md
Normal file
64
tickets/reasoning-history-persist.review.md
Normal file
|
|
@ -0,0 +1,64 @@
|
|||
# Review: reasoning ブロックを history に永続化する
|
||||
|
||||
## 前提・要件の確認
|
||||
|
||||
- **streaming 層で受信した reasoning が `Item::Reasoning` として `worker.history` に append される**
|
||||
- 満たされている。`crates/llm-worker/src/llm_client/scheme/anthropic/events.rs:373-389` で `content_block_stop` 時に `pending_thinking` から `Event::ReasoningItem` を発火、`worker.rs:1006-1011` で `reasoning_item_collector.take_collected()` → `build_assistant_items` 経由で history に append。`tests/reasoning_round_trip_test.rs:25-62` が end-to-end で検証。
|
||||
- **Anthropic `signature` の round-trip 保持**
|
||||
- 満たされている。受信側は `events.rs:350-358` で `signature_delta` を蓄積、`scheme_impl.rs:36-52` で `PendingThinking` に格納、`Item::Reasoning::signature` (`types.rs:100-105`) として保持。送信側は `request.rs:340-367` で `signature` を見て `AnthropicContentPart::Thinking` を投影。`signature` 無し + `encrypted_content` あり → `RedactedThinking`、両方無し → drop の分岐も妥当。
|
||||
- **ツール使用ループ内の reasoning 引き継ぎ**
|
||||
- 達成されている。Item として history に乗っているので、次の `build_request` でそのまま items に含まれる。ただし下記「アーキテクチャ・スコープ」参照: 同一論理ターン内の thinking が wire 上で同一 assistant message に纏まらない既存のメッセージ束ね方の問題が残る。
|
||||
- **通常のマルチターン (新しいユーザー入力をまたぐ) での引き継ぎ**
|
||||
- 同上。Item として残るので保持される。世代別 keep/strip はスコープ外として明記済みで意図通り。
|
||||
- **`Worker::on_thinking_block` callback 互換性**
|
||||
- 維持されている。`Event::ReasoningItem` は `BlockStart/Delta/Stop(Thinking)` と並行発火する設計 (`event.rs:48-58`、`anthropic/events.rs:336-348`、`openai_responses/events.rs:458-471`)。`pod/src/controller.rs:216` の既存 thinking callback が引き続き機能することを確認。
|
||||
- **OpenAI Responses の `encrypted_content` / `summary` を活用**
|
||||
- `openai_responses/events.rs:30-74` で `pending_reasoning` に蓄積、`output_item.done` (Reasoning) で `Event::ReasoningItem` 発火 (`events.rs:366-395`)。送信側は既存の `request.rs:286-309` が `Item::Reasoning` を `InputItem::Reasoning` に投影しており、`encrypted_content` も伝搬する。
|
||||
- **resume 時の reasoning 再現**
|
||||
- `session-store/src/logged_item.rs:36-46, 95-108, 147-159` で `LoggedItem::Reasoning` に `signature` を追加し、永続化スキーマ側でも保持される。`session.rs:206-211` の assistant 群グルーピングにも `is_reasoning()` が含まれており LogEntry::AssistantItems に正しく束ねられる。`tests/reasoning_round_trip_test.rs:142-211` が injected reasoning が outgoing request に乗ることを確認。
|
||||
|
||||
## アーキテクチャ・スコープ
|
||||
|
||||
- **CLAUDE.md「LLM コンテキスト加工原則」遵守**
|
||||
- reasoning を context に差し込むのではなく、必ず `worker.history` に commit してから次リクエストの items に出す設計になっており、原則に沿う。
|
||||
- **layer 分離**
|
||||
- llm-worker 内で完結しており低レベル基盤に閉じている。Pod / 上位層への漏れなし。
|
||||
- **新規モジュールの妥当性**
|
||||
- `timeline/reasoning_item_collector.rs` 新設は `text_block_collector` / `tool_call_collector` と対称な構造で、責務分割が明確。`Event::ReasoningItem` を独立イベントにし、streaming 表示用 `BlockType::Thinking` と persist 用 `ReasoningItem` を別経路にした責務分離も合理的。
|
||||
- **scheme 側の `parse_with_state` 導入**
|
||||
- Anthropic は `content_block_stop` が block_type を持たない都合で既に state が必要だったので、`pending_thinking` の蓄積を同じ場所に置く判断は自然。`signature_delta` をストリーム表示には流さず state にだけ蓄積する選択も合理的。
|
||||
- **`build_assistant_items` の順序 (Reasoning → text → tool_call)**
|
||||
- Anthropic 仕様 (thinking はアシスタントメッセージの先頭) を意識した並びは妥当。ただし下記 Blocking 参照。
|
||||
- **不必要な実装は見当たらない**
|
||||
- 各変更点はチケット要件と直結しており、scope creep なし。スコープ外 3 件 (世代別 keep/strip、`clear_thinking_20251015`、prune-aware) は明示されており触られていない。
|
||||
|
||||
## 指摘事項
|
||||
|
||||
### Blocking
|
||||
|
||||
なし。Anthropic 側の wire 表現に懸念 (Non-blocking 参照) があるが、本チケットで「ある reasoning item が history に commit され、次リクエストの `Request::items` に signature 付きで載る」という最小要件は達成されており、先行マージしてフォローアップで対応する判断は妥当。
|
||||
|
||||
### Non-blocking / Follow-up
|
||||
|
||||
- **Anthropic で同一論理ターンの content blocks が複数の assistant messages に分割される**
|
||||
`crates/llm-worker/src/llm_client/scheme/anthropic/request.rs:242-301` の `convert_items_to_messages` は、`Item::Reasoning` (assistant pending) と `Item::Message{Assistant}` (text) が連続するとき、前者を一度 flush して別 message にしてから text を別 message として追加する。結果として `messages = [..., assistant[Thinking], assistant("text"), assistant[ToolUse], ...]` のように同一論理ターンが3つの assistant message に割れる。Anthropic 仕様上、thinking blocks は最終 text/tool_use と**同一 assistant message の content 配列の先頭**に並ぶ必要があるので、新世代モデル (Opus 4.5+/Sonnet 4.6+) でツール使用ループに入ったとき signature 検証や thinking continuity が壊れる可能性がある。
|
||||
- これは pre-existing な分割パターン (text + tool_call の時点で既に複数 assistant message を生む) を Reasoning が継承した形であり、現行 production が動いている以上 Anthropic 側で許容されている可能性もある。ただし thinking signature の round-trip という本チケットの中核要件にとっては実 wire 上のリスクが残る。
|
||||
- 対応案: `convert_items_to_messages` で「連続する assistant 系 Item (Reasoning + assistant_message + ToolCall) は一つの assistant message に統合する」事後マージを入れる、もしくは `Item::Message{Assistant}` 受信時に `pending_assistant` を flush せず content_part を pending に追加してから flush する形に変える。
|
||||
|
||||
- **wire 構造を検証するテストが薄い**
|
||||
Anthropic 側の `reasoning_with_signature_projects_thinking_part` (`request.rs:817-838`) は thinking part の存在のみ検証し、message 配列内の位置・隣接 message との関係をチェックしていない。上記 follow-up と合わせて「Reasoning + assistant_message + ToolCall を含む history が単一 assistant message にまとまる」アサーションを足すと回帰検出に有効。
|
||||
|
||||
- **session-store: `signature` の round-trip テストが無い**
|
||||
`logged_item.rs:267-287` の `round_trip_reasoning_preserves_encrypted_content` は signature を検証していない。フィールドを追加した以上、JSON シリアライズ → デシリアライズで `signature` が保持されることを 1 ケース足しておくと安全 (現状実装は通るはずだが、将来 `LoggedItem::Reasoning` をいじったときの保険)。
|
||||
|
||||
- **`flush_usage` 直後の `Event::ReasoningItem` 順序**
|
||||
Anthropic では `content_block_stop` で BlockStop と同時に ReasoningItem が発火する。Worker 側は streaming 完了後にまとめて collector を `take_collected()` するので順序問題はないが、scheme 側で 1 回の `parse_with_state` から 2 イベントが返るパスは新しいので、念のため将来的に `Vec<Event>` のサイズ前提で組まれた呼び出し側 (テスト含め) が無いか軽く確認しておくと安心。
|
||||
|
||||
### Nits
|
||||
|
||||
- `crates/llm-worker/src/llm_client/event.rs:54` のコメント「上位層（Worker / ReasoningItemCollector）はこれを `Item::Reasoning` として `worker.history` に append する。」 — これは「Worker が ReasoningItemCollector 経由で取り出して」の方が実装と整合する (collector 自体は append しない)。
|
||||
- `worker.rs:144-145` で `reasoning_item_collector` 自体は `Worker` に保持されているが、外部公開メソッドが無い (collect は内部のみ)。`text_block_collector` / `tool_call_collector` も同パターンなので踏襲としては妥当。
|
||||
|
||||
## 判断
|
||||
|
||||
**Approve with follow-up** — 本チケットの要件 (signature を含む reasoning material を `Item::Reasoning` として history に commit し、scheme の送受信両方向で round-trip させる) は満たされており、tests・既存テストともパス。Anthropic wire 上で同一論理ターンが複数 assistant message に分割される件は要 follow-up だが、これは pre-existing なメッセージ束ね方の問題を Reasoning が継承した形で、本チケット範囲を超える。別チケット (世代別 keep/strip と合わせるのが自然) で対応するのが適切。
|
||||
Loading…
Reference in New Issue
Block a user