fix(session/vertexai): preserve FunctionCall and FunctionResponse IDs in deserialization#739
Open
nuthalapativarun wants to merge 3 commits intogoogle:mainfrom
Open
Conversation
… in deserialization aiplatformToGenaiContent was not populating the ID field when converting aiplatformpb.FunctionCall and aiplatformpb.FunctionResponse to their genai equivalents. The write path (createAiplatformpbContent) correctly serialized the ID, but it was lost on read. This caused rearrangeEventsForFunctionResponsesInHistory to map all function calls and responses to the empty string key, silently dropping all but the last function response event in multi-invocation sessions. Fixes google#679
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the conversion logic in vertexai_client.go to include the ID field for FunctionCall and FunctionResponse parts. A new test case was added to verify this mapping. The reviewer suggested refactoring the test into a table-driven format to match the existing codebase style and expanding it to cover additional fields.
…verage Refactor TestAiplatformToGenaiContent into a table-driven test to match the style of other tests in this file. Also verify that Name, Args, and Response fields are correctly preserved alongside ID during conversion.
Contributor
|
Hi @nuthalapativarun The branch is currently out of date. Could you please update it ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to Issue or Description of Change
Description
aiplatformToGenaiContentwas not populating theIDfield when convertingaiplatformpb.FunctionCallandaiplatformpb.FunctionResponseto theirgenaiequivalents. The write path (createAiplatformpbContent) correctly serialized theID, but it was silently dropped on read.Root cause: In
session/vertexai/vertexai_client.go, the struct literals forgenai.FunctionCallandgenai.FunctionResponseinaiplatformToGenaiContentwere missing theIDfield:Impact:
rearrangeEventsForFunctionResponsesInHistoryuses theIDfield to match function calls with their responses. With all IDs being empty strings, it mapped every call and response to the same""key, silently keeping only the last function response and dropping all others fromreq.Contentsin cross-invocation sessions.Testing Plan
Unit Tests:
TestAiplatformToGenaiContent_PreservesFunctionCallAndResponseIDsinsession/vertexai/vertexai_test.goverifying that theIDfield is correctly preserved for bothFunctionCallandFunctionResponseparts after deserialization.Manual E2E Tests:
The bug is deterministic and fully covered by the unit test. Any multi-invocation session with tool calls would previously drop all but the last function response from history; after this fix the IDs are preserved and matching works correctly.
Checklist