Skip to content
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

Fix Streaming/REST Functionality and Introduce Microphone Example #253

Merged

Conversation

davidvonthenen
Copy link
Contributor

@davidvonthenen davidvonthenen commented Mar 28, 2024

Fixes: #222, #194, and #180

Changes:

  • Fixes breaking all functionality on alpha.4 because of API Key issue. Tested Speak, Prerecorded and Live using Microphone.
  • Introduces a Microphone example using portaudio
  • Introduced Deepgram.Microphone and Deepgram.Microphone.Unstable builds on nuget
  • Removed the LiveClientTests as none of the tests make sense anymore. WIll need to come up with new ones.

Summary by CodeRabbit

  • New Features
    • Introduced a new Deepgram Microphone project for audio input handling.
    • Implemented functionality for real-time transcription from microphone audio streams.
  • Enhancements
    • Updated API key handling across various clients to improve security and usability.
    • Improved audio processing with the initialization and termination of PortAudio.
    • Enhanced project configurations and paths for better organization and clarity.
  • Bug Fixes
    • Fixed nullable types in models to ensure required values are provided, enhancing data integrity.
  • Documentation
    • Updated project references and environment variable handling in example projects for a more streamlined setup.
  • Chores
    • Added clean-up scripts for environment maintenance.
  • Refactor
    • Renamed projects for better alignment with their functionality.
    • Updated constructors and method signatures across clients for consistency and simplicity.

Copy link
Contributor

coderabbitai bot commented Mar 28, 2024

Walkthrough

This update encompasses a significant restructuring and enhancement of the Deepgram development environment, including renaming projects for clarity, adding numerous new projects to bolster the SDK's capabilities, and refining project paths and configurations for better organization and usability. Notably, it introduces the Deepgram.Microphone project, aimed at streamlining audio input handling. Furthermore, adjustments in unit tests and project settings reflect these broad changes, ensuring the SDK remains robust and well-documented.

Changes

Files Change Summary
Deepgram.Dev.sln, Deepgram.DevBuild.sln, Deepgram.sln Added new projects and updated project paths and configurations.
Deepgram.Microphone/* Introduced Deepgram.Microphone project with classes for audio input handling using PortAudio.
Deepgram.Tests/* Updated unit tests reflecting changes in project structure and configurations.
Deepgram/* Major updates across various clients and models, including default parameter adjustments and API key handling.
clean-up.ps1, clean-up.sh Added scripts for cleaning up project directories.
examples/* Updated example projects to align with the SDK's structural changes and new features.

Assessment against linked issues

Objective Addressed Explanation
Unit Tests for UtteranceEnd and SpeechStarted (#222) While the summary indicates substantial changes across the project, including updates to unit tests, there's no explicit mention of tests for UtteranceEnd and SpeechStarted being added or updated. This makes it unclear if the specific objective of issue #222 has been directly addressed.

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and aa6ff4d.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav
Files selected for processing (43)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (5 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (1 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (5 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Learnings (1)
examples/speak/Program.cs (1)
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`.
Files skipped from review due to trivial changes (4)
  • Deepgram.Microphone/Constants.cs
  • Deepgram/Deepgram.csproj
  • clean-up.sh
  • examples/speak/file/hello-world/Speak.csproj
Additional comments (66)
Deepgram.Microphone/GlobalUsings.cs (1)
  • 5-5: The addition of PortAudioSharp to global usings is a good practice for simplifying namespace management across the project, especially given the focus on introducing microphone functionality. This change is clear and well-implemented.
Deepgram.Microphone/Library.cs (1)
  • 15-15: Similar to the Initialize method, consider implementing a more robust logging framework for the Terminate method to enhance maintainability and control over logging output.
Deepgram/Models/Live/v1/Channel.cs (1)
  • 13-13: Changing the Alternatives property to be non-nullable is a positive change that enhances the robustness of the model by ensuring that the property is always provided, aligning with the PR's objectives.
examples/prerecorded/file/Prerecorded.csproj (2)
  • 15-15: Updating the ProjectReference path correctly reflects the reorganized project structure, ensuring that dependencies are accurately resolved.
  • 19-19: The addition of a Using element for Deepgram is a good practice for making dependencies explicit and simplifying namespace usage within the project.
examples/streaming/file/Streaming.csproj (1)
  • 11-11: Correctly updating the ProjectReference path is essential for accurately resolving dependencies following the project's reorganization. This change is well-implemented.
Deepgram/LiveClient.cs (1)
  • 15-15: Allowing an empty string as a default value for the apiKey parameter is a positive change that enhances flexibility in client instantiation and aligns with the PR's objectives to improve API key handling.
Deepgram/SpeakClient.cs (1)
  • 15-15: Making the apiKey parameter optional by allowing an empty string as a default value in the SpeakClient constructor enhances flexibility and aligns with the PR's objectives to improve API key handling.
Deepgram/ManageClient.cs (1)
  • 15-15: Consistent with changes in other client constructors, making the apiKey parameter optional in the ManageClient constructor enhances flexibility and aligns with the PR's objectives to improve API key handling.
Deepgram/OnPremClient.cs (1)
  • 15-15: The change to allow an empty string as the default value for the apiKey parameter in the OnPremClient constructor is noted. It's important to ensure that there's proper handling or documentation for cases where the API key is not provided to avoid unintended behavior.
Deepgram/Constants/Defaults.cs (1)
  • 13-13: Adding a constant for the default API version (DEFAULT_API_VERSION) is a good practice for maintainability and clarity. This centralizes the version information, making future updates easier.
Deepgram/AnalyzeClient.cs (1)
  • 15-15: The change to allow an empty string as the default value for the apiKey parameter in the AnalyzeClient constructor aligns with the updates in other client classes. Ensure there's proper handling or documentation for cases where the API key is not provided.
Deepgram/PreRecordedClient.cs (1)
  • 15-15: The change to allow an empty string as the default value for the apiKey parameter in the PreRecordedClient constructor is consistent with the updates in other client classes. Ensure there's proper handling or documentation for cases where the API key is not provided.
examples/streaming/microphone/Streaming.csproj (1)
  • 11-13: The addition of project references to Deepgram.Microphone, Deepgram, and Microphone in the microphone streaming example's project file is necessary for the example to function correctly. This supports the effort to provide practical examples of the SDK's features.
clean-up.ps1 (1)
  • 5-16: The inclusion of cleanup commands for .vs, dist, obj, and bin directories in the clean-up.ps1 script is a good practice for maintaining a clean development environment. This helps prevent potential build or runtime issues caused by stale files.
Deepgram/Models/Live/v1/ModelInfo.cs (1)
  • 13-13: Making the Arch, Name, and Version properties non-nullable in the ModelInfo class is a good practice for ensuring data integrity and avoiding null reference errors. This aligns with the principle of making illegal states unrepresentable.

Also applies to: 19-19, 25-25

examples/speak/file/hello-world/Program.cs (1)
  • 11-12: Relying on the DEEPGRAM_API_KEY environment variable for the API key in the SpeakClient example is a secure and flexible approach. The added comment provides clarity on setting up the API key, aligning with best practices for handling sensitive information.
Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1)
  • 25-25: The renaming of last_word_end to timestamp in SpeechStartedResponse class improves clarity. Ensure that all references to this property in the SDK and any examples or documentation are updated accordingly to reflect this change.
examples/prerecorded/file/Program.cs (2)
  • 5-5: Renaming the namespace to PreRecorded aligns with the broader reorganization efforts and makes the purpose of the example clearer.
  • 11-12: Using an environment variable for the Deepgram API key is a best practice for security and configurability. Consider adding a comment to clarify that users need to set the DEEPGRAM_API_KEY environment variable with their actual Deepgram API key, similar to the clarification agreed upon in a previous PR.
Deepgram/Models/Live/v1/Alternative.cs (3)
  • 13-13: Making Transcript non-nullable ensures that a transcript value is always provided, enhancing data integrity.
  • 18-18: Setting Confidence as non-nullable aligns with the expectation that a confidence score is always available for a transcript.
  • 24-24: Ensuring Words is non-nullable guarantees that the list of words, even if empty, is always returned, which is useful for consistency in response parsing.
Deepgram/Models/Live/v1/Metadata.cs (3)
  • 13-13: Changing RequestId to a non-nullable type ensures that every response includes a request identifier, which is crucial for tracking and debugging.
  • 19-19: Making ModelUUID non-nullable reinforces the requirement for model identification in every response, aiding in analytics and troubleshooting.
  • 25-25: Ensuring ModelInfo is non-nullable provides consistent access to model details in the response, enhancing the API's usability.
Deepgram/Models/Live/v1/Word.cs (4)
  • 13-13: Making HeardWord non-nullable ensures that every word object contains a recognized word, which is essential for transcription accuracy.
  • 19-19: Setting Start as non-nullable guarantees that the start time of each word is always provided, which is important for precise timing in transcriptions.
  • 25-25: Ensuring End is non-nullable provides a clear endpoint for each word, aiding in the accuracy of transcription timing.
  • 31-31: Making Confidence non-nullable ensures that a confidence score is always associated with each word, which is crucial for evaluating transcription reliability.
Deepgram/Models/Live/v1/TranscriptionResponse.cs (8)
  • 12-12: Making Channel non-nullable ensures that channel information is always included in the transcription response, which is important for multi-channel audio processing.
  • 18-18: Ensuring ChannelIndex is non-nullable provides clarity on the specific channels being transcribed, enhancing the response's utility.
  • 24-24: Setting Duration as non-nullable guarantees that the duration of the audio processed is always available, which is useful for analytics and billing.
  • 30-30: Making IsFinal non-nullable clarifies whether the transcription is complete, which is crucial for real-time transcription scenarios.
  • 36-36: Ensuring MetaData is non-nullable provides consistent access to metadata in every transcription response, enhancing data integrity.
  • 42-42: Setting SpeechFinal as non-nullable indicates definitively whether the speech segment has concluded, which is important for processing continuous audio streams.
  • 48-48: Making Start non-nullable ensures that the start time of the transcription segment is always provided, aiding in precise timing analysis.
  • 55-55: Ensuring Type is non-nullable and defaulting to LiveType.Results provides clarity on the type of transcription response being returned.
Deepgram.DevBuild.sln (1)
  • 8-9: Adding the Deepgram.Microphone project to the solution aligns with the PR's objectives to introduce microphone functionality. Ensure that the project is correctly configured and referenced in the solution.
Deepgram.sln (1)
  • 10-11: The inclusion of the Deepgram.Microphone project in the Deepgram.sln solution file supports the PR's goal of enhancing real-time audio processing capabilities. Verify that all necessary dependencies and configurations are properly set up for this project within the solution.
examples/streaming/file/Program.cs (3)
  • 16-17: Ensure that the LiveClient is properly initialized with any required configuration, such as API keys or client options, which might be missing in this snippet.
  • 24-31: The transcription handling logic now checks for an empty transcript and prints the speaker's transcript if available. This is a good improvement for usability. However, consider adding a more descriptive message when an empty transcription is received to inform the user about the possible reasons or next steps.
  • 63-63: Increasing the delay before stopping the connection from 10 seconds to 45 seconds allows more time for responses. Ensure this change aligns with the expected use case and doesn't introduce unnecessary waiting time in scenarios where quicker feedback is preferred.
Deepgram/Factory/HttpClientFactory.cs (1)
  • 30-38: The refactoring to use DeepgramClientOptions directly in the ConfigureDeepgram method simplifies the API key and client configuration process. This is a positive change for maintainability and usability. Ensure that all callers of this method have been updated to pass DeepgramClientOptions instead of individual parameters.
examples/streaming/microphone/Program.cs (1)
  • 1-74: This new example demonstrates how to use a microphone for live audio streaming and transcription with the Deepgram SDK. It's a valuable addition for developers looking to implement real-time audio processing. Ensure that the example is well-documented, including comments on how to set up and use the microphone, and any prerequisites or dependencies, such as PortAudio.
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (2)
  • 17-20: Adding the ApiKey property directly to DeepgramClientOptions simplifies API key management. This is a positive change for clarity and ease of use. Ensure that all instances of DeepgramClientOptions are updated to use this new property.
  • 69-93: The constructor now allows initializing DeepgramClientOptions with more flexibility, including the API key, keep-alive, on-prem options, and more. This is a good improvement for configurability. However, ensure that error handling and logging are properly implemented, especially when the API key is missing or invalid.
Deepgram/Clients/OnPrem/v1/Client.cs (1)
  • 15-15: Making the apiKey parameter optional in the Client constructor aligns with the changes in API key handling across the SDK. Ensure that this change is consistently applied across all client classes and that proper validation or fallback mechanisms are in place when the API key is not provided.
Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (1)
  • 35-35: The modifications to the test cases reflect the changes in the HttpClientFactory.ConfigureDeepgram method signature. Ensure that all test cases are updated accordingly and that new test cases are added if necessary to cover the new functionality introduced in this PR.
Deepgram.Microphone/Microphone.cs (1)
  • 9-177: The Microphone class is a significant addition, enabling real-time audio capture from the microphone. Ensure that the class is thoroughly tested, especially the audio capture and streaming functionality. Additionally, consider adding more detailed logging and error handling, particularly for scenarios where the default input device is not found or the input is null.
Deepgram/Clients/Speak/v1/Client.cs (1)
  • 15-15: Making the apiKey parameter optional in the Client constructor is consistent with similar changes across other client classes. This enhances flexibility in API key management. Ensure that this approach is uniformly applied and documented across the SDK.
Deepgram.Microphone/Deepgram.Microphone.csproj (4)
  • 4-4: The project targets multiple frameworks (net6.0;net7.0;net8.0;netstandard2.0). Ensure that all dependencies are compatible across these frameworks to avoid runtime issues.
  • 66-73: The package references for netstandard2.0 target older versions (6.*). Consider updating these to the latest stable versions to benefit from bug fixes and performance improvements.
  • 35-35: The PackageId for the unstable builds (Deepgram.Unstable.Microphone.Builds) is distinct, which is good practice for differentiating stable and unstable NuGet packages. Ensure that consumers of your SDK are aware of the differences and use the appropriate package based on their needs.
  • 122-122: Including PortAudioSharp2 as a dependency is crucial for microphone functionality. Ensure that this library's license is compatible with your project's license and that it is actively maintained to avoid future compatibility issues.
Deepgram.Dev.sln (3)
  • 10-10: Adding the "PreRecorded" project under examples\prerecorded\file is consistent with the new structure. Ensure that the project path is correct and that the project file exists at the specified location.
  • 14-14: Renaming and moving the "Speak" project to examples\speak\file\hello-world aligns with the reorganization efforts. Verify that all references to this project have been updated accordingly to prevent build errors.
  • 38-38: The addition of the "Streaming" project in two locations (examples\streaming\file and examples\streaming\microphone) suggests a split in functionality. Confirm that this separation is intentional and clearly documented for SDK users.
Deepgram/Clients/Analyze/v1/Client.cs (1)
  • 15-15: Allowing an empty string as a default value for apiKey in the constructor improves usability by supporting scenarios where the API key might be set later or through other means. However, ensure that there are checks in place to validate the presence of an API key before making API calls to prevent runtime errors.
Deepgram/Models/Live/v1/LiveSchema.cs (2)
  • 190-190: Changing UtteranceEnd from an int? to a string? may affect how users of the SDK specify this value. Ensure that this change aligns with the API's expected data types and that it's clearly documented to avoid confusion.
  • 196-196: Modifying VadEvents from an int? to a bool? suggests a change in how voice activity detection events are handled. Confirm that this change is consistent with the latest API specifications and update the SDK documentation accordingly.
Deepgram/Clients/Live/v1/Client.cs (3)
  • 27-27: Initializing DeepgramClientOptions with the provided apiKey if options is null is a good approach for flexibility in client configuration. Ensure that this pattern is consistently applied across all client classes for a unified SDK experience.
  • 45-45: Adding a headers parameter to the Connect method enhances the SDK's flexibility by allowing users to pass custom headers. Ensure that this functionality is well-documented and that the headers are correctly applied to WebSocket connections.
  • 51-63: Setting request headers for the WebSocket connection, including the authorization header and any custom headers, is crucial for establishing a secure and customized connection. Ensure that headers are sanitized and validated to prevent security vulnerabilities.
Deepgram/Clients/Manage/v1/Client.cs (1)
  • 15-15: Making the apiKey parameter optional in the Client constructor by providing a default empty string is a good improvement for flexibility. However, it's crucial to ensure that the rest of the SDK properly handles cases where the apiKey might be empty to avoid runtime errors or unexpected behavior.
Deepgram/Abstractions/AbstractRestClient.cs (1)
  • 34-36: The changes to the AbstractRestClient constructor for initializing options and configuring the _httpClient are well-implemented. Ensure that the DeepgramClientOptions class and the ConfigureDeepgram method in HttpClientFactory are designed to handle cases where the apiKey might be empty, as this could affect the HTTP client's configuration.
Verification successful

The verification process confirms that the DeepgramClientOptions class and the ConfigureDeepgram method in HttpClientFactory are designed to handle cases where the apiKey might be empty. This design ensures that the system can gracefully handle missing or default values for the apiKey, centralizing this logic within the DeepgramClientOptions class for simplicity and robustness in configuration.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that DeepgramClientOptions and ConfigureDeepgram method handle empty apiKey appropriately.
rg --type cs "DeepgramClientOptions" --context 5
rg --type cs "ConfigureDeepgram" --context 5

Length of output: 47135

Deepgram.Microphone/Library.cs Show resolved Hide resolved
@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch from aa6ff4d to 30f955a Compare March 28, 2024 16:21
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and 30f955a.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav
Files selected for processing (50)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (2 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (5 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (1 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (5 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Learnings (2)
Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1)
The `AutoFaker` library automatically populates all fields of objects it generates, including the `CallBack` field in the `AnalyzeSchema` object, ensuring that it is not null by default in tests.
Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (2)
The `AutoFaker` library automatically populates all fields of objects it generates, including the `CallBack` field in the `PrerecordedSchema` object, ensuring that it is not null by default in tests.

The AutoFaker library automatically populates all fields of objects it generates, including the CallBack field in the PrerecordedSchema object, ensuring that it is not null by default in tests.

Files skipped from review as they are similar to previous changes (40)
  • Deepgram.DevBuild.sln
  • Deepgram.Microphone/Constants.cs
  • Deepgram.Microphone/Deepgram.Microphone.csproj
  • Deepgram.Microphone/GlobalUsings.cs
  • Deepgram.Microphone/Library.cs
  • Deepgram.Microphone/Microphone.cs
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs
  • Deepgram/Abstractions/AbstractRestClient.cs
  • Deepgram/AnalyzeClient.cs
  • Deepgram/Clients/Analyze/v1/Client.cs
  • Deepgram/Clients/Live/v1/Client.cs
  • Deepgram/Clients/Manage/v1/Client.cs
  • Deepgram/Clients/OnPrem/v1/Client.cs
  • Deepgram/Clients/Speak/v1/Client.cs
  • Deepgram/Constants/Defaults.cs
  • Deepgram/Deepgram.csproj
  • Deepgram/Factory/HttpClientFactory.cs
  • Deepgram/LiveClient.cs
  • Deepgram/ManageClient.cs
  • Deepgram/Models/Live/v1/Alternative.cs
  • Deepgram/Models/Live/v1/Channel.cs
  • Deepgram/Models/Live/v1/LiveSchema.cs
  • Deepgram/Models/Live/v1/Metadata.cs
  • Deepgram/Models/Live/v1/ModelInfo.cs
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs
  • Deepgram/Models/Live/v1/Word.cs
  • Deepgram/OnPremClient.cs
  • Deepgram/PreRecordedClient.cs
  • Deepgram/SpeakClient.cs
  • clean-up.ps1
  • clean-up.sh
  • examples/prerecorded/file/Prerecorded.csproj
  • examples/prerecorded/file/Program.cs
  • examples/speak/file/hello-world/Program.cs
  • examples/speak/file/hello-world/Speak.csproj
  • examples/streaming/file/Program.cs
  • examples/streaming/file/Streaming.csproj
  • examples/streaming/microphone/Program.cs
  • examples/streaming/microphone/Streaming.csproj
Additional comments (23)
Deepgram.sln (1)
  • 10-11: The addition of the Deepgram.Microphone project to the solution file is correctly formatted and follows the standard structure for Visual Studio solution files. This change aligns with the PR objectives of enhancing microphone functionality.
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (2)
  • 17-21: Adding the ApiKey property to DeepgramClientOptions is a critical update for API key handling. Ensure that the API key is treated securely throughout the application, especially when logging or handling errors.
  • 69-93: The constructor changes, including the addition of new parameters and the logic for initializing ApiKey, improve flexibility and error handling. However, consider adding logging for the scenarios where the API key is fetched from the environment variable or when it's invalid, as indicated by the TODO comments.
Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1)
  • 20-21: The update to initialize _options with _apiKey in the Setup method aligns with the changes made to the DeepgramClientOptions constructor. This ensures that unit tests reflect the new way of configuring client options.
Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (1)
  • 24-24: The update to initialize _options with an apiKey in the Setup method is correctly implemented and aligns with the changes made to the DeepgramClientOptions constructor. This ensures that the live client tests are configured with the necessary API key.
Deepgram.Dev.sln (2)
  • 10-41: The renaming of projects and the addition of new projects, including "examples", "speak", "streaming", etc., are correctly formatted and follow the standard structure for Visual Studio solution files. These changes enhance the organization and clarity of the solution, aligning with the PR objectives.
  • 66-98: The updates to project configurations and the addition of the NestedProjects section are correctly implemented. These changes ensure that the solution file accurately reflects the new project structure and relationships.
Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1)
  • 20-20: The update to initialize _options with an apiKey in the Setup method is correctly implemented and aligns with the changes made to the DeepgramClientOptions constructor. This ensures that the speak client tests are configured with the necessary API key.
Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (1)
  • 23-23: The update to initialize _clientOptions with an apiKey in the Setup method is correctly implemented and aligns with the changes made to the DeepgramClientOptions constructor. This ensures that the abstract rest client tests are configured with the necessary API key.
Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (3)
  • 19-20: The setup method now initializes DeepgramClientOptions with an API key generated using Faker. This change enhances test isolation by ensuring each test run uses a unique API key. However, consider documenting this approach to clarify the intention behind using dynamic API keys in tests, especially for new contributors or when debugging test failures.
  • 16-23: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]

In tests where AutoFaker is used to generate AnalyzeSchema instances, the CallBack property is explicitly set to null in some cases. This aligns with the learning that AutoFaker automatically populates all fields, including CallBack. Explicitly setting CallBack to null is necessary for tests that depend on this property being unset. This practice ensures clarity and correctness in tests that specifically test callback behavior.

  • 16-23: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [58-58]

The tests for AnalyzeUrlCallBack and AnalyzeFileCallBack methods include scenarios where both the CallBack property and parameter are set, as well as where neither is set, leading to ArgumentException. These tests effectively validate the expected behavior in edge cases, ensuring robust error handling and clear API contract enforcement.

Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (6)
  • 19-20: The modification to initialize DeepgramClientOptions with an API key using the constructor that accepts the API key as a parameter is a good practice. It ensures that the API key is encapsulated within the options, simplifying the configuration of the Deepgram client.
  • 16-23: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]

Setting prerecordedSchema.CallBack = null; directly in the test TranscribeUrl_Should_Call_PostAsync_Returning_SyncResponse is a good practice to ensure the test's intent is clear. However, consider adding a brief comment explaining why the CallBack property is explicitly set to null here, enhancing readability and understanding for future maintainers.

  • 16-23: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [58-58]

The test TranscribeUrl_Should_Throw_ArgumentException_If_CallBack_Not_Null effectively checks for the expected behavior when the CallBack property is not null. This is an important test to ensure that the SDK behaves correctly in scenarios where a callback is not expected. Good job on covering this edge case.

  • 16-23: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [92-92]

In the test TranscribeUrlCallBack_Should_Call_PostAsync_Returning_SyncResponse_With_CallBack_Parameter, the approach of nullifying the CallBack property before testing the callback parameter functionality is correct. It ensures that the test accurately reflects the scenario it aims to validate. However, adding a comment explaining the reason for nullifying the CallBack property could improve code clarity.

  • 16-23: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [126-126]

The test TranscribeUrlCallBack_Should_Throw_ArgumentException_With_CallBack_Property_And_CallBack_Parameter_Set correctly checks for an ArgumentException when both the CallBack property and parameter are set. This test ensures that the SDK's API behaves as expected in misuse scenarios, which is crucial for robustness.

  • 16-23: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [160-160]

For tests involving streams and byte arrays, such as TranscribeFile_With_Stream_Should_Call_PostAsync_Returning_SyncResponse, consider adding utility methods to generate streams and byte arrays. This would not only reduce duplication but also improve the readability and maintainability of the test code.

Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (5)
  • 22-23: Initializing DeepgramClientOptions with an API key in the Setup method is consistent with best practices, ensuring that each test uses a fresh set of options with a unique API key. This change enhances the isolation between tests.
  • 18-26: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [31-31]

The test GetProjects_Should_Call_GetAsync_Returning_ProjectsResponse effectively mocks the HTTP client and verifies that the GetAsync method is called with the correct URL. This pattern is consistently applied across tests, ensuring that the SDK's interaction with the API is correctly implemented and tested.

  • 18-26: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [105-105]

In the test UpdateProject_Should_Call_PatchAsync_Returning_MessageResponse, consider adding assertions to verify that the updateProjectSchema passed to PatchAsync matches the expectations. This would ensure that not only the method is called but also called with the correct parameters.

  • 18-26: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [179-179]

For tests involving project keys, such as GetKeys_Should_Call_GetAsync_Returning_KeysResponse, it's good practice to verify the URL construction, especially the inclusion of the project ID. This ensures that the SDK correctly constructs requests based on the provided project ID.

  • 18-26: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [253-253]

In the CreateKey_Should_Call_PostAsync_Returning_KeyResponse_Without_Expiration_TimeToLive_Set test, the explicit setting of ExpirationDate and TimeToLiveInSeconds to null is a good practice to test default behavior. However, adding a comment explaining why these fields are set to null could improve readability.

@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch 2 times, most recently from 3552953 to 4aa330b Compare March 28, 2024 16:39
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and 4aa330b.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav
Files selected for processing (50)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (2 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (5 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (1 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (5 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Files skipped from review as they are similar to previous changes (48)
  • Deepgram.DevBuild.sln
  • Deepgram.Microphone/Constants.cs
  • Deepgram.Microphone/Deepgram.Microphone.csproj
  • Deepgram.Microphone/GlobalUsings.cs
  • Deepgram.Microphone/Library.cs
  • Deepgram.Microphone/Microphone.cs
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs
  • Deepgram.sln
  • Deepgram/Abstractions/AbstractRestClient.cs
  • Deepgram/AnalyzeClient.cs
  • Deepgram/Clients/Analyze/v1/Client.cs
  • Deepgram/Clients/Live/v1/Client.cs
  • Deepgram/Clients/Manage/v1/Client.cs
  • Deepgram/Clients/OnPrem/v1/Client.cs
  • Deepgram/Clients/Speak/v1/Client.cs
  • Deepgram/Constants/Defaults.cs
  • Deepgram/Deepgram.csproj
  • Deepgram/Factory/HttpClientFactory.cs
  • Deepgram/LiveClient.cs
  • Deepgram/ManageClient.cs
  • Deepgram/Models/Live/v1/Alternative.cs
  • Deepgram/Models/Live/v1/Channel.cs
  • Deepgram/Models/Live/v1/LiveSchema.cs
  • Deepgram/Models/Live/v1/Metadata.cs
  • Deepgram/Models/Live/v1/ModelInfo.cs
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs
  • Deepgram/Models/Live/v1/Word.cs
  • Deepgram/OnPremClient.cs
  • Deepgram/PreRecordedClient.cs
  • Deepgram/SpeakClient.cs
  • clean-up.ps1
  • clean-up.sh
  • examples/prerecorded/file/Prerecorded.csproj
  • examples/prerecorded/file/Program.cs
  • examples/speak/file/hello-world/Program.cs
  • examples/speak/file/hello-world/Speak.csproj
  • examples/streaming/file/Program.cs
  • examples/streaming/file/Streaming.csproj
  • examples/streaming/microphone/Program.cs
  • examples/streaming/microphone/Streaming.csproj
Additional comments (5)
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (3)
  • 17-20: The addition of the ApiKey property is a positive change, ensuring that the API key is explicitly managed within the DeepgramClientOptions class. This aligns with the objective of improving API key handling.
  • 34-36: The Headers property has been removed and moved to the constructor parameters. This change could potentially break existing code that directly assigns values to the Headers property. It's important to ensure that all usages of DeepgramClientOptions throughout the codebase have been updated to reflect this change.
  • 69-93: The constructor now includes parameters for apiKey, keepAlive, onPrem, baseAddress, headers, and apiVersion, which enhances the flexibility and configurability of the DeepgramClientOptions. However, the error handling for an invalid or missing API key (lines 89-92) could be improved by adding logging before throwing the exception. This would aid in debugging and provide clearer feedback to the developer.

Consider adding logging before throwing the ArgumentException to provide more context about the error.

Deepgram.Dev.sln (2)
  • 10-41: The renaming of projects and addition of new projects, including the Deepgram.Microphone project, are significant updates that align with the PR objectives of introducing real-time audio processing capabilities and improving project organization. Ensure that all references to these projects within the solution and any documentation have been updated accordingly.
  • 66-97: The updates to project configurations and the nesting of projects within solution folders are well-organized and contribute to a cleaner solution structure. However, it's important to verify that all project references are correctly updated to reflect these changes, especially in build scripts or CI/CD pipelines that might reference these projects.

@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch from 4aa330b to c1fe3eb Compare March 28, 2024 18:31
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and c1fe3eb.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav
Files selected for processing (54)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (2 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (3 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (3 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (2 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/PreRecorded/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Files skipped from review due to trivial changes (1)
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs
Files skipped from review as they are similar to previous changes (48)
  • Deepgram.DevBuild.sln
  • Deepgram.Microphone/Constants.cs
  • Deepgram.Microphone/Deepgram.Microphone.csproj
  • Deepgram.Microphone/GlobalUsings.cs
  • Deepgram.Microphone/Library.cs
  • Deepgram.Microphone/Microphone.cs
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs
  • Deepgram.sln
  • Deepgram/Abstractions/AbstractRestClient.cs
  • Deepgram/AnalyzeClient.cs
  • Deepgram/Clients/Analyze/v1/Client.cs
  • Deepgram/Clients/Live/v1/Client.cs
  • Deepgram/Clients/Manage/v1/Client.cs
  • Deepgram/Clients/OnPrem/v1/Client.cs
  • Deepgram/Clients/Speak/v1/Client.cs
  • Deepgram/Constants/Defaults.cs
  • Deepgram/Deepgram.csproj
  • Deepgram/Factory/HttpClientFactory.cs
  • Deepgram/LiveClient.cs
  • Deepgram/ManageClient.cs
  • Deepgram/Models/Live/v1/Alternative.cs
  • Deepgram/Models/Live/v1/Channel.cs
  • Deepgram/Models/Live/v1/LiveSchema.cs
  • Deepgram/Models/Live/v1/Metadata.cs
  • Deepgram/Models/Live/v1/ModelInfo.cs
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs
  • Deepgram/Models/Live/v1/Word.cs
  • Deepgram/OnPremClient.cs
  • Deepgram/PreRecordedClient.cs
  • Deepgram/SpeakClient.cs
  • clean-up.ps1
  • clean-up.sh
  • examples/prerecorded/file/Prerecorded.csproj
  • examples/prerecorded/file/Program.cs
  • examples/speak/file/hello-world/Program.cs
  • examples/speak/file/hello-world/Speak.csproj
  • examples/streaming/file/Program.cs
  • examples/streaming/file/Streaming.csproj
  • examples/streaming/microphone/Program.cs
  • examples/streaming/microphone/Streaming.csproj
Additional comments (5)
Deepgram.Tests/Fakes/ConcreteRestClient.cs (1)
  • 9-9: The update to use DeepgramHttpClientOptions in the ConcreteRestClient constructor is a good refinement. Ensure that DeepgramHttpClientOptions provides all necessary configurations required by AbstractRestClient.
Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1)
  • 12-67: Consider adding logging for URL formatting steps and handling potential errors more explicitly. Also, plan to address the TODO comments regarding logging.
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (1)
  • 60-113: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [15-111]

The enhancements in DeepgramClientOptions align with the objectives of improving API key handling and configuration flexibility. Consider adding explicit error handling for URL formatting and addressing the TODO comments.

Deepgram.Dev.sln (1)
  • 7-45: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [10-98]

The updates to the solution file, including project renaming, additions, and path/configuration updates, are well-organized. Ensure all project references and dependencies are correctly updated to avoid build issues.

Deepgram/Clients/PreRecorded/v1/Client.cs (1)
  • 15-15: The update to use DeepgramHttpClientOptions in the Client constructor aligns with the PR's objectives. Ensure that DeepgramHttpClientOptions provides all necessary configurations for the transcription methods.

@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch from c1fe3eb to 573d076 Compare March 28, 2024 18:47
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and 573d076.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav
Files selected for processing (54)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (2 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (3 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (3 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (2 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/PreRecorded/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Files skipped from review as they are similar to previous changes (51)
  • Deepgram.DevBuild.sln
  • Deepgram.Microphone/Constants.cs
  • Deepgram.Microphone/Deepgram.Microphone.csproj
  • Deepgram.Microphone/GlobalUsings.cs
  • Deepgram.Microphone/Library.cs
  • Deepgram.Microphone/Microphone.cs
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs
  • Deepgram.sln
  • Deepgram/Abstractions/AbstractRestClient.cs
  • Deepgram/AnalyzeClient.cs
  • Deepgram/Clients/Analyze/v1/Client.cs
  • Deepgram/Clients/Manage/v1/Client.cs
  • Deepgram/Clients/OnPrem/v1/Client.cs
  • Deepgram/Clients/PreRecorded/v1/Client.cs
  • Deepgram/Clients/Speak/v1/Client.cs
  • Deepgram/Constants/Defaults.cs
  • Deepgram/Deepgram.csproj
  • Deepgram/Factory/HttpClientFactory.cs
  • Deepgram/LiveClient.cs
  • Deepgram/ManageClient.cs
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs
  • Deepgram/Models/Live/v1/Alternative.cs
  • Deepgram/Models/Live/v1/Channel.cs
  • Deepgram/Models/Live/v1/LiveSchema.cs
  • Deepgram/Models/Live/v1/Metadata.cs
  • Deepgram/Models/Live/v1/ModelInfo.cs
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs
  • Deepgram/Models/Live/v1/Word.cs
  • Deepgram/OnPremClient.cs
  • Deepgram/PreRecordedClient.cs
  • Deepgram/SpeakClient.cs
  • clean-up.ps1
  • clean-up.sh
  • examples/prerecorded/file/Prerecorded.csproj
  • examples/prerecorded/file/Program.cs
  • examples/speak/file/hello-world/Program.cs
  • examples/speak/file/hello-world/Speak.csproj
  • examples/streaming/file/Program.cs
  • examples/streaming/file/Streaming.csproj
  • examples/streaming/microphone/Program.cs
  • examples/streaming/microphone/Streaming.csproj
Additional comments (10)
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4)
  • 15-18: The addition of the ApiKey property is a straightforward way to store the Deepgram API key. However, ensure that this property is only set in secure contexts to prevent unintentional exposure of sensitive information.
  • 32-34: The removal of the Headers property and its inclusion in the constructor parameters is a significant change. This approach can improve encapsulation and ensure that headers are set at object creation, enhancing security by reducing the chance for headers to be manipulated post-creation. However, ensure that all existing code that relied on setting Headers directly has been updated to use the new constructor.
  • 67-74: The constructor now accepts several optional parameters, including apiKey, baseAddress, keepAlive, onPrem, headers, and apiVersion. This change improves flexibility and allows for more detailed configuration of the DeepgramClientOptions. Ensure that all calls to this constructor throughout the codebase have been updated to reflect these changes.
  • 77-91: The error handling for an invalid or missing API key is crucial for security and usability. Throwing an ArgumentException if the API key is invalid or missing is a good practice. However, consider enhancing the user experience by providing more detailed error messages or guidance on how to resolve the issue.
Deepgram.Dev.sln (2)
  • 66-77: The project configuration updates for the newly added and renamed projects are crucial for ensuring that the solution builds correctly. Verify that these configurations are consistent across different environments (e.g., development, staging, production) and that they align with the project's overall build and deployment strategies.
  • 82-98: The organization of projects into nested folders using the NestedProjects section improves the solution's navigability in Visual Studio. Ensure that this nesting accurately reflects the logical structure of the projects and their relationships. Consider adding comments or documentation to explain the rationale behind the organization for future maintainers.
Deepgram/Clients/Live/v1/Client.cs (4)
  • 17-27: The update to initialize DeepgramWsClientOptions with the provided apiKey in the constructor is a positive change that simplifies the configuration process. However, ensure that the apiKey is handled securely throughout its lifecycle to prevent unintentional exposure.
  • 45-64: The addition of the headers parameter to the Connect method allows for more flexible request customization. This is a useful feature, especially for advanced use cases. Ensure that the headers are validated and sanitized before being added to the request to prevent security vulnerabilities such as header injection.
  • 74-82: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [77-112]

The error handling and logging within the Connect method and other parts of the class need refinement. Consider implementing a more robust logging strategy to capture detailed information about errors and operational events. Replace Console.WriteLine statements with calls to a logging framework that supports different log levels and destinations.

- Console.WriteLine(_uri); // TODO: logging
+ logger.LogInformation($"Connecting to URI: {_uri}");
  • 279-287: The GetUri method constructs the URI for connecting to the Deepgram API. This method correctly handles query string construction and encoding. However, consider adding error handling for potential exceptions that could arise from URI construction, such as UriFormatException.

@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch from 573d076 to bb1f15c Compare March 28, 2024 18:58
@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch from bb1f15c to 2ed7fa8 Compare March 28, 2024 19:05
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and 2ed7fa8.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav
Files selected for processing (54)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (2 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (3 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (3 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (2 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/PreRecorded/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Files skipped from review as they are similar to previous changes (52)
  • Deepgram.DevBuild.sln
  • Deepgram.Microphone/Constants.cs
  • Deepgram.Microphone/Deepgram.Microphone.csproj
  • Deepgram.Microphone/GlobalUsings.cs
  • Deepgram.Microphone/Library.cs
  • Deepgram.Microphone/Microphone.cs
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs
  • Deepgram.sln
  • Deepgram/Abstractions/AbstractRestClient.cs
  • Deepgram/AnalyzeClient.cs
  • Deepgram/Clients/Analyze/v1/Client.cs
  • Deepgram/Clients/Live/v1/Client.cs
  • Deepgram/Clients/Manage/v1/Client.cs
  • Deepgram/Clients/OnPrem/v1/Client.cs
  • Deepgram/Clients/PreRecorded/v1/Client.cs
  • Deepgram/Clients/Speak/v1/Client.cs
  • Deepgram/Constants/Defaults.cs
  • Deepgram/Deepgram.csproj
  • Deepgram/Factory/HttpClientFactory.cs
  • Deepgram/LiveClient.cs
  • Deepgram/ManageClient.cs
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs
  • Deepgram/Models/Live/v1/Alternative.cs
  • Deepgram/Models/Live/v1/Channel.cs
  • Deepgram/Models/Live/v1/LiveSchema.cs
  • Deepgram/Models/Live/v1/Metadata.cs
  • Deepgram/Models/Live/v1/ModelInfo.cs
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs
  • Deepgram/Models/Live/v1/Word.cs
  • Deepgram/OnPremClient.cs
  • Deepgram/PreRecordedClient.cs
  • Deepgram/SpeakClient.cs
  • clean-up.ps1
  • clean-up.sh
  • examples/prerecorded/file/Prerecorded.csproj
  • examples/prerecorded/file/Program.cs
  • examples/speak/file/hello-world/Program.cs
  • examples/speak/file/hello-world/Speak.csproj
  • examples/streaming/file/Program.cs
  • examples/streaming/file/Streaming.csproj
  • examples/streaming/microphone/Program.cs
  • examples/streaming/microphone/Streaming.csproj
Additional comments (5)
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4)
  • 15-18: The addition of the ApiKey property is a positive change, ensuring that the API key is explicitly managed within the client options. This aligns with the objective of improving API key handling across client constructors.
  • 32-32: The removal of the Headers property and its inclusion in the constructor parameters could potentially impact existing code that relies on setting headers directly on the DeepgramClientOptions object. It's important to verify that all usages have been updated accordingly.
  • 67-74: The constructor now accepts several optional parameters, including apiKey, baseAddress, keepAlive, onPrem, headers, and apiVersion. This is a significant improvement in flexibility and usability, allowing for more granular configuration of the client options. However, ensure that all calls to this constructor throughout the codebase have been updated to reflect these changes.
Verification successful

The verification process has confirmed that the codebase has been updated to use the new DeepgramClientOptions constructor with its optional parameters where applicable. This includes various usage scenarios in unit tests and factory methods, demonstrating the constructor's flexibility and the codebase's alignment with the updated constructor signature.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Check if the test code and other usages have been updated to use the new constructor instead of direct property assignment.
rg --type cs "new DeepgramClientOptions"

Length of output: 963

* 77-91: The error handling for an invalid or missing API key is crucial, especially for non-OnPrem users. Throwing an `ArgumentException` when the API key is invalid or missing is a good practice. However, the TODO comments regarding logging should be addressed to ensure that these critical errors are properly logged for troubleshooting.

Consider implementing logging for these error conditions to aid in debugging and operational monitoring.

Deepgram.Dev.sln (1)
  • 66-98: The updates to project configurations for the newly added and renamed projects are crucial for ensuring that the solution builds correctly across different environments. It's important to verify that these configurations are correctly set up for all relevant build and release scenarios.
Verification successful

The project configurations for the newly added and renamed projects in the Deepgram.Dev.sln file have been verified to be correctly set up for all relevant build and release scenarios, as indicated by the script output. This ensures that the solution builds correctly across different environments.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify project configurations for build and release scenarios.
rg "ActiveCfg|Build.0" Deepgram.Dev.sln

Length of output: 2337

Deepgram.Dev.sln Outdated Show resolved Hide resolved
@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch from 2ed7fa8 to bd8d36e Compare March 28, 2024 19:43
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and bd8d36e.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav
Files selected for processing (54)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (2 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (3 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (3 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (2 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/PreRecorded/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Learnings (1)
Deepgram.Dev.sln (1)
References to "Prerecorded" and other project names in the documentation and build scripts have been updated as part of the PR changes. Remaining mentions of "Prerecorded" in `Deepgram/README.md`, `examples/README.md`, and `README.md` were noted for potential review.
Files skipped from review as they are similar to previous changes (51)
  • Deepgram.DevBuild.sln
  • Deepgram.Microphone/Constants.cs
  • Deepgram.Microphone/Deepgram.Microphone.csproj
  • Deepgram.Microphone/GlobalUsings.cs
  • Deepgram.Microphone/Library.cs
  • Deepgram.Microphone/Microphone.cs
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs
  • Deepgram.sln
  • Deepgram/Abstractions/AbstractRestClient.cs
  • Deepgram/AnalyzeClient.cs
  • Deepgram/Clients/Analyze/v1/Client.cs
  • Deepgram/Clients/Manage/v1/Client.cs
  • Deepgram/Clients/OnPrem/v1/Client.cs
  • Deepgram/Clients/PreRecorded/v1/Client.cs
  • Deepgram/Clients/Speak/v1/Client.cs
  • Deepgram/Constants/Defaults.cs
  • Deepgram/Deepgram.csproj
  • Deepgram/Factory/HttpClientFactory.cs
  • Deepgram/LiveClient.cs
  • Deepgram/ManageClient.cs
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs
  • Deepgram/Models/Live/v1/Alternative.cs
  • Deepgram/Models/Live/v1/Channel.cs
  • Deepgram/Models/Live/v1/LiveSchema.cs
  • Deepgram/Models/Live/v1/Metadata.cs
  • Deepgram/Models/Live/v1/ModelInfo.cs
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs
  • Deepgram/Models/Live/v1/Word.cs
  • Deepgram/OnPremClient.cs
  • Deepgram/PreRecordedClient.cs
  • Deepgram/SpeakClient.cs
  • clean-up.ps1
  • clean-up.sh
  • examples/prerecorded/file/Prerecorded.csproj
  • examples/prerecorded/file/Program.cs
  • examples/speak/file/hello-world/Program.cs
  • examples/speak/file/hello-world/Speak.csproj
  • examples/streaming/file/Program.cs
  • examples/streaming/file/Streaming.csproj
  • examples/streaming/microphone/Program.cs
  • examples/streaming/microphone/Streaming.csproj
Additional comments (9)
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4)
  • 15-18: The addition of the ApiKey property is a good practice for encapsulating the API key handling within the DeepgramClientOptions. This change centralizes the API key management, making it easier to maintain and use across different parts of the SDK.
  • 32-32: The removal of the Headers property and its inclusion in the constructor parameters could impact existing code that relies on setting headers directly on the DeepgramClientOptions object. Ensure that all usages have been updated accordingly.
  • 67-74: The constructor now accepts additional parameters (keepAlive, onPrem, headers, apiVersion) and initializes the ApiKey and other properties accordingly. This change enhances the flexibility and configurability of the client options. However, ensure that all calls to this constructor throughout the codebase have been updated to match the new signature.
Verification successful

The verification process has confirmed that calls to the DeepgramClientOptions constructor throughout the codebase have been updated to match the new signature, utilizing the flexibility and configurability of the updated constructor as intended. Calls vary in the number of arguments passed, demonstrating the effective use of both specified and default parameter values.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for instances of DeepgramClientOptions being instantiated.
rg --type cs "new DeepgramClientOptions"

Length of output: 963

* 77-91: The error handling for an invalid or missing API key is well-implemented, throwing an `ArgumentException` if the API key is not provided and not in an on-premises configuration. This ensures that the SDK fails fast if the configuration is incorrect, aiding in debugging and configuration validation.
Deepgram.Dev.sln (1)
  • 58-74: The updates to the GlobalSection(NestedProjects) section correctly reflect the new project structure and nesting within the solution. This helps maintain the organization and readability of the solution explorer in Visual Studio.
Deepgram/Clients/Live/v1/Client.cs (4)
  • 17-17: The initialization of _deepgramClientOptions with the provided options enhances the flexibility and configurability of the live client. This change allows for more detailed customization of the WebSocket client options.
  • 25-27: The constructor now initializes DeepgramWsClientOptions with the provided apiKey, improving the encapsulation of API key handling within the client. Ensure that all instantiations of the Client class have been updated to reflect this change.
  • 45-64: The addition of the headers parameter to the Connect method allows for more granular control over the request headers sent during the WebSocket connection. This is a valuable addition for scenarios requiring custom headers. Ensure that all calls to Connect have been updated to include this new parameter where necessary.
  • 74-82: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [77-111]

The method for constructing the WebSocket URI and handling the connection logic is well-implemented. However, consider encapsulating the URI construction logic into a separate method for better readability and maintainability. Additionally, replace the Console.WriteLine statement with proper logging.

- Console.WriteLine(_uri); // TODO: logging
+ logger.LogInformation($"Connecting to URI: {_uri}");

Deepgram.Dev.sln Outdated Show resolved Hide resolved
@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch 3 times, most recently from d45000d to ca5d6bc Compare March 28, 2024 23:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and ca5d6bc.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav, !**/*.wav
Files selected for processing (54)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (2 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (3 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (2 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/PreRecorded/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Files skipped from review as they are similar to previous changes (52)
  • Deepgram.Dev.sln
  • Deepgram.DevBuild.sln
  • Deepgram.Microphone/Constants.cs
  • Deepgram.Microphone/Deepgram.Microphone.csproj
  • Deepgram.Microphone/GlobalUsings.cs
  • Deepgram.Microphone/Library.cs
  • Deepgram.Microphone/Microphone.cs
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs
  • Deepgram.sln
  • Deepgram/Abstractions/AbstractRestClient.cs
  • Deepgram/AnalyzeClient.cs
  • Deepgram/Clients/Analyze/v1/Client.cs
  • Deepgram/Clients/Manage/v1/Client.cs
  • Deepgram/Clients/OnPrem/v1/Client.cs
  • Deepgram/Clients/PreRecorded/v1/Client.cs
  • Deepgram/Clients/Speak/v1/Client.cs
  • Deepgram/Constants/Defaults.cs
  • Deepgram/Deepgram.csproj
  • Deepgram/Factory/HttpClientFactory.cs
  • Deepgram/LiveClient.cs
  • Deepgram/ManageClient.cs
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs
  • Deepgram/Models/Live/v1/Alternative.cs
  • Deepgram/Models/Live/v1/Channel.cs
  • Deepgram/Models/Live/v1/LiveSchema.cs
  • Deepgram/Models/Live/v1/Metadata.cs
  • Deepgram/Models/Live/v1/ModelInfo.cs
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs
  • Deepgram/Models/Live/v1/Word.cs
  • Deepgram/OnPremClient.cs
  • Deepgram/PreRecordedClient.cs
  • Deepgram/SpeakClient.cs
  • clean-up.ps1
  • clean-up.sh
  • examples/prerecorded/file/Prerecorded.csproj
  • examples/prerecorded/file/Program.cs
  • examples/speak/file/hello-world/Program.cs
  • examples/speak/file/hello-world/Speak.csproj
  • examples/streaming/file/Program.cs
  • examples/streaming/file/Streaming.csproj
  • examples/streaming/microphone/Program.cs
  • examples/streaming/microphone/Streaming.csproj
Additional Context Used
Additional comments not posted (8)
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (5)

15-18: The addition of the ApiKey property is a good practice for encapsulating the API key. This change aligns with the objective of improving API key handling across client constructors and options classes.


29-29: Setting a default value for APIVersion is a sensible default that simplifies the initialization for common use cases.


32-32: The removal of the Headers property from the class definition is not shown, but it's mentioned in the summary. Ensure that all usages of this property have been updated to accommodate this change, especially in test code or other parts of the SDK where DeepgramClientOptions might be instantiated.


46-46: Enabling KeepAlive for streaming with a default value of false is a reasonable choice, allowing users to opt-in as needed.


51-54: The OnPrem property with a default value of false is clear and straightforward, catering to users who might be using the SDK in an on-premises setup.

Deepgram/Clients/Live/v1/Client.cs (3)

17-17: The initialization of _deepgramClientOptions with the provided options or a new instance is a clean way to ensure that the client is always configured. This change supports the objective of streamlining configuration across the SDK.


24-27: The constructor's update to initialize DeepgramWsClientOptions with the provided apiKey is a good practice, ensuring that the API key is always available for the client's operations.


74-82: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [77-111]

The logging TODO comments should be addressed to ensure that important information is logged correctly, especially during the connection process. Consider implementing structured logging for better traceability and debugging.

Consider replacing the Console.WriteLine statements with structured logging to improve traceability and debugging capabilities.

@davidvonthenen davidvonthenen force-pushed the streaming-interface-refactor branch from ca5d6bc to 85d69ed Compare March 28, 2024 23:57
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e62992e and 85d69ed.
Files ignored due to path filters (1)
  • examples/streaming/file/preamble.wav is excluded by !**/*.wav, !**/*.wav
Files selected for processing (54)
  • Deepgram.Dev.sln (3 hunks)
  • Deepgram.DevBuild.sln (2 hunks)
  • Deepgram.Microphone/Constants.cs (1 hunks)
  • Deepgram.Microphone/Deepgram.Microphone.csproj (1 hunks)
  • Deepgram.Microphone/GlobalUsings.cs (1 hunks)
  • Deepgram.Microphone/Library.cs (1 hunks)
  • Deepgram.Microphone/Microphone.cs (1 hunks)
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs (2 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1 hunks)
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (3 hunks)
  • Deepgram.sln (2 hunks)
  • Deepgram/Abstractions/AbstractRestClient.cs (2 hunks)
  • Deepgram/AnalyzeClient.cs (1 hunks)
  • Deepgram/Clients/Analyze/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Live/v1/Client.cs (4 hunks)
  • Deepgram/Clients/Manage/v1/Client.cs (1 hunks)
  • Deepgram/Clients/OnPrem/v1/Client.cs (1 hunks)
  • Deepgram/Clients/PreRecorded/v1/Client.cs (1 hunks)
  • Deepgram/Clients/Speak/v1/Client.cs (1 hunks)
  • Deepgram/Constants/Defaults.cs (1 hunks)
  • Deepgram/Deepgram.csproj (2 hunks)
  • Deepgram/Factory/HttpClientFactory.cs (1 hunks)
  • Deepgram/LiveClient.cs (1 hunks)
  • Deepgram/ManageClient.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (4 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1 hunks)
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1 hunks)
  • Deepgram/Models/Live/v1/Alternative.cs (1 hunks)
  • Deepgram/Models/Live/v1/Channel.cs (1 hunks)
  • Deepgram/Models/Live/v1/LiveSchema.cs (1 hunks)
  • Deepgram/Models/Live/v1/Metadata.cs (1 hunks)
  • Deepgram/Models/Live/v1/ModelInfo.cs (1 hunks)
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs (1 hunks)
  • Deepgram/Models/Live/v1/Word.cs (1 hunks)
  • Deepgram/OnPremClient.cs (1 hunks)
  • Deepgram/PreRecordedClient.cs (1 hunks)
  • Deepgram/SpeakClient.cs (1 hunks)
  • clean-up.ps1 (1 hunks)
  • clean-up.sh (1 hunks)
  • examples/prerecorded/file/Prerecorded.csproj (1 hunks)
  • examples/prerecorded/file/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Program.cs (1 hunks)
  • examples/speak/file/hello-world/Speak.csproj (1 hunks)
  • examples/streaming/file/Program.cs (2 hunks)
  • examples/streaming/file/Streaming.csproj (1 hunks)
  • examples/streaming/microphone/Program.cs (1 hunks)
  • examples/streaming/microphone/Streaming.csproj (1 hunks)
Files skipped from review as they are similar to previous changes (52)
  • Deepgram.Dev.sln
  • Deepgram.DevBuild.sln
  • Deepgram.Microphone/Constants.cs
  • Deepgram.Microphone/Deepgram.Microphone.csproj
  • Deepgram.Microphone/GlobalUsings.cs
  • Deepgram.Microphone/Library.cs
  • Deepgram.Microphone/Microphone.cs
  • Deepgram.Tests/Fakes/ConcreteRestClient.cs
  • Deepgram.Tests/UnitTests/ClientTests/AbstractRestClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs
  • Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs
  • Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs
  • Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs
  • Deepgram.sln
  • Deepgram/Abstractions/AbstractRestClient.cs
  • Deepgram/AnalyzeClient.cs
  • Deepgram/Clients/Analyze/v1/Client.cs
  • Deepgram/Clients/Manage/v1/Client.cs
  • Deepgram/Clients/OnPrem/v1/Client.cs
  • Deepgram/Clients/PreRecorded/v1/Client.cs
  • Deepgram/Clients/Speak/v1/Client.cs
  • Deepgram/Constants/Defaults.cs
  • Deepgram/Deepgram.csproj
  • Deepgram/Factory/HttpClientFactory.cs
  • Deepgram/LiveClient.cs
  • Deepgram/ManageClient.cs
  • Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs
  • Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs
  • Deepgram/Models/Live/v1/Alternative.cs
  • Deepgram/Models/Live/v1/Channel.cs
  • Deepgram/Models/Live/v1/LiveSchema.cs
  • Deepgram/Models/Live/v1/Metadata.cs
  • Deepgram/Models/Live/v1/ModelInfo.cs
  • Deepgram/Models/Live/v1/SpeechStartedResponse.cs
  • Deepgram/Models/Live/v1/TranscriptionResponse.cs
  • Deepgram/Models/Live/v1/Word.cs
  • Deepgram/OnPremClient.cs
  • Deepgram/PreRecordedClient.cs
  • Deepgram/SpeakClient.cs
  • clean-up.ps1
  • clean-up.sh
  • examples/prerecorded/file/Prerecorded.csproj
  • examples/prerecorded/file/Program.cs
  • examples/speak/file/hello-world/Program.cs
  • examples/speak/file/hello-world/Speak.csproj
  • examples/streaming/file/Program.cs
  • examples/streaming/file/Streaming.csproj
  • examples/streaming/microphone/Program.cs
  • examples/streaming/microphone/Streaming.csproj
Additional Context Used
Additional comments not posted (10)
Deepgram/Models/Authenticate/v1/DeepgramClientOptions.cs (5)

15-18: The addition of the ApiKey property is a critical update for handling authentication. It's essential to ensure that this property is always securely handled and never logged or exposed inappropriately.

Consider adding validation to ensure the API key format is correct upon assignment.


29-29: Setting a default API version is a good practice for maintaining backward compatibility. However, it's important to document any potential impacts of changing the default version on existing integrations.


32-32: The Headers property has been removed according to the summary. This change aligns with the constructor update to include headers as parameters, streamlining header management.
The removal is acknowledged in the summary and previous comments.


46-46: Defaulting KeepAlive to false is a conservative choice that can help avoid unnecessary network traffic. However, consider documenting when it might be beneficial for users to enable this feature.


51-54: The OnPrem property addition is crucial for supporting on-premises deployments. Ensure that the documentation clearly explains how to configure and use this mode.

Deepgram/Clients/Live/v1/Client.cs (5)

17-17: The _deepgramClientOptions field is correctly initialized in the constructor to store WebSocket client options. This centralization of options is a good practice for maintainability.


24-27: The constructor now accepts DeepgramWsClientOptions, which is a positive change for flexibility and configuration. Ensure that the documentation is updated to reflect this new parameter and its usage.


45-64: The Connect method's update to accept an additional headers parameter for setting request headers is a flexible approach that allows for more granular control over the WebSocket connection. Ensure that all calls to this method throughout the SDK have been updated to accommodate this new parameter.

Verification successful

The verification process shows that the calls to the Connect method in the example files do not include the new headers parameter. However, since the headers parameter is optional and the examples are likely intended to demonstrate basic usage, this absence does not necessarily indicate an issue. It would be beneficial to ensure that the SDK documentation or comments within the example files clearly indicate that the Connect method supports additional parameters for more advanced use cases.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for calls to the Connect method to ensure they include the new headers parameter.
rg --type cs 'Connect\('

Length of output: 425


74-82: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [77-111]

The logging TODO comments should be addressed before merging. Proper logging is crucial for debugging and monitoring. Consider using the logger instance already available in the class.

- Console.WriteLine(_uri); // TODO: logging
+ logger.LogInformation($"Connecting to URI: {_uri}");

279-287: The GetUri method is a clean and efficient way to construct the URI for the WebSocket connection. It properly handles query parameters and addons, ensuring flexibility in the SDK's usage.

@davidvonthenen davidvonthenen merged commit fc257a1 into deepgram:main Mar 29, 2024
4 checks passed
@davidvonthenen davidvonthenen deleted the streaming-interface-refactor branch March 29, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unit Tests for UtteranceEnd and SpeechStarted
2 participants