feat: Implement WebSocket support #156
Conversation
WalkthroughThis pull request introduces comprehensive WebSocket functionality to the Coze library, enabling real-time audio, speech, and chat interactions. The changes span multiple modules, adding synchronous and asynchronous WebSocket clients, event handlers, and supporting infrastructure. Key additions include WebSocket event management, audio speech and transcription clients, chat WebSocket capabilities, and utility functions for API token and base URL retrieval. The implementation provides a flexible framework for handling various WebSocket-based communication scenarios with structured event handling and error management. Changes
Sequence DiagramsequenceDiagram
participant User
participant CozeClient
participant WebSocketClient
participant AudioSpeechClient
participant ChatClient
User->>CozeClient: Initialize with credentials
CozeClient->>WebSocketClient: Create WebSocket connection
WebSocketClient->>AudioSpeechClient: Configure audio speech
WebSocketClient->>ChatClient: Configure chat
User->>AudioSpeechClient: Send text for speech synthesis
AudioSpeechClient-->>User: Stream audio updates
User->>ChatClient: Start conversation
ChatClient-->>User: Stream message and audio deltas
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🔭 Outside diff range comments (2)
examples/chat_stream.py (1)
Line range hint
29-46: Add error handling for stream iterationThe stream iteration lacks error handling which could lead to unclear failures.
-for event in coze.chat.stream( - bot_id=bot_id, - user_id=user_id, - additional_messages=[ - Message.build_user_question_text("Tell a 500-word story."), - ], -): +try: + for event in coze.chat.stream( + bot_id=bot_id, + user_id=user_id, + additional_messages=[ + Message.build_user_question_text("Tell a 500-word story."), + ], + ): + if event.event == ChatEventType.CONVERSATION_MESSAGE_DELTA: + print(event.message.content, end="", flush=True) + + if event.event == ChatEventType.CONVERSATION_CHAT_COMPLETED: + print() + print("token usage:", event.chat.usage.token_count) +except Exception as e: + print(f"Error during chat stream: {str(e)}") + exit(1)cozepy/audio/transcriptions/__init__.py (1)
Line range hint
15-38: Update response type and docstrings for consistencyThe class has been renamed to
TranscriptionsClient, but related types and documentation still use "translation".-class CreateTranslationResp(CozeModel): +class CreateTranscriptionResp(CozeModel): # The text of translation text: str class TranscriptionsClient(object): def create( self, *, file: FileTypes, **kwargs, - ) -> CreateTranslationResp: + ) -> CreateTranscriptionResp: """ - create translation + create transcription :param file: The file to be transcribed. - :return: create translation result + :return: create transcription result """
🧹 Nitpick comments (15)
examples/utils/__init__.py (1)
7-15: Consider adding logging for fallback caseThe function correctly handles the API base URL retrieval, but it would be helpful to log when falling back to the default URL for debugging purposes.
def get_coze_api_base() -> str: # The default access is api.coze.com, but if you need to access api.coze.cn, # please use base_url to configure the api endpoint to access coze_api_base = os.getenv("COZE_API_BASE") if coze_api_base: return coze_api_base + + import logging + logging.info("COZE_API_BASE not set, falling back to COZE_CN_BASE_URL") return COZE_CN_BASE_URL # defaultexamples/chat_stream.py (1)
Line range hint
21-23: Replace placeholder values with meaningful examplesThe current example uses placeholder values which might not be helpful for users.
-bot_id = os.getenv("COZE_BOT_ID") or "bot id" +bot_id = os.getenv("COZE_BOT_ID") or "123456" # Example bot ID, replace with your actual bot ID -user_id = "user id" +user_id = "example_user_123" # Example user ID, replace with your actual user identifierAlso, consider adding a comment explaining the format and requirements for these IDs.
cozepy/websockets/chat/__init__.py (1)
131-140: Refactor conditional statements for clarityIn the
_load_eventmethod, the consecutiveifstatements can be changed toelifto improve code readability and prevent unnecessary checks after a match is found.Apply this diff to adjust the conditional statements:
- if event_type == EventType.CONVERSATION_MESSAGE_DELTA.value: + elif event_type == EventType.CONVERSATION_MESSAGE_DELTA.value:Repeat this change for the subsequent conditions:
- if event_type == EventType.CONVERSATION_CHAT_COMPLETED.value: + elif event_type == EventType.CONVERSATION_CHAT_COMPLETED.value:- if event_type == EventType.CONVERSATION_AUDIO_DELTA.value: + elif event_type == EventType.CONVERSATION_AUDIO_DELTA.value:cozepy/websockets/ws.py (1)
174-181: Translate code comments to English for consistencyThe comments in the
_wait_completedmethod are in Chinese. For better maintainability and collaboration with a broader development team, it's recommended to use English for all code comments.Apply this diff to translate the comments:
- # 创建一个 Future 对象来控制阻塞 + # Create a Future object to control blocking future = asyncio.Future() - # 保存原始的事件处理函数 + # Save the original event handlers original_handlers = {} - # 用于跟踪已完成的事件 + # Used to track completed events completed_events = set() - # 定义一个内部处理函数来处理完成事件 + # Define an internal handler function to process completed events async def _handle_completed(client, event):Also applies to: 183-184
cozepy/websockets/audio/__init__.py (1)
15-21: Singleton pattern for client instancesCurrently, each access to the
transcriptionsandspeechproperties creates a new instance of the respective client. This may lead to unexpected behavior if stateful interactions are involved. Consider storing the instances to ensure only one instance per client is used.Apply this diff to implement the singleton pattern:
class AsyncWebsocketsAudioClient(object): def __init__(self, base_url: str, auth: Auth, requester: Requester): self._base_url = base_url self._auth = auth self._requester = requester + self._transcriptions_client = None + self._speech_client = None @property def transcriptions(self) -> "AsyncWebsocketsAudioTranscriptionsClient": - return AsyncWebsocketsAudioTranscriptionsClient( - base_url=self._base_url, - auth=self._auth, - requester=self._requester, - ) + if self._transcriptions_client is None: + self._transcriptions_client = AsyncWebsocketsAudioTranscriptionsClient( + base_url=self._base_url, + auth=self._auth, + requester=self._requester, + ) + return self._transcriptions_client @property def speech(self) -> "AsyncWebsocketsAudioSpeechClient": - return AsyncWebsocketsAudioSpeechClient( - base_url=self._base_url, - auth=self._auth, - requester=self._requester, - ) + if self._speech_client is None: + self._speech_client = AsyncWebsocketsAudioSpeechClient( + base_url=self._base_url, + auth=self._auth, + requester=self._requester, + ) + return self._speech_clientAlso applies to: 23-28
cozepy/websockets/__init__.py (2)
16-21: Implement caching for client instancesSimilar to the
AsyncWebsocketsAudioClient, theaudioandchatproperties inAsyncWebsocketsClientcreate new client instances on each access. To improve performance and maintain state, consider caching these instances.Apply this diff to cache the client instances:
class AsyncWebsocketsClient(object): def __init__(self, base_url: str, auth: Auth, requester: Requester): self._base_url = http_base_url_to_ws(remove_url_trailing_slash(base_url)) self._auth = auth self._requester = requester + self._audio_client = None + self._chat_client = None @property def audio(self) -> AsyncWebsocketsAudioClient: - return AsyncWebsocketsAudioClient( - base_url=self._base_url, - auth=self._auth, - requester=self._requester, - ) + if self._audio_client is None: + self._audio_client = AsyncWebsocketsAudioClient( + base_url=self._base_url, + auth=self._auth, + requester=self._requester, + ) + return self._audio_client @property def chat(self) -> AsyncWebsocketsChatClient: - return AsyncWebsocketsChatClient( - base_url=self._base_url, - auth=self._auth, - requester=self._requester, - ) + if self._chat_client is None: + self._chat_client = AsyncWebsocketsChatClient( + base_url=self._base_url, + auth=self._auth, + requester=self._requester, + ) + return self._chat_clientAlso applies to: 23-29
1-3: Organize imports for better readabilityConsider grouping standard library imports separately from third-party and local imports for improved clarity.
Apply this diff to reorganize the imports:
from cozepy import Auth from cozepy.request import Requester from cozepy.util import http_base_url_to_ws, remove_url_trailing_slash +from .audio import AsyncWebsocketsAudioClient +from .chat import AsyncWebsocketsChatClient +tests/test_audio_translations.py (2)
22-22: Update mock endpoint name for consistencyThe mock endpoint has been updated to use
/v1/audio/transcriptions, but the function namemock_create_translationstill refers to "translation".Consider renaming the function to
mock_create_transcriptionfor consistency with the new endpoint name.
Line range hint
27-37: Rename test classes for consistencyThe test classes still use "Translation" in their names despite the shift to "transcriptions" in the implementation.
Consider renaming:
TestAudioTranslation→TestAudioTranscriptionTestAsyncAudioTranslation→TestAsyncAudioTranscriptionAlso applies to: 41-51
examples/websockets_audio_speech.py (1)
56-59: Add type hints and docstrings for WebSocket operationsThe WebSocket operations lack type hints and documentation.
async with speech() as client: - await client.append(text) + await client.append(text: str) -> None: + """Append text to be converted to speech. + + Args: + text: The text to be converted to speech + """ await client.commit() await client.wait()examples/websockets_audio_transcriptions.py (3)
16-19: Add environment variable validationConsider adding validation for the
COZE_LOGandCOZE_KWARGSenvironment variables to handle missing or malformed values gracefully.coze_log = os.getenv("COZE_LOG") if coze_log: + try: setup_logging(logging.getLevelNamesMapping()[coze_log.upper()]) + except KeyError: + print(f"Warning: Invalid log level '{coze_log}'. Using default.") + +try: kwargs = json.loads(os.getenv("COZE_KWARGS") or "{}") +except json.JSONDecodeError: + print("Warning: Invalid COZE_KWARGS format. Using empty dict.") + kwargs = {}
28-30: Enhance error handling in the error handlerThe current error handler only prints the error message. Consider adding proper error logging and potentially implementing retry logic for recoverable errors.
async def handle_error(client: AsyncWebsocketsAudioTranscriptionsCreateClient, e: Exception): - print(f"Error occurred: {str(e)}") + import logging + logging.error(f"WebSocket error occurred: {str(e)}", exc_info=True) + if isinstance(e, (ConnectionError, TimeoutError)): + logging.info("Attempting to reconnect...") + await client.reconnect()
53-83: Externalize configuration and improve internationalization supportThe main function has hardcoded values that should be configurable:
- The Chinese text should be externalized for better i18n support
- Consider adding configuration for retry attempts and timeouts
+from typing import Optional + +class Config: + def __init__(self): + self.text = os.getenv("COZE_TEXT", "你今天好吗? 今天天气不错呀") + self.max_retries = int(os.getenv("COZE_MAX_RETRIES", "3")) + self.timeout = int(os.getenv("COZE_TIMEOUT", "30")) + async def main(): coze_api_token = get_coze_api_token() coze_api_base = get_coze_api_base() + config = Config() # Initialize Coze client coze = AsyncCoze( auth=TokenAuth(coze_api_token), base_url=coze_api_base, ) # Initialize Audio - speech_stream = wrap_coze_speech_to_iterator(coze, "你今天好吗? 今天天气不错呀") + speech_stream = wrap_coze_speech_to_iterator(coze, config.text)cozepy/coze.py (1)
155-155: Add docstring for the websockets propertyPlease add documentation for the new WebSocket functionality to help users understand its purpose and usage.
@property def websockets(self) -> "AsyncWebsocketsClient": + """ + Returns a WebSocket client for real-time communication. + + This client provides access to WebSocket-based features such as: + - Real-time audio transcriptions + - Interactive chat sessions + + Returns: + AsyncWebsocketsClient: A client for WebSocket operations + """ if not self._websockets: from .websockets import AsyncWebsocketsClient self._websockets = AsyncWebsocketsClient(self._base_url, self._auth, self._requester) return self._websocketsAlso applies to: 243-249
cozepy/chat/__init__.py (1)
28-29: Consider using a more descriptive value for UNKNOWN enum.While adding UNKNOWN values to enums is good practice, using an empty string might not be the most descriptive choice. Consider using "unknown" as the value instead.
Apply this diff:
- UNKNOWN = "" + UNKNOWN = "unknown"Also applies to: 202-203
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.gitignore(1 hunks)cozepy/__init__.py(2 hunks)cozepy/audio/__init__.py(5 hunks)cozepy/audio/transcriptions/__init__.py(3 hunks)cozepy/audio/voices/__init__.py(3 hunks)cozepy/chat/__init__.py(6 hunks)cozepy/coze.py(3 hunks)cozepy/util.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)examples/chat_stream.py(1 hunks)examples/utils/__init__.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)pyproject.toml(1 hunks)tests/test_audio_translations.py(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- cozepy/init.py
- .gitignore
🔇 Additional comments (8)
cozepy/websockets/ws.py (2)
165-167: Handle the case where_load_eventreturnsNoneIn the
_receive_loop, if_load_eventreturnsNone, the handler is not called. Ensure that this behavior is intended. If not, consider adding a warning or handling the situation appropriately.
113-124: Verify compatibility with thewebsocketslibrary versionThe
websockets.connectfunction parameters may have changed in different versions of the library. Specifically, the use ofuser_agent_headerandadditional_headersshould be verified to ensure compatibility with the installed version.Run the following script to check the
websocketslibrary version and review theconnectmethod's parameters:✅ Verification successful
Parameters are compatible with specified websockets versions
The code uses parameters that are supported in the websockets library versions specified in pyproject.toml (v14.1.0 for Python ≥3.9 and v13.1.0 for Python 3.8). Both
user_agent_headerandadditional_headersare valid parameters for thewebsockets.connect()function in these versions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `websockets` library version and the `connect` method parameters. # Check the installed version of websockets pip show websockets | grep Version # Display the `connect` method signature from the source code grep -A 5 "async def connect" "$(python -c 'import websockets; print(websockets.__file__)')"Length of output: 357
Script:
#!/bin/bash # Find dependency files and search for websockets version fd -e txt -e toml -e cfg -e py "requirements|setup|pyproject" --exec rg -l "websockets" # Search for websockets.connect usage in Python files rg "websockets\.connect" -t py -C 3 # Search for any documentation mentioning websockets version fd "README|CHANGELOG|CONTRIBUTING" --exec rg -i "websocket"Length of output: 601
Script:
#!/bin/bash # Check websockets version in pyproject.toml cat pyproject.toml | rg -A 3 -B 3 "websockets"Length of output: 261
cozepy/websockets/__init__.py (1)
11-11: Ensureremove_url_trailing_slashis applied correctlyIn the
__init__method,remove_url_trailing_slashis applied beforehttp_base_url_to_ws. Verify that this order correctly transforms the URL for WebSocket connections without unintentional side effects.cozepy/audio/__init__.py (1)
10-10: LGTM! Clean transition from translations to transcriptionsThe refactoring from translations to transcriptions is well-implemented, maintaining consistency across both synchronous and asynchronous clients.
Also applies to: 23-23, 42-49, 69-69, 96-103
cozepy/audio/voices/__init__.py (2)
Line range hint
226-237: Headers handling is consistent with other methods.The implementation of headers extraction from kwargs and passing it to the request maker follows the same pattern used in other methods like
clone. This maintains consistency across the codebase.
213-213:⚠️ Potential issueBreaking change: Method signature updated.
The addition of
**kwargsto the method signature oflistis a breaking change that might affect existing code. Please ensure this change is documented in the changelog and migration guide.Run the following script to find potential usages that might be affected:
cozepy/chat/__init__.py (1)
225-227: Default values improve error handling.The addition of default values to
ChatErrorandChatUsagemodels makes them more robust by providing sensible defaults. This is a good practice for error handling.Also applies to: 277-283
pyproject.toml (1)
19-26: Verify the latest stable versions of WebSocket dependencies.The WebSocket dependencies have been added with version constraints based on Python versions. Please ensure these versions are secure and up-to-date.
Run the following script to check for the latest versions and any security advisories:
✅ Verification successful
WebSocket dependencies are using latest stable and secure versions
The dependencies are properly configured:
- websocket-client 1.8.0 is the latest version with no known vulnerabilities
- websockets 14.1.0 is the latest version and well above historically patched versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for latest versions and security advisories # Check PyPI for latest versions for pkg in websocket-client websockets; do echo "Checking $pkg..." curl -s "https://pypi.org/pypi/$pkg/json" | jq '.info.version' done # Check for security advisories for pkg in websocket-client websockets; do echo "Checking security advisories for $pkg..." gh api graphql -f query='{ securityVulnerabilities(first: 5, ecosystem: PIP, package: "'$pkg'") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' doneLength of output: 1857
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
cozepy/websockets/audio/speech/__init__.py (1)
109-114:⚠️ Potential issueHandle
Nonefor optionaloutput_audioindumpmethod.The
dumpmethod assumesoutput_audiois not None. This could raise an AttributeError ifoutput_audiois None.Apply this diff to fix the issue:
def dump(self): return { "event_id": self.event_id, "type": EventType.SPEECH_UPDATE, "data": { - "output_audio": self.output_audio.dump(), + "output_audio": self.output_audio.dump() if self.output_audio else None, }, }cozepy/websockets/ws.py (2)
129-131:⚠️ Potential issueEnhance error handling for connection failures.
The exception handling for
InvalidStatusmay result inAttributeErrorife.responseore.response.headersisNone.Apply this diff to fix the issue:
except InvalidStatus as e: - raise CozeAPIError(None, f"{e}", e.response.headers.get("x-tt-logid")) from e + log_id = e.response.headers.get("x-tt-logid") if e.response and e.response.headers else None + raise CozeAPIError(None, f"{e}", log_id) from e
235-237:⚠️ Potential issueFix incorrect usage of
sendmethod in WebSocket connection.The
sendmethod of the WebSocket connection does not accept a second argument.Apply this diff to fix the issue:
async def _send_event(self, event: Event) -> None: log_debug("[%s] send event, type=%s", self._path, event.type.value) - await self._ws.send(event.dump_json(), True) + await self._ws.send(event.dump_json())cozepy/chat/__init__.py (1)
191-194:⚠️ Potential issueAdd error handling and fix the condition check.
The implementation has two issues:
- Missing error handling for base64 decoding as previously suggested
- Incomplete condition check (should check both content_type and type)
Apply this diff to fix both issues:
def get_audio(self) -> Optional[bytes]: - if self.content_type == MessageContentType.AUDIO: - return base64.b64decode(self.content) + if self.content_type == MessageContentType.AUDIO or self.type == MessageContentType.AUDIO: + try: + return base64.b64decode(self.content) + except Exception: + return None return None
🧹 Nitpick comments (2)
cozepy/websockets/audio/transcriptions/__init__.py (1)
50-64: Consider adding audio format validation.While the implementation is correct, consider adding validation for:
- Valid audio formats (e.g., wav, mp3)
- Sample rate ranges (e.g., 8000-48000 Hz)
- Channel counts (e.g., 1-2)
- Bit depth values (e.g., 16, 24, 32)
This would help catch invalid configurations early.
cozepy/websockets/ws.py (1)
143-151: Add timeout handling in send loop.The send loop should handle timeouts to prevent indefinite blocking.
Apply this diff to add timeout handling:
async def _send_loop(self) -> None: try: while True: - event = await self._input_queue.get() + try: + event = await asyncio.wait_for(self._input_queue.get(), timeout=30.0) + except asyncio.TimeoutError: + continue await self._send_event(event) self._input_queue.task_done() except Exception as e: await self._handle_error(e)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.gitignore(1 hunks)cozepy/__init__.py(2 hunks)cozepy/audio/__init__.py(5 hunks)cozepy/audio/transcriptions/__init__.py(3 hunks)cozepy/audio/voices/__init__.py(3 hunks)cozepy/chat/__init__.py(6 hunks)cozepy/coze.py(3 hunks)cozepy/util.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)examples/chat_stream.py(1 hunks)examples/utils/__init__.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)pyproject.toml(1 hunks)tests/test_audio_translations.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- .gitignore
- pyproject.toml
- examples/chat_stream.py
- cozepy/init.py
- examples/utils/init.py
- tests/test_audio_translations.py
- examples/websockets_audio_speech.py
- cozepy/websockets/audio/init.py
- cozepy/audio/transcriptions/init.py
- cozepy/websockets/init.py
- cozepy/websockets/chat/init.py
🔇 Additional comments (16)
cozepy/util.py (2)
47-57: Previous input validation suggestions implemented correctly.The function now includes the recommended input validation and error handling as suggested in the previous review.
Consider additional URL validation.
While the basic validation is good, consider adding:
- URL format validation to ensure the base URL is well-formed
- Handling of URLs with paths or query parameters
def http_base_url_to_ws(base_url: str) -> str: if not base_url: raise ValueError("base_url cannot be empty") if not base_url.startswith("https://"): raise ValueError("base_url must start with 'https://'") + + # Extract domain part before any path or query params + domain_part = base_url.split("/", 3)[2] + if not domain_part: + raise ValueError("Invalid URL format") + + # Convert only the domain part + ws_domain = domain_part.replace("api-", "ws-") if "api-" in domain_part else domain_part.replace("api.", "ws.") + + # Reconstruct URL with any paths/params + parts = base_url.split("/", 3) + parts[0] = "wss:" + parts[2] = ws_domain + return "/".join(parts)
70-70: LGTM! Docstring now matches the parameter name.The parameter name in the docstring has been updated to match the actual parameter name
filepath, improving documentation consistency.examples/websockets_chat.py (3)
28-28: Avoid using global variables for state managementUsing a global list
dfor collecting audio data is not thread-safe and could lead to memory leaks. Consider encapsulating this state within a class or using an instance variable within an async context.
43-44: Add error handling for audio data collectionThe
handle_conversation_audio_deltafunction does not handle potential memory issues or invalid data. This could lead to exceptions or unresponsive behavior if large amounts of audio data are received.
51-55: Improve resource cleanup inhandle_closedThe current implementation assumes successful file I/O operations and does not handle exceptions that may occur during file writing. Additionally, keeping audio data in memory until the connection is closed can lead to high memory usage.
examples/websockets_audio_transcriptions.py (1)
37-51: Improve voice selection and error handling in the audio streamThe current implementation assumes the voices list is non-empty and uses the first voice without validation. It also lacks error handling for API calls and resource cleanup in case of errors.
cozepy/audio/__init__.py (1)
Line range hint
10-49: Refactoring totranscriptionsis consistent and accurateThe update from
translationstotranscriptionsin bothAudioClientandAsyncAudioClientclasses is consistent throughout the file. The property methods and private variables have been correctly updated.Also applies to: 69-103
cozepy/websockets/audio/transcriptions/__init__.py (5)
12-30: LGTM! Well-structured event class implementation.The class correctly handles binary audio data with proper base64 encoding for transmission over WebSocket.
33-47: LGTM! Clean implementation of commit event.The class correctly implements a minimal event structure for committing audio buffers.
86-97: LGTM! Clean implementation of completion event.The class correctly implements a minimal event structure for transcription completion.
100-114: LGTM! Well-structured update event implementation.The class correctly handles transcription content updates.
152-167: LGTM! Clean factory implementation.The class correctly implements the factory pattern for creating transcription clients.
cozepy/coze.py (1)
155-155: LGTM! Clean integration of WebSocket support.The WebSocket client is correctly integrated following the established lazy initialization pattern.
Also applies to: 242-249
cozepy/audio/voices/__init__.py (1)
Line range hint
213-237: LGTM! Clean implementation of async voice listing.The changes correctly add header support and async pagination.
cozepy/chat/__init__.py (2)
28-28: LGTM! Good addition of UNKNOWN states.Adding UNKNOWN states with empty string values is a good practice for handling undefined or unexpected enum values.
Also applies to: 202-202
225-227: LGTM! Sensible default values added.The default values are well-chosen:
- Zero for numeric fields (code and counts)
- Empty string for message field
- UNKNOWN for status enum
Also applies to: 277-283, 312-312
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
cozepy/websockets/ws.py (2)
129-130:⚠️ Potential issueEnhance error handling for connection failures.
The exception handling for
InvalidStatusin theconnectmethod may result inAttributeErrorife.responseore.response.headersisNone.Apply this diff to enhance error handling:
except InvalidStatus as e: - raise CozeAPIError(None, f"{e}", e.response.headers.get("x-tt-logid")) from e + log_id = e.response.headers.get("x-tt-logid") if e.response and e.response.headers else None + raise CozeAPIError(None, f"{e}", log_id) from e
229-232:⚠️ Potential issueFix incorrect usage of
sendmethod in WebSocket connection.The
sendmethod of the WebSocket connection does not accept a second argument.Apply this diff to fix the issue:
async def _send_event(self, event: Event) -> None: log_debug("[%s] send event, type=%s", self._path, event.type.value) if self._ws: - await self._ws.send(event.dump_json(), True) + await self._ws.send(event.dump_json())cozepy/websockets/audio/speech/__init__.py (1)
107-114:⚠️ Potential issueAdd None check for output_audio in dump method
The
dumpmethod could raise anAttributeErrorifoutput_audiois None.Apply this diff to fix the issue:
def dump(self): return { "event_id": self.event_id, "type": EventType.SPEECH_UPDATE, "data": { - "output_audio": self.output_audio.dump(), + "output_audio": self.output_audio.dump() if self.output_audio else None, }, }
🧹 Nitpick comments (7)
cozepy/websockets/ws.py (4)
21-58: Consider grouping event types by domain.The EventType enum could benefit from better organization. Consider using nested classes or prefixes to group related events.
Example refactor:
class EventType(str, Enum): # common ERROR = "error" CLOSED = "closed" - # v1/audio/speech - # req + class Speech: + class Request: + INPUT_TEXT_BUFFER_APPEND = "input_text_buffer.append" + INPUT_TEXT_BUFFER_COMMIT = "input_text_buffer.commit" + SPEECH_UPDATE = "speech.update" + class Response: + INPUT_TEXT_BUFFER_COMMITTED = "input_text_buffer.committed" + SPEECH_AUDIO_UPDATE = "speech.audio.update" + SPEECH_AUDIO_COMPLETED = "speech.audio.completed"
143-151: Consider adding backoff retry logic for send operations.The
_send_loopcould benefit from a retry mechanism with exponential backoff for transient failures.Example implementation:
async def _send_loop(self) -> None: max_retries = 3 base_delay = 1 # seconds try: while True: if event := await self._input_queue.get(): for attempt in range(max_retries): try: await self._send_event(event) break except Exception as e: if attempt == max_retries - 1: raise delay = base_delay * (2 ** attempt) await asyncio.sleep(delay) self._input_queue.task_done() except Exception as e: await self._handle_error(e)
152-169: Add heartbeat mechanism to detect connection health.The WebSocket connection should implement a heartbeat mechanism to detect connection issues early.
Consider implementing a heartbeat mechanism:
- Send periodic ping frames
- Monitor pong responses
- Reconnect if heartbeat fails
Example implementation:
async def _heartbeat_loop(self) -> None: while True: try: await asyncio.sleep(30) # Send heartbeat every 30 seconds if self._ws: await self._ws.ping() except Exception as e: await self._handle_error(e) break
173-205: Improve type hints in _wait_completed method.The event handler type hints could be more specific.
Consider updating the type hints:
- async def _wait_completed(self, events: List[EventType], wait_all: bool) -> None: + async def _wait_completed( + self, + events: List[EventType], + wait_all: bool, + ) -> None: - async def _handle_completed(client, event): + async def _handle_completed( + client: AsyncWebsocketsBaseClient, + event: Event, + ) -> None:cozepy/websockets/audio/speech/__init__.py (2)
154-161: Add docstrings to public methodsThe public methods
append,commit, andupdatelack docstrings explaining their purpose and parameters.Add docstrings to improve API documentation:
async def append(self, text: str) -> None: + """Append text to the input buffer. + + Args: + text: The text to append + """ await self._input_queue.put(InputTextBufferAppendEvent.load(text)) async def commit(self) -> None: + """Commit the current input buffer.""" await self._input_queue.put(InputTextBufferCommitEvent.load(None)) async def update(self, event: SpeechUpdateEvent) -> None: + """Update speech configuration. + + Args: + event: The speech update event containing new configuration + """ await self._input_queue.put(event)
175-176: Consider logging ignored eventsThe
INPUT_AUDIO_BUFFER_COMMITTEDevent is silently ignored. Consider logging this case for better debugging.Apply this diff:
elif event_type == EventType.INPUT_AUDIO_BUFFER_COMMITTED.value: - pass + log_warning("[%s] Ignoring event: %s", self._path, event_type)cozepy/websockets/chat/__init__.py (1)
124-124: Consider adding a heartbeat mechanism.WebSocket connections may timeout without a heartbeat mechanism, especially for long-running connections.
Consider implementing a heartbeat mechanism by:
- Adding a ping/pong event type
- Implementing a periodic ping sender
- Adding connection health monitoring
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.gitignore(1 hunks)cozepy/__init__.py(2 hunks)cozepy/audio/__init__.py(5 hunks)cozepy/audio/transcriptions/__init__.py(3 hunks)cozepy/audio/voices/__init__.py(3 hunks)cozepy/chat/__init__.py(6 hunks)cozepy/coze.py(3 hunks)cozepy/util.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)examples/chat_stream.py(1 hunks)examples/utils/__init__.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)pyproject.toml(1 hunks)tests/test_audio_translations.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- .gitignore
- examples/chat_stream.py
- cozepy/util.py
- pyproject.toml
- cozepy/init.py
- cozepy/audio/transcriptions/init.py
- examples/websockets_audio_speech.py
- tests/test_audio_translations.py
- cozepy/websockets/init.py
- examples/utils/init.py
- cozepy/websockets/audio/init.py
- cozepy/audio/voices/init.py
- cozepy/chat/init.py
🔇 Additional comments (15)
cozepy/audio/__init__.py (2)
42-49: LGTM! Clean implementation of the transcriptions property.The lazy initialization pattern is well implemented, following the same pattern as other properties in the class.
96-103: LGTM! Consistent implementation in the async client.The async implementation mirrors the synchronous version appropriately.
cozepy/websockets/audio/speech/__init__.py (2)
1-9: LGTM! Well-structured imports.The imports are well-organized with appropriate type hints and necessary dependencies for WebSocket functionality.
11-49: LGTM! Well-implemented event classes.The event classes follow consistent patterns for serialization/deserialization and properly handle their respective data types.
cozepy/coze.py (4)
18-18: LGTM! Import statement follows established patterns.The import is correctly placed within the TYPE_CHECKING block and follows the existing import style.
155-155: LGTM! Member variable follows class conventions.The
_websocketsmember is properly declared with correct typing and initialization.
243-249: LGTM! Property implementation follows established patterns.The websockets property getter correctly implements lazy initialization and follows the same pattern as other client properties in the class.
Line range hint
23-155: Verify if synchronous WebSocket support is needed.The WebSocket client is only implemented in the
AsyncCozeclass, while the AI summary suggests it should be available in both classes. If WebSocket support is intended to be async-only, this is fine. However, if synchronous support is needed, similar changes should be made to theCozeclass:class Coze(object): def __init__(self, ...): # Add after other client declarations + self._websockets: Optional[AsyncWebsocketsClient] = None # Add after other properties + @property + def websockets(self) -> "AsyncWebsocketsClient": + if not self._websockets: + from .websockets import AsyncWebsocketsClient + + self._websockets = AsyncWebsocketsClient(self._base_url, self._auth, self._requester) + return self._websocketsRun this script to check if there are any synchronous WebSocket usages in the codebase:
✅ Verification successful
WebSocket implementation is correctly async-only
The WebSocket support is intentionally implemented only in the
AsyncCozeclass, which is the correct approach. This is evidenced by:
- All example files using AsyncCoze for WebSocket operations
- WebSocket clients being specifically designed for async usage
- WebSockets being inherently asynchronous in nature
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential synchronous WebSocket usage # Look for imports or usage of Coze class with WebSocket-related patterns # Search for synchronous Coze class usage with WebSocket patterns rg -l "from.*import.*Coze\b" | xargs -I {} rg -l "websocket" {} || true # Search for WebSocket-related test files that might indicate sync usage fd -g "test_*" | xargs -I {} rg -l "websocket.*Coze\b" {} || trueLength of output: 253
Script:
#!/bin/bash # Check the implementation details in the example files echo "=== websockets_chat.py ===" rg -A 5 "from.*import.*Coze" examples/websockets_chat.py || true echo -e "\n=== websockets_audio_transcriptions.py ===" rg -A 5 "from.*import.*Coze" examples/websockets_audio_transcriptions.py || true echo -e "\n=== websockets_audio_speech.py ===" rg -A 5 "from.*import.*Coze" examples/websockets_audio_speech.py || trueLength of output: 1206
examples/websockets_audio_transcriptions.py (2)
22-26: 🛠️ Refactor suggestionImprove environment variable handling.
The current implementation assumes the environment variable value matches a valid logging level. This could raise KeyError if an invalid level is provided.
-if coze_log: - setup_logging(logging.getLevelNamesMapping()[coze_log.upper()]) +if coze_log: + try: + level = logging.getLevelNamesMapping()[coze_log.upper()] + setup_logging(level) + except KeyError: + logging.warning(f"Invalid log level: {coze_log}") + setup_logging(logging.INFO)Likely invalid or redundant comment.
37-51:⚠️ Potential issueAdd error handling in wrap_coze_speech_to_iterator.
The function assumes that voices.items is non-empty and doesn't handle potential API exceptions.
def wrap_coze_speech_to_iterator(coze: AsyncCoze, text: str): async def iterator(): - voices = await coze.audio.voices.list(**kwargs) - content = await coze.audio.speech.create( - input=text, - voice_id=voices.items[0].voice_id, - response_format=AudioFormat.WAV, - sample_rate=24000, - **kwargs, - ) - for data in content._raw_response.iter_bytes(chunk_size=1024): - yield data + try: + voices = await coze.audio.voices.list(**kwargs) + if not voices.items: + raise ValueError("No voices available") + + content = await coze.audio.speech.create( + input=text, + voice_id=voices.items[0].voice_id, + response_format=AudioFormat.WAV, + sample_rate=24000, + **kwargs, + ) + + async with content._raw_response as response: + for data in response.iter_bytes(chunk_size=1024): + yield data + except Exception as e: + logging.error(f"Error in audio stream: {str(e)}", exc_info=True) + raiseLikely invalid or redundant comment.
examples/websockets_chat.py (2)
76-77:⚠️ Potential issueValidate bot_id before use.
The bot_id is retrieved from environment variable but not validated.
bot_id = os.getenv("COZE_BOT_ID") +if not bot_id: + raise ValueError("Environment variable 'COZE_BOT_ID' is not set")Likely invalid or redundant comment.
28-28:⚠️ Potential issueReplace global list with a proper state management class.
Using a global list 'd' for collecting audio data is not thread-safe and could lead to memory leaks.
-d = [] +class AudioCollector: + def __init__(self): + self._data = [] + self._max_size = 1024 * 1024 * 100 # 100MB limit + + def append(self, data: bytes) -> None: + if sum(len(chunk) for chunk in self._data) + len(data) > self._max_size: + raise MemoryError("Audio data exceeds size limit") + self._data.append(data) + + def get_data(self) -> bytes: + return b"".join(self._data) + + def clear(self) -> None: + self._data.clear() + +audio_collector = AudioCollector()Likely invalid or redundant comment.
cozepy/websockets/chat/__init__.py (1)
134-137:⚠️ Potential issueFix potential type confusion in event handling.
The condition uses
elifafter checking forCONVERSATION_MESSAGE_DELTA, which could lead to missed events.- elif event_type == EventType.CONVERSATION_CHAT_COMPLETED.value: + if event_type == EventType.CONVERSATION_CHAT_COMPLETED.value:Likely invalid or redundant comment.
cozepy/websockets/audio/transcriptions/__init__.py (2)
146-148: 🛠️ Refactor suggestionHandle INPUT_AUDIO_BUFFER_COMMITTED event explicitly.
The method silently passes when event_type is INPUT_AUDIO_BUFFER_COMMITTED.
elif event_type == EventType.INPUT_AUDIO_BUFFER_COMMITTED.value: - pass + return None # Explicitly return None for committed eventsLikely invalid or redundant comment.
67-83:⚠️ Potential issueAdd null check for input_audio in dump method.
The dump method assumes input_audio is not None, which could raise AttributeError.
def dump(self): return { "event_id": self.event_id, "type": self.type.value, - "data": {"input_audio": self.input_audio.dump()}, + "data": {"input_audio": self.input_audio.dump() if self.input_audio else None}, }Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
cozepy/websockets/ws.py (2)
117-118:⚠️ Potential issueAdd null checks for error response handling.
The error handling for
InvalidStatuscould raise anAttributeErrorife.responseore.response.headersisNone.
219-220:⚠️ Potential issueRemove the unnecessary
Trueparameter fromsend.The
sendmethod of the WebSocket connection does not accept a second argument. TheTrueparameter should be removed.cozepy/websockets/audio/transcriptions/__init__.py (1)
109-113:⚠️ Potential issueHandle unhandled event type in
_load_event.The method silently passes when
event_typeisINPUT_AUDIO_BUFFER_COMMITTED. This could lead to unexpected behavior.
🧹 Nitpick comments (5)
examples/websockets_audio_transcriptions.py (2)
22-35: Add return type hints and enhance error logging.The event handlers would benefit from:
- Return type hints (-> None)
- More detailed error logging including exception type
-async def handle_error(client: AsyncWebsocketsAudioTranscriptionsCreateClient, e: Exception): - print(f"Error occurred: {e}") +async def handle_error(client: AsyncWebsocketsAudioTranscriptionsCreateClient, e: Exception) -> None: + logging.error(f"Error occurred: {e}", exc_info=True)
63-63: Parameterize the hardcoded text message.Consider making the text message configurable via environment variable or command-line argument for better flexibility.
- speech_stream = wrap_coze_speech_to_iterator(coze, "你今天好吗? 今天天气不错呀") + text = os.getenv("COZE_TEST_MESSAGE", "你今天好吗? 今天天气不错呀") + speech_stream = wrap_coze_speech_to_iterator(coze, text)examples/websockets_chat.py (1)
84-84: Parameterize the hardcoded text message.Consider making the text message configurable via environment variable or command-line argument for better flexibility.
- speech_stream = wrap_coze_speech_to_iterator(coze, "你今天好吗? 今天天气不错呀") + text = os.getenv("COZE_TEST_MESSAGE", "你今天好吗? 今天天气不错呀") + speech_stream = wrap_coze_speech_to_iterator(coze, text)cozepy/websockets/ws.py (2)
33-33: Document the rationale for ignored events.The events
INPUT_TEXT_BUFFER_COMMITTEDandINPUT_AUDIO_BUFFER_COMMITTEDare marked as "ignored" without explanation. Consider adding documentation to explain why these events are ignored and their significance in the WebSocket protocol.Also applies to: 43-43
49-51: Consider using event type inheritance for shared events.Instead of commenting out duplicate events, consider creating base event types that can be shared across different endpoints. This would make the relationship between similar events more explicit and maintainable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.gitignore(1 hunks)cozepy/__init__.py(2 hunks)cozepy/audio/__init__.py(5 hunks)cozepy/audio/transcriptions/__init__.py(3 hunks)cozepy/audio/voices/__init__.py(3 hunks)cozepy/chat/__init__.py(6 hunks)cozepy/coze.py(3 hunks)cozepy/util.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)examples/chat_stream.py(1 hunks)examples/utils/__init__.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)pyproject.toml(1 hunks)tests/test_audio_translations.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- examples/chat_stream.py
- .gitignore
- cozepy/init.py
- pyproject.toml
- cozepy/audio/transcriptions/init.py
- examples/utils/init.py
- examples/websockets_audio_speech.py
- tests/test_audio_translations.py
- cozepy/coze.py
- cozepy/audio/init.py
- cozepy/websockets/init.py
- cozepy/websockets/chat/init.py
- cozepy/websockets/audio/init.py
- cozepy/websockets/audio/speech/init.py
- cozepy/chat/init.py
🔇 Additional comments (16)
cozepy/util.py (2)
47-57: LGTM! Input validation implemented as suggested.The function now includes proper validation and error handling as recommended in the previous review.
Consider additional URL handling improvements.
While the basic URL conversion works, consider these enhancements:
- Handle URLs with paths and query parameters
- Integrate with
remove_url_trailing_slashdef http_base_url_to_ws(base_url: str) -> str: if not base_url: raise ValueError("base_url cannot be empty") if not base_url.startswith("https://"): raise ValueError("base_url must start with 'https://'") + # Remove trailing slash first + base_url = remove_url_trailing_slash(base_url) + + # Split URL into parts + url_parts = base_url.split("/", 3) + domain = url_parts[2] + path = "/" + url_parts[3] if len(url_parts) > 3 else "" + + # Convert domain base_url = base_url.replace("https://", "wss://") if "api-" in base_url: - return base_url.replace("api-", "ws-") + domain = domain.replace("api-", "ws-") return base_url.replace("api.", "ws.") + + # Reconstruct URL with path + return f"wss://{domain}{path}"
70-70: LGTM! Parameter name in docstring now matches function signature.The docstring update improves code documentation accuracy by aligning the parameter name with the function signature.
examples/websockets_audio_transcriptions.py (3)
1-20: LGTM! Environment setup and imports are well-structured.The code properly handles environment variables and imports all necessary modules for WebSocket and audio functionality.
37-51: Add error handling for voice selection and API calls.The function needs proper error handling for API calls and voice selection.
This issue was previously identified in the chat example. The same concerns apply here:
- No validation of voices list
- No error handling for API calls
- No cleanup of resources
82-83: LGTM! Proper asyncio entry point setup.The main guard correctly uses asyncio.run for the entry point.
examples/websockets_chat.py (5)
28-28: Replace global list with proper state management.Using a global list for audio data collection is not thread-safe and could lead to memory leaks.
This issue was previously identified. Consider using the suggested AudioCollector class for better state management.
43-44: Add error handling for audio data collection.The audio delta handler should handle potential memory issues and invalid data.
This issue was previously identified. Consider implementing the suggested error handling for audio data collection.
51-55: Improve resource cleanup in handle_closed.The current implementation doesn't properly handle file I/O errors.
This issue was previously identified. Consider implementing the suggested error handling and resource cleanup.
76-76: Validate bot_id before use.The bot_id is retrieved from environment variable but not validated.
This issue was previously identified. Consider implementing the suggested validation check.
98-106: LGTM! Proper WebSocket client lifecycle management.The code correctly uses context manager and handles the WebSocket connection lifecycle with proper commit and wait calls.
cozepy/audio/voices/__init__.py (3)
213-213: LGTM! Method signature changes align with WebSocket support.The addition of
**kwargsand the return type change toAsyncNumberPaged[Voice]properly support the new WebSocket protocol implementation.
226-226: LGTM! Headers extraction is properly implemented.The headers extraction follows the established pattern and is correctly typed as
Optional[dict].
Line range hint
237-241: Consider setting is_async=True for consistency.The request_maker sets
is_async=Falsewhich seems inconsistent with the async nature of the method and the WebSocket protocol. Consider updating this tois_async=Trueto maintain consistency with the async implementation.Let's verify if other async methods in the codebase use
is_async=True:cozepy/websockets/ws.py (1)
60-63: LGTM! Well-structured base event class.The
Eventclass provides a solid foundation for event handling with proper type safety and serialization support.cozepy/websockets/audio/transcriptions/__init__.py (2)
14-24: LGTM! Well-implemented audio buffer event.The event properly handles binary data serialization using base64 encoding.
26-28: LGTM! Event classes are well-structured.The event classes are properly implemented with clear type definitions and appropriate data structures.
Also applies to: 45-47, 49-54
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
cozepy/audio/voices/__init__.py (1)
The is_async=False setting is indeed incorrect in this async context
The codebase shows that async operations consistently use
is_async=True. Settingis_async=Falsein an async context is contradictory and could affect WebSocket connection handling.
- Change
is_async=Falsetois_async=Trueincozepy/audio/voices/__init__.pyto maintain consistency with other async operations in the codebase.🔗 Analysis chain
Line range hint
237-246: Consider reviewing the is_async flag setting.While the async implementation looks good, the
is_async=Falseparameter inmake_requestseems contradictory in an async context. This might affect WebSocket connection handling.Let's check if this is a pattern across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other make_request calls with is_async parameter ast-grep --pattern 'make_request($$$, is_async=$_)'Length of output: 53
Script:
#!/bin/bash # Search for make_request calls with context rg "make_request.*is_async" -B 2 -A 2Length of output: 39
Script:
#!/bin/bash # Basic search for make_request calls rg "make_request\(" -B 3 -A 3Length of output: 9661
🧹 Nitpick comments (6)
examples/websockets_audio_speech.py (1)
17-21: Validate environment variablesThe code reads environment variables without validation.
Add validation for required environment variables:
coze_log = os.getenv("COZE_LOG") if coze_log: + try: setup_logging(logging.getLevelNamesMapping()[coze_log.upper()]) + except KeyError: + logging.warning(f"Invalid log level: {coze_log}") + setup_logging(logging.INFO) -kwargs = json.loads(os.getenv("COZE_KWARGS") or "{}") +try: + kwargs = json.loads(os.getenv("COZE_KWARGS") or "{}") +except json.JSONDecodeError as e: + logging.error(f"Invalid COZE_KWARGS JSON: {e}") + kwargs = {}examples/websockets_chat.py (1)
83-84: Externalize the text inputThe text for speech synthesis is hardcoded.
Consider accepting the text as a command-line argument or environment variable:
+ text = os.getenv("COZE_TEXT", "你今天好吗? 今天天气不错呀") # Initialize Audio - speech_stream = wrap_coze_speech_to_iterator(coze, "你今天好吗? 今天天气不错呀") + speech_stream = wrap_coze_speech_to_iterator(coze, text)cozepy/websockets/chat/__init__.py (3)
50-52: Remove commented-out codeThere are commented-out method definitions and event types that should be either implemented or removed.
Either implement the
on_conversation_chat_requires_actionfunctionality or remove the commented code to maintain code cleanliness.Also applies to: 96-97
115-149: Improve event type handlingThe
_load_eventmethod uses string comparison for event types which could be error-prone.Consider using an enum-based approach:
def _load_event(self, message: Dict) -> Optional[WebsocketsEvent]: event_id = message.get("event_id") or "" - event_type = message.get("type") or "" + try: + event_type = WebsocketsEventType(message.get("type") or "") + except ValueError: + log_warning("[%s] unknown event type: %s", self._path, message.get("type")) + return None data = message.get("data") or {} - if event_type == WebsocketsEventType.CONVERSATION_CHAT_CREATED.value: + if event_type == WebsocketsEventType.CONVERSATION_CHAT_CREATED:
17-39: Add docstrings for public event classesThe event classes lack documentation explaining their purpose and usage.
Add docstrings to improve code documentation:
class ConversationChatCreatedEvent(WebsocketsEvent): + """Event emitted when a new chat conversation is created. + + Attributes: + type: The event type (CONVERSATION_CHAT_CREATED) + data: The created chat data + """ type: WebsocketsEventType = WebsocketsEventType.CONVERSATION_CHAT_CREATED data: Chatcozepy/websockets/audio/speech/__init__.py (1)
80-93: Document empty event handler implementationsThe empty implementations should be documented to explain why they're intentionally empty.
Apply this diff to improve documentation:
async def on_input_text_buffer_committed( self, cli: "AsyncWebsocketsAudioSpeechEventHandler", event: InputTextBufferCommittedEvent ): + # This event is ignored as it's only used for internal state tracking pass async def on_speech_audio_update( self, cli: "AsyncWebsocketsAudioSpeechEventHandler", event: SpeechAudioUpdateEvent ): + # Override this method to handle speech audio updates pass async def on_speech_audio_completed( self, cli: "AsyncWebsocketsAudioSpeechEventHandler", event: SpeechAudioCompletedEvent ): + # Override this method to handle speech audio completion pass
🛑 Comments failed to post (2)
examples/websockets_chat.py (1)
29-31: 🛠️ Refactor suggestion
Encapsulate audio delta handling
Similar to the audio speech example, the chat event handler uses class-level mutable state.
Encapsulate the audio data collection using the same pattern as suggested for the audio speech example.
Also applies to: 42-46
cozepy/websockets/audio/speech/__init__.py (1)
34-38: 🛠️ Refactor suggestion
Enhance data validation by using BaseModel
The configuration classes should inherit from
BaseModelfor proper validation and serialization.Apply this diff to improve the implementation:
- class OpusConfig(object): + class OpusConfig(BaseModel): bitrate: Optional[int] = None use_cbr: Optional[bool] = None frame_size_ms: Optional[float] = None - class PCMConfig(object): + class PCMConfig(BaseModel): sample_rate: Optional[int] = None - class OutputAudio(object): + class OutputAudio(BaseModel): codec: Optional[str] + @field_validator('codec') + def validate_codec(cls, v): + if v and v not in ['opus', 'pcm']: + raise ValueError('codec must be either "opus" or "pcm"') + return v pcm_config: Optional["SpeechUpdateEvent.PCMConfig"] opus_config: Optional["SpeechUpdateEvent.OpusConfig"] speech_rate: Optional[int] voice_id: Optional[str]Also applies to: 39-41, 42-48
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (14)
cozepy/websockets/audio/transcriptions/__init__.py (3)
24-26: Consider using a more descriptive method name.The method name
serialize_deltacould be more specific, likeserialize_delta_to_base64to clearly indicate the transformation being performed.
39-44: Add validation for audio parameters.The
InputAudioclass accepts optional parameters without validation. Consider adding validators for:
sample_rate: Should be positivechannel: Should be positivebit_depth: Should be positive and typically a multiple of 8
72-86: Add logging to empty event handlers.The event handler methods are currently empty. Consider adding debug logging to track when these events occur.
async def on_input_audio_buffer_committed( self, cli: "AsyncWebsocketsAudioTranscriptionsCreateClient", event: InputAudioBufferCommittedEvent ): - pass + from cozepy.log import log_debug + log_debug("[audio.transcriptions] Buffer committed: %s", event.event_id)examples/websockets_audio_transcriptions.py (4)
17-20: Document environment variables in README or docstring.The script uses environment variables (
COZE_LOG,COZE_KWARGS) for configuration. Consider adding documentation about these environment variables, their expected values, and their impact on the script's behavior.Add a docstring at the top of the file:
""" WebSocket Audio Transcriptions Example Environment Variables: COZE_LOG: Logging level (e.g., "DEBUG", "INFO", "WARNING") COZE_KWARGS: JSON string of additional keyword arguments for API calls """
28-37: Enhance error handling and use structured logging.The event handlers currently use print statements and lack proper error handling. Consider using the configured logger and adding more robust error handling.
- async def on_closed(self, cli: "AsyncWebsocketsAudioTranscriptionsCreateClient"): - print("Connection closed") + async def on_closed(self, cli: "AsyncWebsocketsAudioTranscriptionsCreateClient"): + logging.info("WebSocket connection closed", extra={"client_id": id(cli)}) - async def on_error(self, cli: "AsyncWebsocketsAudioTranscriptionsCreateClient", e: Exception): - print(f"Error occurred: {e}") + async def on_error(self, cli: "AsyncWebsocketsAudioTranscriptionsCreateClient", e: Exception): + logging.error("WebSocket error occurred", exc_info=e, extra={"client_id": id(cli)}) + # Consider implementing retry logic or cleanup here - async def on_transcriptions_message_update( - self, cli: "AsyncWebsocketsAudioTranscriptionsCreateClient", event: TranscriptionsMessageUpdateEvent - ): - print("Received:", event.data.content) + async def on_transcriptions_message_update( + self, cli: "AsyncWebsocketsAudioTranscriptionsCreateClient", event: TranscriptionsMessageUpdateEvent + ): + logging.info("Received transcription update", extra={ + "client_id": id(cli), + "content": event.data.content + })
66-66: Externalize the text input.The Chinese text is hardcoded in the example. Consider accepting it as a command-line argument or environment variable for better flexibility.
- speech_stream = wrap_coze_speech_to_iterator(coze, "你今天好吗? 今天天气不错呀") + text = os.getenv("COZE_TEXT", "你今天好吗? 今天天气不错呀") + speech_stream = wrap_coze_speech_to_iterator(coze, text)
81-82: Add signal handling for clean shutdown.The script should handle system signals (SIGINT, SIGTERM) to ensure clean shutdown of WebSocket connections and resource cleanup.
+import signal + +def handle_shutdown(loop, signal=None): + """Handle shutdown gracefully""" + if signal: + logging.info(f"Received exit signal {signal.name}") + tasks = [t for t in asyncio.all_tasks() if t is not asyncio.current_task()] + [task.cancel() for task in tasks] + logging.info(f"Cancelling {len(tasks)} tasks") + loop.stop() + if __name__ == "__main__": - asyncio.run(main()) + loop = asyncio.new_event_loop() + signals = (signal.SIGTERM, signal.SIGINT) + for s in signals: + loop.add_signal_handler( + s, lambda s=s: handle_shutdown(loop, signal=s) + ) + try: + loop.run_until_complete(main()) + finally: + loop.close() + logging.info("Successfully shutdown the websocket example.")examples/websockets_audio_speech.py (1)
39-61: Enhance error handling and internationalization support.
- Add error handling for environment variables
- Consider using a configuration file for the sample text
- Add comments explaining the Chinese text for non-Chinese speakers
Apply this diff to improve the implementation:
async def main(): - coze_api_token = get_coze_api_token() - coze_api_base = get_coze_api_base() + try: + coze_api_token = get_coze_api_token() + coze_api_base = get_coze_api_base() + except ValueError as e: + print(f"Failed to get API configuration: {e}") + return # Initialize Coze client coze = AsyncCoze( auth=TokenAuth(coze_api_token), base_url=coze_api_base, ) speech = coze.websockets.audio.speech.create( on_event=AsyncWebsocketsAudioSpeechEventHandlerSub(), **kwargs, ) - # Text to be converted to speech - text = "你今天好吗? 今天天气不错呀" + # Sample text in Chinese: "How are you today? The weather is nice today" + text = "你今天好吗? 今天天气不错呀"examples/websockets_chat.py (1)
83-83: Consider externalizing the text input.The Chinese text is hardcoded in the speech stream initialization. Consider making it configurable.
Apply this diff to make the text configurable:
- speech_stream = wrap_coze_speech_to_iterator(coze, "你今天好吗? 今天天气不错呀") + input_text = os.getenv("COZE_INPUT_TEXT", "你今天好吗? 今天天气不错呀") + speech_stream = wrap_coze_speech_to_iterator(coze, input_text)cozepy/websockets/chat/__init__.py (3)
18-39: Add docstrings to event classes.The event classes lack documentation explaining their purpose and usage.
Add docstrings to improve code documentation:
class ConversationChatCreatedEvent(WebsocketsEvent): + """Event emitted when a new chat conversation is created.""" type: WebsocketsEventType = WebsocketsEventType.CONVERSATION_CHAT_CREATED data: Chat class ConversationMessageDeltaEvent(WebsocketsEvent): + """Event emitted when a new message delta is received.""" type: WebsocketsEventType = WebsocketsEventType.CONVERSATION_MESSAGE_DELTA data: Message class ConversationAudioDeltaEvent(WebsocketsEvent): + """Event emitted when audio data is received.""" type: WebsocketsEventType = WebsocketsEventType.CONVERSATION_AUDIO_DELTA data: Message class ConversationChatCompletedEvent(WebsocketsEvent): + """Event emitted when a chat conversation is completed.""" type: WebsocketsEventType = WebsocketsEventType.CONVERSATION_CHAT_COMPLETED data: Chat
96-98: Remove commented out code.The commented out
updatemethod should be either implemented or removed.Remove the commented code if it's not needed:
- # async def update(self, event: TranscriptionsUpdateEventInputAudio) -> None: - # await self._input_queue.put(TranscriptionsUpdateEvent.load(event)) -
115-149: Refactor event loading for better maintainability.The current implementation uses a long chain of if-elif statements and lacks proper data validation.
Consider refactoring to use a mapping approach:
def _load_event(self, message: Dict) -> Optional[WebsocketsEvent]: event_id = message.get("event_id") or "" event_type = message.get("type") or "" data = message.get("data") or {} + + EVENT_MAPPING = { + WebsocketsEventType.CONVERSATION_CHAT_CREATED.value: (ConversationChatCreatedEvent, Chat), + WebsocketsEventType.CONVERSATION_MESSAGE_DELTA.value: (ConversationMessageDeltaEvent, Message), + WebsocketsEventType.CONVERSATION_AUDIO_DELTA.value: (ConversationAudioDeltaEvent, Message), + WebsocketsEventType.CONVERSATION_CHAT_COMPLETED.value: (ConversationChatCompletedEvent, Chat), + } + + if event_type in EVENT_MAPPING: + try: + event_class, data_class = EVENT_MAPPING[event_type] + return event_class.model_validate( + { + "data": data_class.model_validate(data), + "event_id": event_id, + } + ) + except Exception as e: + log_warning("[%s] failed to load event: %s", self._path, e) + return None + log_warning("[%s] unknown event=%s", self._path, event_type) return Nonecozepy/websockets/ws.py (1)
21-58: Group related event types and add descriptions.The event types could be better organized and documented.
Consider grouping related events and adding docstrings:
class WebsocketsEventType(str, Enum): + """ + Enumeration of WebSocket event types. + + Events are grouped by their functionality: + - Common events (error, closed) + - Speech events (input text, speech updates) + - Transcription events (audio buffer, transcription updates) + - Chat events (conversation updates, messages) + """ # common ERROR = "error" CLOSED = "closed"cozepy/chat/__init__.py (1)
225-227: Document default values in models.The default values in models should be documented to explain their significance.
Add docstrings to explain default values:
class ChatError(CozeModel): + """ + Represents a chat error. + Default code=0 indicates success, other values indicate failure. + Default msg="" indicates no error message. + """ code: int = 0 msg: str = "" class ChatUsage(CozeModel): + """ + Tracks token usage in chat. + All counts default to 0 for new conversations. + """ token_count: int = 0 output_count: int = 0 input_count: int = 0 class Chat(CozeModel): + """Default status is UNKNOWN until the chat is initialized.""" status: ChatStatus = ChatStatus.UNKNOWNAlso applies to: 277-283, 312-312
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.gitignore(1 hunks)cozepy/__init__.py(4 hunks)cozepy/audio/__init__.py(5 hunks)cozepy/audio/transcriptions/__init__.py(3 hunks)cozepy/audio/voices/__init__.py(3 hunks)cozepy/chat/__init__.py(6 hunks)cozepy/coze.py(3 hunks)cozepy/util.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)examples/chat_stream.py(1 hunks)examples/utils/__init__.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)pyproject.toml(1 hunks)tests/test_audio_translations.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- examples/chat_stream.py
- pyproject.toml
- .gitignore
- cozepy/audio/transcriptions/init.py
- cozepy/util.py
- tests/test_audio_translations.py
- cozepy/websockets/init.py
- examples/utils/init.py
- cozepy/init.py
- cozepy/websockets/audio/init.py
🔇 Additional comments (12)
cozepy/audio/__init__.py (1)
10-10: LGTM! Clean transition from translations to transcriptions.The refactoring from translations to transcriptions is implemented consistently across both synchronous and asynchronous clients.
Also applies to: 23-23, 42-49, 96-103
cozepy/websockets/audio/transcriptions/__init__.py (1)
156-158:⚠️ Potential issueHandle INPUT_AUDIO_BUFFER_COMMITTED event explicitly.
The method silently passes when
event_typeisINPUT_AUDIO_BUFFER_COMMITTED. This could lead to unexpected behavior.elif event_type == WebsocketsEventType.INPUT_AUDIO_BUFFER_COMMITTED.value: - pass + return InputAudioBufferCommittedEvent.model_validate( + { + "event_id": event_id, + } + )Likely invalid or redundant comment.
cozepy/coze.py (1)
155-155: LGTM! Clean WebSocket client integration.The WebSocket client integration follows the established pattern of lazy initialization used throughout the codebase.
Also applies to: 242-249
examples/websockets_audio_transcriptions.py (1)
40-54: Previous concerns about voice selection and error handling still apply.The implementation needs improvements in voice selection validation, error handling, and resource cleanup.
cozepy/websockets/audio/speech/__init__.py (2)
19-25: LGTM! Well-structured event class.The class follows best practices with proper type hints and data validation using Pydantic.
62-72: Add error handling for base64 encoding/decoding.The base64 encoding/decoding operations should include proper error handling.
examples/websockets_audio_speech.py (1)
24-37:⚠️ Potential issueImprove thread safety and resource management.
- Using a class variable for data storage is not thread-safe
- Missing error handling for file operations
- No cleanup of collected data after saving
Apply this diff to address these issues:
class AsyncWebsocketsAudioSpeechEventHandlerSub(AsyncWebsocketsAudioSpeechEventHandler): - delta = [] + def __init__(self): + self.delta = [] async def on_speech_audio_update(self, cli: AsyncWebsocketsAudioSpeechCreateClient, event: SpeechAudioUpdateEvent): self.delta.append(event.data.delta) async def on_error(self, cli: AsyncWebsocketsAudioSpeechCreateClient, e: Exception): print(f"Error occurred: {e}") async def on_closed(self, cli: AsyncWebsocketsAudioSpeechCreateClient): print("Speech connection closed, saving audio data to output.wav") - audio_data = b"".join(self.delta) - write_pcm_to_wav_file(audio_data, "output.wav") + try: + audio_data = b"".join(self.delta) + write_pcm_to_wav_file(audio_data, "output.wav") + except IOError as e: + print(f"Failed to save audio data: {e}") + finally: + self.delta.clear() # Clean up collected dataLikely invalid or redundant comment.
examples/websockets_chat.py (3)
56-69: 🛠️ Refactor suggestionAdd error handling and validation in speech iterator.
The function lacks error handling for API calls and validation of voice availability.
Apply this diff to improve error handling:
def wrap_coze_speech_to_iterator(coze: AsyncCoze, text: str): async def iterator(): - voices = await coze.audio.voices.list(**kwargs) - content = await coze.audio.speech.create( - input=text, - voice_id=voices.items[0].voice_id, - response_format=AudioFormat.WAV, - sample_rate=24000, - **kwargs, - ) - for data in content._raw_response.iter_bytes(chunk_size=1024): - yield data + try: + voices = await coze.audio.voices.list(**kwargs) + if not voices.items: + raise ValueError("No voices available") + + content = await coze.audio.speech.create( + input=text, + voice_id=voices.items[0].voice_id, + response_format=AudioFormat.WAV, + sample_rate=24000, + **kwargs, + ) + + for data in content._raw_response.iter_bytes(chunk_size=1024): + yield data + except Exception as e: + log_info(f"Speech synthesis error: {e}") + raiseLikely invalid or redundant comment.
72-75: 🛠️ Refactor suggestionValidate environment variables before use.
The code retrieves environment variables without validation, which could lead to runtime errors.
Apply this diff to add validation:
coze_api_token = get_coze_api_token() coze_api_base = get_coze_api_base() bot_id = os.getenv("COZE_BOT_ID") + if not all([coze_api_token, coze_api_base, bot_id]): + raise ValueError("Required environment variables are not set")Likely invalid or redundant comment.
50-54: 🛠️ Refactor suggestionAdd error handling and cleanup in
on_closed.The method should handle file I/O errors and clean up audio data after saving.
Apply this diff to improve error handling:
async def on_closed(self, cli: AsyncWebsocketsAudioTranscriptionsCreateClient): print("Chat connection closed, saving audio data to output.wav") - audio_data = b"".join(self.delta) - write_pcm_to_wav_file(audio_data, "output.wav") + try: + audio_data = b"".join(self.delta) + write_pcm_to_wav_file(audio_data, "output.wav") + except Exception as e: + log_info(f"Failed to save audio data: {e}") + finally: + self.delta.clear() # Clean up memoryLikely invalid or redundant comment.
cozepy/websockets/ws.py (1)
217-221:⚠️ Potential issueFix WebSocket send method usage.
The
sendmethod is incorrectly called with a second argument.Apply this diff to fix the issue:
async def _send_event(self, event: WebsocketsEvent) -> None: log_debug("[%s] send event, type=%s", self._path, event.type.value) if self._ws: - await self._ws.send(event.model_dump_json(), True) + await self._ws.send(event.model_dump_json())Likely invalid or redundant comment.
cozepy/chat/__init__.py (1)
191-195: 🛠️ Refactor suggestionAdd error handling to audio data decoding.
The
get_audiomethod should handle base64 decoding errors.Apply this diff to add error handling:
def get_audio(self) -> Optional[bytes]: - if self.content_type == MessageContentType.AUDIO: - return base64.b64decode(self.content) - return None + try: + if self.content_type == MessageContentType.AUDIO: + return base64.b64decode(self.content) + except Exception as e: + log_warning(f"Failed to decode audio content: {e}") + return NoneLikely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
cozepy/websockets/audio/speech/__init__.py (1)
142-166:⚠️ Potential issueEnhance security in base64 decoding.
The base64 decoding operation could be exploited with maliciously crafted input. Add proper validation and size limits.
Apply this diff to improve security:
delta_base64 = data.get("delta") if delta_base64 is None: raise ValueError("Missing 'delta' in event data") + if not isinstance(delta_base64, str): + raise ValueError("Invalid delta type") + if len(delta_base64) > 1024 * 1024: # 1MB limit + raise ValueError("Delta exceeds size limit") return SpeechAudioUpdateEvent.model_validate( { "event_id": event_id,cozepy/websockets/ws.py (3)
133-135:⚠️ Potential issueEnhance error handling for connection failures.
Currently, the exception handling for
InvalidStatusin theconnectmethod may result inAttributeErrorife.responseore.response.headersisNone. To prevent this, add a check to ensure that the headers are available before accessing them.Apply this diff to enhance error handling:
except InvalidStatus as e: - raise CozeAPIError(None, f"{e}", e.response.headers.get("x-tt-logid")) from e + log_id = e.response.headers.get("x-tt-logid") if e.response and e.response.headers else None + raise CozeAPIError(None, f"{e}", log_id) from e
151-159: 🛠️ Refactor suggestionAdd timeout handling for WebSocket operations.
The send loop should include timeout handling to prevent indefinite blocking.
Apply this diff to add timeout handling:
async def _send_loop(self) -> None: try: while True: - if event := await self._input_queue.get(): - await self._send_event(event) - self._input_queue.task_done() + try: + if event := await asyncio.wait_for(self._input_queue.get(), timeout=30): # 30s timeout + await self._send_event(event) + self._input_queue.task_done() + except asyncio.TimeoutError: + continue except Exception as e: await self._handle_error(e)
237-241:⚠️ Potential issueFix incorrect usage of
sendmethod in WebSocket connection.The
sendmethod of the WebSocket connection does not accept a second argument. PassingTrueas a second argument is invalid and may cause aTypeError.Apply this diff to fix the issue:
async def _send_event(self, event: WebsocketsEvent) -> None: log_debug("[%s] send event, type=%s", self._path, event.type.value) if self._ws: - await self._ws.send(event.model_dump_json(), True) + await self._ws.send(event.model_dump_json())
🧹 Nitpick comments (1)
cozepy/websockets/ws.py (1)
49-51: Remove commented-out event types.These event types are already defined in the audio transcriptions section. Remove the commented-out duplicates to maintain code cleanliness.
Apply this diff to clean up the code:
- # INPUT_AUDIO_BUFFER_APPEND = "input_audio_buffer.append" - # INPUT_AUDIO_BUFFER_COMMIT = "input_audio_buffer.commit" CHAT_UPDATE = "chat.update"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cozepy/__init__.py(2 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)
🔇 Additional comments (12)
cozepy/websockets/audio/speech/__init__.py (9)
19-24: LGTM!The class is well-structured, using Pydantic models for data validation and inheriting from
WebsocketsEventfor event handling.
28-30: LGTM!The class is correctly implemented as a simple event without data fields, which is appropriate for a commit event.
33-53: LGTM!The class is well-structured with:
- Proper use of Pydantic models for nested configurations
- Appropriate optional fields for flexible configuration
- Clear separation of concerns between Opus and PCM configs
57-59: LGTM!The class is correctly implemented as a simple event without data fields, which is appropriate for a committed event.
62-71: LGTM!The class is well-implemented with:
- Proper use of Pydantic models for data validation
- Correct handling of binary data with base64 encoding
- Clear field serialization strategy
75-77: LGTM!The class is correctly implemented as a simple event without data fields, which is appropriate for a completed event.
79-93: LGTM!The event handler class is well-structured with:
- Proper async method definitions
- Correct type hints for parameters
- Clear separation of event handling responsibilities
96-121: LGTM!The client implementation is well-structured with:
- Proper inheritance from base client
- Clear event handler mapping
- Appropriate path configuration
169-184: LGTM!The factory class is well-implemented with:
- Proper initialization of dependencies
- Clear factory method implementation
- Correct parameter forwarding
cozepy/__init__.py (1)
94-125: LGTM!The changes are well-organized with:
- Clear categorization of imports
- Proper organization of exports in
__all__- Consistent naming conventions
Also applies to: 215-242
cozepy/websockets/ws.py (2)
60-63: LGTM!The base event class is well-designed with:
- Proper use of abstract base class
- Clear field definitions
- Integration with Pydantic model
243-248: LGTM!The base event handler class is well-designed with:
- Proper async method definitions
- Clear type hints
- Appropriate base implementation
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
README.md (1)
Line range hint
1-1: Documentation needs WebSocket protocol examplesThe PR introduces WebSocket support for audio speech, audio transcriptions, and chat interactions, but the README lacks documentation and examples for these new features. Consider adding:
- WebSocket client initialization examples
- Audio speech and transcription usage examples
- Chat interaction examples using WebSocket
- Event handling patterns for WebSocket connections
Would you like me to help generate the documentation sections for the new WebSocket features?
♻️ Duplicate comments (1)
cozepy/websockets/audio/speech/__init__.py (1)
258-282:⚠️ Potential issueEnhance security in base64 decoding.
The base64 decoding operation could be exploited with maliciously crafted input.
Apply this diff to improve security:
delta_base64 = data.get("delta") if delta_base64 is None: raise ValueError("Missing 'delta' in event data") + if not isinstance(delta_base64, str): + raise ValueError("Invalid delta type") + if len(delta_base64) > 1024 * 1024: # 1MB limit + raise ValueError("Delta exceeds size limit") + try: + delta = base64.b64decode(delta_base64) + except binascii.Error as e: + raise ValueError(f"Invalid base64 data: {str(e)}") return SpeechAudioUpdateEvent.model_validate( { "event_id": event_id, "data": SpeechAudioUpdateEvent.Data.model_validate( { - "delta": base64.b64decode(delta_base64), + "delta": delta, } ), } )
🧹 Nitpick comments (3)
examples/websockets_audio_speech.py (1)
26-26: Remove TODO comment after reviewThe TODO comment should be removed or replaced with actual implementation notes.
cozepy/websockets/chat/__init__.py (2)
172-174: Remove or implement commented code.The commented-out method
on_conversation_chat_requires_actionshould either be implemented or removed.
216-218: Remove or implement commented code.The commented-out method
updateshould either be implemented or removed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md(1 hunks)cozepy/__init__.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)
🔇 Additional comments (15)
cozepy/websockets/__init__.py (1)
9-52: Well-structured WebSocket client implementation!The implementation follows good practices:
- Clean separation between sync and async clients
- Proper URL processing for WebSocket connections
- Consistent passing of authentication and requester to child clients
cozepy/websockets/audio/__init__.py (1)
Line range hint
8-52: Clean and consistent audio client implementation!The audio client classes maintain consistency with the parent classes while providing specialized audio functionality through well-defined properties.
examples/websockets_audio_transcriptions.py (2)
81-86: 🛠️ Refactor suggestionAdd error handling for WebSocket operations
The WebSocket operations should handle potential connection and streaming errors.
- async with transcriptions() as client: - async for data in speech_stream(): - await client.append(data) - await client.commit() - await client.wait() + try: + async with transcriptions() as client: + try: + async for data in speech_stream(): + await client.append(data) + await client.commit() + await client.wait() + except Exception as e: + log_info("Error during audio streaming", exc_info=True) + raise + except Exception as e: + log_info("Error in WebSocket connection", exc_info=True) + raiseLikely invalid or redundant comment.
47-61: 🛠️ Refactor suggestionAdd error handling in wrap_coze_speech_to_iterator
The function needs to handle potential API errors and validate the voices list.
def wrap_coze_speech_to_iterator(coze: AsyncCoze, text: str): async def iterator(): - voices = await coze.audio.voices.list(**kwargs) - content = await coze.audio.speech.create( - input=text, - voice_id=voices.items[0].voice_id, - response_format=AudioFormat.WAV, - sample_rate=24000, - **kwargs, - ) - for data in content._raw_response.iter_bytes(chunk_size=1024): - yield data + try: + voices = await coze.audio.voices.list(**kwargs) + if not voices.items: + raise ValueError("No voices available") + + content = await coze.audio.speech.create( + input=text, + voice_id=voices.items[0].voice_id, + response_format=AudioFormat.WAV, + sample_rate=24000, + **kwargs, + ) + + async with content._raw_response as response: + async for data in response.iter_bytes(chunk_size=1024): + yield data + except Exception as e: + log_info(f"Error in speech iterator: {e}") + raiseLikely invalid or redundant comment.
examples/websockets_chat.py (3)
46-48: 🛠️ Refactor suggestionAdd error handling for audio data collection
The audio data collection should handle potential memory issues and invalid data.
async def on_conversation_audio_delta(self, cli: AsyncWebsocketsChatClient, event: ConversationAudioDeltaEvent): - self.delta.append(event.data.get_audio()) + try: + audio_data = event.data.get_audio() + if sum(len(d) for d in self.delta) + len(audio_data) > self._max_size: + raise MemoryError("Audio data exceeds size limit") + self.delta.append(audio_data) + except Exception as e: + log_info(f"Error processing audio data: {e}") + await cli.close()Likely invalid or redundant comment.
35-35: 🛠️ Refactor suggestionReplace class variable with instance variable and add memory management
Using a class variable for audio data can cause issues with multiple instances and memory management.
- delta = [] + def __init__(self): + super().__init__() + self.delta = [] + self._max_size = 100 * 1024 * 1024 # 100MB limitLikely invalid or redundant comment.
75-75: 🛠️ Refactor suggestionValidate bot_id before use
The bot_id should be validated before being used to create the chat client.
bot_id = os.getenv("COZE_BOT_ID") +if not bot_id: + raise ValueError("Environment variable 'COZE_BOT_ID' is not set")Likely invalid or redundant comment.
cozepy/__init__.py (2)
94-125: LGTM! Well-organized imports.The imports are properly organized by module and the new WebSocket-related classes and events are correctly imported.
215-242: LGTM! Properly updated exports.The
__all__list is correctly updated to include the new WebSocket-related entities, maintaining the module's public interface.cozepy/websockets/audio/transcriptions/__init__.py (2)
258-274:⚠️ Potential issueEnhance security in base64 decoding.
The base64 decoding operation could be exploited with maliciously crafted input.
Apply this diff to improve security:
delta_base64 = data.get("delta") if delta_base64 is None: raise ValueError("Missing 'delta' in event data") + if not isinstance(delta_base64, str): + raise ValueError("Invalid delta type") + if len(delta_base64) > 1024 * 1024: # 1MB limit + raise ValueError("Delta exceeds size limit") + try: + delta = base64.b64decode(delta_base64) + except binascii.Error as e: + raise ValueError(f"Invalid base64 data: {str(e)}") return SpeechAudioUpdateEvent.model_validate( { "event_id": event_id, "data": SpeechAudioUpdateEvent.Data.model_validate( { - "delta": base64.b64decode(delta_base64), + "delta": delta, } ), } )Likely invalid or redundant comment.
138-162: 🛠️ Refactor suggestionAdd timeout handling for event loading.
The
_load_eventmethod should handle timeouts when processing events to prevent blocking indefinitely.Apply this diff to add timeout handling:
def _load_event(self, message: Dict) -> Optional[WebsocketsEvent]: + try: + return asyncio.wait_for(self._process_event(message), timeout=30) # 30s timeout + except asyncio.TimeoutError: + log_warning("[v1/audio/transcriptions] timeout while loading event") + return None + + async def _process_event(self, message: Dict) -> Optional[WebsocketsEvent]: event_id = message.get("event_id") or "" event_type = message.get("type") or "" data = message.get("data") or {}Likely invalid or redundant comment.
cozepy/websockets/chat/__init__.py (1)
219-234: 🛠️ Refactor suggestionAdd timeout handling to queue operations.
The
appendandcommitmethods should handle timeouts to prevent blocking indefinitely.Apply this diff to add timeout handling:
async def append(self, delta: bytes) -> None: + try: + await asyncio.wait_for( + self._input_queue.put( + InputAudioBufferAppendEvent.model_validate( + { + "data": InputAudioBufferAppendEvent.Data.model_validate( + { + "delta": delta, + } + ) + } + ) + ), + timeout=30 # 30 seconds timeout + ) + except asyncio.TimeoutError: + raise TimeoutError("Timeout while appending audio data") async def commit(self) -> None: + try: + await asyncio.wait_for( + self._input_queue.put(InputAudioBufferCommitEvent.model_validate({})), + timeout=30 # 30 seconds timeout + ) + except asyncio.TimeoutError: + raise TimeoutError("Timeout while committing audio data")Likely invalid or redundant comment.
cozepy/websockets/ws.py (3)
421-421:⚠️ Potential issueFix incorrect usage of
sendmethod in WebSocket connection.The
sendmethod of the WebSocket connection does not accept a second argument.Apply this diff to fix the issue:
- await self._ws.send(event.model_dump_json(), True) + await self._ws.send(event.model_dump_json())Likely invalid or redundant comment.
139-141:⚠️ Potential issueEnhance error handling for connection failures.
The exception handling for
InvalidStatusmay result inAttributeErrorife.responseore.response.headersisNone.Apply this diff to enhance error handling:
- raise CozeAPIError(None, f"{e}", e.response.headers.get("x-tt-logid")) from e + log_id = e.response.headers.get("x-tt-logid") if e.response and e.response.headers else None + raise CozeAPIError(None, f"{e}", log_id) from eLikely invalid or redundant comment.
325-333: 🛠️ Refactor suggestionAdd timeout handling for WebSocket operations.
The send and receive loops should include timeout handling to prevent indefinite blocking.
Apply this diff to add timeout handling:
async def _send_loop(self) -> None: try: while True: - if event := await self._input_queue.get(): - await self._send_event(event) - self._input_queue.task_done() + try: + if event := await asyncio.wait_for(self._input_queue.get(), timeout=30): # 30s timeout + await self._send_event(event) + self._input_queue.task_done() + except asyncio.TimeoutError: + continue except Exception as e: await self._handle_error(e)Likely invalid or redundant comment.
chyroc
force-pushed
the
ws
branch
2 times, most recently
from
5222568 to
880f579
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 7
♻️ Duplicate comments (6)
cozepy/websockets/chat/__init__.py (1)
113-114: 🛠️ Refactor suggestionAdd timeout handling to queue operations to prevent indefinite blocking
The
input_audio_buffer_appendandinput_audio_buffer_completemethods should handle timeouts when putting items into the queue. This prevents the possibility of blocking indefinitely if the queue is full or not being consumed. This concern was raised in a previous review and is still applicable.Apply this diff to add timeout handling to the synchronous methods:
def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent.Data) -> None: + try: + self._input_queue.put( + InputAudioBufferAppendEvent.model_validate({"data": data}), + timeout=30 # 30 seconds timeout + ) + except queue.Full: + raise TimeoutError("Timeout while appending audio data") def input_audio_buffer_complete(self) -> None: + try: + self._input_queue.put( + InputAudioBufferCompleteEvent.model_validate({}), + timeout=30 # 30 seconds timeout + ) + except queue.Full: + raise TimeoutError("Timeout while completing audio input")Apply this diff to add timeout handling to the asynchronous methods:
async def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent.Data) -> None: + try: + await asyncio.wait_for( + self._input_queue.put(InputAudioBufferAppendEvent.model_validate({"data": data})), + timeout=30 # 30 seconds timeout + ) + except asyncio.TimeoutError: + raise TimeoutError("Timeout while appending audio data") async def input_audio_buffer_complete(self) -> None: + try: + await asyncio.wait_for( + self._input_queue.put(InputAudioBufferCompleteEvent.model_validate({})), + timeout=30 # 30 seconds timeout + ) + except asyncio.TimeoutError: + raise TimeoutError("Timeout while completing audio input")Also applies to: 116-117, 265-266, 268-269
cozepy/websockets/ws.py (3)
352-354:⚠️ Potential issueEnhance error handling for connection failures
The exception handling in the
connectmethod may result in anAttributeErrorife.responseore.response.headersisNone. To prevent this, add a check to ensure that the headers are available before accessing them.Apply this diff to enhance error handling:
except InvalidStatus as e: + log_id = e.response.headers.get("x-tt-logid") if e.response and e.response.headers else None - raise CozeAPIError(None, f"{e}", e.response.headers.get("x-tt-logid")) from e + raise CozeAPIError(None, f"{e}", log_id) from e
480-483:⚠️ Potential issueFix incorrect usage of
sendmethod in WebSocket connectionThe
sendmethod of the WebSocket connection does not accept a second argument. PassingTrueas a second argument is invalid and may cause aTypeError.Apply this diff to fix the issue:
async def _send_event(self, event: WebsocketsEvent) -> None: log_debug("[%s] send event, type=%s", self._path, event.type.value) - await self._ws.send(event.model_dump_json(), True) + await self._ws.send(event.model_dump_json())
370-376: 🛠️ Refactor suggestionAdd timeout handling for WebSocket operations
The
_send_loopand_receive_loopmethods should include timeout handling to prevent indefinite blocking:
- In
_send_loop, the queue operations should have timeouts to handle cases where no events are being received.- In
_receive_loop, theawait self._ws.recv()call should have a timeout to prevent blocking indefinitely.Apply this diff to add timeout handling:
async def _send_loop(self) -> None: try: while True: - event = await self._input_queue.get() + try: + event = await asyncio.wait_for(self._input_queue.get(), timeout=30) # 30 seconds timeout + except asyncio.TimeoutError: + continue await self._send_event(event) self._input_queue.task_done() except Exception as e: await self._handle_error(e) async def _receive_loop(self) -> None: try: while True: if not self._ws: log_debug("[%s] empty websocket conn, close", self._path) break - data = await self._ws.recv() + try: + data = await asyncio.wait_for(self._ws.recv(), timeout=30) # 30 seconds timeout + except asyncio.TimeoutError: + continue message = json.loads(data) event_type = message.get("type") log_debug("[%s] receive event, type=%s, event=%s", self._path, event_type, data) except Exception as e: await self._handle_error(e)Also applies to: 379-396
examples/websockets_audio_transcriptions.py (2)
48-62: 🛠️ Refactor suggestionImprove voice selection and error handling in the audio stream
The current implementation assumes that the
voiceslist is non-empty and uses the first voice without validation. Additionally, there is no error handling for API calls, which can lead to unhandled exceptions if the API fails or returns unexpected results.Apply this diff to improve error handling:
def wrap_coze_speech_to_iterator(coze: AsyncCoze, text: str): async def iterator(): - voices = await coze.audio.voices.list(**kwargs) - content = await coze.audio.speech.create( - input=text, - voice_id=voices.items[0].voice_id, - response_format=AudioFormat.WAV, - sample_rate=24000, - **kwargs, - ) - for data in content._raw_response.iter_bytes(chunk_size=1024): - yield data + try: + voices = await coze.audio.voices.list(**kwargs) + if not voices.items: + raise ValueError("No voices available") + + content = await coze.audio.speech.create( + input=text, + voice_id=voices.items[0].voice_id, + response_format=AudioFormat.WAV, + sample_rate=24000, + **kwargs, + ) + + async with content._raw_response as response: + async for data in response.iter_bytes(chunk_size=1024): + yield data + except Exception as e: + logging.error(f"Error in audio stream: {str(e)}", exc_info=True) + raise return iterator
82-93: 🛠️ Refactor suggestionAdd error handling around WebSocket operations
The WebSocket operations should be wrapped in try-except blocks to handle potential connection issues or streaming errors gracefully.
Apply this diff to add error handling:
# Create and connect WebSocket client - async with transcriptions() as client: - async for delta in speech_stream(): - await client.input_audio_buffer_append( - InputAudioBufferAppendEvent.Data.model_validate( - { - "delta": delta, - } - ) - ) - await client.input_audio_buffer_complete() - await client.wait() + try: + async with transcriptions() as client: + try: + async for delta in speech_stream(): + await client.input_audio_buffer_append( + InputAudioBufferAppendEvent.Data.model_validate( + { + "delta": delta, + } + ) + ) + await client.input_audio_buffer_complete() + await client.wait() + except Exception as e: + logging.error("Error during audio streaming", exc_info=e) + raise + except Exception as e: + logging.error("Error in WebSocket connection", exc_info=e) + raise
🧹 Nitpick comments (1)
cozepy/websockets/chat/__init__.py (1)
152-154: Address the TODO comments forCONVERSATION_CHAT_REQUIRES_ACTIONevent handlingThere are
# TODOcomments in the event handlers for theCONVERSATION_CHAT_REQUIRES_ACTIONevent at lines 152-153 and 302-305. This indicates incomplete implementation of data validation or handling for this event. Completing the implementation is important to ensure proper event processing.Do you want me to help implement the missing parts, or would you like me to open a new GitHub issue to track this task?
Also applies to: 302-306
🛑 Comments failed to post (7)
cozepy/websockets/chat/__init__.py (2)
113-114:
⚠️ Potential issueInconsistent parameter type in
input_audio_buffer_appendmethodThe
input_audio_buffer_appendmethod acceptsdataof typeInputAudioBufferAppendEvent, but it wraps it again usingInputAudioBufferAppendEvent.model_validate({"data": data}). This may lead to incorrect data structures or double-wrapping the event. Consider changing the parameter type toInputAudioBufferAppendEvent.Datato align with the expected structure and maintain consistency with the asynchronous counterpart.Apply this diff to fix the parameter type:
-def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent) -> None: +def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent.Data) -> None: self._input_queue.put(InputAudioBufferAppendEvent.model_validate({"data": data}))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent.Data) -> None: self._input_queue.put(InputAudioBufferAppendEvent.model_validate({"data": data}))
291-307:
⚠️ Potential issueLogical error due to missing
elifstatements in_load_eventmethodIn the
_load_eventmethod ofAsyncWebsocketsChatClient, after the initialifandelifstatements, subsequent conditions should useelifinstead ofif. Usingifwithoutelifmay lead to multiple blocks executing unintentionally if multiple conditions are true. Replaceifwithelifin lines 291 and 299 to ensure proper control flow.Apply this diff to fix the logical error:
elif event_type == WebsocketsEventType.CONVERSATION_CHAT_CREATED.value: return ConversationChatCreatedEvent.model_validate( { "event_id": event_id, "logid": logid, "data": Chat.model_validate(data), } ) - if event_type == WebsocketsEventType.CONVERSATION_MESSAGE_DELTA.value: + elif event_type == WebsocketsEventType.CONVERSATION_MESSAGE_DELTA.value: return ConversationMessageDeltaEvent.model_validate( { "event_id": event_id, "logid": logid, "data": Message.model_validate(data), } ) - if event_type == WebsocketsEventType.CONVERSATION_CHAT_REQUIRES_ACTION.value: + elif event_type == WebsocketsEventType.CONVERSATION_CHAT_REQUIRES_ACTION.value: return ConversationChatRequiresActionEvent.model_validate( { # TODO "event_id": event_id, "logid": logid, "data": Message.model_validate(data), } )Committable suggestion skipped: line range outside the PR's diff.
examples/websockets_chat.py (1)
38-43: 🛠️ Refactor suggestion
Avoid logging stack traces in production code.
Logging full stack traces in production can expose sensitive information and consume excessive log storage.
Consider logging only essential error information:
async def on_error(self, cli: AsyncWebsocketsAudioTranscriptionsClient, e: Exception): - import traceback - - log_info(f"Error occurred: {str(e)}") - log_info(f"Stack trace:\n{traceback.format_exc()}") + log_info(f"Error occurred: {e.__class__.__name__}: {str(e)}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.async def on_error(self, cli: AsyncWebsocketsAudioTranscriptionsClient, e: Exception): log_info(f"Error occurred: {e.__class__.__name__}: {str(e)}")cozepy/websockets/audio/transcriptions/__init__.py (2)
121-122: 🛠️ Refactor suggestion
Add validation and size limits for audio data.
The audio buffer append methods lack validation and size limits for the audio data.
Add validation:
def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent) -> None: + if not data or not data.delta: + raise ValueError("Invalid audio data") + if len(data.delta) > 1024 * 1024: # 1MB limit per chunk + raise ValueError("Audio data chunk exceeds size limit") self._input_queue.put(InputAudioBufferAppendEvent.model_validate({"data": data}))Also applies to: 231-232
127-160: 🛠️ Refactor suggestion
Enhance error handling for event loading.
The
_load_eventmethod needs better error handling for base64 decoding and event validation.Add error handling:
def _load_event(self, message: Dict) -> Optional[WebsocketsEvent]: event_id = message.get("event_id") or "" event_type = message.get("type") or "" data = message.get("data") or {} + if not event_type: + log_warning("[v1/audio/transcriptions] missing event type") + return None if event_type == WebsocketsEventType.INPUT_AUDIO_BUFFER_COMPLETED.value: return InputAudioBufferCompletedEvent.model_validate( { "event_id": event_id, } ) elif event_type == WebsocketsEventType.TRANSCRIPTIONS_MESSAGE_UPDATE.value: + if not isinstance(data.get("content"), str): + log_warning("[v1/audio/transcriptions] invalid content type") + return None return TranscriptionsMessageUpdateEvent.model_validate( { "event_id": event_id, "data": TranscriptionsMessageUpdateEvent.Data.model_validate( { "content": data.get("content") or "", } ), } )Also applies to: 237-266
cozepy/websockets/audio/speech/__init__.py (2)
241-242: 🛠️ Refactor suggestion
Add validation for text data.
The text buffer methods lack validation for the input data.
Add validation:
async def input_text_buffer_append(self, data: InputTextBufferAppendEvent) -> None: + if not data or not data.delta: + raise ValueError("Invalid text data") + if len(data.delta.encode('utf-8')) > 1024 * 1024: # 1MB limit per chunk + raise ValueError("Text data chunk exceeds size limit") await self._input_queue.put(InputTextBufferAppendEvent.model_validate({"data": data})) async def speech_update(self, data: SpeechUpdateEvent.Data) -> None: + if not data or not data.output_audio: + raise ValueError("Invalid speech update data") await self._input_queue.put(SpeechUpdateEvent.model_validate({"data": data}))Also applies to: 247-248
201-214: 🛠️ Refactor suggestion
Remove duplicate event handler implementation.
The
EventHandlerclass insideAsyncWebsocketsAudioSpeechClientduplicates the functionality ofAsyncWebsocketsAudioSpeechEventHandler.Remove the duplicate implementation and use
AsyncWebsocketsAudioSpeechEventHandlerinstead:class AsyncWebsocketsAudioSpeechClient(AsyncWebsocketsBaseClient): - class EventHandler(AsyncWebsocketsBaseEventHandler): - async def on_input_text_buffer_completed( - self, cli: "AsyncWebsocketsAudioSpeechClient", event: InputTextBufferCompletedEvent - ): - pass - - async def on_speech_audio_update(self, cli: "AsyncWebsocketsAudioSpeechClient", event: SpeechAudioUpdateEvent): - pass - - async def on_speech_audio_completed( - self, cli: "AsyncWebsocketsAudioSpeechClient", event: SpeechAudioCompletedEvent - ): - pass
chyroc
force-pushed
the
ws
branch
2 times, most recently
from
32cb694 to
60e650b
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
cozepy/websockets/chat/__init__.py (1)
133-141: 🛠️ Refactor suggestionAdd timeout handling to queue operations.
The queue operations in
chat_update,input_audio_buffer_append, andinput_audio_buffer_completemethods should handle timeouts to prevent blocking indefinitely.Apply this diff to add timeout handling:
def chat_update(self, data: ChatUpdateEvent.Data) -> None: - self._input_queue.put(ChatUpdateEvent.model_validate({"data": data})) + try: + self._input_queue.put( + ChatUpdateEvent.model_validate({"data": data}), + timeout=30 # 30 seconds timeout + ) + except queue.Full: + raise TimeoutError("Timeout while updating chat") def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent) -> None: - self._input_queue.put(InputAudioBufferAppendEvent.model_validate({"data": data})) + try: + self._input_queue.put( + InputAudioBufferAppendEvent.model_validate({"data": data}), + timeout=30 # 30 seconds timeout + ) + except queue.Full: + raise TimeoutError("Timeout while appending audio buffer") def input_audio_buffer_complete(self) -> None: - self._input_queue.put(InputAudioBufferCompleteEvent.model_validate({})) + try: + self._input_queue.put( + InputAudioBufferCompleteEvent.model_validate({}), + timeout=30 # 30 seconds timeout + ) + except queue.Full: + raise TimeoutError("Timeout while completing audio buffer")cozepy/websockets/ws.py (1)
200-208: 🛠️ Refactor suggestionAdd timeout handling to queue operations.
The queue operations in the send loop should handle timeouts to prevent blocking indefinitely.
Apply this diff to add timeout handling:
def _send_loop(self) -> None: try: while True: - if event := self._input_queue.get(): - self._send_event(event) - self._input_queue.task_done() + try: + event = self._input_queue.get(timeout=30) # 30 seconds timeout + if event: + self._send_event(event) + self._input_queue.task_done() + except queue.Empty: + continue except Exception as e: self._handle_error(e)🧰 Tools
🪛 GitHub Actions: CI
[error] 203-203: Invalid syntax error detected by mypy
cozepy/websockets/audio/speech/__init__.py (1)
245-254:⚠️ Potential issueAdd error handling for base64 decoding to prevent exceptions
The base64 decoding operation could raise
binascii.Errorfor invalid base64 strings. Consider adding proper error handling.Apply this diff to enhance error handling:
delta_base64 = data.get("delta") if delta_base64 is None: raise ValueError("Missing 'delta' in event data") +import binascii +try: + decoded_delta = base64.b64decode(delta_base64) +except binascii.Error as e: + raise ValueError(f"Invalid base64 data: {str(e)}") return SpeechAudioUpdateEvent.model_validate( { "event_id": event_id, "logid": logid, "data": SpeechAudioUpdateEvent.Data.model_validate( { - "delta": base64.b64decode(delta_base64), + "delta": decoded_delta, } ), } )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
cozepy/__init__.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)tests/test_conversations_messages.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/test_conversations_messages.py
🚧 Files skipped from review as they are similar to previous changes (7)
- examples/websockets_audio_speech.py
- examples/websockets_audio_transcriptions.py
- cozepy/websockets/init.py
- examples/websockets_chat.py
- cozepy/init.py
- cozepy/websockets/audio/init.py
- cozepy/websockets/audio/transcriptions/init.py
🧰 Additional context used
🪛 GitHub Actions: CI
cozepy/websockets/ws.py
[error] 203-203: Invalid syntax error detected by mypy
🔇 Additional comments (5)
cozepy/websockets/chat/__init__.py (3)
28-42: LGTM! Well-structured event model.The class follows best practices by:
- Using Pydantic models for data validation
- Making fields optional for flexibility
- Using nested models for better organization
46-48: LGTM! Clean event model.The class follows best practices by using the
Chatmodel for data validation.
324-324: Address the TODO comment.The TODO comment should be addressed to ensure complete implementation.
Do you want me to help implement the missing functionality or open a GitHub issue to track this task?
cozepy/websockets/ws.py (2)
510-510:⚠️ Potential issueRemove invalid parameter in WebSocket send.
The
sendmethod of the WebSocket connection does not accept a second argument. TheTrueparameter should be removed.Apply this diff to fix the issue:
- await self._ws.send(event.model_dump_json(), True) + await self._ws.send(event.model_dump_json())Likely invalid or redundant comment.
203-203:⚠️ Potential issueFix syntax error in send loop.
The condition in the if statement is incorrect. The queue get operation should be assigned to a variable before the condition check.
Apply this diff to fix the syntax error:
- if event := self._input_queue.get(): + event = self._input_queue.get() + if event:Likely invalid or redundant comment.
🧰 Tools
🪛 GitHub Actions: CI
[error] 203-203: Invalid syntax error detected by mypy
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (3)
cozepy/websockets/audio/speech/__init__.py (3)
54-56:⚠️ Potential issueFix base64 encoding to return string.
The
serialize_deltamethod returns bytes frombase64.b64encode(). This should be decoded to UTF-8 for proper serialization.@field_serializer("delta") def serialize_delta(self, delta: bytes, _info): - return base64.b64encode(delta) + return base64.b64encode(delta).decode('utf-8')
126-140:⚠️ Potential issueEnhance security in base64 decoding.
The base64 decoding operation needs additional validation for security.
if event_type == WebsocketsEventType.SPEECH_AUDIO_UPDATE.value: delta_base64 = data.get("delta") if delta_base64 is None: raise ValueError("Missing 'delta' in event data") + if not isinstance(delta_base64, str): + raise ValueError("Invalid delta type") + if len(delta_base64) > 1024 * 1024: # 1MB limit + raise ValueError("Delta exceeds size limit") + try: + delta = base64.b64decode(delta_base64) + except binascii.Error as e: + raise ValueError(f"Invalid base64 data: {str(e)}") return SpeechAudioUpdateEvent.model_validate(
227-228:⚠️ Potential issueFix parameter type in input_text_buffer_append method.
The method accepts
dataof typeInputTextBufferAppendEventbut should acceptInputTextBufferAppendEvent.Data.- async def input_text_buffer_append(self, data: InputTextBufferAppendEvent) -> None: - await self._input_queue.put(InputTextBufferAppendEvent.model_validate({"data": data})) + async def input_text_buffer_append(self, data: InputTextBufferAppendEvent.Data) -> None: + await self._input_queue.put(InputTextBufferAppendEvent(data=data))
🧹 Nitpick comments (2)
cozepy/websockets/ws.py (1)
25-71: Improve event type organization and documentation.The event type organization can be enhanced:
- Remove commented duplicate event types
- Use enum class docstrings for better documentation
Apply this diff to improve the organization:
class WebsocketsEventType(str, Enum): + """WebSocket event types for various functionalities. + + Categories: + - Common: Generic events (error, closed) + - Audio/Speech: Text-to-speech events + - Audio/Transcriptions: Speech-to-text events + - Chat: Real-time chat events + """ + # common CLIENT_ERROR = "client_error" # sdk error CLOSED = "closed" # connection closed # error ERROR = "error" # received error event # v1/audio/speech # req INPUT_TEXT_BUFFER_APPEND = "input_text_buffer.append" # send text to server INPUT_TEXT_BUFFER_COMPLETE = ( "input_text_buffer.complete" # no text to send, after audio all received, can close connection ) SPEECH_UPDATE = "speech.update" # send speech config to server # resp INPUT_TEXT_BUFFER_COMPLETED = "input_text_buffer.completed" # received `input_text_buffer.complete` event SPEECH_AUDIO_UPDATE = "speech.audio.update" # received `speech.update` event SPEECH_AUDIO_COMPLETED = "speech.audio.completed" # all audio received, can close connection # v1/audio/transcriptions # req INPUT_AUDIO_BUFFER_APPEND = "input_audio_buffer.append" # send audio to server INPUT_AUDIO_BUFFER_COMPLETE = ( "input_audio_buffer.complete" # no audio to send, after text all received, can close connection ) TRANSCRIPTIONS_UPDATE = "transcriptions.update" # send transcriptions config to server # resp INPUT_AUDIO_BUFFER_COMPLETED = "input_audio_buffer.completed" # received `input_audio_buffer.complete` event TRANSCRIPTIONS_MESSAGE_UPDATE = "transcriptions.message.update" # received `transcriptions.update` event TRANSCRIPTIONS_MESSAGE_COMPLETED = "transcriptions.message.completed" # all audio received, can close connection # v1/chat # req - # INPUT_AUDIO_BUFFER_APPEND = "input_audio_buffer.append" # send audio to server - # INPUT_AUDIO_BUFFER_COMPLETE = "input_audio_buffer.complete" # no audio send, start chat CHAT_UPDATE = "chat.update" # send chat config to server CONVERSATION_CHAT_SUBMIT_TOOL_OUTPUTS = "conversation.chat.submit_tool_outputs" # send tool outputs to servercozepy/websockets/chat/__init__.py (1)
28-41: Enhance ChatConfig documentation and validation.The ChatConfig class needs:
- Field documentation
- Validation for metadata keys and values
- Type constraints for optional fields
Apply this diff to improve the implementation:
class ChatUpdateEvent(WebsocketsEvent): class ChatConfig(BaseModel): + """Chat configuration parameters. + + Attributes: + conversation_id: Unique identifier for the conversation + user_id: Identifier of the user in the conversation + meta_data: Additional metadata for the conversation + custom_variables: Custom variables for chat processing + extra_params: Additional parameters for chat configuration + auto_save_history: Whether to automatically save chat history + """ + model_config = { + "json_schema_extra": { + "examples": [ + { + "conversation_id": "conv_123", + "user_id": "user_456", + "meta_data": {"source": "mobile_app"} + } + ] + } + } + conversation_id: Optional[str] = None user_id: Optional[str] = None - meta_data: Optional[Dict[str, str]] = None + meta_data: Optional[Dict[str, str]] = Field( + default=None, + description="Additional metadata for the conversation", + max_length=10 # Limit number of metadata entries + ) custom_variables: Optional[Dict[str, str]] = None extra_params: Optional[Dict[str, str]] = None auto_save_history: Optional[bool] = None
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
cozepy/__init__.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)cozepy/workflows/runs/__init__.py(2 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)tests/test_conversations_messages.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/test_conversations_messages.py
🚧 Files skipped from review as they are similar to previous changes (3)
- cozepy/websockets/init.py
- examples/websockets_audio_speech.py
- cozepy/websockets/audio/init.py
🧰 Additional context used
🪛 GitHub Actions: CI
cozepy/websockets/audio/transcriptions/__init__.py
[error] 106-106: Argument 'on_event' to 'init' of 'WebsocketsBaseClient' has incompatible type 'Union[WebsocketsAudioTranscriptionsEventHandler, Dict[WebsocketsEventType, Callable[..., Any]]]'
[error] 216-216: Argument 'on_event' to 'init' of 'AsyncWebsocketsBaseClient' has incompatible type 'Union[AsyncWebsocketsAudioTranscriptionsEventHandler, Dict[WebsocketsEventType, Callable[..., Any]]]'
cozepy/websockets/ws.py
[error] 152-152: Need type annotation for '_completed_events' (hint: '_completed_events: Set[] = ...')
[error] 405-405: Argument 1 to '_send_event' of 'AsyncWebsocketsBaseClient' has incompatible type 'Optional[WebsocketsEvent]'; expected 'WebsocketsEvent'
[error] 513-513: Too many arguments for 'send' of 'Connection'
cozepy/websockets/chat/__init__.py
[error] 139-139: Argument 'on_event' to 'init' of 'WebsocketsBaseClient' has incompatible type 'Union[WebsocketsChatEventHandler, Dict[WebsocketsEventType, Callable[..., Any]]]'
[error] 290-290: Argument 'on_event' to 'init' of 'AsyncWebsocketsBaseClient' has incompatible type 'Union[AsyncWebsocketsChatEventHandler, Dict[WebsocketsEventType, Callable[..., Any]]]'
cozepy/websockets/audio/speech/__init__.py
[error] 100-100: Argument 'on_event' to 'init' of 'WebsocketsBaseClient' has incompatible type 'Union[WebsocketsAudioSpeechEventHandler, Dict[WebsocketsEventType, Callable[..., Any]]]'
[error] 222-222: Argument 'on_event' to 'init' of 'AsyncWebsocketsBaseClient' has incompatible type 'Union[EventHandler, Dict[WebsocketsEventType, Callable[..., Any]]]'
🔇 Additional comments (13)
cozepy/websockets/audio/transcriptions/__init__.py (1)
221-222:⚠️ Potential issueAlign parameter type in
transcriptions_updatemethodThe
transcriptions_updatemethod in the asynchronous client acceptsdata: InputAudio, whereas the synchronous version acceptsdata: TranscriptionsUpdateEvent.Data. This inconsistency can lead to confusion and potential errors.Apply this diff to unify the parameter type:
async def transcriptions_update(self, data: InputAudio) -> None: + async def transcriptions_update(self, data: TranscriptionsUpdateEvent.Data) -> None: await self._input_queue.put(TranscriptionsUpdateEvent.model_validate({"data": data}))Likely invalid or redundant comment.
cozepy/workflows/runs/__init__.py (2)
256-256: Confirm change of parameter name from 'event_id' to 'id' in request bodyThe parameter name in the request body has been changed from
"event_id"to"id". Ensure that this change aligns with the API specifications and that the backend expects the parameter as"id".Please double-check the API documentation to confirm that the parameter name should be
"id". If so, the change is appropriate.
388-388: Confirm change of parameter name from 'event_id' to 'id' in request bodyConsistent with the synchronous client, the parameter name in the request body has been changed from
"event_id"to"id"in the asynchronous client. Verify that this change matches the API requirements.Please ensure that the API expects
"id"as the parameter name in the request body for theresumemethod.examples/websockets_audio_transcriptions.py (2)
83-93: Add error handling around WebSocket operationsThe current implementation lacks error handling for the WebSocket operations, which could lead to unhandled exceptions and resource leaks. Wrapping the operations in try-except blocks will help manage potential errors gracefully.
This suggestion is similar to a past review comment regarding error handling in WebSocket operations.
Apply this diff to add error handling:
async with transcriptions() as client: try: async for delta in speech_stream(): await client.input_audio_buffer_append( InputAudioBufferAppendEvent.Data.model_validate( { "delta": delta, } ) ) await client.input_audio_buffer_complete() await client.wait() + except Exception as e: + logging.error("Error during audio streaming", exc_info=True) + raise
58-59: 🛠️ Refactor suggestionEnsure proper context management with asynchronous responses
When working with asynchronous HTTP responses, it's important to use an
async withcontext manager to ensure that resources are properly released.Apply this diff to manage the response context:
- for data in content._raw_response.iter_bytes(chunk_size=1024): + async with content._raw_response as response: + async for data in response.iter_bytes(chunk_size=1024): yield dataLikely invalid or redundant comment.
cozepy/websockets/ws.py (2)
510-514:⚠️ Potential issueRemove extra argument from WebSocket send method.
The WebSocket send method is called with an extra boolean argument that's not supported.
Apply this diff to fix the method call:
async def _send_event(self, event: WebsocketsEvent) -> None: log_debug("[%s] send event, type=%s", self._path, event.event_type.value) if self._ws: - await self._ws.send(event.model_dump_json(), True) + await self._ws.send(event.model_dump_json())Likely invalid or redundant comment.
🧰 Tools
🪛 GitHub Actions: CI
[error] 513-513: Too many arguments for 'send' of 'Connection'
404-406:⚠️ Potential issueFix type mismatch in event handling.
The event from queue.get() is Optional[WebsocketsEvent] but _send_event expects WebsocketsEvent.
Apply this diff to fix the type mismatch:
- event = await self._input_queue.get() - await self._send_event(event) + if event := await self._input_queue.get(): + await self._send_event(event) self._input_queue.task_done()Likely invalid or redundant comment.
🧰 Tools
🪛 GitHub Actions: CI
[error] 405-405: Argument 1 to '_send_event' of 'AsyncWebsocketsBaseClient' has incompatible type 'Optional[WebsocketsEvent]'; expected 'WebsocketsEvent'
examples/websockets_chat.py (3)
60-73: 🛠️ Refactor suggestionAdd error handling and validation.
The function needs:
- Error handling for API calls
- Validation for voices list
- Memory management for audio streaming
Apply this diff to improve the implementation:
def wrap_coze_speech_to_iterator(coze: AsyncCoze, text: str): async def iterator(): - voices = await coze.audio.voices.list(**kwargs) - content = await coze.audio.speech.create( - input=text, - voice_id=voices.items[0].voice_id, - response_format=AudioFormat.WAV, - sample_rate=24000, - **kwargs, - ) - for data in content._raw_response.iter_bytes(chunk_size=1024): - yield data + try: + voices = await coze.audio.voices.list(**kwargs) + if not voices.items: + raise ValueError("No voices available") + + content = await coze.audio.speech.create( + input=text, + voice_id=voices.items[0].voice_id, + response_format=AudioFormat.WAV, + sample_rate=24000, + **kwargs, + ) + + chunk_size = 1024 * 16 # 16KB chunks for better memory usage + for data in content._raw_response.iter_bytes(chunk_size=chunk_size): + yield data + + except Exception as e: + log_error(f"Error in speech iterator: {e}", exc_info=True) + raise return iteratorLikely invalid or redundant comment.
31-58: 🛠️ Refactor suggestionImprove state management and error handling.
The current implementation has several issues:
- Using class variable
deltafor instance data- No memory management for audio data
- Missing error handling
Apply this diff to improve the implementation:
class AsyncWebsocketsChatEventHandlerSub(AsyncWebsocketsChatEventHandler): - delta = [] + def __init__(self): + super().__init__() + self.delta: list[bytes] = [] + self._max_size = 1024 * 1024 * 100 # 100MB limit async def on_error(self, cli: AsyncWebsocketsAudioTranscriptionsClient, e: Exception): import traceback log_info(f"Error occurred: {str(e)}") log_info(f"Stack trace:\n{traceback.format_exc()}") async def on_conversation_chat_created(self, cli: AsyncWebsocketsChatClient, event: ConversationChatCreatedEvent): log_info("[examples] asr completed, logid=%s", event.detail.logid) async def on_conversation_message_delta(self, cli: AsyncWebsocketsChatClient, event: ConversationMessageDeltaEvent): print("Received:", event.data.content) async def on_conversation_audio_delta(self, cli: AsyncWebsocketsChatClient, event: ConversationAudioDeltaEvent): - self.delta.append(event.data.get_audio()) + try: + audio_data = event.data.get_audio() + if audio_data: + if sum(len(chunk) for chunk in self.delta) + len(audio_data) > self._max_size: + raise MemoryError("Audio data exceeds size limit") + self.delta.append(audio_data) + except Exception as e: + log_info(f"Error processing audio data: {e}") + await cli.close() async def on_conversation_chat_completed( self, cli: "AsyncWebsocketsChatClient", event: ConversationChatCompletedEvent ): log_info("[examples] Saving audio data to output.wav") - write_pcm_to_wav_file(b"".join(self.delta), "output.wav") + try: + audio_data = b"".join(self.delta) + write_pcm_to_wav_file(audio_data, "output.wav") + except Exception as e: + log_info(f"Error saving audio data: {e}") + finally: + self.delta.clear() # Free memoryLikely invalid or redundant comment.
76-109: 🛠️ Refactor suggestionAdd environment variable validation and error handling.
The main function needs:
- Validation for required environment variables
- Error handling for WebSocket operations
- Resource cleanup
Apply this diff to improve the implementation:
async def main(): - coze_api_token = get_coze_api_token() - coze_api_base = get_coze_api_base() - bot_id = os.getenv("COZE_BOT_ID") - text = os.getenv("COZE_TEXT") or "你今天好吗? 今天天气不错呀" + try: + # Validate required environment variables + coze_api_token = get_coze_api_token() + if not coze_api_token: + raise ValueError("COZE_API_TOKEN is not set") + + coze_api_base = get_coze_api_base() + if not coze_api_base: + raise ValueError("COZE_API_BASE is not set") + + bot_id = os.getenv("COZE_BOT_ID") + if not bot_id: + raise ValueError("COZE_BOT_ID is not set") + + text = os.getenv("COZE_TEXT") or "你今天好吗? 今天天气不错呀" - # Initialize Coze client - coze = AsyncCoze( - auth=TokenAuth(coze_api_token), - base_url=coze_api_base, - ) - # Initialize Audio - speech_stream = wrap_coze_speech_to_iterator(coze, text) + # Initialize Coze client + coze = AsyncCoze( + auth=TokenAuth(coze_api_token), + base_url=coze_api_base, + ) + + # Initialize Audio + speech_stream = wrap_coze_speech_to_iterator(coze, text) - chat = coze.websockets.chat.create( - bot_id=bot_id, - on_event=AsyncWebsocketsChatEventHandlerSub(), - **kwargs, - ) + chat = coze.websockets.chat.create( + bot_id=bot_id, + on_event=AsyncWebsocketsChatEventHandlerSub(), + **kwargs, + ) - # Create and connect WebSocket client - async with chat() as client: - # Read and send audio data - async for delta in speech_stream(): - await client.input_audio_buffer_append( - InputAudioBufferAppendEvent.Data.model_validate( - { - "delta": delta, - } - ) - ) - await client.input_audio_buffer_complete() - await client.wait() + # Create and connect WebSocket client + async with chat() as client: + try: + # Read and send audio data + async for delta in speech_stream(): + await client.input_audio_buffer_append( + InputAudioBufferAppendEvent.Data.model_validate( + { + "delta": delta, + } + ) + ) + await client.input_audio_buffer_complete() + await client.wait() + except Exception as e: + log_error(f"Error in WebSocket communication: {e}", exc_info=True) + raise + except Exception as e: + log_error(f"Error in main: {e}", exc_info=True) + raiseLikely invalid or redundant comment.
cozepy/__init__.py (2)
94-125: LGTM! Well-organized websocket imports.The new imports are logically grouped and properly structured.
215-242: LGTM! Properly exposed websocket entities.The all list is well-organized with clear section comments.
cozepy/websockets/audio/speech/__init__.py (1)
270-288: LGTM! Well-structured builder implementation.The builder classes provide a clean interface for creating WebSocket clients with proper parameter handling.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
cozepy/websockets/audio/speech/__init__.py (4)
54-56:⚠️ Potential issueFix base64 encoding to return a string.
The
base64.b64encode()function returns bytes, but we need a string for JSON serialization.Apply this diff to fix the issue:
@field_serializer("delta") def serialize_delta(self, delta: bytes, _info): - return base64.b64encode(delta) + return base64.b64encode(delta).decode('utf-8')
126-140:⚠️ Potential issueEnhance validation and error handling for base64 decoding.
The base64 decoding needs additional validation and error handling:
- Validate delta_base64 type and size
- Handle base64 decoding errors
Apply this diff to enhance validation and error handling:
delta_base64 = data.get("delta") if delta_base64 is None: raise ValueError("Missing 'delta' in event data") + if not isinstance(delta_base64, str): + raise ValueError("Invalid delta type") + if len(delta_base64) > 1024 * 1024: # 1MB limit + raise ValueError("Delta exceeds size limit") + try: + delta = base64.b64decode(delta_base64) + except binascii.Error as e: + raise ValueError(f"Invalid base64 data: {str(e)}") return SpeechAudioUpdateEvent.model_validate( { "id": event_id, "detail": detail, "data": SpeechAudioUpdateEvent.Data.model_validate( { - "delta": base64.b64decode(delta_base64), + "delta": delta, } ), } )
105-106:⚠️ Potential issueFix parameter type inconsistency.
The method accepts
InputTextBufferAppendEvent.Databut constructs a new event instance.Apply this diff to fix the inconsistency:
def input_text_buffer_append(self, data: InputTextBufferAppendEvent.Data) -> None: - self._input_queue.put(InputTextBufferAppendEvent.model_validate({"data": data})) + self._input_queue.put(InputTextBufferAppendEvent(data=data))
227-228:⚠️ Potential issueFix parameter type inconsistency in async method.
The async method has the same parameter type inconsistency as the sync version.
Apply this diff to fix the inconsistency:
async def input_text_buffer_append(self, data: InputTextBufferAppendEvent.Data) -> None: - await self._input_queue.put(InputTextBufferAppendEvent.model_validate({"data": data})) + await self._input_queue.put(InputTextBufferAppendEvent(data=data))cozepy/websockets/ws.py (4)
152-152:⚠️ Potential issueAdd type annotation for _completed_events.
The
_completed_eventsfield is missing a type annotation.Apply this diff to fix the issue:
- self._completed_events = set() + self._completed_events: set[WebsocketsEventType] = set()
203-211: 🛠️ Refactor suggestionAdd timeout and memory management to send loop.
The send loop lacks:
- Timeout handling for queue operations
- Memory management for the input queue
Apply this diff to enhance the implementation:
def _send_loop(self) -> None: try: while True: - if event := self._input_queue.get(): - self._send_event(event) - self._input_queue.task_done() + try: + event = self._input_queue.get(timeout=30) # 30s timeout + if event: + if self._input_queue.qsize() > 1000: # Prevent memory exhaustion + raise RuntimeError("Input queue size exceeded limit") + self._send_event(event) + self._input_queue.task_done() + except queue.Empty: + continue except Exception as e: self._handle_error(e)
510-514:⚠️ Potential issueFix incorrect usage of WebSocket send method.
The
sendmethod of the WebSocket connection does not accept a second argument.Apply this diff to fix the issue:
async def _send_event(self, event: WebsocketsEvent) -> None: log_debug("[%s] send event, type=%s", self._path, event.event_type.value) if self._ws: - await self._ws.send(event.model_dump_json(), True) + await self._ws.send(event.model_dump_json())
383-385:⚠️ Potential issueEnhance error handling for connection failures.
The exception handling for
InvalidStatusmay result inAttributeErrorife.responseore.response.headersisNone.Apply this diff to enhance error handling:
except InvalidStatus as e: - raise CozeAPIError(None, f"{e}", e.response.headers.get("x-tt-logid")) from e + log_id = e.response.headers.get("x-tt-logid") if e.response and e.response.headers else None + raise CozeAPIError(None, f"{e}", log_id) from e
🧹 Nitpick comments (2)
cozepy/websockets/chat/__init__.py (2)
153-207: Fix inconsistent conditional statements.The method uses a mix of
ifandelifstatements which could lead to multiple conditions being evaluated unnecessarily. Use consistentelifstatements for mutually exclusive conditions.Apply this diff to fix the issue:
if event_type == WebsocketsEventType.INPUT_AUDIO_BUFFER_COMPLETED.value: return InputAudioBufferCompletedEvent.model_validate( { "id": event_id, "detail": detail, } ) - elif event_type == WebsocketsEventType.CONVERSATION_CHAT_CREATED.value: + if event_type == WebsocketsEventType.CONVERSATION_CHAT_CREATED.value: return ConversationChatCreatedEvent.model_validate( { "id": event_id, "detail": detail, "data": Chat.model_validate(data), } ) - elif event_type == WebsocketsEventType.CONVERSATION_MESSAGE_DELTA.value: + if event_type == WebsocketsEventType.CONVERSATION_MESSAGE_DELTA.value:
307-361: Fix inconsistent conditional statements in async method.The async method has the same issue with inconsistent conditional statements as the sync version.
Apply this diff to fix the issue:
if event_type == WebsocketsEventType.INPUT_AUDIO_BUFFER_COMPLETED.value: return InputAudioBufferCompletedEvent.model_validate( { "id": event_id, "detail": detail, } ) - elif event_type == WebsocketsEventType.CONVERSATION_CHAT_CREATED.value: + if event_type == WebsocketsEventType.CONVERSATION_CHAT_CREATED.value: return ConversationChatCreatedEvent.model_validate( { "id": event_id, "detail": detail, "data": Chat.model_validate(data), } ) - if event_type == WebsocketsEventType.CONVERSATION_MESSAGE_DELTA.value: + elif event_type == WebsocketsEventType.CONVERSATION_MESSAGE_DELTA.value:
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
cozepy/__init__.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)cozepy/workflows/runs/__init__.py(2 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)tests/test_conversations_messages.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/test_conversations_messages.py
🚧 Files skipped from review as they are similar to previous changes (4)
- cozepy/workflows/runs/init.py
- examples/websockets_audio_speech.py
- cozepy/websockets/audio/init.py
- cozepy/websockets/init.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (Python 3.8 on macOS)
🔇 Additional comments (11)
cozepy/websockets/audio/transcriptions/__init__.py (4)
26-28: Correct base64 serialization of 'delta' fieldThe
serialize_deltamethod currently returns abytesobject, which can cause issues during JSON serialization. To ensure proper serialization to JSON, thedeltafield should be encoded as a UTF-8 string.Apply this diff to fix the serialization:
@field_serializer("delta") def serialize_delta(self, delta: bytes, _info): - return base64.b64encode(delta) + return base64.b64encode(delta).decode('utf-8')
93-100: Fix incompatible 'on_event' parameter type in__init__methodThe
on_eventparameter may have an incompatible type when passed to the base classWebsocketsBaseClient. The base class expectson_eventto be of typeDict[WebsocketsEventType, Callable], but it may be an instance ofWebsocketsAudioTranscriptionsEventHandler, leading to a type error.Apply this diff to correct the issue:
def __init__( self, base_url: str, auth: Auth, requester: Requester, on_event: Union[WebsocketsAudioTranscriptionsEventHandler, Dict[WebsocketsEventType, Callable]], **kwargs, ): - if isinstance(on_event, WebsocketsAudioTranscriptionsEventHandler): - on_event = on_event.to_dict( + if isinstance(on_event, WebsocketsAudioTranscriptionsEventHandler): # Convert handler to dict + on_event = { WebsocketsEventType.INPUT_AUDIO_BUFFER_COMPLETED: on_event.on_input_audio_buffer_completed, WebsocketsEventType.TRANSCRIPTIONS_MESSAGE_UPDATE: on_event.on_transcriptions_message_update, WebsocketsEventType.TRANSCRIPTIONS_MESSAGE_COMPLETED: on_event.on_transcriptions_message_completed, - ) + } super().__init__( base_url=base_url, auth=auth,
203-210: Fix incompatible 'on_event' parameter type in async client's__init__methodIn the asynchronous client, the
on_eventparameter may also have an incompatible type when passed toAsyncWebsocketsBaseClient. Ensure thaton_eventis converted to aDictbefore being passed.Apply this diff to correct the issue:
def __init__( self, base_url: str, auth: Auth, requester: Requester, on_event: Union[AsyncWebsocketsAudioTranscriptionsEventHandler, Dict[WebsocketsEventType, Callable]], **kwargs, ): - if isinstance(on_event, AsyncWebsocketsAudioTranscriptionsEventHandler): - on_event = on_event.to_dict( + if isinstance(on_event, AsyncWebsocketsAudioTranscriptionsEventHandler): # Convert handler to dict + on_event = { WebsocketsEventType.INPUT_AUDIO_BUFFER_COMPLETED: on_event.on_input_audio_buffer_completed, WebsocketsEventType.TRANSCRIPTIONS_MESSAGE_UPDATE: on_event.on_transcriptions_message_update, WebsocketsEventType.TRANSCRIPTIONS_MESSAGE_COMPLETED: on_event.on_transcriptions_message_completed, - ) + } super().__init__( base_url=base_url, auth=auth,
230-259: Ensure consistent handling of thedetailfield in_load_eventmethodThe
_load_eventmethod in the asynchronous client does not retrieve or utilize thedetailfield from the incoming message, unlike the synchronous version. This omission can lead to incomplete event data and inconsistent logging.Apply this diff to include the
detailfield:def _load_event(self, message: Dict) -> Optional[WebsocketsEvent]: event_id = message.get("event_id") or "" event_type = message.get("event_type") or "" + detail = WebsocketsEvent.Detail.model_validate(message.get("detail") or {}) data = message.get("data") or {} if event_type == WebsocketsEventType.INPUT_AUDIO_BUFFER_COMPLETED.value: return InputAudioBufferCompletedEvent.model_validate( { "id": event_id, + "detail": detail, } ) elif event_type == WebsocketsEventType.TRANSCRIPTIONS_MESSAGE_UPDATE.value: return TranscriptionsMessageUpdateEvent.model_validate( { "id": event_id, + "detail": detail, "data": TranscriptionsMessageUpdateEvent.Data.model_validate( { "content": data.get("content") or "", } ), } ) elif event_type == WebsocketsEventType.TRANSCRIPTIONS_MESSAGE_COMPLETED.value: return TranscriptionsMessageCompletedEvent.model_validate( { "id": event_id, + "detail": detail, } ) else: - log_warning("[v1/audio/transcriptions] unknown event=%s", event_type) + log_warning("[v1/audio/transcriptions] unknown event=%s, logid=%s", event_type, detail.logid) return Noneexamples/websockets_audio_transcriptions.py (2)
50-57: Add error handling for voice selection and API callsThe current implementation assumes that
voices.itemsis non-empty and uses the first voice without validation. It also lacks error handling for API calls, which may raise exceptions.Consider adding error handling and validation:
async def iterator(): try: voices = await coze.audio.voices.list(**kwargs) + if not voices.items: + raise ValueError("No voices available") content = await coze.audio.speech.create( input=text, voice_id=voices.items[0].voice_id, response_format=AudioFormat.WAV, sample_rate=24000, **kwargs, ) + except Exception as e: + log_info(f"Error in speech iterator: {e}") + return
58-59: Useasync forwhen iterating over an asynchronous generatorThe
iter_bytesmethod is likely an asynchronous generator. Usingforinstead ofasync formay cause issues.Apply this diff to fix the iteration:
async with content._raw_response as response: - for data in response.iter_bytes(chunk_size=1024): + async for data in response.iter_bytes(chunk_size=1024): yield dataexamples/websockets_chat.py (3)
38-84: Avoid using class variables for instance-specific dataThe
deltalist is defined as a class variable, which can cause data mixing between different instances and is not thread-safe.Refactor by moving
deltato an instance variable and handle memory management:class AsyncWebsocketsChatEventHandlerSub(AsyncWebsocketsChatEventHandler): - delta = [] + def __init__(self): + self.delta = [] + self._max_size = 1024 * 1024 * 100 # 100MB limit async def on_error(self, cli: AsyncWebsocketsAudioTranscriptionsClient, e: Exception): import traceback log_info(f"Error occurred: {str(e)}") log_info(f"Stack trace:\n{traceback.format_exc()}") async def on_conversation_audio_delta(self, cli: AsyncWebsocketsChatClient, event: ConversationAudioDeltaEvent): - self.delta.append(event.data.get_audio()) + audio_data = event.data.get_audio() + if audio_data: + if sum(len(chunk) for chunk in self.delta) + len(audio_data) > self._max_size: + log_info("Audio data exceeds size limit") + await cli.close() + return + self.delta.append(audio_data) async def on_conversation_chat_completed( self, cli: "AsyncWebsocketsChatClient", event: ConversationChatCompletedEvent ): log_info("[examples] Saving audio data to output.wav") write_pcm_to_wav_file(b"".join(self.delta), "output.wav")
86-99: Add error handling inwrap_coze_speech_to_iteratorThe function assumes that
voices.itemsis non-empty and does not handle potential exceptions from API calls. This may result in anIndexErroror unhandled exceptions.Apply this diff to add error handling:
def wrap_coze_speech_to_iterator(coze: AsyncCoze, text: str): async def iterator(): + try: voices = await coze.audio.voices.list(**kwargs) + if not voices.items: + raise ValueError("No voices available") content = await coze.audio.speech.create( input=text, voice_id=voices.items[0].voice_id, response_format=AudioFormat.WAV, sample_rate=24000, **kwargs, ) for data in content._raw_response.iter_bytes(chunk_size=1024): yield data + except Exception as e: + log_info(f"Error in speech iterator: {e}") + return return iterator
105-106: Validatebot_idbefore useThe
bot_idenvironment variable may not be set, leading to issues when creating the chat client.Add validation to ensure
bot_idis notNone:bot_id = os.getenv("COZE_BOT_ID") +if not bot_id: + raise ValueError("Environment variable 'COZE_BOT_ID' is not set.") text = os.getenv("COZE_TEXT") or "你今天好吗? 今天天气不错呀"cozepy/__init__.py (2)
94-125: LGTM! Well-organized imports.The imports are properly grouped by module and functionality, following Python's import style guidelines.
215-242: LGTM! Well-documented exports.The
__all__list is properly updated with clear module groupings and comments.
chyroc
force-pushed
the
ws
branch
2 times, most recently
from
8d4836a to
0db6e9c
Compare
chyroc
force-pushed
the
ws
branch
5 times, most recently
from
de39187 to
02b2635
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
examples/utils/__init__.py (1)
26-29:⚠️ Potential issueAdd error handling and timeout for OAuth flow
The OAuth flow implementation needs error handling and timeout.
Move client ID to configuration
Hardcoding the client ID poses a security risk.
Add WebSocket-specific token validation
Since this token will be used for WebSocket connections, consider adding validation for WebSocket-specific scopes or permissions.
#!/bin/bash # Check if the token has required WebSocket permissions curl -s -H "Authorization: Bearer $COZE_API_TOKEN" "${COZE_API_BASE}/v1/auth/token/validate" | jq '.scopes[]' | grep -i 'websocket'cozepy/websockets/audio/speech/__init__.py (3)
54-56:⚠️ Potential issueFix base64 encoding in
serialize_deltamethod.The
base64.b64encode()returns bytes, but we need a string for JSON serialization.Apply this diff to fix the issue:
@field_serializer("delta") def serialize_delta(self, delta: bytes, _info): - return base64.b64encode(delta) + return base64.b64encode(delta).decode('utf-8')
136-136:⚠️ Potential issueAdd error handling for base64 decoding.
The base64 decoding operation could fail with invalid input.
Apply this diff to enhance error handling:
- "delta": base64.b64decode(delta_base64), + "delta": base64.b64decode(delta_base64) if delta_base64 else None,Also add proper error handling:
+ try: return SpeechAudioUpdateEvent.model_validate( { "id": event_id, "detail": detail, "data": SpeechAudioUpdateEvent.Data.model_validate( { "delta": base64.b64decode(delta_base64), } ), } ) + except binascii.Error as e: + raise ValueError(f"Invalid base64 data: {str(e)}")
173-173:⚠️ Potential issueFix incorrect type hints in event handler methods.
The type hints for the
cliparameter incorrectly reference the handler class instead of the client class.Apply this diff to fix the type hints:
async def on_input_text_buffer_completed( - self, cli: "AsyncWebsocketsAudioSpeechClient", event: InputTextBufferCompletedEvent + self, cli: "AsyncWebsocketsAudioSpeechCreateClient", event: InputTextBufferCompletedEvent ): pass async def on_speech_audio_update( - self, cli: "AsyncWebsocketsAudioSpeechClient", event: SpeechAudioUpdateEvent + self, cli: "AsyncWebsocketsAudioSpeechCreateClient", event: SpeechAudioUpdateEvent ): pass async def on_speech_audio_completed( - self, cli: "AsyncWebsocketsAudioSpeechClient", event: SpeechAudioCompletedEvent + self, cli: "AsyncWebsocketsAudioSpeechCreateClient", event: SpeechAudioCompletedEvent ): passAlso applies to: 177-177, 181-181
cozepy/websockets/ws.py (2)
189-189:⚠️ Potential issueEnhance error handling for connection failures.
The exception handling for
InvalidStatusmay result inAttributeErrorife.responseore.response.headersisNone.Apply this diff to enhance error handling:
- raise CozeAPIError(None, f"{e}", e.response.headers.get("x-tt-logid")) from e + log_id = e.response.headers.get("x-tt-logid") if e.response and e.response.headers else None + raise CozeAPIError(None, f"{e}", log_id) from e
206-214: 🛠️ Refactor suggestionAdd timeout handling for WebSocket operations.
The send and receive loops should include timeout handling to prevent indefinite blocking.
Apply this diff to add timeout handling:
async def _send_loop(self) -> None: try: while True: - event = await self._input_queue.get() - await self._send_event(event) - self._input_queue.task_done() + try: + event = await asyncio.wait_for(self._input_queue.get(), timeout=30) + if event: + await self._send_event(event) + self._input_queue.task_done() + except asyncio.TimeoutError: + continue except Exception as e: await self._handle_error(e)Also applies to: 409-417
🧹 Nitpick comments (9)
examples/utils/__init__.py (4)
8-15: Enhance API base URL validation and documentationWhile the function works correctly, consider these improvements:
- Validate custom base URL format when provided via environment variable
- Use a more structured docstring format
def get_coze_api_base() -> str: - # The default access is api.coze.com, but if you need to access api.coze.cn, - # please use base_url to configure the api endpoint to access + """Get the Coze API base URL. + + Returns: + str: The API base URL, either from COZE_API_BASE environment variable + or defaults to COZE_CN_BASE_URL. + """ coze_api_base = os.getenv("COZE_API_BASE") if coze_api_base: + if not coze_api_base.startswith(('http://', 'https://')): + raise ValueError("COZE_API_BASE must start with http:// or https://") return coze_api_base
18-23: Add type hints for environment variableThe function could benefit from explicit type hints for the environment variable.
def get_coze_api_token(workspace_id: Optional[str] = None) -> str: # Get an access_token through personal access token or oauth. - coze_api_token = os.getenv("COZE_API_TOKEN") + coze_api_token: Optional[str] = os.getenv("COZE_API_TOKEN") if coze_api_token: return coze_api_token
32-35: Improve logging setup robustness and documentationThe logging setup could be more robust and better documented.
def setup_examples_logger(): + """Configure logging for examples based on COZE_LOG environment variable. + + The log level can be set using standard Python logging level names + (DEBUG, INFO, WARNING, ERROR, CRITICAL). Defaults to INFO if not set + or if an invalid level is provided. + """ coze_log = os.getenv("COZE_LOG") if coze_log: - setup_logging(logging.getLevelNamesMapping().get(coze_log.upper(), logging.INFO)) + try: + log_level = logging.getLevelNamesMapping()[coze_log.upper()] + except KeyError: + print(f"Warning: Invalid log level '{coze_log}'. Using INFO level.") + log_level = logging.INFO + else: + log_level = logging.INFO + setup_logging(log_level)
1-35: Consider WebSocket-specific configuration needsAs this utility module will support WebSocket functionality, consider adding:
- WebSocket-specific timeout configurations
- WebSocket connection retry parameters
- Heartbeat interval settings
These could be exposed through environment variables like:
COZE_WS_TIMEOUTCOZE_WS_RETRY_COUNTCOZE_WS_HEARTBEAT_INTERVALcozepy/websockets/chat/__init__.py (1)
314-360: Refactor_load_eventmethod to reduce code duplicationThe
_load_eventmethod contains repetitive code blocks for each event type. This makes the code harder to maintain and increases the risk of errors.Consider refactoring the method using a mapping of event types to model classes:
event_type_map = { WebsocketsEventType.INPUT_AUDIO_BUFFER_COMPLETED.value: InputAudioBufferCompletedEvent, WebsocketsEventType.CONVERSATION_CHAT_CREATED.value: ConversationChatCreatedEvent, # Add other event types here } model_class = event_type_map.get(event_type) if model_class: return model_class.model_validate({ "id": event_id, "detail": detail, "data": model_class.Data.model_validate(data) if hasattr(model_class, 'Data') else data, }) else: log_warning("[%s] unknown event, type=%s, logid=%s", self._path, event_type, detail.logid)This refactoring improves maintainability and clarity.
examples/websockets_chat.py (3)
36-40: Move import statement to the top of the fileImporting modules inside functions can lead to inefficiencies and is generally discouraged in Python.
Move the
import tracebackstatement to the top of the file.
73-79: Add error handling and limit memory usage for audio dataAppending audio data without constraints may lead to excessive memory usage. Also, file operations should be wrapped in try-except blocks to handle potential I/O errors.
Consider updating the code as follows:
async def on_conversation_chat_completed( self, cli: "AsyncWebsocketsChatClient", event: ConversationChatCompletedEvent ): log_info("[examples] Saving audio data to output.wav") - write_pcm_to_wav_file(b"".join(self.delta), "output.wav") + try: + audio_data = b"".join(self.delta) + write_pcm_to_wav_file(audio_data, "output.wav") + except Exception as e: + log_info(f"Error saving audio data: {e}") + # Handle the exception as needed + finally: + self.delta.clear() # Clear data to free memoryAdditionally, consider implementing a maximum size limit for
self.deltato prevent excessive memory consumption.
123-128: Simplify data creation using Pydantic modelsWhen creating instances of Pydantic models, you can pass keyword arguments directly instead of using
model_validatewith a dictionary.Update the code as follows:
await client.input_audio_buffer_append( - InputAudioBufferAppendEvent.Data.model_validate( - { - "delta": delta, - } - ) + InputAudioBufferAppendEvent.Data(delta=delta) )cozepy/__init__.py (1)
94-139: Organize imports for better readabilityConsider grouping related imports together and maintaining alphabetical order within each group to improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.coderabbit.yaml(0 hunks)cozepy/__init__.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)cozepy/workflows/runs/__init__.py(2 hunks)examples/utils/__init__.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)pyproject.toml(1 hunks)tests/test_conversations_messages.py(0 hunks)
💤 Files with no reviewable changes (2)
- .coderabbit.yaml
- tests/test_conversations_messages.py
🚧 Files skipped from review as they are similar to previous changes (6)
- cozepy/workflows/runs/init.py
- examples/websockets_audio_speech.py
- cozepy/websockets/init.py
- cozepy/websockets/audio/transcriptions/init.py
- examples/websockets_audio_transcriptions.py
- cozepy/websockets/audio/init.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (Python 3.8 on Windows)
🔇 Additional comments (8)
pyproject.toml (1)
19-23: LGTM! Well-structured version constraints for websockets.The version constraints are properly configured based on Python version compatibility, following best practices with semantic versioning.
Let's verify the versions for security and compatibility:
✅ Verification successful
✓ Websockets dependency configuration is secure and well-structured
The version constraints are properly configured with latest stable releases for each Python version range, all well above known security patch thresholds. The implementation in the codebase demonstrates proper usage of the library.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for security advisories and latest versions of the websockets library # Check PyPI for latest versions curl -s https://pypi.org/pypi/websockets/json | jq '.info.version' # Check for security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: PIP, package: "websockets") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' # Check compatibility with other dependencies rg -A 5 "websockets" --glob "!pyproject.toml"Length of output: 49684
cozepy/websockets/chat/__init__.py (2)
273-282: Ensure consistent handling ofon_eventparameterIn the
AsyncWebsocketsChatClientconstructor, the handling of theon_eventparameter differs from the synchronousWebsocketsChatClient. This inconsistency might lead to unexpected behavior in event handling between synchronous and asynchronous clients.Please verify that the
on_eventparameter is processed consistently in both clients. Consider aligning the implementation to prevent discrepancies.
302-304:⚠️ Potential issueFix parameter type mismatch in
input_audio_buffer_appendThe method
input_audio_buffer_appendexpectsInputAudioBufferAppendEvent.Dataas the parameter type, but you are passing it toInputAudioBufferAppendEvent.model_validate()which expects the full event data.Apply this diff to correct the parameter type:
- async def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent.Data) -> None: - await self._input_queue.put(InputAudioBufferAppendEvent.model_validate({"data": data})) + async def input_audio_buffer_append(self, data: InputAudioBufferAppendEvent) -> None: + await self._input_queue.put(data)Or, adjust the call to pass the correct data structure.
Likely invalid or redundant comment.
examples/websockets_chat.py (2)
101-103:⚠️ Potential issueValidate
bot_idbefore useThe
bot_idretrieved from the environment variable may beNone, which could cause errors when creating the chat client.Add validation for
bot_id:bot_id = os.getenv("COZE_BOT_ID") +if not bot_id: + raise ValueError("Environment variable 'COZE_BOT_ID' is not set.")Likely invalid or redundant comment.
34-35:⚠️ Potential issueUse instance variable instead of class variable for
deltaDefining
deltaas a class variable can lead to shared state across instances, which may cause unexpected behavior in concurrent scenarios.Apply this diff to make
deltaan instance variable:class AsyncWebsocketsChatEventHandlerSub(AsyncWebsocketsChatEventHandler): - delta = [] + def __init__(self): + self.delta = []Likely invalid or redundant comment.
cozepy/__init__.py (1)
229-270: Ensure all new entities are included in__all__Verify that all newly added public classes and functions are included in the
__all__list to properly export them as part of the module's public API.Double-check the
__all__list to confirm that it matches the public interface you intend to expose.cozepy/websockets/ws.py (2)
28-114: Well-structured event types and base classes!The code demonstrates good organization with:
- Clear event type categorization
- Proper use of Pydantic models
- Good separation of concerns
301-548: Well-implemented event handlers!The code demonstrates good practices:
- Proper error logging with stack traces
- Clean separation between sync and async handlers
- Good error propagation
chyroc
force-pushed
the
ws
branch
2 times, most recently
from
51359c8 to
605fc81
Compare
chyroc
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
examples/websockets_chat.py (2)
34-34:⚠️ Potential issueReplace class variable with instance variable.
Using a class variable
deltafor storing audio data can cause issues if multiple instances of the handler are created.Apply this diff to fix the issue:
- delta = [] + def __init__(self): + super().__init__() + self.delta = []
72-74:⚠️ Potential issueAdd error handling and memory management for audio data collection.
The audio data collection should handle potential memory issues and invalid data.
Apply this diff to enhance error handling:
async def on_conversation_audio_delta(self, cli: AsyncWebsocketsChatClient, event: ConversationAudioDeltaEvent): - self.delta.append(event.data.get_audio()) + try: + audio_data = event.data.get_audio() + if not audio_data: + return + current_size = sum(len(d) for d in self.delta) + if current_size + len(audio_data) > 100 * 1024 * 1024: # 100MB limit + raise MemoryError("Audio data exceeds size limit") + self.delta.append(audio_data) + except Exception as e: + log_info(f"Error collecting audio data: {e}") + await cli.close()cozepy/websockets/audio/transcriptions/__init__.py (1)
26-28:⚠️ Potential issueCorrect base64 encoding to return a string.
The
base64.b64encode()function returns bytes, which should be decoded to UTF-8 for proper string serialization.Apply this diff to fix the issue:
@field_serializer("delta") def serialize_delta(self, delta: bytes, _info): - return base64.b64encode(delta) + return base64.b64encode(delta).decode('utf-8')
🧹 Nitpick comments (7)
cozepy/websockets/ws.py (3)
19-20: Translate warning message to English for better internationalization.The warning message is currently in Chinese. Consider translating it to English to improve accessibility for international users.
- # 警告: 当前Python版本不支持asyncio websockets - # 如果Python版本小于3.8,则不支持异步websockets功能 + # Warning: Current Python version does not support asyncio websockets + # Async websockets functionality requires Python >= 3.8
90-91: Remove redundant commented event types.These event types are already defined in the audio transcriptions section (lines 77-80). Instead of commenting them out, consider referencing the existing event types.
565-565: Fix type hint for error parameter.The type hint for the error parameter in
on_errorshould beExceptionto match the base class and allow handling of all error types.- async def on_error(self, cli: "AsyncWebsocketsBaseClient", e: CozeAPIError): + async def on_error(self, cli: "AsyncWebsocketsBaseClient", e: Exception):cozepy/websockets/chat/__init__.py (3)
79-81: Add missing data field to ConversationMessageCompletedEvent.The event class should include a data field to maintain consistency with other event classes and provide completion information.
class ConversationMessageCompletedEvent(WebsocketsEvent): event_type: WebsocketsEventType = WebsocketsEventType.CONVERSATION_MESSAGE_COMPLETED + data: Optional[Message] = None
106-138: Add docstrings and improve type hints for event handler methods.The event handler methods would benefit from docstrings explaining their purpose and expected behavior. This would help users implementing custom handlers.
Example improvement for one method:
def on_chat_created(self, cli: "WebsocketsChatClient", event: ChatCreatedEvent): + """Handle chat creation event. + + Args: + cli: The WebSocket chat client instance + event: The chat created event containing creation details + """ pass
1-499: Consider adding connection health monitoring.The WebSocket implementation would benefit from adding:
- Heartbeat mechanism to detect connection health
- Automatic reconnection strategy
- Connection state monitoring and metrics
This would improve the reliability and observability of the WebSocket connections in production environments.
cozepy/__init__.py (1)
229-270: Consider standardizing module section commentsWhile the organization is good, consider standardizing the comment format for consistency:
- # websockets.audio.speech + # websockets/audio/speech - # websockets.audio.transcriptions + # websockets/audio/transcriptions - # websockets.chat + # websockets/chat - # websockets + # websockets/wsThis would match the import paths and maintain consistency with other module section comments in the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.coderabbit.yaml(0 hunks)cozepy/__init__.py(2 hunks)cozepy/websockets/__init__.py(1 hunks)cozepy/websockets/audio/__init__.py(1 hunks)cozepy/websockets/audio/speech/__init__.py(1 hunks)cozepy/websockets/audio/transcriptions/__init__.py(1 hunks)cozepy/websockets/chat/__init__.py(1 hunks)cozepy/websockets/ws.py(1 hunks)cozepy/workflows/runs/__init__.py(2 hunks)examples/utils/__init__.py(1 hunks)examples/websockets_audio_speech.py(1 hunks)examples/websockets_audio_transcriptions.py(1 hunks)examples/websockets_chat.py(1 hunks)pyproject.toml(1 hunks)tests/test_conversations_messages.py(0 hunks)
💤 Files with no reviewable changes (2)
- .coderabbit.yaml
- tests/test_conversations_messages.py
🚧 Files skipped from review as they are similar to previous changes (6)
- pyproject.toml
- examples/utils/init.py
- cozepy/workflows/runs/init.py
- cozepy/websockets/audio/init.py
- examples/websockets_audio_transcriptions.py
- cozepy/websockets/init.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (Python 3.8 on Windows)
🔇 Additional comments (16)
examples/websockets_audio_speech.py (3)
24-24: Remove or address the TODO comment.The TODO comment "todo review" should be addressed or removed if no longer needed.
Would you like me to help review and suggest improvements for this class?
30-30:⚠️ Potential issueReplace class variable with instance variable.
Using a class variable
deltafor storing audio data can cause issues if multiple instances of the handler are created, as they would share the same list.Apply this diff to fix the issue:
- delta = [] + def __init__(self): + super().__init__() + self.delta = []Likely invalid or redundant comment.
37-39:⚠️ Potential issueAdd error handling and memory management for audio data collection.
The audio data collection should handle potential memory issues and invalid data.
Apply this diff to enhance error handling:
async def on_speech_audio_update(self, cli: AsyncWebsocketsAudioSpeechClient, event: SpeechAudioUpdateEvent): - self.delta.append(event.data.delta) + try: + if not event.data.delta: + return + current_size = sum(len(d) for d in self.delta) + if current_size + len(event.data.delta) > 100 * 1024 * 1024: # 100MB limit + raise MemoryError("Audio data exceeds size limit") + self.delta.append(event.data.delta) + except Exception as e: + log_info(f"Error collecting audio data: {e}") + await cli.close()Likely invalid or redundant comment.
examples/websockets_chat.py (2)
82-95:⚠️ Potential issueAdd error handling in speech iterator.
The function assumes that
voices.itemsis non-empty and does not handle potential exceptions from API calls.Apply this diff to add error handling:
def wrap_coze_speech_to_iterator(coze: AsyncCoze, text: str): async def iterator(): - voices = await coze.audio.voices.list(**kwargs) - content = await coze.audio.speech.create( - input=text, - voice_id=voices.items[0].voice_id, - response_format=AudioFormat.WAV, - sample_rate=24000, - **kwargs, - ) - for data in content._raw_response.iter_bytes(chunk_size=1024): - yield data + try: + voices = await coze.audio.voices.list(**kwargs) + if not voices.items: + raise ValueError("No voices available") + + content = await coze.audio.speech.create( + input=text, + voice_id=voices.items[0].voice_id, + response_format=AudioFormat.WAV, + sample_rate=24000, + **kwargs, + ) + + for data in content._raw_response.iter_bytes(chunk_size=1024): + if not data: + continue + yield data + except Exception as e: + log_info(f"Error in speech iterator: {e}") + raise return iteratorLikely invalid or redundant comment.
98-103: 🛠️ Refactor suggestionValidate environment variables and externalize text.
The code has a few areas for improvement:
- Environment variables are used without validation
- The text is hardcoded
Apply this diff to improve the code:
+def validate_env_vars(): + """Validate required environment variables.""" + required_vars = { + "COZE_API_TOKEN": get_coze_api_token(), + "COZE_API_BASE": get_coze_api_base(), + "COZE_BOT_ID": os.getenv("COZE_BOT_ID"), + } + missing = [k for k, v in required_vars.items() if not v] + if missing: + raise ValueError(f"Missing required environment variables: {', '.join(missing)}") + return required_vars + async def main(): - coze_api_token = get_coze_api_token() - coze_api_base = get_coze_api_base() - bot_id = os.getenv("COZE_BOT_ID") - text = os.getenv("COZE_TEXT") or "你今天好吗? 今天天气不错呀" + env_vars = validate_env_vars() + text = os.getenv("COZE_TEXT", "你今天好吗? 今天天气不错呀")Likely invalid or redundant comment.
cozepy/websockets/audio/speech/__init__.py (2)
59-61:⚠️ Potential issueCorrect base64 encoding to return a string.
The
base64.b64encode()function returns bytes, which should be decoded to UTF-8 for proper string serialization.Apply this diff to fix the issue:
@field_serializer("delta") def serialize_delta(self, delta: bytes, _info): - return base64.b64encode(delta) + return base64.b64encode(delta).decode('utf-8')Likely invalid or redundant comment.
138-148:⚠️ Potential issueEnhance security in base64 decoding.
The base64 decoding operation needs proper validation and size limits.
Apply this diff to improve security:
delta_base64 = data.get("delta") if delta_base64 is None: raise ValueError("Missing 'delta' in event data") + if not isinstance(delta_base64, str): + raise ValueError("Invalid delta type") + if len(delta_base64) > 1024 * 1024: # 1MB limit + raise ValueError("Delta exceeds size limit") + try: + delta = base64.b64decode(delta_base64) + except Exception as e: + raise ValueError(f"Invalid base64 data: {str(e)}") return SpeechAudioUpdateEvent.model_validate( { "id": event_id, "detail": detail, "data": SpeechAudioUpdateEvent.Data.model_validate( { - "delta": base64.b64decode(delta_base64), + "delta": delta, } ), } )Likely invalid or redundant comment.
cozepy/websockets/ws.py (3)
237-245: 🛠️ Refactor suggestionAdd timeout handling to send loop operations.
The send loop could block indefinitely on queue operations. Consider adding timeout handling to prevent this.
def _send_loop(self) -> None: try: while True: - event = self._input_queue.get() - self._send_event(event) - self._input_queue.task_done() + try: + event = self._input_queue.get(timeout=30) # 30 seconds timeout + self._send_event(event) + self._input_queue.task_done() + except queue.Empty: + continue except Exception as e: self._handle_error(e)Likely invalid or redundant comment.
422-424: 🛠️ Refactor suggestionEnhance error handling for InvalidStatus.
The error handling for
InvalidStatuscould fail ife.responseore.response.headersisNone. Add null checks to prevent this.except InvalidStatus as e: - raise CozeAPIError(None, f"{e}", e.response.headers.get("x-tt-logid")) from e + log_id = e.response.headers.get("x-tt-logid") if e.response and e.response.headers else None + raise CozeAPIError(None, f"{e}", log_id) from eLikely invalid or redundant comment.
246-267: 🛠️ Refactor suggestionAdd timeout handling to receive loop operations.
The receive loop could block indefinitely on WebSocket receive operations. Consider adding timeout handling.
def _receive_loop(self) -> None: try: while True: if not self._ws: log_debug("[%s] empty websocket conn, close", self._path) break - data = self._ws.recv() + try: + data = self._ws.recv(timeout=30) # 30 seconds timeout + except websockets.exceptions.TimeoutError: + continue message = json.loads(data)Likely invalid or redundant comment.
cozepy/websockets/chat/__init__.py (1)
178-186: 🛠️ Refactor suggestionAdd timeout handling to queue operations.
The queue operations in
chat_update,conversation_chat_submit_tool_outputs, and other methods should include timeout handling to prevent blocking.def chat_update(self, data: ChatUpdateEvent.Data) -> None: - self._input_queue.put(ChatUpdateEvent.model_validate({"data": data})) + try: + self._input_queue.put( + ChatUpdateEvent.model_validate({"data": data}), + timeout=30 # 30 seconds timeout + ) + except queue.Full: + raise TimeoutError("Timeout while updating chat")Likely invalid or redundant comment.
cozepy/__init__.py (5)
94-105: Well-structured WebSocket audio speech imports!The imports are logically organized with clear naming conventions, following a consistent pattern of clients, event handlers, and event types.
106-117: Consistent structure in WebSocket audio transcriptions imports!The imports maintain consistency with the speech module structure, providing a clear separation of concerns between speech and transcription functionalities.
118-130: Comprehensive WebSocket chat event structure!The chat events cover various scenarios (creation, updates, completion, tool outputs) with clear and descriptive naming.
131-139: Well-organized core WebSocket types and configurations!The base WebSocket types and audio configurations are appropriately placed in the ws module, providing essential building blocks for the WebSocket implementation.
Line range hint
94-270: Verify completeness of WebSocket public interfaceThe WebSocket implementation looks comprehensive. Let's verify that all imported entities are properly exposed in all.
✅ Verification successful
WebSocket interface is properly exposed
All WebSocket-related entities are correctly included in the
__all__list. The implementation maintains a complete and consistent public interface.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all imported WebSocket entities are exposed in __all__ # Extract imported names from websockets.* imports echo "Checking for unexposed WebSocket entities..." rg "^from \.websockets\." cozepy/__init__.py -A 20 | grep -E ' \w+,' | sed 's/ //g' | sed 's/,//g' > /tmp/imports.txt # Extract WebSocket-related names from __all__ rg "^ # websockets" cozepy/__init__.py -A 20 | grep -E ' "\w+",' | sed 's/ //g' | sed 's/,//g' | sed 's/"//g' > /tmp/exports.txt # Compare the files diff /tmp/imports.txt /tmp/exports.txtLength of output: 2286
Summary by CodeRabbit
Release Notes
New Features
Improvements
Dependencies
websocketslibrary with version-specific compatibility.Bug Fixes