-
Notifications
You must be signed in to change notification settings - Fork 646
fix(py): fix test failures and enforce type checking in CI (zero lint except samples) #4259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
273e34c to
323400c
Compare
Summary of ChangesHello @yesudeep, 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 systematically refines the Python codebase by focusing on type correctness and consistency. It introduces explicit type hints, updates data structures to leverage Pydantic models and enums, and standardizes API interactions across various plugins. The changes aim to make the code more robust, maintainable, and easier to understand for developers by reducing ambiguity in data types and function signatures. 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. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive set of fixes to improve type checking and overall code robustness across the Python codebase. The changes are of high quality and include replacing magic strings with enums, adding assertions and None checks for better type safety, refactoring for clarity and maintainability, and updating tests to use real objects instead of mocks. These modifications significantly enhance the reliability and maintainability of the code. I have one minor suggestion regarding environment variable handling in a sample file to improve consistency.
1bae451 to
0ad182f
Compare
0ad182f to
ff6efef
Compare
ff6efef to
6af87a8
Compare
1f82369 to
aa8d4a3
Compare
|
/gemini review |
aa8d4a3 to
e4061a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a wide range of fixes to address linting and type-checking diagnostics across the Python codebase. The changes significantly improve type safety and code robustness by adding explicit None checks, replacing string literals with enum members, updating Pydantic models to modern standards, and using stricter types. The tests have also been updated to reflect these changes, using real type instances instead of mocks, which is a great improvement. I've identified a couple of instances where type errors were silenced instead of being fixed and have provided suggestions for the correct implementation.
e4061a9 to
c88f8c6
Compare
This commit addresses test failures caused by type annotation fixes and
enables strict type checking in CI.
CI Enforcement:
- Remove --exit-zero from uv ty check in python.yml to fail CI on
type errors
Bug Fixes:
- Fix MCP server read_resource to convert AnyUrl to str before passing
to resource actions (server.py)
- Add callable() check for action.matches in find_matching_resource
to prevent NoneType errors with dynamic action providers (resource.py)
Test Expectation Updates:
- Update schema expectations for nullable types to use anyOf format
instead of simple type field (Pydantic v2 behavior for int | None)
- veneer_test.py: 10 test expectations
- schema_test.py: 1 test expectation
Type Annotation Fixes:
- Fix incorrect nullable type annotations across plugins:
- value: int = None -> value: int | None = None
- Updates to Ollama, Vertex-AI, Google-GenAI, Anthropic, MCP,
compat-oai, DeepSeek, xAI plugins and core genkit packages
Context:
Pydantic v2 generates {"anyOf": [{"type": "integer"}, {"type": "null"}]}
for int | None fields, while the old incorrect annotations int = None
produced {"type": "integer"}. This commit updates all test expectations
to match the correct schema output.
c88f8c6 to
1d16a39
Compare
fix(py): fix test failures and enforce type checking in CI
This commit addresses test failures caused by type annotation fixes and
enables strict type checking in CI.
CI Enforcement:
type errors
Bug Fixes:
to resource actions (server.py)
to prevent NoneType errors with dynamic action providers (resource.py)
Test Expectation Updates:
instead of simple type field (Pydantic v2 behavior for int | None)
Type Annotation Fixes:
compat-oai, DeepSeek, xAI plugins and core genkit packages
Context:
Pydantic v2 generates {"anyOf": [{"type": "integer"}, {"type": "null"}]}
for int | None fields, while the old incorrect annotations int = None
produced {"type": "integer"}. This commit updates all test expectations
to match the correct schema output.