You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Count notification retention in calendar days, not 336 hours - #687
FiredKeyStore._cutoffLabel() and NotificationIds._prune each computed the 14-day retention cutoff as dayLabelOf(DateTime.now().subtract(const Duration(days: 14))). That is 336 hours, not 14 calendar days: across a DST change one of those days is 23 or 25 h, so in the hour either side of local midnight the cutoff lands one label early or late. AGENTS.md §3.7 calls this out ("Day-length arithmetic must not assume 86400 s"), and day_label.dart already has dayLabelBefore for exactly this.
Impact is small, to be upfront: twice a year, dated fired-key / slot bookkeeping is kept a day longer or shorter. The dedupe only needs today's keys. The other reason to fix it is that the same cutoff was written out twice and could drift.
What changes
FiredKeyStore.cutoffLabel([DateTime? now]) (was private _cutoffLabel()): dayLabelBefore(todayLabel(now), retentionDays)!. Both of its callers (the DB prune and the prefs-mirror prune) use it.
NotificationIds._prune uses FiredKeyStore.cutoffLabel() instead of its own copy (NotificationIds.retentionDays is already defined as FiredKeyStore.retentionDays). Its now-unused day_label.dart import is dropped.
Tests
New test/notify_retention_dst_test.dart moves the process timezone to America/New_York (libc setenv/tzset, as day_window_dst_test does) and checks the cutoff at 00:30 the day after the 23 h day (2026-02-23, the old code gave 2026-02-22), at 23:30 the day after the 25 h day (2026-10-19, old: 2026-10-20), and on an ordinary day. A first test asserts the zone really has DST, so the others can't pass vacuously. With the old formula, the two DST cases fail.
All 9 test files that touch FiredKeyStore / NotificationIds: 86 passed.
flutter analyze: no issues.
Full suite not re-run for this change: it only touches the two prune paths, all of whose tests are above.
Notification history is now pruned based on calendar days, keeping retention consistent across stored notification records and IDs. This avoids differences caused by elapsed-hour calculations and helps ensure that records older than the retention period are handled consistently, including around changes in the time of day.
FiredKeyStore and NotificationIds each computed the retention cutoff as
dayLabelOf(now.subtract(Duration(days: 14))). That is 14 x 24 h: across
a DST change one day is 23 or 25 h, so in the hour either side of local
midnight the cutoff landed one label early or late (AGENTS.md: day
arithmetic must not assume 86400 s).
One shared FiredKeyStore.cutoffLabel([now]) now does it with the
existing calendar helpers, dayLabelBefore(todayLabel(now), 14), and
NotificationIds uses it instead of its own copy. The effect was only a
day more or less of notification bookkeeping kept twice a year, but the
two copies could also have drifted apart.
Reviewing files that changed from the base of the PR and between e10dcaf and fa9963d.
Files ignored due to path filters: 1
test/notify_retention_dst_test.dart is excluded by !test/**
Files selected for processing: 2
lib/notify/fired_keys.dart
lib/notify/notification_ids.dart
No actionable comments were generated in the recent review.
Walkthrough
Notification retention pruning now uses a shared calendar-day cutoff helper. The helper accepts an optional date. Database, SharedPreferences, and notification ID pruning use it.
FiredKeyStore.cutoffLabel([DateTime? now]) calculates the cutoff using calendar-day labels. Database and SharedPreferences pruning use the helper. Notification ID pruning also uses it and removes its day_label.dart import.
This change computes the 14-day notification retention cutoff by calendar day instead of subtracting 336 hours, which avoids off-by-one-day cutoffs around daylight-saving changes. No actionable merge-blocking risk was found.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: retention cutoff is now counted in calendar days instead of 336 hours. The raw summary confirms this: FiredKeyStore.cutoffLabel replaces the 14x24-hour subtraction, an…
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.
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Full details: Title check
Explanation
The title names the main change: retention cutoff is now counted in calendar days instead of 336 hours. The raw summary confirms this: FiredKeyStore.cutoffLabel replaces the 14x24-hour subtraction, and both prune paths use it. The title is specific and concise and matches the PR objectives, which cite DST-related off-by-one cutoff labels as the motivation.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 3fd7f37 and 43df1db.
Files ignored due to path filters: 1
test/notify_retention_dst_test.dart is excluded by !test/**
Files selected for processing: 2
lib/notify/fired_keys.dart
lib/notify/notification_ids.dart
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.
Verify the retention cutoff across daylight-saving transitions and on an ordinary day.
test/notify_retention_dst_test.dart L1-67
Tips and commands
Interacting with Sourcery
Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="test/notify_retention_dst_test.dart"line_range="30" />
<code_context>
+ lib.lookupFunction<_UnsetenvNative, _UnsetenvDart>('unsetenv')(key);+ } else {
+ final value = tz.toNativeUtf8();+ lib.lookupFunction<_SetenvNative, _SetenvDart>('setenv')(key, value, 1);+ calloc.free(value);+ }
</code_context>
<issue_to_address>
**Windows test setup fails**
When the Flutter test suite runs on Windows, `_setProcessTz` looks up POSIX `setenv`/`tzset` symbols that Windows does not provide, so `setUpAll` throws before assertions run and the test file fails.
Use a Windows-compatible way to set and restore the process time zone, or skip this setup on Windows.
Also at `test/notify_retention_dst_test.dart:42`.
</issue_to_address>
Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the new calendar-day cutoff is used to prune both fired-key and notification-slot claims. If it removes a claim too soon, a later event can send a duplicate notification; reverting does not restore claims already deleted.
Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
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
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.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Why
FiredKeyStore._cutoffLabel()andNotificationIds._pruneeach computed the 14-day retention cutoff asdayLabelOf(DateTime.now().subtract(const Duration(days: 14))). That is 336 hours, not 14 calendar days: across a DST change one of those days is 23 or 25 h, so in the hour either side of local midnight the cutoff lands one label early or late. AGENTS.md §3.7 calls this out ("Day-length arithmetic must not assume 86400 s"), andday_label.dartalready hasdayLabelBeforefor exactly this.Impact is small, to be upfront: twice a year, dated fired-key / slot bookkeeping is kept a day longer or shorter. The dedupe only needs today's keys. The other reason to fix it is that the same cutoff was written out twice and could drift.
What changes
FiredKeyStore.cutoffLabel([DateTime? now])(was private_cutoffLabel()):dayLabelBefore(todayLabel(now), retentionDays)!. Both of its callers (the DB prune and the prefs-mirror prune) use it.NotificationIds._pruneusesFiredKeyStore.cutoffLabel()instead of its own copy (NotificationIds.retentionDaysis already defined asFiredKeyStore.retentionDays). Its now-unusedday_label.dartimport is dropped.Tests
test/notify_retention_dst_test.dartmoves the process timezone to America/New_York (libcsetenv/tzset, asday_window_dst_testdoes) and checks the cutoff at 00:30 the day after the 23 h day (2026-02-23, the old code gave2026-02-22), at 23:30 the day after the 25 h day (2026-10-19, old:2026-10-20), and on an ordinary day. A first test asserts the zone really has DST, so the others can't pass vacuously. With the old formula, the two DST cases fail.FiredKeyStore/NotificationIds: 86 passed.flutter analyze: no issues.🤖 Generated with Claude Code
Summary by Sourcery
Use calendar-day retention cutoffs consistently for fired-key and notification-slot pruning.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit