fix(tools): Add type checking for schema parameter in _sanitize_schem… #4364
Conversation
…a_formats_for_gemini Fixes google#4363 Added early type checking to handle non-dict schema types (lists and primitives) before attempting to iterate with schema.items(). This prevents AttributeError when the function is called recursively with non-dict types.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @paulparkinson, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of the schema sanitization utility for Gemini by resolving a critical Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Response from ADK Triaging Agent Hello @paulparkinson, thank you for creating this PR! This PR is a bug fix. Could you please provide logs or a screenshot after the fix is applied? This information will help reviewers to review your PR more efficiently. Thanks! |
![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 fixes a potential AttributeError in _sanitize_schema_formats_for_gemini by adding a type check for the schema parameter. The fix is correct, but I've suggested a way to simplify the new logic. I also pointed out that the function's type hints are now inconsistent with its behavior and should be updated to maintain code clarity and correctness.
| # Handle non-dict schemas early | ||
| if isinstance(schema, list): | ||
| # Handle array schemas - pass through | ||
| return schema | ||
| elif not isinstance(schema, dict): | ||
| # Handle primitive types - pass through | ||
| return schema | ||
|
|
There was a problem hiding this comment.
The logic to handle non-dictionary schemas can be simplified. Instead of checking for list and then not dict, a single check for not isinstance(schema, dict) is sufficient as it covers lists, primitives, and any other non-dictionary types.
Additionally, with this change, the function's type hints are now incorrect. The schema parameter and the return type are both hinted as dict[str, Any], but the function now accepts and returns other types. Please update the function signature to reflect this, for example:
def _sanitize_schema_formats_for_gemini(
schema: Any, preserve_null_type: bool = False
) -> Any:| # Handle non-dict schemas early | |
| if isinstance(schema, list): | |
| # Handle array schemas - pass through | |
| return schema | |
| elif not isinstance(schema, dict): | |
| # Handle primitive types - pass through | |
| return schema | |
| # Handle non-dict schemas early | |
| if not isinstance(schema, dict): | |
| return schema | |
2 participants
fix(tools): Add type checking for schema parameter in _sanitize_schema_formats_for_gemini
Fixes #4363
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
The
_sanitize_schema_formats_for_gemini()function in_gemini_schema_util.pyassumes theschemaparameter is always a dictionary and immediately callsschema.items()on line 160. However, when the function is called recursively with nested schemas, it can receive lists or primitive types, causing anAttributeError: 'list' object has no attribute 'items'.Solution:
Added early type checking at the beginning of the function to handle non-dict schema types:
schemais a list, return it as-is (pass through)schemais a primitive type, return it as-is (pass through)schema.items()iteration whenschemais actually a dictThis approach maintains backward compatibility while preventing the AttributeError for recursive calls with non-dict types.
Testing Plan
Unit Tests:
The fix addresses a runtime error that would occur when processing schemas with nested non-dict types. Existing unit tests covering schema sanitization should now pass without errors for these edge cases.
Manual End-to-End (E2E) Tests:
To manually test this fix:
_sanitize_schema_formats_for_gemini()with such a schemaAttributeErroris raisedThe fix is defensive in nature - it handles edge cases that were previously causing crashes. The core logic for dict schemas remains unchanged.
Checklist
Additional context
The fix adds 8 lines of defensive type checking code that prevents a crash scenario when the function encounters non-dict types during recursive processing. This is a minimal, targeted fix that preserves all existing functionality while extending support to handle edge cases properly.
Code change location:
src/google/adk/tools/_gemini_schema_util.pylines 148-154