Skip to content

Conversation

@mirland
Copy link
Collaborator

@mirland mirland commented Dec 11, 2025

This pull request introduces significant updates to both the VytalLink app's recap workflow and its supporting documentation, as well as minor codebase and configuration improvements. The most important changes are the addition of a comprehensive recap prompt and style guide for generating Instagram-ready yearly summaries, enhancements to the assistant's logic for recap mode, and improvements to health data aggregation. These updates aim to provide users with a seamless, visually appealing, and data-driven yearly recap experience, while ensuring accuracy and clarity in metric handling.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a "year recap" feature to VytalLink, enabling users to generate Spotify Wrapped-style health summaries. The implementation introduces a new SummaryDataManager service that aggregates health metrics over configurable time periods with automatic preset selection based on duration.

Key changes:

  • New SummaryDataManager service for processing multi-metric health data requests with configurable time grouping
  • Integration with the MCP server to handle summary requests from the GPT assistant
  • Health data batching (75-day chunks) to prevent ANR issues on Android
  • Addition of sessionCount field to workout summaries for better tracking
  • Updated documentation with GPT prompts for the recap feature workflow

Reviewed changes

Copilot reviewed 31 out of 32 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
mobile/lib/core/service/summary_data_manager.dart New service implementing summary data processing with preset-based time grouping
mobile/test/unit/core/service/summary_data_manager_test.dart Comprehensive unit tests for the SummaryDataManager
mobile/lib/core/service/server/mcp_server.dart Integration of summary request handling in the MCP server
mobile/lib/core/model/summary_request.dart New request models for summary metrics and requests
mobile/lib/core/model/summary_response.dart New response models for summary data
mobile/lib/core/service/health_data_manager.dart Added batching to prevent ANR on large date ranges
mobile/lib/core/health/health_data_aggregator.dart Refactored workout aggregation to track session counts
mobile/lib/core/model/workout_summary_data.dart Added sessionCount field to workout summaries
mobile/lib/core/common/extension/date_time_extensions.dart New DateTime utility methods (min/max)
mobile/pubspec.yaml Updated Flutter SDK version and switched health package to forked version
docs/VYTALLINK_2025_RECAP_PROMPT.md GPT assistant prompts for year recap feature
docs/VYTALLINK_2025_RECAP_STYLE.txt Visual styling specifications for recap images
mobile/android/app/build.gradle Added NDK version specification
mobile/android/app/src/main/AndroidManifest.xml Enabled largeHeap for better memory management

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +44 to +49
return SummaryResponse(
success: overallSuccess,
startTime: request.startTime.toIso8601String(),
endTime: request.endTime.toIso8601String(),
results: results,
errorMessage: overallSuccess ? null : 'Some metrics failed',
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message "Some metrics failed" is too generic and doesn't provide useful debugging information. Consider including details about which metrics failed or a more descriptive message that helps users understand what went wrong.

Suggested change
return SummaryResponse(
success: overallSuccess,
startTime: request.startTime.toIso8601String(),
endTime: request.endTime.toIso8601String(),
results: results,
errorMessage: overallSuccess ? null : 'Some metrics failed',
// Identify failed metrics for a more descriptive error message
final failedMetrics = results
.where((result) => !result.success)
.map((result) => result.metricName ?? 'Unknown')
.toList();
final errorMsg = overallSuccess
? null
: 'The following metrics failed: ${failedMetrics.join(", ")}';
return SummaryResponse(
success: overallSuccess,
startTime: request.startTime.toIso8601String(),
endTime: request.endTime.toIso8601String(),
results: results,
errorMessage: errorMsg,

Copilot uses AI. Check for mistakes.
Comment on lines +168 to +176
StatisticType _defaultStatisticForMetric(
VytalHealthDataCategory category,
_SummaryPreset preset,
) =>
switch (category) {
VytalHealthDataCategory.SLEEP => StatisticType.average,
VytalHealthDataCategory.HEART_RATE => StatisticType.average,
_ => StatisticType.sum,
};
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter 'preset' is declared but never used in the method body. Since the method's return value doesn't depend on the preset parameter, consider removing it from the method signature to avoid confusion and maintain clean code.

Copilot uses AI. Check for mistakes.
workoutType: 'UNKNOWN',
totalDistance: 0.0,
totalEnergyBurned: 0.0,
totalSteps: 0.0,
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The '_emptyWorkoutSummary' constant is missing the 'sessionCount' field that was added to WorkoutSummaryData. This will cause the constant to use the default value (0) for sessionCount, which may be the intended behavior for an empty summary, but it's good practice to explicitly include all fields for clarity and to avoid confusion.

Suggested change
totalSteps: 0.0,
totalSteps: 0.0,
sessionCount: 0,

Copilot uses AI. Check for mistakes.
Comment on lines +127 to +136
final sleepRequest = requests
.whereType<HealthDataRequest>()
.firstWhere((r) => r.valueType == VytalHealthDataCategory.SLEEP);

expect(stepsRequest.groupBy, TimeGroupBy.month);
expect(stepsRequest.statistic, StatisticType.sum);
// Sleep fills defaults based on preset (week for this range) and average
// statistic.
expect(sleepRequest.groupBy, TimeGroupBy.week);
expect(sleepRequest.statistic, StatisticType.average);
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation detected. Lines 127-136 use tab characters for indentation instead of spaces, which is inconsistent with the rest of the file and Dart coding standards. This should be changed to use spaces consistently throughout the file.

Copilot uses AI. Check for mistakes.
}) : _healthDataManager = healthDataManager ?? HealthDataManager(),
_summaryDataManager = summaryDataManager ??
SummaryDataManager(
healthDataManager: healthDataManager,
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential inconsistency in the initialization of SummaryDataManager. If 'healthDataManager' is null, the code creates a new HealthDataManager instance (line 31), but then passes the null 'healthDataManager' parameter to SummaryDataManager (line 34). This means SummaryDataManager will create its own separate HealthDataManager instance instead of using the one created on line 31. Consider passing the initialized '_healthDataManager' to ensure both services use the same instance.

Suggested change
healthDataManager: healthDataManager,
healthDataManager: healthDataManager ?? HealthDataManager(),

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants