feat: support arbitrary properties in speak provider #374
Conversation
WalkthroughThe changes introduce dynamic extensibility to the SpeakProvider type, allowing it to accept arbitrary JSON properties via dynamic member access and a backing dictionary. The Speak record’s Provider property is updated to dynamic. A new example project demonstrates using arbitrary keys in the agent WebSocket settings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Program
participant Deepgram
participant WebSocketClient
User->>Program: Start application
Program->>Deepgram: Initialize library (debug log)
Program->>WebSocketClient: Create client with options
Program->>WebSocketClient: Connect with settings (including arbitrary keys)
WebSocketClient-->>Program: Connection result (success/failure)
alt On success
Program->>WebSocketClient: Subscribe to event handlers
WebSocketClient-->>Program: Emit events (Open, Welcome, SettingsApplied, etc.)
Program->>User: Output event data to console
User->>Program: Press key to exit
Program->>WebSocketClient: Stop connection
Program->>Deepgram: Terminate library
else On failure
Program->>User: Output error message
end
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs (1)
90-98: Consider improving snake_case conversion for edge cases.The current implementation may not handle edge cases correctly, such as consecutive uppercase letters or acronyms.
Here's a more robust implementation:
private static string ToSnakeCase(string name) { - return string.Concat( - name.Select((x, i) => - i > 0 && char.IsUpper(x) ? "_" + char.ToLower(x) : char.ToLower(x).ToString() - ) - ); + return Regex.Replace( + Regex.Replace(name, @"([A-Z]+)([A-Z][a-z])", "$1_$2"), + @"([a-z\d])([A-Z])", + "$1_$2" + ).ToLower(); }This handles cases like:
XMLParser→xml_parserIOStream→io_streamcamelCase→camel_caseexamples/agent/websocket/arbitrary_keys/Agent.csproj (1)
1-18: Example project configuration looks good.The project is properly configured with appropriate settings. Consider using forward slashes in the project reference path for better cross-platform compatibility.
- <ProjectReference Include="..\..\..\..\Deepgram\Deepgram.csproj" /> + <ProjectReference Include="../../../../Deepgram/Deepgram.csproj" />examples/agent/websocket/arbitrary_keys/Program.cs (1)
63-122: Consider refactoring the repetitive subscription pattern.While the current implementation is functional and clear for educational purposes, the repetitive subscription pattern could be refactored for better maintainability.
Consider this approach to reduce code duplication:
+ var eventTypes = new Dictionary<string, Func<Task<bool>>> + { + ["OpenResponse"] = () => agentClient.Subscribe(new EventHandler<OpenResponse>((sender, e) => + Console.WriteLine($"----> {e.Type} received"))), + ["WelcomeResponse"] = () => agentClient.Subscribe(new EventHandler<WelcomeResponse>((sender, e) => + Console.WriteLine($"----> {e} received"))), + // ... other event types + }; + + foreach (var eventType in eventTypes) + { + if (!await eventType.Value()) + { + Console.WriteLine($"Failed to subscribe to {eventType.Key} event"); + return; + } + }However, the current explicit approach is perfectly acceptable for an example file as it clearly shows each event type and its handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
output_1.wavis excluded by!**/*.wav,!**/*.wav
📒 Files selected for processing (4)
Deepgram/Models/Agent/v2/WebSocket/Speak.cs(1 hunks)Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs(1 hunks)examples/agent/websocket/arbitrary_keys/Agent.csproj(1 hunks)examples/agent/websocket/arbitrary_keys/Program.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
examples/agent/websocket/arbitrary_keys/Agent.csproj (1)
Learnt from: OoLunar
PR: deepgram/deepgram-dotnet-sdk#352
File: examples/Deepgram.Examples.Manage.Members/Deepgram.Examples.Manage.Members.csproj:4-4
Timestamp: 2024-11-05T22:01:40.632Z
Learning: In the Deepgram .NET SDK, when the main library targets stable .NET versions, it's acceptable for example projects to target newer .NET versions (e.g., .NET 9.0), as newer .NET applications can consume dependencies from older versions.
Deepgram/Models/Agent/v2/WebSocket/Speak.cs (2)
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.
🧬 Code Graph Analysis (2)
Deepgram/Models/Agent/v2/WebSocket/Speak.cs (2)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs (2)
SpeakProvider(9-104)ToString(100-103)Deepgram/Utilities/JsonSerializeOptions.cs (1)
JsonSerializeOptions(7-13)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs (15)
Deepgram/Models/Agent/v2/WebSocket/Speak.cs (1)
ToString(27-30)Deepgram/Models/Agent/v2/WebSocket/ListenProvider.cs (1)
ToString(32-35)Deepgram/Models/Agent/v2/WebSocket/Endpoint.cs (1)
ToString(20-23)Deepgram/Models/Agent/v2/WebSocket/Think.cs (1)
ToString(32-35)Deepgram/Models/Agent/v2/WebSocket/Agent.cs (1)
ToString(38-41)Deepgram/Models/Agent/v2/WebSocket/Settings.cs (1)
ToString(34-37)Deepgram/Models/Agent/v2/WebSocket/CartesiaVoice.cs (1)
ToString(20-23)Deepgram/Models/Agent/v2/WebSocket/FunctionEndpoint.cs (1)
ToString(24-27)Deepgram/Models/Agent/v2/WebSocket/Function.cs (1)
ToString(28-31)Deepgram/Models/Agent/v2/WebSocket/Input.cs (1)
ToString(20-23)Deepgram/Models/Agent/v2/WebSocket/Listen.cs (1)
ToString(20-23)Deepgram/Models/Agent/v2/WebSocket/Output.cs (1)
ToString(28-31)Deepgram/Models/Agent/v2/WebSocket/UpdatePromptSchema.cs (1)
ToString(23-26)Deepgram/Models/Agent/v2/WebSocket/ThinkProvider.cs (1)
ToString(34-37)Deepgram/Utilities/JsonSerializeOptions.cs (1)
JsonSerializeOptions(7-13)
🔇 Additional comments (7)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs (2)
11-53: Existing properties maintained correctly.All existing properties are preserved with their original attributes and documentation, ensuring backward compatibility.
100-104: ToString implementation follows SDK patterns.The method correctly serializes the object including dynamic properties, maintaining consistency with other model classes.
examples/agent/websocket/arbitrary_keys/Program.cs (5)
1-12: LGTM! Clean imports and namespace structure.The using statements are appropriate and the namespace follows standard conventions.
19-23: Good logging setup for example purposes.The debug logging level is appropriate for demonstration, and the commented verbose option provides good flexibility for users who want more detailed output.
44-44: Excellent demonstration of the new arbitrary properties feature!This line effectively showcases the new dynamic capability of
SpeakProviderto accept arbitrary JSON properties, which is the core feature of this PR.
55-61: Good connection handling with proper error management.The async connection logic and early return on failure provide robust error handling for the example.
124-137: Proper cleanup and exception handling.The cleanup sequence and exception handling provide a good user experience and proper resource management for the example application.
| using System.Text.Json; | ||
| using System.Text.Json.Serialization; | ||
| using System.Text.RegularExpressions; | ||
| using System.Collections.Generic; | ||
| using System.Dynamic; |
There was a problem hiding this comment.
Add missing System.Linq import.
The ToSnakeCase method uses the Select extension method which requires System.Linq.
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.RegularExpressions;
using System.Collections.Generic;
using System.Dynamic;
+using System.Linq;🤖 Prompt for AI Agents
In Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs at the top of the file
(lines 1 to 5), add the missing `using System.Linq;` directive to enable the use
of the `Select` extension method in the `ToSnakeCase` method. This will resolve
the compilation error related to the missing LINQ namespace.
| public override bool TryGetMember(GetMemberBinder binder, out object? result) | ||
| { | ||
| var name = binder.Name; | ||
| if (AdditionalProperties != null && AdditionalProperties.TryGetValue(name, out var value)) | ||
| { | ||
| result = value.ValueKind switch | ||
| { | ||
| JsonValueKind.String => value.GetString(), | ||
| JsonValueKind.Number => value.GetDouble(), | ||
| JsonValueKind.True => true, | ||
| JsonValueKind.False => false, | ||
| _ => value.ToString() | ||
| }; | ||
| return true; | ||
| } | ||
| result = null; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Fix property name inconsistency and add null handling.
The TryGetMember method doesn't convert property names to snake_case, but TrySetMember does. This creates an inconsistency when accessing dynamically set properties. Additionally, GetString() can return null which should be handled.
public override bool TryGetMember(GetMemberBinder binder, out object? result)
{
- var name = binder.Name;
+ var name = ToSnakeCase(binder.Name);
if (AdditionalProperties != null && AdditionalProperties.TryGetValue(name, out var value))
{
result = value.ValueKind switch
{
- JsonValueKind.String => value.GetString(),
+ JsonValueKind.String => value.GetString() ?? string.Empty,
JsonValueKind.Number => value.GetDouble(),
JsonValueKind.True => true,
JsonValueKind.False => false,
_ => value.ToString()
};
return true;
}
result = null;
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public override bool TryGetMember(GetMemberBinder binder, out object? result) | |
| { | |
| var name = binder.Name; | |
| if (AdditionalProperties != null && AdditionalProperties.TryGetValue(name, out var value)) | |
| { | |
| result = value.ValueKind switch | |
| { | |
| JsonValueKind.String => value.GetString(), | |
| JsonValueKind.Number => value.GetDouble(), | |
| JsonValueKind.True => true, | |
| JsonValueKind.False => false, | |
| _ => value.ToString() | |
| }; | |
| return true; | |
| } | |
| result = null; | |
| return false; | |
| } | |
| public override bool TryGetMember(GetMemberBinder binder, out object? result) | |
| { | |
| var name = ToSnakeCase(binder.Name); | |
| if (AdditionalProperties != null && AdditionalProperties.TryGetValue(name, out var value)) | |
| { | |
| result = value.ValueKind switch | |
| { | |
| JsonValueKind.String => value.GetString() ?? string.Empty, | |
| JsonValueKind.Number => value.GetDouble(), | |
| JsonValueKind.True => true, | |
| JsonValueKind.False => false, | |
| _ => value.ToString() | |
| }; | |
| return true; | |
| } | |
| result = null; | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs around lines 59 to 76,
the TryGetMember method does not convert property names to snake_case like
TrySetMember does, causing inconsistency in property access. Also, GetString()
can return null and should be handled safely. To fix this, convert binder.Name
to snake_case before using it to access AdditionalProperties, and ensure that
when GetString() returns null, result is assigned a safe default such as an
empty string or null as appropriate.
| /// <summary> | ||
| /// The provider for the TTS. | ||
| /// Must be "deepgram". | ||
| /// </summary> | ||
| [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] | ||
| [JsonPropertyName("provider")] | ||
| public dynamic Provider { get; set; } = new SpeakProvider(); |
There was a problem hiding this comment.
Update the outdated XML documentation.
The comment states the provider "Must be 'deepgram'", but the SpeakProvider class supports multiple providers including "deepgram", "eleven_labs", "cartesia", and "open_ai".
/// <summary>
/// The provider for the TTS.
- /// Must be "deepgram".
+ /// Supports "deepgram", "eleven_labs", "cartesia", "open_ai", or custom providers with arbitrary properties.
/// </summary>🤖 Prompt for AI Agents
In Deepgram/Models/Agent/v2/WebSocket/Speak.cs between lines 9 and 15, update
the XML documentation comment for the Provider property to reflect that it
supports multiple providers such as "deepgram", "eleven_labs", "cartesia", and
"open_ai" instead of stating it must be "deepgram". Modify the summary to
accurately describe the range of supported providers.
|
|
||
| // Set "DEEPGRAM_API_KEY" environment variable to your Deepgram API Key | ||
| DeepgramWsClientOptions options = new DeepgramWsClientOptions(null, null, true); | ||
| var agentClient = ClientFactory.CreateAgentWebSocketClient(apiKey: "", options: options); |
There was a problem hiding this comment.
Fix the empty API key configuration.
The empty API key will cause connection failures. For an example, consider using environment variable fallback or clear documentation about setting the API key.
Apply this diff to improve API key handling:
- var agentClient = ClientFactory.CreateAgentWebSocketClient(apiKey: "", options: options);
+ var agentClient = ClientFactory.CreateAgentWebSocketClient(apiKey: Environment.GetEnvironmentVariable("DEEPGRAM_API_KEY") ?? "", options: options);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var agentClient = ClientFactory.CreateAgentWebSocketClient(apiKey: "", options: options); | |
| // Load the API key from an environment variable (falls back to empty if not set) | |
| - var agentClient = ClientFactory.CreateAgentWebSocketClient(apiKey: "", options: options); | |
| + var agentClient = ClientFactory.CreateAgentWebSocketClient( | |
| + apiKey: Environment.GetEnvironmentVariable("DEEPGRAM_API_KEY") ?? "", | |
| + options: options | |
| + ); |
🤖 Prompt for AI Agents
In examples/agent/websocket/arbitrary_keys/Program.cs at line 29, the API key is
set to an empty string, which will cause connection failures. Update the code to
retrieve the API key from an environment variable or another secure source, and
add a check to ensure the API key is not empty before creating the agent client.
Include clear comments or documentation to guide users on how to set the API key
properly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs (2)
1-5: Add missingSystem.Linqimport.
24-41: Fix property name inconsistency and add null handling.
🧹 Nitpick comments (1)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs (1)
55-63: Consider optimizing ToSnakeCase for better performance.The current implementation creates multiple string allocations. For better performance, especially with longer property names, consider using
StringBuilder.private static string ToSnakeCase(string name) { - return string.Concat( - name.Select((x, i) => - i > 0 && char.IsUpper(x) ? "_" + char.ToLower(x) : char.ToLower(x).ToString() - ) - ); + var builder = new System.Text.StringBuilder(name.Length * 2); + for (int i = 0; i < name.Length; i++) + { + var c = name[i]; + if (i > 0 && char.IsUpper(c)) + { + builder.Append('_'); + } + builder.Append(char.ToLower(c)); + } + return builder.ToString(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs (2)
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.
🔇 Additional comments (2)
Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs (2)
9-18: LGTM!The class structure correctly inherits from
DynamicObjectto support arbitrary properties, and theTypeproperty implementation aligns with the established pattern in the codebase.
65-69:❌ Incorrect review comment
Verify JsonSerializeOptions.DefaultOptions and remove unnecessary Regex.Unescape.
- The
Regex.Unescapecall appears unnecessary for JSON serialization output- Need to verify that
JsonSerializeOptions.DefaultOptionsis defined in the codebaseIf
DefaultOptionsis not defined, consider using the default constructor:public override string ToString() { - return Regex.Unescape(JsonSerializer.Serialize(this, JsonSerializeOptions.DefaultOptions)); + return JsonSerializer.Serialize(this); }
🏁 Script executed:
#!/bin/bash # Description: Verify if JsonSerializeOptions.DefaultOptions is defined # Search for DefaultOptions definition ast-grep --pattern 'class JsonSerializeOptions { $$$ static $_ DefaultOptions $$$ $$$ }' # Also search with ripgrep as backup rg -A 3 "DefaultOptions.*JsonSerializeOptions|JsonSerializeOptions.*DefaultOptions"Length of output: 53619
Ignore removal of Regex.Unescape and DefaultOptions validation
• JsonSerializeOptions.DefaultOptions is already defined and used throughout the SDK.
• Per project convention, Regex.Unescape is intentionally applied in all ToString() overrides when serializing to JSON (e.g. MetadataResponse, UnhandledResponse).No changes required here.
Likely an incorrect or invalid review comment.
| public override bool TrySetMember(SetMemberBinder binder, object? value) | ||
| { | ||
| // Convert the property name to snake_case | ||
| var name = ToSnakeCase(binder.Name); | ||
| if (AdditionalProperties == null) | ||
| AdditionalProperties = new Dictionary<string, JsonElement>(); | ||
|
|
||
| var json = JsonSerializer.Serialize(value); | ||
| AdditionalProperties[name] = JsonDocument.Parse(json).RootElement; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Add error handling for JSON serialization.
The JsonDocument.Parse method can throw exceptions if the serialized JSON is invalid. Consider wrapping it in a try-catch block to handle potential errors gracefully.
public override bool TrySetMember(SetMemberBinder binder, object? value)
{
// Convert the property name to snake_case
var name = ToSnakeCase(binder.Name);
if (AdditionalProperties == null)
AdditionalProperties = new Dictionary<string, JsonElement>();
- var json = JsonSerializer.Serialize(value);
- AdditionalProperties[name] = JsonDocument.Parse(json).RootElement;
- return true;
+ try
+ {
+ var json = JsonSerializer.Serialize(value);
+ AdditionalProperties[name] = JsonDocument.Parse(json).RootElement;
+ return true;
+ }
+ catch
+ {
+ return false;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public override bool TrySetMember(SetMemberBinder binder, object? value) | |
| { | |
| // Convert the property name to snake_case | |
| var name = ToSnakeCase(binder.Name); | |
| if (AdditionalProperties == null) | |
| AdditionalProperties = new Dictionary<string, JsonElement>(); | |
| var json = JsonSerializer.Serialize(value); | |
| AdditionalProperties[name] = JsonDocument.Parse(json).RootElement; | |
| return true; | |
| } | |
| public override bool TrySetMember(SetMemberBinder binder, object? value) | |
| { | |
| // Convert the property name to snake_case | |
| var name = ToSnakeCase(binder.Name); | |
| if (AdditionalProperties == null) | |
| AdditionalProperties = new Dictionary<string, JsonElement>(); | |
| try | |
| { | |
| var json = JsonSerializer.Serialize(value); | |
| AdditionalProperties[name] = JsonDocument.Parse(json).RootElement; | |
| return true; | |
| } | |
| catch | |
| { | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In Deepgram/Models/Agent/v2/WebSocket/SpeakProvider.cs around lines 43 to 53,
the code uses JsonDocument.Parse without error handling, which can throw
exceptions if the JSON is invalid. Wrap the serialization and parsing logic
inside a try-catch block to catch potential exceptions, handle them gracefully
(e.g., log the error or ignore the assignment), and ensure the method still
returns a boolean indicating success or failure.
Proposed changes
Types of changes
What types of changes does your code introduce to the community .NET SDK?
Put an
xin the boxes that applyChecklist
Put an
xin 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.Further comments
Summary by CodeRabbit