-
Notifications
You must be signed in to change notification settings - Fork 36
feat: adds support for speak fallback #381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new suite of unit tests was added for the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Unit Test
participant Speak as Speak
participant Config as SpeakProviderConfig
Test->>Speak: Create Speak (single or multi-provider)
alt Single provider
Speak->>Test: Expose Provider, Endpoint
Test->>Speak: Call ToString()
Speak->>Test: Return single-provider JSON
else Multi-provider
Speak->>Config: Create SpeakProviderConfig (for each provider)
Config->>Speak: Return to SpeakProviders list
Speak->>Test: Expose SpeakProviders
Test->>Speak: Call ToString()
Speak->>Test: Return multi-provider JSON
end
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Deepgram/Models/Agent/v2/WebSocket/Speak.cs (1)
20-24
: Endpoint property documentation could be more concise.The documentation comment is overly verbose and could be simplified while maintaining clarity.
Consider simplifying the documentation:
- /// Custom endpoint for custom models - to use a custom model, set provider.type to the flavour of API you are using (e.g. open_ai for OpenAI-like APIs). - /// Note: This is for backward compatibility with single provider format. + /// Custom endpoint for custom models. For backward compatibility with single provider format.Deepgram.Tests/UnitTests/ClientTests/AgentSpeakTests.cs (2)
16-19
: Empty Setup method should be removed.The Setup method is empty and serves no purpose. Consider removing it to reduce code clutter.
- [SetUp] - public void Setup() - { - }
1-311
: Consider adding edge case tests.The test suite is comprehensive but could benefit from additional edge case scenarios:
- Empty
SpeakProviders
list behavior- Null provider handling
- Invalid JSON structure scenarios
- Large provider arrays
Consider adding these test cases to improve coverage:
[Test] public void Speak_EmptyProvidersList_Should_Handle_Gracefully() { var speak = new Speak { SpeakProviders = new List<SpeakProviderConfig>() }; var result = speak.ToString(); result.Should().NotBeNull(); var parsed = JsonDocument.Parse(result); parsed.RootElement.GetProperty("speak").GetArrayLength().Should().Be(0); } [Test] public void SpeakProviderConfig_WithNullEndpoint_Should_Serialize_Correctly() { var provider = new Provider(); provider.Type = "deepgram"; var config = new SpeakProviderConfig { Provider = provider, Endpoint = null }; var result = config.ToString(); var parsed = JsonDocument.Parse(result); parsed.RootElement.TryGetProperty("endpoint", out _).Should().BeFalse(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Deepgram.Tests/UnitTests/ClientTests/AgentSpeakTests.cs
(1 hunks)Deepgram/Models/Agent/v2/WebSocket/Speak.cs
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#253
File: Deepgram.Dev.sln:0-0
Timestamp: 2024-10-09T02:19:48.728Z
Learning: References to "Prerecorded" and other project names in the documentation and build scripts have been updated as part of the PR changes. Remaining mentions of "Prerecorded" in `Deepgram/README.md`, `examples/README.md`, and `README.md` were noted for potential review.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#253
File: Deepgram.Dev.sln:0-0
Timestamp: 2024-07-29T07:02:00.905Z
Learning: References to "Prerecorded" and other project names in the documentation and build scripts have been updated as part of the PR changes. Remaining mentions of "Prerecorded" in `Deepgram/README.md`, `examples/README.md`, and `README.md` were noted for potential review.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Clients/Interfaces/v2/ISpeakWebSocketClient.cs:49-49
Timestamp: 2024-10-28T19:43:32.373Z
Learning: In the 'deepgram-dotnet-sdk' project, existing documentation bugs will be addressed in a later pass, so minor documentation issues in code comments may be deferred during code reviews.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Clients/Interfaces/v2/ISpeakWebSocketClient.cs:106-107
Timestamp: 2024-10-28T18:22:59.455Z
Learning: In `Deepgram/Clients/Interfaces/v2/ISpeakWebSocketClient.cs`, commented-out methods related to binary send, such as `SpeakWithStream`, `SendBinary`, and `SendBinaryImmediately`, are intentionally left in the code to indicate that binary send is not currently supported.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#347
File: Deepgram/ClientFactory.cs:210-261
Timestamp: 2024-11-01T16:16:34.733Z
Learning: In `Deepgram/ClientFactory.cs`, the deprecated methods `CreateSpeakClient`, `CreatePreRecordedClient`, and `CreateOnPremClient` are intentionally retained for backward compatibility and should not be suggested for removal in code reviews.
Deepgram/Models/Agent/v2/WebSocket/Speak.cs (10)
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Clients/Interfaces/v2/ISpeakWebSocketClient.cs:106-107
Timestamp: 2024-10-28T18:22:59.455Z
Learning: In `Deepgram/Clients/Interfaces/v2/ISpeakWebSocketClient.cs`, commented-out methods related to binary send, such as `SpeakWithStream`, `SendBinary`, and `SendBinaryImmediately`, are intentionally left in the code to indicate that binary send is not currently supported.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#309
File: Deepgram/Models/Speak/v1/WebSocket/v1/CommandSource.cs:7-22
Timestamp: 2024-10-09T02:19:46.087Z
Learning: The `Type` property in the `CommandSource` class within `Deepgram.Models.Speak.v1.WebSocket` can be dynamically initialized based on implementation requirements.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#309
File: Deepgram/Models/Speak/v1/WebSocket/v1/CommandSource.cs:7-22
Timestamp: 2024-06-28T17:40:53.347Z
Learning: The `Type` property in the `CommandSource` class within `Deepgram.Models.Speak.v1.WebSocket` can be dynamically initialized based on implementation requirements.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#347
File: Deepgram/ClientFactory.cs:210-261
Timestamp: 2024-11-01T16:16:34.733Z
Learning: In `Deepgram/ClientFactory.cs`, the deprecated methods `CreateSpeakClient`, `CreatePreRecordedClient`, and `CreateOnPremClient` are intentionally retained for backward compatibility and should not be suggested for removal in code reviews.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#309
File: Deepgram/Models/Speak/v1/SyncResponse.cs:7-18
Timestamp: 2024-06-28T17:38:05.314Z
Learning: User dvonthenen has intentionally kept the deprecated `SyncResponse` class in the `Deepgram.Models.Speak.v1` namespace to allow users a transition period without needing to immediately switch to the new `Deepgram.Models.Speak.v1.REST.SyncResponse`.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Clients/Interfaces/v2/ISpeakWebSocketClient.cs:85-85
Timestamp: 2024-10-28T18:23:10.963Z
Learning: In the `ISpeakWebSocketClient` interface, the method `SpeakWithText(string data)` is intentionally named and should not be renamed.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Models/Listen/v2/WebSocket/Average.cs:26-29
Timestamp: 2024-10-28T18:04:20.601Z
Learning: In `Deepgram/Models/Listen/v2/WebSocket/Average.cs`, it's acceptable to let exceptions in the `ToString()` method surface to the top when the method is mainly used for debugging.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#309
File: Deepgram/Models/Speak/v1/AsyncResponse.cs:7-18
Timestamp: 2024-06-28T17:42:39.521Z
Learning: User dvonthenen has intentionally kept the deprecated `AsyncResponse` class in the `Deepgram.Models.Speak.v1` namespace to allow users a transition period without needing to immediately switch to the new `Deepgram.Models.Speak.v1.REST.AsyncResponse`.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#309
File: Deepgram/Models/Speak/v1/AsyncResponse.cs:7-18
Timestamp: 2024-10-09T02:19:46.086Z
Learning: User dvonthenen has intentionally kept the deprecated `AsyncResponse` class in the `Deepgram.Models.Speak.v1` namespace to allow users a transition period without needing to immediately switch to the new `Deepgram.Models.Speak.v1.REST.AsyncResponse`.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#336
File: Deepgram/Models/Analyze/v1/TextSource.cs:9-14
Timestamp: 2024-09-27T14:19:19.336Z
Learning: In the `TextSource` class of the Deepgram .NET SDK, the `Text` property can remain mutable as per the user's preference.
Deepgram.Tests/UnitTests/ClientTests/AgentSpeakTests.cs (11)
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#347
File: Deepgram/ClientFactory.cs:210-261
Timestamp: 2024-11-01T16:16:34.733Z
Learning: In `Deepgram/ClientFactory.cs`, the deprecated methods `CreateSpeakClient`, `CreatePreRecordedClient`, and `CreateOnPremClient` are intentionally retained for backward compatibility and should not be suggested for removal in code reviews.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Clients/Interfaces/v2/ISpeakWebSocketClient.cs:106-107
Timestamp: 2024-10-28T18:22:59.455Z
Learning: In `Deepgram/Clients/Interfaces/v2/ISpeakWebSocketClient.cs`, commented-out methods related to binary send, such as `SpeakWithStream`, `SendBinary`, and `SendBinaryImmediately`, are intentionally left in the code to indicate that binary send is not currently supported.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#350
File: Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs:103-106
Timestamp: 2024-11-04T16:22:15.635Z
Learning: In `DeepgramHttpClientOptions.cs`, when suggesting changes to `ApiKey` initialization logic, ensure that setting `ApiKey` via the property before the constructor runs is properly handled to avoid introducing bugs.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#0
File: :0-0
Timestamp: 2024-07-29T07:02:00.905Z
Learning: The Deepgram .NET SDK examples are designed to be self-contained to facilitate ease of understanding and use. Centralizing functionalities like JSON parsing options among various examples is not preferred.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#0
File: :0-0
Timestamp: 2024-07-29T07:02:00.905Z
Learning: The Deepgram .NET SDK examples are designed to be self-contained, without centralizing elements like JSON parsing options among various examples. This design choice aims to make each example easily understandable and runnable on its own.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#309
File: Deepgram/Models/Speak/v1/WebSocket/v1/CommandSource.cs:7-22
Timestamp: 2024-10-09T02:19:46.087Z
Learning: The `Type` property in the `CommandSource` class within `Deepgram.Models.Speak.v1.WebSocket` can be dynamically initialized based on implementation requirements.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#309
File: Deepgram/Models/Speak/v1/WebSocket/v1/CommandSource.cs:7-22
Timestamp: 2024-06-28T17:40:53.347Z
Learning: The `Type` property in the `CommandSource` class within `Deepgram.Models.Speak.v1.WebSocket` can be dynamically initialized based on implementation requirements.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#0
File: :0-0
Timestamp: 2024-07-29T07:02:00.905Z
Learning: The Deepgram .NET SDK examples are designed to be self-contained, with a preference against centralizing JSON parsing options among various examples to ensure each example is easily understandable and runnable on its own.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Models/Listen/v2/WebSocket/ResultResponse.cs:79-85
Timestamp: 2024-10-28T18:10:11.373Z
Learning: In the Deepgram .NET SDK, when implementing `ToString()` methods in model classes (e.g., `ResultResponse.cs` in `Deepgram/Models/Listen/v2/WebSocket`), it's acceptable to let exceptions bubble up when these methods are primarily used in debug messages.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#343
File: examples/text-to-speech/websocket/simple/Program.cs:47-108
Timestamp: 2024-10-23T17:23:51.386Z
Learning: Example code in `examples/text-to-speech/websocket/simple/Program.cs` is intended to be simple and does not need to be fully optimized or perfect.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Models/Listen/v2/WebSocket/Average.cs:26-29
Timestamp: 2024-10-28T18:04:20.601Z
Learning: In `Deepgram/Models/Listen/v2/WebSocket/Average.cs`, it's acceptable to let exceptions in the `ToString()` method surface to the top when the method is mainly used for debugging.
🧬 Code Graph Analysis (1)
Deepgram/Models/Agent/v2/WebSocket/Speak.cs (2)
Deepgram/Models/Agent/v2/WebSocket/Provider.cs (2)
Provider
(9-68)ToString
(64-67)Deepgram/Utilities/JsonSerializeOptions.cs (1)
JsonSerializeOptions
(7-13)
🔇 Additional comments (12)
Deepgram/Models/Agent/v2/WebSocket/Speak.cs (4)
11-12
: Documentation accurately describes backward compatibility approach.The updated documentation clearly explains the backward compatibility strategy and guides users on when to use the new
SpeakProviders
property.
26-33
: Clear documentation for the new array format feature.The documentation comprehensively explains the new
SpeakProviders
property, its precedence over single provider properties, and the intended use case.
38-41
: ToString() implementation follows established pattern.The
ToString()
override is consistent with the pattern used throughout the codebase, as shown in the relevant code snippets. It properly usesJsonSerializeOptions.DefaultOptions
andRegex.Unescape
for consistent serialization.
44-70
: SpeakProviderConfig record is well-structured and consistent.The new record follows the same design patterns as the parent
Speak
record, with appropriate JSON attributes, documentation, andToString()
implementation. The structure supports the array format requirements effectively.Deepgram.Tests/UnitTests/ClientTests/AgentSpeakTests.cs (8)
23-50
: Comprehensive backward compatibility test.The test effectively verifies that single provider configuration works as expected and maintains backward compatibility. The assertions properly validate both the Provider and Endpoint properties while ensuring SpeakProviders remains null.
52-78
: Effective JSON serialization validation.The test verifies that the
ToString()
method produces valid JSON and contains expected properties. The JSON parsing validation ensures the serialization works correctly.
84-120
: Thorough array format testing.The test properly validates the new multi-provider array format, checking both the structure and content of the
SpeakProviders
list. The assertions verify different provider types and endpoint configurations.
122-158
: Robust array JSON serialization test.The test validates that the array format serializes correctly to JSON, including proper array structure verification. The JsonDocument parsing ensures the output matches expected format.
160-187
: Complete SpeakProviderConfig structure validation.The test comprehensively validates the new
SpeakProviderConfig
record, ensuring all properties are correctly set and accessible.
189-221
: Effective SpeakProviderConfig JSON serialization test.The test validates that individual
SpeakProviderConfig
instances serialize correctly to JSON, with proper JSON parsing verification.
227-278
: Comprehensive array format JSON structure validation.The test verifies that the complete array format serializes correctly and matches the expected JSON structure. The detailed assertions ensure proper nesting and property values.
280-308
: Effective single provider format validation.The test ensures that when using single provider format, the JSON output excludes the array property and maintains the expected structure. The negative assertion for the "speak" property name is particularly valuable.
i did a local test app and this checks out. ✅ Perfect! 🎉 The test application ran successfully and all tests passed. Let me summarize what we've accomplished: ✅ Test Results SummaryAll Tests Completed Successfully! The speak provider options functionality works correctly with: 1. Single Provider Format (Backward Compatibility)
2. Array Provider Format (New Multi-Provider Support)
3. WebSocket Integration
4. JSON Serialization
🔧 What This Test Proves
🧹 CleanupFuture Steps: After testing completes, we can clean up the The speak provider functionality is working perfectly! ✨ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Approved
Proposed changes
This PR implements two new features for the Deepgram .NET SDK Agent WebSocket client:
2. Speak Provider Array Configuration
Enhances the Agent Speak configuration to support multiple TTS providers in array format while maintaining full backward compatibility with existing single provider configurations. This allows for more flexible fallback scenarios and multi-provider TTS setups.
Types of changes
What types of changes does your code introduce to the community .NET SDK?
Checklist
Further comments
Implementation Approach
Speak Array Configuration:
SpeakProviders
property takes precedence when set, allowing array configurationsSpeakProviderConfig
objects can have their own provider settings and endpointsTesting Coverage
Comprehensive test suites added:
AgentSpeakTests.cs
: 8 tests for Speak array configurationTest scenarios include:
JSON Format Examples
Speak Array Configuration:
Verification Steps
dotnet build
- All projects compile successfullydotnet test
- All 135 tests pass (18 new tests added)Known Issues
None. All tests pass and backward compatibility is maintained.
Breaking Changes
None. This is a purely additive change that maintains full backward compatibility.
Types of changes
Checklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Summary by CodeRabbit
New Features
Tests