From 2524c33a5b0d1c18783128f24eb9cfb1dc2eb466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 23 Jul 2026 10:38:37 -0400 Subject: [PATCH] fix(chat): relocate tool-result images to a batched user message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C3 (Pi openai-completions relocation): tool messages are text-only on the OpenAI chat wire — image parts inside role:tool content 400 on strict validators. Images from a consecutive tool-result run now batch into ONE synthetic user message appended after the run (a user message may not interrupt tool responses answering the same assistant's tool_calls); an image-only result carries a '(see attached image)' pointer placeholder. Applied at the typed conversion layer, so every chat dialect gets the universally-valid shape. Verified: models+sampling-types+sampler+chat-state+shell all green, clippy clean. --- .../kigi-sampling-types/src/conversation.rs | 173 ++++++++++++++---- 1 file changed, 136 insertions(+), 37 deletions(-) diff --git a/crates/codegen/kigi-sampling-types/src/conversation.rs b/crates/codegen/kigi-sampling-types/src/conversation.rs index 61e7715..5b94d4e 100644 --- a/crates/codegen/kigi-sampling-types/src/conversation.rs +++ b/crates/codegen/kigi-sampling-types/src/conversation.rs @@ -1773,31 +1773,18 @@ pub fn conversation_item_to_chat_message(item: ConversationItem) -> ChatRequestM } } ConversationItem::ToolResult(t) => { - if t.images.is_empty() { - ChatRequestMessage::tool(t.tool_call_id, t.content.as_ref().to_owned()) + // Tool messages are TEXT-ONLY on the OpenAI chat wire (the spec + // allows string/text-parts; image parts 400 on strict + // validators). Images ride a synthetic user message appended + // after the consecutive tool-result run by + // `conversation_to_chat_messages` — the Pi `openai-completions` + // relocation. An image-only result gets a pointer placeholder. + let text = if t.content.is_empty() && !t.images.is_empty() { + "(see attached image)".to_string() } else { - let mut blocks = vec![ChatContentBlock::Text { - text: t.content.as_ref().to_owned(), - }]; - for img in t.images { - if let ContentPart::Image { url } = img { - blocks.push(ChatContentBlock::ImageUrl { - image_url: ImageUrl { - url: url.as_ref().to_owned(), - }, - }); - } - } - ChatRequestMessage { - role: Role::Tool, - content: MessageContent::Blocks(blocks), - name: None, - tool_calls: Vec::new(), - tool_call_id: Some(t.tool_call_id), - model_id: None, - reasoning_content: None, - } - } + t.content.as_ref().to_owned() + }; + ChatRequestMessage::tool(t.tool_call_id, text) } // Backend tool calls have no Chat Completions equivalent. // Emit a synthetic assistant message so the model sees context @@ -1839,16 +1826,44 @@ pub fn conversation_item_to_chat_message(item: ConversationItem) -> ChatRequestM pub fn conversation_to_chat_messages(items: Vec) -> Vec { let mut out: Vec = Vec::with_capacity(items.len()); let mut pending_reasoning: Vec = Vec::new(); + // Images from the current consecutive tool-result run. Tool messages + // are text-only on the OpenAI chat wire, and a user message may not + // interrupt the run (tool messages must directly follow their + // assistant's tool_calls), so images batch here and flush as ONE + // synthetic user message after the run — Pi's `openai-completions` + // relocation pattern. + let mut pending_tool_images: Vec = Vec::new(); + let flush_tool_images = |pending: &mut Vec, + out: &mut Vec| { + if pending.is_empty() { + return; + } + let mut blocks = vec![ChatContentBlock::Text { + text: "Attached image(s) from tool result:".to_string(), + }]; + blocks.append(pending); + out.push(ChatRequestMessage { + role: Role::User, + content: MessageContent::Blocks(blocks), + name: None, + tool_calls: Vec::new(), + tool_call_id: None, + model_id: None, + reasoning_content: None, + }); + }; for item in items { match item { ConversationItem::Reasoning(r) => { + flush_tool_images(&mut pending_tool_images, &mut out); let text = reasoning_item_text(&r); if !text.is_empty() { pending_reasoning.push(text); } } ConversationItem::Assistant(_) => { + flush_tool_images(&mut pending_tool_images, &mut out); let mut msg = conversation_item_to_chat_message(item); if !pending_reasoning.is_empty() { msg.reasoning_content = Some(pending_reasoning.join("\n")); @@ -1864,6 +1879,21 @@ pub fn conversation_to_chat_messages(items: Vec) -> Vec { + // Collect the run's images before the text-only conversion. + for img in &t.images { + if let ContentPart::Image { url } = img { + pending_tool_images.push(ChatContentBlock::ImageUrl { + image_url: ImageUrl { + url: url.as_ref().to_owned(), + }, + }); + } + } + pending_reasoning.clear(); out.push(conversation_item_to_chat_message(item)); } other => { @@ -1871,11 +1901,13 @@ pub fn conversation_to_chat_messages(items: Vec) -> Vec = msgs.iter().map(|m| m.role).collect(); + assert_eq!( + roles, + vec![ + Role::User, + Role::Assistant, + Role::Tool, + Role::Tool, + Role::User, + Role::Assistant + ], + "images flush as ONE user message after the tool run: {msgs:#?}" + ); + // Tool messages are text-only; the image-only result carries the + // pointer placeholder. + assert!(matches!(&msgs[2].content, MessageContent::Text(t) if t == "(see attached image)")); + assert!(matches!(&msgs[3].content, MessageContent::Text(t) if t == "text out")); + // The batched user message carries the lead text + BOTH images. + let MessageContent::Blocks(blocks) = &msgs[4].content else { + panic!("image carrier must be a blocks message: {msgs:#?}"); + }; + assert_eq!(blocks.len(), 3); + assert!( + matches!(&blocks[0], ChatContentBlock::Text { text } if text.starts_with("Attached image")) + ); + let urls: Vec<&str> = blocks[1..] + .iter() + .filter_map(|b| match b { + ChatContentBlock::ImageUrl { image_url } => Some(image_url.url.as_str()), + _ => None, + }) + .collect(); + assert_eq!( + urls, + vec!["data:image/png;base64,AAA", "data:image/png;base64,BBB"] + ); + } + #[test] fn conversation_to_chat_messages_folds_reasoning_into_following_assistant() { let items = vec![