feat(helpers): add non-text Part, Message, and Artifact helpers#1004
feat(helpers): add non-text Part, Message, and Artifact helpers#1004martimfasantos wants to merge 2 commits intoa2aproject:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces new helper functions (new_data_part, new_raw_part, and new_url_part) to proto_helpers.py for creating various Part message types, along with corresponding unit tests. A suggestion was made to broaden the type hint for new_data_part from dict[str, Any] to Any, as google.protobuf.Value can represent any JSON-serializable type, including lists and scalars.
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/helpers/proto_helpers.py | 94.12% | 95.35% | 🟢 +1.23% |
| Total | 93.02% | 93.03% | 🟢 +0.02% |
Generated by coverage-comment.yml
Part.data is google.protobuf.Value in the v1.0 spec, which requires a non-obvious ParseDict dance to construct from a plain dict. Add new_data_part() to hide that complexity. Also add new_raw_part() and new_url_part() to cover the remaining non-text Part variants that had no helpers, keeping the API consistent with new_text_message() et al.
d44e706 to
e2a1f26
Compare
| Returns: | ||
| A Part with the data field set. | ||
| """ | ||
| return Part(data=ParseDict(data, struct_pb2.Value())) |
There was a problem hiding this comment.
why not including media_type and filename in this case?
There was a problem hiding this comment.
I saw this as a legacy DataPart from 0.3 - something like a dict. In that case, including a media type and file name doesn’t really make sense I believe. If I needed those parameters, I’d use a Raw Part instead. Am I missing something?
Description
CONTRIBUTINGGuide.bash scripts/format.shfrom the repository root to format)Summary
proto_helpers.pyprovidednew_text_messageandnew_text_artifactfor the text Part variant, but nothing for the three remaining Part types (data,raw,url). This PR completes the set.The
datacase is especially awkward without a helper.Part.dataisgoogle.protobuf.Valuein the v1.0 spec, which requires a non-obviousParseDictdance to construct from a plain Python value:New helpers
Part primitives (building blocks, mirror the existing implicit
Part(text=...)pattern):new_data_part(data)data(google.protobuf.Value)new_raw_part(raw, media_type, filename)raw(bytes)new_url_part(url, media_type, filename)url(str)Message helpers (mirror
new_text_message):new_data_message(data, role, context_id, task_id)new_data_partnew_raw_message(raw, media_type, filename, role, context_id, task_id)new_raw_partnew_url_message(url, media_type, filename, role, context_id, task_id)new_url_partArtifact helpers (mirror
new_text_artifact):new_data_artifact(name, data, description, artifact_id)new_data_partnew_raw_artifact(name, raw, media_type, filename, description, artifact_id)new_raw_partnew_url_artifact(name, url, media_type, filename, description, artifact_id)new_url_partChanges
src/a2a/helpers/proto_helpers.py— 9 new helper functionstests/helpers/test_proto_helpers.py— tests for all new helpers (35 total, all passing)Reviewer feedback addressed
new_data_parttype hint broadened fromdict[str, Any]toAny, sincegoogle.protobuf.Valueaccepts any JSON-serializable value, not just dicts. Added a list-value test to cover this.