-
Notifications
You must be signed in to change notification settings - Fork 640
port more expressions/stages and integration tests #7346
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: wuandy/RealPpl_5
Are you sure you want to change the base?
Conversation
Firebase AI Mock Responses Check
|
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.
Summary of Changes
Hello @wu-hui, 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 delivers a substantial upgrade to the Firestore Pipeline, introducing a wealth of new data transformation and analysis capabilities. It focuses on expanding the available expressions and stages, providing developers with more powerful tools for complex data manipulation directly within the client-side SDK. The changes also include API refinements and enhanced configuration options to improve usability and control over pipeline execution.
Highlights
- New Pipeline Expressions and Stages: This pull request introduces a comprehensive set of new expressions and pipeline stages, significantly expanding the data processing capabilities of the Firestore Pipeline. Key additions include vector similarity search (findNearest), field manipulation (addFields, removeFields, replaceWith), sampling (sample), set operations (union), array flattening (unnest), and a wide range of new functions for string manipulation, mathematical operations, timestamp conversions, and logical comparisons.
- Enhanced Pipeline Options and Configuration: New options classes have been introduced to provide more granular control over pipeline execution and individual stages. This includes PipelineOptions for overall execution, CollectionSourceOptions and CollectionGroupOptions for source configuration, AggregateOptions for aggregations, FindNearestOptions for vector search, and UnnestOptions for array flattening, allowing for fine-tuned behavior and hints for the backend.
- API Renames and Refinements: Several API methods and internal class names have been updated for clarity and consistency. Notable renames include
convertFrom
tocreateFrom
for initializing pipelines from existing queries,AggregateWithAlias
toAliasedAggregate
,ExprWithAlias
toAliasedExpr
,replace
toreplaceWith
, andarrayOffset
toarrayGet
. Internal expression names likelogical_max
andlogical_min
have been simplified tomax
andmin
respectively. - Comprehensive Integration Tests: Extensive new integration tests have been added to validate the functionality of all newly ported expressions and stages. This includes tests for vector embeddings, various data types, metadata handling, string functions, advanced mathematical operations, and timestamp conversions. Previously ignored tests for min/max accumulations and logical operations have also been enabled, ensuring robust coverage.
- Improved Documentation: The Javadoc for the Pipeline API has been significantly updated across numerous methods, providing clearer explanations, usage examples, and distinctions between the standard
pipeline()
andrealtimePipeline()
functionalities, helping developers better understand and utilize these powerful features.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Generated by 🚫 Danger |
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 significantly expands the capabilities of Firestore pipelines by adding numerous new expressions, stages, and options. It also includes a substantial number of integration tests to cover the new functionality. Key changes include the introduction of many new expression functions (e.g., for math, strings, data manipulation), new stages (e.g., findNearest
, unnest
), and options classes for more granular control over pipeline execution. The public API has been improved with better Javadoc and examples, and some existing methods have been refactored for clarity and consistency, such as renaming convertFrom
to createFrom
.
My feedback focuses on improving API clarity by removing a potentially redundant function and fixing a critical bug that could lead to a NullPointerException
. Overall, this is a solid contribution that greatly enhances the Firestore pipeline feature.
fun withDistanceField(distanceField: String?): FindNearestOptions? { | ||
return withDistanceField(field(distanceField!!)) | ||
} |
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.
The method withDistanceField(distanceField: String?)
is declared with a nullable String?
parameter but uses the non-null assertion operator !!
on it. This will cause a NullPointerException
if a null
value is passed. Additionally, the return type FindNearestOptions?
is nullable and inconsistent with other builder-style methods in this class, such as withLimit
, which return a non-null FindNearestOptions
.
To ensure type safety and prevent runtime crashes, the method signature should be changed to accept a non-null String
and return a non-null FindNearestOptions
.
fun withDistanceField(distanceField: String?): FindNearestOptions? { | |
return withDistanceField(field(distanceField!!)) | |
} | |
fun withDistanceField(distanceField: String): FindNearestOptions { | |
return withDistanceField(field(distanceField)) | |
} |
The public api surface has changed for the subproject firebase-firestore: Please update the api.txt files for the subprojects being affected by this change by running ./gradlew ${subproject}:generateApiTxtFile. Also perform a major/minor bump accordingly. |
Coverage Report 1Affected Products
Test Logs |
Size Report 1Affected Products
Test Logs |
d013c6d
to
164fbde
Compare
164fbde
to
2110f36
Compare
c713424
to
2851bf4
Compare
The public api surface has changed for the subproject firebase-firestore: Please update the api.txt files for the subprojects being affected by this change by running ./gradlew ${subproject}:generateApiTxtFile. Also perform a major/minor bump accordingly. |
No description provided.