-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[Vertex AI] Fix decoding ModalityTokenCount
when tokenCount
is 0
#14747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) 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 is currently in preview and 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 to provide feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses an issue where the tokenCount
field in ModalityTokenCount
was not being properly decoded when it was omitted from the JSON response (specifically when the value was 0). The changes include updating the ModalityTokenCount
struct to handle the missing tokenCount
field and adding a unit test to verify the fix. Overall, the changes are well-structured and address the issue effectively.
Summary of Findings
- Decoding
tokenCount
: The pull request correctly addresses the issue of decodingModalityTokenCount.tokenCount
when it is not returned in the response by usingdecodeIfPresent
and providing a default value of 0. - Unit test for missing
tokenCount
: The pull request includes a unit test (testDecodeModalityTokenCount_missingTokenCount_defaultsToZero
) to verify that thetokenCount
defaults to 0 when it is missing from the JSON response.
Merge Readiness
The pull request is well-structured and includes a unit test to verify the fix. The changes address the issue effectively and are ready to be merged. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM on acceptable CI
Decodes
ModalityTokenCount.tokenCount
as0
when it is not returned in the response. This fixes #14745.Context: When the backend sets a
tokenCount
of0
for aContentModality
in aModalityTokenCount
, thetokenCount
field is omitted from the JSON response since0
is the default value. This results in aDecodingError.keyNotFound
when decoding thetokenCount
.#no-changelog