Skip to content

Calories: stop double-counting basal, make the live gauge survive its re-score - #227

Merged
abdulsaheel merged 8 commits into
OpenStrap:mainfrom
svssathvik7:fix/calorie-accuracy
Aug 11, 2026
Merged

Calories: stop double-counting basal, make the live gauge survive its re-score#227
abdulsaheel merged 8 commits into
OpenStrap:mainfrom
svssathvik7:fix/calorie-accuracy

Conversation

@svssathvik7

@svssathvik7 svssathvik7 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Calories are computed in four places (day derivation, the pure 1 Hz pipeline,
the live session tick, and the manual/re-score path). Three had independently
drifted. This routes all four through one shared implementation.

Bumps kAlgoVersion 61 → 62 — every fix below moves persisted numbers, so
finalized days recompute.


Issue 1 — Daily active calories double-count the basal minute

_keytelCaloriesWake, a derivation-local copy of Keytel, summed the full
active rate for every active minute. calories_total came from
Calories.dailyEnergy, whose active component nets out the basal minute
already counted inside the total.

The same minute was therefore paid for twice.

  • calories overstated by basalPerMin × active-minutes — ~70 kcal on a day
    with one hard hour, scaling with active time.
  • It propagated: the Health export writes BASAL_ENERGY_BURNED as
    calories_total − calories, so basal was understated by the same amount.
  • Root cause was a second implementation of a published formula. That is what
    let the two drift.

Fix: local copy deleted. Active and total come from one wakeDayEnergy
pass over the full-day series, so total − active == basal holds by
construction.


Issue 2 — The live gauge does not survive its own re-score

The tick billed the raw Keytel active rate for every second the band reported a
heart rate — no activity gate, no resting floor.
Calories.estimateBoutCalories, which the substrate re-score and every manually
logged session use, bills the Harris–Benedict resting rate below
RHR + 0.30 × HRR and Keytel only above it.

  • The two rates differ by ~2× at 70 bpm, so warm-up, inter-set rest and
    cool-down were charged at roughly double.
  • A session would finish, re-score, and land lower than the number the athlete
    had been watching for the last hour
    .

Fix: the tick recomputes from the retained per-minute series through the
same rates and the same gate. Recompute rather than accrue, matching what
accrueHr already does for strain — that is what lets a nightly resting HR
arriving mid-session re-score the whole bout instead of only the seconds after
it landed.


Issue 3 — The fitness anchor is computed and then ignored

Keytel publishes two active-EE models; the one that reads VO₂max is the more
accurate. vo2maxEstimate (Uth: 15.3 × HRmax/RHR) was already being
computed
in the crossday pipeline and shown on the Body screen. The consumer
that benefits most never saw it, so every user was scored against the derivation
cohort's mean fitness.

Fix: threaded through all four call sites via one shared vo2maxFor helper.

  • No usable resting HR → vo2maxFor abstains and the original published model
    runs, unchanged. Never fabricates a fitness level.
  • No crossday dependency: Uth VO₂max is a function of age and resting HR, so
    each site resolves it locally and day derivation stays deterministic.

What moves for users

  • Daily active calories go down (they were inflated); exported basal goes
    up by the same amount.
  • Workout calories go down somewhat — the low-intensity portions were
    overcharged.
  • Both now shift with fitness: lower resting HR → higher burn at the same heart
    rate, and vice versa.
  • Users without the profile anchors or a resting HR still see , not a guess.
    Unchanged.

Reviewer note

wakeDayEnergy falls back to heightCm ?? 170.0 for the Mifflin floor rather
than abstaining. Deliberate: it matches the two existing callers
(onehz_pipeline, manual_session) and preserves the documented
hasCalorieAnchors contract (age/mass/sex, not height). Height moves the
Mifflin floor ~6 kcal/cm/day, well inside this estimate's error bar — unlike
body mass and sex, which change the active rate directly and stay hard-gated.

Depends on

OpenStrap/analytics#42Calories gains an optional vo2max. Merge that
first, then bump the openstrap_analytics SHA in pubspec.yaml. Issue 3's
wiring does not compile against the current pin.

Testing

  • test/daily_energy_consistency_test.dart (new) — pins the double count and
    the total − active == basal invariant.
  • test/live_rescore_calorie_parity_test.dart (new) — the live figure equals
    what computeManualSessionStats says about the same stream. Calls the real
    re-score, not a stand-in: a stand-in kept passing the old model when the
    fitness term went in, which is precisely how these paths drifted.
  • test/vo2max_calorie_wiring_test.dart (new) — every path picks up the fitness
    term together, and abstains together without a resting HR.
  • test/workout_calorie_anchors_test.dart — updated. One case ("a costed
    session that came to nothing reports zero") is no longer reachable: with a
    resting floor a costed session is always positive.

Expected values are hand-computed from the published equations, so the tests
fail if the paths ever agree on a wrong number.

Full suite: 1781 passed, 2 pre-existing skips. flutter analyze clean.

Summary by CodeRabbit

  • Improvements
    • Improved daily and workout calorie calculations using heart rate, activity, resting rate, profile details, and covered time.
    • Added more consistent calorie and strain results between live workouts, manual sessions, and daily activity.
    • Improved handling of incomplete profile or sensor data; unavailable calorie results are no longer reported as zero.
    • Standardized fitness calculations across profile sex formats.
  • Live Activity
    • Lock-screen and Dynamic Island displays now show “—” when calories or strain are unavailable.

… re-score

Calories are computed in four places and three of them had drifted apart. All
four now route through one shared implementation. kAlgoVersion 61 -> 62: every
fix below moves persisted numbers, so finalized days recompute.

1. THE DAY DOUBLE-COUNTS THE BASAL MINUTE. `_keytelCaloriesWake`, a
derivation-local copy of Keytel, summed the FULL active rate per active minute,
while `calories_total` came from `Calories.dailyEnergy`, whose active component
nets out the basal minute already inside the total. The same minute was paid for
twice: `calories` ran high by basalPerMin x active-minutes (~70 kcal on a day
with one hard hour, scaling with active time), and because the Health export
writes BASAL_ENERGY_BURNED as `calories_total - calories`, basal ran low by the
same amount. The local copy is deleted; both figures come from one
`wakeDayEnergy` pass, so `total - active == basal` holds by construction.

2. THE LIVE GAUGE DID NOT SURVIVE ITS OWN RE-SCORE. The tick billed the raw
Keytel active rate for every second the band reported a heart rate, with no
activity gate and no resting floor, while `estimateBoutCalories` bills the
Harris-Benedict resting rate below RHR + 0.30*HRR. The two differ by ~2x at
70 bpm, so warm-up, inter-set rest and cool-down were charged at roughly double
and the session re-scored lower than the number the athlete watched all hour.
The tick now recomputes from the retained per-minute series through the same
rates and gate — recompute, not accrue, matching what accrueHr already does for
strain, which is what lets a resting HR landing mid-session re-score the whole
bout instead of only the seconds after it arrived.

3. THE FITNESS ANCHOR WAS COMPUTED AND IGNORED. Keytel publishes a second,
more accurate model that reads VO2max, and `vo2maxEstimate` was already being
computed for the Body screen. The consumer that benefits most never saw it, so
everyone was scored against the cohort's mean fitness. Threaded through all
four paths via one shared `vo2maxFor` helper. No usable resting HR means no
fitness term and the original model, never a fabricated fitness level. No
crossday dependency: Uth VO2max is a function of age and resting HR, so each
site resolves it locally and day derivation stays deterministic.

Requires OpenStrap/analytics#42 (Calories gains an optional vo2max) merged and
the pinned SHA bumped; this does not compile against the current pin.

Tests: three new files, expected values hand-computed from the published
equations so they fail if the paths ever agree on a wrong number. The parity
test calls the real re-score rather than a stand-in — a stand-in kept passing
the old model when the fitness term went in, which is exactly how these drifted.
workout_calorie_anchors_test loses one case that is no longer reachable: with a
resting floor a costed session is always positive.
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@abdulsaheel, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 85882f0f-7f18-45dc-ac2f-b5658623fb4e

📥 Commits

Reviewing files that changed from the base of the PR and between 0ac4391 and f455dbf.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • lib/compute/derivation_engine.dart
  • lib/state/app_state.dart
  • pubspec.yaml
📝 Walkthrough

Walkthrough

The PR centralizes daily and live calorie calculation, adds shared sex normalization, updates day-activity propagation, preserves unavailable Live Activity metrics, and adds tests for energy consistency, live rescoring, profile anchors, sampling, and gap handling.

Changes

Calorie scoring

Layer / File(s) Summary
Shared energy and profile normalization
lib/compute/profile.dart, lib/compute/derivation_engine.dart, lib/compute/manual_session.dart, lib/compute/onehz_pipeline.dart, lib/compute/crossday_pipeline.dart
Adds workoutSex and uses it across analytics paths. Adds wakeDayEnergy with height and profile-anchor validation.
Unified day activity derivation
lib/compute/derivation_engine.dart
Adds applyDayActivity, unifies wake energy propagation, includes basal energy in TDEE, and removes duplicate movement energy calculations.
Live workout rescoring and parity
lib/state/app_state.dart
Recomputes calories from retained HR durations with activity gating, resting-rate handling, elapsed intervals, and capped gaps.
Nullable live activity metrics
lib/live/live_activity.dart, ios/LiveActivityBridge.swift, ios/OpenStrapWidget/OpenStrapWidgetLiveActivity.swift
Preserves unavailable strain and calorie values as null and renders them as .
Calorie consistency and fallback validation
test/daily_energy_consistency_test.dart, test/live_rescore_calorie_parity_test.dart, test/workout_calorie_anchors_test.dart
Adds coverage for daily energy invariants, live parity, missing anchors, height validation, sampling intervals, sleep exclusion, and contact-loss gaps.

Estimated code review effort: 5 (Critical) | ~90+ minutes

Sequence Diagram(s)

sequenceDiagram
  participant DayBlocks
  participant applyDayActivity
  participant wakeDayEnergy
  participant TDEE
  DayBlocks->>applyDayActivity: day activity inputs
  applyDayActivity->>wakeDayEnergy: wake HR and covered minutes
  wakeDayEnergy-->>applyDayActivity: active, basal, and total energy
  applyDayActivity->>TDEE: unified energy bundle
Loading
sequenceDiagram
  participant HeartRateStream
  participant LiveWorkoutState
  participant computeManualSessionStats
  HeartRateStream->>LiveWorkoutState: accepted HR sample
  LiveWorkoutState->>LiveWorkoutState: retain duration and invoke _scoreCalories
  LiveWorkoutState->>computeManualSessionStats: shared calorie inputs
  computeManualSessionStats-->>LiveWorkoutState: recalculated calorie estimate
Loading

Possibly related PRs

  • OpenStrap/edge#217: Shares calorie-anchor validation, nullable calorie handling, and changes across the calorie computation paths.
  • OpenStrap/edge#189: Shares workout calorie and strain changes in manual_session.dart and app_state.dart.
  • OpenStrap/edge#182: Shares movement, step, and day-activity derivation changes in derivation_engine.dart.

Suggested labels: Review effort 5/5

Suggested reviewers: abdulsaheel, dannymcc, localhoop

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the two primary changes: removing basal double-counting and preserving the live calorie gauge during re-scoring.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@svssathvik7

Copy link
Copy Markdown
Contributor Author

⚠️ CI is red on purpose — blocked on OpenStrap/analytics#42

The test check fails at the Analyze step with four errors, all the same one:

error • The named parameter 'vo2max' isn't defined • lib/compute/derivation_engine.dart:3439:7
error • The named parameter 'vo2max' isn't defined • lib/compute/manual_session.dart:359:7
error • The named parameter 'vo2max' isn't defined • lib/compute/onehz_pipeline.dart:531:9
error • The named parameter 'vo2max' isn't defined • lib/state/app_state.dart:4719:15

These are the four call sites Issue 3 wires up. Calories.dailyEnergy and
Calories.estimateBoutCalories only gain the optional vo2max parameter in
OpenStrap/analytics#42, and pubspec.yaml still pins openstrap_analytics
to a SHA from before it. So CI resolves the old package and the analyze step
cannot see the parameter.

This is not a defect in this PR — it is the dependency called out under
"Depends on" in the description. No code change here will turn it green.

Unblock order

  1. Merge name the sample gap cap so both sides of a bout can read it analytics#42.
  2. Bump the openstrap_analytics ref / resolved-ref in edge/pubspec.yaml
    to the resulting commit, and refresh pubspec.lock.
  3. Re-run CI here. Issues 1 and 2 are self-contained and unaffected either way.

Verified locally

Against the analytics branch via a gitignored pubspec_overrides.yaml (the
documented local-dev path in pubspec.yaml):

  • flutter test1781 passed, 2 pre-existing skips
  • flutter analyze lib/ test/ — clean

pubspec.lock in this PR is deliberately unchanged and still points at the
current pin, so the SHA bump lands as one reviewable commit in step 2 rather
than being smuggled in here.

Reviewing before analytics#42 merges

Issues 1 and 2 stand on their own and are worth reviewing now:

  • Issue 1_keytelCaloriesWake deleted; calories and calories_total
    come from one wakeDayEnergy pass, so total − active == basal holds by
    construction.
  • Issue 2 — the live tick recomputes through the same rates and gate as
    estimateBoutCalories, so the gauge survives its own re-score.

Both are covered by tests that do not touch the fitness term
(daily_energy_consistency_test.dart, and everything in
live_rescore_calorie_parity_test.dart except the fitness-model constants).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/compute/derivation_engine.dart`:
- Around line 3855-3870: The duplicate ana.Calories.dailyEnergy computation in
_stepsAndEnergy must be removed so the wakeDayEnergy pass is the sole writer of
both calories and calories_total, preserving VO₂max adjustment and dayMinutes
proration; update lib/compute/derivation_engine.dart lines 3855-3870
accordingly. In test/daily_energy_consistency_test.dart lines 79-86, assert
total minus active equals basal using the scalar pair on the day result after
_computeDayBlocks, rather than only validating the wakeDayEnergy return value.

In `@lib/compute/onehz_pipeline.dart`:
- Around line 494-510: The inline VO₂max anchor in
lib/compute/onehz_pipeline.dart lines 494-510 must match the shared sex
resolution used by vo2maxFor, including both 'f' and 'female' as female; prefer
reusing one anchor definition if practical. Add deriveDayBundle coverage in
test/vo2max_calorie_wiring_test.dart lines 1-23 that verifies
scalars['calories'] with and without a resting HR.

In `@test/daily_energy_consistency_test.dart`:
- Around line 79-86: Extend the daily energy consistency test to exercise the
persisted day-result flow through _computeDayBlocks, then assert that
scalars['calories_total'] minus scalars['calories'] equals the persisted basal
value within tolerance. Retain the existing wakeDayEnergy assertion, but ensure
the new case covers the scalar pair after _stepsAndEnergy completes and
overwrites calories_total.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4a7077a3-e1e4-4d27-9824-98f3c63a5507

📥 Commits

Reviewing files that changed from the base of the PR and between fd34a05 and 702b40f.

📒 Files selected for processing (8)
  • lib/compute/derivation_engine.dart
  • lib/compute/manual_session.dart
  • lib/compute/onehz_pipeline.dart
  • lib/state/app_state.dart
  • test/daily_energy_consistency_test.dart
  • test/live_rescore_calorie_parity_test.dart
  • test/vo2max_calorie_wiring_test.dart
  • test/workout_calorie_anchors_test.dart

Comment thread lib/compute/derivation_engine.dart
Comment thread lib/compute/onehz_pipeline.dart Outdated
Comment thread test/daily_energy_consistency_test.dart
follow-up on the same defect class the previous commit was aimed at. the
calorie work had four things still wrong with it.

1. the day still ran TWO dailyEnergy calls. _applyWakeDayFeatures wrote
calories and calories_total from wakeDayEnergy, then _stepsAndEnergy ran its
own dailyEnergy further down the same day block and overwrote calories_total
with it — gated on profile.isComplete instead of hasCalorieAnchors, over a flat
1440 minutes instead of the covered span, and with no vo2max. so the persisted
pair came from two different estimates and health_export's
BASAL_ENERGY_BURNED = total - active was off by -21/+45/+138 kcal/day at rhr
45/55/80. the duplicate is deleted; the TDEE block is published once, from the
one pass, next to the scalars it belongs to.

2. the day's active calories had quietly moved from the wake series to the
whole-day series, so sleep was billed as exercise. dailyEnergy's flex gate is
0.50*HRmax = 104 - 0.35*age, i.e. 79.5 bpm at 70, which an ordinary sleeping
heart rate clears — 2,640 kcal a night of invented active energy for a 70y at
82 bpm. active is the wake span again, and the basal floor stays pro-rated over
the whole covered day because basal metabolism does not stop overnight. said
plainly in wakeDayEnergy's doc and in the version note, and onehz_pipeline is
marked as the mirror rather than a second definition.

3. the live tick scored per minute-mean. that put the activity gate on the
minute where estimateBoutCalories puts it on the sample, so a sawtooth sitting
at the gate billed 1.19 kcal/min against the re-score's 3.24 — about 123 kcal
over a zone-2 hour. it also billed a completed minute a flat 60 s whatever
backed it, billed the minute in progress its SAMPLE COUNT as seconds, and lost
a contact-loss gap the re-score charges. it now keeps seconds-at-each-bpm and
bills per sample, gap cap included, which is what the re-score does. new parity
cases for each of those; the old ones all used a 1 Hz stream transitioning
exactly on a minute boundary and could not see any of it.

4. the vo2max anchor and the sex normalisation each existed twice with
different mappings. both now live in profile.dart as vo2maxAnchor and
workoutSex, on raw values so the isolate-side pipeline can use them too. that
also fixes TRIMP, which tested sex == 'f' while the calorie path beside it
accepted 'female' — a profile written by the profile screen got female calories
and male strain off one field.

also: the live activity pushed a literal 0 kcal to the lock screen for a
session it correctly refused to score, while the in-app gauge showed "—".
strain and calories are nullable through the channel now and the widget renders
the absence.

kAlgoVersion stays 62; its note is rewritten to describe what the code actually
does now, including the wake-vs-whole-day split and which of these move a
persisted day.

still needs analytics#42 merged and the pin bumped before this compiles.
The contact-loss case uses a 61 s outage, which bills the same whether the
150 s bound is there or not, so it proved the two sides agree about gaps
without proving either of them stops. A 200 s outage separates them: 389
billed seconds against 439 of wall clock.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
lib/compute/derivation_engine.dart (2)

3930-3933: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Correct the series named in this comment.

The comment states the pass runs "over the full-day series". The call at Line 3967 passes perMin, which _perMinuteMeanWake builds from the WAKE span only. The doc block at Lines 3960-3963 states the correct behaviour. The whole point of the v62 note is to stop the wake-versus-whole-day split from drifting, so this sentence contradicts the change it documents.

📝 Proposed comment correction
       // Calories are NOT computed here any more. Active and total both come
-      // from the single `wakeDayEnergy` pass below, over the full-day series —
-      // scoring active separately here, off a different series and without the
-      // basal netting, is exactly how the two figures drifted apart.
+      // from the single `wakeDayEnergy` pass below: active over the WAKE
+      // series, basal pro-rated over the covered day. Scoring active
+      // separately here, off a different series and without the basal
+      // netting, is exactly how the two figures drifted apart.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/compute/derivation_engine.dart` around lines 3930 - 3933, Correct the
comment near the wakeDayEnergy pass to name the actual series passed at the call
site: perMin, representing the wake-span series from _perMinuteMeanWake, rather
than describing it as a full-day series. Keep the explanation about active and
total values sharing the same wakeDayEnergy pass and basal netting intact.

3956-3977: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Derive dayMinutes from record coverage, not motion.length.

_motionMinutes includes only samples with hr > 0, while onehz_pipeline.dart counts every distinct dayTsSec minute in wornMinuteBuckets. Passing motion.length undercounts basal coverage and makes calories_total - calories too low. Use distinct epoch-minute buckets from daySub.tsSec for dayMinutes, while retaining HR-based energy eligibility.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/compute/derivation_engine.dart` around lines 3956 - 3977, Update the
`wakeDayEnergy` call in the derivation flow to compute `dayMinutes` from
distinct epoch-minute buckets derived from `daySub.tsSec`, matching
`wornMinuteBuckets` coverage rather than using `motion.length`. Preserve
`motion` for HR-based energy eligibility and continue passing the resulting
coverage count to `wakeDayEnergy`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/compute/onehz_pipeline.dart`:
- Around line 500-510: Update the calorie computation flow around fitnessAnchor
so both early-read and canonical calorie paths receive the same
resting-HR-derived anchor. Reuse the existing day-series fallback used by
rhrScalar when no sleep session exists, and pass that value consistently to
DerivationEngine’s scMap['rhr'] and the onehz_pipeline prof['resting_hr'] path
instead of reading separate sources.

In `@lib/state/app_state.dart`:
- Around line 4827-4830: Export a shared analytics gap-cap constant from
openstrap_analytics, replacing the unexported 150.0 default literal in
estimateBoutCalories. Update _gapCapS in AppState to reference that exported
constant so live gauge and re-score use the same value.

In `@test/daily_energy_consistency_test.dart`:
- Line 245: Update the assertion for block['value'] in the daily energy
consistency test to allow a 1 kcal tolerance when comparing it with
block['active'] + block['basal']. Keep the existing exact double-precision
invariant covered at the earlier assertion and preserve the current fixture
checks.

---

Outside diff comments:
In `@lib/compute/derivation_engine.dart`:
- Around line 3930-3933: Correct the comment near the wakeDayEnergy pass to name
the actual series passed at the call site: perMin, representing the wake-span
series from _perMinuteMeanWake, rather than describing it as a full-day series.
Keep the explanation about active and total values sharing the same
wakeDayEnergy pass and basal netting intact.
- Around line 3956-3977: Update the `wakeDayEnergy` call in the derivation flow
to compute `dayMinutes` from distinct epoch-minute buckets derived from
`daySub.tsSec`, matching `wornMinuteBuckets` coverage rather than using
`motion.length`. Preserve `motion` for HR-based energy eligibility and continue
passing the resulting coverage count to `wakeDayEnergy`.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dfe8ce8c-536f-4742-ba5b-913683582e04

📥 Commits

Reviewing files that changed from the base of the PR and between 702b40f and a7bb65a.

📒 Files selected for processing (12)
  • ios/LiveActivityBridge.swift
  • ios/OpenStrapWidget/OpenStrapWidgetLiveActivity.swift
  • lib/compute/crossday_pipeline.dart
  • lib/compute/derivation_engine.dart
  • lib/compute/manual_session.dart
  • lib/compute/onehz_pipeline.dart
  • lib/compute/profile.dart
  • lib/live/live_activity.dart
  • lib/state/app_state.dart
  • test/daily_energy_consistency_test.dart
  • test/live_rescore_calorie_parity_test.dart
  • test/vo2max_calorie_wiring_test.dart

Comment thread lib/compute/onehz_pipeline.dart Outdated
Comment thread lib/state/app_state.dart Outdated
Comment thread test/daily_energy_consistency_test.dart Outdated
The two calorie paths were reading different resting heart rates. The pipeline
takes the sleep-gated one and the engine took the day's published rhr scalar,
which with no sleep session is nocturnalRhr over WAKING heart rate. That is not
a resting HR: it reads high, Uth divides by it, and the fitness model then
prices the day as though the user were deconditioned. The pipeline already
refuses that value for the same reason, so the engine does too, and a day
without a night falls back to the published age/mass/sex model rather than
guessing at a fitness level.

The live gauge restated the 150 s gap cap as its own literal. It reads the
analytics constant now, since the whole point of that path is to stop standing
in for a missing sample at the instant the re-score does.

The rounded triple in the day block is asserted within a kcal. The three ints
are rounded independently off one double triple, and round(a + b) is not
round(a) + round(b) once the fractions carry, so exact equality was passing on
the arithmetic of one fixture. The exact form is still asserted on the doubles,
where it actually holds.
The v62 note still described the fitness model as running whenever the day
carries a resting HR, and promised that days without one are unaffected. Both
are now false: the predicate is a SLEEP-DERIVED resting HR, and a no-sleep day
that does carry an rhr scalar moves. Describing a change as the opposite of what
it does is worse than not describing it.

The gate tested the window alone. A window can be non-null while its slice comes
back empty, and the pipeline then falls back to whole-day HR — so a day could
still land on two different Keytel models, one step further in. It tests for
substrate inside the window now, which is what the pipeline means.

The no-sleep test compared a slept day against a no-sleep one, but dropping the
window also stops the wake series excluding the sleep hours, so it grew from
four hours to six and two of those sit above the flex point for that profile.
The number moved for a reason unrelated to the anchor, and the assertion passed
with the gate removed. Both comparisons hold the series fixed now and vary only
the anchor.
…ories

The VO2max anchor is out. Uth over Tanaka collapses to (1285.7 - 4.3268*age)
divided by resting HR, so there is no VO2 in it — it is resting heart rate
wearing a fitness label, and its error is wider than the spread of the thing it
claims to estimate. Feeding it to Keytel's fitness-adjusted model moved a trait
metric by tens of kcal a day on ordinary night-to-night resting-HR noise. The
sleep gate went with it: that gate existed only to decide whether the anchor was
trustworthy, so with no anchor `wakeDayEnergy` has no business taking a resting
HR at all.

The day's calorie triple now requires a real height and goes absent without one.
`dailyEnergy` defines active as the surplus over the Mifflin basal minute, so
the height term sits inside the active figure as well as the total — the 170 cm
stand-in was moving both. On a 35 y / 80 kg male with 600 wake minutes at
130 bpm, 150 cm against 195 cm is active 6500 vs 6383 and total 8068 vs 8232.
Those are persisted to day_result and exported to Apple Health and Health
Connect, so the stand-in was writing a body the user does not have into their
health record — larger than the double-count this change removes. Publishing
active alone for a height-less profile is not available: recovering it would
mean not netting the basal minute out, which is the double-count. The 1 Hz
pipeline's early-read calories gates the same way, so Today does not show a
figure the derived day then withdraws.

What the change is actually for is unchanged. The day's energy came from two
implementations that disagreed: a derivation-local Keytel sum billed the full
rate on every active minute while a second dailyEnergy call, differently gated
and over a flat 1440 minutes, overwrote calories_total on the way out. Both
scalars and the TDEE block now come from one pass, so total - active == basal
holds by construction and the basal the Health export derives by subtraction is
the figure the pass produced. calories_basal is carried for that block alone and
is deliberately not a day_result scalar.

The live gauge still bills a per-bpm seconds histogram through the same
per-sample gate, resting floor and gap cap as the substrate re-score, and
workoutSex is still the one sex normalisation — including the TRIMP constant
that read 'f' alone and scored a profile stored as 'female' as male.

The v62 note said the session paths are scored on read and carry no algo version
of their own. Only the second half is true: stopWorkout writes the live figure
into sessions.calories and the re-score only replaces it when the band handed
over 90% of the window, so a sparse session keeps the live number permanently.
Corrected, along with the rest of the note.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/daily_energy_consistency_test.dart`:
- Around line 350-369: Extend regression coverage for full female profile
values: in test/daily_energy_consistency_test.dart:350-369, add deriveDayBundle
cases comparing workoutSex values 'female' and 'f'. In
lib/compute/onehz_pipeline.dart:506-535 and :1097-1100, assert equivalent female
active-calorie and TRIMP/strain outputs; in
lib/compute/derivation_engine.dart:3942-3952, assert applyDayActivity yields
equivalent female strain. In lib/compute/derivation_engine.dart:4094-4096,
preserve the shared helper as the sole normalization path.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6fc7f927-b6b4-4e0c-b6d5-52317a3a84c8

📥 Commits

Reviewing files that changed from the base of the PR and between a7bb65a and 0ac4391.

⛔ Files ignored due to path filters (1)
  • docs/star-history.svg is excluded by !**/*.svg
📒 Files selected for processing (8)
  • lib/compute/derivation_engine.dart
  • lib/compute/manual_session.dart
  • lib/compute/onehz_pipeline.dart
  • lib/compute/profile.dart
  • lib/state/app_state.dart
  • test/daily_energy_consistency_test.dart
  • test/live_rescore_calorie_parity_test.dart
  • test/workout_calorie_anchors_test.dart
💤 Files with no reviewable changes (2)
  • lib/compute/manual_session.dart
  • lib/compute/profile.dart

Comment on lines +350 to +369
test('scores the day when the profile carries a real height', () {
expect(
caloriesOf(const {
'age': 34,
'sex': 'm',
'weight_kg': 72,
'height_cm': 178,
}),
isNotNull,
);
});

test('abstains without one instead of standing 170 cm in', () {
expect(
caloriesOf(const {'age': 34, 'sex': 'm', 'weight_kg': 72}),
isNull,
reason: 'the active term is a surplus over the Mifflin basal minute, '
'so an imputed height moves it',
);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add regression coverage for full female profile values.

The changed paths now rely on workoutSex, but the new fixtures only use sex: 'm'. Add cases that compare 'female' with 'f' and assert equal female-calorie and female-TRIMP or strain results.

  • test/daily_energy_consistency_test.dart#L350-L369: add a deriveDayBundle case for 'female' and 'f'.
  • lib/compute/onehz_pipeline.dart#L506-L535: verify equivalent active-calorie outputs for both female encodings.
  • lib/compute/onehz_pipeline.dart#L1097-L1100: verify the strain curve uses the female coefficient for both encodings.
  • lib/compute/derivation_engine.dart#L3942-L3952: verify applyDayActivity produces equivalent female strain outputs.
  • lib/compute/derivation_engine.dart#L4094-L4096: keep the shared helper as the only normalization path.

As per coding guidelines, “When adding or changing a capability, cover every call path” and “Behavior changes ... must include regression tests.”

📍 Affects 3 files
  • test/daily_energy_consistency_test.dart#L350-L369 (this comment)
  • lib/compute/onehz_pipeline.dart#L506-L535
  • lib/compute/onehz_pipeline.dart#L1097-L1100
  • lib/compute/derivation_engine.dart#L3942-L3952
  • lib/compute/derivation_engine.dart#L4094-L4096
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/daily_energy_consistency_test.dart` around lines 350 - 369, Extend
regression coverage for full female profile values: in
test/daily_energy_consistency_test.dart:350-369, add deriveDayBundle cases
comparing workoutSex values 'female' and 'f'. In
lib/compute/onehz_pipeline.dart:506-535 and :1097-1100, assert equivalent female
active-calorie and TRIMP/strain outputs; in
lib/compute/derivation_engine.dart:3942-3952, assert applyDayActivity yields
equivalent female strain. In lib/compute/derivation_engine.dart:4094-4096,
preserve the shared helper as the sole normalization path.

Source: Coding guidelines

The live workout scorer reads Calories.defaultMergeGapCapS so it stops standing
in for a missing heart-rate sample at the same instant the re-score of the same
stream does. That constant only exists from this commit onward.
@abdulsaheel
abdulsaheel merged commit ed357f8 into OpenStrap:main Aug 11, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants