Skip to content

fix: keep tool results consecutive when OpenAI-compatible tool images are emitted - #12233

Open
shoemoney wants to merge 1 commit into
aaif-goose:mainfrom
shoemoney:fix/openai-defer-tool-image-messages
Open

shoemoney wants to merge 1 commit into
aaif-goose:mainfrom
shoemoney:fix/openai-defer-tool-image-messages

Conversation

@shoemoney

@shoemoney shoemoney commented Sep 18, 2026

Copy link
Copy Markdown

Closes #11893

Summary

Issue #11893 is Ready on the Goose Issues board (moved out of Inbox by @jamadeo on 2026-09-09).

format_messages_with_options declares and flushes image_messages inside the per-content-block loop, so each image-bearing tool result is immediately followed by its synthetic role: "user" image message. All tool results of one batch live in a single Message::user(), so two image-returning tools in one assistant response serialize as:

assistant: tool_calls=[call_a, call_b]
tool: call_a
user: image_a
tool: call_b
user: image_b

Strict OpenAI-compatible endpoints reject that with Bad request (400): An assistant message with 'tool_calls' must be followed by tool messages responding to each 'tool_call_id'. The shape is persisted in the transcript, so every later turn re-sends it and fails identically.

This implements the two-phase emit @jamadeo specified on the issue: emit every role: "tool" message first in tool-call order, then the synthetic role: "user" image messages. It ports the pending_image_messages deferral that formats/databricks.rs already has (merged in #9241) into the sibling formatter, which never got it. The March attempts #7393 and #7450 touched both files and were closed; #9241 landed the Databricks half alone.

The production change is four lines in one file: declare pending_image_messages beside output (message-scoped instead of content-block-scoped), push into it, and output.append(&mut pending_image_messages) once the content loop finishes. output.insert(0, converted) still puts the message itself ahead of everything, so ordering downstream is unchanged.

Testing

The verification plan on the issue is that the formatter must put all tool results of a batch before the synthetic image messages, with tool-call order preserved.

Two regression tests were added to formats::openai::tests, mirroring test_parallel_tool_responses_with_images_are_consecutive and test_mixed_tool_responses_image_and_text_ordering that already exist in formats/databricks.rs. The openai formatter had no equivalent coverage.

Tests were written first and confirmed failing on pristine main at d57c9a4, before the source change:

running 2 tests
test formats::openai::tests::test_parallel_tool_responses_with_images_are_consecutive ... FAILED
test formats::openai::tests::test_mixed_tool_responses_image_and_text_ordering ... FAILED

---- formats::openai::tests::test_parallel_tool_responses_with_images_are_consecutive stdout ----
assertion `left == right` failed
  left: ["assistant", "tool", "user", "tool", "user"]
 right: ["assistant", "tool", "tool", "user", "user"]

---- formats::openai::tests::test_mixed_tool_responses_image_and_text_ordering stdout ----
assertion `left == right` failed
  left: ["assistant", "tool", "user", "tool", "tool", "user"]
 right: ["assistant", "tool", "tool", "tool", "user", "user"]

test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 607 filtered out

The left side is exactly the wire order reported in the issue.

With the fix applied:

$ cargo test -p goose-provider-types --lib formats::openai
running 186 tests
test formats::openai::tests::test_merge_split_tool_calls_with_image_gap ... ok
test formats::openai::tests::test_mixed_tool_responses_image_and_text_ordering ... ok
test formats::openai::tests::test_parallel_tool_responses_with_images_are_consecutive ... ok
test result: ok. 186 passed; 0 failed; 0 ignored; 0 measured; 423 filtered out

$ cargo test -p goose-provider-types
running 609 tests
test result: ok. 609 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
     Running tests/prefix_invariance.rs
running 8 tests
test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ cargo clippy -p goose-provider-types --all-targets -- -D warnings
    Finished `dev` profile

$ cargo fmt --check -p goose-provider-types
(no output)

$ cargo check -p goose-providers
    Finished `dev` profile

@GreenTea321 flagged on the issue that test_merge_split_tool_calls_with_image_gap might have to be inverted because it asserts the interleaved order. It does not: that test calls merge_split_tool_call_messages directly on a hand-built Vec<Value>, so it is independent of the emit site. It passes unchanged, as shown above.

Deliberately not covered: this is the emit-site reordering only. merge_split_tool_call_messages and is_image_only_user_message are untouched, and a session whose transcript already contains the bad interleaving is not repaired retroactively, the shape just stops being produced. Non-vision models and single-image tool results serialize byte-identically, since no synthetic image message is created in those cases. This is a formatter change rather than an agent-loop change, so the agent.rs / state_machine parity requirement does not apply.

Also not verified here: the live 400 against a real OpenAI-compatible vision endpoint. The reporter reproduced that on macOS 1.49.0 and @GreenTea321 independently on Windows 1.50.0; this PR verifies the wire shape those reports blame.

Related Issues

Discussion: #11893. Prior art: #9241 (merged, Databricks half), #7393 and #7450 (closed), #7400.

… are emitted

A tool result carrying an image is followed by a synthetic role:"user"
image message. That message was flushed inside the per-content-block loop,
so with two image-returning tools in one assistant tool_calls batch the
request serialized as tool, user, tool, user. Strict OpenAI-compatible
endpoints reject that with "An assistant message with 'tool_calls' must be
followed by tool messages responding to each 'tool_call_id'", and because
the shape is persisted in the transcript every later turn fails the same
way, so the session stays broken.

Defer the image messages to the end of the message, matching the
pending_image_messages handling already in the Databricks formatter.

Fixes aaif-goose#11893
@shoemoney
shoemoney requested a review from a team as a code owner September 18, 2026 18:42
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.

OpenAI-compatible provider: tool image messages interrupt parallel tool results and cause HTTP 400

1 participant