feat: Introduce schema bundling in A2uiSchemaManager and add a validator#557
feat: Introduce schema bundling in A2uiSchemaManager and add a validator#557nan-yu wants to merge 2 commits intogoogle:mainfrom
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 schema bundling and validation capabilities, which is a significant feature enhancement. The changes include a new build hook for packaging schema files, a robust A2uiSchemaManager for handling schema loading and bundling, and an A2uiValidator class. The addition of comprehensive unit and integration tests is commendable and ensures the reliability of the new functionality. I've identified a critical issue related to a missing import that would lead to a runtime error, a high-severity issue concerning error silencing that could complicate debugging, and a couple of medium-severity suggestions for code cleanup and correctness. Overall, this is a well-structured contribution.
| role_description: str, | ||
| workflow_description: str = "", | ||
| ui_description: str = "", | ||
| selected_components: List[str] = [], |
There was a problem hiding this comment.
The type hint List is used here but is not imported, which will cause a NameError at runtime. Since the project requires Python 3.10+, you can use the built-in list type hint instead.
| selected_components: List[str] = [], | |
| selected_components: list[str] = [], |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
This except Exception: pass block silently ignores all errors during schema loading from package resources, including potential JSON decoding errors or permission issues. This makes debugging very difficult. This pattern is repeated for other fallback mechanisms in this method. It's better to log the exception to provide visibility into why loading failed before proceeding to the next fallback method.
| except Exception: | |
| pass | |
| except Exception as e: | |
| logging.debug("Could not load schema '%s' from package: %s", filename, e) |
| raise FileNotFoundError( | ||
| f"Could not load package resource {filename} in {self.package_path}: {e}" | ||
| ) |
There was a problem hiding this comment.
Wrapping all exceptions in FileNotFoundError can be misleading, especially for issues like JSON decoding errors or permission problems. Raising a more general IOError for unexpected failures would provide a clearer indication of the problem's nature and improve debuggability.
| raise FileNotFoundError( | |
| f"Could not load package resource {filename} in {self.package_path}: {e}" | |
| ) | |
| raise IOError( | |
| f"Could not load package resource {filename} in {self.package_path}: {e}" | |
| ) from e |
| self, | ||
| schema: Dict[str, Any], | ||
| source_properties: Dict[str, Any], | ||
| mapping: Dict[str, str] = None, |
There was a problem hiding this comment.
The mapping parameter is unused within the _inject_additional_properties function. It should be removed to simplify the method signature and improve code clarity.
nan-yu
force-pushed
the
a2ui-validator
branch
4 times, most recently
from
45abdbb to
283be8d
Compare
- Add a base InferenceStrategy class - Add PackSpecsBuildHook to copy JSON schemas into the package during build time. - Update pyproject.toml to include assets and configure the build hook. - Implement A2uiSchemaManager for robust schema loading, pruning, and prompt generation.
nan-yu
force-pushed
the
a2ui-validator
branch
from
283be8d to
c42f84f
Compare
1 participant
Description
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. For larger changes, raising an issue first helps reduce redundant work.
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.