fix: handle double quotes and new lines #357
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe pull request introduces modifications to the Deepgram WebSocket client for text-to-speech functionality. The changes focus on improving string processing in the Changes
Sequence DiagramsequenceDiagram
participant Client
participant WebSocket
participant EventHandlers
Client->>WebSocket: Connect
Client->>WebSocket: SpeakWithText(text)
WebSocket-->>Client: Open Response
WebSocket-->>Client: Metadata Response
Client->>EventHandlers: Trigger Metadata Event
WebSocket-->>Client: Audio Response
Client->>EventHandlers: Trigger Audio Event
Client->>WebSocket: Flush
WebSocket-->>Client: Flushed Response
Client->>WebSocket: Close Connection
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Deepgram/Clients/Speak/v2/WebSocket/Client.cs (1)
291-296: LGTM! Consider adding input validation.The special character escaping is implemented correctly, handling newlines, quotes, and control characters. This fixes the core issue with text processing.
Consider adding null/empty string validation before processing:
public void SpeakWithText(string text) { + if (string.IsNullOrEmpty(text)) + { + throw new ArgumentException("Text cannot be null or empty", nameof(text)); + } TextSource textSource = new TextSource(text.Replace("\r\n", "\\n")tests/edge_cases/string_parsing/Program.cs (1)
114-119: LGTM! Consider enhancing test validation.The test cases comprehensively cover various string formats and special characters, which is excellent for regression testing.
Consider adding validation of the output WAV file or logging the response content to verify the text was processed correctly. For example:
speakClient.SpeakWithText(@"This is the first ""test string""!"); +Console.WriteLine("Expected output: This is the first \"test string\"!"); +// Add validation of the audio response or text processing response
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Deepgram/Clients/Speak/v2/WebSocket/Client.cs(1 hunks)tests/edge_cases/string_parsing/Program.cs(1 hunks)tests/edge_cases/string_parsing/StringParsing.csproj(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/edge_cases/string_parsing/StringParsing.csproj
Comment on lines
+30
to
+98
| if (appendWavHeader) | ||
| { | ||
| using (BinaryWriter writer = new BinaryWriter(File.Open(@"output.wav", FileMode.Append))) | ||
| { | ||
| Console.WriteLine("Adding WAV header to output.wav"); | ||
| byte[] wavHeader = new byte[44]; | ||
| int sampleRate = 48000; | ||
| short bitsPerSample = 16; | ||
| short channels = 1; | ||
| int byteRate = sampleRate * channels * (bitsPerSample / 8); | ||
| short blockAlign = (short)(channels * (bitsPerSample / 8)); | ||
|
|
||
| wavHeader[0] = 0x52; // R | ||
| wavHeader[1] = 0x49; // I | ||
| wavHeader[2] = 0x46; // F | ||
| wavHeader[3] = 0x46; // F | ||
| wavHeader[4] = 0x00; // Placeholder for file size (will be updated later) | ||
| wavHeader[5] = 0x00; // Placeholder for file size (will be updated later) | ||
| wavHeader[6] = 0x00; // Placeholder for file size (will be updated later) | ||
| wavHeader[7] = 0x00; // Placeholder for file size (will be updated later) | ||
| wavHeader[8] = 0x57; // W | ||
| wavHeader[9] = 0x41; // A | ||
| wavHeader[10] = 0x56; // V | ||
| wavHeader[11] = 0x45; // E | ||
| wavHeader[12] = 0x66; // f | ||
| wavHeader[13] = 0x6D; // m | ||
| wavHeader[14] = 0x74; // t | ||
| wavHeader[15] = 0x20; // Space | ||
| wavHeader[16] = 0x10; // Subchunk1Size (16 for PCM) | ||
| wavHeader[17] = 0x00; // Subchunk1Size | ||
| wavHeader[18] = 0x00; // Subchunk1Size | ||
| wavHeader[19] = 0x00; // Subchunk1Size | ||
| wavHeader[20] = 0x01; // AudioFormat (1 for PCM) | ||
| wavHeader[21] = 0x00; // AudioFormat | ||
| wavHeader[22] = (byte)channels; // NumChannels | ||
| wavHeader[23] = 0x00; // NumChannels | ||
| wavHeader[24] = (byte)(sampleRate & 0xFF); // SampleRate | ||
| wavHeader[25] = (byte)((sampleRate >> 8) & 0xFF); // SampleRate | ||
| wavHeader[26] = (byte)((sampleRate >> 16) & 0xFF); // SampleRate | ||
| wavHeader[27] = (byte)((sampleRate >> 24) & 0xFF); // SampleRate | ||
| wavHeader[28] = (byte)(byteRate & 0xFF); // ByteRate | ||
| wavHeader[29] = (byte)((byteRate >> 8) & 0xFF); // ByteRate | ||
| wavHeader[30] = (byte)((byteRate >> 16) & 0xFF); // ByteRate | ||
| wavHeader[31] = (byte)((byteRate >> 24) & 0xFF); // ByteRate | ||
| wavHeader[32] = (byte)blockAlign; // BlockAlign | ||
| wavHeader[33] = 0x00; // BlockAlign | ||
| wavHeader[34] = (byte)bitsPerSample; // BitsPerSample | ||
| wavHeader[35] = 0x00; // BitsPerSample | ||
| wavHeader[36] = 0x64; // d | ||
| wavHeader[37] = 0x61; // a | ||
| wavHeader[38] = 0x74; // t | ||
| wavHeader[39] = 0x61; // a | ||
| wavHeader[40] = 0x00; // Placeholder for data chunk size (will be updated later) | ||
| wavHeader[41] = 0x00; // Placeholder for data chunk size (will be updated later) | ||
| wavHeader[42] = 0x00; // Placeholder for data chunk size (will be updated later) | ||
| wavHeader[43] = 0x00; // Placeholder for data chunk size (will be updated later) | ||
|
|
||
| writer.Write(wavHeader); | ||
| appendWavHeader = false; | ||
| } | ||
| } | ||
|
|
||
| if (e.Stream != null) | ||
| { | ||
| using (BinaryWriter writer = new BinaryWriter(File.Open(@"./output.wav", FileMode.Append))) | ||
| { | ||
| writer.Write(e.Stream.ToArray()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve file handling robustness.
The WAV file handling could be more robust to prevent potential issues:
Consider these improvements:
+const string OUTPUT_FILE = "output.wav";
+
+// Clean up existing file
+if (File.Exists(OUTPUT_FILE))
+{
+ File.Delete(OUTPUT_FILE);
+}
+
if (appendWavHeader)
{
- using (BinaryWriter writer = new BinaryWriter(File.Open(@"output.wav", FileMode.Append)))
+ try
+ {
+ using (BinaryWriter writer = new BinaryWriter(File.Open(OUTPUT_FILE, FileMode.Create)))
+ {
// ... existing WAV header code ...
+ }
+ }
+ catch (IOException ex)
+ {
+ Console.WriteLine($"Error writing WAV header: {ex.Message}");
+ return;
+ }
}Committable suggestion skipped: line range outside the PR's diff.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Types of changes
What types of changes does your code introduce to the community .NET SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Documentation