-
Notifications
You must be signed in to change notification settings - Fork 86
fix(coze/api): fix speech bug #141
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
Conversation
WalkthroughThis pull request introduces several updates related to the speech functionality in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant useWsAPI
participant WsSpeechClient
User->>useWsAPI: Call startSpeech()
alt No existing client
useWsAPI->>WsSpeechClient: Instantiate new client
WsSpeechClient-->>useWsAPI: Client instance created
useWsAPI->>WsSpeechClient: Attach event listeners ('data', 'error', 'completed')
else Existing client
useWsAPI->>WsSpeechClient: Reuse existing client
end
useWsAPI->>WsSpeechClient: Call appendAndComplete()
User->>useWsAPI: Call stopSpeech()
useWsAPI->>WsSpeechClient: Call disconnect()
sequenceDiagram
participant Client as WsSpeechClient
participant Player as wavStreamPlayer
participant WS as WebSocket
Client->>Client: Check if playbackTimeout exists
alt Timeout is set
Client->>Client: Clear playbackTimeout
end
Client->>Player: Call interrupt() on wavStreamPlayer
Client->>WS: Call closeWs() to close the connection
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
examples/coze-js-web/src/pages/chat-x/use-ws-api.ts (3)
330-330: Consider removing redundant optional chainingSince you've already checked that
speechClientRef.currentexists in the if condition above, the optional chaining operator here is redundant.- await speechClientRef.current?.connect(); + await speechClientRef.current.connect();
337-337: Consider removing redundant optional chainingSimilar to the connect call, the optional chaining is redundant here since we've already verified the client exists.
- speechClientRef.current?.appendAndComplete(message); + speechClientRef.current.appendAndComplete(message);
302-338: Consider adding cleanup for speech clientThe component doesn't clean up the speech client when unmounted, which could lead to resource leaks. Consider adding a cleanup function in a useEffect hook.
+ useEffect(() => { + // Cleanup function to disconnect speech client when component unmounts + return () => { + if (speechClientRef.current) { + speechClientRef.current.disconnect(); + speechClientRef.current = null; + } + }; + }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
common/changes/@coze/api/fix-speech_2025-03-12-13-51.json(1 hunks)examples/coze-js-web/src/pages/chat-x/use-ws-api.ts(1 hunks)packages/coze-js/README.md(2 hunks)packages/coze-js/README.zh-CN.md(2 hunks)packages/coze-js/package.json(1 hunks)packages/coze-js/src/ws-tools/speech/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Node.js v20 (ubuntu-latest)
🔇 Additional comments (11)
packages/coze-js/package.json (1)
3-3: Version update aligns with speech functionality patchThe version bump from "1.1.0-beta.2" to "1.1.0-beta.3" correctly follows semantic versioning for a patch release that addresses the speech bug fix.
common/changes/@coze/api/fix-speech_2025-03-12-13-51.json (1)
1-11: Well-structured change log entryThe change log properly documents the speech bug fix as a patch-level update, which aligns with the version bump in package.json and the actual code changes.
packages/coze-js/src/ws-tools/speech/index.ts (1)
137-142: Improved resource cleanup in disconnect methodThe enhanced disconnect method now properly clears the playbackTimeout and directly interrupts the wavStreamPlayer, which prevents potential memory leaks and ensures cleaner resource management compared to the previous implementation.
This approach is better than the previous implementation because:
- It properly handles the timeout cleanup
- It directly calls the wavStreamPlayer's interrupt method rather than going through the internal interrupt method that would emit events and reset tracking information
packages/coze-js/README.zh-CN.md (3)
217-217: Updated example with explicit Chinese textReplacing the variable with the actual Chinese text makes the example more concrete and easier to understand.
235-237: Improved documentation structureMoving the disconnect comment and method call to this position in the example improves the logical flow of the code example.
239-240: Clearer example with explicit text segmentsBreaking the append operation into two explicit text segments better demonstrates how the append method can be used for partial text synthesis.
packages/coze-js/README.md (3)
218-218: Updated example to use static string instead of variableThe example has been updated to use a static string 'Hello, Coze!' instead of a variable message. This change aligns with the simplified examples throughout the documentation and makes the intent clearer.
236-238: Improved client lifecycle managementThe README now properly demonstrates the disconnection step before the append operations, which aligns with the intended usage pattern. This change helps developers understand the correct sequence of operations when working with the speech client.
240-241: Improved example for append operationsThe example now shows how to send multiple text fragments separately with
append(), which better demonstrates the incremental nature of the API compared to the previous example.examples/coze-js-web/src/pages/chat-x/use-ws-api.ts (2)
309-327: Improved client instance managementThe code now reuses the existing speech client instance instead of recreating it on each call to
startSpeech(). This change improves resource management and prevents potential memory leaks.Additionally, the code now properly sets up event listeners for completed speech events, which improves the feedback mechanism.
341-342: Updated disconnection methodChanged from using
interrupt()todisconnect(), which properly cleans up resources. This aligns with the changes in the documentation and ensures a complete disconnection of the client.
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #141 +/- ##
==========================================
- Coverage 91.82% 90.69% -1.13%
==========================================
Files 117 76 -41
Lines 5469 3752 -1717
Branches 1106 684 -422
==========================================
- Hits 5022 3403 -1619
+ Misses 447 347 -100
- Partials 0 2 +2
... and 72 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Summary by CodeRabbit