Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update to work with Google Gemini #428

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sepatel
Copy link

@sepatel sepatel commented Feb 21, 2025

Q A
Bug fix? yes
New feature? no
BC breaks? yes
Related Issue Fix #412

Describe your change

Enable the id on the ChatCompletion to be nullable. Unfortunately this does technically break backwards compatibility as it was defined as a required field but is not actually provided back by all services that are openai compatible. This is also in direct contradiction of the openai documented standard which states that the id is a required field as well.

What problem is this fixing?

The inability for Gemini to work correctly.

@sepatel
Copy link
Author

sepatel commented Feb 23, 2025

My original description saying that id is supposed to be a required field according to the docs is actually incorrect.

It is probably important to also note though that the gemini api's are actually openai compliant because the missing id field is actually not a required field.

https://platform.openai.com/docs/api-reference/chat/create
https://platform.openai.com/docs/api-reference/chat/object

@stardomains3
Copy link

Don't forget to change streaming chunk too. Google models responses fail with it also.

@Serializable
public final data class ChatCompletionChunk(
    id: String,
    created: Long,
    model: ModelId,
    choices: List<ChatChunk>,
    usage: Usage? = COMPILED_CODE,
    systemFingerprint: String? = COMPILED_CODE
)

@morki
Copy link

morki commented Mar 17, 2025

Thank you, it hits as as well.

@Melodeiro
Copy link

Melodeiro commented Mar 24, 2025

To not to break the backward compatibility, probably you could use some default value, maybe even autogenerated ID. As for extra parameters, i'd even provide the raw response string and ignore unknown fields, because there are many "openai-compatible" API's which adds extra params here and there (perplexity for example). Universally and easily it would be possible to extract it using JSON node parsing. Otherwise the response object might grow more and more nullable fields. Thats my considerations

Also added in citations to better handle perplexity's citations.
@Melodeiro
Copy link

Nice! Do you mind adding https://jina.ai/deepsearch/ as well?

@sepatel
Copy link
Author

sepatel commented Apr 6, 2025

To not to break the backward compatibility, probably you could use some default value

I'm not a fan of "faking" bad data and it creates a bad baseline as well. The official documentation states it can be null, the id value has specific meaning that if a person uses it for that meaning will get messed up by the fake data as well. It is better to break the compatibility or note it for people and move forward with the correct specifications that lineup with the official published documentation as well.

Don't forget to change streaming chunk too. Google models responses fail with it also.

Thanks, I completely forgot about that but I've now pushed up that fix too.

@Melodeiro
Copy link

Good point

@sepatel
Copy link
Author

sepatel commented Apr 6, 2025

Nice! Do you mind adding https://jina.ai/deepsearch/ as well?

I think that should be done as a separate PR actually as this one is focused around Gemini. Once it is merged in, I'm happy to add DeepSeek, DeepSearch, Groq, and a few other popular ones just to help make lives a bit easier for people.

@Melodeiro
Copy link

@sepatel Would you mind adding those to your repo while they aren't merged here, please?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when using Gemini OpenAI API
4 participants