From bd3314ee69b14fa92adf3ba1d7481611d5dd640e Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Tue, 16 Dec 2025 11:16:18 -0300 Subject: [PATCH] fix(openai,openaicompat): drop empty messages with warning (#102) Assisted-by: Anthropic and Grok via Crush Co-authored-by: Crush --- providers/openai/language_model_hooks.go | 35 ++ providers/openai/openai_test.go | 335 +++++++++++++++++- providers/openai/responses_language_model.go | 33 ++ .../openaicompat/language_model_hooks.go | 35 ++ providers/openaicompat/openaicompat_test.go | 195 +++++++++- 5 files changed, 613 insertions(+), 20 deletions(-) diff --git a/providers/openai/language_model_hooks.go b/providers/openai/language_model_hooks.go index 9ca2c64f520a1dbf01f95f46ac097689b2e6d045..d58c1794f540b19e7599792c22a370831987f5e7 100644 --- a/providers/openai/language_model_hooks.go +++ b/providers/openai/language_model_hooks.go @@ -437,6 +437,13 @@ func DefaultToPrompt(prompt fantasy.Prompt, _, _ string) ([]openai.ChatCompletio } } } + if !hasVisibleUserContent(content) { + warnings = append(warnings, fantasy.CallWarning{ + Type: fantasy.CallWarningTypeOther, + Message: "dropping empty user message (contains neither user-facing content nor tool results)", + }) + continue + } messages = append(messages, openai.UserMessage(content)) case fantasy.MessageRoleAssistant: // simple assistant message just text content @@ -491,6 +498,13 @@ func DefaultToPrompt(prompt fantasy.Prompt, _, _ string) ([]openai.ChatCompletio }) } } + if !hasVisibleAssistantContent(&assistantMsg) { + warnings = append(warnings, fantasy.CallWarning{ + Type: fantasy.CallWarningTypeOther, + Message: "dropping empty assistant message (contains neither user-facing content nor tool calls)", + }) + continue + } messages = append(messages, openai.ChatCompletionMessageParamUnion{ OfAssistant: &assistantMsg, }) @@ -541,3 +555,24 @@ func DefaultToPrompt(prompt fantasy.Prompt, _, _ string) ([]openai.ChatCompletio } return messages, warnings } + +func hasVisibleUserContent(content []openai.ChatCompletionContentPartUnionParam) bool { + for _, part := range content { + if part.OfText != nil || part.OfImageURL != nil || part.OfInputAudio != nil || part.OfFile != nil { + return true + } + } + return false +} + +func hasVisibleAssistantContent(msg *openai.ChatCompletionAssistantMessageParam) bool { + // Check if there's text content + if !param.IsOmitted(msg.Content.OfString) || len(msg.Content.OfArrayOfContentParts) > 0 { + return true + } + // Check if there are tool calls + if len(msg.ToolCalls) > 0 { + return true + } + return false +} diff --git a/providers/openai/openai_test.go b/providers/openai/openai_test.go index 448c1b132c2941d64bcba936b68bacb795bfc834..97296fd974d9cb069649e3dcdb167e88994dfe0b 100644 --- a/providers/openai/openai_test.go +++ b/providers/openai/openai_test.go @@ -202,9 +202,10 @@ func TestToOpenAiPrompt_FileParts(t *testing.T) { messages, warnings := DefaultToPrompt(prompt, "openai", "gpt-5") - require.Len(t, warnings, 1) + require.Len(t, warnings, 2) // unsupported type + empty message require.Contains(t, warnings[0].Message, "file part media type application/something not supported") - require.Len(t, messages, 1) // Message is still created but with empty content array + require.Contains(t, warnings[1].Message, "dropping empty user message") + require.Empty(t, messages) // Message is now dropped because it's empty }) t.Run("should add audio content for audio/wav file parts", func(t *testing.T) { @@ -2857,3 +2858,333 @@ func TestDoStream(t *testing.T) { require.Equal(t, int64(10), finishPart.Usage.ReasoningTokens) }) } + +func TestDefaultToPrompt_DropsEmptyMessages(t *testing.T) { + t.Parallel() + + t.Run("should drop truly empty assistant messages", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hello"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{}, + }, + } + + messages, warnings := DefaultToPrompt(prompt, "openai", "gpt-4") + + require.Len(t, messages, 1, "should only have user message") + require.Len(t, warnings, 1) + require.Equal(t, fantasy.CallWarningTypeOther, warnings[0].Type) + require.Contains(t, warnings[0].Message, "dropping empty assistant message") + }) + + t.Run("should keep assistant messages with text content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hello"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hi there!"}, + }, + }, + } + + messages, warnings := DefaultToPrompt(prompt, "openai", "gpt-4") + + require.Len(t, messages, 2, "should have both user and assistant messages") + require.Empty(t, warnings) + }) + + t.Run("should keep assistant messages with tool calls", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "What's the weather?"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + fantasy.ToolCallPart{ + ToolCallID: "call_123", + ToolName: "get_weather", + Input: `{"location":"NYC"}`, + }, + }, + }, + } + + messages, warnings := DefaultToPrompt(prompt, "openai", "gpt-4") + + require.Len(t, messages, 2, "should have both user and assistant messages") + require.Empty(t, warnings) + }) + + t.Run("should drop user messages without visible content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.FilePart{ + Data: []byte("not supported"), + MediaType: "application/unknown", + }, + }, + }, + } + + messages, warnings := DefaultToPrompt(prompt, "openai", "gpt-4") + + require.Empty(t, messages) + require.Len(t, warnings, 2) // One for unsupported type, one for empty message + require.Contains(t, warnings[1].Message, "dropping empty user message") + }) + + t.Run("should keep user messages with image content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.FilePart{ + Data: []byte{0x01, 0x02, 0x03}, + MediaType: "image/png", + }, + }, + }, + } + + messages, warnings := DefaultToPrompt(prompt, "openai", "gpt-4") + + require.Len(t, messages, 1) + require.Empty(t, warnings) + }) + + t.Run("should keep user messages with tool results", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleTool, + Content: []fantasy.MessagePart{ + fantasy.ToolResultPart{ + ToolCallID: "call_123", + Output: fantasy.ToolResultOutputContentText{Text: "done"}, + }, + }, + }, + } + + messages, warnings := DefaultToPrompt(prompt, "openai", "gpt-4") + + require.Len(t, messages, 1) + require.Empty(t, warnings) + }) + + t.Run("should keep user messages with tool error results", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleTool, + Content: []fantasy.MessagePart{ + fantasy.ToolResultPart{ + ToolCallID: "call_456", + Output: fantasy.ToolResultOutputContentError{Error: errors.New("boom")}, + }, + }, + }, + } + + messages, warnings := DefaultToPrompt(prompt, "openai", "gpt-4") + + require.Len(t, messages, 1) + require.Empty(t, warnings) + }) +} + +func TestResponsesToPrompt_DropsEmptyMessages(t *testing.T) { + t.Parallel() + + t.Run("should drop truly empty assistant messages", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hello"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{}, + }, + } + + input, warnings := toResponsesPrompt(prompt, "system") + + require.Len(t, input, 1, "should only have user message") + require.Len(t, warnings, 1) + require.Equal(t, fantasy.CallWarningTypeOther, warnings[0].Type) + require.Contains(t, warnings[0].Message, "dropping empty assistant message") + }) + + t.Run("should keep assistant messages with text content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hello"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hi there!"}, + }, + }, + } + + input, warnings := toResponsesPrompt(prompt, "system") + + require.Len(t, input, 2, "should have both user and assistant messages") + require.Empty(t, warnings) + }) + + t.Run("should keep assistant messages with tool calls", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "What's the weather?"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + fantasy.ToolCallPart{ + ToolCallID: "call_123", + ToolName: "get_weather", + Input: `{"location":"NYC"}`, + }, + }, + }, + } + + input, warnings := toResponsesPrompt(prompt, "system") + + require.Len(t, input, 2, "should have both user and assistant messages") + require.Empty(t, warnings) + }) + + t.Run("should drop user messages without visible content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.FilePart{ + Data: []byte("not supported"), + MediaType: "application/unknown", + }, + }, + }, + } + + input, warnings := toResponsesPrompt(prompt, "system") + + require.Empty(t, input) + require.Len(t, warnings, 2) // One for unsupported type, one for empty message + require.Contains(t, warnings[1].Message, "dropping empty user message") + }) + + t.Run("should keep user messages with image content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.FilePart{ + Data: []byte{0x01, 0x02, 0x03}, + MediaType: "image/png", + }, + }, + }, + } + + input, warnings := toResponsesPrompt(prompt, "system") + + require.Len(t, input, 1) + require.Empty(t, warnings) + }) + + t.Run("should keep user messages with tool results", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleTool, + Content: []fantasy.MessagePart{ + fantasy.ToolResultPart{ + ToolCallID: "call_123", + Output: fantasy.ToolResultOutputContentText{Text: "done"}, + }, + }, + }, + } + + input, warnings := toResponsesPrompt(prompt, "system") + + require.Len(t, input, 1) + require.Empty(t, warnings) + }) + + t.Run("should keep user messages with tool error results", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleTool, + Content: []fantasy.MessagePart{ + fantasy.ToolResultPart{ + ToolCallID: "call_456", + Output: fantasy.ToolResultOutputContentError{Error: errors.New("boom")}, + }, + }, + }, + } + + input, warnings := toResponsesPrompt(prompt, "system") + + require.Len(t, input, 1) + require.Empty(t, warnings) + }) +} diff --git a/providers/openai/responses_language_model.go b/providers/openai/responses_language_model.go index 85a86513d73f56de56b20f9a968fb97ef1b1ab7a..39ee8e427e881cb88cfb206c2e75849cf1f83ee8 100644 --- a/providers/openai/responses_language_model.go +++ b/providers/openai/responses_language_model.go @@ -440,9 +440,18 @@ func toResponsesPrompt(prompt fantasy.Prompt, systemMessageMode string) (respons } } + if !hasVisibleResponsesUserContent(contentParts) { + warnings = append(warnings, fantasy.CallWarning{ + Type: fantasy.CallWarningTypeOther, + Message: "dropping empty user message (contains neither user-facing content nor tool results)", + }) + continue + } + input = append(input, responses.ResponseInputItemParamOfMessage(contentParts, responses.EasyInputMessageRoleUser)) case fantasy.MessageRoleAssistant: + startIdx := len(input) for _, c := range msg.Content { switch c.GetType() { case fantasy.ContentTypeText: @@ -513,6 +522,16 @@ func toResponsesPrompt(prompt fantasy.Prompt, systemMessageMode string) (respons } } + if !hasVisibleResponsesAssistantContent(input, startIdx) { + warnings = append(warnings, fantasy.CallWarning{ + Type: fantasy.CallWarningTypeOther, + Message: "dropping empty assistant message (contains neither user-facing content nor tool calls)", + }) + // Remove any items that were added during this iteration + input = input[:startIdx] + continue + } + case fantasy.MessageRoleTool: for _, c := range msg.Content { if c.GetType() != fantasy.ContentTypeToolResult { @@ -564,6 +583,20 @@ func toResponsesPrompt(prompt fantasy.Prompt, systemMessageMode string) (respons return input, warnings } +func hasVisibleResponsesUserContent(content responses.ResponseInputMessageContentListParam) bool { + return len(content) > 0 +} + +func hasVisibleResponsesAssistantContent(items []responses.ResponseInputItemUnionParam, startIdx int) bool { + // Check if we added any assistant content parts from this message + for i := startIdx; i < len(items); i++ { + if items[i].OfMessage != nil || items[i].OfFunctionCall != nil { + return true + } + } + return false +} + func toResponsesTools(tools []fantasy.Tool, toolChoice *fantasy.ToolChoice, options *ResponsesProviderOptions) ([]responses.ToolUnionParam, responses.ResponseNewParamsToolChoiceUnion, []fantasy.CallWarning) { warnings := make([]fantasy.CallWarning, 0) var openaiTools []responses.ToolUnionParam diff --git a/providers/openaicompat/language_model_hooks.go b/providers/openaicompat/language_model_hooks.go index 2f21f7130e950b1dddee83de45591cbf4a571b1f..0aa721d5aa75a9996f7b11c7008d37f1c051199d 100644 --- a/providers/openaicompat/language_model_hooks.go +++ b/providers/openaicompat/language_model_hooks.go @@ -287,6 +287,13 @@ func ToPromptFunc(prompt fantasy.Prompt, _, _ string) ([]openaisdk.ChatCompletio } } } + if !hasVisibleCompatUserContent(content) { + warnings = append(warnings, fantasy.CallWarning{ + Type: fantasy.CallWarningTypeOther, + Message: "dropping empty user message (contains neither user-facing content nor tool results)", + }) + continue + } messages = append(messages, openaisdk.UserMessage(content)) case fantasy.MessageRoleAssistant: // simple assistant message just text content @@ -358,6 +365,13 @@ func ToPromptFunc(prompt fantasy.Prompt, _, _ string) ([]openaisdk.ChatCompletio "reasoning_content": reasoningText, }) } + if !hasVisibleCompatAssistantContent(&assistantMsg) { + warnings = append(warnings, fantasy.CallWarning{ + Type: fantasy.CallWarningTypeOther, + Message: "dropping empty assistant message (contains neither user-facing content nor tool calls)", + }) + continue + } messages = append(messages, openaisdk.ChatCompletionMessageParamUnion{ OfAssistant: &assistantMsg, }) @@ -407,3 +421,24 @@ func ToPromptFunc(prompt fantasy.Prompt, _, _ string) ([]openaisdk.ChatCompletio } return messages, warnings } + +func hasVisibleCompatUserContent(content []openaisdk.ChatCompletionContentPartUnionParam) bool { + for _, part := range content { + if part.OfText != nil || part.OfImageURL != nil || part.OfInputAudio != nil || part.OfFile != nil { + return true + } + } + return false +} + +func hasVisibleCompatAssistantContent(msg *openaisdk.ChatCompletionAssistantMessageParam) bool { + // Check if there's text content + if !param.IsOmitted(msg.Content.OfString) || len(msg.Content.OfArrayOfContentParts) > 0 { + return true + } + // Check if there are tool calls + if len(msg.ToolCalls) > 0 { + return true + } + return false +} diff --git a/providers/openaicompat/openaicompat_test.go b/providers/openaicompat/openaicompat_test.go index 89e2ae1e78fbf18aa762943c2e5e35922d27bfc3..a603e5e6681f3d06586e78b0c9efce4e9d7fc10d 100644 --- a/providers/openaicompat/openaicompat_test.go +++ b/providers/openaicompat/openaicompat_test.go @@ -1,6 +1,7 @@ package openaicompat import ( + "errors" "testing" "charm.land/fantasy" @@ -81,16 +82,14 @@ func TestToPromptFunc_ReasoningContent(t *testing.T) { messages, warnings := ToPromptFunc(prompt, "", "") - require.Empty(t, warnings) - require.Len(t, messages, 2) + require.Len(t, warnings, 1) + require.Contains(t, warnings[0].Message, "dropping empty assistant message") + require.Len(t, messages, 1) // Only user message, assistant message dropped - // Assistant message with only reasoning - msg := messages[1].OfAssistant + // User message - unchanged + msg := messages[0].OfUser require.NotNil(t, msg) - extraFields := msg.ExtraFields() - reasoningContent, hasReasoning := extraFields["reasoning_content"] - require.True(t, hasReasoning) - require.Equal(t, "Internal reasoning only...", reasoningContent) + require.Equal(t, "Hello", msg.Content.OfString.Value) }) t.Run("should not add reasoning_content to messages without reasoning", func(t *testing.T) { @@ -219,24 +218,19 @@ func TestToPromptFunc_ReasoningContent(t *testing.T) { messages, warnings := ToPromptFunc(prompt, "", "") - require.Len(t, warnings, 1) + require.Len(t, warnings, 2) // unsupported type + empty message require.Contains(t, warnings[0].Message, "not supported") - // Should have all 3 messages (matching openai behavior - don't skip empty content) - require.Len(t, messages, 3) + require.Contains(t, warnings[1].Message, "dropping empty user message") + // Should have only 2 messages (empty content message is now dropped) + require.Len(t, messages, 2) msg1 := messages[0].OfUser require.NotNil(t, msg1) require.Equal(t, "Hello", msg1.Content.OfString.Value) - // Second message has empty content (unsupported attachment was skipped) msg2 := messages[1].OfUser require.NotNil(t, msg2) - content2 := msg2.Content.OfArrayOfContentParts - require.Len(t, content2, 0) - - msg3 := messages[2].OfUser - require.NotNil(t, msg3) - require.Equal(t, "After unsupported", msg3.Content.OfString.Value) + require.Equal(t, "After unsupported", msg2.Content.OfString.Value) }) t.Run("should detect PDF file IDs using strings.HasPrefix", func(t *testing.T) { @@ -273,3 +267,168 @@ func TestToPromptFunc_ReasoningContent(t *testing.T) { require.Equal(t, "file-abc123xyz", filePart.File.FileID.Value) }) } + +func TestToPromptFunc_DropsEmptyMessages(t *testing.T) { + t.Parallel() + + t.Run("should drop truly empty assistant messages", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hello"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{}, + }, + } + + messages, warnings := ToPromptFunc(prompt, "", "") + + require.Len(t, messages, 1, "should only have user message") + require.Len(t, warnings, 1) + require.Equal(t, fantasy.CallWarningTypeOther, warnings[0].Type) + require.Contains(t, warnings[0].Message, "dropping empty assistant message") + }) + + t.Run("should keep assistant messages with text content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hello"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "Hi there!"}, + }, + }, + } + + messages, warnings := ToPromptFunc(prompt, "", "") + + require.Len(t, messages, 2, "should have both user and assistant messages") + require.Empty(t, warnings) + }) + + t.Run("should keep assistant messages with tool calls", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "What's the weather?"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + fantasy.ToolCallPart{ + ToolCallID: "call_123", + ToolName: "get_weather", + Input: `{"location":"NYC"}`, + }, + }, + }, + } + + messages, warnings := ToPromptFunc(prompt, "", "") + + require.Len(t, messages, 2, "should have both user and assistant messages") + require.Empty(t, warnings) + }) + + t.Run("should drop user messages without visible content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.FilePart{ + Data: []byte("not supported"), + MediaType: "application/unknown", + }, + }, + }, + } + + messages, warnings := ToPromptFunc(prompt, "", "") + + require.Empty(t, messages) + require.Len(t, warnings, 2) // unsupported type + empty message + require.Contains(t, warnings[1].Message, "dropping empty user message") + }) + + t.Run("should keep user messages with image content", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.FilePart{ + Data: []byte{0x01, 0x02, 0x03}, + MediaType: "image/png", + }, + }, + }, + } + + messages, warnings := ToPromptFunc(prompt, "", "") + + require.Len(t, messages, 1) + require.Empty(t, warnings) + }) + + t.Run("should keep user messages with tool results", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleTool, + Content: []fantasy.MessagePart{ + fantasy.ToolResultPart{ + ToolCallID: "call_123", + Output: fantasy.ToolResultOutputContentText{Text: "done"}, + }, + }, + }, + } + + messages, warnings := ToPromptFunc(prompt, "", "") + + require.Len(t, messages, 1) + require.Empty(t, warnings) + }) + + t.Run("should keep user messages with tool error results", func(t *testing.T) { + t.Parallel() + + prompt := fantasy.Prompt{ + { + Role: fantasy.MessageRoleTool, + Content: []fantasy.MessagePart{ + fantasy.ToolResultPart{ + ToolCallID: "call_456", + Output: fantasy.ToolResultOutputContentError{Error: errors.New("boom")}, + }, + }, + }, + } + + messages, warnings := ToPromptFunc(prompt, "", "") + + require.Len(t, messages, 1) + require.Empty(t, warnings) + }) +}