Python a2ui_core: Implement catalog with components validation#1577
Conversation
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini summary |
|
Warning Gemini encountered an error creating the summary. You can try again by commenting |
|
/gemini summary |
Summary of ChangesThis pull request implements a comprehensive validation layer for the A2UI core library. It establishes a structured approach to catalog management, allowing for both statically defined Pydantic models and dynamic JSON schema-based validation. The changes include new utilities for checking component integrity, analyzing graph topology, and ensuring that component references and function calls conform to the defined catalog specifications. This ensures higher reliability for A2UI protocol payloads. Highlights
Activity
|
jacobsimionato
left a comment
jacobsimionato
left a comment
There was a problem hiding this comment.
Hey I left a bunch of comments, but I'm conscious you have a long chain, so happy to defer most of this. The validation APIs are new, so it's natural for us to iterate on them.
I would be super keen to remove the custom_single_refs etc if possible before submission!
…alysis, and schema recursion constraints
…es for component references
…er separation of concerns
…class for granular validation control
…omponent reference extraction
jacobsimionato
left a comment
jacobsimionato
left a comment
There was a problem hiding this comment.
I think there are some architectural improvements that would be possible here to make this logic more flexible and more consistent with web_core. E.g.
- Avoid the branching between the JSON and Model Catalog variants, so that there is one single way of interacting with Catalogs and Components. That way, there can be multiple ways to create catalogs (by loading JSON files, programmatically, by converting some other format to a Catalog object), and then all logic that processes catalogs (e.g. validation) can be written once and work anywhere. I also think there's a possibility we might need to deal with hybrid catalogs where some Components are loaded from JSON (e.g. basic catalog components?) and others are defined programmatically (e.g. some custom component).
- *Add first-class Component object and Function objects, to help developers mix and match components and functions. This should reduce the API surface on Catalog directly.
- Unify validation logic so it doesn't need to branch between model and JSON formats, seeing as that leaks implementation details of how the Catalog object was created.
But, there is a chain of PRs that I want to unblock. So I figure the fastest way is to approve here and try to get the others submitted too, and then we can always iterate later.
I filed a bug with a detailed proposal that we can always try to implement later. I'm curious about your thoughts on that - #1606
WDYT?
| """Retrieves the JSON Schema representing the catalog's theme.""" | ||
| raise NotImplementedError("Subclasses must implement get_theme_schema()") | ||
|
|
||
| def extract_ref_fields(self) -> Dict[str, Tuple[Set[str], Set[str]]]: |
There was a problem hiding this comment.
Maybe this function could move to the validation package rather than being in the Catalog models themselves. That way we have good separation of concerns where the Catalog just represents the Catalog data, and all manipulation or analysis of that data happens separately.
There was a problem hiding this comment.
This method is also used in the streaming parser, while it is also used for validation. It makes sense to move it to the validation package. As you suggested, we can refactor it in a follow up PR.
| self.spec_version = spec_version | ||
| self.catalog_id = catalog_id | ||
|
|
||
| def get_component_schema(self, comp_type: str) -> Optional[Dict[str, Any]]: |
There was a problem hiding this comment.
I think it would be helpful to have a top-level Component and Function classes (or perhaps ComponentApi, FunctionApi, FunctionImplementation) classes in the design, so that then Catalog can have a neat API that just exposes Component, and then each Component can optionally have a schema and class etc. That way, we can add additional properties to Component or Function more easily.
| func_class.model_validate(args) | ||
|
|
||
|
|
||
| class JsonCatalogValidator(CatalogValidator): |
There was a problem hiding this comment.
It seems like ModelCatalogValidator and JsonCatalogValidator are basically the same. My understanding is that Pydantic models can be converted to JSON schemas. So how about we write validation logic once against the JSON, and do validation of pydantic-backed models etc by converting them to JSON first?
|
Submitting the changes in this PR. Comments will be resolved in a separate PR. |
2 participants
Summary of Changes
This pull request implements a comprehensive validation layer for the A2UI core library. It establishes a structured approach to catalog management, allowing for both statically defined Pydantic models and dynamic JSON schema-based validation. The changes include new utilities for checking component integrity, analyzing graph topology, and ensuring that component references and function calls conform to the defined catalog specifications. This ensures higher reliability for A2UI protocol payloads.
Highlights
JsonCatalogto support server-side validation using JSON Schema (Draft 2020-12), enabling dynamic checks without requiring pre-compiled Pydantic models.jsonschemaandreferencingas new dependencies to support the dynamic JSON schema validation requirements.Activity