New list insights feature among other stylistic changes#160
New list insights feature among other stylistic changes#160aadishsamir123 merged 8 commits intomainfrom
Conversation
…ive data models and insights - Added AnalyticsService for tracking shopping insights, including total lists created, items added, and completion rates. - Introduced ListAnalyticsService for detailed list-specific insights, including item activity and collaborator contributions. - Created data models for insights, completion data, and activity tracking. - Implemented date range calculations for various timeframes. - Developed unit tests for both services to ensure functionality and data integrity.
📝 WalkthroughWalkthroughAdds user- and list-level analytics services and models, two new insights screens with navigation and animations, replaces several CircularProgressIndicator usages with a CustomLoadingSpinner, updates analyzer suppressions and Copilot/docs, adds unit tests, and bumps app versioning. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as App (UI / Navigation)
participant UIS as UserInsightsScreen
participant LIS as ListInsightsScreen
participant AS as AnalyticsService
participant LAS as ListAnalyticsService
participant FS as Firestore
User->>App: open "Your Insights" or select list → Insights
App->>UIS: navigate to UserInsightsScreen
UIS->>AS: request metrics (generateKeyInsights, trends, itemsPerDay)
AS->>FS: query user lists / items / categories
FS-->>AS: return documents
AS-->>UIS: aggregated insights & chart data
UIS-->>User: render cards & charts
User->>App: open specific list insights
App->>LIS: navigate to ListInsightsScreen(listId)
LIS->>LAS: request list metrics (activityTimeline, categoryBreakdown, collaborators)
LAS->>FS: query lists/{id}/items and categories
FS-->>LAS: return documents
LAS-->>LIS: aggregated list insights & timelines
LIS-->>User: render activity chart, breakdowns, collaborator list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
coderabbitai
bot
added
app:phone
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (10)
test/unit/services/analytics_service_test.dart (1)
10-33: Tests don't exercise actual service methods.These tests create local values and assert their types rather than calling the actual
AnalyticsServicemethods. While the comments mention "Full integration would require Firebase mocking," these tests provide minimal coverage value.Consider either:
- Adding Firebase mocks (using
fake_cloud_firestorefrom dev dependencies) to test actual behavior- Removing these placeholder tests to avoid misleading coverage metrics
lib/services/list_analytics_service.dart (2)
166-199: Performance: full collection scan can be avoided with composite index.The comment states "we can't use multiple where clauses," but Firestore supports composite queries on different fields with an index. This approach loads all completed items into memory regardless of timeframe.
Consider adding a composite index on
(completed, completedAt)and querying directly:final snapshot = await _firestore .collection('lists') .doc(listId) .collection('items') .where('completed', isEqualTo: true) .where('completedAt', isGreaterThanOrEqualTo: dateRange.start) .where('completedAt', isLessThanOrEqualTo: dateRange.end) .count() .get();
386-396: Performance: sequential async calls can be parallelized.Six independent Firestore calls are awaited sequentially. Use
Future.waitto parallelize:final results = await Future.wait([ getTotalItems(listId), getCompletedItems(listId), getCompletionPercentage(listId), getItemsAddedInTimeframe(listId, timeFrame), getItemsCompletedInTimeframe(listId, timeFrame), getAverageItemsPerDay(listId, timeFrame), ]); final totalItems = results[0] as int; final completedItems = results[1] as int; // ... etclib/widgets/expandable_list_group_widget.dart (1)
146-151: Consider explicit size parameter for better clarity.The
CustomLoadingSpinneris constrained to 12×12 viaSizedBox, but the spinner's default size is 50.0. Consider passing an explicit size parameter toCustomLoadingSpinnerfor clarity, and addingconstto prevent unnecessary rebuilds.Apply this diff:
- ? SizedBox( - width: 12, - height: 12, - child: CustomLoadingSpinner(), - ) + ? const SizedBox( + width: 12, + height: 12, + child: CustomLoadingSpinner(size: 12), + )lib/screens/list_insights.dart (1)
35-63: Consider parallel data loading for better performance.The four analytics queries execute sequentially, which could slow down the initial load. Since they appear independent, consider using
Future.waitto load them in parallel.Apply this diff:
Future<void> _loadData() async { setState(() => _isLoading = true); try { - final insights = await ListAnalyticsService.generateListInsights( - widget.listId, _selectedTimeFrame); - final categories = - await ListAnalyticsService.getCategoryBreakdown(widget.listId); - final activity = await ListAnalyticsService.getActivityTimeline( - widget.listId, _selectedTimeFrame); - final collaborators = await ListAnalyticsService.getCollaboratorActivity( - widget.listId, _selectedTimeFrame); + final results = await Future.wait([ + ListAnalyticsService.generateListInsights(widget.listId, _selectedTimeFrame), + ListAnalyticsService.getCategoryBreakdown(widget.listId), + ListAnalyticsService.getActivityTimeline(widget.listId, _selectedTimeFrame), + ListAnalyticsService.getCollaboratorActivity(widget.listId, _selectedTimeFrame), + ]); + + if (!mounted) return; setState(() { - _keyInsights = insights; - _categoryBreakdown = categories; - _activityTimeline = activity; - _collaboratorActivity = collaborators; + _keyInsights = results[0] as List<ListInsightData>; + _categoryBreakdown = results[1] as List<CategoryBreakdown>; + _activityTimeline = results[2] as List<ItemActivityData>; + _collaboratorActivity = results[3] as List<CollaboratorActivity>; _isLoading = false; }); } catch (e) {lib/screens/list_view.dart (2)
241-273: Inconsistent animation on unselected Insights icon in NavigationRail.The unselected
iconfor the Insights destination is a staticIconwithoutAnimatedBuilder, whileselectedIconusesAnimatedBuilder. This differs from the Items and Options destinations where both icons are animated. Consider wrapping the unselected icon with the same animation for consistency.NavigationRailDestination( - icon: Icon( - Icons.donut_small, - color: _selectedIndex == 1 - ? Colors.white - : (isDark ? Colors.grey[400] : Colors.green[600]), - ), + icon: AnimatedBuilder( + animation: _insightsSpinAnimation, + builder: (context, child) { + return Transform.rotate( + angle: _selectedIndex == 1 + ? _insightsSpinAnimation.value * 2 * 3.14159 + : 0.0, + child: Icon( + Icons.donut_small, + color: _selectedIndex == 1 + ? Colors.white + : (isDark ? Colors.grey[400] : Colors.green[600]), + ), + ); + }, + ), selectedIcon: AnimatedBuilder(
402-416: Same inconsistency: unselected Insights icon missing animation in NavigationBar.For consistency with the NavigationRail and other destinations, wrap the unselected icon with
AnimatedBuilder.NavigationDestination( - icon: const Icon(Icons.donut_small), + icon: AnimatedBuilder( + animation: _insightsSpinAnimation, + builder: (context, child) { + return Transform.rotate( + angle: _selectedIndex == 1 + ? _insightsSpinAnimation.value * 2 * 3.14159 + : 0.0, + child: const Icon(Icons.donut_small), + ); + }, + ), selectedIcon: AnimatedBuilder(lib/services/analytics_service.dart (3)
131-142: N+1 query pattern may cause performance issues.Looping through all user lists and issuing a separate Firestore query for each list's items can be slow for users with many lists. Consider:
- Using Firestore collection group queries if the schema supports it
- Implementing pagination or limiting the number of lists processed
- Adding client-side caching for repeated analytics views
489-544: Sequential await calls ingenerateKeyInsightsimpact load time.Six sequential async operations compound latency. Consider parallel execution:
static Future<List<ShoppingInsight>> generateKeyInsights( TimeFrame timeFrame) async { try { - final totalLists = await getTotalListsCreated(timeFrame); - final totalItems = await getTotalItemsAdded(timeFrame); - final totalCompleted = await getTotalItemsCompleted(timeFrame); - final avgCompletion = await getAverageCompletionPercentage(timeFrame); - final collaborators = await getCollaboratorCount(timeFrame); - final productivity = await getProductivityTrend(timeFrame); + final results = await Future.wait([ + getTotalListsCreated(timeFrame), + getTotalItemsAdded(timeFrame), + getTotalItemsCompleted(timeFrame), + getAverageCompletionPercentage(timeFrame), + getCollaboratorCount(timeFrame), + getProductivityTrend(timeFrame), + ]); + + final totalLists = results[0] as int; + final totalItems = results[1] as int; + final totalCompleted = results[2] as int; + final avgCompletion = results[3] as double; + final collaborators = results[4] as int; + final productivity = results[5] as double; return [ // ... rest unchanged ];Note:
getProductivityTrendinternally callsgetTotalItemsAdded, so there's redundant fetching. Consider refactoring to share data.
547-559: Consider makingDateRangeandChartDataimmutable withconstconstructors.These are simple data classes that would benefit from immutability.
class DateRange { final DateTime start; final DateTime end; - DateRange(this.start, this.end); + const DateRange(this.start, this.end); } class ChartData { final DateTime date; final double value; - ChartData({required this.date, required this.value}); + const ChartData({required this.date, required this.value}); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/copilot-instructions.md(3 hunks)analysis_options.yaml(1 hunks)lib/screens/home.dart(2 hunks)lib/screens/list_insights.dart(1 hunks)lib/screens/list_view.dart(11 hunks)lib/screens/user_insights.dart(1 hunks)lib/services/analytics_service.dart(1 hunks)lib/services/list_analytics_service.dart(1 hunks)lib/widgets/advert.dart(2 hunks)lib/widgets/animated_google_button.dart(2 hunks)lib/widgets/expandable_list_group_widget.dart(1 hunks)lib/widgets/loading_spinner.dart(1 hunks)lib/widgets/place_selector.dart(2 hunks)lib/widgets/smart_suggestions_widget.dart(2 hunks)test/unit/services/analytics_service_test.dart(1 hunks)test/unit/services/list_analytics_service_test.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: Use snake_case for file names, PascalCase for classes, and camelCase for variables in Dart code
UseStreamBuilder<QuerySnapshot>andStreamBuilder<DocumentSnapshot>for real-time Firebase sync instead of Provider/ChangeNotifier
UseFieldValue.serverTimestamp()forcreatedAt/updatedAtfields in Firestore documents
PreferSingleTickerProviderStateMixin+AnimationControllerfor animations
UseFutureBuilderfor one-time async operations like checking update status, not for ongoing streams
UseListGroupsServicefor group CRUD operations and direct Firestore queries for list/item CRUD operations
UsekDebugModechecks for debug prints and conditional debugging logic
Files:
lib/widgets/animated_google_button.dartlib/widgets/place_selector.dartlib/widgets/smart_suggestions_widget.dartlib/widgets/loading_spinner.dartlib/widgets/advert.dartlib/screens/list_insights.dartlib/screens/home.darttest/unit/services/list_analytics_service_test.darttest/unit/services/analytics_service_test.dartlib/widgets/expandable_list_group_widget.dartlib/screens/user_insights.dartlib/services/analytics_service.dartlib/screens/list_view.dartlib/services/list_analytics_service.dart
analysis_options.yaml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Set
use_build_context_synchronously: ignoreinanalysis_options.yamlfor Flutter linting
Files:
analysis_options.yaml
lib/services/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
lib/services/**/*.dart: Implement all services as static utility classes without singletons or instances (e.g.,ListGroupsService,CategoriesService,GoogleAuthService)
Wrap all service method errors withSentry.captureException()including contextual hints viaHint.withMap()
Use Credential Manager on Android (v2.0.0 API) for Google Sign-In to support passkey authentication
Usegoogle_sign_inwith client ID fromGoogleAuthService._webClientIdfor web authentication flows
UseHomeWidgetService.updateWidget()to sync shopping list data to Android home widget launcher
CallUpdateService.checkForUpdate()to trigger Android Play update flow for in-app updates
Files:
lib/services/analytics_service.dartlib/services/list_analytics_service.dart
🧠 Learnings (13)
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Prefer `SingleTickerProviderStateMixin` + `AnimationController` for animations
Applied to files:
lib/widgets/animated_google_button.dart.github/copilot-instructions.mdlib/screens/list_view.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/wear/screens/**/*.dart : Implement WearOS UI in `lib/wear/screens/` using circular layouts with rotary scroll support via `rotary_scrollbar` package and ambient mode via `wear_plus`
Applied to files:
lib/widgets/animated_google_button.dartlib/widgets/smart_suggestions_widget.dartlib/screens/home.dart.github/copilot-instructions.mdlib/screens/list_view.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Use `HomeWidgetService.updateWidget()` to sync shopping list data to Android home widget launcher
Applied to files:
lib/widgets/place_selector.dartlib/widgets/smart_suggestions_widget.dartlib/widgets/advert.dartlib/screens/list_insights.dartlib/screens/home.dart.github/copilot-instructions.mdlib/widgets/expandable_list_group_widget.dartlib/services/analytics_service.dartlib/screens/list_view.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `StreamBuilder<QuerySnapshot>` and `StreamBuilder<DocumentSnapshot>` for real-time Firebase sync instead of Provider/ChangeNotifier
Applied to files:
lib/widgets/place_selector.dartlib/widgets/smart_suggestions_widget.dartlib/screens/home.dart.github/copilot-instructions.mdlib/screens/list_view.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FutureBuilder` for one-time async operations like checking update status, not for ongoing streams
Applied to files:
lib/widgets/place_selector.dart.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to analysis_options.yaml : Set `use_build_context_synchronously: ignore` in `analysis_options.yaml` for Flutter linting
Applied to files:
analysis_options.yaml
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `ListGroupsService` for group CRUD operations and direct Firestore queries for list/item CRUD operations
Applied to files:
lib/screens/home.dart.github/copilot-instructions.mdtest/unit/services/list_analytics_service_test.dartlib/services/analytics_service.dartlib/screens/list_view.dartlib/services/list_analytics_service.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Organize Flutter project structure with `lib/screens/` for full-page UI, `lib/widgets/` for reusable components, and `lib/services/` for business logic & Firebase interactions
Applied to files:
lib/screens/home.dartlib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FieldValue.serverTimestamp()` for `createdAt`/`updatedAt` fields in Firestore documents
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Wrap all service method errors with `Sentry.captureException()` including contextual hints via `Hint.withMap()`
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Implement all services as static utility classes without singletons or instances (e.g., `ListGroupsService`, `CategoriesService`, `GoogleAuthService`)
Applied to files:
.github/copilot-instructions.mdtest/unit/services/list_analytics_service_test.dartlib/services/analytics_service.dartlib/services/list_analytics_service.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Use Credential Manager on Android (v2.0.0 API) for Google Sign-In to support passkey authentication
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/main.dart : Use Sentry with 100% trace and profile sample rate as configured in `main.dart`
Applied to files:
.github/copilot-instructions.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Tests & Coverage
- GitHub Check: Build WearOS Flavor
- GitHub Check: Build Phone Flavor
- GitHub Check: Build Web (WASM)
- GitHub Check: Seer Code Review
- GitHub Check: Build Web (WASM)
- GitHub Check: Build WearOS (Debug)
- GitHub Check: Build Phone (Debug)
🔇 Additional comments (25)
analysis_options.yaml (1)
4-4: LGTM — suppression is reasonable for Flutter widget patterns.This lint suppression is commonly needed when widget
Stateclasses use private types. As per coding guidelines,use_build_context_synchronously: ignoreis already expected.test/unit/services/analytics_service_test.dart (1)
74-227: Good coverage of data model constructors and edge cases.The data model tests properly validate field storage, calculation logic (
completionPercentage), and edge cases (zero items, single items). These provide meaningful coverage.test/unit/services/list_analytics_service_test.dart (1)
120-193: Good integration tests for data model operations.The sorting and list operations tests properly validate that data models work correctly in collections. These provide practical coverage for typical usage patterns.
.github/copilot-instructions.md (2)
98-108: Clear platform-specific loading indicator documentation.Good addition documenting the loading indicator patterns with code examples for both Phone/Web (
CustomLoadingSpinner) and WearOS (CircularProgressIndicator).
378-408: Comprehensive analytics architecture documentation.The dual-level insights system documentation with file references, navigation paths, and tab order conventions will help maintain consistency. The field name conventions (lines 395-400) are especially valuable for avoiding query bugs.
lib/services/list_analytics_service.dart (1)
62-63: Service pattern follows guidelines.Static utility class with
Sentry.captureExceptionincluding contextual hints aligns with the established service layer pattern.lib/widgets/animated_google_button.dart (1)
2-2: LGTM! Consistent loading indicator upgrade.The replacement of
CircularProgressIndicatorwithCustomLoadingSpinneraligns with the project-wide standardization of loading UX while preserving the existing color logic based on dark mode.Also applies to: 99-101
lib/screens/home.dart (1)
15-15: LGTM! User insights navigation properly integrated.The new "Your Insights" drawer item follows the established navigation pattern, appropriately closes the drawer before navigating, and uses consistent styling with other menu items.
Also applies to: 783-797
lib/widgets/place_selector.dart (1)
3-3: LGTM! Consistent loading indicator upgrade.The replacement aligns with the project-wide standardization of loading indicators using
CustomLoadingSpinner.Also applies to: 140-140
lib/widgets/advert.dart (1)
4-4: LGTM! Consistent loading indicator upgrade.The replacement of
CircularProgressIndicatorwithCustomLoadingSpinnerin the ad placeholder aligns with the project-wide loading UX standardization.Also applies to: 141-141
lib/widgets/smart_suggestions_widget.dart (1)
2-2: LGTM! Consistent loading indicator upgrade.The replacement aligns with the project-wide standardization of loading indicators.
Also applies to: 61-61
lib/screens/list_insights.dart (1)
1-550: LGTM! Well-structured analytics screen with good UX patterns.The
ListInsightsScreendemonstrates solid architecture with:
- Proper separation of concerns via builder methods
- Appropriate empty state handling for all data sections
- Theme-aware styling using
ColorScheme- Pull-to-refresh support via
RefreshIndicator- Consistent loading states
lib/widgets/loading_spinner.dart (1)
20-20: TheLoadingIndicatorM3EVariant.containedvariant is supported in m3e_collection 0.3.7 and is the correct way to use the contained variant of theLoadingIndicatorM3Ewidget. No changes needed.Likely an incorrect or invalid review comment.
lib/screens/list_view.dart (6)
30-31: Good use ofTickerProviderStateMixinfor multiple animation controllers.Using
TickerProviderStateMixininstead ofSingleTickerProviderStateMixinis correct here since multipleAnimationControllerinstances require separate tickers. Based on learnings, this aligns with the preferred animation pattern.
78-90: LGTM: Insights animation controller setup.The animation controller follows the same pattern as the existing options animation, with proper duration and curve settings. The 3.0 full rotations ensures the donut icon returns to its original orientation.
93-99: Animation controllers properly disposed.All three animation controllers are correctly disposed in the
disposemethod, preventing memory leaks.
679-683: Good replacement ofCircularProgressIndicatorwithCircularProgressIndicatorM3E.This aligns with the PR objective of standardizing loading indicators across the app.
913-938: Proper Sentry error handling with contextual hints.The
_toggleItemCompletionmethod correctly wraps errors withSentry.captureException()including contextual hints viaHint.withMap(), following the coding guidelines for services.
966-967: Good use ofFieldValue.serverTimestamp()fordeletedAt.This follows the coding guideline to use
FieldValue.serverTimestamp()for timestamp fields in Firestore documents.lib/screens/user_insights.dart (3)
163-201: Good implementation of time frame selector.The button implementation with proper visual feedback, dark/light theming, and accessibility (using Material + InkWell for ripple effects) is well done.
488-551: Good defensive coding in_buildSimpleBarChart.The method handles empty data, calculates max value safely with fallback, and limits display to last 7 data points. The tooltip provides additional context on hover.
847-866: Well-structured icon mapping helper.The exhaustive switch statement ensures all
IconValueenum cases are handled, preventing runtime errors if new values are added.lib/services/analytics_service.dart (3)
63-65: Service follows static utility class pattern correctly.The
AnalyticsServiceis implemented as a static utility class with private static members for_authand_firestore, following the coding guidelines.
106-111: Good Sentry error handling with contextual hints.All public methods properly wrap exceptions with
Sentry.captureException()and include action context viaHint.withMap(), following the coding guidelines.
264-271: Firestore composite index required for this query.The query combines a range filter on
createdAtwith an inequality filter ondeletedacross different fields. Firestore requires a composite index for this query pattern. If this index doesn't exist, Firestore will return an error with a direct link to create it in the Firebase console.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
lib/services/analytics_service.dart (2)
270-273: Samedeletedfield filter concern applies here.The
isNotEqualTo: truefilter has the same limitation noted earlier.
76-81: Date arithmetic edge case persists for month/quarter boundaries.When
now.dayis 31 and calculating a month ago (e.g., from March 31),DateTime(now.year, now.month - 1, now.day)normalizes Feb 31 → March 2/3, producing ~3 days instead of ~30 days. Same issue exists for quarter calculation.Consider using duration subtraction or clamping to last valid day:
case TimeFrame.month: - final monthAgo = DateTime(now.year, now.month - 1, now.day); + final monthAgo = startOfDay.subtract(const Duration(days: 30)); return DateRange(monthAgo, now); case TimeFrame.quarter: - final quarterAgo = DateTime(now.year, now.month - 3, now.day); + final quarterAgo = startOfDay.subtract(const Duration(days: 90)); return DateRange(quarterAgo, now);lib/services/list_analytics_service.dart (4)
76-78: Same date arithmetic edge case asAnalyticsService.The month calculation has the same overflow issue when
now.dayexceeds the target month's days.
451-456:DateRangeis duplicated fromanalytics_service.dart.Extract to a shared models file to avoid duplication.
184-186: Off-by-one: boundary dates excluded from completed items count.
isAfterandisBeforeexclude items completed exactly atdateRange.startordateRange.end. This was flagged in a previous review.Use inclusive comparisons:
if (completedAt != null && - completedAt.isAfter(dateRange.start) && - completedAt.isBefore(dateRange.end)) { + !completedAt.isBefore(dateRange.start) && + !completedAt.isAfter(dateRange.end)) { count++; }
323-329: Inconsistent field name:createdAtshould beaddedAt.This method queries
'createdAt'while other methods (getItemsAddedInTimeframe,getActivityTimeline) use'addedAt'for items. This inconsistency will cause collaborator activity to not match the intended timeframe.final itemsSnapshot = await _firestore .collection('lists') .doc(listId) .collection('items') - .where('createdAt', isGreaterThanOrEqualTo: dateRange.start) - .where('createdAt', isLessThanOrEqualTo: dateRange.end) + .where('addedAt', isGreaterThanOrEqualTo: dateRange.start) + .where('addedAt', isLessThanOrEqualTo: dateRange.end) .get();
🧹 Nitpick comments (6)
.github/copilot-instructions.md (1)
410-430: Test writing guidance could be enhanced with analytics-specific examples.The "Testing Responsibilities" and "Test Writing Quick Rules" sections (lines 410–430) are solid, but they do not include explicit guidance on testing analytics queries with the documented field-name constraints (e.g., verifying that
'addedAt'and'completed'fields are correctly queried, or that'deletedAt'filters work properly). Consider adding a brief note or example test pattern for analytics-specific scenarios to reinforce the field-naming conventions and prevent regressions.Example enhancement (optional):
// Add to section "Test Writing Quick Rules" or create a new subsection: **Analytics Query Testing**: When testing analytics services, verify field names explicitly: - Use 'addedAt' (not 'createdAt') for item timestamp queries - Use 'completed' (not 'checked') for completion state queries - Filter deleted items with .where('deleted', isNotEqualTo: true) - Use only 'addedAt' for time-range queries; 'completedAt' does not exist Example:dart
test('getItemAdditionRate queries addedAt field correctly', () {
// Mock Firestore to capture the query
// Verify that .where('addedAt', ...) is called, not .where('createdAt', ...)
expect(querySnapshot.docs.any((doc) => doc['addedAt'] != null), isTrue);
});lib/services/analytics_service.dart (2)
491-545: Consider parallelizing independent async calls ingenerateKeyInsights.The method makes 6 sequential async calls that are independent. Using
Future.waitcould improve performance.static Future<List<ShoppingInsight>> generateKeyInsights( TimeFrame timeFrame) async { try { - final totalLists = await getTotalListsCreated(timeFrame); - final totalItems = await getTotalItemsAdded(timeFrame); - final totalCompleted = await getTotalItemsCompleted(timeFrame); - final avgCompletion = await getAverageCompletionPercentage(timeFrame); - final collaborators = await getCollaboratorCount(timeFrame); - final productivity = await getProductivityTrend(timeFrame); + final results = await Future.wait([ + getTotalListsCreated(timeFrame), + getTotalItemsAdded(timeFrame), + getTotalItemsCompleted(timeFrame), + getAverageCompletionPercentage(timeFrame), + getCollaboratorCount(timeFrame), + getProductivityTrend(timeFrame), + ]); + final totalLists = results[0] as int; + final totalItems = results[1] as int; + final totalCompleted = results[2] as int; + final avgCompletion = results[3] as double; + final collaborators = results[4] as int; + final productivity = results[5] as double;
548-560:DateRangeclass is duplicated inlist_analytics_service.dart.Consider extracting shared data models (like
DateRangeand potentiallyChartData) to a common models file to avoid duplication.lib/screens/list_insights.dart (1)
230-238: Max value calculation could be simplified.The fold logic works but is verbose.
final maxValue = _activityTimeline.fold<int>( 0, - (max, data) { - final dataMax = data.addedCount > data.completedCount - ? data.addedCount - : data.completedCount; - return dataMax > max ? dataMax : max; - }, + (max, data) => [max, data.addedCount, data.completedCount].reduce((a, b) => a > b ? a : b), );Or use
dart:math:import 'dart:math' as math; // ... final maxValue = _activityTimeline.fold<int>( 0, (max, data) => math.max(max, math.max(data.addedCount, data.completedCount)), );lib/services/list_analytics_service.dart (2)
286-292: Date boundary workaround extends range by 1 day on each side.The
subtract(Duration(days: 1))andadd(Duration(days: 1))effectively make the comparison inclusive, but also include items from one day before/after the intended range. Consider using the cleaner inclusive comparison pattern:- if (completedDateOnly - .isAfter(dateRange.start.subtract(const Duration(days: 1))) && - completedDateOnly - .isBefore(dateRange.end.add(const Duration(days: 1)))) { + if (!completedDateOnly.isBefore(dateRange.start) && + !completedDateOnly.isAfter(dateRange.end)) {
386-448:generateListInsightscould parallelize independent async calls.Same optimization opportunity as in
AnalyticsService.generateKeyInsights.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/copilot-instructions.md(3 hunks)android/app/build.gradle(1 hunks)lib/screens/list_insights.dart(1 hunks)lib/services/analytics_service.dart(1 hunks)lib/services/list_analytics_service.dart(1 hunks)pubspec.yaml(1 hunks)test/unit/services/list_analytics_service_test.dart(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- test/unit/services/list_analytics_service_test.dart
🧰 Additional context used
📓 Path-based instructions (5)
**/{Makefile,*.sh,build.gradle,pubspec.yaml}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
flutter run --release --flavor phone -d <device>for building the phone app
Files:
android/app/build.gradle
**/{Makefile,*.sh,build.gradle}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
flutter run --release --flavor wear -d <device> --target lib/wear/wear_main.dartfor building the WearOS app
Files:
android/app/build.gradle
android/app/build.gradle
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
android/app/build.gradle: Use version code formatXXYYYYYYYwhere XX=platform (30=phone, 40=wear) and YYYYYYY=versionCode for releases
Firebase flavors are defined inandroid/app/build.gradlewith dimension 'platform' for phone and wear separation
Files:
android/app/build.gradle
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: Use snake_case for file names, PascalCase for classes, and camelCase for variables in Dart code
UseStreamBuilder<QuerySnapshot>andStreamBuilder<DocumentSnapshot>for real-time Firebase sync instead of Provider/ChangeNotifier
UseFieldValue.serverTimestamp()forcreatedAt/updatedAtfields in Firestore documents
PreferSingleTickerProviderStateMixin+AnimationControllerfor animations
UseFutureBuilderfor one-time async operations like checking update status, not for ongoing streams
UseListGroupsServicefor group CRUD operations and direct Firestore queries for list/item CRUD operations
UsekDebugModechecks for debug prints and conditional debugging logic
Files:
lib/screens/list_insights.dartlib/services/list_analytics_service.dartlib/services/analytics_service.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
lib/services/**/*.dart: Implement all services as static utility classes without singletons or instances (e.g.,ListGroupsService,CategoriesService,GoogleAuthService)
Wrap all service method errors withSentry.captureException()including contextual hints viaHint.withMap()
Use Credential Manager on Android (v2.0.0 API) for Google Sign-In to support passkey authentication
Usegoogle_sign_inwith client ID fromGoogleAuthService._webClientIdfor web authentication flows
UseHomeWidgetService.updateWidget()to sync shopping list data to Android home widget launcher
CallUpdateService.checkForUpdate()to trigger Android Play update flow for in-app updates
Files:
lib/services/list_analytics_service.dartlib/services/analytics_service.dart
🧠 Learnings (16)
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to android/app/build.gradle : Use version code format `XXYYYYYYY` where XX=platform (30=phone, 40=wear) and YYYYYYY=versionCode for releases
Applied to files:
android/app/build.gradle
📚 Learning: 2025-12-05T14:08:58.025Z
Learnt from: aadishsamir123
Repo: ASDev-Official/shopsync PR: 147
File: android/app/build.gradle:51-58
Timestamp: 2025-12-05T14:08:58.025Z
Learning: For android/app/build.gradle in the shopsync repository: Use version code format `XXYYYYYYY` where XX=platform (30=phone, 40=wear), YYYYYYY=versionCode. Phone uses base 300000000, wear uses base 400000000.
Applied to files:
android/app/build.gradle
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to android/app/build.gradle : Firebase flavors are defined in `android/app/build.gradle` with dimension 'platform' for phone and wear separation
Applied to files:
android/app/build.gradle
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/{Makefile,*.sh,build.gradle,pubspec.yaml} : Use `flutter run --release --flavor phone -d <device>` for building the phone app
Applied to files:
android/app/build.gradle
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/{Makefile,*.sh,build.gradle} : Use `flutter run --release --flavor wear -d <device> --target lib/wear/wear_main.dart` for building the WearOS app
Applied to files:
android/app/build.gradle
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FieldValue.serverTimestamp()` for `createdAt`/`updatedAt` fields in Firestore documents
Applied to files:
.github/copilot-instructions.mdlib/services/list_analytics_service.dartlib/services/analytics_service.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `StreamBuilder<QuerySnapshot>` and `StreamBuilder<DocumentSnapshot>` for real-time Firebase sync instead of Provider/ChangeNotifier
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FutureBuilder` for one-time async operations like checking update status, not for ongoing streams
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/wear/screens/**/*.dart : Implement WearOS UI in `lib/wear/screens/` using circular layouts with rotary scroll support via `rotary_scrollbar` package and ambient mode via `wear_plus`
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `ListGroupsService` for group CRUD operations and direct Firestore queries for list/item CRUD operations
Applied to files:
.github/copilot-instructions.mdlib/services/list_analytics_service.dartlib/services/analytics_service.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Prefer `SingleTickerProviderStateMixin` + `AnimationController` for animations
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Wrap all service method errors with `Sentry.captureException()` including contextual hints via `Hint.withMap()`
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Implement all services as static utility classes without singletons or instances (e.g., `ListGroupsService`, `CategoriesService`, `GoogleAuthService`)
Applied to files:
.github/copilot-instructions.mdlib/services/list_analytics_service.dartlib/services/analytics_service.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Use `HomeWidgetService.updateWidget()` to sync shopping list data to Android home widget launcher
Applied to files:
.github/copilot-instructions.mdlib/screens/list_insights.dartlib/services/analytics_service.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Use Credential Manager on Android (v2.0.0 API) for Google Sign-In to support passkey authentication
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/main.dart : Use Sentry with 100% trace and profile sample rate as configured in `main.dart`
Applied to files:
.github/copilot-instructions.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Phone Flavor
- GitHub Check: Build WearOS Flavor
- GitHub Check: Build Web (WASM)
- GitHub Check: Build WearOS (Debug)
- GitHub Check: Build Phone (Debug)
🔇 Additional comments (18)
android/app/build.gradle (2)
52-57: Version code calculation follows the correct platform-versioning format.The phone flavor versionCode is correctly computed as
300000000 + 5 = 300000005, where30represents the phone platform and0000005is the padded 7-digit version code, aligning with the documented format.
53-53: Note: Version label may need alignment.The PR is labeled
semver-minor, but the version increment from4.3.1-phoneto5.0.0-phonerepresents a major version bump. This is likely intentional given the source branch name includesv5.0.0, but verify the label is correct for the PR context.Also applies to: 56-56
.github/copilot-instructions.md (2)
98-109: Loading indicators pattern is clear and well-targeted by platform.The new section provides straightforward, platform-specific guidance with code examples for Phone/Web and WearOS, aligning with the PR's objective to standardize loading UI across platforms.
376-409: Analytics architecture documentation is comprehensive and directly addresses prior field-naming inconsistencies.The "UPDATE COPILOT INSTRUCTIONS" directive (line 376) and "Analytics & Insights Architecture" section (lines 378–409) provide clear, actionable guidance on dual-level insights, file locations, services, and critically, the correct field-name conventions. Line 396 explicitly documents
'addedAt'for items (NOT'createdAt') and line 397 clarifies that items lack a'completedAt'field—resolving the inconsistency flagged in the prior review. Navigation tab order with specific index positions and animation styles (lines 403–409) is well-detailed.All referenced files and classes exist at the documented paths:
lib/services/analytics_service.dart(AnalyticsService)lib/services/list_analytics_service.dart(ListAnalyticsService)lib/screens/user_insights.dart(UserInsightsScreen)lib/screens/list_insights.dart(ListInsightsScreen)lib/services/analytics_service.dart (4)
1-66: Data models and service structure look good.The enums, data classes, and service declaration follow the project's static utility class pattern.
ShoppingInsight,ListCompletionData, andCategoryInsightare well-structured with appropriate computed properties (e.g.,completionPercentageinListCompletionData).
171-183: Field name issue resolved; sequential queries may impact performance.The
'completed'field is now correctly used (previously was'checked'). However, the N+1 query pattern (fetching all lists, then querying items for each) could be slow for users with many lists. This is acceptable for analytics but worth noting for future optimization.
332-347: LGTM - Consistent pattern with correct field usage.Uses
'completed'field correctly and follows the established query pattern.
369-437: Trend calculation methods are well-implemented.
getCollaboratorCountcorrectly collects unique collaborators excluding the current user.getProductivityTrendandgetCompletionTrendhandle edge cases (zero days/items) appropriately.lib/screens/list_insights.dart (7)
6-28: Widget structure is well-organized.Clean separation of required parameters (
listId,listName) and internal state variables for loading status and data collections.
65-98: Build method structure is clean with proper loading state handling.Good use of
RefreshIndicatorwithAlwaysScrollableScrollPhysicsto enable pull-to-refresh. The modular approach with separate builder methods keeps the code readable.
118-141: Time frame button implementation is straightforward.Selection state management and data reload on change work correctly.
162-210: Key insights grid implementation looks good.Proper use of
shrinkWrapandNeverScrollableScrollPhysicsfor nested scrollable content. Empty state is handled appropriately.
310-354: Bar chart rendering logic is clean with proper minimum height handling.The
height < 1 ? 1 : heightensures bars are always visible even for zero values.
397-449: Category breakdown with progress indicators is well-implemented.Proper division-by-zero guard and text overflow handling for long category names.
505-516: EmptyuserNamecrash fix is properly implemented.The
isNotEmptycheck with'?'fallback correctly addresses the previously flaggedRangeErrorissue.lib/services/list_analytics_service.dart (3)
35-46:CategoryBreakdown.percentagecalculation is now correct.The formula correctly uses
completedCount / itemCount(previously wasitemCount / itemCount).
84-164: Basic item methods are well-implemented.Consistent field usage (
completed,addedAt), proper Sentry integration with contextual hints, and appropriate null-coalescing for count results.
201-254: Category breakdown implementation is clean.Good use of record types for aggregation and proper category name lookup from the categories subcollection.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/services/list_analytics_service.dart (3)
35-46: CategoryBreakdown percentage is correct; consider clampingThe completion-rate formula is now correct and guarded against division by zero. If you expect any data inconsistencies where
completedCount > itemCount, consider clamping to[0, 100]for robustness:}) : percentage = itemCount == 0 ? 0 : ((completedCount / itemCount) * 100).clamp(0, 100);
166-199: Consider pushing completedAt timeframe filtering into FirestoreCurrent implementation fetches all completed items for the list and then filters by
completedAtin memory. This is fine for small lists but can become expensive as histories grow.You can likely move the timeframe filter server‑side for better performance, e.g.:
final snapshot = await _firestore .collection('lists') .doc(listId) .collection('items') .where('completed', isEqualTo: true) .where('completedAt', isGreaterThanOrEqualTo: dateRange.start) .where('completedAt', isLessThanOrEqualTo: dateRange.end) .get(); return snapshot.docs.length;This will require an appropriate composite index on
completed+completedAt.
317-365: Clarify timeframe semantics for collaborator completion countsHere the timeframe is enforced via
addedAt, butitemsCompletedincrements for any item that is currently completed, even ifcompletedAtfalls outside the timeframe. That effectively reports:“Activity on items created in this timeframe,”
not
“Activity (adds + completions) that occurred in this timeframe.”If the UX intends the latter, you may want to also bound
itemsCompletedbycompletedAtwithindateRange, similar togetItemsCompletedInTimeframe.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/services/list_analytics_service.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: Use snake_case for file names, PascalCase for classes, and camelCase for variables in Dart code
UseStreamBuilder<QuerySnapshot>andStreamBuilder<DocumentSnapshot>for real-time Firebase sync instead of Provider/ChangeNotifier
UseFieldValue.serverTimestamp()forcreatedAt/updatedAtfields in Firestore documents
PreferSingleTickerProviderStateMixin+AnimationControllerfor animations
UseFutureBuilderfor one-time async operations like checking update status, not for ongoing streams
UseListGroupsServicefor group CRUD operations and direct Firestore queries for list/item CRUD operations
UsekDebugModechecks for debug prints and conditional debugging logic
Files:
lib/services/list_analytics_service.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
lib/services/**/*.dart: Implement all services as static utility classes without singletons or instances (e.g.,ListGroupsService,CategoriesService,GoogleAuthService)
Wrap all service method errors withSentry.captureException()including contextual hints viaHint.withMap()
Use Credential Manager on Android (v2.0.0 API) for Google Sign-In to support passkey authentication
Usegoogle_sign_inwith client ID fromGoogleAuthService._webClientIdfor web authentication flows
UseHomeWidgetService.updateWidget()to sync shopping list data to Android home widget launcher
CallUpdateService.checkForUpdate()to trigger Android Play update flow for in-app updates
Files:
lib/services/list_analytics_service.dart
🧠 Learnings (3)
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `ListGroupsService` for group CRUD operations and direct Firestore queries for list/item CRUD operations
Applied to files:
lib/services/list_analytics_service.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Implement all services as static utility classes without singletons or instances (e.g., `ListGroupsService`, `CategoriesService`, `GoogleAuthService`)
Applied to files:
lib/services/list_analytics_service.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FieldValue.serverTimestamp()` for `createdAt`/`updatedAt` fields in Firestore documents
Applied to files:
lib/services/list_analytics_service.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build Phone Flavor
- GitHub Check: Build Web (WASM)
- GitHub Check: Build WearOS Flavor
- GitHub Check: Tests & Coverage
- GitHub Check: Build Web (WASM)
- GitHub Check: Build WearOS (Debug)
- GitHub Check: Build Phone (Debug)
🔇 Additional comments (1)
lib/services/list_analytics_service.dart (1)
62-139: Service structure & error handling align with guidelinesStatic utility pattern, direct Firestore access for lists/items, and consistent
Sentry.captureException+Hint.withMapusage look solid and match the documented service conventions.
…just date range calculations
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
lib/screens/user_insights.dart (1)
584-703: Add error handling to top categories FutureBuilder.The FutureBuilder lacks error state handling. Errors will render the empty state instead of informing the user of the failure.
Add error handling after the loading check:
builder: (context, snapshot) { if (snapshot.connectionState == ConnectionState.waiting) { return Container( height: 200, decoration: BoxDecoration( color: isDark ? Colors.grey[800] : Colors.white, borderRadius: BorderRadius.circular(16), border: Border.all( color: isDark ? Colors.grey[700]! : Colors.grey[200]!, width: 1, ), ), child: const Center(child: CustomLoadingSpinner()), ); } + if (snapshot.hasError) { + debugPrint('Error loading top categories: ${snapshot.error}'); + return Container( + height: 200, + decoration: BoxDecoration( + color: isDark ? Colors.grey[800] : Colors.white, + borderRadius: BorderRadius.circular(16), + border: Border.all( + color: isDark ? Colors.grey[700]! : Colors.grey[200]!, + width: 1, + ), + ), + child: Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Text( + 'Failed to load categories', + style: TextStyle( + color: isDark ? Colors.grey[400] : Colors.grey[600], + ), + ), + const SizedBox(height: 8), + TextButton.icon( + onPressed: () => setState(() {}), + icon: const Icon(Icons.refresh), + label: const Text('Retry'), + ), + ], + ), + ), + ); + } + final categories = snapshot.data ?? [];
🧹 Nitpick comments (1)
lib/screens/user_insights.dart (1)
51-833: Consider targeted retry for individual failed sections.The retry buttons (e.g., line 344) call
setState(() {})which rebuilds the entire screen and re-executes all FutureBuilders, not just the failed one. This causes unnecessary network calls for sections that loaded successfully.Consider storing individual Future variables in state and updating only the failed one:
class _UserInsightsScreenState extends State<UserInsightsScreen> { TimeFrame _selectedTimeFrame = TimeFrame.month; late Future<List<ShoppingInsight>> _insightsFuture; late Future<double> _completionTrendFuture; // ... other futures @override void initState() { super.initState(); _loadData(); } void _loadData() { _insightsFuture = AnalyticsService.generateKeyInsights(_selectedTimeFrame); _completionTrendFuture = AnalyticsService.getCompletionTrend(_selectedTimeFrame); // ... initialize other futures } void _retryCompletionTrend() { setState(() { _completionTrendFuture = AnalyticsService.getCompletionTrend(_selectedTimeFrame); }); } // Update _loadData() when time frame changes void _onTimeFrameChanged(TimeFrame newFrame) { setState(() { _selectedTimeFrame = newFrame; _loadData(); }); } }Then use the stored futures in FutureBuilders and call specific retry methods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/copilot-instructions.md(3 hunks)lib/screens/list_insights.dart(1 hunks)lib/screens/user_insights.dart(1 hunks)lib/services/analytics_service.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/screens/list_insights.dart
- lib/services/analytics_service.dart
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: Use snake_case for file names, PascalCase for classes, and camelCase for variables in Dart code
UseStreamBuilder<QuerySnapshot>andStreamBuilder<DocumentSnapshot>for real-time Firebase sync instead of Provider/ChangeNotifier
UseFieldValue.serverTimestamp()forcreatedAt/updatedAtfields in Firestore documents
PreferSingleTickerProviderStateMixin+AnimationControllerfor animations
UseFutureBuilderfor one-time async operations like checking update status, not for ongoing streams
UseListGroupsServicefor group CRUD operations and direct Firestore queries for list/item CRUD operations
UsekDebugModechecks for debug prints and conditional debugging logic
Files:
lib/screens/user_insights.dart
🧠 Learnings (12)
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/wear/screens/**/*.dart : Implement WearOS UI in `lib/wear/screens/` using circular layouts with rotary scroll support via `rotary_scrollbar` package and ambient mode via `wear_plus`
Applied to files:
lib/screens/user_insights.dart.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Organize Flutter project structure with `lib/screens/` for full-page UI, `lib/widgets/` for reusable components, and `lib/services/` for business logic & Firebase interactions
Applied to files:
lib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FutureBuilder` for one-time async operations like checking update status, not for ongoing streams
Applied to files:
lib/screens/user_insights.dart.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `StreamBuilder<QuerySnapshot>` and `StreamBuilder<DocumentSnapshot>` for real-time Firebase sync instead of Provider/ChangeNotifier
Applied to files:
lib/screens/user_insights.dart.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FieldValue.serverTimestamp()` for `createdAt`/`updatedAt` fields in Firestore documents
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `ListGroupsService` for group CRUD operations and direct Firestore queries for list/item CRUD operations
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Prefer `SingleTickerProviderStateMixin` + `AnimationController` for animations
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Wrap all service method errors with `Sentry.captureException()` including contextual hints via `Hint.withMap()`
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Implement all services as static utility classes without singletons or instances (e.g., `ListGroupsService`, `CategoriesService`, `GoogleAuthService`)
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Use `HomeWidgetService.updateWidget()` to sync shopping list data to Android home widget launcher
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/services/**/*.dart : Use Credential Manager on Android (v2.0.0 API) for Google Sign-In to support passkey authentication
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/main.dart : Use Sentry with 100% trace and profile sample rate as configured in `main.dart`
Applied to files:
.github/copilot-instructions.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build Web (WASM)
- GitHub Check: Build Phone Flavor
- GitHub Check: Tests & Coverage
- GitHub Check: Build WearOS Flavor
- GitHub Check: Build WearOS (Debug)
- GitHub Check: Build Web (WASM)
- GitHub Check: Build Phone (Debug)
🔇 Additional comments (5)
.github/copilot-instructions.md (4)
393-409: Excellent: Explicit field name documentation prevents prior issue recurrence.The addition of the "Analytics & Insights Architecture" section with explicit field name mappings (lines 393–409) directly addresses the prior review's feedback on
createdAtvsaddedAtinconsistency. By documenting that items use'addedAt'(not'createdAt'), have no'completedAt'field, and that deleted items live inrecycled_itemssubcollection, you've created a clear reference that should prevent similar field-name mix-ups in future analytics implementations.The tab order specification (Items → Insights → Options, lines 403–408) and animation cues (donut spin with Icons.donut_small → Icons.donut_large) are actionable and will help maintain consistency as the feature scales.
Verify that
lib/services/list_analytics_service.dartandlib/services/analytics_service.dart(referenced but not provided) strictly follow these field name conventions throughout their query and aggregation logic.
98-108: Clear platform-specific loading indicator guidance.The new "Loading Indicators" section provides concrete guidance on when to use
CustomLoadingSpinner()(Phone/Web) vsCircularProgressIndicator()(WearOS), with code examples. This should reduce inconsistency across platforms as the codebase evolves.Ensure that the migration from
CircularProgressIndicatortoCustomLoadingSpinner(mentioned in the PR summary) is complete across all phone/web screens, and that no strayCircularProgressIndicatorinstances remain whereCustomLoadingSpinnershould be used.
376-377: "UPDATE COPILOT INSTRUCTIONS" directive sets clear expectation.Making it explicit that new features require copilot-instructions.md updates will help keep guidance synchronized with implementation and reduce gaps in future architectural decisions.
410-437: Testing Responsibilities expanded with analytics-specific guidance.The expanded section now includes expectations around analytics documentation (line 414) and testing field names/logic (implied by context). Lines 422–429 ("Test Writing Quick Rules") are practical and align with the AAA pattern emphasized elsewhere.
lib/screens/user_insights.dart (1)
51-72: Error handling properly implemented.The previous review concern about missing error state handling has been addressed. The FutureBuilder now correctly handles
snapshot.hasErrorand displays an appropriate error state.
…ove productivity card layout
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/unit/services/analytics_service_test.dart (1)
68-72: Placeholder test doesn't validategenerateKeyInsightsbehavior.Similar to the earlier service method tests, this validates the type but doesn't call
generateKeyInsights. Consider using Firebase mocking to test that the method generates insights based on actual user data.
🧹 Nitpick comments (5)
lib/screens/user_insights.dart (1)
327-328: GatedebugPrintcalls behindkDebugModeas per logging guidelinesMultiple error branches log using
debugPrintunconditionally:
- Line 327–328: completion trend
- Line 437–438: productivity trend
- Line 533–535: items-per-day trend
- Line 784–785: list completions
Project guidelines call for wrapping debug logging with
kDebugMode. You can centralize this by importingkDebugModeand guarding the logs:-import 'package:flutter/material.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter/foundation.dart';Then update the logs, for example:
- if (snapshot.hasError) { - debugPrint('Error loading completion trend: ${snapshot.error}'); + if (snapshot.hasError) { + if (kDebugMode) { + debugPrint( + 'Error loading completion trend: ${snapshot.error}', + ); + } return SizedBox(Apply the same
if (kDebugMode)guard pattern to the otherdebugPrintusages in this file.Also applies to: 437-438, 533-535, 784-785
test/unit/services/analytics_service_test.dart (4)
10-33: Consider using Firebase mocking to test actual service methods.These tests validate basic Dart types but don't invoke the actual
AnalyticsServicemethods, limiting their value for catching regressions. To improve coverage, consider usingfake_cloud_firestoreormockitoto mock Firebase and verify that the service methods return expected results with test data.For example:
test('getTotalListsCreated returns correct count with test data', () async { // Arrange: set up fake Firestore with test lists final fakeFirestore = FakeFirebaseFirestore(); // ... populate test data // Act final result = await AnalyticsService.getTotalListsCreated(userId, TimeFrame.week); // Assert expect(result, expectedCount); });
35-42: Use consistent date precision in week test.The test compares
weekAgo(derived fromstartOfDaywithout time component) withnow(includes current time), leading to inconsistent precision. Additionally, this test doesn't call_getDateRange(which is private).Consider using a deterministic test date for both values, similar to the month test at lines 44-51:
-test('_getDateRange week returns correct date range', () { - final now = DateTime.now(); - final startOfDay = DateTime(now.year, now.month, now.day); - final weekAgo = startOfDay.subtract(const Duration(days: 7)); - - // Verify the week range logic - expect(weekAgo.difference(now).inDays, lessThanOrEqualTo(-7)); -}); +test('_getDateRange week returns correct date range', () { + final testDate = DateTime(2024, 3, 15); + final weekAgo = DateTime(2024, 3, 8); + + // Verify the week range logic + expect(weekAgo.difference(testDate).inDays, equals(-7)); +});
53-66: Date range tests don't invoke the actual_getDateRangemethod.These tests validate date arithmetic but not the actual
_getDateRangebehavior. Since_getDateRangeis private, consider testing it indirectly through public methods that use it, or refactor to make date range logic testable with dependency injection.Also, lines 61-66 just verify that
DateTime(2020).yearequals2020, which is trivial—consider removing or replacing with a test of a public method that uses the all-time range.
74-227: Excellent coverage of data models and edge cases.The tests for
ChartData,ListCompletionData,CategoryInsight,ShoppingInsight,DateRange, and enum types are well-structured and cover important edge cases (zero items, 100% completion, same start/end dates, etc.). These tests provide solid validation of the analytics data models.Minor note: Lines 152-162 explicitly set
color: null, which is redundant ifcoloris an optional parameter with anulldefault. You can omit the parameter entirely:test('ShoppingInsight with color', () { final insight = ShoppingInsight( title: 'Test', value: '10', subtitle: 'items', iconType: IconValue.shoppingCart, - color: null, ); expect(insight.color, isNull); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/screens/user_insights.dart(1 hunks)lib/services/analytics_service.dart(1 hunks)test/unit/services/analytics_service_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/services/analytics_service.dart
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: Use snake_case for file names, PascalCase for classes, and camelCase for variables in Dart code
UseStreamBuilder<QuerySnapshot>andStreamBuilder<DocumentSnapshot>for real-time Firebase sync instead of Provider/ChangeNotifier
UseFieldValue.serverTimestamp()forcreatedAt/updatedAtfields in Firestore documents
PreferSingleTickerProviderStateMixin+AnimationControllerfor animations
UseFutureBuilderfor one-time async operations like checking update status, not for ongoing streams
UseListGroupsServicefor group CRUD operations and direct Firestore queries for list/item CRUD operations
UsekDebugModechecks for debug prints and conditional debugging logic
Files:
lib/screens/user_insights.darttest/unit/services/analytics_service_test.dart
🧠 Learnings (4)
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Organize Flutter project structure with `lib/screens/` for full-page UI, `lib/widgets/` for reusable components, and `lib/services/` for business logic & Firebase interactions
Applied to files:
lib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to lib/wear/screens/**/*.dart : Implement WearOS UI in `lib/wear/screens/` using circular layouts with rotary scroll support via `rotary_scrollbar` package and ambient mode via `wear_plus`
Applied to files:
lib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FutureBuilder` for one-time async operations like checking update status, not for ongoing streams
Applied to files:
lib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `StreamBuilder<QuerySnapshot>` and `StreamBuilder<DocumentSnapshot>` for real-time Firebase sync instead of Provider/ChangeNotifier
Applied to files:
lib/screens/user_insights.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build Web (WASM)
- GitHub Check: Tests & Coverage
- GitHub Check: Build Phone Flavor
- GitHub Check: Build WearOS Flavor
- GitHub Check: Build Phone (Debug)
- GitHub Check: Build Web (WASM)
- GitHub Check: Build WearOS (Debug)
🔇 Additional comments (2)
lib/screens/user_insights.dart (1)
7-146: Overall screen structure and async patterns look solidState management, time-frame selection, use of
FutureBuilderfor one-shot analytics calls, and loading/empty/error handling in most sections are all consistent and well-structured. The layout composition and dark/light theming are coherent and match the documented architecture for insights.test/unit/services/analytics_service_test.dart (1)
44-51: Good fix for the month boundary edge case.Using deterministic dates (
DateTime(2024, 3, 15)andDateTime(2024, 2, 15)) correctly addresses the previous review concern about day overflow when calculating month boundaries. This approach ensures the test is stable regardless of when it runs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/screens/user_insights.dart (1)
644-763: Add error handling to top categories FutureBuilder.The
_buildTopCategoriesCardFutureBuilder still lacks error state handling. WhenAnalyticsService.getMostUsedCategoriesthrows an exception, the code treats it as empty data (line 663-666), showing "No data available" instead of an informative error message with retry capability. This is inconsistent with the other insight cards that have explicit error handling.Add error handling after the loading check:
builder: (context, snapshot) { if (snapshot.connectionState == ConnectionState.waiting) { return Container( height: 200, decoration: BoxDecoration( color: isDark ? Colors.grey[800] : Colors.white, borderRadius: BorderRadius.circular(16), border: Border.all( color: isDark ? Colors.grey[700]! : Colors.grey[200]!, width: 1, ), ), child: const Center(child: CustomLoadingSpinner()), ); } + + if (snapshot.hasError) { + debugPrint('Error loading top categories: ${snapshot.error}'); + return Container( + height: 200, + decoration: BoxDecoration( + color: isDark ? Colors.grey[800] : Colors.white, + borderRadius: BorderRadius.circular(16), + border: Border.all( + color: isDark ? Colors.grey[700]! : Colors.grey[200]!, + width: 1, + ), + ), + child: Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Text( + 'Failed to load category data', + style: TextStyle( + color: isDark ? Colors.grey[400] : Colors.grey[600], + ), + ), + const SizedBox(height: 8), + TextButton.icon( + onPressed: () => setState(() {}), + icon: const Icon(Icons.refresh), + label: const Text('Retry'), + ), + ], + ), + ), + ); + } final categories = snapshot.data ?? [];
🧹 Nitpick comments (3)
lib/screens/user_insights.dart (3)
328-328: Wrap debug prints inkDebugModechecks per coding guidelines.The coding guidelines require
kDebugModechecks for debug prints. WhiledebugPrintis automatically stripped in release builds, explicitly guarding these calls aligns with the project's conventions.Add the import at the top of the file:
import 'package:flutter/foundation.dart';Then wrap the debug prints:
- if (snapshot.hasError) { - debugPrint('Error loading completion trend: ${snapshot.error}'); + if (snapshot.hasError) { + if (kDebugMode) { + debugPrint('Error loading completion trend: ${snapshot.error}'); + }Apply similar changes to lines 437-438, 534, and 785.
Based on coding guidelines, which specify using
kDebugModechecks for debug prints.Also applies to: 437-438, 534-534, 785-785
561-569: Consider using_buildEmptyState()helper for consistency.The inline empty state could use the existing
_buildEmptyState()helper (lines 932-973) for consistency with other sections of the screen.Replace with:
final data = snapshot.data ?? []; if (data.isEmpty) { - return Center( - child: Text( - 'No data available', - style: TextStyle( - color: isDark ? Colors.grey[400] : Colors.grey[600], - ), - ), - ); + return _buildEmptyState(); }
591-593: Remove redundant isEmpty check.The
data.isEmptycheck at line 591 is redundant since the method already returns early at line 580 if data is empty.Simplify to:
- final maxValue = data.isEmpty - ? 1 - : data.map((e) => e.value).reduce((a, b) => a > b ? a : b); + final maxValue = data.map((e) => e.value).reduce((a, b) => a > b ? a : b);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/screens/user_insights.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: Use snake_case for file names, PascalCase for classes, and camelCase for variables in Dart code
UseStreamBuilder<QuerySnapshot>andStreamBuilder<DocumentSnapshot>for real-time Firebase sync instead of Provider/ChangeNotifier
UseFieldValue.serverTimestamp()forcreatedAt/updatedAtfields in Firestore documents
PreferSingleTickerProviderStateMixin+AnimationControllerfor animations
UseFutureBuilderfor one-time async operations like checking update status, not for ongoing streams
UseListGroupsServicefor group CRUD operations and direct Firestore queries for list/item CRUD operations
UsekDebugModechecks for debug prints and conditional debugging logic
Files:
lib/screens/user_insights.dart
🧠 Learnings (3)
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Organize Flutter project structure with `lib/screens/` for full-page UI, `lib/widgets/` for reusable components, and `lib/services/` for business logic & Firebase interactions
Applied to files:
lib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FutureBuilder` for one-time async operations like checking update status, not for ongoing streams
Applied to files:
lib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `StreamBuilder<QuerySnapshot>` and `StreamBuilder<DocumentSnapshot>` for real-time Firebase sync instead of Provider/ChangeNotifier
Applied to files:
lib/screens/user_insights.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build Web (WASM)
- GitHub Check: Build Phone Flavor
- GitHub Check: Build WearOS Flavor
- GitHub Check: Tests & Coverage
- GitHub Check: Build WearOS (Debug)
- GitHub Check: Build Web (WASM)
- GitHub Check: Build Phone (Debug)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/screens/user_insights.dart (3)
327-328: UsekDebugModecheck for debug prints.Per coding guidelines, debug prints should be wrapped with
kDebugModechecks to ensure they don't execute in release builds.+import 'package:flutter/foundation.dart'; + if (snapshot.hasError) { - debugPrint('Error loading completion trend: ${snapshot.error}'); + if (kDebugMode) { + debugPrint('Error loading completion trend: ${snapshot.error}'); + }This pattern should be applied to all
debugPrintcalls in this file (lines 328, 437-438, 534, 664, 819). Based on coding guidelines requiringkDebugModechecks for debug prints.
579-594: Redundant empty check after early return.Line 591 checks
data.isEmptyagain, but the early return at line 580 already handles this case. The ternary will always take the non-empty branch.- final maxValue = data.isEmpty - ? 1 - : data.map((e) => e.value).reduce((a, b) => a > b ? a : b); + final maxValue = data.map((e) => e.value).reduce((a, b) => a > b ? a : b);Alternatively, consider using
foldor importingdart:mathformax:final maxValue = data.map((e) => e.value).fold(1.0, max);
1009-1050: Inconsistent error state: missing retry button.The
_buildErrorState()method lacks a retry button, unlike the inline error states in other FutureBuilders (e.g., lines 343-347, 452-457, 548-552). Users encountering errors in the key insights section have no way to retry without navigating away.Widget _buildErrorState() { final isDark = Theme.of(context).brightness == Brightness.dark; return Container( padding: const EdgeInsets.all(32), decoration: BoxDecoration( color: isDark ? Colors.grey[800] : Colors.white, borderRadius: BorderRadius.circular(16), border: Border.all( color: isDark ? Colors.grey[700]! : Colors.grey[200]!, width: 1, ), ), child: Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ Icon( - Icons.analytics_outlined, + Icons.error_outline, size: 48, color: isDark ? Colors.grey[600] : Colors.grey[400], ), const SizedBox(height: 16), Text( 'An error occurred', style: TextStyle( fontSize: 16, color: isDark ? Colors.grey[400] : Colors.grey[600], ), ), const SizedBox(height: 8), Text( 'Please try again later', style: TextStyle( fontSize: 12, color: isDark ? Colors.grey[500] : Colors.grey[500], ), ), + const SizedBox(height: 16), + TextButton.icon( + onPressed: () => setState(() {}), + icon: const Icon(Icons.refresh), + label: const Text('Retry'), + ), ], ), ), ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/screens/user_insights.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: Use snake_case for file names, PascalCase for classes, and camelCase for variables in Dart code
UseStreamBuilder<QuerySnapshot>andStreamBuilder<DocumentSnapshot>for real-time Firebase sync instead of Provider/ChangeNotifier
UseFieldValue.serverTimestamp()forcreatedAt/updatedAtfields in Firestore documents
PreferSingleTickerProviderStateMixin+AnimationControllerfor animations
UseFutureBuilderfor one-time async operations like checking update status, not for ongoing streams
UseListGroupsServicefor group CRUD operations and direct Firestore queries for list/item CRUD operations
UsekDebugModechecks for debug prints and conditional debugging logic
Files:
lib/screens/user_insights.dart
🧠 Learnings (3)
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Organize Flutter project structure with `lib/screens/` for full-page UI, `lib/widgets/` for reusable components, and `lib/services/` for business logic & Firebase interactions
Applied to files:
lib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `FutureBuilder` for one-time async operations like checking update status, not for ongoing streams
Applied to files:
lib/screens/user_insights.dart
📚 Learning: 2025-12-05T14:59:55.596Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T14:59:55.596Z
Learning: Applies to **/*.dart : Use `StreamBuilder<QuerySnapshot>` and `StreamBuilder<DocumentSnapshot>` for real-time Firebase sync instead of Provider/ChangeNotifier
Applied to files:
lib/screens/user_insights.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Web (WASM)
- GitHub Check: Build WearOS Flavor
- GitHub Check: Build Phone Flavor
- GitHub Check: Build Phone (Debug)
- GitHub Check: Build WearOS (Debug)
🔇 Additional comments (7)
lib/screens/user_insights.dart (7)
1-12: LGTM!File naming (snake_case), class naming (PascalCase), and import structure follow Flutter conventions and coding guidelines.
14-146: Well-structured screen layout with appropriate async patterns.Good use of
FutureBuilderfor one-time async data fetching, consistent with coding guidelines. The theme-aware styling and modular section composition are well organized.
148-205: Clean time frame selector implementation.The horizontal scrollable selector with proper Material ripple effects and theme-aware styling is well implemented.
207-287: LGTM!Grid layout for insight cards is correctly configured with
shrinkWrapandNeverScrollableScrollPhysicsfor proper scrolling behavior within the parentSingleChildScrollView.
644-797: LGTM!Error handling, loading states, and empty state handling are properly implemented. Theme-aware styling is consistent with other cards.
799-964: LGTM!List completion card properly handles all states (loading, error, empty, data). Theme-aware colors are correctly applied to the completion percentage badge.
1052-1092: LGTM!Icon utility methods provide clean mappings from
IconValueenum to colors and icon data. All enum cases are covered.
1 participant
This pull request introduces a new dual-level analytics and insights architecture, adds user and list insights screens, and standardizes loading indicators across the app. It also updates documentation and UI navigation to reflect these changes. The most important changes are grouped below:
Analytics & Insights Architecture
UserInsightsScreen), and list-level insights accessible via a new tab in each list (ListInsightsScreen). Both use dedicated analytics services. [1] [2] [3] [4] [5].github/copilot-instructions.mdwith conventions for analytics field names, navigation tab order, and architectural patterns for future development.Navigation & UI Enhancements
Loading Indicator Standardization
CircularProgressIndicatorwithCustomLoadingSpinnerorCircularProgressIndicatorM3Efor a consistent look across phone, web, and WearOS. Updated relevant widgets and documentation. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Documentation & Linting
library_private_types_in_public_apiwarnings. [1] [2]Miscellaneous
Summary by CodeRabbit
New Features
Improvements
Tests
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.