Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces state-binding capabilities to A2uiRequestProcessor, allowing it to accept, create, and validate a RendererState during response parsing and streaming. It also updates the processor to reuse a single Parser instance. The review feedback suggests comparing the active and held catalogs as sets rather than lists in _check_state to prevent false-positive A2uiCatalogError exceptions caused by differing catalog order.
aaefb04 to
03bc630
Compare
03bc630 to
cf7201b
Compare
A parser holds no state now, so the processor creates one and reuses it, and the caller chooses how long the renderer state lives. 1. `parser` returns the one parser the processor reuses. `create_parser()` is removed. 2. `create_state()` creates an empty `RendererState` that holds the active catalogs. 3. `A2uiRequestProcessor(state=...)` and `A2uiGenerator.create_processor(..., state=...)` bind a state, so a canvas agent's `parse_response` and the new `open_stream` use it when a call passes none. Without one, each call checks the response against a new state, as a chat agent wants. A state that holds other catalogs than the processor is rejected. Part of #3263. Signed-off-by: Nan Yu <nanyu@google.com>
cf7201b to
bd0e829
Compare
gspencergoog
left a comment
There was a problem hiding this comment.
Binds an optional RendererState to A2uiRequestProcessor (constructor and A2uiGenerator.create_processor), adds create_state(), open_stream(state=) and parse_response(state=), and replaces create_parser() with a cached parser property. The processor code holds up: the ordered catalog comparison matches how RendererState resolves v0.8 surfaces and placeholders, every built-in parser holds only configuration so reuse is safe, and a failed parse_response turn rolls the bound state back. Both existing review threads are addressed at a67204fec.
The one concern is sequencing within the stack: the breaking removal of create_parser() lands here while its guide update (#3271) and sample migrations (#3272) land two merges later, and ci-samples is skipped on this PR. Commenting rather than requesting changes, since the fixes exist in the stack.
🤖 Agent-generated
| """Creates a fresh Parser instance bound to the active catalogs.""" | ||
| return self._format.create_parser() | ||
| @property | ||
| def parser(self) -> Parser: |
There was a problem hiding this comment.
Removing create_parser() lands before its callers migrate: agent_development.md:20,234,249 and both workspace samples (restaurant_finder/agent.py:300, custom-components-example/agent.py:524) still call it. #3271 and #3272 fix them, so main is broken only between those merges, and ci-samples is skipped here so CI will not flag it. samples/community/agent/adk/gemini_enterprise/v0_9/agent.py:339 (PyPI-pinned) is in neither PR.
Consider moving the removal into #3272, or merging #3270 through #3272 together, and adding the gemini_enterprise/v0_9 call site to #3272.
🤖 Agent-generated
There was a problem hiding this comment.
…m' into feat/python-agent-processor-state Signed-off-by: Nan Yu <nanyu@google.com>
- accepts_state(state) tells whether a state holds the processor's active catalogs in order, so an agent that keeps a state per conversation can tell when to make a new one; _check_state raises when it's False. - Say that a processor bound to a state serves only that conversation. - Test that A2uiGenerator.create_processor forwards state, and rejects a state for other catalogs. Signed-off-by: Nan Yu <nanyu@google.com>
… SDKs Addresses the review of #3283, checking each rule against the Python (#3270), TypeScript (#3279) and Dart (#3277) pull requests: 1. `compile` takes `is_final`; a cut block is applied and its surfaces skip `check_topology` and are marked checked, and a cut Direct JSON block raises. `parse_response` errors carry `partial_results`. 2. Language-neutral wording for named arguments, the unsupported-stream error, and the callback form of `transaction` (returns the callback's result, must be synchronous). 3. A failing `feed` undoes only what that chunk applied; a failed stream marks the surfaces it emitted to as checked. 4. `RendererState.mark_checked`, and what `check_topology` does with duplicate, unknown, deleted and assumed ids. 5. The undo log records the state's own bookkeeping, copies the properties an in-place update replaces, keeps each component's `catalogId` and the v0.8 buffer when replaying a deleted surface, and treats a nested transaction as a position in the log. 6. How a v0.8 state buffers updates before `beginRendering`. 7. How `validate_payload` picks the catalog of an assumed surface, and that it accepts one message on its own. 8. The three linear-cost rules are requirements for every SDK. 9. `A2uiRequestProcessor.accepts_state`, used in the code example. 10. The conformance plan says what `parse_chunk` runs and that no suite covers RendererState, canvas turns or validate_payload yet. 11. A "Where the SDKs differ" list of the gaps the SDKs still have. Signed-off-by: Nan Yu <nanyu@google.com>
…nt-processor-state Signed-off-by: Nan Yu <nanyu@google.com>
Part of #3263. Stacked on #3269.
A parser holds no state now, so
A2uiRequestProcessorcreates one and reuses it, and the caller chooses how long the renderer state lives.parserproperty returns the one parser the processor reuses. Breaking:create_parser()is removed.create_state()creates an emptyRendererStatethat holds the active catalogs.A2uiRequestProcessor(state=...)andA2uiGenerator.create_processor(..., state=...)bind a state.parse_responseand the newopen_streamuse it when a call passes none. Without one, each call checks the response against a new state, as a chat agent wants.A2uiCatalogError, at binding time or when passed to a call.Testing
test_renderer_state_parsing.py: canvas turns against a passed state, a bound state used by default, and a state for other catalogs rejected.uv run pytest python/a2ui_agent/tests python/a2ui_a2a/tests python/a2ui_adk/tests samples/agent/adk/tests: 1470 passed, 2 xfailed.uv run mypy python/a2ui_agent/src python/a2ui_a2a/src: no issues.