change embedding response object to list for openai compatibility #444
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Changes the OpenAI embedding response object type from 'embedding' to 'list' to align with OpenAI's API specification, improving compatibility with external services like KubeAI.
- Changed
objectfield enum/const from 'embedding' to 'list' in/docs/assets/openapi.jsonOpenAIEmbeddingResult schema - Updated
OpenAIEmbeddingResultclass in/libs/infinity_emb/infinity_emb/fastapi_schemas/pymodels.pyto use 'list' as object type - Added test file
/libs/infinity_emb/tests/unit_test/fastapi_schemas/test_response.pyto verify response object is 'list' - Potential backwards compatibility concerns for clients relying on 'embedding' value need consideration
3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| class OpenAIEmbeddingResult(BaseModel): | ||
| object: Literal["embedding"] = "embedding" | ||
| object: Literal["list"] = "list" | ||
| data: list[_EmbeddingObject] |
There was a problem hiding this comment.
logic: This is a breaking change that could affect existing clients. Consider adding a version flag or deprecation period to maintain backward compatibility.
| def test_embedding_response(): | ||
| res = OpenAIEmbeddingResult(data=[], model="hi", usage={"prompt_tokens": 5, "total_tokens": 10}) | ||
| assert res.model_dump()["object"] == "list" |
There was a problem hiding this comment.
style: test only verifies object field, should also verify data, model and usage fields are present and correctly structured
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #444 +/- ##
=======================================
Coverage 79.18% 79.18%
=======================================
Files 41 41
Lines 3248 3248
=======================================
Hits 2572 2572
Misses 676 676 ☔ View full report in Codecov by Sentry. |
|
Thanks! Seems like I missed that! |
Description
Return list instead of embedding in response to be compatible with openai.
Respone definition from OpenAI see here
Related Issue
#371
Types of Change
Checklist
Additional Notes
Maybe we need to add backwards compatibility if someone relies on the embedding field.
License
By submitting this PR, I confirm that my contribution is made under the terms of the MIT license.