-
Notifications
You must be signed in to change notification settings - Fork 657
feat(py): add Checks, Chroma, Pinecone, Cloud SQL PostgreSQL plugins with tests and Pydantic standardization #4328
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
base: main
Are you sure you want to change the base?
Conversation
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 expands the Python Genkit library's capabilities by introducing robust features for model operation tracking, dynamic action management, and enhanced testing. It also broadens its ecosystem integration with new plugins for AI safety (Google Checks) and popular vector databases (ChromaDB, Pinecone), alongside practical RAG sample applications. These additions aim to provide developers with more control, flexibility, and tools for building advanced AI applications. 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 is a substantial pull request that adds several new features and plugins, significantly increasing feature parity with the JavaScript implementation. The additions of generate_operation, Dynamic Action Providers (DAP), and the model testing suite are well-implemented and thoroughly documented. The new plugins for Chroma, Pinecone, and Checks are also great additions to the ecosystem.
My review focuses on a few key areas for improvement:
- Adherence to the project's own coding guidelines regarding import locations.
- Performance optimization in the new
checksplugin by avoiding unnecessary authentication token refreshes. - Improving code clarity and maintainability in the
checksplugin by leveraging Pydantic's aliasing features for cleaner data model parsing.
Overall, this is excellent work that brings a lot of valuable functionality to the Python version of Genkit.
1176500 to
e47ca92
Compare
|
|
||
| @abc.abstractmethod | ||
| async def init(self) -> list[Action]: | ||
| async def init(self, registry: 'Registry | None' = None) -> list[Action]: |
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.
registry was removed from plugins on purpose
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.
how does one resolve embedders from a plugin then?
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.
d596cdf to
1a01b91
Compare
1a01b91 to
1b409e0
Compare
|
/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 is a substantial and well-structured pull request that adds significant new capabilities to the Python SDK, including production-ready vector store plugins for Chroma and Pinecone, comprehensive RAG samples, and important bug fixes. The work to ensure Python 3.14 compatibility and standardize plugin initialization signatures is also a valuable improvement. The code is generally of high quality with good documentation. I've identified one critical issue with a package dependency version, an incomplete implementation in the new checks plugin, and a couple of areas in the new RAG samples where the code can be simplified for better readability and maintainability. Overall, this is an excellent contribution.
|
/gemini review |
|
/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 is a substantial and well-executed pull request that adds significant new functionality with the Chroma, Pinecone, and Checks plugins. The standardization of Pydantic models using alias_generator=to_camel is a great improvement for code clarity and maintainability. The update to the Plugin.init signature to include the registry is a thoughtful change that enables more powerful and decoupled plugins, as demonstrated in the new vector store implementations. The comprehensive test coverage and addition of new samples are also commendable. I've left a few minor comments related to adhering to the new contribution guidelines introduced in this same PR. Overall, excellent work!
eb96dda to
eed6433
Compare
|
@huangjeff5 need your input here about the |
f89b80e to
f1792fc
Compare
|
/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 is an impressive and substantial pull request that brings the Python SDK much closer to feature parity with the JavaScript implementation. The addition of the Checks, Chroma, Pinecone, and Cloud SQL PostgreSQL plugins is a major step forward.
I'm particularly impressed with:
- The thorough documentation updates in
GEMINI.mdand the newplugins/README.md, which significantly improve developer guidance and discoverability. - The standardization of Pydantic models using
alias_generator=to_camel, which enhances consistency and maintainability across the codebase. - The robust implementation of the new plugins, especially the thoughtful handling of authentication and database connections in the
cloud-sql-pgplugin. - The pragmatic workarounds for Python 3.14 compatibility issues in
noxfile.pyandpyproject.toml.
I have a few minor suggestions for improvement:
- There is some code duplication in the new
checksplugin that could be refactored for better maintainability. - In several of the new vector store plugins (
chroma,pinecone,cloud-sql-pg), there are opportunities to improve error handling by logging exceptions instead of silently passing, which would aid in debugging data quality issues.
Overall, this is a high-quality contribution that significantly enhances the capabilities and developer experience of the Python SDK. Great work!
py/plugins/cloud-sql-pg/src/genkit/plugins/cloud_sql_pg/plugin.py
Outdated
Show resolved
Hide resolved
a25f306 to
1df7af8
Compare
Implements the resolve() method to create and return the guardrails evaluator action, matching the JS implementation in evaluation.ts. Key changes: - Add _create_evaluator_action() that creates the evaluator Action - Add _evaluate_datapoint() to call Checks API for content classification - Update list_actions() to return single checks/guardrails evaluator - Update tests to verify new behavior The evaluator evaluates content against all configured safety policies and returns scores with pass/fail status based on violation results.
This change standardizes all Pydantic models that need camelCase JSON serialization to use `alias_generator=to_camel` instead of manual `Field(alias='...')` patterns. This approach: - Ensures type checkers understand models correctly - Allows Pythonic constructor calls with snake_case field names - Automatically handles camelCase serialization/deserialization Changes: - Add guideline to GEMINI.md documenting the preferred pattern - Update google-genai plugin models (gemini.py, veo.py, lyria.py) - Update checks plugin guardrails models - Update all sample files with RpgCharacter and menu schemas - Add comprehensive tests for Checks, Chroma, and Pinecone plugins - Fix evaluator resolution in Checks plugin to return list of scores Test coverage added: - Checks plugin: evaluator, guardrails, metrics, middleware tests - Chroma plugin: retriever, indexer, helpers tests - Pinecone plugin: retriever, indexer, helpers tests
- Fix TODO format in generate_schema_typing to use proper issue link format - Replace type: ignore comments in Chroma plugin with proper casts - Use cast(Embeddings, ...) for embedding vectors - Use cast(Include, ...) for include fields - Use cast(Metadatas, ...) for metadata lists - Import chromadb types (Embedding, Embeddings, Include, Metadata, Metadatas) This follows GEMINI.md guidelines to fix type errors rather than suppress them.
- Change default k from 10 to 4 to match JS implementation - Add ignore_metadata_columns auto-population logic in retriever - Strip .gserviceaccount.com from IAM principal emails (matches JS) - Add close_connection() alias for close() (JS API parity) - Add from_engine_args() alias for from_connection_string() (JS API parity) - Add metadata_json_column existence check in _check_columns() - Update tests to reflect new default k=4
Renamed test files to have unique prefixes matching other plugins: - engine_test.py -> cloud_sql_pg_engine_test.py - indexes_test.py -> cloud_sql_pg_indexes_test.py - plugin_test.py -> cloud_sql_pg_plugin_test.py This fixes pytest collection errors where test modules with the same name across different plugins would conflict with each other.
The __init__.py file was causing the tests directory to be treated as a Python package, leading to module name conflicts during pytest collection in CI. Other plugins (checks, chroma) don't have __init__.py in their tests directories.
- Rename package to genkit-plugin-cloud-sql-pg to match GEMINI.md guidelines - Add genkit-plugin-cloud-sql-pg to workspace dependencies in pyproject.toml - This ensures the plugin is installed in CI environments during test collection - Update README and sample pyproject.toml with new package name - Regenerate uv.lock
1df7af8 to
740c494
Compare
| self._xai_client = XAIClient(api_key=api_key, **cast(dict[str, Any], xai_params)) | ||
|
|
||
| async def init(self) -> list: | ||
| async def init(self, registry: 'Registry | None' = None) -> list: |
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.
No string types
| self.model_locations = model_locations or {} | ||
|
|
||
| async def init(self) -> list[Action]: | ||
| async def init(self, registry: 'Registry | None' = None) -> list[Action]: |
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.
No string types
| @@ -0,0 +1,176 @@ | |||
| Apache License | |||
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.
Hm, is this the right license?
| self.client = partial(ollama_api.AsyncClient, host=self.server_address) | ||
|
|
||
| async def init(self) -> list: | ||
| async def init(self, registry: 'Registry | None' = None) -> list: |
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.
No string types
| return self.name | ||
|
|
||
| async def init(self) -> list[Action]: | ||
| async def init(self, registry: 'Registry | None' = None) -> list[Action]: |
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.
No string types
| ) | ||
|
|
||
| async def init(self) -> list[Action]: | ||
| async def init(self, registry: 'Registry | None' = None) -> list[Action]: |
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.
No string types
| ) | ||
|
|
||
| async def init(self) -> list[Action]: | ||
| async def init(self, registry: 'Registry | None' = None) -> list[Action]: |
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.
No string types
| # Run nox with Python 3.12 to avoid workspace compatibility issues | ||
| # with chromadb (which doesn't support Python 3.14 yet). | ||
| # Nox will still create the test environment with the target Python version. | ||
| uv run --python 3.12 --directory py nox -s "tests-${{ matrix.python-version }}" |
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.
So we're actually checking python 3.12 for all tests?
Summary
This PR brings the Python SDK closer to feature parity with the canonical JavaScript implementation by adding several key plugins, standardizing Pydantic model patterns, and establishing robust testing conventions.
New Plugins (with JS Parity)
Cloud SQL PostgreSQL Plugin (NEW)
Full-featured vector store integration for Cloud SQL for PostgreSQL:
PostgresEnginesupporting Cloud SQL Connector with IAM and password authentication.PostgresRetrieverandPostgresIndexerwith metadata filtering and JSON support.Checks Plugin
Google Checks AI Safety integration for content policy evaluation:
Chroma Plugin
ChromaDB vector store integration for local RAG applications:
where,whereDocument), and MD5-based ID generation.Pinecone Plugin
Pinecone vector store integration for cloud-native RAG:
Core SDK Improvements
Pydantic Model Standardization
Standardized all Pydantic models to use
alias_generator=to_camelandpopulate_by_name=True.py/GEMINI.md.Test Naming Convention & CI Stability
Established a strict naming convention to prevent module collection conflicts in large-scale test runs:
{plugin_name}_{component}_test.py(e.g.,cloud_sql_pg_engine_test.py).__init__.pyfrom tests directories to ensure unique module paths.py/GEMINI.md.New Samples
Testing & Quality
All linters and tests pass with zero errors.
Total new tests added: ~160 tests