Skip to content

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

Merged
merged 2 commits into from
Mar 24, 2025
Merged

Stream tts #2661

merged 2 commits into from
Mar 24, 2025

Conversation

shaohuzhang1
Copy link
Contributor

What this PR does / why we need it?

Summary of your change

Please indicate you've done the following:

  • Made sure tests are passing and test coverage is added if needed.
  • Made sure commit message follow the rule of Conventional Commits specification.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed.

Copy link

f2c-ci-robot bot commented Mar 24, 2025

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.

Copy link

f2c-ci-robot bot commented Mar 24, 2025

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@@ -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)
}
Copy link
Contributor Author

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:

  1. Variable Naming Consistency: Use consistent naming conventions for variables and methods to improve code readability.
  2. Comments: Add comments for complex operations or logic flow to help other developers understand the code more easily.
  3. 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:

  1. Consistent Variable Names: Changed chunk_answer to chunk_content.
  2. Event Emission: Added a method finishProcessingResponse() which emits events at proper points.
  3. 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.
  4. Commented Functionality: Added docstrings and explanations where suitable to clarify complex sections of the code.
  5. 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"
Copy link
Contributor Author

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
})

Copy link
Contributor Author

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:

  1. Ensure that the function definitions (handleScroll, sendMessage) have been correctly defined elsewhere in your code.

  2. 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.

  3. 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.

  4. 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!

@shaohuzhang1 shaohuzhang1 merged commit dcee1b6 into main Mar 24, 2025
3 checks passed
@shaohuzhang1 shaohuzhang1 deleted the stream_tts branch March 24, 2025 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant