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

Fixes #4774: Add error handling for system prompt #4863

Conversation

mdelder
Copy link
Contributor

@mdelder mdelder commented Mar 27, 2025

  • Ensure that Claude models on VertexAI send correct prompts, even if the systemMessage is undefined.

Signed-off-by: "Michael Elder" @mdelder

Description

Provide better error handling for Gemini models that have ChatMessages[] that TextMessagePart(s) instead of just strings.

Checklist

  • [] The relevant docs, if any, have been updated or created
  • [] The relevant tests, if any, have been updated or created
  • I don't often test, but when I do, I do it in production. Jokes aside, I have verified this locally but haven't run or added any additional unit tests for this change.

Screenshots

[ For visual changes, include screenshots. ]

Testing instructions

See #4714 to reproduce the error fixed by this Pull Request.

…prompt

+ Ensure that Claude models on VertexAI send
  correct prompts, even if the systemMessage
  is undefined.

Signed-off-by: "Michael Elder" @mdelder
@mdelder mdelder requested a review from a team as a code owner March 27, 2025 18:26
@mdelder mdelder requested review from RomneyDa and removed request for a team March 27, 2025 18:26
Copy link

netlify bot commented Mar 27, 2025

Deploy Preview for continuedev ready!

Name Link
🔨 Latest commit bb5e8b6
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/67eb6a3e7805fe0008fa6d00
😎 Deploy Preview https://deploy-preview-4863--continuedev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@Patrick-Erichsen Patrick-Erichsen left a comment

Choose a reason for hiding this comment

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

Thanks for both documenting this issue, and for the PR to resolve it! 👌

It feels like a bit of a code-smell that we have this removeSystemMessage fn in the first place, but this PR isn't the place to resolve that.

@Patrick-Erichsen
Copy link
Collaborator

I just pushed up a commit to resolve a small type error: https://github.com/continuedev/continue/actions/runs/14114019429/job/39744113210?pr=4863#step:8:6

I pulled down this branch and wasn't getting any error in Gemini.ts so not sure what the issue there was, but my commit resolved it, along with some default prettier formatting.

@Patrick-Erichsen Patrick-Erichsen merged commit 8d9341a into continuedev:main Apr 1, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants