feat(sleep-reports): align sleep calculations, fix charts, and improve data accuracy#854
feat(sleep-reports): align sleep calculations, fix charts, and improve data accuracy#854CodeWithCJ merged 10 commits intomainfrom
Conversation
…e data accuracy
**Key Changes:**
* **Logic Alignment:** Updated `SleepScience` service to use the verified `time_asleep_in_seconds` from devices instead of manually summing sleep stages. This fixes the discrepancy between "Daily
Breakdown" and "History Table."
* **Metric Accuracy:** Fixed the MCTQ "Sleep Need" calculation to prevent it from spiking to 10 hours based on a single catch-up weekend. Implemented a confidence-weighted baseline with a 9-hour "sanity
cap" for limited data.
* **Data Aggregation:** Implemented "Session Merging" in the Sleep Report. Naps and multiple sleep sessions on the same day are now aggregated into a single "Biological Sleep Day," correcting Sleep
Debt, Efficiency, and Score calculations.
* **Chart Improvements:**
* **Sleep Debt:** Added Red (Debt) and Green (Surplus) background zones and set the Y-Axis to `['auto', 'auto']` to properly visualize negative debt (surplus).
* **Efficiency:** Set Y-Axis to `['auto', 100]` to reveal trends in high-efficiency ranges.
* **Consistency:** Fixed the "Cross-Midnight" bedtime bug (23:00 to 01:00) by normalizing post-midnight times to 24h+ (e.g., 25:00), creating a continuous trend line.
* **Tooltips:** Added custom tooltips to "Sleep Stages" to show human-readable durations (e.g., "1h 30m") instead of raw decimals.
* **UI/UX Polish:**
* Merged the "Daily Breakdown" data into the main History Table (added Impact/Weight column and color-coded Debt).
* Updated "Insight" logic to flag "High Debt" (>1.5h) even if the daily Sleep Score is good.
* Fixed `NaN` display issues in Health Metric charts (SpO2, HRV) by adding null checks for averages.
**Fixes:**
* Resolved TypeScript errors (`undefined` index access) in report aggregators.
* Fixed chart rendering issues where `ReferenceArea` was missing or invisible.
2* Corrected "Awake Periods" property naming in `SleepEntry` type.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the accuracy and presentation of sleep data within the application. It refines sleep calculations by leveraging device-provided data, improves the robustness of the personalized sleep need metric, and aggregates multiple sleep sessions into a single biological sleep day for more coherent reporting. Furthermore, it introduces several chart and UI/UX improvements, such as better visualization of sleep debt and efficiency, a fix for cross-midnight bedtimes, and clearer data presentation in tables and tooltips. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to sleep analytics and reporting. Key changes include adding new translation keys for 'impact' and 'high debt', and refining existing ones. The SleepAnalyticsCharts component now features improved bedtime/wake time formatting to handle cross-midnight scenarios, a custom tooltip for sleep stage charts, and visual enhancements to the sleep debt chart with ReferenceArea and ReferenceLine components to indicate debt/surplus zones. The SleepAnalyticsTable has been updated to include an 'Impact' column, conditional styling for sleep debt, and a revised insight logic that prioritizes 'High Debt'. The SleepReport component underwent a major refactor to aggregate multiple sleep entries per day into a single data point for more accurate daily analytics, and its CSV export now includes 'Weight (%)'. Additionally, the SleepDebtBreakdown component was removed. On the backend, the sleepScienceService was updated to allow negative sleep debt (surplus) in calculations and to introduce a confidence metric for personalized sleep need, with a weighted pull towards a default sleep need for lower confidence data. Review comments highlighted the need to remove redundant ReferenceArea components in SleepAnalyticsCharts, optimize the renderHeaderActions call in SleepAnalyticsTable to avoid inefficient re-renders, and improve type safety by using SleepStageEvent[] instead of any[] for allStageEvents and defining a specific type for combinedEntry.
| <ReferenceArea | ||
| y1={0} | ||
| y2={100} | ||
| fill="#ff0000" | ||
| fillOpacity={0.05} | ||
| /> | ||
| <ReferenceArea | ||
| y1={-100} | ||
| y2={0} | ||
| fill="#00ff00" | ||
| fillOpacity={0.05} | ||
| /> |
There was a problem hiding this comment.
This block adds ReferenceArea components that are redundant with another set defined on lines 569-582. The y-axis range [-100, 100] here also seems incorrect for sleep debt measured in hours. To avoid redundancy and use the more appropriate range from the other block, I suggest removing these ReferenceArea components, while keeping the useful ReferenceLine that follows.
| {renderHeaderActions ? ( | ||
| renderHeaderActions() !== null && ( | ||
| <div className="flex justify-end mb-4">{renderHeaderActions()}</div> | ||
| ) | ||
| ) : ( | ||
| <div className="flex justify-end mb-4"> | ||
| <Button onClick={handleExportClick}> | ||
| {t('sleepAnalyticsTable.exportToCSV', 'Export to CSV')} | ||
| </Button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
The renderHeaderActions function is called twice here: once for the conditional check and again for rendering. This is inefficient as it can cause the component to render twice. It's better to call it once, store the result in a variable, and then use that variable for both the check and the render. You can achieve this with an IIFE (Immediately Invoked Function Expression) to keep the logic contained within the JSX.
{(() => {
if (renderHeaderActions) {
const actions = renderHeaderActions();
return actions !== null ? (
<div className="flex justify-end mb-4">{actions}</div>
) : null;
}
return (
<div className="flex justify-end mb-4">
<Button onClick={handleExportClick}>
{t('sleepAnalyticsTable.exportToCSV', 'Export to CSV')}
</Button>
</div>
);
})()}
| let earliestBedtime = mainEntry.bedtime; | ||
| let latestWakeTime = mainEntry.wake_time; | ||
|
|
||
| const allStageEvents: any[] = []; |
There was a problem hiding this comment.
Using any[] bypasses TypeScript's type safety. Since SleepStageEvent is already defined and imported, please use SleepStageEvent[] here to ensure type correctness throughout the data processing pipeline.
| const allStageEvents: any[] = []; | |
| const allStageEvents: SleepStageEvent[] = []; |
| totalAwakeDuration: aggregatedStages.awake, | ||
| }; | ||
|
|
||
| const combinedEntry: any = { |
There was a problem hiding this comment.
Using any for combinedEntry weakens type safety. This object represents an aggregated sleep session and has a different structure from the base SleepEntry (e.g., it includes is_aggregated). To maintain type safety and improve code clarity, consider defining a new type, for example AggregatedSleepEntry, and use it here. This will also help ensure it's compatible with the CombinedSleepData type it's being assigned to.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request delivers a substantial set of features and fixes for the sleep reports, greatly improving data accuracy and visualization. The introduction of session merging for a single 'Biological Sleep Day' is a major enhancement. The backend changes to sleep need and sleep debt calculations make the metrics more robust and accurate. The frontend sees numerous chart improvements, such as background zones for sleep debt and better handling of cross-midnight times, which significantly enhance usability. My review focuses on a few areas to improve code quality: removing duplicate translation keys, cleaning up redundant chart components, improving performance by avoiding repeated function calls, and enhancing maintainability by using constants for colors.
SparkyFitnessFrontend/src/pages/Reports/SleepAnalyticsCharts.tsx
Outdated
Show resolved
Hide resolved
SparkyFitnessFrontend/src/pages/Reports/SleepAnalyticsTable.tsx
Outdated
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to the sleep reporting and analytics features. Key changes include aligning frontend and backend sleep calculations, implementing session merging to aggregate multiple sleep periods in a day, and substantially improving the accuracy of sleep need and sleep debt calculations. The UI has also been enhanced with better chart visualizations, more informative tooltips, and a consolidated history table.
My review focuses on improving code maintainability by addressing hardcoded values used for calculating user insights. By extracting these thresholds into named constants, the code becomes more readable and easier to update consistently across different components. The core logic for data aggregation and the updated scientific calculations on the backend appear solid and well-implemented.
SparkyFitnessFrontend/src/pages/Reports/SleepAnalyticsTable.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
feat(sleep-reports): align sleep calculations, fix charts, and improve data accuracy
Key Changes:
SleepScienceservice to use the verifiedtime_asleep_in_secondsfrom devices instead of manually summing sleep stages. This fixes the discrepancy between "DailyBreakdown" and "History Table."
cap" for limited data.
Debt, Efficiency, and Score calculations.
['auto', 'auto']to properly visualize negative debt (surplus).['auto', 100]to reveal trends in high-efficiency ranges.NaNdisplay issues in Health Metric charts (SpO2, HRV) by adding null checks for averages.Fixes:
undefinedindex access) in report aggregators.ReferenceAreawas missing or invisible.2* Corrected "Awake Periods" property naming in
SleepEntrytype.Description
Provide a brief summary of your changes.
Related Issue
PR type [x] Issue [x] New Feature [ ] Documentation
Linked Issue: #
Checklist
Please check all that apply:
pnpm run validate(especially for Frontend).en) translation file (if applicable).rls_policies.sqlfor any new user-specific tables.Screenshots (if applicable)
Before
[Insert screenshot/GIF here]
After
[Insert screenshot/GIF here]