-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat: support nova 3 and keyterms #363
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes enforce a model-specific validation for keyterms across various client connection methods and transcription configurations. In both WebSocket clients and REST transcription methods, the code now verifies that keyterms are only used when the model string begins with "nova-3", throwing a DeepgramException if this rule is violated. Additionally, the Listen models have been updated to require explicit specification of the model and to include a new nullable Keyterms property. Examples have been updated accordingly to configure the agent and transcription requests with the proper model and keyterms values. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant O as Options
participant WS as WebSocket Server
C->>C: Call Connect() method
C->>C: Validate options (model & keyterms)
alt Model is not "nova-3" and keyterms are present
C->>C: Throw DeepgramException("Keyterms is only supported in Nova 3 models.")
else Valid configuration
C->>WS: Initiate WebSocket connection
WS-->>C: Acknowledge connection
end
Possibly related PRs
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (1)
examples/speech-to-text/rest/url/Program.cs (1)
29-30
: Consider using constants for model names.While the changes correctly demonstrate nova-3 and keyterms usage, using string literals for model names could be fragile. Consider defining these as constants to make updates easier and prevent typos.
+ public static class DeepgramModels + { + public const string Nova3 = "nova-3"; + } - Model = "nova-3", + Model = DeepgramModels.Nova3,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
Deepgram/Clients/Agent/v2/Websocket/Client.cs
(2 hunks)Deepgram/Clients/Listen/v2/WebSocket/Client.cs
(2 hunks)Deepgram/Models/Agent/v2/WebSocket/Listen.cs
(1 hunks)Deepgram/Models/Listen/v1/REST/PreRecordedSchema.cs
(1 hunks)Deepgram/Models/Listen/v2/WebSocket/LiveSchema.cs
(1 hunks)examples/agent/websocket/simple/Program.cs
(2 hunks)examples/speech-to-text/rest/file/Program.cs
(2 hunks)examples/speech-to-text/rest/intent/Program.cs
(2 hunks)examples/speech-to-text/rest/sentiment/Program.cs
(2 hunks)examples/speech-to-text/rest/summary/Program.cs
(2 hunks)examples/speech-to-text/rest/topic/Program.cs
(2 hunks)examples/speech-to-text/rest/url/Program.cs
(2 hunks)
🧰 Additional context used
📓 Learnings (1)
Deepgram/Clients/Listen/v2/WebSocket/Client.cs (2)
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Models/Listen/v2/WebSocket/Average.cs:26-29
Timestamp: 2024-11-12T11:33:21.382Z
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#345
File: Deepgram/Models/Listen/v2/WebSocket/Average.cs:5-5
Timestamp: 2024-11-12T11:33:21.382Z
Learning: The project's codebase includes using directives such as `System.Text.Json`, `System.Text.Json.Serialization`, and `System.Text.RegularExpressions` at the top level, so it is not necessary to include them in individual files.
🔇 Additional comments (12)
Deepgram/Models/Agent/v2/WebSocket/Listen.cs (2)
11-11
: Good change: Removing default model value enforces explicit model selection.This change helps prevent accidental use of the wrong model version, especially important since keyterms are only supported with nova-3.
13-15
: LGTM: Keyterms property follows consistent JSON serialization pattern.The property is properly nullable and uses the same JsonIgnore condition as other properties.
examples/speech-to-text/rest/topic/Program.cs (1)
33-34
: Same feedback applies regarding model name constants.See previous comment about using constants for model names.
examples/speech-to-text/rest/intent/Program.cs (1)
33-34
: Same feedback applies regarding model name constants.See previous comment about using constants for model names.
examples/speech-to-text/rest/summary/Program.cs (1)
33-34
: LGTM! Good example of nova-3 with keyterms.The changes demonstrate proper usage of the nova-3 model with keyterms, using a relevant keyterm "Call Center" for the audio file being processed.
examples/speech-to-text/rest/sentiment/Program.cs (1)
33-34
: LGTM! Consistent implementation.The changes maintain consistency with other examples, using the nova-3 model and appropriate keyterm for sentiment analysis.
examples/speech-to-text/rest/file/Program.cs (1)
40-41
: LGTM! Well-adapted example.The changes demonstrate proper adaptation of keyterms to match the specific audio content, using "Bueller" for the Bueller-Life-moves-pretty-fast.wav file.
Deepgram/Models/Listen/v2/WebSocket/LiveSchema.cs (1)
120-126
: LGTM! Well-implemented property with proper documentation.The Keyterms property is implemented following C# best practices:
- Proper XML documentation with reference to official docs
- Consistent JSON serialization attributes
- Correctly implemented as nullable
examples/agent/websocket/simple/Program.cs (1)
198-199
: LGTM!The changes correctly configure the agent to use the nova-3 model and specify "Deepgram" as a keyterm.
Deepgram/Models/Listen/v1/REST/PreRecordedSchema.cs (1)
151-157
: LGTM!The Keyterms property is well-documented and properly implemented with appropriate JSON serialization attributes.
Deepgram/Clients/Listen/v2/WebSocket/Client.cs (1)
53-56
: LGTM!The validation ensures that keyterms are only used with nova-3 models, throwing a clear error message when this constraint is violated.
Deepgram/Clients/Agent/v2/Websocket/Client.cs (1)
55-58
: LGTM!The validation ensures that keyterms are only used with nova-3 models, maintaining consistency with the Listen client implementation.
Proposed changes
Types of changes
What types of changes does your code introduce to the community .NET SDK?
Put an
x
in the boxes that applyChecklist
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.Further comments
Summary by CodeRabbit