diff --git a/openai/openai.go b/openai/openai.go index 8a8550977..556c2fd8e 100644 --- a/openai/openai.go +++ b/openai/openai.go @@ -11,6 +11,7 @@ import ( "fmt" "log/slog" "net/http" + "slices" "strings" "time" @@ -683,16 +684,24 @@ func FromChatRequest(r ChatCompletionRequest, thinking ...*model.Thinking) (*api return nil, errors.New("invalid message format") } } - // a tool message is a single result, so keep its parts together and - // preserve the tool call it answers. an [img] placeholder keeps each - // image at its position among the text parts - if strings.ToLower(msg.Role) == "tool" && (len(messages) > start || toolName != "" || msg.ToolCallID != "") { - tool := api.Message{Role: msg.Role, ToolName: toolName, ToolCallID: msg.ToolCallID} - for _, part := range messages[start:] { - tool.Content += part.Content + strings.Repeat("[img]", len(part.Images)) - tool.Images = append(tool.Images, part.Images...) + // a tool message is a single result, so every message built from it + // keeps the tool call it answers. text parts are merged into one + // message; parts with images stay split so each image keeps its + // position among the text + if strings.ToLower(msg.Role) == "tool" { + parts := messages[start:] + if slices.ContainsFunc(parts, func(m api.Message) bool { return len(m.Images) > 0 }) { + for i := range parts { + parts[i].ToolName = toolName + parts[i].ToolCallID = msg.ToolCallID + } + } else if len(parts) > 0 || toolName != "" || msg.ToolCallID != "" { + var sb strings.Builder + for _, part := range parts { + sb.WriteString(part.Content) + } + messages = append(messages[:start], api.Message{Role: msg.Role, Content: sb.String(), ToolName: toolName, ToolCallID: msg.ToolCallID}) } - messages = append(messages[:start], tool) } // since we might have added multiple messages above, if we have tools // calls we'll add them to the last message diff --git a/openai/openai_test.go b/openai/openai_test.go index 1b9ec1e7c..59be1bbab 100644 --- a/openai/openai_test.go +++ b/openai/openai_test.go @@ -158,6 +158,30 @@ func TestFromChatRequest_WithImage(t *testing.T) { } func TestFromChatRequest_ToolMessageArrayContent(t *testing.T) { + var req ChatCompletionRequest + if err := json.Unmarshal([]byte(`{"model":"test-model","messages":[ + {"role":"assistant","tool_calls":[{"id":"call_1","type":"function","function":{"name":"get_weather","arguments":"{}"}}]}, + {"role":"tool","tool_call_id":"call_1","content":[{"type":"text","text":"sunny [img]"},{"type":"text","text":", 72F [img-99]"}]} + ]}`), &req); err != nil { + t.Fatal(err) + } + + result, err := FromChatRequest(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // text parts are merged into one result; tool text is kept as sent + if len(result.Messages) != 2 { + t.Fatalf("expected 2 messages, got %d", len(result.Messages)) + } + tool := result.Messages[1] + if tool.Role != "tool" || tool.ToolCallID != "call_1" || tool.ToolName != "get_weather" || tool.Content != "sunny [img], 72F [img-99]" { + t.Errorf("expected tool message for call_1/get_weather, got role=%q id=%q name=%q content=%q", tool.Role, tool.ToolCallID, tool.ToolName, tool.Content) + } +} + +func TestFromChatRequest_ToolMessageArrayContentWithImages(t *testing.T) { const image2 = `iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==` imgData, _ := base64.StdEncoding.DecodeString(image) img2Data, _ := base64.StdEncoding.DecodeString(image2) @@ -201,20 +225,22 @@ func TestFromChatRequest_ToolMessageArrayContent(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - if len(result.Messages) != 3 { - t.Fatalf("expected 3 messages, got %d", len(result.Messages)) + // parts with images stay split in order, each answering the same call + if len(result.Messages) != 6 { + t.Fatalf("expected 6 messages, got %d", len(result.Messages)) } - - tool := result.Messages[2] - if tool.Role != "tool" || tool.ToolCallID != "call_1" || tool.ToolName != "screenshot" { - t.Errorf("expected tool message for call_1/screenshot, got role=%q id=%q name=%q", tool.Role, tool.ToolCallID, tool.ToolName) - } - // each image keeps its position among the text parts - if tool.Content != "Before: [img] After: [img]" { - t.Errorf("expected content 'Before: [img] After: [img]', got %q", tool.Content) - } - if len(tool.Images) != 2 || string(tool.Images[0]) != string(imgData) || string(tool.Images[1]) != string(img2Data) { - t.Errorf("expected both tool images to be kept in order, got %d images", len(tool.Images)) + wantContent := []string{"Before: ", "", " After: ", ""} + wantImages := [][]byte{nil, imgData, nil, img2Data} + for i, msg := range result.Messages[2:] { + if msg.Role != "tool" || msg.ToolCallID != "call_1" || msg.ToolName != "screenshot" { + t.Errorf("part %d: expected tool message for call_1/screenshot, got role=%q id=%q name=%q", i, msg.Role, msg.ToolCallID, msg.ToolName) + } + if msg.Content != wantContent[i] { + t.Errorf("part %d: expected content %q, got %q", i, wantContent[i], msg.Content) + } + if wantImages[i] == nil && len(msg.Images) != 0 || wantImages[i] != nil && (len(msg.Images) != 1 || string(msg.Images[0]) != string(wantImages[i])) { + t.Errorf("part %d: unexpected images, got %d", i, len(msg.Images)) + } } }