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

Support intent-progress for Assist #5132

Merged
merged 2 commits into from
Mar 24, 2025
Merged

Support intent-progress for Assist #5132

merged 2 commits into from
Mar 24, 2025

Conversation

TimoPtr
Copy link
Collaborator

@TimoPtr TimoPtr commented Mar 19, 2025

Summary

intent-progress allow streaming the answer of a LLM, right now it is not playing the sound before the text is fully written but that's a limitation of the API nothing we can do in the app.

This PR is inspired by iOS PR home-assistant/iOS#3483 and the frontend PR home-assistant/frontend#24143

The frontend is handling the role but it seems that in our case we don't need for now at least.

I took the liberty to replace the callback with unnamed arguments with a sealed class for better usability and evolution.

Screenshots

demo_stream.webm

Any other notes

It only works with ChatGPT integration for now.

Copy link
Member

@bgoncal bgoncal left a comment

Choose a reason for hiding this comment

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

Looks good to me

is AssistEvent.MessageChunk -> {
val lastMessage = _conversation.last()
if (lastMessage == haMessage) {
// Remove '...' message and add the chunk received
Copy link
Member

Choose a reason for hiding this comment

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

In iOS I have a "typing" animation while we don't get a response, perhaps something you could add in a future PR

@TimoPtr TimoPtr requested a review from dshokouhi March 19, 2025 13:00
@@ -40,6 +41,14 @@ data class AssistPipelineIntentStart(
val intentInput: String
) : AssistPipelineEventData

@JsonIgnoreProperties(ignoreUnknown = true)
data class AssistPipelineIntentProgress(
val chat_log_delta: AssistChatLogDelta?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val chat_log_delta: AssistChatLogDelta?
val chatLogDelta: AssistChatLogDelta?

Doesn't this work? The code base uses camel case, Jackson will convert the snake case from the API.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed I was too fast while copy pasting from JSON.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Any reason to not have a tool that check for this kind of thing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@TimoPtr TimoPtr mentioned this pull request Mar 20, 2025
@TimoPtr TimoPtr requested a review from jpelgrom March 20, 2025 10:07
@TimoPtr TimoPtr merged commit b497048 into master Mar 24, 2025
15 checks passed
@TimoPtr TimoPtr deleted the feature/intent_progress branch March 24, 2025 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants