Skip to content

fix(google-genai): handle tool_calls when the message contains other content #8005

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

Merged
merged 7 commits into from
May 13, 2025

Conversation

andantonyan
Copy link
Contributor

This update modifies convertMessageContentToParts to ensure that tool_calls on a message are processed even when the message also contains content (either as a string or an array). Previously, the presence of content could lead to tool_calls being overlooked. This change correctly converts both textual/array content and any accompanying tool_calls into their respective Google AI Part structures within the same message conversion.

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 10, 2025
Copy link

vercel bot commented Apr 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchainjs-docs ✅ Ready (Inspect) Visit Preview May 12, 2025 1:48pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchainjs-api-refs ⬜️ Ignored (Inspect) May 12, 2025 1:48pm

@dosubot dosubot bot added the auto:refactor A large refactor of a feature(s) or restructuring of many files label Apr 10, 2025
@jacoblee93
Copy link
Collaborator

CC @afirstenberg if you have a minute!

@exi
Copy link

exi commented May 7, 2025

Whats the status on this? I'm seeing the issue when using Librechat with a Gemini-based agent. Leading to failed requests because langchain-google-genai is dropping the tool messages.

@danny-avila
Copy link

@benjamincburns @afirstenberg hoping to see this merged, lgtm!

@benjamincburns benjamincburns changed the title refactor(common): restructure convertMessageContentToParts refactor(google-gen-ai): restructure convertMessageContentToParts May 8, 2025
@benjamincburns benjamincburns changed the title refactor(google-gen-ai): restructure convertMessageContentToParts refactor(google-genai): restructure convertMessageContentToParts May 8, 2025
@benjamincburns
Copy link
Contributor

benjamincburns commented May 8, 2025

@andantonyan if you're still around & able would you be willing to add one or more tests for this? I'm afraid that the complexity here is a bit beyond what I'm comfortable merging without test updates. That said, if @afirstenberg is able to review and is happy w/ the change, then I'm happy to merge & release.

@andantonyan
Copy link
Contributor Author

Hi @benjamincburns ,

Thanks for your comment and the review.

Regarding the tests, what kind of tests would you like to see added? I'll be available from mid next week and will be happy to allocate some time for this.

As you mentioned, let's also wait to hear from @afirstenberg – perhaps if they are happy with the changes, additional tests won't be needed for merging.

Thanks

@benjamincburns
Copy link
Contributor

benjamincburns commented May 8, 2025

what kind of tests would you like to see added?

Ideally readable unit tests that target each branch of convertMessageContentToParts, but I'll take what I can get. I think the bare minimum would be one or more tests (ideally unit tests) that fail prior to your change, pass after it, and would be sufficient to prevent this issue from regressing.

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels May 12, 2025
@andantonyan
Copy link
Contributor Author

Hi @benjamincburns

I've added the unit tests for convertMessageContentToParts as you suggested. They cover the key scenarios for handling AIMessages with both content and tool_calls, plus relevant regressions.

Ready for another look when you get a chance.

@dosubot dosubot bot added the lgtm PRs that are ready to be merged as-is label May 13, 2025
@benjamincburns
Copy link
Contributor

Thanks for the contribution @andantonyan!

@benjamincburns benjamincburns changed the title refactor(google-genai): restructure convertMessageContentToParts fix(google-genai): handle tool_calls when the message contains other content May 13, 2025
@benjamincburns benjamincburns merged commit ff96850 into langchain-ai:main May 13, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:refactor A large refactor of a feature(s) or restructuring of many files google lgtm PRs that are ready to be merged as-is size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants