Fix inline catalog validation and multi-surface support#831
Fix inline catalog validation and multi-surface support#831nan-yu merged 3 commits intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
The pull request refactors the UI catalog selection and validation logic to support multi-surface rendering and incremental updates. The _select_catalog function now merges inline catalogs with a base catalog, and the component validation (_validate_component_integrity, _validate_topology) distinguishes between initial renders (where root component and orphan checks apply) and incremental updates (where these checks are skipped). Corresponding test cases have been added or updated to reflect this new behavior. Additionally, the OrgChart component schema was updated to allow chain to be either a path reference or an array, and the action schema was detailed. A review comment suggests improving the readability and maintainability of a long JSON string in prompt_builder.py by defining it as a Python dictionary and then serializing it.
| client_ui_capabilities_str = ( | ||
| '{"inlineCatalogs":[{"catalogId": "inline_catalog",' | ||
| ' "components":{"OrgChart":{"type":"object","properties":{"chain":{"type":"array","items":{"type":"object","properties":{"title":{"type":"string"},"name":{"type":"string"}},"required":["title","name"]}},"action":{"$ref":"#/definitions/Action"}},"required":["chain"]},"WebFrame":{"type":"object","properties":{"url":{"type":"string"},"html":{"type":"string"},"height":{"type":"number"},"interactionMode":{"type":"string","enum":["readOnly","interactive"]},"allowedEvents":{"type":"array","items":{"type":"string"}}}}}}]}' | ||
| ' "components":{"OrgChart":{"type":"object","properties":{"chain":{"oneOf":[{"type":"object","properties":{"path":{"type":"string"}},"required":["path"]},{"type":"array","items":{"type":"object","properties":{"title":{"type":"string"},"name":{"type":"string"}},"required":["title","name"]}}]},"action":{"type":"object","properties":{"name":{"type":"string"},"context":{"type":"array","items":{"type":"object","properties":{"key":{"type":"string"},"value":{"type":"object","properties":{"path":{"type":"string"},"literalString":{"type":"string"},"literalNumber":{"type":"number"},"literalBoolean":{"type":"boolean"}}}},"required":["key","value"]}}},"required":["name"]}},"required":["chain"]},"WebFrame":{"type":"object","properties":{"url":{"type":"string"},"html":{"type":"string"},"height":{"type":"number"},"interactionMode":{"type":"string","enum":["readOnly","interactive"]},"allowedEvents":{"type":"array","items":{"type":"string"}}}}}}]}' | ||
| ) |
There was a problem hiding this comment.
This long, single-line JSON string is difficult to read and maintain. To improve readability and reduce the chance of syntax errors, consider defining this structure as a Python dictionary and then serializing it to a JSON string using json.dumps.
client_ui_capabilities_dict = {
"inlineCatalogs": [{
"catalogId": "inline_catalog",
"components": {
"OrgChart": {
"type": "object",
"properties": {
"chain": {"oneOf": [
{"type": "object", "properties": {"path": {"type": "string"}}, "required": ["path"]},
{"type": "array", "items": {"type": "object", "properties": {"title": {"type": "string"}, "name": {"type": "string"}}, "required": ["title", "name"]}}
]},
"action": {"type": "object", "properties": {
"name": {"type": "string"},
"context": {"type": "array", "items": {"type": "object", "properties": {
"key": {"type": "string"},
"value": {"type": "object", "properties": {
"path": {"type": "string"}, "literalString": {"type": "string"},
"literalNumber": {"type": "number"}, "literalBoolean": {"type": "boolean"}
}}
}, "required": ["key", "value"]}}
}, "required": ["name"]}
},
"required": ["chain"]
},
"WebFrame": {
"type": "object",
"properties": {
"url": {"type": "string"}, "html": {"type": "string"}, "height": {"type": "number"},
"interactionMode": {"type": "string", "enum": ["readOnly", "interactive"]},
"allowedEvents": {"type": "array", "items": {"type": "string"}}
}
}
}
}]
}
client_ui_capabilities_str = json.dumps(client_ui_capabilities_dict)193b962 to
7b6f441
Compare
zeroasterisk
left a comment
There was a problem hiding this comment.
Thanks for the thorough fix, @sarthak96agarwal — the multi-surface root tracking and inline catalog merging changes are solid.
Heads up: @nanyu is actively working on this area of the codebase. Before we merge, could you rebase onto the latest main and confirm there are no conflicts with recent changes? That'll make it easier for Nan to review and coordinate.
Thanks!
- Validator: support multiple surfaces with different root IDs by tracking per-surface roots instead of a single global root - Validator: distinguish initial renders (beginRendering/createSurface) from incremental updates; skip root and orphan checks on updates while still catching cycles, self-refs, and duplicates - Manager: merge inline catalog components onto the base catalog instead of using inline-only; allow both inlineCatalogs and supportedCatalogIds in client capabilities - Agent: remove schema from system prompt (provided per-request via client capabilities); pass client_ui_capabilities through to catalog selection and validation - Executor: extract a2uiClientCapabilities from all DataParts (not just request parts) so UI events use the correct catalog - Client: inline OrgChart action schema (was unresolvable $ref) and add path-reference oneOf for chain (matches agent examples)
7b6f441 to
215b64a
Compare
nan-yu
left a comment
There was a problem hiding this comment.
Thanks for cleaning up the inline catalogs! That’s been a bit of a 'muddy' area for us, and your changes definitely make the implementation more robust.
One small note: we should keep in mind that Inline catalogs sent by the client at runtime are supported but not recommended in production.
|
The CI is currently failing. Could you please run a formatter on the python code: |
Fixes #796
Summary
This PR fixes inline catalog handling in the schema manager, ensures client capabilities are extracted from all DataPart types (not just
request), fixes an unresolvable$refin the prompt builder's test JSON, and adds test coverage for the recently-landed multi-surface and incremental update validator logic.Problems
inline_catalogs[0]and ignored the rest. Clients that split custom components across multiple inline catalog definitions would lose all but the first.supportedCatalogIdscouldn't be used alongsideinlineCatalogs— Sending both raised aValueError, but clients legitimately need both:supportedCatalogIdsto pick the base catalog andinlineCatalogsto extend it with custom components.supportedCatalogIdswas ignored when selecting the base for inline catalog merging — The base catalog was always_supported_catalogs[0]even when the client specified a preferred base viasupportedCatalogIds.client_ui_capabilitiesnot extracted fromuserActionDataParts — The executor only extracted capabilities fromrequestDataParts. When a UI event fires (userAction), that branch is never hit, so the LLM response is validated against the wrong catalog.$ref— The prompt builder's test JSON used$ref: "#/definitions/Action"which doesn't resolve in an inline catalog context.Note: Multi-surface root tracking and incremental update detection were independently fixed in upstream. This PR adds test coverage for that logic.
Changes
agent_sdks/python/src/a2ui/core/schema/manager.pyValueErrorwhen bothinlineCatalogsandsupportedCatalogIdsare presentsupportedCatalogIdsto select which base catalog to merge ontosamples/agent/adk/contact_multiple_surfaces/agent_executor.pya2uiClientCapabilitiesfrom any DataPart that has metadata, before theuserAction/requestbranch — fixes UI events using the wrong catalog for validationsamples/agent/adk/contact_multiple_surfaces/prompt_builder.py$ref), addoneOffor OrgChart chain propertyagent_sdks/python/tests/core/schema/test_validator.pybeginRendering+ twosurfaceUpdatewith different roots)Key Design Decisions
Inline catalogs are additive extensions. Components from each inline catalog are merged onto the base catalog via
components.update(). This ensures standard components remain available alongside custom ones.All inline catalogs from the client are merged. If a client sends multiple inline catalogs, all their components are merged onto the base. This supports clients that split custom components across multiple catalog definitions.
Both
inlineCatalogsandsupportedCatalogIdsare allowed simultaneously.supportedCatalogIdsselects the base catalog, andinlineCatalogsextend it.Validation Steps
pytest tests/core/schema/test_validator.py -v)contact_multiple_surfacesagent +contactclient end-to-endPre-launch Checklist
If you need help, consider asking for advice on the discussion board.