-
-
Notifications
You must be signed in to change notification settings - Fork 72
Steps from a real pedometer only; movement minutes on measured evidence #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
abdulsaheel
wants to merge
6
commits into
main
Choose a base branch
from
feat/real-steps-phone-pedometer
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
c53f399
Steps from a real pedometer only; movement minutes on measured evidence
abdulsaheel 87a8a5d
Address review: dropped phone steps, a discarded floor, Android + DST…
abdulsaheel 07ade7f
Fix the DST, ordering and honesty gaps in the steps rework
abdulsaheel f804659
Close the bot findings I had not actually read
abdulsaheel abe614f
Make the backfill floor guard's reachability obvious
abdulsaheel f97e278
Stop an all-zero phone read erasing a real day; unlabel absent steps
abdulsaheel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
Large diffs are not rendered by default.
Oops, something went wrong.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| /// PURE policy for the frozen personal movement floor. | ||
| /// | ||
| /// The floor is a SINGLE persisted personal scalar, not a per-day value: once | ||
| /// committed it is applied to every day, past and future. That is the whole | ||
| /// point of freezing it — a floor derived from the signal it thresholds cancels | ||
| /// the trend it exists to report if it keeps tracking the user (measured on real | ||
| /// substrate: 37 active minutes at 1x, 1.5x, 2x AND 3x activity when | ||
| /// recomputed, versus 23 -> 254 frozen). | ||
| /// | ||
| /// Because it is one shared scalar, resolving it is a READ-MODIFY-WRITE against | ||
| /// state every day of a sweep touches. `DerivationEngine.run()` dispatches days | ||
| /// NEWEST-FIRST through a concurrent worker pool, so the decisions below have to | ||
| /// be order-independent or the frozen floor is decided by a race. These helpers | ||
| /// are pure so that property is unit-testable without a database. | ||
| library; | ||
|
|
||
| import '../data/day_label.dart'; | ||
|
|
||
| /// The `YYYY-MM-DD` label [back] calendar days before [dayId]. | ||
| /// | ||
| /// CALENDAR arithmetic, never `Duration`. `DateTime.subtract(Duration(days: n))` | ||
| /// is ABSOLUTE: from local midnight on 2026-03-10 (US), subtracting 24 h lands | ||
| /// at 23:00 on 2026-03-07 because 2026-03-08 was only 23 h long — so the walk | ||
| /// SKIPS 2026-03-08 entirely and the caller mis-counts the gap. Feeding an | ||
| /// out-of-range day field to the `DateTime` constructor normalises correctly. | ||
| String? dayLabelBefore(String dayId, int back) { | ||
| final d = DateTime.tryParse(dayId); | ||
| if (d == null) return null; | ||
| return dayLabelOf(DateTime(d.year, d.month, d.day - back)); | ||
| } | ||
|
|
||
| /// Consecutive days immediately before [dayId] with no entry in [have]. | ||
| /// | ||
| /// A missing `dyn_p90` daily summary means the band produced no usable motion | ||
| /// that day, i.e. it was not worn. Used only as a re-freeze trigger: a long gap | ||
| /// suggests the body/device relationship may have changed enough that the frozen | ||
| /// floor should be re-estimated. | ||
| /// | ||
| /// Returns 0 when [have] is empty — an empty history is "no information", not "a | ||
| /// 60-day gap", and must not be allowed to trigger a re-freeze. | ||
| int wearGapDays({ | ||
| required Set<String> have, | ||
| required String dayId, | ||
| int maxScan = 60, | ||
| }) { | ||
| if (have.isEmpty) return 0; | ||
| var gap = 0; | ||
| for (var back = 1; back <= maxScan; back++) { | ||
| final label = dayLabelBefore(dayId, back); | ||
| if (label == null) return gap; | ||
| if (have.contains(label)) break; | ||
| gap++; | ||
| } | ||
| return gap; | ||
| } | ||
|
|
||
| /// Age of the frozen floor as seen from [dayId], NEVER negative. | ||
| /// | ||
| /// A day BEFORE the freeze date is not a stale floor — it is a backfill. The | ||
| /// previous `.abs()` made every historical re-derive look maximally stale, which | ||
| /// matters because a `kAlgoVersion` bump re-derives days newest-first: walking | ||
| /// backwards past `maxAgeDays` tripped the staleness rule and re-froze the | ||
| /// shared floor onto an OLDER `frozenOn`, which could then trip again on the | ||
| /// next real derive. Clamping to 0 makes a backfill day simply consume the | ||
| /// stored floor, which is what "frozen" means. | ||
| int daysSinceFrozen({required String frozenOn, required String dayId}) { | ||
| final from = DateTime.tryParse(frozenOn); | ||
| final to = DateTime.tryParse(dayId); | ||
| if (from == null || to == null) return 0; | ||
| final diff = to.difference(from).inDays; | ||
| return diff > 0 ? diff : 0; | ||
| } | ||
|
|
||
| /// May [dayId] commit (or re-commit) the shared floor? | ||
| /// | ||
| /// A day may only move the floor FORWARD in time. Without this, a backfill day | ||
| /// in a newest-first sweep could overwrite a freeze that a newer day had just | ||
| /// established, making the persisted floor — and therefore every day's | ||
| /// `active_min` — depend on which worker in the pool finished last. | ||
| /// | ||
| /// This is the same principle `_BaselineHistoryCache.valuesBefore` already | ||
| /// states for baselines: a sweep must not make the result depend on sweep order. | ||
| bool mayCommitFloorOn({required String? frozenOn, required String dayId}) { | ||
| if (frozenOn == null) return true; | ||
| return dayId.compareTo(frozenOn) >= 0; | ||
| } | ||
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: OpenStrap/edge
Length of output: 2927
🏁 Script executed:
Repository: OpenStrap/edge
Length of output: 20576
🏁 Script executed:
Repository: OpenStrap/edge
Length of output: 9822
🌐 Web query:
Dart documentation DateTime inDays Duration days property rounds toward zero example local DST💡 Result:
In Dart, the Duration.inDays property returns the number of entire days spanned by a Duration object [1][2]. The value is calculated using integer division (
~/) of the duration's total microseconds by the number of microseconds in a day [1][3]. This effectively truncates the result toward zero, meaning it returns the number of whole days and discards any remaining time (hours, minutes, etc.) [4][5][6]. For example: - A duration of 4 days and 3 hours will return 4 for inDays [1][2]. - A duration of 88 hours (3 days and 16 hours) will return 3 for inDays [4][6]. Regarding Daylight Saving Time (DST) and DateTime calculations: The Duration class represents a fixed span of time independent of calendar or time zone context [5]. When using DateTime.difference to calculate a duration between two points in local time, the result is the absolute difference in microseconds [7][8]. Because DST changes can cause a day to have 23 or 25 hours rather than 24, calculating the difference between two calendar dates in local time may not yield a whole multiple of 24 hours [9][7]. If a DST transition causes a period to be shorter than a full 24-hour day (e.g., 23 hours), Duration.inDays will return 0 for that period because the time span is less than one complete day [9]. If you need to count calendar days regardless of DST shifts, it is recommended to perform calculations using UTC time instead of local time [9].Citations:
🌐 Web query:
Dart DateTime inDays Duration property rounds toward zero example local DST💡 Result:
In Dart, the Duration.inDays property retrieves the number of entire days in a duration by performing integer division of the total microseconds by the number of microseconds in a day [1][2]. Consequently, it effectively truncates toward zero—it always rounds down to the nearest whole number (e.g., a duration of 4 days and 23 hours returns 4) [1][2]. Because Duration is a context-independent measurement of elapsed time—where 1 day is strictly defined as 24 hours—it does not account for calendar-based changes like Daylight Saving Time (DST) [2][3][4]. When you use DateTime.difference on local times that cross a DST transition, the resulting Duration may not contain a multiple of 24 hours, leading to behavior that can appear "incorrect" if you are expecting calendar days [5][6][4]. Example of DST impact on inDays: If you calculate the difference between two midnights in a local time zone that transitions to DST, the elapsed time might be 23 hours instead of 24. Even though it is "one calendar day" apart, Duration.inDays will return 0 because it has not yet reached the 24-hour threshold [5][4][7]. Example of rounding: const duration = Duration(hours: 47); print(duration.inDays); // Output: 1 (47 hours / 24 = 1.958, rounded down to 1) [1][2] To perform calendar-aware arithmetic that respects DST or standard calendar days, it is recommended to use UTC time to avoid local zone shifts or to manually handle day increments using the DateTime constructor [5][6][3][8].
Citations:
Use UTC calendar fields for
daysSinceFrozen.daysSinceFrozenusesto.difference(from).inDayson local-midnight instants. Near spring-forward, one calendar day can be 23 hours, soinDaysreports 0 and delays the re-freeze age check. Subtract the constructed UTC calendar days before clamping at zero. The caller fromDerivationEngine.run()reads this age during newest-first sweeps.Proposed fix
📝 Committable suggestion
📍 Affects 2 files
lib/compute/movement_floor_policy.dart#L66-L71(this comment)test/movement_floor_policy_test.dart#L11-L18test/movement_floor_policy_test.dart#L70-L86🤖 Prompt for AI Agents
Source: Coding guidelines