feat(plugins): add SaveFilesAsArtifactsPlugin, porting adk-python's file-upload offload - #1412
Open
svetanis wants to merge 1 commit into
Open
feat(plugins): add SaveFilesAsArtifactsPlugin, porting adk-python's file-upload offload#1412svetanis wants to merge 1 commit into
svetanis wants to merge 1 commit into
Conversation
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:
adk-python deprecated
save_input_blobs_as_artifactsin favour ofSaveFilesAsArtifactsPlugin.adk-java has the parameter but not the plugin, so
RunConfig.saveInputBlobsAsArtifacts(true)is theonly way to keep uploaded bytes out of every later LLM request — and it loses two things:
Runnernames every artifactartifact_{invocationId}_{index}and never readsBlob.displayName, so an upload ofreport.pdfis stored under an opaque id.
EventActions.artifactDeltastays empty and the session's artifact bookkeeping never records theupload.
Solution:
A new
SaveFilesAsArtifactsPluginincom.google.adk.plugins, registered like any other plugin. Foreach
inlineDatapart of the incoming user message it saves the part to the configuredBaseArtifactService— named fromBlob.displayName, falling back to the existingartifact_{invocationId}_{index}— replaces it with[Uploaded Artifact: "<name>"]in the messagethat reaches the model and the session, and reports the saved versions through
EventActions.artifactDelta. A failed save keeps the original part and logs, without failing theinvocation. The failure is reported only through the logger, matching adk-python — so an artifact
service that is misconfigured or unreachable degrades silently to no offload, with each upload
continuing to reach the model inline.
No existing file is modified, and the only new public surface is the plugin class itself — the
hand-off helper is package-private. Registration uses the three surfaces that already exist
(
Runner.Builder.plugins,App.Builder.plugins,InMemoryRunner).Four new files, all in
com/google/adk/plugins/:core/src/main/java/…/SaveFilesAsArtifactsPlugin.javacore/src/main/java/…/PendingArtifactDelta.javaonUserMessageCallback→beforeAgentCallbackhand-offcore/src/test/java/…/SaveFilesAsArtifactsPluginTest.javacore/src/test/java/…/PendingArtifactDeltaTest.javaPendingArtifactDeltais package-private and tested on its own — no new package, no new public type.Four deliberate divergences from adk-python:
temp:{plugin}:pending_delta:{invocationId}).Python keys on the plugin name alone, so a stash stranded by a failed invocation is drained by the
next invocation in that session, reporting artifacts it never saved. The invocation id makes a
stranded entry inert.
State.remove.removewrites theState.REMOVEDsentinel into the event's state delta, which does not survive a JSON round trip.Maybe.empty(), where Python returns theunmodified message. Same outcome for this plugin, but
PluginManagerearly-exits on the firstnon-empty return — so Python's form also suppresses every later plugin's
on_user_message_callbackwhenever this one is disabled. An emptyMaybecannot.saving it, so the file is dropped rather than offloaded. The cap guards the inline-request limit,
but the blob has already been replaced by a placeholder here, so it never reaches the model either
way. This port saves whatever it is handed and lets the artifact service decide what it can hold.
attach_file_referenceis deliberately not ported. It needsget_artifact_version(...).canonical_uri, which has no equivalent onBaseArtifactService— addingit means changing the artifact-service SPI and every implementation. That is a separate, larger,
separately reviewable change. This port is placeholder-only, which is behaviourally what
RunConfig.saveInputBlobsAsArtifactsalready does.Testing Plan
Unit Tests:
38 tests across two classes, mirroring
GlobalInstructionPluginTest. Both contexts are mocked;State,SessionandInMemoryArtifactServiceare real, so thetemp:state hand-off between thehooks is exercised rather than stubbed.
SaveFilesAsArtifactsPluginTest(26) covers the display-name vs generated-name choice, theplaceholder swap, part order, multiple and duplicate uploads, the artifact delta and its drain, the
afterRunCallbackcleanup, the failed-save and partial-failure paths, both constructors, and that the caller'sContentcomes back untouched, for all three ways genai lets you build one.PendingArtifactDeltaTest(12) covers the hand-off's own rules: thetemp:key convention,invocation and plugin scoping, clearing by overwrite rather than
State.remove, reading back statethat has lost its type information, and that a second drain writes nothing — a write would set the
state delta, and
BaseAgentemits an event for any before-agent callback that leaves one.Manual End-to-End (E2E) Tests:
gemini-3.5-flash.A six-row harness builds a
Runnerexplicitly (anInMemoryRunnercannot substitute a failingartifact service) and registers this PR's plugin class, so every row below exercises the code in
this diff rather than a copy of it. Below, flag means
RunConfig.saveInputBlobsAsArtifacts(true)and plugin meansSaveFilesAsArtifactsPlugin.artifact_<invocationId>_1,artifactDelta(none)report.pdf,artifactDelta{report.pdf=0}artifact_<invocationId>_1— parity with the flagA and B are the change, side by side:
Both offload the payload losslessly and both show the model a placeholder — the change is the file
name and the bookkeeping, not the offload.
Row F is the migration row: the plugin strips every
inlineDatapart before the runner's flag loopsees the message, so enabling both is safe and there is no double save.
Checklist