fix(media): preserve tool-message ordering for multi-tool-call scenarios
Move synthetic user message (carrying base64 tool images) to after the entire contiguous tool-message block instead of immediately after each tool message. This preserves the assistant→tool→tool ordering required by OpenAI-compatible APIs. Also fix load_image to use generic [image: photo] placeholder so injectPathTags can properly replace it with the actual path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
cfb6c76537
commit
c0d8568af9
3 changed files with 76 additions and 13 deletions
|
|
@ -24,7 +24,8 @@ import (
|
||||||
// For user messages: images get path tags only ([image:/path]) so the LLM
|
// For user messages: images get path tags only ([image:/path]) so the LLM
|
||||||
// can decide whether to view them via load_image or operate on the file.
|
// can decide whether to view them via load_image or operate on the file.
|
||||||
// For tool messages: images are base64-encoded and appended as a synthetic
|
// For tool messages: images are base64-encoded and appended as a synthetic
|
||||||
// user message (many APIs don't support image_url in tool messages).
|
// user message after the contiguous tool-message block ends, preserving
|
||||||
|
// the required assistant→tool ordering for LLM APIs.
|
||||||
// Non-image files always get path tags regardless of role.
|
// Non-image files always get path tags regardless of role.
|
||||||
// Returns a new slice; original messages are not mutated.
|
// Returns a new slice; original messages are not mutated.
|
||||||
func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxSize int) []providers.Message {
|
func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxSize int) []providers.Message {
|
||||||
|
|
@ -33,17 +34,36 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS
|
||||||
}
|
}
|
||||||
|
|
||||||
result := make([]providers.Message, 0, len(messages))
|
result := make([]providers.Message, 0, len(messages))
|
||||||
|
var pendingToolImages []string
|
||||||
|
|
||||||
|
for idx, m := range messages {
|
||||||
|
// When leaving a tool-message block, flush any accumulated images
|
||||||
|
// as a synthetic user message.
|
||||||
|
if m.Role != "tool" && len(pendingToolImages) > 0 {
|
||||||
|
result = append(result, providers.Message{
|
||||||
|
Role: "user",
|
||||||
|
Content: "[Loaded image from tool result above]",
|
||||||
|
Media: pendingToolImages,
|
||||||
|
})
|
||||||
|
pendingToolImages = nil
|
||||||
|
}
|
||||||
|
|
||||||
for _, m := range messages {
|
|
||||||
if len(m.Media) == 0 {
|
if len(m.Media) == 0 {
|
||||||
result = append(result, m)
|
result = append(result, m)
|
||||||
|
if idx == len(messages)-1 && len(pendingToolImages) > 0 {
|
||||||
|
result = append(result, providers.Message{
|
||||||
|
Role: "user",
|
||||||
|
Content: "[Loaded image from tool result above]",
|
||||||
|
Media: pendingToolImages,
|
||||||
|
})
|
||||||
|
pendingToolImages = nil
|
||||||
|
}
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
msg := m
|
msg := m
|
||||||
resolved := make([]string, 0, len(m.Media))
|
resolved := make([]string, 0, len(m.Media))
|
||||||
var pathTags []string
|
var pathTags []string
|
||||||
var toolImageDataURLs []string
|
|
||||||
|
|
||||||
for _, ref := range m.Media {
|
for _, ref := range m.Media {
|
||||||
if !strings.HasPrefix(ref, "media://") {
|
if !strings.HasPrefix(ref, "media://") {
|
||||||
|
|
@ -72,13 +92,10 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS
|
||||||
mime := detectMIME(localPath, meta)
|
mime := detectMIME(localPath, meta)
|
||||||
pathTags = append(pathTags, buildPathTag(mime, localPath))
|
pathTags = append(pathTags, buildPathTag(mime, localPath))
|
||||||
|
|
||||||
// For tool results (e.g. load_image), base64-encode images into a
|
|
||||||
// separate user message — many LLM APIs don't support image_url in
|
|
||||||
// tool messages.
|
|
||||||
if m.Role == "tool" && strings.HasPrefix(mime, "image/") {
|
if m.Role == "tool" && strings.HasPrefix(mime, "image/") {
|
||||||
dataURL := encodeImageToDataURL(localPath, mime, info, maxSize)
|
dataURL := encodeImageToDataURL(localPath, mime, info, maxSize)
|
||||||
if dataURL != "" {
|
if dataURL != "" {
|
||||||
toolImageDataURLs = append(toolImageDataURLs, dataURL)
|
pendingToolImages = append(pendingToolImages, dataURL)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -89,14 +106,14 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS
|
||||||
}
|
}
|
||||||
result = append(result, msg)
|
result = append(result, msg)
|
||||||
|
|
||||||
// Append a synthetic user message carrying the image data so the LLM
|
// If this is the last message and we have pending images, flush them.
|
||||||
// can see it (tool messages don't support image_url in most APIs).
|
if idx == len(messages)-1 && len(pendingToolImages) > 0 {
|
||||||
if len(toolImageDataURLs) > 0 {
|
|
||||||
result = append(result, providers.Message{
|
result = append(result, providers.Message{
|
||||||
Role: "user",
|
Role: "user",
|
||||||
Content: "[Loaded image from tool result above]",
|
Content: "[Loaded image from tool result above]",
|
||||||
Media: toolImageDataURLs,
|
Media: pendingToolImages,
|
||||||
})
|
})
|
||||||
|
pendingToolImages = nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -4746,6 +4746,52 @@ func TestResolveMediaRefs_ToolRoleImageAppendedAsUserMessage(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestResolveMediaRefs_MultiToolCallPreservesOrdering(t *testing.T) {
|
||||||
|
store := media.NewFileMediaStore()
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
// Create image for tool #1
|
||||||
|
pngPath := filepath.Join(dir, "loaded.png")
|
||||||
|
pngHeader := []byte{
|
||||||
|
0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, // PNG signature
|
||||||
|
0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, // IHDR
|
||||||
|
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02,
|
||||||
|
0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xDE,
|
||||||
|
}
|
||||||
|
os.WriteFile(pngPath, pngHeader, 0o644)
|
||||||
|
imgRef, _ := store.Store(pngPath, media.MediaMeta{}, "test")
|
||||||
|
|
||||||
|
// Simulate: assistant called load_image + read_file, two tool results follow
|
||||||
|
messages := []providers.Message{
|
||||||
|
{Role: "assistant", Content: "Let me load the image and read the file."},
|
||||||
|
{Role: "tool", Content: "Image loaded [image: photo]", Media: []string{imgRef}},
|
||||||
|
{Role: "tool", Content: "file contents here"},
|
||||||
|
}
|
||||||
|
result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize)
|
||||||
|
|
||||||
|
// assistant, tool#1, tool#2 must remain contiguous — no user in between
|
||||||
|
if result[0].Role != "assistant" {
|
||||||
|
t.Fatalf("result[0] expected assistant, got %q", result[0].Role)
|
||||||
|
}
|
||||||
|
if result[1].Role != "tool" {
|
||||||
|
t.Fatalf("result[1] expected tool, got %q", result[1].Role)
|
||||||
|
}
|
||||||
|
if result[2].Role != "tool" {
|
||||||
|
t.Fatalf("result[2] expected tool, got %q", result[2].Role)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Synthetic user message should come AFTER the tool block
|
||||||
|
if len(result) != 4 {
|
||||||
|
t.Fatalf("expected 4 messages (assistant + 2 tool + synthetic user), got %d", len(result))
|
||||||
|
}
|
||||||
|
if result[3].Role != "user" {
|
||||||
|
t.Fatalf("result[3] expected user, got %q", result[3].Role)
|
||||||
|
}
|
||||||
|
if len(result[3].Media) != 1 || !strings.HasPrefix(result[3].Media[0], "data:image/png;base64,") {
|
||||||
|
t.Fatal("expected synthetic user message to contain base64 image")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestResolveMediaRefs_OversizedImageSkipsBase64KeepsPathTag(t *testing.T) {
|
func TestResolveMediaRefs_OversizedImageSkipsBase64KeepsPathTag(t *testing.T) {
|
||||||
store := media.NewFileMediaStore()
|
store := media.NewFileMediaStore()
|
||||||
dir := t.TempDir()
|
dir := t.TempDir()
|
||||||
|
|
|
||||||
|
|
@ -150,7 +150,7 @@ func (t *LoadImageTool) Execute(ctx context.Context, args map[string]any) *ToolR
|
||||||
// Build the tool result text. The media:// ref in Media will be picked
|
// Build the tool result text. The media:// ref in Media will be picked
|
||||||
// up by resolveMediaRefs in agent_media.go and base64-encoded for tool
|
// up by resolveMediaRefs in agent_media.go and base64-encoded for tool
|
||||||
// result messages (role="tool"), so the LLM can see the image content.
|
// result messages (role="tool"), so the LLM can see the image content.
|
||||||
msg := fmt.Sprintf("Image loaded: %s\n[image: %s]", filename, ref)
|
msg := fmt.Sprintf("Image loaded: %s\n[image: photo]", filename)
|
||||||
|
|
||||||
return &ToolResult{
|
return &ToolResult{
|
||||||
ForLLM: msg,
|
ForLLM: msg,
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue