Conversation
There was a problem hiding this comment.
Code Review
This pull request transitions the project to a local Firebase iOS SDK dependency and updates imports from FirebaseAI to FirebaseAILogic. It significantly refines the recipe filtering UI by introducing a Form-based layout, adding a search field for instructions, and improving the rating and tag selection components. Additionally, the data model and store were updated to dynamically calculate likes and average ratings using Firestore pipelines. Feedback includes ensuring the base pipeline consistently includes aggregated fields to prevent UI data loss, removing debug print statements, and renaming a custom error type to avoid shadowing standard library enums.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions the project to a local Firebase SDK reference, renames the FirebaseAI module to FirebaseAILogic, and enhances the recipe filtering system. Key changes include adding a search field for recipe instructions, displaying like counts, and refactoring the Recipe model to compute average ratings dynamically using Firestore pipelines. Review feedback identifies a critical issue where the local SDK path will break external builds and points out several flaws in the Firestore pipeline logic, such as the requirement for the search stage to be positioned first and the need to ensure the averageRating field is present when sorting by rating to avoid query failures.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the project dependencies, notably replacing FirebaseAI with FirebaseAILogic and adding FoundationModels.framework. The UI for the FilterView has been significantly overhauled using a Form-based layout within a NavigationStack, and a new search filter for recipe instructions has been introduced. Additionally, the app now supports displaying and sorting by 'likes' using Firestore pipelines for real-time aggregation. Review feedback suggests refactoring duplicated pipeline logic in RecipeStore, optimizing pipeline stages by combining addFields calls, and restoring the averageRating property to the Recipe model to prevent data loss in the UI when sorting or filtering by rating.
| filters = filters.define([Field("__name__").documentId().as("parentRecipeId")]).addFields([ | ||
| db.pipeline() | ||
| .collection("likes") | ||
| .where(Field("recipeId").equal(Variable("parentRecipeId"))) | ||
| .aggregate([CountAll().as("count")]) | ||
| .toScalarExpression() | ||
| .as("likes") | ||
| ]) | ||
| // Add rating sometimes | ||
| if shouldAddAverageRating { | ||
| filters = filters.addFields([ | ||
| Subcollection(RecipeStore.reviewsSubcollection) | ||
| .aggregate([Field("rating").average().as("averageRating")]) | ||
| .toScalarExpression() | ||
| .as("averageRating") | ||
| ]) | ||
| } |
There was a problem hiding this comment.
Multiple addFields stages are being added sequentially. These can be combined into a single addFields call with an array of augmentations to reduce the number of stages in the pipeline and potentially improve execution efficiency.
// Always add likes
filters = filters.define([Field("__name__").documentId().as("parentRecipeId")])
var augmentations: [Augmentation] = [
db.pipeline()
.collection("likes")
.where(Field("recipeId").equal(Variable("parentRecipeId")))
.aggregate([CountAll().as("count")])
.toScalarExpression()
.as("likes")
]
// Add rating sometimes
if shouldAddAverageRating {
augmentations.append(
Subcollection(RecipeStore.reviewsSubcollection)
.aggregate([Field("rating").average().as("averageRating")])
.toScalarExpression()
.as("averageRating")
)
}
filters = filters.addFields(augmentations)
No description provided.