-
Notifications
You must be signed in to change notification settings - Fork 565
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: text-only mode returning empty responses #33
base: main
Are you sure you want to change the base?
Conversation
src/app/App.tsx
Outdated
const sessionConfig = { | ||
modalities: ["text"], | ||
instructions, | ||
tools, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work on this PR! I really appreciate it.
I found one point I’d like to mention in this PR.
The isAudioPlaybackEnabled
boolean value is used to distinguish between the AI’s audio mode and text-only mode. However, it seems that the user’s audio mode is also being turned off.
I think it would be better if the user’s audio mode remained available, regardless of the isAudioPlaybackEnabled
setting.
Let me know what you think! 😊
const sessionConfig = { | |
modalities: ["text"], | |
instructions, | |
tools, | |
}; | |
const sessionConfig = { | |
modalities: ["text"], | |
input_audio_format: "pcm16", | |
input_audio_transcription: { model: "whisper-1" }, | |
instructions, | |
tools, | |
turn_detection: turnDetection, | |
}; |
Summarizing the changes, the updated code would look like this:
const sessionConfig = {
modalities: ["text"],
input_audio_format: "pcm16",
input_audio_transcription: { model: "whisper-1" },
instructions,
tools,
turn_detection: turnDetection,
};
if (isAudioPlaybackEnabled) {
sessionConfig.modalities.push("audio"); // Changing this to "text" will disable audio
Object.assign(sessionConfig, {
voice: "coral",
output_audio_format: "pcm16",
});
}
const sessionUpdateEvent = {
type: "session.update",
session: sessionConfig,
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AbrahamNobleOX
Thanks for your response! I really appreciate the discussion.
What I meant to say is that isAudioPlaybackEnabled
should be used only to control the AI's audio mode and should be independent of user audio input and Whisper transcription.
Also, Push to Talk isn’t actually a mute button—it controls turn detection. When enabled, the user’s voice is recognized only while the Talk button is pressed.
Your PR does provide more flexibility in toggling audio mode, which is great! However, I think that turning off isAudioPlaybackEnabled
also disables Whisper transcription and turn detection based on the Push-to-Talk boolean value, which might not be the intended behavior.
Apologies if I misunderstood anything, and I’d love to hear your thoughts! 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paix26875
Great explanation.
"Mute" wasn't the best choice of word but it acts as such, hence why I called it that 😊 Sorry for calling that, but I understand it's purpose.
Currently trying to implement your code suggestion and will push an update when I test and it fits.
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested your code suggestion and it works great.
I've also pushed the update accordingly.
Thank you for pointing that out! 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for taking the time to understand my point and incorporate the changes! I appreciate it. Great work! 🚀
This pull request resolves the issue where the text-only mode was returning empty responses from the server.
While the solution suggested by the issue's author is functional, this PR offers a more better and efficient approach.
ISSUE: #32
Testing: