-
Notifications
You must be signed in to change notification settings - Fork 55
Add support for new bot-output event, providing corresponding callbac… #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ks and deprecating bot-transcription callbacks Resolves #158
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.
Pull Request Overview
This PR adds support for a new bot-output event that provides a unified interface for bot output with metadata about whether text is spoken. It deprecates the existing bot-transcription callbacks in favor of this new event type.
Key Changes
- Introduces
BOT_OUTPUTmessage type andBotOutputDatatype withtext,spoken, and optionalaggregationmetadata - Adds corresponding
botOutputevent handlers and callbacks throughout the client - Implements deprecation warning for
bot-transcriptioncallbacks
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| client-js/rtvi/messages.ts | Defines new BOT_OUTPUT message type and BotOutputData type structure |
| client-js/rtvi/events.ts | Adds BotOutput event and callback type definitions |
| client-js/client/client.ts | Implements bot-output event handling and deprecation warning logic |
| client-js/CHANGELOG.md | Documents the addition and deprecation in release notes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| export type BotOutputData = { | ||
| text: string; | ||
| spoken: boolean; | ||
| aggregated_by?: "word" | "sentence" | string; |
Copilot
AI
Oct 21, 2025
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.
The aggregated_by field allows arbitrary strings which could lead to inconsistent values. Consider using only the union of literal types 'word' | 'sentence' or defining an enum to ensure type safety and prevent typos.
| export type BotOutputData = { | |
| text: string; | |
| spoken: boolean; | |
| aggregated_by?: "word" | "sentence" | string; | |
| export enum AggregatedByType { | |
| WORD = "word", | |
| SENTENCE = "sentence", | |
| } | |
| export type BotOutputData = { | |
| text: string; | |
| spoken: boolean; | |
| aggregated_by?: AggregatedByType; |
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.
no. the point is that the value can be any string, but adding in the "word" | "sentence" makes it clear that those are sort-of built-in or common values that can be expected.
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 also wonder if we should have an extra field like "type" or "kind." But I’m not sure whether we’ll have that information inside Pipecat.
|
|
||
| ### Added | ||
|
|
||
| - BotOutput |
Copilot
AI
Oct 21, 2025
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.
The changelog entry for BotOutput lacks detail. Include a brief description of what the feature does, such as 'Added BotOutput event providing unified bot output with spoken metadata and aggregation information'.
| - BotOutput | |
| - Added BotOutput event providing unified bot output with spoken metadata and aggregation information. |
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 agree with Copilot on this one. 😅
But I guess you just left it here for now so you’d remember to describe it later.
|
|
||
| ### Deprecated | ||
|
|
||
| - BotTranscription |
Copilot
AI
Oct 21, 2025
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.
The deprecation notice should mention the replacement: 'Deprecated BotTranscription in favor of BotOutput event'.
| - BotTranscription | |
| - Deprecated `BotTranscription` in favor of the new `BotOutput` event. |
| onBotTranscript: (text) => { | ||
| if (!this._botTranscriptionWarned) { | ||
| logger.warn( | ||
| "[Pipecat Client] Bot transcription is deprecated. Please use the onBotOutput instead." |
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.
nit
| "[Pipecat Client] Bot transcription is deprecated. Please use the onBotOutput instead." | |
| "[Pipecat Client] onBotTranscript is deprecated. Please use the onBotOutput instead." |
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.
Nice improvement. 🚀
I added a comment about some possible extra fields, but I believe that even if we decide they’re worth adding, it can be a follow up improvement in the future.
…ks and deprecating bot-transcription callbacks
Resolves #158