-
Notifications
You must be signed in to change notification settings - Fork 843
Introduce new helper type for acting as a liaison between rtvi message… #1589
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
Conversation
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.
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/pipecat/processors/frameworks/rtvi_helpers/llm.py:66
- [nitpick] The method name 'setupActions' does not follow Python's snake_case naming convention. Consider renaming it to 'setup_actions' for consistency.
def setupActions(self):
APPEND_TO_MESSAGES = "append_to_messages", | ||
GET_CONTEXT = "get_context", | ||
SET_CONTEXT = "set_context", | ||
RUN = "run", |
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 trailing comma in the enum member assignment causes its value to be a tuple instead of a string, which may lead to unexpected behavior. Consider removing the comma to assign a string value.
APPEND_TO_MESSAGES = "append_to_messages", | |
GET_CONTEXT = "get_context", | |
SET_CONTEXT = "set_context", | |
RUN = "run", | |
APPEND_TO_MESSAGES = "append_to_messages" | |
GET_CONTEXT = "get_context" | |
SET_CONTEXT = "set_context" | |
RUN = "run" |
Copilot uses AI. Check for mistakes.
if run_immediately: | ||
await rtvi.interrupt_bot() | ||
|
||
# We just interrupted the bot so it should be find to use the |
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.
Typo detected: 'find' should be 'fine'.
# We just interrupted the bot so it should be find to use the | |
# We just interrupted the bot so it should be fine to use the |
Copilot uses AI. Check for mistakes.
RUN = "run", | ||
|
||
|
||
class RTVIHelper(BaseModel): |
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.
if/when we introduce other types of helpers (tts, stt), I can move this to a separate file
Codecov ReportAttention: Patch coverage is
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
…s and the llm This somewhat parallels the helper system the RTVI clients have.
88b9fc4
to
424c2a3
Compare
going a different direction |
…s and the llm
This somewhat parallels the helper system the RTVI clients have.
Please describe the changes in your PR. If it is addressing an issue, please reference that as well.
The best way to describe this new class is to see how it's implemented in the simple chatbot example, but basically, if you want to handle rtvi messages generated from client llm helpers, the server introduces its own llm helper that handles those messages.
Marking this as a draft pull request because i need to test with a service other than openai, but otherwise, I think it's ready and could use feedback for this approach.