Fix off-by-one in stats period start date and day count#621
Open
MtlPhil wants to merge 2 commits intoloopandlearn:devfrom
Open
Fix off-by-one in stats period start date and day count#621MtlPhil wants to merge 2 commits intoloopandlearn:devfrom
MtlPhil wants to merge 2 commits intoloopandlearn:devfrom
Conversation
## Problem The stats model had a systematic off-by-one error in how it calculated the number of days in an analysis period, and used `Date()` (now, today) as the end boundary, including the current incomplete day in averages. ### Root causes 1. **Today was used as `endDate`** (`AggregatedStatsViewModel.updatePeriod`). Today is a partial day and should never be included in averages. 2. **Exclusive date diff used as day count** (`StatsDataService.updateDateRange`). `dateComponents([.day], from: start, to: end)` returns the number of whole days between two instants, which excludes the end day. A range of Apr 19–Apr 25 returned 6, not 7. 3. **Quick-select presets were off by one** (`DateRangePicker.setDateRange`). Pressing "7d" subtracted 7 days from the start of yesterday, producing an 8-day window (Apr 18–25) instead of a 7-day window (Apr 19–25). 4. **Day count label was exclusive** (`DateRangePicker.dayCount`). The "(N days)" header label used the same exclusive diff, showing one fewer day than the range actually covered. 5. **Bolus cutoff re-derived from `Date()`** (`SimpleStatsViewModel`). The cutoff for filtering bolus dates was recalculated as `Date() - requestedDays * 86400` instead of using `dataService.startDate`, making it inconsistent with the resolved date range after today was removed from the end boundary. 6. **Same re-derivation bug in `calculateActualDaysCovered`**. The helper also anchored its own cutoff to `Date()` rather than `dataService.startDate`. 7. **Carbs denominator used days-with-data, not period length** (`SimpleStatsViewModel`). `avgCarbs` divided total carbs by `dailyCarbs.count` (number of days that had at least one carb entry), which inflates the average whenever the user had carb-free days in the period. The band-aid `max(dailyCarbs.count, 1)` was a symptom of this. ## Fix **`AggregatedStatsViewModel.updatePeriod()`** - `endDate` = 23:59:59 of yesterday (last complete day), computed via `startOfDay(for: Date()) - 1 second` using the display calendar. - `startDate` = midnight of `endDay - (days - 1)` so that a "7d" period covers exactly 7 calendar days inclusive (e.g. Apr 19–Apr 25). **`StatsDataService.updateDateRange()`** - `daysToAnalyze` = `daysBetween + 1`, where `daysBetween` is the exclusive `dateComponents` diff between the start-of-day of each boundary. Computing on day-start timestamps avoids DST-induced sub-day remainders from inflating the count. **`DateRangePicker.setDateRange()`** - Start offset changed from `-(days)` to `-(days - 1)` so quick-select presets (7d, 14d, 30d, 90d) produce inclusive ranges. **`DateRangePicker.dayCount`** - Day count = exclusive diff between start-of-day boundaries + 1, ensuring the header label matches the actual number of days covered. **`SimpleStatsViewModel` — bolus cutoff** - `cutoffTime` now reads `dataService.startDate.timeIntervalSince1970` directly. This is consistent with the resolved period and avoids re-deriving a different value from the current clock. **`SimpleStatsViewModel` — carbs denominator** - Denominator changed from `dailyCarbs.count` to `dataService.daysToAnalyze` so that carb-free days are included in the average (total carbs spread over the full period, not just days with entries). **`SimpleStatsViewModel.calculateActualDaysCovered()`** - Cutoff changed from `Date() - requestedDays * 86400` to `dataService.startDate.timeIntervalSince1970` for the same reason as the bolus fix above. ## Time zone behaviour All day-boundary arithmetic uses `dateTimeUtils.displayCalendar()`, which applies the user's configured graph time zone or the device's current time zone. This means: - DST transitions are handled correctly: `startOfDay(for:)` and `date(byAdding: .day)` use calendar days, not fixed 86400-second intervals, so 23-hour and 25-hour DST days do not shift boundaries. - Travel (device time zone change) causes the analysis window to be recomputed relative to the new local midnight on the next load, which is the expected behaviour. - Users with a fixed graph time zone are fully insulated from travel: all boundaries stay anchored to the configured zone.
AggregatedStatsView.init() was hardcoding the initial @State dates with value: -7 from endDayStart, producing an 8-day window (Apr 18–Apr 25) instead of the intended 7-day inclusive window (Apr 19–Apr 25). The previous commit fixed setDateRange() and updatePeriod() but missed this init(), which bypasses both and seeds the @State directly.
bjorkert
added a commit
that referenced
this pull request
Apr 27, 2026
bjorkert
added a commit
that referenced
this pull request
Apr 27, 2026
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
1 participant
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.
The stats model has a systematic off-by-one error in how the analysis window start date is computed, causing the selected period to span one extra day and all per-day averages to be divided by the wrong count.
Problem
When a quick-select preset (7d, 14d, 30d) is chosen, or when the stats view opens, the end date is correctly set to the end of yesterday (the last complete day). However, the start date is computed by subtracting
Ncalendar days from the start of the end day rather thanN - 1, producing a window ofN + 1days.For example, selecting "7d" with yesterday = Apr 25 produces:
The day count label compounds the issue by using an exclusive date diff (
dateComponents([.day], from: start, to: end)), so it displays "7 days" for an 8-day window — the label and the actual range disagree in opposite directions.StatsDataService.updateDateRange()then computesdaysToAnalyzefrom the same exclusive diff, storing 7 for an 8-day range. This value is used as the denominator in per-day averages, overstating them.Secondary inconsistencies:
SimpleStatsViewModelre-derives its own anchor fromDate() - requestedDays * 86400instead of readingdataService.startDate, so it can diverge from the resolved period.avgCarbsusesdailyCarbs.count(days with at least one entry) as the denominator rather than the full period length, inflating the average on carb-free days.calculateActualDaysCovered()has the sameDate()-anchored re-derivation.Fix
endDay - (N - 1)so a "7d" preset spans exactly Apr 19–Apr 25 (7 days inclusive).daysToAnalyzeis computed asdaysBetween + 1on day-start boundaries.calculateActualDaysCoveredreaddataService.startDatedirectly.avgCarbsdenominator usesdataService.daysToAnalyze.AggregatedStatsView.init()had a separate hardcodedvalue: -7offset that was also corrected to-(7 - 1).Time zone note
All date arithmetic goes through
dateTimeUtils.displayCalendar(), which respects the user's configured graph time zone or the device time zone.startOfDay(for:)anddate(byAdding: .day)use calendar days rather than fixed 86 400-second intervals, so DST transitions are handled correctly.