Skip to content

Conversation

@alexhancock
Copy link
Collaborator

We weren't properly handling when messages have multiple text chunks in providers/formats/openai.rs

This surfaced in #5626 when encountering a message with content like this:

"content": [
  {
    "type": "text",
    "text": "--- Resource: file:///Users/pjv/development/projects/test%20python%20project/test2.py ---\nfrom typing import NamedTuple\n\n\nclass Person(NamedTuple):\n  name: str\n  age: int | None = None\n\n\nperson = Person(\"Alice\", 25)\n\n---\n"
  },
  {
    "type": "text",
    "text": " what does this code do?"
  }
],

Previously when a message contained multiple text content chunks, we would overwrite the earlier ones and replace them with the last one. So this message what does this code do? would lead to an answer like what code?

Example

Before:

Screenshot 2025-11-21 at 3 26 02 PM

After:

Screenshot 2025-11-21 at 3 29 51 PM

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where messages containing multiple text content blocks were incorrectly handled in the OpenAI format provider. Previously, when multiple text chunks were present in a single message, only the last chunk would be retained. The fix accumulates all text parts and joins them with newlines, ensuring all content is preserved.

Key Changes:

  • Accumulate text blocks in a text_parts vector instead of overwriting converted["content"]
  • Join accumulated text blocks with newlines after processing all content
  • Add test coverage for multiple text block scenarios

Comment on lines 72 to 77
if let Some(image_path) = detect_image_path(&text.text) {
// Try to load and convert the image
if let Ok(image) = load_image_file(image_path) {
converted["content"] = json!([
{"type": "text", "text": text.text},
convert_image(&image, image_format)
]);
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When an image path is detected and successfully loaded (lines 72-77), this code directly assigns to converted["content"], which will overwrite any previously accumulated text_parts. This breaks the fix for multiple text content chunks.

If a message has multiple text blocks and one contains an image path, only the text with the image will be included, losing the other text blocks. Consider accumulating the text in text_parts and handling the image separately, or restructuring to build the content array incrementally.

Copilot uses AI. Check for mistakes.
@alexhancock
Copy link
Collaborator Author

Closing in favor of #5839 which had some similar code to fix images in acp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants