-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat(google-genai): Support Gemini system instructions #7235
feat(google-genai): Support Gemini system instructions #7235
Conversation
…ction feat: Add direct system instruction invocation for GenAI [local PR]
feat: Update convertSystemMessageToHumanContent logic [local pr]
…rameter set to false
"False" system message test cases
added test cases for third parameter true
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Hi @jacoblee93 and @afirstenberg - this is our PR to support system instructions for capable Generative AI models based on the implementation plan shared under the issue. If there are any points of concern or improvements, please do not hesitate to let us know. Thank you and have a great rest of your day! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, apologies for the delay in review!
Thanks for the review @jacoblee93! I also want to let you know that I pushed a new commit to fix formatting issues as I did yarn lint but did not do yarn format which is my mistake, should I re-request a review from you? |
Nope all good, about to pull and try it out now |
Thank you! |
@jacoblee93 Thanks for the review! |
…7235) Co-authored-by: Gary Chen <thegary.chen@mail.utoronto.ca> Co-authored-by: martinl498 <martinloo498@gmail.com> Co-authored-by: Shannon Budiman <shannon.budiman@mail.utoronto.ca> Co-authored-by: Jacob Lee <jacoblee93@gmail.com>
Fixes #5069
Currently for Google Gen AI, system message (it can only be the first element in list of messages) are prepended to the following human message which is used to steer the model's behavior. The handling of system messages is the same although some newer gen ai models support direct invocation of system messages via the systemInstructions field per https://ai.google.dev/gemini-api/docs/system-instructions?lang=node
This PR provides support for models that allow direct system instructions by providing an alternative method of handling system messages when possible or set via convertSystemMessageToHumanContent. The changes in this PR are modeled after the changes in google-common to support system instructions (#5089)
High Level Outline of Changes and Additions:
Test Cases created:
Given: Input has single system message followed by one user message
When: convertBaseMessagesToContent is invoked with 3rd parameter set to true
Then: the System message is removed from prompt and passed as systemInstruction field instead and actualPrompt would only have 1 user message under the role of user
Given: Input has single system message followed by one user message
When: convertBaseMessagesToContent is invoked with 3rd parameter set to false
Then: the System message is not removed from prompt as actualPrompt would only have the system message prepended to the user message under the role of user
Given: Input has a system message that is not the first message
When: convertBaseMessagesToContent is invoked with 3rd parameter set to true
Then: convertBaseMessagesToContent should raise the error - "System message should be the first one"
Given: Input has a system message that is not the first message
When: convertBaseMessagesToContent is invoked with 3rd parameter set to false
Then: convertBaseMessagesToContent should raise the error - "System message should be the first one"
Given: Input has multiple system messages
When: convertBaseMessagesToContent is invoked with 3rd parameter set to true
Then: convertBaseMessagesToContent should raise the error - "System message should be the first one"
Given: Input has multiple system messages
When: convertBaseMessagesToContent is invoked with 3rd parameter set to false
Then: convertBaseMessagesToContent should raise the error - "System message should be the first one"
Given: Input has no system message and one user message
When: convertBaseMessagesToContent is invoked with 3rd parameter set to true
Then: actualPrompt would only have the user message under the role of user
Given: Input has no system message and one user message
When: convertBaseMessagesToContent is invoked with 3rd parameter set to false
Then: actualPrompt would only have the user message under the role of user
Given: Input has no system message and multiple user messages
When: convertBaseMessagesToContent is invoked with 3rd parameter set to true
Then: actualPrompt would only have the first user message followed by the next user messages as separate messages
Given: Input has no system message and multiple user messages
When: convertBaseMessagesToContent is invoked with 3rd parameter set to false
Then: actualPrompt would only have the first user message followed by the next user messages as separate messages
Credits:
Issue selection - @shannon-ab
Implementation - @chrisnathan20 @garychen2002
Testing - @shannon-ab @martinl498