fix(runner): build a new message when saving input blobs, instead of writing into the caller's Content#1378
Open
svetanis wants to merge 1 commit into
Open
Conversation
…riting into the caller's Content
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
With
saveInputBlobsAsArtifacts(true),Runner.appendNewMessageToSessionreplaces each inline blob witha placeholder by writing into the parts list of the
Contentthe caller passed torunAsync(
Runner.java:376-382).That list is never copied on the way in, so for an immutable one the
setthrowsUnsupportedOperationException— before the model is called, and before anything is appended:builder().parts(List.of(text, blob))List.of, uncopiedbuilder().parts(textBuilder, blobBuilder)ImmutableListContent.fromParts(text, blob)Arrays.asListSolution:
Rewrite a copy of the parts list and build a new
Contentfor the event. The loop moves into ahelper returning both the prepared message and the saves it scheduled:
so
appendNewMessageToSessionbecomes one decision:The functional change is one line —
new ArrayList<>(...), which always acceptsset. Documentedbehaviour is unchanged: the persisted message still carries the placeholder, the blob is still saved, and
the model still never sees it.
Testing Plan
Unit Tests:
Nine tests added. Five fail without the fix (regression tests); four pass in both states (parity tests
pinning behaviour that must not change):
..._immutablePartsList_savesArtifactAndCompletes..._partBuilderPartsList_savesArtifactAndCompletes..._appendedEventReplacesBlobWithPlaceholder..._doesNotModifyCallerMessage..._storesBlobVerbatim..._fromPartsConstruction_savesArtifactAndCompletes..._disabled_keepsBlobAndSavesNothing..._textOnlyMessage_passesThroughUnchanged..._disabledWithTextOnlyMessage_passesThroughUnchangedThe last two cover ordinary traffic: a text-only prompt with the option on (the runner still walks the
parts and, after the fix, copies them), and the default path with the option off (the rewrite is skipped
entirely and the caller's message passes through uncopied).
Manual End-to-End (E2E) Tests:
A demo drives one real agent turn per row through
InMemoryRunner(no custom wiring) againstgemini-2.5-flash, varying only the construction and the flag, and reads back both the artifact storeand the session afterwards.
Before:
After (same demo):
On every row where the offload ran, the payload came back out of storage verbatim, the model was never
shown the blob, and the model was shown the placeholder — so this is not merely "stopped crashing".
Checklist