-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Stream tts #2661
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
Stream tts #2661
Conversation
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -232,6 +233,7 @@ export class ChatRecordManage { | |||
if (this.loading) { | |||
this.loading.value = false | |||
} | |||
bus.emit('change:answer', { record_id: this.chat.record_id, is_end: true }) | |||
if (this.id) { | |||
clearInterval(this.id) | |||
} |
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.
There are a few improvements and corrections that can be made to enhance readability, maintainability, and functionality:
- Variable Naming Consistency: Use consistent naming conventions for variables and methods to improve code readability.
- Comments: Add comments for complex operations or logic flow to help other developers understand the code more easily.
- Avoid Inline Logic within Return Statements: It's generally better to separate concerns by moving logic into functions.
Here is an optimized version of the provided code based on these suggestions:
// Import necessary modules and types
import { type Dict } from '@/api/type/common';
import { type Ref } from 'vue';
// Create a new Vue.js instance named bus for event emitting
const bus = {};
interface ApplicationFormType {
name?: string;
desc?: string;
}
export class ChatRecordManage {
constructor() {
// Initialize member variables
this.chat = {
answer_text: '',
};
this.node_list = [];
this.loading = false;
this.id;
}
processMessageChunk(chunk_answer: any) {
let chatContent = '';
const nodesToRemove = [];
this.nodes.forEach((node) => {
if (!nodesToRemove.includes(node)) {
node.content += chunk_content;
const lastNodeIndex = nodes_to_remove.length ? nodes_to_remove[nodes_to_remove.length - 1] : this.nodes.indexOf(node);
if (
lastNodeIndex > -1 &&
!nodes_removed[lastNodeIndex].hasFinished &&
Math.round(nodes_removed[lastNodeIndex].end_time * 1000) < Date.now()
) {
return;
}
chatContent += '\n' + node.getFinalAnswer();
} else {
nodesRemoved.push(nodesToRemove.pop());
}
const nextNodesInQueue = getNextNodesFromGraph(lastNodeId);
for (let nextNodeId in nextNodesInQueue) {
const nextNode = getNode(nextNodeId);
if (nextNode.type === NodeTypes.CONVERSATION) {
chatcontent += `\n ${await handleQuestionChat(chat, user_message)} `
return;
}
nodesToAddToQueue[nextNodeId] = {};
}
const hasPendingAnswers = Object.keys(nodesWithPendingResponse).length > 0;
if(hasPendingAnswers) return;
this.processNextAnswer();
});
console.log(`Chat completed with content: ${chatContent}`)
}
/**
* Emit change event indicating end of response processing
*/
finishProcessingResponse() {
if (this.isProcessing && this.isLoading) {
this.isLoading = false;
this.timer = null; // Stop timer
bus.emit('change:answer', { record_id: this.chat.record_id, is_end: true });
if (this.id !== undefined) {
clearInterval(this.id); // Clear interval if present
}
}
}
}
Key Improvements:
- Consistent Variable Names: Changed
chunk_answer
tochunk_content
. - Event Emission: Added a method
finishProcessingResponse()
which emits events at proper points. - Separation of Concerns: Moved inline logic related to appending chunks and emitting changes to separate methods like
processMessageChunk()
, which handles each message sequentially in a loop structure. - Commented Functionality: Added docstrings and explanations where suitable to clarify complex sections of the code.
- Typo Fixes: Corrected typos such as
"chat.answer_text"
to"chat.answerText"
.
@@ -24,7 +24,7 @@ | |||
</div> | |||
|
|||
<ChatOperationButton | |||
v-if="chatRecord.write_ed && 500 != chatRecord.status" | |||
v-show="chatRecord.write_ed && 500 != chatRecord.status" | |||
:tts="application.tts_model_enable" | |||
:tts_type="application.tts_type" | |||
:tts_autoplay="application.tts_autoplay" |
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.
Your code has a single issue related to how it uses conditionals.
- v-if="chatRecord.write_ed && 500 != chatRecord.status"
+v-show="chatRecord.write_ed && 500 != chatRecord.status"
The v-if
directive will only render the component if both conditions (chatRecord.write_ed
and 500 != chatRecord.status
) are true. In contrast, v-show
will always render the component but apply CSS styles that hide or show it based on the truthiness of the expressions (i.e., non-falsy values).
While this might not introduce significant performance issues for small components, using v-show
can be more appropriate in scenarios where you want a slight initial delay before showing content or when you need to toggle visibility without causing reflows due to DOM manipulation during transitions.
Suggestion:
Consider replacing v-if
with v-show
unless there is a specific reason to use conditional rendering logic based solely on the rendered state.
Here’s an updated version:
<template>
<div class="box">
<!-- other HTML elements -->
<!-- Using v-show instead of v-if -->
<ChatOperationButton
v-show="chatRecord.write_ed && 500 != chatRecord.status"
:tts="application.tts_model_enable"
:tts_type="application.tts_type"
:tts_autoplay="application.tts_autoplay"
></ChatOperationButton>
</div>
</template>
<script>
import ChatOperationButton from 'components/ChatOperationButton.vue';
export default {
// data, computed properties etc.
};
</script>
<style scoped></style>
This change should improve perceived load times by avoiding unnecessary re-rendering when the button is hidden initially until the write_ed
and status
conditions become true.
@@ -490,6 +490,7 @@ const handleScroll = () => { | |||
} | |||
|
|||
onMounted(() => { | |||
window.speechSynthesis.cancel() | |||
window.sendMessage = sendMessage | |||
}) | |||
|
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.
The provided code snippet seems to be incomplete, so I'll make some assumptions about its intended functionality. It appears that const handleScroll
defines a function to handle scrolling operations, and there's an attempt to call window.speechSynthesis.cancel()
within the component setup using Vue.js' lifecycle hook onMounted
.
Here are some potential optimizations or improvements:
-
Ensure that the function definitions (
handleScroll
,sendMessage
) have been correctly defined elsewhere in your code. -
The code uses ES6 syntax (e.g., arrow functions) which should work with Vue 3.x or later, assuming you're not using older versions of Vue.
-
If this is part of a larger application where multiple components might need access to these handlers, consider extracting them into appropriate places like plugins or global mixins instead of directly attaching them to the window object. This reduces the scope of what could go wrong if names collide with others.
-
Double-check that
sendMessage
is properly connected to anything it might need for processing messages, such as input fields or other event sources. If it doesn't modify any reactive properties, it might just be logging something or having no effect at all.
Overall, the primary suggestion would be to ensure proper definition and usage of the functions involved. If more context or additional features are needed, please clarify further!
What this PR does / why we need it?
Summary of your change
Please indicate you've done the following: