-
Notifications
You must be signed in to change notification settings - Fork 652
fix(py/genkit): resolve all ty type errors and ensure all functions have proper types #4270
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
Fix all type checking issues identified by ty in test files: - Add MockerFixture import for pytest-mock's mocker fixture - Add assertions before calling functions expecting non-None values (e.g., assert resp.message is not None before text_from_message) - Change mock fixture return types to Any when tests create mock objects with mocked methods that break type contracts - Add cast() calls for nested dictionary accesses where types can't be inferred - Fix function signatures where return types didn't match returned values (e.g., return input.value or 0 for int return type) - Rename veneer/resource_test.py to veneer_resource_test.py to fix pytest import collision with ai/resource_test.py - Add noqa: ANN401 comments for intentional Any usage in fixtures - Use ActionKind.CUSTOM enum instead of string literal 'custom' - Fix BatchEvaluatorFn signature (async function returning list) - Fix Flask context provider to use RequestData[Request] - Fix HttpOptionsDict import in google-genai tests Reduces ty errors from 414 to 0.
ecaa453 to
925958f
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 significantly enhances the type safety and maintainability of the Python Genkit codebase. It systematically addresses and resolves numerous type checking issues identified by 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. Ignored Files
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 is an excellent pull request that significantly improves the type safety of the Python codebase. The extensive changes to resolve ty type errors are thorough and well-executed. Replacing Any with more specific types like object, TypedDict, and generics, which significantly enhances code clarity and maintainability. The refactoring of some logic to be more robust and type-safe is also a welcome improvement. The consistency across the codebase is also commendable. Great job!
|
/gemini review |
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 does an excellent job of resolving a large number of type errors throughout the Python codebase. The changes significantly improve type safety and code robustness. I appreciate the thoroughness, especially the move from Any to more specific types like object or TypedDict, the addition of return types, and the refactoring of logic to be more type-safe. The introduction of asyncio.Queue for streaming in flows.py is a great fix for a potential bug.
I have found one important issue in Action.stream that could lead to incorrect behavior for consumers of streaming flows, and one suggestion to improve JSON serialization in flows.py.
Overall, this is a high-quality contribution that greatly improves the maintainability of the Python code.
- Replace MockInput classes with ResourceInput instances in tests - Use resource() function instead of Action constructor for dynamic providers - Change provider_fn return type from Action|None to object to avoid Pydantic schema generation errors - Remove unused Action import All 729 tests now pass.
9e5aec8 to
48a5496
Compare
…ave proper types (#4270)
Fix all type checking issues identified by ty in test files:
(e.g., assert resp.message is not None before text_from_message)
objects with mocked methods that break type contracts
be inferred
values (e.g., return input.value or 0 for int return type)
pytest import collision with ai/resource_test.py
Reduces ty errors from 414 to 0.