music-assistant / hass-music-assistant

Turn your Home Assistant instance into a jukebox, hassle free streaming of your favorite media to Home Assistant media players.
Apache License 2.0
1.32k stars 49 forks source link

Expose the query slot in MassPlayMediaOnMediaPlayer only if the mass specific LLM agent is configured #2685

Closed tronikos closed 2 months ago

tronikos commented 2 months ago

fixes #2636

OzGav commented 2 months ago

Do we need that note we just added to the docs anymore if this gets approved? https://music-assistant.io/integration/voice/

tronikos commented 2 months ago

Do we need that note we just added to the docs anymore if this gets approved? https://music-assistant.io/integration/voice/

No

OzGav commented 2 months ago

OK so why would it fail now if the query argument is not used?

tronikos commented 2 months ago

It won't fail. If you haven't setup the MA Specific Conversation Agent the LLM won't see the query argument.

Keep in mind there is the general LLM, that you interact with, that sees all your exposed entities and all the registered intents. This is the LLM that decides when and how to call MassPlayMediaOnMediaPlayer. If you don't have an MA specific LLM configured, the general LLM won't know the query argument so it will never use it. If you have configured an MA specific LLM, the general LLM will be able to use the query if it decides so. And when the query is specified, the intent will internally call its MA specific LLM to translate the query to artist/track/album.

OzGav commented 2 months ago

Yes that’s what I thought so why would we need to keep

If you choose not to setup the MA Specific Conversation Agent then you may encounter errors unless you tell the LLM via the prompt to not use the query argument for the MassPlayMediaOnMediaPlayer intent

tronikos commented 2 months ago

We should remove it. We don't need it. That's what I said earlier. You asked me: "Do we need that note we just added to the docs anymore if this gets approved?" and I answered: "No"

OzGav commented 2 months ago

Sorry I thought I asked should we remove it 🤦‍♂️

jozefKruszynski commented 2 months ago

Excellent solution, I tried to do similar, but still don't know enough about python to know that we could simply overrider that property function. Thanks @tronikos for putting the time in.