Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new multi-catalog sales dashboard sample, featuring a Python ADK agent and an Angular client that render UI from two catalogs simultaneously. The review feedback focuses on enhancing the robustness of the implementation through defensive programming. Key suggestions include handling alternative line endings in stream parsing, using optional chaining to guard against missing properties in TypeScript, validating input types before executing string or dictionary operations, and resolving a variable shadowing issue to improve code maintainability.
| const events = buffer.split('\n\n'); | ||
| buffer = events.pop() ?? ''; |
There was a problem hiding this comment.
The stream parser splits the buffer using buffer.split('\\n\\n'). If a network proxy, load balancer, or server middleware normalizes the line endings to \\r\\n\\r\\n, the literal \\n\\n sequence will not be present, causing the split to fail and the stream parsing to break completely. Using a regular expression split like buffer.split(/\\r?\\n\\r?\\n/) ensures robust handling of both \\n\\n and \\r\\n\\r\\n line endings across different network environments.
| const events = buffer.split('\n\n'); | |
| buffer = events.pop() ?? ''; | |
| const events = buffer.split(/\r?\n\r?\n/); | |
| buffer = events.pop() ?? ''; |
| if (event.kind !== 'task' && event.kind !== 'status-update') { | ||
| continue; | ||
| } | ||
| const parts = event.status.message?.parts ?? []; |
There was a problem hiding this comment.
Defensively guard against cases where event.status might be undefined or null. Using optional chaining event.status?.message prevents potential runtime TypeError crashes if the status is missing.
| const parts = event.status.message?.parts ?? []; | |
| const parts = event.status?.message?.parts ?? []; |
References
- In TypeScript, when handling partial objects that may not fully implement an interface, defensively default their properties (e.g., using nullish coalescing or logical OR) to prevent runtime TypeErrors.
| return; | ||
| } | ||
| const result = (response as SendMessageSuccessResponse).result as Task; | ||
| const parts = result.kind === 'task' ? (result.status.message?.parts ?? []) : []; |
There was a problem hiding this comment.
Defensively guard against cases where result.status might be undefined or null. Using optional chaining result.status?.message prevents potential runtime TypeError crashes if the status is missing.
| const parts = result.kind === 'task' ? (result.status.message?.parts ?? []) : []; | |
| const parts = result.kind === 'task' ? (result.status?.message?.parts ?? []) : []; |
References
- In TypeScript, when handling partial objects that may not fully implement an interface, defensively default their properties (e.g., using nullish coalescing or logical OR) to prevent runtime TypeErrors.
| if not isinstance(part.root, DataPart): | ||
| continue | ||
| action = part.root.data.get("action") |
There was a problem hiding this comment.
Defensively verify that part.root.data is a dictionary before calling .get(). If the client sends a non-dictionary data part (or null), accessing .get() directly on part.root.data will raise an AttributeError.
| if not isinstance(part.root, DataPart): | |
| continue | |
| action = part.root.data.get("action") | |
| if not isinstance(part.root, DataPart) or not isinstance(part.root.data, dict): | |
| continue | |
| action = part.root.data.get("action") |
References
- When validation checks or function arguments can be represented as either a dictionary (named) or a list (positional), always verify the type (e.g., using isinstance(args, dict)) before performing key-based lookups or assignments to prevent TypeError crashes.
| regions if the region is unknown. | ||
| """ | ||
| logger.info(f"--- TOOL CALLED: get_sales_metrics (region: {region}) ---") | ||
| key = region.strip().lower() |
There was a problem hiding this comment.
Defensively check if region is a string before calling .strip(). Since LLMs can sometimes pass unexpected types or None to tools, calling .strip() directly on a non-string value will raise an AttributeError and crash the tool execution.
if not isinstance(region, str):
return json.dumps({
"error": f"Invalid region type: {type(region).__name__}",
"knownRegions": [m["region"] for m in _SALES_METRICS.values()],
})
key = region.strip().lower()| const data = part.data as A2uiMessage | A2uiMessage[]; | ||
| messages.push(...(Array.isArray(data) ? data : [data])); |
There was a problem hiding this comment.
The variable name data shadows the outer parameter data of the handleData function. This can lead to developer confusion and is often flagged by linters. Renaming the inner variable to something more descriptive, like payload or uiMessages, improves maintainability.
| const data = part.data as A2uiMessage | A2uiMessage[]; | |
| messages.push(...(Array.isArray(data) ? data : [data])); | |
| const payload = part.data as A2uiMessage | A2uiMessage[]; | |
| messages.push(...(Array.isArray(payload) ? payload : [payload])); |
f3643dc to
4edf47b
Compare
101677d to
03b3802
Compare
The agent is built with the Python agent SDK and answers with two surfaces: one on a custom dashboard catalog and one on the basic catalog. It negotiates a processor per request from the client's keyed capabilities, and validates the response with open_stream when streaming and parse_response otherwise. The Angular client registers both catalogs, sends its capabilities and a useStreaming flag, and uses message/stream or message/send depending on ENABLE_STREAMING. check_e2e.py checks a running agent over A2A in both modes.
…s; log each message The app iterated the surface group's map, which is not a signal, so with zoneless change detection new surfaces only rendered when isLoading changed at the end of the stream. The client now keeps the surfaces in a signal fed by onSurfaceCreated and onSurfaceDeleted. The agent, the client's server and the browser console now log each streamed A2UI message with its time.
The agent, the client's server and the browser console now log each A2UI message as JSON, in both streaming and non-streaming mode, so the catalog of each surface can be checked in the logs.
…I v1.0 v0.9 binds each surface to one catalog. v1.0 lets a component name its own catalogId, so the agent now speaks v1.0 and answers with one dashboard surface: the basic catalog is its default catalog, for the layout, the text and the Refresh button, and each StatCard and ProgressBar names the dashboard catalog. - The dashboard catalog moves to catalogs/1.0 in the v1.0 catalog format, without MetricGrid; basic Rows lay out its components. - The examples move to examples/1.0, with @path bindings and UAX #31 component ids. - The executor reads a2uiRendererCapabilities, the v1.0 metadata key. - The instruction is passed to ADK as a callable, since the v1.0 schemas contain {XID_Start}, which ADK read as a state variable. - A model error ends the task with a message instead of dropping the connection. - check_e2e.py checks that one surface resolves components to both catalogs. - The Angular client uses provideA2UI and the v1.0 BasicCatalog from @a2ui/angular, sends a2uiRendererCapabilities with the v1.0 extension, and its dashboard components use ShadowDom encapsulation, since the renderer mounts them inside the basic catalog's shadow roots.
03b3802 to
f50fe7e
Compare
4edf47b to
2a74ee1
Compare
…/samples-multi-catalog Signed-off-by: Nan Yu <nanyu@google.com>
Signed-off-by: Nan Yu <nanyu@google.com>
…/samples-multi-catalog
…/samples-multi-catalog Signed-off-by: Nan Yu <nanyu@google.com>
Summary
This PR adds a sample that runs the Python agent SDK end to end with two catalogs mixed on one surface, using A2UI v1.0, in both streaming and non-streaming mode. In v0.9 each surface is bound to one catalog. v1.0 lets a component name its own
catalogId, so one surface can mix catalogs.samples/agent/adk/multi_catalog. It answers with onedashboardsurface. The basic catalog is the surface's default catalog, for the layout, the text and a Refresh button. EachStatCardandProgressBarnames the sample's dashboard catalog in its owncatalogId.samples/client/angular/projects/multi_catalog. It registers the v1.0 basic catalog and the dashboard catalog withprovideA2UIfrom@a2ui/angular.Part of #3262 and #3263. The PR is stacked on #3272.
How the agent uses the SDK
A2uiGeneratorholdsCatalogConfig(BasicCatalog(V1_0)),CatalogConfig.from_path("catalogs/1.0/dashboard_catalog.json"), and the example turns inexamples/1.0/.generator.create_processor(capabilities). The capabilities are the keyeda2uiRendererCapabilitiesin the message metadata, the key the v1.0 A2A extension defines. The system prompt isprocessor.prompt_snippetplus the sample's rules.useStreamingin the message metadata.stream_response_to_parts(processor.open_stream(), tokens, version=...)validates each message before it is sent. Each component is checked against the catalog it resolves to.to_a2a_parts(processor.parse_response(text))sends the whole response in the final message.refresh_dashboardaction as a{"version": "v1.0", "action": ...}data part, and the agent shows the dashboard again.Other things the sample handles:
{XID_Start}, which ADK reads as a session state variable in a string instruction. The agent passes the instruction as a callable, which ADK uses as is.Client notes
ViewEncapsulation.ShadowDomto keep their styles.onSurfaceCreatedandonSurfaceDeleted, so a surface renders while the rest of the response streams.Verification
With
gemini-3.6-flash:uv run check_e2e.pysends a question and a Refresh action over A2A in each mode. For each response it checks that there is one surface, that its components resolve to both catalogs, and thatvalidate_payloadaccepts the messages against both catalogs. All checks passed for Europe and North America, for example:In each response, the basic catalog held
Button,Column,RowandText, and the dashboard catalog heldProgressBarandStatCard.In headless Chromium, the built client (
ENABLE_STREAMING=trueandfalse) rendered the dashboard for the question and again after clicking Refresh, with no browser errors. The dashboard components render asa2ui-ng-statcardanda2ui-ng-progressbarinside the basic catalog'sa2ui-basic-row.ng build multi_catalog,eslint,prettier --check,pyink --check, andpytest samples/agent/adk/testspass.Notes
create_processorfails for a client that supports only one of them. The agent then answers with a text message.catalogId. The dashboard catalog'sinstructionsand the sample's UI description say so, and the examples show it.