fix(artifacts): preserve inline display names#5914
Conversation
|
I have analyzed Pull Request #5914, which addresses a key bug in the ADK repository where the I have compiled the comprehensive analysis into a premium PR Analysis Report artifact. Please inspect the report directly here: 📖 analysis_report.md Key Summary of the Analysis
Open Decision Points & Refinements (Nits)I recommend asking the contributor to address the following minor styling nits so the codebase maintains strict style alignment:
|
|
Hi @he-yufeng , Thank you for your contribution! We appreciate you taking the time to submit this pull request. |
6af8a81 to
9992407
Compare
|
Rebased this branch onto current main and force-pushed the cleaned head. The PR diff is still limited to the artifact display-name roundtrip change. Validated locally:
|
|
I have completed the analysis for PR #5914, which addresses a key bug where the original display name of binary inline artifacts was lost during save/load pathways on disk and cloud backends. Below is a summary of the analysis. You can also view the full detailed analysis in the premium report artifact: 🔍 ADK Pull Request Analysis: PR #5914Title: Executive Summary
Minor Refinement Points (Typing Nits)To match ADK's modern typing conventions, we should request that the contributor modernize newly added or updated parameter/field annotations:
|
Summary
FileArtifactServiceandGcsArtifactServicecurrently preserve the bytes and MIME type ofinline_dataartifacts, but dropinline_data.display_nameon load. That leaves downstream converters without the user-facing filename even when the caller provided one.This stores the display name alongside the artifact payload and restores it when loading binary artifacts:
custom_metadataremains unchanged when artifact version metadata is listed or fetchedFixes #5833.
Tested
PYTHONPATH=src python -m pytest tests/unittests/artifacts/test_artifact_service.py::test_save_load_preserves_inline_data_display_name -qPYTHONPATH=src python -m py_compile src/google/adk/artifacts/file_artifact_service.py src/google/adk/artifacts/gcs_artifact_service.py tests/unittests/artifacts/test_artifact_service.pypython -m pyink --check src/google/adk/artifacts/file_artifact_service.py src/google/adk/artifacts/gcs_artifact_service.py tests/unittests/artifacts/test_artifact_service.pygit diff --checkI also ran the full artifact-service test file locally with
PYTHONPATH=src. It passed all GCS/in-memory/display-name cases, with two existing Windows-only failures in tests that parsefile:///C:/...URIs back withPath(unquote(parsed.path)), producing/C:/...; those are unrelated to this change.pre-commit run --files ...passed the format/import hooks, but the two repository-local bash hooks could not run on this Windows checkout because/bin/bashis unavailable.