Fix duration display showing 6:60 instead of 7:00#13
Merged
Conversation
When summing many sleep segments, floating-point imprecision could leave the remainder at 3599 seconds. Rounding that to minutes produced 60 without rolling into the next hour. Calculate total minutes first, then split into hours and minutes so values like 420 minutes always display as 7:00. Co-authored-by: Greg <gsbernstein@users.noreply.github.com>
Owner
Author
|
Thanks @virbedi! |
There was a problem hiding this comment.
Pull request overview
Fixes an edge-case in TimeFormatter.formatDuration where floating-point imprecision could cause minute rounding to hit 60 without carrying into the hour (e.g., showing 6:60 instead of 7:00) when summing many sleep segment durations.
Changes:
- Compute
totalMinutesby rounding the full duration in minutes. - Derive
hoursandminutesfromtotalMinutesvia integer division/modulo to guaranteeminutesis always0...59.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
The bots were wrong, this shouldn't require floating point imprecision or summing multiple ranges. Just normal rounding. Updated description. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Sleep durations can display as 6:60 instead of 7:00.
Root cause
TimeFormatter.formatDurationcomputed hours and minutes separately from seconds:Hours are truncated while minutes are rounded independently. Any duration from X:59:30 to X:59:59 produces X hours with a remainder that rounds to 60 minutes — without rolling into the next hour. This affects any call to
formatDuration, not just day totals.HealthKit durations are exact
TimeIntervalvalues (including seconds), while individual segment rows round for display. A day total can therefore be something like 6h 59m 45s even when the listed segments appear to sum to a whole number of hours. If the total were exactly 420 minutes (25200 seconds), the old code would actually display 7:00 correctly — so 6:60 indicates sub-minute seconds in the underlying data, not a summation bug.Fix
Calculate total minutes first, then split into hours and minutes:
Durations like 6h 59m 45s now correctly display as
7:00.