fix: add system prompt to google gen-ai request - #10
Conversation
JS-1218 JS Google GenAI - response model missing
|
a586bee to
e26bc8c
Compare
Current versions of google's genai SDK allow passing the system prompt separate from the user messages. Update test to accurately track that we're including this in the span.
e26bc8c to
a36f2e9
Compare
| contents, | ||
| contents: prompt, | ||
| config: { | ||
| systemInstruction: [ system ], |
There was a problem hiding this comment.
Bug: The systemInstruction in config is incorrectly passed as an array [system] instead of a string.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The generateContent() call will fail at runtime because the systemInstruction parameter within the config object is incorrectly passed as an array [system] instead of a string. The Google GenAI JavaScript SDK expects systemInstruction to be a string, which will lead to either a validation error from the API or an SDK error during parameter processing. This prevents the system instruction from being properly applied, causing the API call to fail.
💡 Suggested Fix
Change systemInstruction: [ system ] to systemInstruction: system in the config object to pass the system string directly, aligning with SDK documentation.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: sdks/js/google-genai/cases/1-simple.js#L23
Potential issue: The `generateContent()` call will fail at runtime because the
`systemInstruction` parameter within the `config` object is incorrectly passed as an
array `[system]` instead of a string. The Google GenAI JavaScript SDK expects
`systemInstruction` to be a string, which will lead to either a validation error from
the API or an SDK error during parameter processing. This prevents the system
instruction from being properly applied, causing the API call to fail.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6609577
There was a problem hiding this comment.
This actually can be a PartListUnion, so a Part object, string, or array of Part objects and strings. (Annoying, I know, but including it this way helped flush out a bug in the JS SDK, actually.)
Move the message reformatting into a separate util for google-genai, and add unit test coverage for that file. Add an integration test scenario to ensure that the system message will be included if provided in the config params. Related to getsentry/testing-ai-sdk-integrations#10 Fix JS-1218
Move the message reformatting into a separate util for google-genai, and add unit test coverage for that file. Add an integration test scenario to ensure that the system message will be included if provided in the config params. Related to getsentry/testing-ai-sdk-integrations#10 Fix JS-1218
Move the message reformatting into a separate util for google-genai, and add unit test coverage for that file. Add an integration test scenario to ensure that the system message will be included if provided in the config params. Related to getsentry/testing-ai-sdk-integrations#10 Fix JS-1218
Move the message reformatting into a separate util for google-genai, and add unit test coverage for that file. Add an integration test scenario to ensure that the system message will be included if provided in the config params. Related to getsentry/testing-ai-sdk-integrations#10 Fix JS-1218
Current versions of google's genai SDK allow passing the system prompt separate from the user messages.
Update test to accurately track that we're including this in the span.
Related to JS-1218