Refactor Python SDK Catalog Architecture for Web Core Parity#1612
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the A2UI catalog and validation system by consolidating ModelCatalog and JsonCatalog into a single unified Catalog class, and merging their validators into a single CatalogValidator using the jsonschema engine. The review feedback highlights several critical bugs and defensive programming opportunities, including: incorrect handling of undefined Pydantic fields (which default to PydanticUndefined rather than None), false-positive validation errors for schema-only components lacking a model_class, potential IndexErrors when normalizing empty function names, and potential AttributeErrors when parsing non-dictionary schemas (such as booleans or lists) in Catalog.from_json and CatalogValidator.
jacobsimionato
left a comment
jacobsimionato
left a comment
There was a problem hiding this comment.
Wow, thanks for jumping on this so fast!!
nan-yu
force-pushed
the
re-arch-4
branch
5 times, most recently
from
f0cbce1 to
da3a36e
Compare
|
|
||
| self.theme_schema = theme_schema | ||
| self.theme_class = theme_class | ||
| self._catalog_schema: Optional[Dict[str, Any]] = None |
There was a problem hiding this comment.
Not blocking: I think we should remove this seeing as it's optional anyway, but feel free to leave it for now and we can always do that later once you're back!
Here is a proposal: #1626
And here is a proposal I'm making to the 1.0 specification to further restrict the catalog format to help us move in this direction: #1627
There was a problem hiding this comment.
This will go to a separate PR to unblock the chain.
…ogSchemaValidator
2 participants
Description
Fixes #1606
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.