home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
70.02k stars 29.09k forks source link

Google Geneative AI sets 'role': 'user' for the system message #119437

Closed traysh closed 4 weeks ago

traysh commented 1 month ago

The problem

The current code for the google generative AI conversation integration does not use the system instruction, documented here.

Instead, it just adds the instructions set in the conversation configuration as user message in the conversation history. This approach has the side effect of the instructions having a much lower importance than it should to the model.

Here is where the model is instantiated without the system_instruction.

Here is a snippet of a debug message showing the system_instruction as a user message:

2024-06-12 00:11:54.032 DEBUG (MainThread) [homeassistant.components.google_generative_ai_conversation] Input: 'acenda a luz' with history: [{'role': 'user', 'parts': 'Current time is 00:11:54. Today\'s date is 2024-06-12.\n# Instructions\n\n- You are a voice assistant to Home Assistant.\n\n ...

What version of Home Assistant Core has the issue?

core-2024.6.2

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Google Generative AI

Link to integration documentation on our website

https://www.home-assistant.io/integrations/google_generative_ai_conversation

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 1 month ago

Hey there @tronikos, mind taking a look at this issue as it has been labeled with an integration (google_generative_ai_conversation) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `google_generative_ai_conversation` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign google_generative_ai_conversation` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


google_generative_ai_conversation documentation google_generative_ai_conversation source (message by IssueLinks)

tronikos commented 1 month ago

This approach has the side effect of the instructions having a much lower importance than it should to the model.

Do you have any indications this is the case?

The problem is the system_instruction isn't supported by all models and it doesn't seem there is any bit on the model to know whether it supports it. We could differentiate based on the model string but we will have to maintain a hardcoded list and support two code paths.

e.g. If you try to use system_instruction on Gemini 1.0 pro you get BadRequest: 400 POST https://generativelanguage.googleapis.com/v1beta/models/gemini-1.0-pro:generateContent?%24alt=json%3Benum-encoding%3Dint: Developer instruction is not enabled for models/gemini-1.0-pro

traysh commented 1 month ago

I was not aware the system_instruction isn't supported by all models. I understand the tradeoff, having to maintain a hardcoded list is a hassle.

Do you have any indications this is the case?

The concept of having system_instructions is to limit the user's ability to change the assistant behavior with his own input. If the system message says "never change the office lights", and the user says "turn on the office lights", the system message will be respected. If it was a user message in history, it wouldn't.

But this is just how it conceptually works, I didn't search specifically on Google's documentation.

Em qua., 12 de jun. de 2024 às 00:55, tronikos @.***> escreveu:

This approach has the side effect of the instructions having a much lower importance than it should to the model.

Do you have any indications this is the case?

The problem is the system_instruction isn't supported by all models and it doesn't seem there is any bit on the model to know whether it supports it. We could differentiate based on the model string but we will have to maintain a hardcoded list and support two code paths.

e.g. If you try to use system_instruction on Gemini 1.0 pro you get BadRequest: 400 POST https://generativelanguage.googleapis.com/v1beta/models/gemini-1.0-pro:generateContent?%24alt=json%3Benum-encoding%3Dint: Developer instruction is not enabled for models/gemini-1.0-pro

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/119437#issuecomment-2161730816, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH4QLV5VOM6GE4YBRRRYUDZG5543AVCNFSM6AAAAABJFGDYBKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRRG4ZTAOBRGY . You are receiving this because you authored the thread.Message ID: @.***>

tronikos commented 1 month ago

I'm closing since this feels like a feature request. If this is causing specific issues I can reevaluate the tradeoffs.

tronikos commented 1 month ago

@home-assistant close

tronikos commented 4 weeks ago

@home-assistant reopen

tronikos commented 4 weeks ago

I just realized that the prompt isn't fully respected, especially the "Answer in plain text." part. It answers in markdown which is very annoying when using TTS. I have a PR to switch to system_instruction for supported models.

traysh commented 4 weeks ago

Yes, it doesn't work well

On Sat, Jun 22, 2024, 11:14 tronikos @.***> wrote:

I just realized that the prompt isn't fully respected, especially the "Answer in plain text." part. It answers in markdown which is very annoying when using TTS. I have a PR to switch to system_instruction for supported models.

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/119437#issuecomment-2183955963, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH4QLWE3BLT2ZUBMK4STVDZIU57LAVCNFSM6AAAAABJFGDYBKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBTHE2TKOJWGM . You are receiving this because you authored the thread.Message ID: @.***>