-
Notifications
You must be signed in to change notification settings - Fork 36
feat: add support for agent context history #385
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds agent history support: new history models, Settings flags/tags, WebSocket client subscribe/send/processing for History and function-call responses, unit tests, example apps demonstrating context-history, and a CODEOWNERS entry. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller Code
participant Client as AgentWebSocketClient
participant WS as WebSocket Connection
participant Subscriber as History Subscriber
Caller->>Client: SendHistoryConversationText(role, content)
Client->>Client: Validate inputs & build payload
Client->>WS: SendMessageImmediately(serialize(payload))
WS->>Client: Receive message (type=History)
Client->>Client: Deserialize HistoryResponse
Client->>Subscriber: Invoke subscribed handlers with HistoryResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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)
Deepgram/Models/Agent/v2/WebSocket/Context.cs (1)
19-19
: Consider type safety implications ofList<object>?
.While
List<object>?
provides flexibility for mixed message types (conversation text and function calls), it sacrifices compile-time type safety. However, this appears intentional given the requirement to support heterogeneous message types in the agent history feature.Consider documenting the expected object types in the XML comment for better developer experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Deepgram.Tests/UnitTests/ClientTests/AgentHistoryTests.cs
(1 hunks)Deepgram/Clients/Agent/v2/Websocket/Client.cs
(1 hunks)Deepgram/Models/Agent/v2/WebSocket/Agent.cs
(1 hunks)Deepgram/Models/Agent/v2/WebSocket/AgentType.cs
(2 hunks)Deepgram/Models/Agent/v2/WebSocket/Context.cs
(1 hunks)Deepgram/Models/Agent/v2/WebSocket/Flags.cs
(1 hunks)Deepgram/Models/Agent/v2/WebSocket/HistoryConversationText.cs
(1 hunks)Deepgram/Models/Agent/v2/WebSocket/HistoryFunctionCalls.cs
(1 hunks)Deepgram/Models/Agent/v2/WebSocket/Settings.cs
(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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.
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#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/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.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#310
File: Deepgram/ClientFactory.cs:63-72
Timestamp: 2024-06-28T17:26:13.034Z
Learning: The `CreateOnPremClient` method in `Deepgram/ClientFactory.cs` is intentionally retained to allow users time to migrate to the new `CreateSelfHostedClient` method. This strategy ensures backward compatibility and a smooth transition for users.
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#310
File: Deepgram/ClientFactory.cs:63-72
Timestamp: 2024-10-09T02:19:46.087Z
Learning: The `CreateOnPremClient` method in `Deepgram/ClientFactory.cs` is intentionally retained to allow users time to migrate to the new `CreateSelfHostedClient` method. This strategy ensures backward compatibility and a smooth transition for users.
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#347
File: Deepgram/ClientFactory.cs:146-161
Timestamp: 2024-11-01T16:17:11.630Z
Learning: In `Deepgram/ClientFactory.cs`, the method `CreateListenWebSocketClient(int version, ...)` is intentionally returning `object` as a placeholder because there is only one version of the client. It should not be suggested to change the return type until multiple versions are implemented.
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`.
Deepgram/Models/Agent/v2/WebSocket/Agent.cs (4)
Learnt from: dvonthenen
PR: #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.
Learnt from: dvonthenen
PR: #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: #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: #345
File: Deepgram/Abstractions/v1/LocalFileWithMetadata.cs:15-15
Timestamp: 2024-10-28T18:26:15.761Z
Learning: In the Deepgram/Abstractions/v1/LocalFileWithMetadata.cs
file, for the LocalFileWithMetadata
class, the user chooses to keep the Metadata
property as-is, without initializing it or adding null checks at this time.
Deepgram/Models/Agent/v2/WebSocket/AgentType.cs (3)
Learnt from: dvonthenen
PR: #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: #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: #347
File: Deepgram/ClientFactory.cs:146-161
Timestamp: 2024-11-01T16:17:11.630Z
Learning: In Deepgram/ClientFactory.cs
, the method CreateListenWebSocketClient(int version, ...)
is intentionally returning object
as a placeholder because there is only one version of the client. It should not be suggested to change the return type until multiple versions are implemented.
Deepgram/Models/Agent/v2/WebSocket/Context.cs (10)
Learnt from: dvonthenen
PR: #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: #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: #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: #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: #347
File: Deepgram/ClientFactory.cs:146-161
Timestamp: 2024-11-01T16:17:11.630Z
Learning: In Deepgram/ClientFactory.cs
, the method CreateListenWebSocketClient(int version, ...)
is intentionally returning object
as a placeholder because there is only one version of the client. It should not be suggested to change the return type until multiple versions are implemented.
Learnt from: dvonthenen
PR: #345
File: Deepgram/Models/Common/v2/WebSocket/ErrorResponse.cs:57-60
Timestamp: 2024-10-28T19:41:36.112Z
Learning: In Deepgram/Models/Common/v2/WebSocket/ErrorResponse.cs
, the ToString()
method in the ErrorResponse
class is used for debugging purposes, and exceptions are allowed to surface to the top.
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: #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#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#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.
Deepgram/Models/Agent/v2/WebSocket/Flags.cs (6)
Learnt from: dvonthenen
PR: #345
File: Deepgram/Models/Common/v2/WebSocket/ErrorResponse.cs:57-60
Timestamp: 2024-10-28T19:41:36.112Z
Learning: In Deepgram/Models/Common/v2/WebSocket/ErrorResponse.cs
, the ToString()
method in the ErrorResponse
class is used for debugging purposes, and exceptions are allowed to surface to the top.
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: #345
File: Deepgram/Clients/Listen/v2/WebSocket/Client.cs:609-609
Timestamp: 2024-10-28T19:08:03.818Z
Learning: In the file Deepgram/Clients/Listen/v2/WebSocket/Client.cs
, the direct casting of resultResponse.IsFinal
to bool
without null checking is acceptable and preferred.
Learnt from: dvonthenen
PR: #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#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#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.
Deepgram/Models/Agent/v2/WebSocket/HistoryConversationText.cs (8)
Learnt from: dvonthenen
PR: #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: #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: #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.
Learnt from: dvonthenen
PR: #345
File: Deepgram/Models/Common/v2/WebSocket/ErrorResponse.cs:57-60
Timestamp: 2024-10-28T19:41:36.112Z
Learning: In Deepgram/Models/Common/v2/WebSocket/ErrorResponse.cs
, the ToString()
method in the ErrorResponse
class is used for debugging purposes, and exceptions are allowed to surface to the top.
Learnt from: dvonthenen
PR: #347
File: Deepgram/ClientFactory.cs:146-161
Timestamp: 2024-11-01T16:17:11.630Z
Learning: In Deepgram/ClientFactory.cs
, the method CreateListenWebSocketClient(int version, ...)
is intentionally returning object
as a placeholder because there is only one version of the client. It should not be suggested to change the return type until multiple versions are implemented.
Learnt from: dvonthenen
PR: #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: #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: #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.
Deepgram/Models/Agent/v2/WebSocket/Settings.cs (4)
Learnt from: dvonthenen
PR: #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: #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: #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: #345
File: Deepgram/Clients/Interfaces/v2/IListenWebSocketClient.cs:25-70
Timestamp: 2024-10-28T18:14:28.097Z
Learning: In the file Deepgram/Clients/Interfaces/v2/IListenWebSocketClient.cs
, for the Subscribe
methods, when the parameter eventHandler
is self-explanatory, it's acceptable to omit descriptions in the XML documentation. Therefore, I'll avoid flagging missing parameter descriptions for eventHandler
in Subscribe
methods.
Deepgram/Models/Agent/v2/WebSocket/HistoryFunctionCalls.cs (12)
Learnt from: dvonthenen
PR: #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: #347
File: Deepgram/ClientFactory.cs:146-161
Timestamp: 2024-11-01T16:17:11.630Z
Learning: In Deepgram/ClientFactory.cs
, the method CreateListenWebSocketClient(int version, ...)
is intentionally returning object
as a placeholder because there is only one version of the client. It should not be suggested to change the return type until multiple versions are implemented.
Learnt from: dvonthenen
PR: #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: #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: #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: #345
File: Deepgram/Clients/Interfaces/v2/IListenWebSocketClient.cs:25-70
Timestamp: 2024-10-28T18:14:28.097Z
Learning: In the file Deepgram/Clients/Interfaces/v2/IListenWebSocketClient.cs
, for the Subscribe
methods, when the parameter eventHandler
is self-explanatory, it's acceptable to omit descriptions in the XML documentation. Therefore, I'll avoid flagging missing parameter descriptions for eventHandler
in Subscribe
methods.
Learnt from: dvonthenen
PR: #345
File: Deepgram/Models/Common/v2/WebSocket/ErrorResponse.cs:57-60
Timestamp: 2024-10-28T19:41:36.112Z
Learning: In Deepgram/Models/Common/v2/WebSocket/ErrorResponse.cs
, the ToString()
method in the ErrorResponse
class is used for debugging purposes, and exceptions are allowed to surface to the top.
Learnt from: dvonthenen
PR: #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#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: #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#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#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.
Deepgram.Tests/UnitTests/ClientTests/AgentHistoryTests.cs (4)
Learnt from: dvonthenen
PR: #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: #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: #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.
Deepgram/Clients/Agent/v2/Websocket/Client.cs (10)
Learnt from: dvonthenen
PR: #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: #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: #347
File: Deepgram/ClientFactory.cs:141-143
Timestamp: 2024-11-01T16:20:16.878Z
Learning: In Deepgram/ClientFactory.cs
, the use of ListenV1.Client
is intentional for backward compatibility and should not be suggested for replacement with ListenV2.Client
in code reviews.
Learnt from: dvonthenen
PR: #347
File: Deepgram/ClientFactory.cs:146-161
Timestamp: 2024-11-01T16:17:11.630Z
Learning: In Deepgram/ClientFactory.cs
, the method CreateListenWebSocketClient(int version, ...)
is intentionally returning object
as a placeholder because there is only one version of the client. It should not be suggested to change the return type until multiple versions are implemented.
Learnt from: dvonthenen
PR: #345
File: Deepgram/Clients/Interfaces/v2/IListenWebSocketClient.cs:25-70
Timestamp: 2024-10-28T18:14:28.097Z
Learning: In the file Deepgram/Clients/Interfaces/v2/IListenWebSocketClient.cs
, for the Subscribe
methods, when the parameter eventHandler
is self-explanatory, it's acceptable to omit descriptions in the XML documentation. Therefore, I'll avoid flagging missing parameter descriptions for eventHandler
in Subscribe
methods.
Learnt from: dvonthenen
PR: #345
File: Deepgram/Abstractions/v2/AbstractRestClient.cs:392-407
Timestamp: 2024-10-28T19:02:12.199Z
Learning: In the Deepgram/Abstractions/v2/AbstractRestClient.cs
file, the user prefers to keep the existing stream handling logic in the PostAsync<R, S, T>
method.
Learnt from: dvonthenen
PR: #345
File: Deepgram/Abstractions/v2/AbstractWebSocketClient.cs:63-71
Timestamp: 2024-10-28T18:21:03.304Z
Learning: In the Deepgram/Abstractions/v2/AbstractWebSocketClient.cs
file, within the AbstractWebSocketClient.Connect
method, when the client is already connected (_clientWebSocket
is not null
), it's acceptable to return true
to indicate that the connection is already established, rather than returning false
or throwing an exception.
Learnt from: dvonthenen
PR: #310
File: Deepgram/ClientFactory.cs:63-72
Timestamp: 2024-06-28T17:26:13.034Z
Learning: The CreateOnPremClient
method in Deepgram/ClientFactory.cs
is intentionally retained to allow users time to migrate to the new CreateSelfHostedClient
method. This strategy ensures backward compatibility and a smooth transition for users.
Learnt from: dvonthenen
PR: #310
File: Deepgram/ClientFactory.cs:63-72
Timestamp: 2024-10-09T02:19:46.087Z
Learning: The CreateOnPremClient
method in Deepgram/ClientFactory.cs
is intentionally retained to allow users time to migrate to the new CreateSelfHostedClient
method. This strategy ensures backward compatibility and a smooth transition for users.
Learnt from: dvonthenen
PR: #345
File: Deepgram/Clients/Speak/v2/WebSocket/Client.cs:363-383
Timestamp: 2024-10-28T19:14:28.631Z
Learning: In the Speak WebSocket Client, the SendMessage
method should not be made async since messages are sent via a queue.
🔇 Additional comments (10)
Deepgram/Models/Agent/v2/WebSocket/Agent.cs (1)
16-21
: LGTM! Clean integration of context history support.The addition of the
Context
property is well-implemented, following established SDK patterns with proper JSON serialization attributes, clear documentation, and nullable design for optional usage.Deepgram/Models/Agent/v2/WebSocket/AgentType.cs (2)
27-27
: LGTM! Proper enum extension for history support.The addition of the
History
enum member follows established patterns and supports the new agent history functionality.
41-41
: LGTM! Consistent constant addition.The
History
constant string properly corresponds to the enum member and maintains consistency with existing patterns.Deepgram/Models/Agent/v2/WebSocket/Settings.cs (1)
23-28
: LGTM! Well-integrated flags configuration.The addition of the
Flags
property is properly implemented with consistent JSON serialization attributes, default initialization, and clear documentation following established SDK patterns.Deepgram/Models/Agent/v2/WebSocket/Flags.cs (1)
12-28
: LGTM! Well-designed flags configuration record.The
Flags
record is properly implemented with:
- Consistent JSON serialization patterns
- Sensible default value (
true
for history reporting)- Clear documentation
- Standard
ToString()
override following SDK conventionsThe implementation integrates well with the overall agent history feature.
Deepgram/Models/Agent/v2/WebSocket/Context.cs (1)
12-28
: LGTM! Context record properly implements history storage.The
Context
record follows established SDK patterns with proper JSON serialization, clear documentation, and standardToString()
implementation. The flexible design supports the mixed message types requirement for agent history.Deepgram/Models/Agent/v2/WebSocket/HistoryConversationText.cs (1)
12-42
: Well-implemented record with proper JSON serialization.The
HistoryConversationText
record follows the established SDK patterns with appropriate JSON attributes, default values, and serialization behavior. The implementation is clean and consistent with other WebSocket model classes in the SDK.Deepgram/Models/Agent/v2/WebSocket/HistoryFunctionCalls.cs (1)
12-81
: Well-structured record types for function call history.Both
HistoryFunctionCalls
andHistoryFunctionCall
records are properly implemented with:
- Appropriate container/item relationship
- Correct JSON property naming (including snake_case for
client_side
)- Consistent serialization patterns with other SDK models
- Proper nullable property handling
The implementation follows established SDK conventions and API specification requirements.
Deepgram.Tests/UnitTests/ClientTests/AgentHistoryTests.cs (1)
16-535
: Comprehensive and well-structured test suite.This test class provides excellent coverage of the new agent history functionality with:
- Complete method coverage: Both string parameter and schema object overloads
- Thorough argument validation: Tests for null, empty, and invalid inputs with proper exception verification
- Serialization verification: JSON parsing tests confirm correct property names and values
- Proper mocking: Uses NSubstitute effectively to isolate units under test
- Clear test structure: Follows AAA pattern with descriptive test names
- Edge case coverage: Tests default values, snake_case serialization, and nested objects
The test quality is high and follows established SDK testing patterns.
Deepgram/Clients/Agent/v2/Websocket/Client.cs (1)
546-646
: Well-implemented client methods for agent history functionality.The new
SendHistoryConversationText
andSendHistoryFunctionCalls
methods are excellently implemented with:
- Consistent API design: Both string parameter and schema object overloads follow established SDK patterns
- Comprehensive validation: Proper null/empty checks with descriptive error messages and correct parameter names
- Good logging practices: Debug and warning logs provide useful troubleshooting information
- Proper serialization: Uses UTF-8 encoding and established
SendMessageImmediately
pattern- Clear documentation: XML comments properly describe parameters and functionality
The implementation maintains consistency with existing client methods while adding the new history functionality seamlessly.
I need to double checkout the approach after chatting with @lukeocodes |
40682c8
to
67733ab
Compare
I double checked this approach and it checks out, this is ready @lukeocodes |
feat: add support for agent tags
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: 2
🔭 Outside diff range comments (1)
examples/agent/websocket/simple/Program.cs (1)
27-52
: Ensure library cleanup on early returns and exceptionsThere are multiple early returns (no input device, mic init failure, connection failure, mic start exception). In those branches the Terminate calls won’t run, potentially leaving PortAudio/Deepgram libraries initialized.
Consider wrapping the main body in a try/finally that always stops/disposes the output stream (if created) and calls Deepgram.Microphone.Library.Terminate() and Deepgram.Library.Terminate(), or add cleanup before each early return. Example structure (outside selected lines):
try { // main logic... } finally { try { _outputStream?.Stop(); _outputStream?.Dispose(); _outputStream = null; } catch {} Deepgram.Microphone.Library.Terminate(); Deepgram.Library.Terminate(); }Also applies to: 169-175, 208-213
🧹 Nitpick comments (11)
Deepgram.Tests/UnitTests/ClientTests/AgentClientTests.cs (1)
392-646
: Solid Tags coverage; consider adding edge-case validations for tag itemsGreat breadth of tests validating presence/absence and serialization behavior of tags. Two optional improvements:
- Add tests for invalid tag items (null or empty strings) and decide desired behavior (reject vs. serialize nulls). Today, System.Text.Json will serialize null elements inside arrays even if the property itself is ignored when null.
- Add a deduplication or normalization test if the API expects unique tags or specific casing.
If null/empty values are not allowed by the API, we should either validate in SettingsSchema (on set) or in the client before send, and assert via unit tests.
Would you like me to add tests for null/empty tag entries and, if disallowed by the API, a small guard in SettingsSchema to reject them?
Deepgram/Models/Agent/v2/WebSocket/FunctionCallRequestResponse.cs (1)
22-27
: Additions look correct; ensure API oneOf behavior and tests cover presence/absenceThe new functions payload matches the HistoryFunctionCall type and is nullable with correct JSON naming. Two small asks:
- Verify server behavior when both the legacy fields and functions are present; ensure only the allowed variant is serialized/sent per API’s oneOf.
- Ensure unit tests cover serialization with functions present vs. omitted.
Additionally, the Type property XML doc currently says “SettingsConfiguration event type.” Consider updating to “FunctionCallRequest event type.” to avoid confusion (outside selected lines).
Deepgram/Models/Agent/v2/WebSocket/HistoryResponse.cs (2)
17-21
: MakeType
non-nullable and drop the redundant null-ignore attribute
Type
is always initialized toAgentType.History
and has no setter. Using a nullable type plusWhenWritingNull
is redundant and slightly misleading. Consider making it non-nullable and removing the ignore attribute.- [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] [JsonPropertyName("type")] [JsonConverter(typeof(JsonStringEnumConverter))] - public AgentType? Type { get; } = AgentType.History; + public AgentType Type { get; } = AgentType.History;
36-41
: Clarify oneOf semantics between conversation and function call historyThis model represents either conversation history (role/content) or function-call history (function_calls). Clarifying mutual exclusivity in the docs helps avoid ambiguous payload expectations.
- /// List of function call objects (for function call history) + /// List of function call objects (for function call history). + /// Mutually exclusive with Role/Content: when FunctionCalls is present, Role/Content will typically be null.Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs (1)
187-216
: API surface additions for History sending are consistent; verify role constraints in docsThe overloads for sending conversation text and function-call history align with existing patterns. Consider documenting accepted role values explicitly (e.g., "user" | "assistant") to guide callers.
If the backend strictly requires role ∈ {"user","assistant"}, please confirm and update the XML docs accordingly. If "system" or other roles are valid, reflect that here for clarity.
Deepgram/Clients/Agent/v2/Websocket/Client.cs (2)
565-591
: Normalize and validaterole
for schema complianceTo avoid server-side rejections and ensure payload conformance, normalize
role
to lowercase and restrict to allowed values. Also set the normalized value on the outgoing schema.public async Task SendHistoryConversationText(string role, string content) { - if (string.IsNullOrWhiteSpace(role)) + if (string.IsNullOrWhiteSpace(role)) { Log.Warning("SendHistoryConversationText", "Role cannot be null or empty"); throw new ArgumentException("Role cannot be null or empty", nameof(role)); } + var normalizedRole = role.Trim().ToLowerInvariant(); + if (normalizedRole != "user" && normalizedRole != "assistant") + { + Log.Warning("SendHistoryConversationText", $"Invalid role: {role}. Expected 'user' or 'assistant'."); + throw new ArgumentException("Role must be 'user' or 'assistant'", nameof(role)); + } + if (string.IsNullOrWhiteSpace(content)) { Log.Warning("SendHistoryConversationText", "Content cannot be null or empty"); throw new ArgumentException("Content cannot be null or empty", nameof(content)); } var historyMessage = new HistoryConversationText { - Role = role, + Role = normalizedRole, Content = content }; await SendHistoryConversationText(historyMessage); }
593-621
: Apply the same role normalization and reduce PII in debug logsMirror the role normalization for the schema overload. Also consider reducing PII exposure in debug logs by logging a truncated content preview instead of the full text.
public async Task SendHistoryConversationText(HistoryConversationText historyConversationText) { if (historyConversationText == null) { Log.Warning("SendHistoryConversationText", "HistoryConversationText cannot be null"); throw new ArgumentNullException(nameof(historyConversationText)); } if (string.IsNullOrWhiteSpace(historyConversationText.Role)) { Log.Warning("SendHistoryConversationText", "Role cannot be null or empty"); throw new ArgumentException("Role cannot be null or empty", nameof(historyConversationText.Role)); } + var normalizedRole = historyConversationText.Role.Trim().ToLowerInvariant(); + if (normalizedRole != "user" && normalizedRole != "assistant") + { + Log.Warning("SendHistoryConversationText", $"Invalid role: {historyConversationText.Role}. Expected 'user' or 'assistant'."); + throw new ArgumentException("Role must be 'user' or 'assistant'", nameof(historyConversationText.Role)); + } + historyConversationText.Role = normalizedRole; + if (string.IsNullOrWhiteSpace(historyConversationText.Content)) { Log.Warning("SendHistoryConversationText", "Content cannot be null or empty"); throw new ArgumentException("Content cannot be null or empty", nameof(historyConversationText.Content)); } - Log.Debug("SendHistoryConversationText", $"Sending History Conversation Text: {historyConversationText.Role} - {historyConversationText.Content}"); + var preview = historyConversationText.Content.Length > 100 + ? historyConversationText.Content.Substring(0, 100) + "…" + : historyConversationText.Content; + Log.Debug("SendHistoryConversationText", $"Sending History Conversation Text: role={historyConversationText.Role}, content_preview=\"{preview}\""); byte[] data = Encoding.UTF8.GetBytes(historyConversationText.ToString()); await SendMessageImmediately(data); }examples/agent/websocket/context-history/Program.cs (4)
369-370
: Align message with mock weather implementationThe sample uses mock weather data; avoid implying live data.
- Console.WriteLine("🔧 Function calling is enabled - I can fetch real weather data!"); + Console.WriteLine("🔧 Function calling is enabled - returning sample weather data!");
548-565
: Potential unbounded audio queue growth
audioQueue
can grow if playback lags behind the producer, leading to memory pressure. Consider a max size with drop-oldest behavior.Example approach (outside selected lines):
- Keep a constant MAX_QUEUE_BYTES or MAX_QUEUE_CHUNKS.
- Before enqueue, if over limit, dequeue until under limit or discard incoming chunk with a warning in verbose logs.
617-663
: Reduce per-callback allocations in OutputCallbackAllocating a new buffer every callback (
new byte[bytesToWrite]
) increases GC churn. Consider reusing a preallocated buffer sized to the maximum expected callback size or rent from ArrayPool.Example (outside selected lines):
- Use
ArrayPool<byte>.Shared.Rent(bytesToWrite)
andReturn
afterMarshal.Copy
.- Or maintain a static reusable buffer grown with
Array.Resize
when needed, guarded byaudioLock
.
417-423
: Parsing arguments viaDictionary<string, object>
is brittle
JsonSerializer.Deserialize<Dictionary<string, object>>
producesJsonElement
values, and.ToString()
works for simple cases but can be fragile for nested objects. Consider deserializing to a small typed DTO or usingJsonNode
for more robust parsing.Example (outside selected lines):
- Define
record WeatherArgs(string location, string unit = "fahrenheit");
var args = JsonSerializer.Deserialize<WeatherArgs>(argumentsJson);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/CODEOWNERS
(1 hunks)Deepgram.Tests/UnitTests/ClientTests/AgentClientTests.cs
(1 hunks)Deepgram/Clients/Agent/v2/Websocket/Client.cs
(4 hunks)Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs
(2 hunks)Deepgram/Models/Agent/v2/WebSocket/FunctionCallRequestResponse.cs
(2 hunks)Deepgram/Models/Agent/v2/WebSocket/FunctionCallResponseSchema.cs
(2 hunks)Deepgram/Models/Agent/v2/WebSocket/HistoryResponse.cs
(1 hunks)Deepgram/Models/Agent/v2/WebSocket/Settings.cs
(1 hunks)examples/agent/websocket/context-history/ContextHistory.csproj
(1 hunks)examples/agent/websocket/context-history/Program.cs
(1 hunks)examples/agent/websocket/simple/Program.cs
(7 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/CODEOWNERS
🚧 Files skipped from review as they are similar to previous changes (1)
- Deepgram/Models/Agent/v2/WebSocket/Settings.cs
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2024-11-05T22:01:40.632Z
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.
Applied to files:
examples/agent/websocket/context-history/ContextHistory.csproj
examples/agent/websocket/simple/Program.cs
📚 Learning: 2024-07-29T07:02:00.905Z
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.
Applied to files:
examples/agent/websocket/context-history/ContextHistory.csproj
examples/agent/websocket/simple/Program.cs
📚 Learning: 2024-07-29T07:02:00.905Z
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.
Applied to files:
examples/agent/websocket/context-history/ContextHistory.csproj
examples/agent/websocket/simple/Program.cs
📚 Learning: 2024-10-28T18:14:28.097Z
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#345
File: Deepgram/Clients/Interfaces/v2/IListenWebSocketClient.cs:25-70
Timestamp: 2024-10-28T18:14:28.097Z
Learning: In the file `Deepgram/Clients/Interfaces/v2/IListenWebSocketClient.cs`, for the `Subscribe` methods, when the parameter `eventHandler` is self-explanatory, it's acceptable to omit descriptions in the XML documentation. Therefore, I'll avoid flagging missing parameter descriptions for `eventHandler` in `Subscribe` methods.
Applied to files:
Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs
Deepgram/Clients/Agent/v2/Websocket/Client.cs
📚 Learning: 2024-10-28T18:22:59.455Z
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.
Applied to files:
Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs
examples/agent/websocket/simple/Program.cs
📚 Learning: 2024-10-09T02:19:46.086Z
Learnt from: dvonthenen
PR: deepgram/deepgram-dotnet-sdk#316
File: Deepgram/Clients/Speak/v1/WebSocket/v1/Client.cs:0-0
Timestamp: 2024-10-09T02:19:46.086Z
Learning: The event subscription methods in the Deepgram .NET SDK cannot be abstracted into a common method due to concrete type requirements for each event handler.
Applied to files:
Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs
Deepgram/Clients/Agent/v2/Websocket/Client.cs
📚 Learning: 2024-10-23T17:23:51.386Z
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.
Applied to files:
examples/agent/websocket/simple/Program.cs
examples/agent/websocket/context-history/Program.cs
🧬 Code Graph Analysis (5)
Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs (2)
Deepgram/Clients/Agent/v2/Websocket/Client.cs (16)
Task
(56-142)Task
(150-167)Task
(174-186)Task
(193-205)Task
(211-223)Task
(229-241)Task
(247-259)Task
(265-277)Task
(283-295)Task
(301-313)Task
(319-331)Task
(337-349)Task
(355-367)Task
(373-385)Task
(391-403)Task
(410-426)Deepgram/Models/Agent/v2/WebSocket/FunctionCallResponseSchema.cs (1)
FunctionCallResponseSchema
(12-40)
Deepgram/Models/Agent/v2/WebSocket/HistoryResponse.cs (3)
Deepgram/Models/Agent/v2/WebSocket/FunctionCallRequestResponse.cs (1)
ToString
(32-35)Deepgram/Models/Agent/v2/WebSocket/FunctionCallResponseSchema.cs (1)
ToString
(36-39)Deepgram/Utilities/JsonSerializeOptions.cs (1)
JsonSerializeOptions
(7-13)
Deepgram.Tests/UnitTests/ClientTests/AgentClientTests.cs (3)
Deepgram/Models/Agent/v2/WebSocket/Settings.cs (2)
SettingsSchema
(7-57)ToString
(53-56)Deepgram/Models/Agent/v2/WebSocket/Agent.cs (1)
ToString
(45-48)Deepgram/Models/Agent/v2/WebSocket/InjectUserMessageSchema.cs (1)
ToString
(26-29)
examples/agent/websocket/context-history/Program.cs (7)
Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs (17)
Task
(18-19)Task
(24-24)Task
(33-33)Task
(40-40)Task
(47-47)Task
(53-54)Task
(59-59)Task
(65-65)Task
(71-71)Task
(77-77)Task
(83-83)Task
(89-89)Task
(95-95)Task
(101-101)Task
(107-107)Task
(113-113)SendBinary
(162-162)Deepgram/ClientFactory.cs (2)
ClientFactory
(16-301)CreateAgentWebSocketClient
(138-141)Deepgram/Models/Agent/v2/WebSocket/Settings.cs (1)
SettingsSchema
(7-57)Deepgram/Models/Agent/v2/WebSocket/Provider.cs (1)
Provider
(9-68)Deepgram/Models/Agent/v2/WebSocket/FunctionCallRequestResponse.cs (1)
ToString
(32-35)Deepgram/Models/Agent/v2/WebSocket/FunctionCallResponseSchema.cs (2)
ToString
(36-39)FunctionCallResponseSchema
(12-40)Deepgram/Models/Agent/v2/WebSocket/HistoryResponse.cs (1)
ToString
(46-49)
Deepgram/Clients/Agent/v2/Websocket/Client.cs (3)
Deepgram/Models/Agent/v2/WebSocket/FunctionCallResponseSchema.cs (2)
ToString
(36-39)FunctionCallResponseSchema
(12-40)Deepgram/Models/Agent/v2/WebSocket/HistoryConversationText.cs (1)
ToString
(38-41)Deepgram/Models/Agent/v2/WebSocket/HistoryFunctionCalls.cs (2)
ToString
(31-34)ToString
(77-80)
🔇 Additional comments (12)
examples/agent/websocket/context-history/ContextHistory.csproj (1)
1-19
: LGTM: example project setup is clean and consistent with repo patterns
- Targeting net8.0 for examples, implicit usings, nullable enabled, and local project references are all aligned with established example conventions.
examples/agent/websocket/simple/Program.cs (3)
181-202
: Confirm SendBinary call in microphone callback is thread-safe and non-blockingThe push_callback likely runs on a PortAudio thread. If SendBinary is async, blocking, or not thread-safe, this can drop audio or deadlock. Safer approach: enqueue audio into a concurrent buffer and have a dedicated Task draining it to SendBinary.
Can you confirm SendBinary is non-blocking and thread-safe? If not, I can provide a Channel-based sender loop to decouple capture from send.
146-152
: Audio IO formats align; good choice for real-time pathUsing linear16 at 24kHz for both input and output, with no container, simplifies real-time streaming and is consistent with the playback pipeline configuration.
56-58
: Don’t hardcode the API key; read from environment and validateUsing an empty string will cause auth failures and is easy to forget to change. Prefer env var retrieval with a clear error if missing.
Apply this diff:
- DeepgramWsClientOptions options = new DeepgramWsClientOptions(null, null, true); - var agentClient = ClientFactory.CreateAgentWebSocketClient(apiKey: "", options: options); + var apiKey = Environment.GetEnvironmentVariable("DEEPGRAM_API_KEY"); + if (string.IsNullOrWhiteSpace(apiKey)) + { + Console.WriteLine("Error: DEEPGRAM_API_KEY environment variable is not set."); + return; + } + DeepgramWsClientOptions options = new DeepgramWsClientOptions(null, null, true); + var agentClient = ClientFactory.CreateAgentWebSocketClient(apiKey: apiKey, options: options);⛔ Skipped due to learnings
Learnt from: dvonthenen PR: deepgram/deepgram-dotnet-sdk#338 File: tests/edge_cases/reconnect_same_object/Program.cs:0-0 Timestamp: 2024-09-30T17:08:25.314Z Learning: When using the Deepgram client, setting the `apiKey` parameter to an empty string will automatically retrieve the API key from the `DEEPGRAM_API_KEY` environment variable.
Learnt from: dvonthenen PR: deepgram/deepgram-dotnet-sdk#338 File: tests/edge_cases/reconnect_same_object/Program.cs:0-0 Timestamp: 2024-10-09T02:19:46.086Z Learning: When using the Deepgram client, setting the `apiKey` parameter to an empty string will automatically retrieve the API key from the `DEEPGRAM_API_KEY` environment variable.
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#245 File: examples/speak/Program.cs:11-11 Timestamp: 2024-07-29T07:02:00.905Z Learning: The user `dvonthenen` agreed to add a comment clarifying that `"REPLACE-WITH-YOUR-API-KEY"` is a placeholder for the user's actual Deepgram API key in `examples/speak/Program.cs`.
Deepgram/Models/Agent/v2/WebSocket/FunctionCallResponseSchema.cs (1)
21-31
: Schema rename to id/name/content acknowledged; verify id requirementThe shift from function_call_id/output to id/name/content aligns with the new contract. Since id is critical for correlating responses, confirm whether it’s always present. If required by the API, drop the WhenWritingNull and enforce non-null.
If the API guarantees id in responses, change:
- Remove JsonIgnore(WhenWritingNull) for Id, and possibly make it non-nullable.
- Add a simple guard/validation path if we ever send this schema back (not shown here).
Minor doc nit (outside selected lines): the summary says “SettingsConfiguration event type.” Consider “FunctionCallResponse event type.” to match semantics.
Deepgram/Models/Agent/v2/WebSocket/HistoryResponse.cs (1)
46-49
: LGTM on ToString overrideConsistent with other models using JsonSerializeOptions.DefaultOptions and Regex.Unescape. No issues spotted.
Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs (1)
127-131
: History event subscription addition looks goodThe new Subscribe(EventHandler) matches existing patterns and naming. No concerns.
Deepgram/Clients/Agent/v2/Websocket/Client.cs (5)
48-48
: New History event handler field is consistentPrivate event definition for History aligns with the rest of the event fields.
387-403
: History Subscribe implementation follows thread-safety patternUsing _mutexSubscribe and appending the handler mirrors existing subscriptions. Looks correct.
623-665
: LGTM on sending function-call historyValidation that the list is non-null/non-empty, wrapping, and serialization are solid. Logging only the count avoids leaking arguments/responses.
666-688
: Validate FunctionCallResponseName
only if required by APICurrently only Id is required. If the API expects Name to be present for correlation/telemetry, add a check and a warning to aid debugging.
If Name is required, add:
+ if (string.IsNullOrWhiteSpace(functionCallResponse.Name)) + { + Log.Warning("SendFunctionCallResponse", "Name cannot be null or empty"); + throw new ArgumentException("Name cannot be null or empty", nameof(functionCallResponse.Name)); + }
996-1013
: History message dispatching is correctDeserialization, listener guard, logging, and dispatch are all consistent with other event types.
@lukeocodes this is done, and now there is a context history example to test with. |
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 (5)
examples/agent/websocket/context-history/Program.cs (5)
20-27
: Consider using a more realistic temperature format in the mock data.The temperatures in the mock data dictionary inconsistently mix actual Fahrenheit values with converted ones (note the comments about °C to °F conversions). For better clarity, either use consistent Fahrenheit values or document the original Celsius values more clearly.
private static readonly Dictionary<string, WeatherInfo> WeatherData = new() { ["new york"] = new() { Temperature = 72, Condition = "sunny", Humidity = 45 }, - ["london"] = new() { Temperature = 64, Condition = "cloudy", Humidity = 80 }, // 18°C to °F - ["tokyo"] = new() { Temperature = 77, Condition = "rainy", Humidity = 90 }, // 25°C to °F - ["paris"] = new() { Temperature = 68, Condition = "partly cloudy", Humidity = 60 }, // 20°C to °F - ["sydney"] = new() { Temperature = 82, Condition = "sunny", Humidity = 50 }, // 28°C to °F + ["london"] = new() { Temperature = 64, Condition = "cloudy", Humidity = 80 }, // ~18°C + ["tokyo"] = new() { Temperature = 77, Condition = "rainy", Humidity = 90 }, // ~25°C + ["paris"] = new() { Temperature = 68, Condition = "partly cloudy", Humidity = 60 }, // ~20°C + ["sydney"] = new() { Temperature = 82, Condition = "sunny", Humidity = 50 }, // ~28°C };
163-187
: Consider adding await to the Task.Run call.The fire-and-forget pattern using
Task.Run
without await could potentially hide exceptions. Consider awaiting the task or at least logging any exceptions that might occur.// Handle the weather function call if (functionCall.Name == "get_weather") { - _ = Task.Run(() => HandleWeatherFunctionCall(agentClient, functionCall)); + _ = Task.Run(async () => + { + try + { + await HandleWeatherFunctionCall(agentClient, functionCall); + } + catch (Exception ex) + { + Console.WriteLine($"❌ Unhandled exception in weather function call: {ex.Message}"); + } + }); }
346-369
: Consider making the audio buffer size configurable.The hardcoded chunk size of 8192 might not be optimal for all scenarios. Consider making it configurable or using a standard size that aligns with typical audio frame boundaries.
+ const int AUDIO_CHUNK_SIZE = 8192; // Standard chunk size for 16kHz mono audio // Create microphone with proper sample rate and debugging microphone = new Microphone( push_callback: (audioData, length) => { // ... existing callback code ... }, rate: 16000, // Match the agent's expected input rate (16kHz) - chunkSize: 8192, // Standard chunk size + chunkSize: AUDIO_CHUNK_SIZE, channels: 1, // Mono device_index: PortAudio.DefaultInputDevice, format: SampleFormat.Int16 );
394-399
: Add null check before disposing the output stream.While you check for null, consider using the null-conditional operator for cleaner code.
// Stop and dispose PortAudio output stream - if (_outputStream != null) - { - _outputStream.Stop(); - _outputStream.Dispose(); - _outputStream = null; - } + _outputStream?.Stop(); + _outputStream?.Dispose(); + _outputStream = null;
511-539
: Remove unused methodGenerateMockWeatherResponse
.This method appears to be unused in the current implementation since
GetWeather
handles all weather data generation. Consider removing it to reduce code clutter.Since this method duplicates functionality already present in
GetWeather
and is not referenced anywhere in the code, it should be removed for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/agent/websocket/context-history/Program.cs
(1 hunks)examples/agent/websocket/simple/Program.cs
(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/agent/websocket/simple/Program.cs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-23T17:23:51.386Z
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.
Applied to files:
examples/agent/websocket/context-history/Program.cs
🧬 Code Graph Analysis (1)
examples/agent/websocket/context-history/Program.cs (8)
Deepgram/Clients/Interfaces/v2/IAgentWebSocketClient.cs (15)
Task
(18-19)Task
(24-24)Task
(33-33)Task
(40-40)Task
(47-47)Task
(53-54)Task
(59-59)Task
(65-65)Task
(71-71)Task
(77-77)Task
(83-83)Task
(89-89)Task
(95-95)Task
(101-101)Task
(107-107)Deepgram/Logger/Log.cs (1)
Information
(91-94)Deepgram/ClientFactory.cs (2)
ClientFactory
(16-301)CreateAgentWebSocketClient
(138-141)Deepgram/Models/Agent/v2/WebSocket/Settings.cs (1)
SettingsSchema
(7-57)Deepgram/Models/Agent/v2/WebSocket/Provider.cs (1)
Provider
(9-68)Deepgram/Models/Agent/v2/WebSocket/FunctionCallRequestResponse.cs (1)
ToString
(32-35)Deepgram/Models/Agent/v2/WebSocket/FunctionCallResponseSchema.cs (2)
ToString
(36-39)FunctionCallResponseSchema
(12-40)Deepgram/Models/Agent/v2/WebSocket/HistoryResponse.cs (1)
ToString
(46-49)
🔇 Additional comments (7)
examples/agent/websocket/context-history/Program.cs (7)
1-14
: LGTM! File headers and imports look good.The copyright notice, license information, and using statements are properly structured. The imports cover all necessary functionality for WebSocket communication, audio processing, and JSON serialization.
107-125
: Well-documented example structure for function call history!The
functionCallHistoryExample
variable effectively demonstrates the data structure developers need for implementing function call history in production applications. The clear documentation and commented implementation guidance are excellent for educational purposes.
279-283
: Good implementation of conversation context!The conversation context is properly configured using the new
Context
andMessages
properties, correctly demonstrating how to provide initial conversation history to the agent.
414-460
: Robust error handling in weather function call handler!The implementation properly handles exceptions and sends appropriate error responses back to the agent, ensuring the conversation can continue even when function calls fail.
489-493
: Temperature conversion looks correct.The Fahrenheit to Celsius conversion formula is accurate and properly implemented.
542-546
: Thread-safe audio queue implementation!Good use of locking mechanism to ensure thread safety when accessing the audio queue from multiple threads.
620-666
: Well-implemented audio output callback with proper buffer management!The callback correctly handles buffer exhaustion, maintains continuous playback, and properly manages the transition between audio chunks. The decision to keep the stream running (line 647) is good for reducing latency in conversational scenarios.
🤖 Add Agent Function Call Context / History Feature
TL;DR
Implemented the Agent Function Call Context / History feature for the Deepgram .NET SDK, enabling agents to maintain conversation context and function call history. This includes new models for history messages, settings integration, client methods, and comprehensive testing.
What Changed
🔧 Core Models Added
Flags.cs
- Agent settings flags with history toggleContext.cs
- Conversation context storage for mixed message typesHistoryConversationText.cs
- User/assistant chat message historyHistoryFunctionCalls.cs
- Function call history with arguments and responses🏗️ Integration Updates
Settings.cs
- AddedFlags
property to settings schemaAgent.cs
- AddedContext
property for conversation historyAgentType.cs
- AddedHistory
enum value and constantClient.cs
- AddedSendHistoryConversationText()
andSendHistoryFunctionCalls()
methodsTesting
📊 Test Coverage
🧪 Test Categories
API Specification Compliance
✅ Full Compliance Achieved
history: boolean
with defaulttrue
messages: array
supporting mixed message typestype: "History"
identifierarguments: type: string
)oneOf
specification requirements🔍 Validated Against Spec
role
andcontent
function_calls
array structureUsage Example
Basic Agent Configuration with History
Sending History Messages
Files Modified
🆕 New Files
Deepgram/Models/Agent/v2/WebSocket/Flags.cs
Deepgram/Models/Agent/v2/WebSocket/Context.cs
Deepgram/Models/Agent/v2/WebSocket/HistoryConversationText.cs
Deepgram/Models/Agent/v2/WebSocket/HistoryFunctionCalls.cs
Deepgram.Tests/UnitTests/ClientTests/AgentHistoryTests.cs
📝 Modified Files
Deepgram/Models/Agent/v2/WebSocket/Settings.cs
- Added Flags propertyDeepgram/Models/Agent/v2/WebSocket/Agent.cs
- Added Context propertyDeepgram/Models/Agent/v2/WebSocket/AgentType.cs
- Added History enum/constantDeepgram/Clients/Agent/v2/Websocket/Client.cs
- Added send methodsQuality Assurance
🔒 Code Quality
📋 Validation Checklist
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
New Features
Bug Fixes
Tests
Chores