Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public ChatResponse internalCall(Prompt prompt, ChatResponse previousChatRespons
"id", chatCompletion.id() != null ? chatCompletion.id() : "",
"role", choice.message().role() != null ? choice.message().role().name() : "",
"index", choice.index() != null ? choice.index() : 0,
"finishReason", choice.finishReason() != null ? choice.finishReason().name() : "",
"finishReason", getFinishReasonJson(choice.finishReason()),
"refusal", StringUtils.hasText(choice.message().refusal()) ? choice.message().refusal() : "",
"annotations", choice.message().annotations() != null ? choice.message().annotations() : List.of(Map.of()));
return buildGeneration(choice, metadata, request);
Expand Down Expand Up @@ -317,7 +317,7 @@ public Flux<ChatResponse> internalStream(Prompt prompt, ChatResponse previousCha
"id", id,
"role", roleMap.getOrDefault(id, ""),
"index", choice.index() != null ? choice.index() : 0,
"finishReason", choice.finishReason() != null ? choice.finishReason().name() : "",
"finishReason", getFinishReasonJson(choice.finishReason()),
"refusal", StringUtils.hasText(choice.message().refusal()) ? choice.message().refusal() : "",
"annotations", choice.message().annotations() != null ? choice.message().annotations() : List.of());
return buildGeneration(choice, metadata, request);
Expand Down Expand Up @@ -420,8 +420,8 @@ private Generation buildGeneration(Choice choice, Map<String, Object> metadata,
toolCall.function().name(), toolCall.function().arguments()))
.toList();

String finishReason = (choice.finishReason() != null ? choice.finishReason().name() : "");
var generationMetadataBuilder = ChatGenerationMetadata.builder().finishReason(finishReason);
var generationMetadataBuilder = ChatGenerationMetadata.builder()
.finishReason(getFinishReasonJson(choice.finishReason()));

List<Media> media = new ArrayList<>();
String textContent = choice.message().content();
Expand Down Expand Up @@ -451,6 +451,14 @@ private Generation buildGeneration(Choice choice, Map<String, Object> metadata,
return new Generation(assistantMessage, generationMetadataBuilder.build());
}

private String getFinishReasonJson(OpenAiApi.ChatCompletionFinishReason finishReason) {
if (finishReason == null) {
return "";
}
// Return enum name for backward compatibility
return finishReason.name();
}

private ChatResponseMetadata from(OpenAiApi.ChatCompletion result, RateLimit rateLimit, Usage usage) {
Assert.notNull(result, "OpenAI ChatCompletionResult must not be null");
var builder = ChatResponseMetadata.builder()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,258 @@
/*
* Copyright 2023-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.ai.openai.chat;

import java.util.List;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.micrometer.observation.ObservationRegistry;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import reactor.core.publisher.Flux;

import org.springframework.ai.chat.model.ChatResponse;
import org.springframework.ai.chat.prompt.Prompt;
import org.springframework.ai.model.ModelOptionsUtils;
import org.springframework.ai.model.tool.ToolCallingManager;
import org.springframework.ai.openai.OpenAiChatModel;
import org.springframework.ai.openai.OpenAiChatOptions;
import org.springframework.ai.openai.api.OpenAiApi;
import org.springframework.ai.openai.api.OpenAiApi.ChatCompletionChunk;
import org.springframework.ai.openai.api.OpenAiApi.ChatCompletionChunk.ChunkChoice;
import org.springframework.ai.openai.api.OpenAiApi.ChatCompletionFinishReason;
import org.springframework.ai.openai.api.OpenAiApi.ChatCompletionMessage;
import org.springframework.ai.openai.api.OpenAiApi.ChatCompletionMessage.Role;
import org.springframework.ai.openai.api.OpenAiApi.ChatCompletionRequest;
import org.springframework.ai.retry.RetryUtils;
import org.springframework.retry.support.RetryTemplate;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.BDDMockito.given;

/**
* Tests for OpenAI streaming responses with various finish_reason scenarios, particularly
* focusing on edge cases like empty string finish_reason values.
*
* @author Mark Pollack
* @author Christian Tzolov
*/
@ExtendWith(MockitoExtension.class)
public class OpenAiStreamingFinishReasonTests {

@Mock
private OpenAiApi openAiApi;

private OpenAiChatModel chatModel;

@Test
void testStreamingWithNullFinishReason() {
// Setup
setupChatModel();

var choice = new ChunkChoice(null, 0, new ChatCompletionMessage("Hello", Role.ASSISTANT), null);
ChatCompletionChunk chunk = new ChatCompletionChunk("id", List.of(choice), 666L, "model", null, null,
"chat.completion.chunk", null);

given(this.openAiApi.chatCompletionStream(isA(ChatCompletionRequest.class), any()))
.willReturn(Flux.just(chunk));

// Execute
Flux<ChatResponse> result = this.chatModel.stream(new Prompt("test"));

// Verify
List<ChatResponse> responses = result.collectList().block();
assertThat(responses).hasSize(1);
ChatResponse response = responses.get(0);
assertThat(response).isNotNull();
assertThat(response.getResult().getOutput().getText()).isEqualTo("Hello");
assertThat(response.getResult().getMetadata().getFinishReason()).isEqualTo("");
}

@Test
void testStreamingWithValidFinishReason() {
// Setup
setupChatModel();

var choice = new ChunkChoice(ChatCompletionFinishReason.STOP, 0,
new ChatCompletionMessage("Complete response", Role.ASSISTANT), null);
ChatCompletionChunk chunk = new ChatCompletionChunk("id", List.of(choice), 666L, "model", null, null,
"chat.completion.chunk", null);

given(this.openAiApi.chatCompletionStream(isA(ChatCompletionRequest.class), any()))
.willReturn(Flux.just(chunk));

// Execute
Flux<ChatResponse> result = this.chatModel.stream(new Prompt("test"));

// Verify
List<ChatResponse> responses = result.collectList().block();
assertThat(responses).hasSize(1);
ChatResponse response = responses.get(0);
assertThat(response).isNotNull();
assertThat(response.getResult().getOutput().getText()).isEqualTo("Complete response");
assertThat(response.getResult().getMetadata().getFinishReason()).isEqualTo("STOP");
}

@Test
void testJsonDeserializationWithEmptyStringFinishReason() throws JsonProcessingException {
// Test the specific JSON from the issue report
String problematicJson = """
{
"id": "chatcmpl-msg_bdrk_012bpm3yfa9inEuftTWYQ46F",
"object": "chat.completion.chunk",
"created": 1726239401,
"model": "claude-3-5-sonnet-20240620",
"choices": [{
"index": 0,
"delta": {
"role": "assistant",
"content": ""
},
"finish_reason": ""
}]
}
""";

// This should either work correctly or throw a clear exception
ChatCompletionChunk chunk = ModelOptionsUtils.jsonToObject(problematicJson, ChatCompletionChunk.class);

// If deserialization succeeds, verify the structure
assertThat(chunk).isNotNull();
assertThat(chunk.choices()).hasSize(1);

var choice = chunk.choices().get(0);
assertThat(choice.index()).isEqualTo(0);
assertThat(choice.delta().content()).isEmpty();

// The key test: what happens with empty string finish_reason?
// This might be null if Jackson handles empty string -> enum conversion
// gracefully
assertThat(choice.finishReason()).isNull();
}

@Test
void testJsonDeserializationWithNullFinishReason() throws JsonProcessingException {
// Test with null finish_reason (should work fine)
String validJson = """
{
"id": "chatcmpl-test",
"object": "chat.completion.chunk",
"created": 1726239401,
"model": "gpt-4",
"choices": [{
"index": 0,
"delta": {
"role": "assistant",
"content": "Hello"
},
"finish_reason": null
}]
}
""";

ChatCompletionChunk chunk = ModelOptionsUtils.jsonToObject(validJson, ChatCompletionChunk.class);

assertThat(chunk).isNotNull();
assertThat(chunk.choices()).hasSize(1);

var choice = chunk.choices().get(0);
assertThat(choice.finishReason()).isNull();
assertThat(choice.delta().content()).isEqualTo("Hello");
}

@Test
void testStreamingWithEmptyStringFinishReasonUsingMockWebServer() {
// Setup
setupChatModel();

// Simulate the problematic response by creating a chunk that would result from
// deserializing JSON with empty string finish_reason
try {
// Try to create a chunk with what would happen if empty string was
// deserialized
var choice = new ChunkChoice(null, 0, new ChatCompletionMessage("", Role.ASSISTANT), null);
ChatCompletionChunk chunk = new ChatCompletionChunk("chatcmpl-msg_bdrk_012bpm3yfa9inEuftTWYQ46F",
List.of(choice), 1726239401L, "claude-3-5-sonnet-20240620", null, null, "chat.completion.chunk",
null);

given(this.openAiApi.chatCompletionStream(isA(ChatCompletionRequest.class), any()))
.willReturn(Flux.just(chunk));

// Execute
Flux<ChatResponse> result = this.chatModel.stream(new Prompt("test"));

// Verify that the streaming works even with null finish_reason
List<ChatResponse> responses = result.collectList().block();
assertThat(responses).hasSize(1);
ChatResponse response = responses.get(0);
assertThat(response).isNotNull();
assertThat(response.getResult().getMetadata().getFinishReason()).isEqualTo("");

}
catch (Exception e) {
// If this fails, it indicates the issue exists in our processing
System.out.println("Streaming failed with empty finish_reason: " + e.getMessage());
throw e;
}
}

@Test
void testModelOptionsUtilsJsonToObjectWithEmptyFinishReason() {
// Test the specific method mentioned in the issue
String jsonWithEmptyFinishReason = """
{
"id": "chatcmpl-msg_bdrk_012bpm3yfa9inEuftTWYQ46F",
"object": "chat.completion.chunk",
"created": 1726239401,
"model": "claude-3-5-sonnet-20240620",
"choices": [{
"index": 0,
"delta": {
"role": "assistant",
"content": ""
},
"finish_reason": ""
}]
}
""";

ChatCompletionChunk chunk = ModelOptionsUtils.jsonToObject(jsonWithEmptyFinishReason,
ChatCompletionChunk.class);

assertThat(chunk).isNotNull();
assertThat(chunk.choices()).hasSize(1);

var choice = chunk.choices().get(0);
// The critical test: how does ModelOptionsUtils handle empty string -> enum?
assertThat(choice.finishReason()).isNull();
}

private void setupChatModel() {
RetryTemplate retryTemplate = RetryUtils.DEFAULT_RETRY_TEMPLATE;
ToolCallingManager toolCallingManager = ToolCallingManager.builder().build();
ObservationRegistry observationRegistry = ObservationRegistry.NOOP;
this.chatModel = new OpenAiChatModel(this.openAiApi, OpenAiChatOptions.builder().build(), toolCallingManager,
retryTemplate, observationRegistry);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.databind.cfg.CoercionAction;
import com.fasterxml.jackson.databind.cfg.CoercionInputShape;
import com.fasterxml.jackson.databind.json.JsonMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
Expand Down Expand Up @@ -72,6 +74,13 @@ public abstract class ModelOptionsUtils {
.build()
.configure(DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT, true);

static {
// Configure coercion for empty strings to null for Enum types
// This fixes the issue where empty string finish_reason values cause
// deserialization failures
OBJECT_MAPPER.coercionConfigFor(Enum.class).setCoercion(CoercionInputShape.EmptyString, CoercionAction.AsNull);
}

private static final List<String> BEAN_MERGE_FIELD_EXCISIONS = List.of("class");

private static final ConcurrentHashMap<Class<?>, List<String>> REQUEST_FIELD_NAMES_PER_CLASS = new ConcurrentHashMap<>();
Expand Down
Loading