Repository navigation
Conversation
…RequestProcessor and RendererState - Allow SendA2uiToClientToolset to accept structured example turns (such as processor.examples) or None in addition to markdown strings, and avoid duplicating schema or example instructions already present in llm_request.config.system_instruction. - Add optional a2ui_state (RendererState or A2uiStateProvider) to SendA2uiToClientToolset so tool-based Canvas agents validate and apply payloads against a session RendererState. - Add optional state parameter to parse_content_to_parts in a2ui.a2a. - Allow A2uiPartConverter to accept processor and state, and infer version from the active catalogs' protocol_version when omitted. - Update A2uiEventConverter to read system:a2ui_processor, system:a2ui_state, and system:a2ui_stream from session state, reuse A2uiPartConverter across events, and stream partial ADK events via ResponseStream. Signed-off-by: Nan Yu <nanyu@google.com>
There was a problem hiding this comment.
Code Review
This pull request enhances the A2UI ADK library to support Canvas UI states, structured example turns, and incremental streaming. Key updates include allowing SendA2uiToClientToolset to accept structured example turns and an optional a2ui_state for validation, updating A2uiPartConverter to accept a processor and state, and enabling A2uiEventConverter to support incremental streaming via ResponseStream. Feedback on these changes suggests avoiding Python object ID recycling issues by tracking ResponseStream objects directly instead of using id(stream), and improving type flexibility by checking for callable and Iterable instead of Sequence when handling collections of examples.
| self._stream_key = stream_key | ||
| self._cached_converter_key: tuple[Any, ...] | None = None | ||
| self._cached_part_converter: A2uiPartConverter | None = None | ||
| self._active_stream_ids: set[int] = set() |
There was a problem hiding this comment.
Using id(stream) as an identifier in a set can lead to subtle bugs due to object ID recycling in Python. If a stream object is garbage collected and a new one is allocated at the same memory address, a non-streaming response might be incorrectly identified as an active stream. It is safer to store the stream objects directly in a set (or a WeakSet from weakref to avoid keeping aborted streams alive).
| self._active_stream_ids: set[int] = set() | |
| self._active_streams: set[ResponseStream] = set() |
| stream_id = id(stream) | ||
| flushed_final = False | ||
|
|
||
| def _convert_part(part: genai_types.Part) -> list[a2a_types.Part]: | ||
| nonlocal flushed_final | ||
| if part.text is not None: | ||
| if is_partial: | ||
| self._active_stream_ids.add(stream_id) | ||
| try: | ||
| return to_a2a_parts( | ||
| stream.feed(part.text), | ||
| version=base_converter._version, | ||
| ) | ||
| except Exception as e: | ||
| logger.warning("Failed to stream A2UI chunk: %s", e) | ||
| return [] | ||
| if stream_id in self._active_stream_ids: | ||
| self._active_stream_ids.discard(stream_id) |
There was a problem hiding this comment.
Update the stream tracking logic to use the stream object directly instead of id(stream) to prevent ID recycling issues.
| stream_id = id(stream) | |
| flushed_final = False | |
| def _convert_part(part: genai_types.Part) -> list[a2a_types.Part]: | |
| nonlocal flushed_final | |
| if part.text is not None: | |
| if is_partial: | |
| self._active_stream_ids.add(stream_id) | |
| try: | |
| return to_a2a_parts( | |
| stream.feed(part.text), | |
| version=base_converter._version, | |
| ) | |
| except Exception as e: | |
| logger.warning("Failed to stream A2UI chunk: %s", e) | |
| return [] | |
| if stream_id in self._active_stream_ids: | |
| self._active_stream_ids.discard(stream_id) | |
| flushed_final = False | |
| def _convert_part(part: genai_types.Part) -> list[a2a_types.Part]: | |
| nonlocal flushed_final | |
| if part.text is not None: | |
| if is_partial: | |
| self._active_streams.add(stream) | |
| try: | |
| return to_a2a_parts( | |
| stream.feed(part.text), | |
| version=base_converter._version, | |
| ) | |
| except Exception as e: | |
| logger.warning("Failed to stream A2UI chunk: %s", e) | |
| return [] | |
| if stream in self._active_streams: | |
| self._active_streams.discard(stream) |
| if callable(a2ui_catalogs) | ||
| else self._to_catalog_list(a2ui_catalogs) | ||
| ) | ||
| self._a2ui_examples = a2ui_examples |
There was a problem hiding this comment.
When distinguishing between a callable provider and a collection of items, check callable() to identify the provider rather than checking isinstance(..., Sequence). If a collection is passed directly, convert it to a list once (e.g., in the constructor) to support generators/sets without exhausting them, and explicitly reject single items, strings, or bytes with a clear TypeError.
if a2ui_examples is not None:
if callable(a2ui_examples):
self._a2ui_examples = a2ui_examples
elif isinstance(a2ui_examples, (str, bytes)):
raise TypeError("a2ui_examples cannot be a string or bytes")
elif isinstance(a2ui_examples, collections.abc.Iterable):
self._a2ui_examples = list(a2ui_examples)
else:
raise TypeError("a2ui_examples must be a callable provider or an iterable collection")References
- When distinguishing between a callable provider and a collection of items in Python, check callable() to identify the provider rather than checking isinstance(..., Sequence). If a collection is passed directly, convert it to a list once (e.g., in the constructor) to support generators/sets without exhausting them, and explicitly reject single items, strings, or bytes with a clear TypeError.
| if isinstance(examples, Sequence) and not isinstance( | ||
| examples, (str, bytes) | ||
| ): | ||
| if not examples: | ||
| return "" | ||
| decompiler = DirectJsonDecompiler() | ||
| blocks = [ | ||
| f"<a2ui-json>\n{decompiler.decompile(turn)}\n</a2ui-json>" | ||
| for turn in examples | ||
| ] | ||
| return "### Examples:\n" + "\n\n".join(blocks) |
There was a problem hiding this comment.
To support any iterable examples (such as generators or sets) returned by dynamic providers, check isinstance(examples, Iterable) instead of Sequence, and convert it to a list before processing.
if isinstance(examples, Iterable) and not isinstance(
examples, (str, bytes)
):
examples_list = list(examples)
if not examples_list:
return ""
decompiler = DirectJsonDecompiler()
blocks = [
f"<a2ui-json>\n{decompiler.decompile(turn)}\n</a2ui-json>"
for turn in examples_list
]
return "### Examples:\n" + "\n\n".join(blocks)
Summary
Connects
python/a2ui_adkto theA2uiRequestProcessor,RendererState, andResponseStreamarchitecture introduced in #3268–#3271:SendA2uiToClientToolset:Sequence[Sequence[AgentToRendererMessage]], such asprocessor.examples) orNonein addition to markdown strings, fixing theTypeErrorwhen passingprocessor.examples.process_llm_request()when they are already present inllm_request.config.system_instruction.a2ui_state(RendererStateorA2uiStateProvider) to validate and apply tool payloads against a CanvasRendererState.A2uiPartConverter&parse_content_to_parts:parse_content_to_partsaccepts an optionalstate: RendererState | None.A2uiPartConverteraccepts optionalprocessor: A2uiRequestProcessor | Noneandstate: RendererState | None, and infersversionfrom the active catalogs'protocol_versionwhenversionis omitted.A2uiEventConverter:system:a2ui_processor,system:a2ui_state, andsystem:a2ui_streamfrom session state alongsidesystem:a2ui_catalogs.A2uiPartConverteracross events instead of instantiating a throw-away converter and parser on every ADK event.ResponseStreamwhen partial ADK events (event.partial == True) arrive.Test Plan
uv run pytest python/a2ui_a2a/tests python/a2ui_adk/testsuv run mypy python/a2ui_a2a/src python/a2ui_adk/src