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 introduces a new validation layer for A2UI JSON payloads, which is a great addition to ensure data integrity beyond basic schema validation. The new validation.py module is well-structured and includes checks for component integrity, topology (cycles, orphans), recursion depth, and path syntax. The accompanying test suite in test_validation.py is comprehensive and covers many important edge cases.
My feedback includes a critical point about silent error handling that could lead to validation being skipped, and a suggestion to improve maintainability by refactoring hardcoded values into constants. Overall, this is a solid contribution that significantly improves the robustness of A2UI message processing.
| except Exception: | ||
| # If schema traversal fails, return empty map | ||
| pass |
There was a problem hiding this comment.
Using a broad except Exception: with a pass is dangerous. It will silently swallow any errors that occur during schema parsing (e.g., due to an unexpected schema structure), resulting in an empty ref_map. This effectively disables all component reference validation without any warning, which could allow invalid JSON to pass through. At a minimum, this should log a warning to indicate that reference validation was skipped. A better approach would be to handle more specific exceptions related to key/index errors during dictionary traversal.
| if global_depth > 50: | ||
| raise ValueError("Global recursion limit exceeded: Depth > 50") | ||
|
|
||
| if isinstance(item, list): | ||
| for x in item: | ||
| traverse(x, global_depth + 1, func_depth) | ||
| return | ||
|
|
||
| if isinstance(item, dict): | ||
| # Check for path | ||
| if PATH in item and isinstance(item[PATH], str): | ||
| path = item[PATH] | ||
| if not re.fullmatch(JSON_POINTER_PATTERN, path): | ||
| raise ValueError(f"Invalid JSON Pointer syntax: '{path}'") | ||
|
|
||
| # Check for FunctionCall | ||
| is_func = CALL in item and ARGS in item | ||
|
|
||
| if is_func: | ||
| if func_depth >= 5: | ||
| raise ValueError(f"Recursion limit exceeded: {FUNCTION_CALL} depth > 5") |
There was a problem hiding this comment.
The recursion depth limits (50 for global, 5 for function calls) are hardcoded as magic numbers. To improve readability and maintainability, it's better to define these as module-level constants (e.g., MAX_GLOBAL_DEPTH = 50, MAX_FUNC_CALL_DEPTH = 5). This makes their purpose clear and allows them to be easily located and modified in one place.
a2a_agents/python/a2ui_agent/src/a2ui/extension/send_a2ui_to_client_toolset.py
Show resolved
Hide resolved
|
The build was failing because of the format check. |
done! |
2 participants
To cover common errors not covered by the json schema