Conversation
hanzeINGH
commented
Mar 3, 2025
hanzeINGH
commented
- support normal transcriptions api
- support coze websocket api
Change-Id: I12ae9f032638b32264d5dbf4ac13578922fffb48
Change-Id: Ic2427cb8f1ff9d20ad2f31c7e50f0aad1e812e5e
Change-Id: Iacf33262e6d5fb0e0537a834b3726686be854e08
Change-Id: I09ffad819a8963805cdd44c806dd8619ee323c2f
Change-Id: I0b7bd881bae19cab4fbae6f8906757fe06fe0999
Change-Id: I8edd9ead4048ffc9537da965305356cdd197b47f
WalkthroughThe pull request makes extensive updates across the project. In CI configurations and dependency files, Java version support, dependency versions, and exclusion rules are adjusted. Numerous API client classes have new interfaces and data models for audio transcription functionality and have undergone annotation changes (e.g., replacing @NotNull with @nonnull). The authentication and OAuth methods are simplified, while a host of new WebSocket client classes, builders, callback handlers, and event models are added to support real-time chat, audio speech, and transcription features. Multiple test and example files have also been introduced or updated. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant TS as TranscriptionService
participant ATA as AudioTranscriptionAPI
participant S as Server
C->>TS: create(CreateTranscriptionsReq)
TS->>ATA: uploadFile(file or bytes)
ATA-->>TS: CreateTranscriptionsResp
TS-->>C: Return transcription response
sequenceDiagram
participant WSClient as WebSocket Client
participant Handler as Callback Handler
participant Server as WebSocket Server
WSClient->>Server: Send event (e.g., speech update)
Server-->>WSClient: Acknowledge / Send event JSON
WSClient->>Handler: handleEvent(json payload)
Handler-->>WSClient: Process and trigger callback
Poem
✨ Finishing Touches
🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketChatClient.java (1)
33-35:⚠️ Potential issueMethod signature mismatch with caller and missing URL encoding.
The method signature doesn't match how it's called in the constructor (line 29), and it doesn't handle URL encoding of parameters, which could cause issues if the bot ID contains special characters.
-protected static String buildUrl(String wsHost, WebsocketChatCreateReq req) { +protected static String buildUrl(String wsHost, WebsocketChatCreateReq req) { - return String.format("%s%s?bot_id=%s", wsHost, uri, req.getBotID()); + try { + return String.format("%s%s?bot_id=%s", wsHost, uri, + java.net.URLEncoder.encode(req.getBotID(), "UTF-8")); + } catch (java.io.UnsupportedEncodingException e) { + throw new RuntimeException("Failed to encode URL parameter", e); + } }
🧹 Nitpick comments (14)
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java (5)
31-33: Consider adding parameter validation.The
speechUpdatemethod should validate that thedataparameter is not null before sending the event.public void speechUpdate(SpeechUpdateEventData data) { + if (data == null) { + throw new IllegalArgumentException("SpeechUpdateEventData cannot be null"); + } this.sendEvent(SpeechUpdateEvent.builder().data(data).build()); }
36-38: Add null check for the data parameter.Similar to the
speechUpdatemethod, you should validate that thedataparameter is not null.public void inputTextBufferAppend(String data) { + if (data == null) { + throw new IllegalArgumentException("Buffer text cannot be null"); + } this.sendEvent(InputTextBufferAppendEvent.of(data)); }
56-57: Replace Chinese comment with English for international accessibility.The comment is in Chinese. For better collaboration in an international context, translate the comment to English.
- // 这里在上面已经抛出异常了，直接 return 即可 + // Exception has already been thrown above, just return
51-95: Extract event handling logic into separate methods.The
handleEventmethod is quite long and handles multiple event types. Consider extracting the logic for each event type into separate methods to improve readability and maintainability.Here's how you might refactor the switch case:
protected void handleEvent(WebSocket ws, String text) { try { JsonNode jsonNode = objectMapper.readTree(text); String eventType = parseEventType(jsonNode, text); if (eventType == null) { // Exception has already been thrown above, just return return; } switch (eventType) { case EventType.SPEECH_CREATED: - SpeechCreatedEvent speechCreatedEvent = - objectMapper.treeToValue(jsonNode, SpeechCreatedEvent.class); - handler.onSpeechCreated(this, speechCreatedEvent); + handleSpeechCreatedEvent(jsonNode); break; case EventType.SPEECH_UPDATED: - SpeechUpdatedEvent speechUpdatedEvent = - objectMapper.treeToValue(jsonNode, SpeechUpdatedEvent.class); - handler.onSpeechUpdated(this, speechUpdatedEvent); + handleSpeechUpdatedEvent(jsonNode); break; // ... other cases similarly refactored default: logger.error("unknown event type: {}, event string: {}", eventType, text); } } catch (Exception e) { handler.onClientException(this, new RuntimeException(e)); } } private void handleSpeechCreatedEvent(JsonNode jsonNode) throws Exception { SpeechCreatedEvent speechCreatedEvent = objectMapper.treeToValue(jsonNode, SpeechCreatedEvent.class); handler.onSpeechCreated(this, speechCreatedEvent); } private void handleSpeechUpdatedEvent(JsonNode jsonNode) throws Exception { SpeechUpdatedEvent speechUpdatedEvent = objectMapper.treeToValue(jsonNode, SpeechUpdatedEvent.class); handler.onSpeechUpdated(this, speechUpdatedEvent); } // ... other handler methods
92-94: Log exception details before passing to handler.Currently, exceptions are caught and passed directly to the handler without logging. Consider logging the exception details to aid in debugging.
} catch (Exception e) { + logger.error("Exception while handling WebSocket event", e); handler.onClientException(this, new RuntimeException(e)); }api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketAudioTranscriptionsClient.java (8)
28-31: Consider translating non-English comments.There are several comments in Chinese throughout the code. Consider translating these to English for improved readability and consistency across the codebase.
For this specific comment: "发送转录配置更新事件" means "Send transcription configuration update event".
-// 发送转录配置更新事件 +// Send transcription configuration update event
33-36: Consider translating non-English comments.This comment "发送语音缓冲区追加事件" means "Send audio buffer append event".
-// 发送语音缓冲区追加事件 +// Send audio buffer append event
38-40: Consider adding a comment for consistency.For consistency with the other methods, consider adding a comment for this overloaded version of
inputAudioBufferAppend.+// Send audio buffer append event with structured data public void inputAudioBufferAppend(InputAudioBufferAppendEvent.Data data) { this.sendEvent(InputAudioBufferAppendEvent.builder().data(data).build()); }
42-45: Consider translating non-English comments.This comment "发送语音缓冲区清除事件" means "Send audio buffer clear event".
-// 发送语音缓冲区清除事件 +// Send audio buffer clear event
47-50: Consider translating non-English comments.This comment "发送语音缓冲区完成事件" means "Send audio buffer complete event".
-// 发送语音缓冲区完成事件 +// Send audio buffer complete event
62-65: Previous null check implementation looks good.The null check for eventType before using it in the switch statement has been implemented correctly, addressing the previous review comment. This prevents potential NullPointerExceptions.
However, the comment "这里在上面已经抛出异常了，直接 return 即可" (which means "an exception has already been thrown above, just return directly") seems confusing since no exception is thrown for null eventType in the visible code.
if (eventType == null) { - // 这里在上面已经抛出异常了，直接 return 即可 + // Return early if no event type was found return; }
101-103: Consider notifying the callback handler for unknown event types.Currently, unknown event types are only logged but not reported to the callback handler. This could lead to silent failures where the client code isn't aware of unhandled event types.
default: logger.error("unknown event type: {}, event string: {}", eventType, text); + handler.onClientException(this, new RuntimeException("Unknown event type: " + eventType));
104-106: Consider preserving original exception details.When catching and forwarding exceptions, wrapping them in a RuntimeException loses some context from the original exception. Consider adding a descriptive message to provide more context.
} catch (Exception e) { - handler.onClientException(this, new RuntimeException(e)); + handler.onClientException(this, new RuntimeException("Error handling WebSocket event: " + e.getMessage(), e)); }api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketChatClient.java (1)
81-86: Replace non-English comments with English for consistency.The code contains comments in Chinese. To maintain consistency and improve readability for a broader audience, translate these comments to English.
- // 解析 JSON + // Parse JSON JsonNode jsonNode = objectMapper.readTree(text); String eventType = parseEventType(jsonNode, text); if (eventType == null) { - // 这里在上面已经抛出异常了，直接 return 即可 + // Exception already thrown above, just return return; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketAudioTranscriptionsClient.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketChatClient.java(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java
[error] 6-6: Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.22.8:check (default-cli) on project coze-api: The following files had format violations. Run 'mvn spotless:apply' to fix these violations.
🔇 Additional comments (9)
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java (3)
16-24: Class structure looks good with appropriate inheritance.The class correctly extends
BaseWebsocketClientand has a well-defined constructor that passes required parameters to the parent class. The pattern for handling WebSocket connections is consistent with object-oriented design principles.
53-58:Details
✅ Verification successful
Verify the implementation of parseEventType method.
The code refers to a
parseEventTypemethod which is not defined in this class. Ensure this method is properly implemented in a parent class or add it to this class if it's missing.
🏁 Script executed:
#!/bin/bash # Check if parseEventType is defined in the parent class echo "Checking BaseWebsocketClient for parseEventType method..." BASEWS_PATH=$(find . -name "BaseWebsocketClient.java" | head -1) if [ -n "$BASEWS_PATH" ]; then cat "$BASEWS_PATH" | grep -n "parseEventType" else echo "BaseWebsocketClient.java not found" fiLength of output: 590
Implementation Confirmed –
parseEventTypeis properly implementedIt has been verified that the
parseEventTypemethod is defined in the parent classBaseWebsocketClient.java(line 64). No modifications are needed in the current file.
1-96:Details
❓ Verification inconclusive
Fix Spotless formatting issues.
The pipeline failures indicate that there are Spotless formatting violations. Run
mvn spotless:applyto fix these automatically.
🏁 Script executed:
#!/bin/bash # Check what formatting issues are present echo "Checking for formatting issues in the file..." mvn spotless:check -DspotlessFiles=api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java echo "Note: Run 'mvn spotless:apply' to fix these violations automatically."Length of output: 492
Please fix Spotless formatting issues.
The pipeline clearly indicates formatting violations as enforced by Spotless. Although our verification script couldn’t run the Maven command (mvn was not found in the sandbox), the original pipeline errors suggest that the file is not formatted according to project standards. Please runmvn spotless:applylocally to auto-correct the violations and commit the changes.🧰 Tools
🪛 GitHub Actions: CI
[error] 6-6: Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.22.8:check (default-cli) on project coze-api: The following files had format violations. Run 'mvn spotless:apply' to fix these violations.
api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketAudioTranscriptionsClient.java (3)
14-16: Implementation follows inheritance pattern effectively.The class correctly extends
BaseWebsocketClientand properly declares the callback handler field with the final modifier to ensure immutability after initialization. The static URI constant is well-defined.
97-101: Error event handling is appropriate.The code properly deserializes error events and forwards them to the handler's onError method.
18-22:Details
✅ Verification successful
Constructor design is appropriate but consider visibility.
The constructor is currently marked as
protected, suggesting this class is intended to be instantiated through a factory method or another class. Ensure this aligns with your intended usage pattern.
🏁 Script executed:
#!/bin/bash # Find references to WebsocketAudioTranscriptionsClient instantiation rg -A 3 "new WebsocketAudioTranscriptionsClient" --type java # Look for potential factory methods or builders rg -A 3 "WebsocketAudioTranscriptionsClient.*create|build|getInstance" --type javaLength of output: 118119
Constructor Visibility Verified and Approved
The protected constructor inWebsocketAudioTranscriptionsClientis being used intentionally. Our review of the codebase shows that the class is instantiated via the dedicated builder inWebsocketAudioTranscriptionsBuilder.javaand within tests (in the same package) where direct instantiation is permissible. This aligns with the intended design pattern. Please ensure that instantiation externally is meant to occur only through the builder or appropriate subclassing.
- Instantiation through the builder is confirmed.
- Test code (in the same package) directly accesses the constructor, which is acceptable given its protected modifier.
api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketChatClient.java (3)
78-184: Refactor large switch statement for better maintainability.The big switch block handling multiple event types is correct but rather lengthy. Consider using a more modular approach, such as a map of event types to handlers, to improve readability and reduce maintenance overhead.
@Override protected void handleEvent(WebSocket ws, String text) { try { // Parse JSON JsonNode jsonNode = objectMapper.readTree(text); String eventType = parseEventType(jsonNode, text); if (eventType == null) { // Already thrown exception above, just return return; } + EventHandler eventHandler = EVENT_HANDLERS.get(eventType); + if (eventHandler != null) { + eventHandler.handle(this, jsonNode, objectMapper, handler); + } else { + logger.error("unknown event type: {}, event string: {}", eventType, text); + } - switch (eventType) { - case EventType.CHAT_CREATED: - ChatCreatedEvent chatCreatedEvent = - objectMapper.treeToValue(jsonNode, ChatCreatedEvent.class); - handler.onChatCreated(this, chatCreatedEvent); - break; - // ... remaining cases - default: - logger.error("unknown event type: {}, event string: {}", eventType, text); - } } catch (Exception e) { handler.onClientException(this, new RuntimeException(e)); } } + // Define an interface for event handlers + private interface EventHandler { + void handle(WebsocketChatClient client, JsonNode jsonNode, ObjectMapper mapper, + WebsocketChatCallbackHandler handler) throws Exception; + } + + // Create a static map of event types to handlers + private static final Map<String, EventHandler> EVENT_HANDLERS = new HashMap<>(); + + static { + EVENT_HANDLERS.put(EventType.CHAT_CREATED, (client, jsonNode, mapper, handler) -> { + ChatCreatedEvent event = mapper.treeToValue(jsonNode, ChatCreatedEvent.class); + handler.onChatCreated(client, event); + }); + + // Add other event handlers similarly... + }
22-23:Details
❓ Verification inconclusive
Verify thread safety of the ObjectMapper initialization.
Ensure that
Utils.getMapper()returns a thread-safe instance of ObjectMapper, as the WebsocketChatClient will likely be used in a multi-threaded environment.
🏁 Script executed:
#!/bin/bash # Check how Utils.getMapper() is implemented rg -A 10 "getMapper" --include="Utils.java"Length of output: 483
Verify Thread Safety of ObjectMapper Initialization
The current verification command did not yield useful output due to an incorrect flag usage. Please manually confirm that
Utils.getMapper()inUtils.javareturns a thread-safe ObjectMapper instance. Specifically:
- Re-run the search using a corrected command (e.g.,
rg -A 10 "getMapper" -g "Utils.java") to locate its implementation.- Verify that the ObjectMapper instance is configured or shared in a way that is safe for concurrent use in a multi-threaded environment, as required by
WebsocketChatClient.
28-31:Details
❓ Verification inconclusive
Constructor needs proper error handling for WebSocket connection failures.
The constructor initializes the WebSocket connection but doesn't provide any handling for potential connection failures. Consider adding connection error handling either here or in the parent class.
🏁 Script executed:
#!/bin/bash # Check for connection error handling in BaseWebsocketClient rg -A 5 "BaseWebsocketClient.*constructor" --include="*.java"Length of output: 505
Action Required: Enhance Error Handling for WebSocket Connection Failures
It appears that the
WebsocketChatClientconstructor currently lacks explicit error handling for potential WebSocket connection failures. Our initial attempt to locate any error handling logic in the parent class (BaseWebsocketClient) was inconclusive (the original ripgrep command failed due to incompatible flags). Please re-run the following script to verify whether the parent class implements connection error management, and if not, update the constructor (or the parent class) to handle errors (for example, by using try–catch blocks) and ensure exceptions are both managed and logged:#!/bin/bash rg -A 5 "BaseWebsocketClient.*constructor" -g "*.java"
- Confirm whether
BaseWebsocketClientprovides error handling during WebSocket initialization.- If error handling is absent or insufficient, add proper try–catch logic in the constructor (or refactor the parent class) to gracefully manage connection failures.
- Ensure that any exceptions encountered during connection establishment are logged to aid debugging.
api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketChatClient.java
Show resolved
Hide resolved
Change-Id: I315a3b35a1730f8efe28924db584e8f68762ec82
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java (3)
28-41: Consider translating the Chinese comments for better internationalization.The comments above methods are in Chinese which may hinder collaboration with developers who don't understand the language. For an open-source project, English comments would be more universally accessible.
- // 发送语音配置更新事件 + // Send speech configuration update event public void speechUpdate(SpeechUpdateEventData data) { this.sendEvent(SpeechUpdateEvent.builder().data(data).build()); } - // 发送文本缓冲区追加事件 + // Send text buffer append event public void inputTextBufferAppend(String data) { this.sendEvent(InputTextBufferAppendEvent.of(data)); } - // 发送文本缓冲区完成事件 + // Send text buffer complete event public void inputTextBufferComplete() { this.sendEvent(new InputTextBufferCompleteEvent()); }
49-56: Translate the Chinese comment and improve error handling clarity.The comment on line 54 is in Chinese and suggests that an exception has already been thrown. However, the code flow doesn't explicitly show this. Consider clarifying the error handling flow and translating the comment for better understandability.
protected void handleEvent(WebSocket ws, String text) { try { JsonNode jsonNode = objectMapper.readTree(text); String eventType = parseEventType(jsonNode, text); if (eventType == null) { - // 这里在上面已经抛出异常了，直接 return 即可 + // The parseEventType method has already handled the error, just return return; }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 55-55: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java#L55
Added line #L55 was not covered by tests
57-89: Consider using a map-based dispatch for event handling.The current switch statement is functional but could be refactored to use a map of event types to handler functions for better maintainability, especially if more event types are added in the future.
You could refactor this to use a more maintainable approach:
// Define a functional interface for event handlers private interface EventHandler<T> { void handle(T event) throws Exception; } // Create a map of event types to handlers private final Map<String, EventHandler<?>> eventHandlers = Map.of( EventType.SPEECH_CREATED, (EventHandler<SpeechCreatedEvent>) event -> handler.onSpeechCreated(this, event), EventType.SPEECH_UPDATED, (EventHandler<SpeechUpdatedEvent>) event -> handler.onSpeechUpdated(this, event), // ... other event types ); // Then in handleEvent: EventHandler<?> eventHandler = eventHandlers.get(eventType); if (eventHandler != null) { Object event = objectMapper.treeToValue(jsonNode, eventHandler.getClass().getGenericInterfaces()[0].getActualTypeArguments()[0]); eventHandler.handle(event); } else { logger.error("unknown event type: {}, event string: {}", eventType, text); }This approach would reduce the amount of repetitive code and make adding new event types easier.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java
[warning] 45-45: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java#L45
Added line #L45 was not covered by tests
[warning] 55-55: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java#L55
Added line #L55 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (Java 17 on Windows)
- GitHub Check: test (Java 11 on Windows)
🔇 Additional comments (4)
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java (4)
1-14: Well-structured class organization for WebSocket audio speech client.The class extends BaseWebsocketClient and provides dedicated WebSocket communication for audio speech events. The imports are correctly organized and include all necessary dependencies.
15-22: Constructor and field setup looks good.The constructor properly initializes the client by passing parameters to the parent class and storing the callback handler. The URI constant is appropriately defined as a private static final field.
90-93: Good exception handling with callback propagation.The exception handling correctly catches any exceptions during event processing and propagates them to the callback handler, which is a good practice for WebSocket clients.
52-52:Details
✅ Verification successful
Verify the implementation of parseEventType method.
The code calls
parseEventType(jsonNode, text), but this method is not defined in this class. Ensure this method exists in the parent class or a utility class and handles null checking for the "event_type" field as mentioned in previous reviews.
🏁 Script executed:
#!/bin/bash # Search for the parseEventType method implementation in the codebase echo "Searching for parseEventType method in the codebase..." rg -A 15 "protected\s+String\s+parseEventType" --type java # Check if BaseWebsocketClient contains this method echo "Checking BaseWebsocketClient for parseEventType..." rg -A 15 "class BaseWebsocketClient" --type java | rg -A 30 "parseEventType" --context 3Length of output: 2385
Confirmed: The parent class implementation of
parseEventTypeis correctly defined and includes proper null checking. The method is found inapi/src/main/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketClient.javaand verifies that if the"event_type"field is missing, it logs an error, invokes the callback, and returnsnull. No changes are needed in theWebsocketAudioSpeechClientusage of this method.
...java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClient.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
api/src/test/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketClientTest.java (6)
30-31: Translate comment to English for consistency.The comment on line 30 is in Chinese. For better maintainability and consistency, it's recommended to keep all code comments in English.
- // 创建一个测试用的具体实现类 + // Create a concrete implementation class for testing
66-76: Enhance constructor test with more meaningful assertions.The current test only verifies that the client isn't null, which is a minimal check. Consider adding assertions that verify the client is properly configured with the expected timeout values.
@Test void testConstructor() { // 验证构造函数是否正确初始化了 WebSocket TestBaseWebsocketClient client = new TestBaseWebsocketClient( new OkHttpClient(), "ws://test.com", mockCallbackHandler, BaseReq.builder().writeTimeout(1).connectTimeout(2).readTimeout(3).build()); assertNotNull(client); + // Verify that timeout values are correctly passed to the client + // This would require exposing getters or additional test methods to verify these values }
83-93: Improve the event sending test to verify message content.The current test verifies that
send()is called but doesn't check the actual content of the message. Consider capturing the message content and verifying it properly represents the event.@Test void testSendEvent() { // 创建测试事件 BaseEvent testEvent = new BaseEvent(); + // Set some properties on the test event to make verification meaningful + // testEvent.setProperty("value"); // 发送事件 client.testSendEvent(testEvent); // 验证 WebSocket.send() 被调用 - verify(mockWebSocket).send(anyString()); + // Capture the actual string sent + ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass(String.class); + verify(mockWebSocket).send(messageCaptor.capture()); + String sentMessage = messageCaptor.getValue(); + + // Verify the message contains expected content based on the event + assertNotNull(sentMessage); + // Additional assertions based on the expected JSON structure }
95-108: Replace Thread.sleep with a more deterministic approach.Using
Thread.sleep()in tests makes them brittle and potentially slow. Consider using a more deterministic approach for testing asynchronous behavior, such as CountDownLatch, CompletableFuture, or Awaitility.@Test void testClose() throws InterruptedException { // 执行关闭 client.close(); // 验证 WebSocket 关闭被调用 verify(mockWebSocket).close(eq(1000), eq("Normal closure")); - // 等待一小段时间确保关闭操作完成 - TimeUnit.MILLISECONDS.sleep(100); + // Instead of sleeping, consider using a CountDownLatch or other mechanism + // to synchronize with the completion of the close operation if necessary // 再次调用关闭，确保不会出错 client.close(); }
146-163: Consider alternative to reflection for testing private fields.Using reflection to access private fields breaks encapsulation and can make tests brittle. Consider adding protected getters in the base class for testing purposes or using other design approaches that avoid reflection.
@Test void testListenerShutdown() { - // 使用反射设置模拟的 listener - try { - java.lang.reflect.Field listenerField = - BaseWebsocketClient.class.getDeclaredField("listener"); - listenerField.setAccessible(true); - listenerField.set(client, mockListener); - } catch (Exception e) { - throw new RuntimeException(e); - } + // Alternative approach: Add a protected method in BaseWebsocketClient + // like protected void setListener(BaseWebSocketListener listener) for testing + // Then call: ((TestBaseWebsocketClient)client).setListener(mockListener); // 关闭客户端 client.close(); // 验证 listener 的 shutdown 被调用 verify(mockListener).shutdown(); }
165-173: Enhance the handleEvent delegation test to verify behavior.The current test only ensures no exceptions are thrown but doesn't verify any actual behavior. Consider improving this test to validate that the handleEvent method performs its expected actions.
@Test void testHandleEventDelegation() { String testMessage = "test message"; + // Create a spy of the client to verify method interactions + TestBaseWebsocketClient clientSpy = spy(client); + // 调用 handleEvent - client.handleEvent(mockWebSocket, testMessage); + clientSpy.handleEvent(mockWebSocket, testMessage); // 由于是空实现，这里主要验证调用不会抛出异常 + // If possible, verify some expected behavior or interactions + // For example, if the method should call other methods or handle the message in some way: + // verify(clientSpy).someMethodThatShouldBeCalled(testMessage); + // verify(mockCallbackHandler).someExpectedInteraction(); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/test/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketClientTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (Java 17 on Windows)
- GitHub Check: test (Java 11 on Windows)
🔇 Additional comments (1)
api/src/test/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketClientTest.java (1)
1-174: LGTM - Well-structured and comprehensive test class.This test class is well-structured with good test coverage for different scenarios like initialization, event sending, closing behavior, and resource cleanup. The use of a concrete test subclass to test the abstract base class is a good approach.
Change-Id: I0d3c4d6c138639b705a50319aa253b324a379ff2
Change-Id: I04e30f6f92f17df59a9d8ef880d4129b5fb1ddea
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (37)
example/src/main/java/example/auth/JWTOAuthExample.java (1)
39-39: Remove commented-out debugging code.The commented-out line that concatenates "123" to the public key ID appears to be leftover debugging code. This would break authentication if uncommented and could confuse users following this example.
- // jwtOauthPublicKeyID+="123";README.md (1)
990-1107: Comprehensive WebSocket documentation with excellent examples.The added WebSocket section provides thorough explanation and examples for all three main WebSocket features: chat, speech synthesis, and transcription. The code examples demonstrate the key functionality including creating clients, handling events via callbacks, and proper resource cleanup.
Consider formatting the bare URL at line 995 as a proper Markdown link:
-https://www.coze.cn/open/docs/guides/websocket_openapi +[https://www.coze.cn/open/docs/guides/websocket_openapi](https://www.coze.cn/open/docs/guides/websocket_openapi)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
995-995: Bare URL used
null(MD034, no-bare-urls)
api/src/main/java/com/coze/openapi/service/auth/Auth.java (1)
7-7: Document the purpose of the new refreshAt fieldThe addition of a protected
refreshAtfield suggests it's used to track token refresh timing, but it lacks documentation explaining its purpose, expected values, and how subclasses should use it.Add documentation for the new field:
protected String accessToken; protected String refreshToken; + /** + * Timestamp (in milliseconds) indicating when the access token should be refreshed. + * Subclasses should use this field to determine when to refresh the token. + */ protected long refreshAt; protected long expiresIn; protected OAuthClient client;example/src/main/java/example/websocket/audio/transcriptions/WebsocketTranscriptionsExample.java (6)
27-30: Update the example description commentThe header comment appears to be copied from a chat example and doesn't accurately describe this audio transcription example.
Replace with an appropriate description:
/* -This example describes how to use the chat interface to initiate conversations, -poll the status of the conversation, and obtain the messages after the conversation is completed. +This example demonstrates how to use the WebSocket transcription API to transcribe audio data, +process transcription events, and handle the results through callback methods. * */
32-39: Remove unused Weather classThe
Weatherclass is defined with Lombok annotations but is never used in the example.Remove the unused class to improve code clarity:
- @Data - @Builder - @AllArgsConstructor - @NoArgsConstructor - private static class Weather { - @JsonProperty("weather") - private String weather; - }
41-46: Improve buffer documentation and usageThe
CallbackHandlerallocates a 10MB buffer with a Chinese comment, but the buffer is never used in the code.Either use the buffer or remove it, and translate the comment:
private static class CallbackHandler extends WebsocketsAudioTranscriptionsCallbackHandler { - private final ByteBuffer buffer = ByteBuffer.allocate(1024 * 1024 * 10); // 分配 10MB 缓冲区 + // If the buffer is needed: + private final ByteBuffer buffer = ByteBuffer.allocate(1024 * 1024 * 10); // Allocate 10MB buffer + // Or if not needed, remove it entirely public CallbackHandler() { super(); }
98-100: Remove irrelevant comment about chat APIThe comment about non-streaming chat API doesn't relate to this WebSocket transcription example.
Remove the irrelevant comment:
- // For non-streaming chat API, it is necessary to create a chat first and then poll the chat - // results. public static void main(String[] args) throws Exception {
102-104: Add validation for environment variablesThe example reads environment variables without checking if they exist, which could lead to NullPointerExceptions.
Add validation for environment variables:
- String token = System.getenv("COZE_API_TOKEN"); - String voiceID = System.getenv("COZE_VOICE_ID"); + String token = System.getenv("COZE_API_TOKEN"); + if (token == null || token.isEmpty()) { + System.err.println("COZE_API_TOKEN environment variable is not set"); + System.exit(1); + } + + String voiceID = System.getenv("COZE_VOICE_ID"); + if (voiceID == null || voiceID.isEmpty()) { + System.err.println("COZE_VOICE_ID environment variable is not set"); + System.exit(1); + }
148-148: Replace hardcoded sleep with a more appropriate waiting mechanismThe example uses a hardcoded sleep of 100 seconds, which is not a good practice for examples.
Consider using a more appropriate waiting mechanism:
- TimeUnit.SECONDS.sleep(100); + // Wait for a reasonable time or implement a proper termination condition + System.out.println("Waiting for transcription to complete (press Enter to exit)..."); + System.in.read();Or, if a countdown is preferred:
- TimeUnit.SECONDS.sleep(100); + // Wait for a reasonable time with a countdown + int waitSeconds = 30; + System.out.println("Waiting for transcription to complete for " + waitSeconds + " seconds..."); + for (int i = waitSeconds; i > 0; i--) { + if (i % 5 == 0) { + System.out.println(i + " seconds remaining..."); + } + TimeUnit.SECONDS.sleep(1); + }api/src/main/java/com/coze/openapi/service/service/websocket/audio/WebsocketsAudioClient.java (2)
8-17: Add validation and documentation to WebsocketsAudioClientThe class follows good design principles, but lacks parameter validation and documentation.
Add validation and JavaDoc:
+/** + * Client for WebSocket audio operations. + * Provides access to audio speech and transcription functionality over WebSocket. + */ public class WebsocketsAudioClient { private final WebsocketsAudioSpeechBuilder websocketAudioSpeechBuilder; private final WebsocketsAudioTranscriptionsBuilder websocketAudioTranscriptionsBuilder; + /** + * Creates a new WebsocketsAudioClient with the specified base URL and HTTP client. + * + * @param baseUrl The base URL for WebSocket connections + * @param httpClient The OkHttpClient for handling HTTP requests + * @throws NullPointerException if baseUrl or httpClient is null + */ public WebsocketsAudioClient(String baseUrl, OkHttpClient httpClient) { + Objects.requireNonNull(baseUrl, "baseUrl cannot be null"); + Objects.requireNonNull(httpClient, "httpClient cannot be null"); this.websocketAudioSpeechBuilder = new WebsocketsAudioSpeechBuilder(baseUrl, httpClient); this.websocketAudioTranscriptionsBuilder = new WebsocketsAudioTranscriptionsBuilder(baseUrl, httpClient); }
19-21: Add documentation for speech methodThe method lacks JavaDoc explaining its purpose and return value.
Add method documentation:
+ /** + * Returns the builder for WebSocket audio speech operations. + * + * @return the WebsocketsAudioSpeechBuilder instance + */ public WebsocketsAudioSpeechBuilder speech() { return websocketAudioSpeechBuilder; }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 20-20: api/src/main/java/com/coze/openapi/service/service/websocket/audio/WebsocketsAudioClient.java#L20
Added line #L20 was not covered by testsapi/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsClient.java (8)
17-20: Consider clarifying the class-level documentation.
Providing a short class-level Javadoc would help future developers quickly grasp the purpose and usage of this WebSocket client.
21-25: Validatereqparameter and ensure non-null constraints.
Though this constructor assignsreqto the local handler, there is no explicit check for potential null references. Ifreqis unexpectedly null, the code might throw a NullPointerException.public WebsocketsAudioTranscriptionsClient( OkHttpClient client, String wsHost, WebsocketsAudioTranscriptionsCreateReq req) { + if (req == null || req.getCallbackHandler() == null) { + throw new IllegalArgumentException("WebsocketsAudioTranscriptionsCreateReq and its handler cannot be null"); + } super(client, buildUrl(wsHost), req.getCallbackHandler(), req); this.handler = req.getCallbackHandler(); }
27-29: Use uppercase constants for static URIs.
By convention, static constants likeuriare often set in uppercase, e.g.URI, to emphasize their immutability.-private static final String uri = "/v1/audio/transcriptions"; +private static final String URI = "/v1/audio/transcriptions";
36-39: Enhance type-safety by reducing conversions from String to byte[].
UsingStringfor audio data might risk needing repeated conversions. If the ultimate format is byte[], consider using a single method that accepts byte[] to avoid repeated overhead.
41-44: Handle length limits or large audio chunks.
Appending large byte arrays can risk excessive memory usage if not managed. Ensure there’s a maximum buffer size or streaming approach in place to avoid potential OOM errors.
46-49: Add logging to indicate buffer clearing.
A debug log here can help observe when the audio buffer is cleared.public void inputAudioBufferClear() { + logger.debug("Clearing the input audio buffer..."); this.sendEvent(new InputAudioBufferClearEvent()); }
52-54: Add an optional confirmation or callback for buffer completion.
Upon completing the buffer, it might be helpful to log or provide a callback that confirms the operation’s success.
61-111: Consider structured error responses instead of throwing generic exceptions.
In the event of malformed inputs, constructing a dedicated error response or a typed exception could provide more clarity to clients.api/src/main/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsListener.java (3)
26-30: Avoid directSystem.out.printlnfor logging.
Use a proper logging framework to maintain consistent logging levels and to integrate logs with application-wide logging systems.@Override public void onOpen(WebSocket webSocket, okhttp3.Response response) { - System.out.println("onOpen"); + // logger.info("WebSocket connection opened"); }
41-44: Log a reason foronClosingevent.
Consider logging the reason or code to better trace connection closures.
46-50: Provide additional diagnostic on failure.
Consider logging additional info (like HTTP response code or the remote endpoint) for debugging.api/src/main/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsClient.java (4)
20-25: Document the rationale for single-threaded execution.
While this design can simplify concurrency, mention in a class Javadoc or comment that events are handled sequentially to avoid concurrency issues.
28-54: Enforce HTTPS/TLS usage if security is a concern.
Currently, there is no check whether the scheme is wss or ws. If security is crucial, ensure the required protocol is wss.
56-58: Add validation for null or invalid event data.
Ensure thateventis non-null to avoid accidentalNullPointerException.
64-73: Consider returning a custom enum instead of string forparseEventType.
Using an enum for known event types can help catch typos at compile time and reduce runtime errors.example/src/main/java/example/websocket/chat/ChatExample.java (2)
50-51: Consider making the buffer dynamically scalable or verifying usage.A fixed 10MB buffer could become insufficient (risking overflow) or waste memory if actual usage is significantly smaller. A more flexible strategy (e.g., using a dynamically expandable structure) would be advisable.
63-66: Fix minor spelling error in log statement.Replace "Revieve" with "Receive" for clarity:
- System.out.printf("Revieve: %s\n", event.getData().getContent()); + System.out.printf("Receive: %s\n", event.getData().getContent());api/src/test/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsListenerTest.java (2)
49-52: Consider asserting behavior intestOnOpen().Currently, this test only verifies that
onOpendoes not fail. You could add minimal assertions or use a log-capturing library (e.g.,slf4j-test) to confirm that a log message is printed, ensuring the method’s logic is exercised and tested more thoroughly.
55-66: Refactor repetitiveCountDownLatchlogic.You are using similar
CountDownLatchsetup anddoAnswerstubbing for verifying asynchronous callbacks across multiple test methods. Consider refactoring into a helper method or a reusable utility to comply with DRY principles. This will simplify the tests and make them easier to maintain.Also applies to: 75-86, 133-154
api/src/test/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsClientTest.java (3)
95-108: Consider concurrency implications when closing websockets.
Currently, thetestClose()method verifies that multipleclose()calls do not throw exceptions. However, if multiple threads attempt to close the websocket simultaneously, additional synchronization or concurrency checks may be required to avoid potential race conditions.
110-129: Beware of thread interruption side effects.
IntestCloseWithInterruption(), manually interrupting the current thread and then clearing the interrupt status could affect subsequent tests running on the same thread. Consider isolating this scenario or using a dedicated thread to avoid unexpected side effects.
165-174: Expand test coverage for event handling.
testHandleEventDelegation()only ensures that no exception is thrown. Consider verifying the actual behavior by asserting that the incoming event is processed or delegated to a mocked handler method, thus providing stricter validation of the event flow.api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClient.java (1)
36-38: Consider URI-encoding dynamic query parameters.
buildUrldirectly appends thebot_idto the query string. If thebot_idcan contain special characters, you may need to URI-encode it to avoid potential parsing issues.api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCallbackHandler.java (1)
13-13: Consider removing the empty constructorThe empty constructor doesn't add any functionality beyond what Java would automatically provide. Unless there's a specific reason to make it explicit, it could be removed to reduce boilerplate.
- public WebsocketsAudioTranscriptionsCallbackHandler() {}example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java (2)
39-39: Add buffer overflow protection for ByteBufferThe ByteBuffer is allocated with a fixed size (10MB), but there's no check to ensure it doesn't overflow when new data is appended. Consider adding a check or using a dynamically resizable buffer.
- private final ByteBuffer buffer = ByteBuffer.allocate(1024 * 1024 * 10); // 分配 10MB 缓冲区 + // Allocate 10MB buffer with overflow protection + private ByteBuffer buffer = ByteBuffer.allocate(1024 * 1024 * 10);And in the onSpeechAudioUpdate method:
@Override public void onSpeechAudioUpdate( WebsocketsAudioSpeechClient client, SpeechAudioUpdateEvent event) { - buffer.put(event.getDelta()); + byte[] delta = event.getDelta(); + // Ensure buffer has enough capacity + if (buffer.remaining() < delta.length) { + // Double buffer size + ByteBuffer newBuffer = ByteBuffer.allocate(buffer.capacity() * 2); + buffer.flip(); + newBuffer.put(buffer); + buffer = newBuffer; + } + buffer.put(delta); }
125-125: Replace fixed sleep with a more elegant waiting mechanismUsing a fixed sleep time of 100 seconds is not ideal for an example. Consider using a CountDownLatch or CompletableFuture to wait for the completion event.
+ // Create a latch to wait for completion + java.util.concurrent.CountDownLatch completionLatch = new java.util.concurrent.CountDownLatch(1); + + // Modify the CallbackHandler class to countdown the latch on completion + // public void onSpeechAudioCompleted(...) { + // try { + // ExampleUtils.writePcmToWavFile(...); + // completionLatch.countDown(); // Signal completion + // } catch (IOException e) { ... } + // } + - TimeUnit.SECONDS.sleep(100); + // Wait for completion with a reasonable timeout + boolean completed = completionLatch.await(30, TimeUnit.SECONDS); + if (!completed) { + System.out.println("Warning: Operation did not complete within the timeout period"); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
README.md(2 hunks)api/src/main/java/com/coze/openapi/service/auth/Auth.java(2 hunks)api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java(1 hunks)api/src/main/java/com/coze/openapi/service/auth/TokenAuth.java(0 hunks)api/src/main/java/com/coze/openapi/service/service/CozeAPI.java(9 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/WebsocketsClient.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/WebsocketsAudioClient.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechBuilder.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechClient.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCreateReq.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsBuilder.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCallbackHandler.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsClient.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCreateReq.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatCallbackHandler.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClient.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClientBuilder.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatCreateReq.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/common/BaseCallbackHandler.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsClient.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsListener.java(1 hunks)api/src/test/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClientTest.java(1 hunks)api/src/test/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketAudioTranscriptionsClientTest.java(1 hunks)api/src/test/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClientTest.java(1 hunks)api/src/test/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsClientTest.java(1 hunks)api/src/test/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsListenerTest.java(1 hunks)example/src/main/java/example/auth/JWTOAuthExample.java(2 hunks)example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java(1 hunks)example/src/main/java/example/websocket/audio/transcriptions/WebsocketTranscriptionsExample.java(1 hunks)example/src/main/java/example/websocket/chat/ChatExample.java(1 hunks)
💤 Files with no reviewable changes (1)
- api/src/main/java/com/coze/openapi/service/auth/TokenAuth.java
🚧 Files skipped from review as they are similar to previous changes (3)
- api/src/main/java/com/coze/openapi/service/service/websocket/common/BaseCallbackHandler.java
- api/src/test/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketAudioSpeechClientTest.java
- api/src/test/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketAudioTranscriptionsClientTest.java
🧰 Additional context used
🪛 GitHub Check: codecov/patch
api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsBuilder.java
[warning] 12-13: api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsBuilder.java#L12-L13
Added lines #L12 - L13 were not covered by tests
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechBuilder.java
[warning] 12-12: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechBuilder.java#L12
Added line #L12 was not covered by tests
api/src/main/java/com/coze/openapi/service/service/websocket/audio/WebsocketsAudioClient.java
[warning] 20-20: api/src/main/java/com/coze/openapi/service/service/websocket/audio/WebsocketsAudioClient.java#L20
Added line #L20 was not covered by tests
[warning] 24-24: api/src/main/java/com/coze/openapi/service/service/websocket/audio/WebsocketsAudioClient.java#L24
Added line #L24 was not covered by tests
api/src/main/java/com/coze/openapi/service/service/websocket/WebsocketsClient.java
[warning] 20-20: api/src/main/java/com/coze/openapi/service/service/websocket/WebsocketsClient.java#L20
Added line #L20 was not covered by tests
[warning] 24-24: api/src/main/java/com/coze/openapi/service/service/websocket/WebsocketsClient.java#L24
Added line #L24 was not covered by tests
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java
[warning] 12-12: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java#L12
Added line #L12 was not covered by tests
[warning] 15-15: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java#L15
Added line #L15 was not covered by tests
[warning] 18-18: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java#L18
Added line #L18 was not covered by tests
[warning] 22-22: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java#L22
Added line #L22 was not covered by tests
[warning] 26-26: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java#L26
Added line #L26 was not covered by tests
[warning] 30-30: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java#L30
Added line #L30 was not covered by tests
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechClient.java
[warning] 45-45: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechClient.java#L45
Added line #L45 was not covered by tests
[warning] 55-55: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechClient.java#L55
Added line #L55 was not covered by tests
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCreateReq.java
[warning] 10-10: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCreateReq.java#L10
Added line #L10 was not covered by tests
api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCallbackHandler.java
[warning] 17-17: api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCallbackHandler.java#L17
Added line #L17 was not covered by tests
[warning] 21-21: api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCallbackHandler.java#L21
Added line #L21 was not covered by tests
[warning] 25-25: api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCallbackHandler.java#L25
Added line #L25 was not covered by tests
api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java
[warning] 38-38: api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java#L38
Added line #L38 was not covered by tests
[warning] 44-44: api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java#L44
Added line #L44 was not covered by tests
[warning] 46-46: api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java#L46
Added line #L46 was not covered by tests
[warning] 48-48: api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java#L48
Added line #L48 was not covered by tests
[warning] 50-50: api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java#L50
Added line #L50 was not covered by tests
api/src/main/java/com/coze/openapi/service/service/CozeAPI.java
[warning] 117-117: api/src/main/java/com/coze/openapi/service/service/CozeAPI.java#L117
Added line #L117 was not covered by tests
🪛 markdownlint-cli2 (0.17.2)
README.md
995-995: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (Java 17 on Windows)
- GitHub Check: test (Java 11 on Windows)
🔇 Additional comments (25)
example/src/main/java/example/auth/JWTOAuthExample.java (1)
81-85: LGTM: Improved API construction with builder pattern.Good refactoring to use the builder pattern for CozeAPI and JWTOAuth construction. This approach increases code readability and flexibility compared to direct instantiation, following modern Java practices.
api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatCreateReq.java (1)
8-16: Clean implementation with proper validationThe
WebsocketsChatCreateReqclass is well-structured with appropriate Lombok annotations and proper field validation. The@NonNullannotations ensure that bothbotIDandcallbackHandlerare required fields, preventing potential null reference issues during WebSocket operations.The use of
@EqualsAndHashCode(callSuper = true)correctly includes parent class fields in equality checks, which is important when extendingBaseReq.api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java (2)
27-29: Good refactoring for improved readabilityExtracting the token refresh condition into a separate method improves code readability and maintainability. This refactoring makes the token refresh logic more explicit and easier to understand.
38-38:Details
❓ Verification inconclusive
Update to token refresh calculation
The refresh calculation now uses the dynamic
getRefreshBefore()method instead of a fixed value, which provides more appropriate refresh timing based on token TTL.
🏁 Script executed:
#!/bin/bash # Check if there are any unit tests for JWTOAuth refresh logic rg -l "JWTOAuth.*test|test.*JWTOAuth" --type javaLength of output: 51
Action: Verify and Strengthen Token Refresh Testing
The updated token refresh calculation using the dynamic
getRefreshBefore()method appears correct. However, our initial test file search did not locate any unit tests covering the JWTOAuth refresh logic. To ensure the robustness of this change, please verify manually whether tests exist for this functionality or add new tests if necessary.
- Location:
api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java(Line 38)- Updated Code:
this.refreshAt = this.expiresIn - getRefreshBefore();- Recommendation: Confirm that unit tests cover this new logic, or add tests to validate the dynamic refresh behavior.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 38-38: api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.java#L38
Added line #L38 was not covered by testsapi/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCreateReq.java (1)
8-15:Details
✅ Verification successful
Well-structured WebSocket request class
The implementation follows the same pattern as other WebSocket request classes, with proper extension of
BaseReqand appropriate Lombok annotations. The@NonNullannotation oncallbackHandlerensures that callback handling is properly set up before WebSocket operations begin.
🏁 Script executed:
#!/bin/bash # Check for similar WebSocket request classes to verify consistency rg "WebsocketsAudio.*CreateReq|Websockets.*CreateReq" --type javaLength of output: 3939
Consistent Implementation for WebSocket Request Class
The
WebsocketsAudioSpeechCreateReqclass is implemented consistently with our other WebSocket request classes. The use of Lombok annotations (such as@Data,@SuperBuilder,@NoArgsConstructor,@AllArgsConstructor, and@EqualsAndHashCode(callSuper = true)) alongside the@NonNullannotation oncallbackHandlerensures that callback handling is set up properly before any WebSocket operations. Verification shows that similar patterns are used in classes likeWebsocketsChatCreateReqandWebsocketsAudioTranscriptionsCreateReq.
- Location:
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCreateReq.java(Lines 8-15)All findings confirm that the design is consistent and well-structured.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 10-10: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCreateReq.java#L10
Added line #L10 was not covered by testsapi/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCreateReq.java (1)
8-15: Consistent implementation of WebSocket request classThe class maintains consistent structure with other WebSocket request classes, properly extending
BaseReqand using the same set of Lombok annotations. The@NonNullannotation oncallbackHandlercorrectly enforces that the callback handler must be provided, which is essential for processing WebSocket events.api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClientBuilder.java (1)
1-14:Details
❓ Verification inconclusive
LGTM! Well-designed builder implementation.
The builder pattern implementation is clean, focused, and follows good practices:
- Proper encapsulation with private final fields
- Effective use of Lombok's @AllArgsConstructor to reduce boilerplate
- Simple and clear API with a single create method
One thing to verify is test coverage for this class since a similar class (WebsocketsAudioSpeechBuilder) was flagged for lacking test coverage:
🏁 Script executed:
#!/bin/bash # Check for test files that might cover this class find . -name "*WebsocketsChatClientBuilder*Test.java" -o -name "*WebsocketTest*.java" | grep -v "/target/"Length of output: 106
Attention: Verify Test Coverage for WebsocketsChatClientBuilder
LGTM on the builder implementation—its encapsulation, use of Lombok’s @AllArgsConstructor, and single-method API are well done. However, the test search script returned no output, which raises a concern about whether this class is adequately covered by tests. Please manually verify if test cases exist for this class (perhaps under non-standard naming or in an unexpected location) and, if not, add appropriate test coverage similar to what’s expected for classes like WebsocketsAudioSpeechBuilder.
README.md (1)
74-76: Great addition of WebSocket examples to the table!The examples table now includes entries for websocket chat, speech synthesis, and transcription, making it easy for users to find these examples.
api/src/main/java/com/coze/openapi/service/auth/Auth.java (1)
25-25:Details
✅ Verification successful
Update the token method documentation to reflect its abstract status
The token() method has been changed from a concrete implementation to abstract, which means subclasses now must implement their own token refresh logic. The existing documentation doesn't reflect this change.
Update the method documentation:
/** * 获取token + * Subclasses must implement this method to provide their own token retrieval and refresh logic. * * @return token */ public abstract String token();Let's check which subclasses need to implement this abstract method:
🏁 Script executed:
#!/bin/bash # Find all direct subclasses of Auth class echo "Finding direct subclasses of Auth..." find . -name "*.java" -type f -exec grep -l "extends Auth" {} \;Length of output: 281
Action: Update the token() method Javadoc in Auth
Please update the documentation in the abstract class as follows:
/** * 获取token + * Subclasses must implement this method to provide their own token retrieval and refresh logic. * * @return token */ public abstract String token();This ensures that all direct subclasses of Auth (currently located in
./api/src/main/java/com/coze/openapi/service/auth/JWTOAuth.javaand./api/src/main/java/com/coze/openapi/service/auth/TokenAuth.java) are clearly informed that they must implement their own token refresh logic.api/src/main/java/com/coze/openapi/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsClient.java (2)
31-35: Check for null or invalid data intranscriptionsUpdate.
Consider validating thedataobject before sending an event to avoid runtime errors in casedatais null or partially populated.
56-59: LGTM!
Returning the handler is straightforward and consistent.api/src/main/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsListener.java (4)
31-34: LGTM!
Submitting the message to a single-threaded executor helps avoid blocking the OkHttp thread.
36-39: LGTM!
Properly delegating binary messages to the same event handler.
52-55: LGTM!
Gracefully handling the onClosed event by notifying the callback.
57-61: Validate executor state before shutdown.
This approach is correct; it checks if the executor is not shut down. No additional concerns here.api/src/main/java/com/coze/openapi/service/service/websocket/common/BaseWebsocketsClient.java (1)
75-100: LGTM!
The shutdown logic orderly closes the WebSocket and executor service, handling interruptions gracefully.api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java (1)
12-12: Recommend adding unit tests for these handlers.Static analysis indicates these lines currently lack coverage. We suggest writing or updating tests that invoke each event-handling method to ensure the correctness of your WebSocket audio speech flow.
Also applies to: 15-15, 18-18, 22-22, 26-26, 30-30
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-12: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechCallbackHandler.java#L12
Added line #L12 was not covered by testsapi/src/test/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClientTest.java (1)
1-873: LGTM!These tests thoroughly cover the event-handling functionality and validate diverse message flows. No major concerns identified.
api/src/main/java/com/coze/openapi/service/service/CozeAPI.java (3)
116-118: Add coverage for thewebsockets()method.Line 117 is flagged as uncovered. Consider adding a test case that calls
cozeAPI.websockets()to ensure correct instantiation and usage of theWebsocketsClient.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 117-117: api/src/main/java/com/coze/openapi/service/service/CozeAPI.java#L117
Added line #L117 was not covered by tests
204-205: Good addition of theAudioTranscriptionAPI.These lines integrate a new interface for audio transcription. The approach looks solid and aligns with the rest of the service setup.
207-210: Use a robust URL transformation for websocket connections.This naive replacement may fail for more complex or unexpected URL formats. See prior discussions about URI parsing and reconstruction.
api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechClient.java (2)
44-46: Extend test coverage togetCallbackHandler().The
getCallbackHandler()method is not exercised by current tests, causing the lowered coverage. Consider adding a test that verifies a correct handler is returned.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 45-45: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechClient.java#L45
Added line #L45 was not covered by tests
54-56: Test for null or invalideventTypescenario.This line handles the case where
eventTypeis null, but no test currently covers it. A test that sends malformed or incomplete JSON would raise coverage and confirm robust error handling.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 55-55: api/src/main/java/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechClient.java#L55
Added line #L55 was not covered by testsapi/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatCallbackHandler.java (1)
22-23: Abstract methods do not provide defaults.
This abstract callback handler has empty, no-op methods for each event. Ensure that subclasses properly override these methods to handle events as intended.example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java (1)
92-134: Overall well-structured example with proper resource managementThe example does a good job of properly initializing the WebSocket client, sending events, and cleaning up resources in the finally block. The approach of extending the callback handler and implementing the required methods provides a clear pattern for users to follow.
...va/com/coze/openapi/service/service/websocket/audio/speech/WebsocketsAudioSpeechBuilder.java
Show resolved
Hide resolved
api/src/main/java/com/coze/openapi/service/service/websocket/WebsocketsClient.java
Show resolved
Hide resolved
...api/service/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsBuilder.java
Show resolved
Hide resolved
api/src/main/java/com/coze/openapi/service/service/websocket/audio/WebsocketsAudioClient.java
Show resolved
Hide resolved
...ice/service/websocket/audio/transcriptions/WebsocketsAudioTranscriptionsCallbackHandler.java
Show resolved
Hide resolved
example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java
Outdated
Show resolved
Hide resolved
example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java
Show resolved
Hide resolved
example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java
Show resolved
Hide resolved
example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
example/src/main/java/example/websocket/chat/ChatExample.java (7)
50-50: Consider translating Chinese comments to English for consistency.The comment "分配 10MB 缓冲区" (allocating 10MB buffer) should be translated to English to maintain code consistency and improve readability for all developers.
- private final ByteBuffer buffer = ByteBuffer.allocate(1024 * 1024 * 10); // 分配 10MB 缓冲区 + private final ByteBuffer buffer = ByteBuffer.allocate(1024 * 1024 * 10); // Allocating 10MB buffer
59-59: Remove or uncomment unused code.There's a commented-out line of code that appears to be leftover from development. Either remove it entirely or uncomment it if it's intended to be used.
- // client.sendEvent(new BaseEvent());
65-65: Fix typo in debug message."Revieve" should be "Receive".
- System.out.printf("Revieve: %s\n", event.getData().getContent()); + System.out.printf("Receive: %s\n", event.getData().getContent());
113-115: Consider translating Chinese comments and text to English for consistency.The comment "模拟端插件返回" and the hardcoded Chinese weather text should be translated to English for consistency or externalized for proper localization.
- // 模拟端插件返回 - .output(Utils.toJson(new Weather("今天深圳的天气是 10 到 20 摄氏度"))) + // Simulating plugin response + .output(Utils.toJson(new Weather("Today's weather in Shenzhen is 10 to 20 degrees Celsius")))
167-169: Consider improving comments for clarity.The comment "模拟人说话的间隔" should be translated to English, and you might consider making the sleep duration configurable.
- // 模拟人说话的间隔 + // Simulate natural speech pacing TimeUnit.MILLISECONDS.sleep(100);
80-90: Consider adding file path handling for output.wav.The current implementation writes to "output.wav" in the current directory. For better usability, consider:
- Using a configurable output path
- Adding timestamp or unique identifier to prevent overwriting
- Creating directories if they don't exist
@Override public void onConversationAudioCompleted( WebsocketsChatClient client, ConversationAudioCompletedEvent event) { try { - ExampleUtils.writePcmToWavFile(buffer.array(), "output.wav"); + String outputPath = System.getProperty("user.dir") + "/output_" + + System.currentTimeMillis() + ".wav"; + ExampleUtils.writePcmToWavFile(buffer.array(), outputPath); - System.out.println("========= Output Audio Completed ========="); + System.out.println("========= Output Audio Completed: " + outputPath + " ========="); isDone = true; } catch (IOException e) { e.printStackTrace(); } }
155-159: Consider externalizing the hardcoded text input.The hardcoded Chinese input text should be externalized for better flexibility and to avoid embedding literal strings in the code.
CreateSpeechReq.builder() - .input("今天深圳的天气怎么样?") + .input(System.getenv().getOrDefault("SPEECH_TEXT", "今天深圳的天气怎么样?")) .voiceID(voiceID) .responseFormat(AudioFormat.WAV) .sampleRate(24000)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java(1 hunks)example/src/main/java/example/websocket/audio/transcriptions/WebsocketTranscriptionsExample.java(1 hunks)example/src/main/java/example/websocket/chat/ChatExample.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- example/src/main/java/example/websocket/audio/transcriptions/WebsocketTranscriptionsExample.java
- example/src/main/java/example/websocket/audio/speech/WebsocketAudioSpeechExample.java
🔇 Additional comments (2)
example/src/main/java/example/websocket/chat/ChatExample.java (2)
149-149: Update WebSocketsChatCreateReq to match the suggestion from previous review.Based on a previous review comment, consider changing the method call to match the suggested pattern.
- .create(new WebsocketsChatCreateReq(botID, new CallbackHandler())); + .create(new WebsocketChatCreateReq(botID, new CallbackHandler()));
49-49: Update class declaration to match the suggestion from previous review.Based on a previous review comment, the class declaration should be updated.
- private static class CallbackHandler extends WebsocketsChatCallbackHandler { + private static class CallbackHandler extends WebsocketChatCallbackHandler {
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/test/java/com/coze/openapi/service/auth/JWTOAuthTest.java (1)
81-95: Avoid Thread.sleep in unit testsUsing
Thread.sleep()in tests can make them flaky and unreliable, especially in CI environments where system load varies. Consider refactoring this test to use dependency injection for time or a more deterministic approach.Consider using a mock clock or time provider that can be controlled within the test:
// Add TimeProvider interface to JWTOAuth public interface TimeProvider { long currentTimeMillis(); } // Modify JWTOAuth to accept a TimeProvider private final TimeProvider timeProvider; // In your test, use a mock TimeProvider @Mock private TimeProvider timeProvider; // Instead of Thread.sleep, advance the mock time when(timeProvider.currentTimeMillis()).thenReturn(System.currentTimeMillis()); String token1 = jwtOAuth.token(); // Advance time by simulating 3 seconds passing when(timeProvider.currentTimeMillis()).thenReturn(System.currentTimeMillis() + 3000); String token2 = jwtOAuth.token();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/test/java/com/coze/openapi/service/auth/JWTOAuthTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (Java 17 on Windows)
- GitHub Check: test (Java 11 on Windows)
🔇 Additional comments (5)
api/src/test/java/com/coze/openapi/service/auth/JWTOAuthTest.java (5)
49-63: Verify token caching behavior expectationThe test
testTokenCachingverifies that the same token is returned for consecutive calls, but also expectsgetAccessTokento be called twice (line 62). If caching is working as intended, the method should only be called once for the initial token retrieval, with subsequent calls using the cached value.Please verify if this is the expected behavior. If caching is intended, consider updating the verification to:
- verify(jwtClient, times(2)).getAccessToken((Integer) any(), any(), any()); + verify(jwtClient, times(1)).getAccessToken((Integer) any(), any(), any());
97-118: Good test coverage for TTL variationsThis test thoroughly verifies how different TTL values are handled, ensuring the correct TTL is passed to the
getAccessTokenmethod based on the specified conditions.
1-16: Well-structured test class with appropriate importsThe test class is well-organized with appropriate imports for JUnit 5 and Mockito. The
@ExtendWith(MockitoExtension.class)annotation correctly enables Mockito integration with JUnit 5.
24-34: Good constructor validation testsThese tests properly validate the constructor behavior for both error cases (null client) and successful initialization.
65-79: Effective testing of custom parametersThis test effectively verifies that custom TTL and session name parameters are correctly passed to the client when using the builder pattern.
Change-Id: I7f687143392203a3b524e24b2255b77e2bb1ecb6
Change-Id: If1fcbdb9bf7a275eaeb2bbdea5571ecaa2b02294
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
api/src/main/java/com/coze/openapi/client/websocket/event/downstream/InputAudioBufferSpeechStoppedEvent.java (1)
1-19: Well-structured event class with good use of Lombok annotations.The class is well-designed with appropriate use of Lombok annotations to reduce boilerplate code. The event type is properly defined with a default value and JSON property mapping. The extension of BaseEvent with proper callSuper settings in annotations ensures correct behavior inheritance.
Consider adding a brief Javadoc comment to describe when this event is triggered in the WebSocket communication flow, which would improve API documentation.
package com.coze.openapi.client.websocket.event.downstream; import com.coze.openapi.client.websocket.common.BaseEvent; import com.coze.openapi.client.websocket.event.EventType; import com.fasterxml.jackson.annotation.JsonProperty; import lombok.*; import lombok.experimental.SuperBuilder; +/** + * Event triggered when speech input has stopped in an audio buffer. + * This is part of the downstream events in the WebSocket communication flow. + */ @Data @SuperBuilder @NoArgsConstructor @EqualsAndHashCode(callSuper = true) @ToString(callSuper = true) public class InputAudioBufferSpeechStoppedEvent extends BaseEvent { @JsonProperty("event_type") @Builder.Default private final String eventType = EventType.INPUT_AUDIO_BUFFER_SPEECH_STOPPED; }api/src/main/java/com/coze/openapi/client/websocket/event/downstream/InputAudioBufferSpeechStartedEvent.java (2)
1-21: Class structure looks good, but there's a discrepancy in the comments.The class is well-structured using appropriate Lombok annotations and extends BaseEvent correctly. However, there's a discrepancy between:
- The class name and eventType constant which indicate "speech started"
- The comment on line 16 which specifies "event_type: input_audio_buffer.completed"
These should be aligned to avoid confusion for developers using this API.
Either:
- Update the class name and event type constant to reflect "completed" instead of "started", or
- Update the comment to correctly reflect the "speech started" event
Additionally, consider adding proper JavaDoc comments in English for better API documentation.
10-14: Consider simplifying Lombok annotations.The current set of annotations has some redundancy:
@Dataalready includes functionality of@ToStringand@EqualsAndHashCode- Explicitly adding them with
callSuper = trueis valid but could be simplifiedYou could either:
@Data @SuperBuilder @NoArgsConstructor -@EqualsAndHashCode(callSuper = true) -@ToString(callSuper = true)Or if you need the callSuper behavior:
-@Data +@Getter +@Setter @SuperBuilder @NoArgsConstructor @EqualsAndHashCode(callSuper = true) @ToString(callSuper = true)api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatCallbackHandler.java (2)
6-9: Consider using an interface or providing default/no-op implementations.All methods in this abstract class have empty bodies, suggesting that subclasses may only need to implement a subset. Converting this to an interface with default methods or providing explicit no-op methods could promote clarity for consumers and reduce boilerplate in subclasses.
10-82: Document expected behavior for each event handler.Even if the methods remain no-op by default, providing minimal documentation (beyond the inline comments) about what developers should do or expect within each event handler can improve clarity and maintainability.
api/src/main/java/com/coze/openapi/client/websocket/event/model/TurnDetection.java (2)
1-22: Field name inconsistency between code and JSON propertyThe field
silenceDurationMson line 21 is mapped to the JSON propertysuffix_padding_ms. This naming inconsistency could cause confusion for developers. The Java field name suggests it's measuring silence duration, while the JSON property suggests it's about suffix padding.Consider renaming the field to match the JSON property's semantic meaning:
@JsonProperty("suffix_padding_ms") - private Integer silenceDurationMs; + private Integer suffixPaddingMs;
1-22: Missing JavaDoc documentationThis class lacks documentation explaining its purpose and the meaning of its fields. Since it's part of a WebSocket event model, proper documentation would help developers understand when and how to use it.
Consider adding JavaDoc comments to the class and its fields:
+/** + * Represents voice activity detection configuration for WebSocket communication. + * Used to configure client-side VAD (Voice Activity Detection) settings. + */ @Data @Builder @NoArgsConstructor @AllArgsConstructor @EqualsAndHashCode public class TurnDetection { @Builder.Default @JsonProperty("type") + /** + * The type of turn detection. Defaults to "client_vad". + */ private String type = "client_vad"; @JsonProperty("prefix_padding_ms") + /** + * The padding time in milliseconds to add before detected speech. + */ private Integer prefixPaddingMs; @JsonProperty("suffix_padding_ms") + /** + * The padding time in milliseconds to add after detected speech. + */ private Integer silenceDurationMs; }api/src/main/java/com/coze/openapi/client/websocket/event/model/ChatUpdateEventData.java (3)
9-14: Consider using @builder instead of @SuperBuilder if inheritance is not neededThe class is using
@SuperBuilderannotation, which is designed for inheritance hierarchies. If this class is not meant to be extended or doesn't extend another class, consider using the simpler@Builderannotation instead.@Data -@SuperBuilder +@Builder @NoArgsConstructor @AllArgsConstructor public class ChatUpdateEventData {
1-27: Missing JavaDoc documentationThis class lacks documentation explaining its purpose and the role of its fields in the WebSocket event structure. Documentation would be valuable for understanding how this class fits into the WebSocket communication flow.
Consider adding JavaDoc comments to the class and its fields:
+/** + * Represents the data payload of a chat update event sent over WebSocket. + * Contains configurations and audio settings for the chat session. + */ @Data @SuperBuilder @NoArgsConstructor @AllArgsConstructor public class ChatUpdateEventData { @JsonProperty("input_audio") + /** + * Configuration for the audio input in the chat session. + */ private InputAudio inputAudio; @JsonProperty("output_audio") + /** + * Configuration for the audio output in the chat session. + */ private OutputAudio outputAudio; @JsonProperty("chat_config") + /** + * General configuration settings for the chat session. + */ private ChatConfig chatConfig; @JsonProperty("turn_detection") + /** + * Voice activity detection configuration for determining when a user has finished speaking. + */ private TurnDetection turnDetection; }
16-26: Consider adding null validation or @nonnull annotationsThe class contains reference type fields without any null validation. For a more robust implementation, consider adding null validation or using annotations like
@NonNullto make null-safety intentions explicit.@JsonProperty("input_audio") +@lombok.NonNull private InputAudio inputAudio; @JsonProperty("output_audio") +@lombok.NonNull private OutputAudio outputAudio; @JsonProperty("chat_config") +@lombok.NonNull private ChatConfig chatConfig; @JsonProperty("turn_detection") +@lombok.NonNull private TurnDetection turnDetection;Alternatively, if these fields are optional, document that explicitly in JavaDoc.
api/src/main/java/com/coze/openapi/client/websocket/event/EventType.java (4)
3-10: Consider making the class non-instantiable or using an enum for type safety.This class contains only constants and does not need to be instantiated. You might consider adding a private constructor to prevent instantiation. Alternatively, an enum could provide additional type safety and reduce potential string typos.
Example using a private constructor:
public class EventType { + private EventType() { + // Prevent instantiation + } // common public static final String CLIENT_ERROR = "client_error"; // sdk error ...
13-17: Confirm naming conventions for text buffer events.The constants here use domain-specific naming (
input_text_buffer), which is appropriate. Just ensure consistency with naming conventions across the rest of your WebSocket events to maintain clarity.
18-29: Clarify purpose of response vs. request events.Some events here (e.g.,
SPEECH_UPDATED) have a comment that suggests it sends config to the server. Typically, “.updated” events represent server responses. Consider updating the comment to clarify if it’s actually a server response or a client request.
55-62: Check for tool output event naming consistency.
CONVERSATION_CHAT_SUBMIT_TOOL_OUTPUTSuses underscores for “tool_outputs,” while other events use dot notation or different naming styles. Consider aligning the naming style to reduce confusion.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/src/main/java/com/coze/openapi/client/websocket/event/EventType.java(1 hunks)api/src/main/java/com/coze/openapi/client/websocket/event/downstream/InputAudioBufferSpeechStartedEvent.java(1 hunks)api/src/main/java/com/coze/openapi/client/websocket/event/downstream/InputAudioBufferSpeechStoppedEvent.java(1 hunks)api/src/main/java/com/coze/openapi/client/websocket/event/model/ChatUpdateEventData.java(1 hunks)api/src/main/java/com/coze/openapi/client/websocket/event/model/TurnDetection.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatCallbackHandler.java(1 hunks)api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClient.java(1 hunks)api/src/test/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClientTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/test/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClientTest.java
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (Java 17 on Windows)
- GitHub Check: test (Java 11 on Windows)
🔇 Additional comments (4)
api/src/main/java/com/coze/openapi/service/service/websocket/chat/WebsocketsChatClient.java (2)
60-68: Double-check the Base64 encoding usage for audio data.When handling audio data, ensure that the encoding (UTF-8 vs. raw bytes) is consistent downstream. This can help avoid issues where user-provided text in
inputAudioBufferAppend(String)is not handled as expected, especially if it contains non-ASCII characters.Could you confirm that any downstream processing expects Base64-encoded UTF-8 strings for all text-based data?
84-196: Reduce code duplication in the large event-type switch.This switch statement for parsing and routing events to the handler is quite extensive. A registry or lookup-based approach (e.g., a map of event types to lambda functions) can improve maintainability and allow for dynamic event type additions.
api/src/main/java/com/coze/openapi/client/websocket/event/EventType.java (2)
33-51: Ensure consistent event naming between text and audio buffers.You have parallel structures of
INPUT_TEXT_BUFFER_*andINPUT_AUDIO_BUFFER_*. This is good for clarity. Just verify that event states (e.g., “.completed” vs. “.complete”) are consistently used in both text and audio contexts to avoid confusion.
64-92: Verify coverage and usage of newly introduced events.This file defines many events for chat and conversation flows. Make sure you have corresponding handlers or usage points for all of them, especially those with side effects (e.g.,
.chat.canceled,.chat.requires_action). This ensures no “dead” event types remain unused.
| protected WebsocketsChatClient(OkHttpClient client, String wsHost, WebsocketsChatCreateReq req) { | ||
| super(client, buildUrl(wsHost, req), req.getCallbackHandler(), req); | ||
| this.handler = req.getCallbackHandler(); | ||
| } | ||
|
|
||
| protected static String buildUrl(String wsHost, WebsocketsChatCreateReq req) { | ||
| return String.format("%s%s?bot_id=%s", wsHost, uri, req.getBotID()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate or handle invalid bot ID inputs.
The buildUrl method blindly injects req.getBotID() into the URL. If bot_id is missing or invalid, it might lead to a runtime failure. Consider validating or handling edge cases (e.g., null, empty string, unencoded special characters) in req.getBotID() to avoid malformed URLs.
2 participants
