fix(openai): add index field to image response for LiteLLM compatibility #63
Conversation
LiteLLM's Pydantic model requires an index field in each image object. Without it, responses fail validation with "images.0.index Field required".
Summary of ChangesHello @tinyc0der, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a compatibility issue with LiteLLM by enhancing the structure of OpenAI-compatible image responses. It introduces an Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly adds the index field to image objects in OpenAI-compatible responses, which resolves the Pydantic validation error in LiteLLM. The implementation is straightforward and addresses the issue described. My review focuses on improving the maintainability of the changes. The logic for creating the image payload has been duplicated in four different places across three files. I've left comments suggesting a refactoring to a shared helper function to reduce this duplication, which will make the code easier to maintain in the future. I've also suggested using a struct for JSON marshaling for better type safety and readability, and adding logging for marshaling errors where a logger is available.
| imageIndex := len(gjson.Get(template, "choices.0.delta.images").Array()) | ||
| imagePayload, err := json.Marshal(map[string]any{ | ||
| "index": imageIndex, | ||
| "type": "image_url", | ||
| "image_url": map[string]string{ | ||
| "url": imageURL, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| template, _ = sjson.Set(template, "choices.0.delta.role", "assistant") | ||
| template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", imagePayload) | ||
| template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", string(imagePayload)) |
There was a problem hiding this comment.
This block of code for creating and appending an image payload is duplicated in other response translator files (gemini-cli_openai_response.go, gemini_openai_response.go). To improve maintainability and reduce redundancy, consider extracting this logic into a single, reusable helper function in a shared package.
For better type safety and readability, it's also recommended to use a struct for JSON marshaling instead of a map[string]any.
Finally, it would be beneficial to log the error from json.Marshal to aid in debugging, as it's currently being silently ignored.
imageIndex := len(gjson.Get(template, "choices.0.delta.images").Array())
type imagePayload struct {
Index int `json:"index"`
Type string `json:"type"`
ImageURL map[string]string `json:"image_url"`
}
payload := imagePayload{
Index: imageIndex,
Type: "image_url",
ImageURL: map[string]string{
"url": imageURL,
},
}
imagePayloadBytes, err := json.Marshal(payload)
if err != nil {
log.Warnf("antigravity openai response: failed to marshal image payload: %v", err)
continue
}
template, _ = sjson.Set(template, "choices.0.delta.role", "assistant")
template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", string(imagePayloadBytes))| imageIndex := len(gjson.Get(template, "choices.0.delta.images").Array()) | ||
| imagePayload, err := json.Marshal(map[string]any{ | ||
| "index": imageIndex, | ||
| "type": "image_url", | ||
| "image_url": map[string]string{ | ||
| "url": imageURL, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| template, _ = sjson.Set(template, "choices.0.delta.role", "assistant") | ||
| template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", imagePayload) | ||
| template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", string(imagePayload)) |
There was a problem hiding this comment.
This block of code for creating and appending an image payload is duplicated in other response translator files (antigravity_openai_response.go, gemini_openai_response.go). To improve maintainability and reduce redundancy, consider extracting this logic into a single, reusable helper function in a shared package.
Additionally, for better type safety and readability, it's recommended to use a struct for JSON marshaling instead of a map[string]any.
imageIndex := len(gjson.Get(template, "choices.0.delta.images").Array())
type imagePayload struct {
Index int `json:"index"`
Type string `json:"type"`
ImageURL map[string]string `json:"image_url"`
}
payload := imagePayload{
Index: imageIndex,
Type: "image_url",
ImageURL: map[string]string{
"url": imageURL,
},
}
imagePayloadBytes, err := json.Marshal(payload)
if err != nil {
continue
}
template, _ = sjson.Set(template, "choices.0.delta.role", "assistant")
template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", string(imagePayloadBytes))| imageIndex := len(gjson.Get(template, "choices.0.delta.images").Array()) | ||
| imagePayload, err := json.Marshal(map[string]any{ | ||
| "index": imageIndex, | ||
| "type": "image_url", | ||
| "image_url": map[string]string{ | ||
| "url": imageURL, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| template, _ = sjson.Set(template, "choices.0.delta.role", "assistant") | ||
| template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", imagePayload) | ||
| template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", string(imagePayload)) |
There was a problem hiding this comment.
This block of code for creating and appending an image payload is duplicated elsewhere in this file and in other response translator files. To improve maintainability and reduce redundancy, consider extracting this logic into a single, reusable helper function.
For better type safety and readability, it's also recommended to use a struct for JSON marshaling instead of a map[string]any.
Finally, it would be beneficial to log the error from json.Marshal to aid in debugging.
imageIndex := len(gjson.Get(template, "choices.0.delta.images").Array())
type imagePayload struct {
Index int `json:"index"`
Type string `json:"type"`
ImageURL map[string]string `json:"image_url"`
}
payload := imagePayload{
Index: imageIndex,
Type: "image_url",
ImageURL: map[string]string{
"url": imageURL,
},
}
imagePayloadBytes, err := json.Marshal(payload)
if err != nil {
log.Warnf("gemini openai response: failed to marshal image payload in streaming: %v", err)
continue
}
template, _ = sjson.Set(template, "choices.0.delta.role", "assistant")
template, _ = sjson.SetRaw(template, "choices.0.delta.images.-1", string(imagePayloadBytes))| imageIndex := len(gjson.Get(template, "choices.0.message.images").Array()) | ||
| imagePayload, err := json.Marshal(map[string]any{ | ||
| "index": imageIndex, | ||
| "type": "image_url", | ||
| "image_url": map[string]string{ | ||
| "url": imageURL, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| template, _ = sjson.Set(template, "choices.0.message.role", "assistant") | ||
| template, _ = sjson.SetRaw(template, "choices.0.message.images.-1", imagePayload) | ||
| template, _ = sjson.SetRaw(template, "choices.0.message.images.-1", string(imagePayload)) |
There was a problem hiding this comment.
This block of code for creating and appending an image payload is duplicated elsewhere in this file and in other response translator files. To improve maintainability and reduce redundancy, consider extracting this logic into a single, reusable helper function.
For better type safety and readability, it's also recommended to use a struct for JSON marshaling instead of a map[string]any.
Finally, it would be beneficial to log the error from json.Marshal to aid in debugging.
imageIndex := len(gjson.Get(template, "choices.0.message.images").Array())
type imagePayload struct {
Index int `json:"index"`
Type string `json:"type"`
ImageURL map[string]string `json:"image_url"`
}
payload := imagePayload{
Index: imageIndex,
Type: "image_url",
ImageURL: map[string]string{
"url": imageURL,
},
}
imagePayloadBytes, err := json.Marshal(payload)
if err != nil {
log.Warnf("gemini openai response: failed to marshal image payload in non-streaming: %v", err)
continue
}
template, _ = sjson.Set(template, "choices.0.message.role", "assistant")
template, _ = sjson.SetRaw(template, "choices.0.message.images.-1", string(imagePayloadBytes))
1 participant
Summary
indexfield to image objects in OpenAI-compatible responsesTest plan
indexfield