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

text input from datastream for v1.0 #1521

Merged
merged 5 commits into from
Feb 19, 2025
Merged

text input from datastream for v1.0 #1521

merged 5 commits into from
Feb 19, 2025

Conversation

longcw
Copy link
Contributor

@longcw longcw commented Feb 19, 2025

No description provided.

@longcw longcw requested a review from davidzhao February 19, 2025 11:01
Copy link

changeset-bot bot commented Feb 19, 2025

⚠️ No Changeset found

Latest commit: fffa2dc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@longcw longcw requested a review from theomonnom February 19, 2025 11:01
extra={"text": text, "participant": self._participant_identity},
)
self._agent.interrupt()
self._agent.generate_reply(user_input=text)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are calling agent inside the RoomInput, is that okay? should the agent.input has a text input, or we make agent required for RoomInput?

Copy link
Member

Choose a reason for hiding this comment

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

Let's make agent required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll do that when merging the RoomInput and Output.

@longcw longcw merged commit 9525860 into dev-1.0 Feb 19, 2025
@longcw longcw deleted the longc/text-input branch February 19, 2025 15:12
@@ -48,6 +50,7 @@ class RoomOutputOptions:
DEFAULT_ROOM_INPUT_OPTIONS = RoomInputOptions()
DEFAULT_ROOM_OUTPUT_OPTIONS = RoomOutputOptions()
LK_PUBLISH_FOR_ATTR = "lk.publish_for"
LK_TEXT_INPUT_TOPIC = "lk.room_text_input"
Copy link
Contributor

Choose a reason for hiding this comment

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

we should be clear about what topics we want to support. If the goal is to have this work ootb with chat components, then choosing a custom topic here might not be the best choice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the question is are we going to keep the chat components in python/js sdk, and having both the original and the datastream or only the datastream for it? If so I can adjust here accordingly. cc @davidzhao

Copy link
Member

Choose a reason for hiding this comment

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

the chat components are for client-side. but Python/Node agents should agree on the same topic so that it works with the client-side components

@lukasIO what do you recommend we should use? is client-side component listening to both the transcription topic and chat?

Copy link
Contributor

Choose a reason for hiding this comment

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

currently chat components only listen to the chat topic and also send their messages only on the chat topic

Copy link
Member

Choose a reason for hiding this comment

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

how would this work with how we are sending transcriptions? do you suggest also sending transcriptions to chat topic?

Copy link
Contributor

Choose a reason for hiding this comment

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

that was my understanding, yes. But maybe I misunderstood or am forgetting something.
Why wouldn't you want it on the chat topic?

Copy link
Member

Choose a reason for hiding this comment

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

mostly wondering if there's any conflicts between what the agent would want to use as input.. versus what is being spit out as output.

i.e. if there are two agents in the room, would that cause any cross talk.. or if the agent is being added to a livestream with a chat feature, would it automatically start interpreting random transcripts.

for that reason it seems it might be a good idea to be explicit about what is being sent to the agent as input?

@longcw
Copy link
Contributor Author

longcw commented Feb 20, 2025

Do we wan to keep the ChatManager in python sdk, related PR livekit/python-sdks#360?

@davidzhao
Copy link
Member

Do we wan to keep the ChatManager in python sdk, related PR livekit/python-sdks#360?

IMO we should deprecate and remove ChatManager.. agents should not use it any longer

theomonnom pushed a commit that referenced this pull request Feb 21, 2025
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.

4 participants