Conversation
wrenj
requested review from
dmandar,
jacobsimionato,
nan-yu and
zeroasterisk
as code owners
![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 introduces a C++ validation library for A2UI JSON payloads, including comprehensive checks for schema adherence, component integrity, topology, and recursion limits. The implementation is well-structured with good separation of concerns and is accompanied by a thorough set of unit tests. My review focuses on improving code clarity, maintainability, and performance in the C++ implementation, as well as a minor correction in the documentation. The core logic is sound, and the suggestions aim to refine the implementation.
|
|
||
| for (const auto& comp : components) { | ||
| auto id_it = comp.find(ID); | ||
| if (id_it == comp.end() || !id_it->is_string()) continue; |
There was a problem hiding this comment.
If the id isn't a string, isn't that a validation failure?
There not being any IDs might also be a failure, although that's less clear. It's probably "valid" to have an empty updateComponents message, although I'd argue we should disallow that on efficiency grounds.
| dfs(ROOT); | ||
| } | ||
|
|
||
| std::unordered_set<std::string> orphans; |
There was a problem hiding this comment.
When streaming, it's allowable to send updateComponents messages that don't include a root yet. You can later send one more that includes a root.
Those earlier messages won't validate because they will all be orphans until the root component arrives.
You should probably skip the orphan check (and maybe other topology checks) until you have seen a root component.
2 participants
No description provided.