-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fix/transcript finish #3324
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
base: main
Are you sure you want to change the base?
Fix/transcript finish #3324
Conversation
Summary of ChangesHello @ChrisQlasty, 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 resolves an issue where the ADK (Agent Development Kit) was failing to properly propagate the Highlights
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.
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 correctly addresses the issue of the finished flag not being passed for input/output transcriptions by relaxing the validation conditions in GeminiLlmConnection.receive. The change is simple and effective. The accompanying unit test, test_receive_transcript_finished, properly verifies this new behavior for both input and output transcriptions. I've added a couple of suggestions to the new test code to improve its conciseness and maintainability by reducing code duplication. Overall, this is a good fix.
|
Hello, is there any chance to have this bug fixed? Also I see that in general 'live' models seem to not have big interest from the ADK users and developers which is quite surprising. |
|
This is a good contribution. Could you resolve conflicts so we can merge? Why do you think we have less interests from users? |
|
Please fix pyink, also close or resolve gemini code comments. |
Added a new test for receiving usage metadata and server content.
Refactored Msg class to use a single mock instance and simplified transcription response checks.
|
Thanks for your response @hangfei! I have resolved code comments and updated the PR according to merged main branch, so it looks clean again I believe. The reason I assumed 'less interest' is that there are only few PR's in the adk-python, which have the 'live' label assigned. But maybe much more of the PR's and issues deal with it under other labels as well. |
Problem:
ADK seems to not pass output/input transcription
finishedflag from the Gemini.Solution:
Relaxation of checking conditions of valid llm_response for input/output transcription.
Testing Plan
Unit test
test_receive_transcript_finishedchecks if input/output transcription message with no text butfinishedflag is received.Unit Tests:
Manual End-to-End (E2E) Tests:
Configure Gemini Agent to produce input & output transcriptions. Observe incoming transcript messages - to see if the finished flag appears at the end agents & users statement.
Checklist
Additional context
The mentioned
finishedflag can be obtained in native Gemini APIs like via Websocket but not via ADK.