Repository navigation
DST: doses on clock-change days were deleted and never rang - #20
Conversation
The helper's commit fixed the day loop; the time of day had the same flaw. On spring-forward day midnight + 480 minutes is 09:00, not 08:00: the dose was saved an hour late, _reconcileOrphanedDoses saw it wasn't a listed slot and deleted it with its alarm, so nothing rang that day. DoseScheduler.slot builds the time from date parts. Same fix for the Today window (ended at 23:00 on the 25-hour day) and the adherence grid (26 weeks back crosses a clock change, so older cells landed on 23:00 the day before and showed the previous day's data). CI runs the tests in Europe/Warsaw so the new DST test means something. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Dose slot construction and DST validation lib/services/scheduler.dart, test/dst_test.dart, .github/workflows/ci.yml |
DoseScheduler.slot constructs a dose time from the date and minute offset. Dose materialization uses this helper. The DST test checks 08:00 and 23:30 slots on two 2026 clock-change dates. CI sets TZ to Europe/Warsaw. |
Adherence calendar ranges lib/ui/adherence_grid.dart, lib/ui/today_page.dart |
The grid constructs dates using calendar components. TodayPage.refresh uses calendar dates for the query end and the start of the 26-week adherence window. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Merge Risk: 
c7252
-
Dose scheduling and adherence dates now use calendar dates, which fixes doses being moved or deleted on clock-change days. Two narrow edge cases remain: a slot inside the spring-forward gap, and a one-time duplicate-alarm case on upgrade. Neither blocks merging.
Security architecture risk: 
c7252
-
The change fixes daylight-saving drift without adding access or privileges. A narrow upgrade case can nevertheless leave an extra medication alarm armed when older doses coexist with newly calculated doses. The exposure is limited to the affected installation and transition window.
Retained concerns
- Medium · reliability · inferred: An upgrade before fall-back doses are due can leave an extra alarm armed. In Europe/Warsaw, a legacy schedule containing 02:30 and 03:30 can materialise two distinct 02:30 instants. Both pass reconciliation because 02:30 remains a configured slot. The new calculation adds the correct 03:30 occurrence without removing either legacy row, allowing three independently armed doses for two intended slots. This is an inferred migration and cleanup failure introduced by mixing old and new timestamp identities, rather than an attacker-driven security finding.
Security review details
-
Security Blast Radius
- inferred — The demonstrated failure scope is the affected installation’s dose records and native alarms. It requires an existing medication schedule and legacy materialised state; the inspected change does not establish cross-user, cross-service or infrastructure authority expansion.
Trust Boundaries and Controls
- inferred — The public-entrypoint signals correspond to in-process Dart helpers, widgets and tests, not newly exposed network handlers. The inspected flow remains schedule input through the medication editor, stored medication data, scheduler reconciliation and the existing native alarm channel. No new attacker-controlled source or control bypass was established.
Resilience and Maintainability Implications
- inferred — The database-to-alarm transition remains non-atomic: insertion, cancellation, deletion and arming are separate awaited operations. A later successful sync can repair an unarmed pending row through exact-key idempotency, but this recovery mechanism predates the PR and does not repair calendar-equivalent legacy duplicates.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the main bug and its effect: doses on clock-change days were deleted and never rang. The changeset fixes DST handling in dose scheduling (new DoseScheduler.slot helper, day loop, Today… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the main bug and its effect: doses on clock-change days were deleted and never rang. The changeset fixes DST handling in dose scheduling (new DoseScheduler.slot helper, day loop, Today page, adherence grid) and adds DST tests, so the title matches the primary change. It is specific enough for a teammate scanning history. Its wording is a bit long, but it is not vague or misleading.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (5 skipped: 5 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
What was wrong
DoseScheduler._materialiseDosesbuilt dose times by addingDurations to local midnight, and in Dart that's exact elapsed time. Across a DST change, wall-clock time is off by an hour._reconcileOrphanedDosessaw that 09:00 isn't one of the medication's slots (coversSlot), cancelled its alarm and deleted it. Every dose after 02:00 that day silently didn't ring. It happened again on every sync.start + 24h, so on the 25-hour day it ended at 23:00 and doses between 23:00 and midnight weren't listed.What changed
DoseScheduler.slot(day, minutes)builds the time from date parts (DateTime(y, m, d, 0, minutes)). The day loop usesDateTime(y, m, d + n).today_page.dartandadherence_grid.dartuse the same date-part construction instead ofadd/subtract(Duration(days: …)).test/dst_test.dart: on both 2026 clock-change days, each slot keeps its date and wall-clock minute andcoversSlotaccepts it.flutter testwithTZ=Europe/Warsaw. In UTC this test can't fail, so without it CI wouldn't catch a regression.Validation
flutter analyze: no issues.flutter test: 29 passed under bothTZ=Europe/WarsawandTZ=UTC.day.add(Duration(minutes: …))and the new test fails in Warsaw (Expected: <480>, Actual: <540>).flutter testrun after a change sometimes fails with "loading …widget_test.dart". It happens on untouchedmastertoo and passes on rerun, so it isn't from this change.Who did what
The Codex helper (gpt-6-luna) found the day-loop half (first commit). I found the time-of-day, Today and grid cases, wrote the test and the CI change, and ran everything (second commit).
🤖 Generated with Claude Code
Summary by CodeRabbit