Customized API endpoint might returns "usage": null rather than {"p…#482
Customized API endpoint might returns "usage": null rather than {"p…#482pillsilly wants to merge 1 commit intoAsyncFuncAI:mainfrom
Conversation
…mpt_tokens": N, "total_tokens": N}. Add implmenetation to fall back as handling when "usage" field is empty.
Summary of ChangesHello, 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 enhances the 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. Changelog
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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a fallback mechanism for parsing embedding responses, specifically for cases where the usage field might be null. The overall approach is sound and addresses the problem described. However, I've found a bug in the error handling logic within the new fallback path. If the fallback parsing itself fails, the system currently reports the original error, which could be confusing for debugging. I've provided a suggestion to improve the error reporting in this scenario to make it more accurate and helpful.
| except Exception as e2: | ||
| log.error(f"Fallback embedding parsing also failed: {e2}") | ||
| log.error(f"Error parsing the embedding response: {e}") | ||
| return EmbedderOutput(data=[], error=str(e), raw_response=response) |
There was a problem hiding this comment.
There is an issue with the error handling here. If the fallback parsing fails and this except block for e2 is entered, the code logs e2 but then proceeds to log the original error e and returns an EmbedderOutput with error=str(e). This is misleading because the more immediate cause of failure is e2.
The returned error should reflect that the fallback mechanism also failed. I suggest modifying this block to return immediately with a more informative error message that includes details from both exceptions.
except Exception as e2:
error_msg = f"Standard parsing failed ('{e}') and fallback parsing also failed ('{e2}')"
log.error(error_msg)
return EmbedderOutput(data=[], error=error_msg, raw_response=response)
log.error(f"Error parsing the embedding response: {e}")
return EmbedderOutput(data=[], error=str(e), raw_response=response)|
@sng-asyncfunc kindly help to check this PR. thanks. |
Customized API endpoint might returns "usage": null rather than {"prompt_tokens": N, "total_tokens": N}.
Add implementation to fall back as handling when "usage" field is empty.