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

Add support for intent-progress assist events #24143

Merged
merged 2 commits into from
Feb 10, 2025
Merged

Add support for intent-progress assist events #24143

merged 2 commits into from
Feb 10, 2025

Conversation

balloob
Copy link
Member

@balloob balloob commented Feb 9, 2025

Breaking change

Proposed change

Leverage new intent-progress events from home-assistant/core#138095 to show partial responses as soon as they come available. Is backwards compatible with the original events.

CleanShot.2025-02-07.at.22.09.39.mp4

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

if (delta.role) {
// If currentDeltaRole exists, it means we're receiving our
// second or later message. Let's add it to the chat.
// Reuse the previous message if it had no content
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't find where "if it had no content" is happening.

I read this as this block is checking if the existing message has content and appending to it but otherwise dropping the final delta. Is the point here that if there is already content then a delta with an assistant role is empty and ends the message?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the previous message had no content that we shown, the content of hassMessage is still equal to . So we check that on line 501:

hassMessage.text !== "…"

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel i am missing something silly:

  • If it is "..." then it has no content.
  • If it is not "..." then it has content.

Copy link
Contributor

Choose a reason for hiding this comment

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

I read hassMessage.text !== "…" as "the message has content"

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. So if a new message comes in, and the previous message no longer just contains , some content was added. In that case, we add a second message to the log and start adding the deltas to that one. You can see that happen in the video above, where a 2nd "role" comes in, and it starts a new chat bubble.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK i think removing "Reuse the previous message if it had no content" would be helpful since its covered by the comment "new message and previous message has content". That comment implies to me that its part of the if statement, but its not, its part of the fact that the delta is arriving.

if (delta.role) {
// If currentDeltaRole exists, it means we're receiving our
// second or later message. Let's add it to the chat.
if (currentDeltaRole && delta.role && hassMessage.text !== "…") {
Copy link
Contributor

@allenporter allenporter Feb 10, 2025

Choose a reason for hiding this comment

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

OK so can hassMessage.text ever be ... in this case? If so, then it seems like we drop the delta. (If not then just always overwrite instead and drop the check for hassMessage.text).

The way i read this is: The existing value should just decide of ... is stripped but as written it also deciding if the delta is appending.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still now confused by:

  • we update hassmessage.text
  • then replace hassmessage entirely

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it can be if the first message only contained tool calls and no content.

Copy link
Member Author

Choose a reason for hiding this comment

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

if we don't have this check, we will end up with an empty speech bubble (because the Assistant message had no content.

Copy link
Member Author

Choose a reason for hiding this comment

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

  • User to Assistant: Turn on the lights in the living room
  • Assistant to HA: tool-call: turn on lights
  • HA to Assistant: tool-call-result: lights turned on
  • Assistant to User: the lights have been turned on

Copy link
Contributor

@allenporter allenporter Feb 10, 2025

Choose a reason for hiding this comment

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

What is this doing? Why update .text to then overwrite the entire object?

hassMessage.text = ...
hassMessage = {...}

Copy link
Member Author

Choose a reason for hiding this comment

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

hassMessage is a pointer to an object that is already being rendered. When we update text and call request update, it re-renders it. Add message also rerenders it.

While we’re receiving content, we always have … at the end of the message that we’re processing (see video). When a second message comes in, we remove the … from the current last message and then add a new message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, got it.

@balloob balloob merged commit f3d5544 into dev Feb 10, 2025
15 checks passed
@balloob balloob deleted the assist-streaming branch February 10, 2025 04:36
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.

2 participants