Skip to content

Commit 42a1bba

Browse files
authored
model/openai: sanitize empty tool_calls chunks (#745)
1 parent eb1d374 commit 42a1bba

File tree

2 files changed

+160
-1
lines changed

2 files changed

+160
-1
lines changed

model/openai/openai.go

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1252,7 +1252,12 @@ func (m *Model) handleStreamingResponse(
12521252
// Always accumulate for correctness (tool call deltas are assembled later),
12531253
// but skip chunks with reasoning content that would cause the SDK accumulator to panic.
12541254
if !m.hasReasoningContent(chunk.Choices) {
1255-
acc.AddChunk(chunk)
1255+
// Sanitize chunks before feeding them into the upstream accumulator to
1256+
// avoid known panics when JSON.ToolCalls is marked present but the
1257+
// typed ToolCalls slice is empty, especially on finish_reason chunks.
1258+
sanitizedChunk := sanitizeChunkForAccumulator(chunk)
1259+
1260+
acc.AddChunk(sanitizedChunk)
12561261
if m.accumulateChunkUsage != nil {
12571262
accUsage, chunkUsage := completionUsageToModelUsage(acc.Usage), completionUsageToModelUsage(chunk.Usage)
12581263
usage := inverseOPENAISKDAddChunkUsage(accUsage, chunkUsage)
@@ -1304,6 +1309,44 @@ func (m *Model) handleStreamingResponse(
13041309
}
13051310
}
13061311

1312+
// sanitizeChunkForAccumulator returns a defensive copy of the given chunk that
1313+
// avoids structures known to cause panics in the upstream OpenAI SDK
1314+
// accumulator. In particular, it clears JSON.ToolCalls metadata when it is
1315+
// marked present but the typed ToolCalls slice is empty on a finish_reason
1316+
// chunk, which would otherwise lead to an out-of-range access in
1317+
// chatCompletionResponseState.update.
1318+
func sanitizeChunkForAccumulator(chunk openai.ChatCompletionChunk) openai.ChatCompletionChunk {
1319+
if len(chunk.Choices) == 0 {
1320+
return chunk
1321+
}
1322+
1323+
choice := chunk.Choices[0]
1324+
delta := choice.Delta
1325+
1326+
// Only sanitize the specific pattern that is known to be unsafe for the
1327+
// accumulator:
1328+
// - finish_reason is set (e.g. "tool_calls" or "stop")
1329+
// - JSON.ToolCalls is marked present
1330+
// - but the typed ToolCalls slice is empty
1331+
if choice.FinishReason == "" ||
1332+
!delta.JSON.ToolCalls.Valid() ||
1333+
len(delta.ToolCalls) != 0 {
1334+
return chunk
1335+
}
1336+
1337+
sanitized := chunk
1338+
sanitized.Choices = make([]openai.ChatCompletionChunkChoice, len(chunk.Choices))
1339+
copy(sanitized.Choices, chunk.Choices)
1340+
1341+
// Clear the JSON metadata for ToolCalls on the first choice only. This
1342+
// preserves finish_reason and usage semantics while preventing the
1343+
// accumulator from treating this as a tool-call delta that must have at
1344+
// least one element.
1345+
sanitized.Choices[0].Delta.JSON.ToolCalls = respjson.Field{}
1346+
1347+
return sanitized
1348+
}
1349+
13071350
// updateToolCallIndexMapping updates the tool call index mapping.
13081351
func (m *Model) updateToolCallIndexMapping(chunk openai.ChatCompletionChunk, idToIndexMap map[string]int) {
13091352
if len(chunk.Choices) > 0 && len(chunk.Choices[0].Delta.ToolCalls) > 0 {

model/openai/openai_test.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ package openai
1111

1212
import (
1313
"context"
14+
"encoding/json"
1415
"fmt"
1516
"io"
1617
"net/http"
@@ -3892,6 +3893,121 @@ func TestToolCallIndexMapping(t *testing.T) {
38923893
})
38933894
}
38943895

3896+
// TestChatCompletionAccumulator_ToolCallsEmpty_Panics verifies that the
3897+
// upstream openai-go accumulator panics when JSON.ToolCalls is marked
3898+
// present but the typed ToolCalls slice is empty. This documents the
3899+
// panic behavior that our framework needs to defensively guard against.
3900+
func TestChatCompletionAccumulator_ToolCallsEmpty_Panics(t *testing.T) {
3901+
// This JSON mimics a streaming chunk where the provider sends an empty
3902+
// tool_calls array together with a tool_calls finish_reason.
3903+
raw := []byte(`{
3904+
"id": "test",
3905+
"object": "chat.completion.chunk",
3906+
"created": 1699200000,
3907+
"model": "gpt-3.5-turbo",
3908+
"choices": [
3909+
{
3910+
"index": 0,
3911+
"delta": {
3912+
"tool_calls": []
3913+
},
3914+
"finish_reason": "tool_calls"
3915+
}
3916+
]
3917+
}`)
3918+
3919+
var chunk openai.ChatCompletionChunk
3920+
require.NoError(t, json.Unmarshal(raw, &chunk), "failed to unmarshal test chunk")
3921+
3922+
defer func() {
3923+
if r := recover(); r == nil {
3924+
t.Fatalf("expected panic when adding chunk with JSON.ToolCalls valid and empty ToolCalls slice, but no panic occurred")
3925+
}
3926+
}()
3927+
3928+
var acc openai.ChatCompletionAccumulator
3929+
acc.AddChunk(chunk)
3930+
}
3931+
3932+
// TestSanitizeChunkForAccumulator_FinishReasonToolCalls verifies that
3933+
// sanitizeChunkForAccumulator clears JSON.ToolCalls metadata for chunks
3934+
// that have a finish_reason and an empty ToolCalls slice, which would
3935+
// otherwise cause the upstream accumulator to panic.
3936+
func TestSanitizeChunkForAccumulator_FinishReasonToolCalls(t *testing.T) {
3937+
raw := []byte(`{
3938+
"id": "test",
3939+
"object": "chat.completion.chunk",
3940+
"created": 1699200000,
3941+
"model": "gpt-3.5-turbo",
3942+
"choices": [
3943+
{
3944+
"index": 0,
3945+
"delta": {
3946+
"content": "",
3947+
"tool_calls": []
3948+
},
3949+
"finish_reason": "tool_calls"
3950+
}
3951+
]
3952+
}`)
3953+
3954+
var chunk openai.ChatCompletionChunk
3955+
require.NoError(t, json.Unmarshal(raw, &chunk))
3956+
require.Len(t, chunk.Choices, 1)
3957+
require.Equal(t, "tool_calls", chunk.Choices[0].FinishReason)
3958+
require.True(t, chunk.Choices[0].Delta.JSON.ToolCalls.Valid())
3959+
require.Len(t, chunk.Choices[0].Delta.ToolCalls, 0)
3960+
3961+
sanitized := sanitizeChunkForAccumulator(chunk)
3962+
3963+
// Original chunk should remain unchanged.
3964+
require.True(t, chunk.Choices[0].Delta.JSON.ToolCalls.Valid())
3965+
3966+
// Sanitized chunk should have ToolCalls metadata cleared but still carry
3967+
// the same finish_reason and an empty typed ToolCalls slice.
3968+
require.Len(t, sanitized.Choices, 1)
3969+
assert.Equal(t, "tool_calls", sanitized.Choices[0].FinishReason)
3970+
assert.False(t, sanitized.Choices[0].Delta.JSON.ToolCalls.Valid())
3971+
assert.Len(t, sanitized.Choices[0].Delta.ToolCalls, 0)
3972+
}
3973+
3974+
// TestSanitizeChunkForAccumulator_NoFinishReason ensures that chunks without
3975+
// a finish_reason are left untouched even if they carry an empty ToolCalls
3976+
// array, since these are safe for the accumulator (it will use the content
3977+
// branch instead of the tool_calls branch).
3978+
func TestSanitizeChunkForAccumulator_NoFinishReason(t *testing.T) {
3979+
raw := []byte(`{
3980+
"id": "test",
3981+
"object": "chat.completion.chunk",
3982+
"created": 1699200000,
3983+
"model": "gpt-3.5-turbo",
3984+
"choices": [
3985+
{
3986+
"index": 0,
3987+
"delta": {
3988+
"content": "hello",
3989+
"tool_calls": []
3990+
},
3991+
"finish_reason": null
3992+
}
3993+
]
3994+
}`)
3995+
3996+
var chunk openai.ChatCompletionChunk
3997+
require.NoError(t, json.Unmarshal(raw, &chunk))
3998+
require.Len(t, chunk.Choices, 1)
3999+
require.Equal(t, "", chunk.Choices[0].FinishReason)
4000+
require.True(t, chunk.Choices[0].Delta.JSON.ToolCalls.Valid())
4001+
require.Len(t, chunk.Choices[0].Delta.ToolCalls, 0)
4002+
4003+
sanitized := sanitizeChunkForAccumulator(chunk)
4004+
4005+
// Chunks without finish_reason should not be modified.
4006+
assert.Equal(t, chunk, sanitized)
4007+
assert.True(t, sanitized.Choices[0].Delta.JSON.ToolCalls.Valid())
4008+
assert.Len(t, sanitized.Choices[0].Delta.ToolCalls, 0)
4009+
}
4010+
38954011
// TestStreamingCallbackIntegration tests the integration of streaming callbacks.
38964012
func TestStreamingCallbackIntegration(t *testing.T) {
38974013
t.Run("streaming with chat stream complete callback", func(t *testing.T) {

0 commit comments

Comments
 (0)