Preserve OpenAI response metadata#5929
Conversation
ilayaperumalg
left a comment
There was a problem hiding this comment.
Thanks for the PR! It's a great catch to preserve these additional properties.
However, there is one significant issue with leaking SDK-specific types into Spring AI's generic abstractions, and one minor refactoring suggestion:
- Leaking
com.openai.core.JsonValue:
By doingresult._additionalProperties().forEach(metadataBuilder::keyValue);, the map is populated with OpenAI SDK'sJsonValueobjects. As shown in the added test (metadata.<JsonValue>get(\non_null_field\)), this forces users to cast values to an OpenAI-specific class. This breaks Spring AI's portability—if a user switches to another provider, their code expectingJsonValuewill break.
Recommendation: Unwrap the JsonValue into standard Java types (String, Number, Boolean, List, Map) before adding them. You can use Jackson for this:
result._additionalProperties().forEach((key, jsonValue) -> {
try {
Object unmarshalled = ModelOptionsUtils.JSON_MAPPER.readValue(jsonValue.toString(), Object.class);
metadataBuilder.keyValue(key, unmarshalled);
} catch (Exception e) {
metadataBuilder.keyValue(key, jsonValue.toString());
}
});- Minor Map Copying Simplification:
Infrom(ChatResponseMetadata chatResponseMetadata, Usage usage), creating a temporary map via Streams is fine, but you could simplify it by iterating directly over the entries:
ChatResponseMetadata.Builder builder = ChatResponseMetadata.builder()
.id(chatResponseMetadata.getId())
.usage(usage)
.model(chatResponseMetadata.getModel());
chatResponseMetadata.entrySet().forEach(e -> builder.keyValue(e.getKey(), e.getValue()));
return builder.build();54748c3 to
3d452d1
Compare
Fixes spring-projectsGH-5922 (spring-projects#5922) Signed-off-by: Yaner <yaner@yaner-here.top>
efdd543 to
64fb781
Compare
|
|
||
| private static final ToolCallingManager DEFAULT_TOOL_CALLING_MANAGER = ToolCallingManager.builder().build(); | ||
|
|
||
| private static final ObjectMapper objectMapper = new ObjectMapper(); |
There was a problem hiding this comment.
Can we reuse the JSON_MAPPER from ModelOptionsUtils?
There was a problem hiding this comment.
@yaner-here The changes look good except the ObjectMapper usage. I can update when merging.
|
@yaner-here Thank you for the PR and the follow up updates! Rebased and merged via 145b250. Added a change to replace the ObjectMapper with |

Fixes #5922. Preserves unmapped root-level OpenAI chat completion response fields in
ChatResponseMetadata.The OpenAI SDK exposes provider-specific root fields through
ChatCompletion._additionalProperties(). These fields were previously dropped when Spring AI mappedChatCompletionintoChatResponseMetadata. This change copies those additional properties into the metadata map.