Skip to content

Commit b1ffb5f

Browse files
committed
llms/googleai: enhance handling of empty text parts in convertParts function
- Updated the convertParts function to skip completely empty text parts without reasoning, preventing unnecessary API errors. - Modified unit tests to validate the new behavior, ensuring that empty text without reasoning is excluded from the output. - Enhanced test cases to cover various scenarios involving empty text and reasoning, improving overall test coverage and accuracy.
1 parent 712d720 commit b1ffb5f

2 files changed

Lines changed: 70 additions & 15 deletions

File tree

llms/googleai/googleai.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,13 @@ func convertParts(parts []llms.ContentPart) ([]*genai.Part, error) {
544544

545545
switch p := part.(type) {
546546
case llms.TextContent:
547+
// Skip completely empty text parts without reasoning.
548+
// Empty parts serve no purpose and cause Gemini API errors:
549+
// "required oneof field 'data' must have one initialized field"
550+
if p.Text == "" && (p.Reasoning == nil || p.Reasoning.IsEmpty()) {
551+
continue
552+
}
553+
547554
// Optimization: skip empty text parts that only carry signature
548555
// when we already have tool call with signature.
549556
// This prevents duplicate signatures in the same message.

llms/googleai/googleai_core_unit_test.go

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,13 @@ func TestSignatureDeduplication(t *testing.T) {
5757
result, err := convertParts(parts)
5858
require.NoError(t, err)
5959

60-
// Should get: empty text part (kept because no signature to dedupe) + tool call
61-
assert.Len(t, result, 2, "Should have both parts")
60+
// Empty text without reasoning is SKIPPED (prevents Gemini API errors)
61+
// Only tool call should remain
62+
assert.Len(t, result, 1, "Empty text without reasoning should be skipped")
6263

6364
// Only tool call should have signature
64-
assert.Nil(t, result[0].ThoughtSignature, "Empty text without reasoning should have no signature")
65-
assert.Equal(t, testSig, result[1].ThoughtSignature, "Tool call should have signature")
65+
assert.Equal(t, testSig, result[0].ThoughtSignature, "Tool call should have signature")
66+
assert.NotNil(t, result[0].FunctionCall, "Remaining part should be function call")
6667
})
6768

6869
t.Run("scenario_2_mistaken_duplicate_signature", func(t *testing.T) {
@@ -433,21 +434,60 @@ func TestConvertParts(t *testing.T) {
433434
t.Parallel()
434435

435436
tests := []struct {
436-
name string
437-
parts []llms.ContentPart
438-
wantErr bool
437+
name string
438+
parts []llms.ContentPart
439+
wantErr bool
440+
expectedLength int // -1 means same as input
439441
}{
440442
{
441-
name: "empty parts",
442-
parts: []llms.ContentPart{},
443-
wantErr: false,
443+
name: "empty parts",
444+
parts: []llms.ContentPart{},
445+
wantErr: false,
446+
expectedLength: 0,
444447
},
445448
{
446449
name: "text content",
447450
parts: []llms.ContentPart{
448451
llms.TextContent{Text: "Hello world"},
449452
},
450-
wantErr: false,
453+
wantErr: false,
454+
expectedLength: 1,
455+
},
456+
{
457+
name: "empty text without reasoning - should be skipped",
458+
parts: []llms.ContentPart{
459+
llms.TextContent{Text: "", Reasoning: nil},
460+
},
461+
wantErr: false,
462+
expectedLength: 0, // Empty text without reasoning is skipped
463+
},
464+
{
465+
name: "empty text with reasoning - should be kept",
466+
parts: []llms.ContentPart{
467+
llms.TextContent{
468+
Text: "",
469+
Reasoning: &reasoning.ContentReasoning{
470+
Content: "thinking",
471+
Signature: []byte("sig"),
472+
},
473+
},
474+
},
475+
wantErr: false,
476+
expectedLength: 1, // Empty text with reasoning is kept
477+
},
478+
{
479+
name: "mixed: empty text + tool call",
480+
parts: []llms.ContentPart{
481+
llms.TextContent{Text: "", Reasoning: nil}, // Will be skipped
482+
llms.ToolCall{
483+
FunctionCall: &llms.FunctionCall{
484+
Name: "search",
485+
Arguments: `{"q":"test"}`,
486+
},
487+
},
488+
},
489+
wantErr: false,
490+
expectedLength: 1, // Only tool call remains
451491
},
452492
{
453493
name: "binary content",
@@ -457,7 +497,8 @@ func TestConvertParts(t *testing.T) {
457497
Data: []byte("fake image data"),
458498
},
459499
},
460-
wantErr: false,
500+
wantErr: false,
501+
expectedLength: 1,
461502
},
462503
{
463504
name: "tool call",
@@ -469,7 +510,8 @@ func TestConvertParts(t *testing.T) {
469510
},
470511
},
471512
},
472-
wantErr: false,
513+
wantErr: false,
514+
expectedLength: 1,
473515
},
474516
{
475517
name: "tool call response",
@@ -479,7 +521,8 @@ func TestConvertParts(t *testing.T) {
479521
Content: "It's sunny in Paris",
480522
},
481523
},
482-
wantErr: false,
524+
wantErr: false,
525+
expectedLength: 1,
483526
},
484527
{
485528
name: "tool call with invalid JSON",
@@ -505,7 +548,12 @@ func TestConvertParts(t *testing.T) {
505548
}
506549

507550
assert.NoError(t, err)
508-
assert.Len(t, result, len(tt.parts))
551+
552+
expectedLen := tt.expectedLength
553+
if expectedLen == -1 {
554+
expectedLen = len(tt.parts)
555+
}
556+
assert.Len(t, result, expectedLen, "Expected %d parts, got %d", expectedLen, len(result))
509557

510558
// Basic validation that all parts are created
511559
for i, part := range result {

0 commit comments

Comments
 (0)