[Fix] asr finished in on_event#69
Conversation
Summary of ChangesHello @lengjiayi, 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 in the Automatic Speech Recognition (ASR) module where the completion of a recognition segment might not be accurately detected. The change enhances the termination condition to account for cases where the output stream contains data but explicitly signals completion, ensuring more robust and reliable ASR processing. 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
|
Summary of ChangesHello @lengjiayi, 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 addresses a bug in the Automatic Speech Recognition (ASR) module where the completion of an ASR task might not have been correctly detected in all scenarios. The change ensures that the system accurately recognizes when an ASR process has finished, either by receiving an empty output or an explicit 'finished' flag, leading to more reliable timestamping of completion events. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request modifies the __receive_worker method in dashscope/audio/asr/recognition.py to include an additional condition for completing the audio stream. Specifically, it checks if the 'finished' key exists in part.output and if its value is True. This change ensures that the audio stream completes when the server explicitly signals the end of the stream, in addition to the existing check for an empty output.
| for part in responses: | ||
| if part.status_code == HTTPStatus.OK: | ||
| if len(part.output) == 0: | ||
| if len(part.output) == 0 or ('finished' in part.output and part.output['finished'] == True): |
There was a problem hiding this comment.
The current logic uses or which might lead to unexpected behavior. If len(part.output) == 0 is false but the second condition 'finished' in part.output and part.output['finished'] == True is true, the code will still proceed as if the condition is met. This could potentially cause issues if the intention is to only complete when both conditions are true. It is recommended to use and to ensure both conditions are met before proceeding.
Consider changing or to and to ensure that the complete timestamp is only set when both the output is empty and the 'finished' flag is present and true.
| if len(part.output) == 0 or ('finished' in part.output and part.output['finished'] == True): | |
| if len(part.output) == 0 and ('finished' in part.output and part.output['finished'] == True): |
There was a problem hiding this comment.
Code Review
This pull request addresses a fix in the ASR recognition worker to correctly handle the end of a task. The change adds a condition to check for a finished: true flag in the server response. My review includes a suggestion to refactor this condition for better readability and to adhere to Python best practices, making the code more concise and maintainable.
| for part in responses: | ||
| if part.status_code == HTTPStatus.OK: | ||
| if len(part.output) == 0: | ||
| if len(part.output) == 0 or ('finished' in part.output and part.output['finished'] == True): |
There was a problem hiding this comment.
The condition to check for completion can be simplified to be more concise and idiomatic Python.
- Checking for an empty dictionary can be done with
not part.output. - Checking for the presence and truthiness of the
'finished'key can be done withpart.output.get('finished'). This avoids verbose key existence checks. - Comparing a boolean value with
== Trueis generally discouraged.1
This makes the code more readable and maintainable.
| if len(part.output) == 0 or ('finished' in part.output and part.output['finished'] == True): | |
| if not part.output or part.output.get('finished'): |
Style Guide References
Footnotes
-
PEP 8 advises against comparing boolean values to
TrueorFalseusing==. ↩
2 participants
No description provided.