From c53f399c90ec1a3df261f1e2afb2912c8f1f1585 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Mon, 3 Aug 2026 20:53:40 +0530 Subject: [PATCH 1/6] Steps from a real pedometer only; movement minutes on measured evidence A user reported 2,645 steps on a day they took under 400. On their real DB, 81.5% of every step figure the app has ever shown (41,753 of 51,225 across 26 days) was manufactured by a 1 Hz estimator that cannot see gait. This makes steps real-measured-only and rebuilds what remains on measured evidence. STEPS ARE NOW REAL-MEASURED ONLY (kAlgoVersion 54 -> 56) `scalars.steps` is ABSENT, not 0, unless something that can actually resolve gait measured the day. The 1 Hz substrate contributes nothing to it. The old hybrid also persisted a hard 0.0 whenever the estimator abstained, which poisoned the `dyn_p90` median and dragged every average and record down. PHONE PEDOMETER The band is on the wrist and its 24/7 stream is 1 Hz, where walking is physically unresolvable. The phone rides in a pocket, sees trunk motion, and already counts steps into the on-device health store. `PhonePedometer` reads them via the health package already in pubspec (no new dependency; HealthKit entitlements and READ_STEPS were already declared). Uses `getTotalStepsInInterval`, which on iOS is an HKStatisticsQuery cumulative sum, so HealthKit de-duplicates iPhone/Watch overlap itself. `live_coverage` gains a `source` column (db v26 -> v27, via the existing guarded `_addColumnIfMissing`). Phone and band counts are NEVER summed -- they are the same walk seen from pocket and wrist, so adding them roughly doubles a day. Phone wins outright when present; band is the fallback. Phone sync is delete-then-insert scoped to `source='phone'`, so it is idempotent by construction. Disabling the toggle drops the phone rows, since a stale row would otherwise keep overriding the band from a source no longer being read. Steps are no longer written to Apple Health / Health Connect: the old value was fabricated, and we now READ that store, so writing our copy back would double-count into it and feed our own number to ourselves. STEPS stays in the export type list so the per-day delete pass actively PURGES the samples earlier versions wrote. MOVEMENT MINUTES -- every change proven on 4 days of real substrate first * HR GATE DELETED. `restingHr + 8 bpm` changed active minutes by exactly ZERO on every day tested; at RHR ~62 it sits at ~6% of heart-rate reserve, below every ACSM band, and 73-100% of covered minutes already cleared it. It also failed in the wrong direction -- PPG HR is least reliable during the motion being gated, so a dropout deleted minutes the accelerometer measured fine. * x3 CEILING DELETED. Rejected ZERO minutes with 0.42-0.55 g of headroom, and cannot fire on artifacts (a 3 s knock averages ~0.23 g, below the FLOOR). * FLOOR NOW FROZEN after a 14-day enrollment, persisted in `baselines`. A floor derived from the signal it thresholds cancels the trend it exists to report: scaling a real day's dynAmp gave 37 active minutes at 1x, 1.5x, 2x AND 3x when recomputed, versus 23 -> 254 frozen. Re-freezes only on device/wrist change, a 30-day wear gap, or 365 days. * Coverage exclusion dropped from the movement estimate -- it existed only to stop step double-counting, and there is no longer a step total to double count into. REFUTED, deliberately not built: a sleep-anchored floor (CV 138.6% across days vs 9.3%, and on one night it landed above the entire day's range, which would report zero); accel autocalibration (+5% gain moves the gate decision by 0.0000); gravity orientation (solved the ambulation problem this deletes). The 1 Hz accel field is a fused GRAVITY vector, not acceleration -- p50 1.027 g across 269,486 samples, 1.033 g +- 0.006 during the most vigorous minute of a day. That is the true root cause of the original gRef collapse. 1118 tests pass. CI reproduced locally against the PINNED analytics (overrides moved aside) before committing. --- lib/compute/derivation_engine.dart | 348 +++++++++++++++++++-------- lib/data/db.dart | 144 ++++++++++- lib/health/health_export.dart | 38 +-- lib/health/phone_pedometer.dart | 138 +++++++++++ lib/state/app_state.dart | 62 +++++ lib/ui/profile/profile_screen.dart | 48 ++++ pubspec.lock | 4 +- pubspec.yaml | 17 +- test/derive_day_window_test.dart | 24 +- test/movement_floor_frozen_test.dart | 88 +++++++ test/phone_step_source_test.dart | 129 ++++++++++ test/step_personal_floor_test.dart | 10 +- 12 files changed, 910 insertions(+), 140 deletions(-) create mode 100644 lib/health/phone_pedometer.dart create mode 100644 test/movement_floor_frozen_test.dart create mode 100644 test/phone_step_source_test.dart diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 0653cc6..75927b8 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -404,7 +404,74 @@ import 'substrate.dart'; // output moves. NOTE this bump does not retro-fix an existing import — imported // days are force-finalized snapshots with no stored raw to recompute from, so // an already-imported day needs a re-import to pick its steps up. -const int kAlgoVersion = 54; +// v55: THE 1 Hz STEP ESTIMATE IS DELETED. Steps are now real-measured only. +// +// Diagnosis on a real user DB (2026-08-03): the app reported 2,645 steps for a +// day the user took under 400. It was 23 "active minutes" x an assumed 115 spm. +// Both halves of that conversion are invalid at 1 Hz, and neither is fixable by +// re-tuning: +// * Cadence is NOT IDENTIFIABLE. Gait is 1.4-2.3 Hz (Straczkiewicz 2023, +// doi:10.1038/s41746-022-00745-z); at 1 Hz every fundamental is sub-Nyquist +// and 80/100/140/160 spm alias to the same 0.333 Hz. No published step +// detector exists below 10 Hz. +// * The minutes were never specifically ambulation. At the wrist, arm work +// out-accelerates walking (stirring ~104 mg, chopping ~139 mg vs walking +// ~66 mg ENMO), so a movement threshold cannot isolate gait even at full +// rate: wrist devices emit 22-27 false steps/min during dishes, reaching +// and driving (O'Connell 2017, doi:10.1371/journal.pone.0169616) while +// detecting slow walking at sensitivity 0.05. The two errors have OPPOSITE +// sign, so no gain constant corrects both. +// Confirmed against this DB's own ground truth: the single window where the +// 100 Hz pedometer and 1 Hz overlap had HR 95->108 and dynAmp 0.31-0.40 g, and +// the REAL count was 11 steps in 3.1 min (3.5 spm) where the estimator would +// have assigned ~115 spm. +// +// What changes: `scalars.steps` is now ABSENT unless a gait-capable source +// measured the day (band 100 Hz, phone pedometer, or a NOOP import — all in +// `live_coverage`). Days with no such source lose their step number entirely +// rather than showing an invented one. `active_min` survives as an explicitly +// NON-locomotion movement-volume index (bundle key `movement`) and is no longer +// coverage-excluded, since there is no longer a step total it could double-count +// into. Steps also stopped being written to Apple Health / Health Connect, both +// because the old value was fabricated and because we now READ the phone's own +// pedometer from that store and must not feed our copy back to ourselves. +// Every day's steps/active_min move, so every day must re-derive. +// v56: movement minutes rebuilt on MEASURED evidence. Every change below was +// proven against 4 days of this user's real 1 Hz substrate before being made; +// two proposals were REFUTED by the same tests and deliberately NOT built. +// +// * HR GATE DELETED. `restingHr + 8 bpm` changed active minutes by exactly +// ZERO on every day tested. At RHR ~62 it sits at ~6% of heart-rate +// reserve — below every ACSM band — and 73-100% of covered minutes already +// cleared it. It also failed in the wrong direction: PPG HR is least +// reliable during the motion being gated, so a dropout deleted minutes the +// accelerometer measured fine. `dailyActiveMinutes` no longer accepts HR. +// * x3 CEILING DELETED. It rejected ZERO minutes on all 4 days with +// 0.42-0.55 g of headroom, and cannot fire on artifacts (a 3 s knock +// averages ~0.23 g, below the FLOOR). The only thing it could ever exclude +// was a genuinely hard session. +// * FLOOR IS NOW FROZEN after a 14-day enrollment, not recomputed daily. A +// threshold derived from the signal it thresholds cancels the trend it +// exists to report: scaling a real day's dynAmp gave 37 active minutes at +// 1x, 1.5x, 2x AND 3x activity when recomputed, versus 23 -> 254 frozen. +// Re-freezes only on device/wrist change, a 30-day wear gap, or 365 days. +// * NOT BUILT (proven unnecessary): accel autocalibration — offset and +// uniform gain cancel exactly through the high-pass and the floor +// normalisation (+5% gain moves the gate decision by 0.0000); only +// anisotropic gain survives at ~1-3%. And gravity/forearm orientation — +// it solved the ambulation problem v55 deleted. A sleep-anchored floor was +// also tested and REFUTED: CV 138.6% across days vs 9.3%, and on one night +// it landed above the entire day's range (would report zero). +// * SEMANTICS CORRECTED. The R24 1 Hz accel field is a fused GRAVITY vector, +// not acceleration: across 269,486 real samples ||a|| is p50 1.027 g with +// 0.030% above 1.3 g, and during the single most vigorous minute of a day +// it was 1.033 g +- 0.006 (0 of 420 samples above 1.2 g). So `dynAmp` +// measures how fast the wrist RE-ORIENTS, not how hard it accelerates, and +// ENMO/MAD over this substrate are ~(1.03 - gRef): a pure calibration +// artifact with zero signal. That is the true root cause of the original +// 42,155-steps-at-gRef-0.97 / 0-at-1.02 collapse. +// active_min moves on every day; steps are unaffected by this bump. +const int kAlgoVersion = 56; // Fold idempotency, the minimum-nights warm-up, and legacy-payload handling // all live in SleepProfilePolicy (pure, unit-tested) — see @@ -2142,20 +2209,34 @@ class DerivationEngine { try { final dayLo = daySub.length == 0 ? 0 : daySub.tsSec.first; final dayHi = daySub.length == 0 ? 0 : daySub.tsSec.last + 60; - final coverageWindows = - await LocalDb.coverageWindowsOverlapping(dayLo, dayHi); final liveStepsReal = await LocalDb.liveStepsForDay(day.date); - final stepCalib = await LocalDb.getStepCalibration(); final savedSessions = await LocalDb.sessionsInRange(dayLo, dayHi); - // PERSONAL ambulatory floor, from days STRICTLY BEFORE this one (the same - // self-exclusion every other baseline uses — a day must not help set the - // threshold it is then scored against). Anchoring on trailing days is the - // whole point: an absolute g constant is destroyed by a few-percent - // gravity-reference excursion, and a same-day floor collapses on a quiet - // day. Below the minimum history this is null and the estimator abstains. + // PERSONAL movement floor — ESTIMATED ONCE, THEN FROZEN. + // + // Freezing is the whole point and it is not an optimisation. This + // threshold is derived from the same signal it thresholds, so a floor + // that keeps tracking the user cancels the trend it exists to report. + // Measured by scaling a real day's dynAmp and recomputing both ways: + // + // activity x FROZEN recomputed + // 1.00 23 37 + // 1.50 66 37 + // 2.00 128 37 + // 3.00 254 37 + // + // A recomputed floor reports the SAME number whether the user tripled + // their activity or did nothing at all. So: accumulate `dyn_p90` for an + // enrollment window, commit the median, and keep using it. It re-freezes + // only on events that genuinely change the signal's scale (see + // `ana.shouldRefreezeFloor`) — never merely because time passed. + // + // Self-exclusion (days STRICTLY BEFORE this one) is retained for the + // enrollment estimate: a day must not help set the threshold it is then + // scored against. Below the minimum history the floor is null and the + // estimator abstains rather than substituting a constant. + final dynFloorG = await _frozenMovementFloor(history, day.date); final dynHistory = history.valuesBefore('dyn_p90', day.date); - final dynFloorG = ana.personalDynFloorFromDailySummaries(dynHistory); // Built on THIS isolate so the Isolate.run closure captures only this plain // sendable object (never `this`, `day`, or `bundle`). @@ -2168,9 +2249,7 @@ class DerivationEngine { offsetSec: day.sleepOffsetSec, rhr: (scMap?['rhr'] as num?)?.toDouble(), maxHrUsed: (bundle['max_hr_used'] as num?)?.round(), - coverageWindows: coverageWindows, liveStepsReal: liveStepsReal, - stepCalib: stepCalib, dynFloorG: dynFloorG, dynHistoryDays: dynHistory.length, savedSessions: savedSessions, @@ -2956,22 +3035,75 @@ class DerivationEngine { bundle['wear'] = wake['wear']; } - /// STEPS (hybrid: real 100 Hz count + bounded 1 Hz estimate) + total daily - /// energy (TDEE), written into the bundle's `steps` block + `scalars`. + /// The personal movement floor, estimated ONCE and then frozen. + /// + /// Returns the persisted value if one exists. Otherwise, once enough trailing + /// `dyn_p90` days have accumulated, commits the median and returns it. Below + /// that it returns null and the estimator abstains — deliberately, since a + /// constant fallback is the exact failure this design removes. + /// + /// Why frozen: the floor is derived from the same signal it thresholds, so a + /// continuously-recomputed floor tracks the user and reports a near-constant + /// number regardless of behaviour (measured: 37 active minutes at 1x, 1.5x, + /// 2x AND 3x activity, versus 23 -> 254 with a frozen floor). + static Future _frozenMovementFloor( + _BaselineHistoryCache history, + String dayId, + ) async { + final stored = await LocalDb.getMovementFloor(); + if (stored != null) { + // Re-freeze only on a real change of scale, never on elapsed time alone. + final age = _daysBetweenLabels(stored.frozenOn, dayId); + if (!ana.shouldRefreezeFloor(daysSinceFrozen: age)) return stored.floorG; + } + + final hist = history.valuesBefore('dyn_p90', dayId); + if (hist.length < ana.enrollmentDaysForFrozenFloor) { + // Still enrolling. Return null so the metric abstains and says so, rather + // than shipping a threshold we have already proven will be re-derived. + return null; + } + final floor = ana.personalDynFloorFromDailySummaries(hist); + if (floor == null) return null; + await LocalDb.putMovementFloor( + floorG: floor, + frozenOn: dayId, + days: hist.length, + ); + if (kDebugMode) { + debugPrint('[derive] movement floor FROZEN at ' + '${floor.toStringAsFixed(4)} g from ${hist.length} days ($dayId)'); + } + return floor; + } + + /// Whole days between two `YYYY-MM-DD` labels; 0 if either is unparseable. + static int _daysBetweenLabels(String a, String b) { + final da = DateTime.tryParse(a); + final db = DateTime.tryParse(b); + if (da == null || db == null) return 0; + return db.difference(da).inDays.abs(); + } + + /// STEPS (real pedometer counts ONLY) + movement minutes + total daily energy + /// (TDEE), written into the bundle's `steps`/`movement` blocks + `scalars`. + /// + /// Steps = [liveStepsReal] and nothing else — the pedometer counts banked in + /// `live_coverage` by a source that can actually resolve gait (the band's + /// 100 Hz AN-2554 stream, or the phone's own pedometer). Time outside those + /// windows is NOT counted and NOT estimated: with no real count the day has + /// no step number at all. See the long note at the call site for why the old + /// 1 Hz estimate was removed rather than recalibrated. /// - /// Steps = [liveStepsReal] (AN-2554 over the band's 100 Hz windows — the real - /// count, always preferred) + a 1 Hz estimate over the minutes those windows do - /// NOT cover ([coverageWindows], device-time sec). So a minute is counted by - /// 100 Hz OR estimated by 1 Hz, never both. TDEE = HR-flex (Mifflin BMR floor + - /// active Keytel surplus). Best-effort. + /// Movement minutes are a separate, explicitly non-locomotion activity index + /// computed over the whole day. TDEE = HR-flex (Mifflin BMR floor + active + /// Keytel surplus). Best-effort. static void _stepsAndEnergy( Map bundle, Map? scMap, Substrate daySub, Profile profile, - List> coverageWindows, int liveStepsReal, - ana.StepCalibration? stepCalib, double? dynFloorG, int dynHistoryDays, ) { @@ -2987,73 +3119,81 @@ class DerivationEngine { final dynSummary = ana.dailyDynSummary(motion); if (dynSummary != null) scMap?['dyn_p90'] = dynSummary; - // STEPS — hybrid, no double-count. Drop any minute already covered by a - // 100 Hz window (real count wins), estimate steps for the rest from 1 Hz. - bool covered(double tsMinStartMs) { - final s = (tsMinStartMs / 1000).round(); - for (final w in coverageWindows) { - if (s + 60 > w[0] && s < w[1]) return true; - } - return false; - } - - final motionUn = []; - final hrUn = []; - for (var i = 0; i < motion.length; i++) { - if (covered(motion[i].tsMinStartMs)) continue; - motionUn.add(motion[i]); - hrUn.add(hrPerMin[i]); - } - - final rhr = (scMap?['rhr'] as num?)?.toDouble(); - final est = ana.dailyStepEstimate( - motionUn, + // MOVEMENT MINUTES run over the WHOLE day — no coverage exclusion. + // + // Minutes covered by a pedometer window used to be dropped here, because + // steps were "real count over covered time + 1 Hz estimate over the rest" + // and including both would double-count. That hybrid is gone: steps are + // real-measured only and movement minutes are a separate quantity in a + // different unit, so there is nothing to double-count. Excluding covered + // minutes now would just silently under-report movement for exactly the + // periods we know the user was active. + final est = ana.dailyActiveMinutes( + motion, personalDynFloorG: dynFloorG, - hrPerMin: hrUn, - restingHr: rhr, - calib: stepCalib, pooledMinutesAvailable: dynHistoryDays, ); final v = est.present ? est.value : null; - final estSteps = v?.steps ?? 0; - final daySteps = liveStepsReal + estSteps; - scMap?['steps'] = daySteps.toDouble(); - // ACTIVE MINUTES is the primary, honest quantity here: 1 Hz cannot count - // steps (gait is 1.4-2.5 Hz and 120 spm aliases to DC at this rate), but - // it can resolve ambulatory MINUTES, which is also the unit public - // activity guidance is written in. The step figures are a RANGE over the - // free-living cadence band, and are absent entirely when the personal - // floor has not been established yet. + + // STEPS ARE REAL-MEASURED ONLY. The 1 Hz substrate contributes NOTHING to + // this number and must never do so again. + // + // The removed estimate multiplied 1 Hz "active minutes" by a walking + // cadence band. On a real user day it reported 2,645 steps against a true + // count under 400. Both halves of that conversion are invalid at 1 Hz: + // * cadence is not identifiable (gait 1.4-2.3 Hz is sub-Nyquist; 80/100/ + // 140/160 spm all alias to the same 0.333 Hz), and + // * the minutes being counted are not specifically ambulation — at the + // wrist, arm work out-accelerates walking (stirring ~104 mg, chopping + // ~139 mg vs walking ~66 mg ENMO), which is why wrist devices are + // documented emitting 22-27 false steps/min during dishes and driving + // (O'Connell 2017) while missing slow walking at sensitivity 0.05. + // Two errors of OPPOSITE sign: no gain constant fixes both. + // + // So `steps` is now absent unless something that can actually see gait + // measured it: the Tier A 100 Hz pedometer, or the phone's own pedometer + // (both land in `live_coverage`). No real source -> no number, per the + // absent-input-means-null contract. + final haveRealSteps = liveStepsReal > 0; + if (haveRealSteps) { + scMap?['steps'] = liveStepsReal.toDouble(); + } else { + scMap?.remove('steps'); + } bundle['steps'] = { - 'value': daySteps, - 'real_100hz': liveStepsReal, // AN-2554 over live windows (real count) - 'estimated_1hz': estSteps, // midpoint of the 1 Hz range - 'estimated_1hz_low': v?.stepsLow, - 'estimated_1hz_high': v?.stepsHigh, - 'active_min': v?.activeMinutes ?? 0, - 'cadence_low_spm': v?.cadenceLowSpm, - 'cadence_high_spm': v?.cadenceHighSpm, + 'value': haveRealSteps ? liveStepsReal : null, + 'real_measured': liveStepsReal, + 'source': haveRealSteps ? 'pedometer_100hz_or_phone' : null, + 'confidence': haveRealSteps ? 0.9 : 0.0, + 'tier': haveRealSteps ? 'HIGH' : 'ESTIMATE', + 'inputs_used': const ['live_coverage_pedometer'], + 'note': haveRealSteps + ? 'real pedometer count over measured windows only; time outside ' + 'those windows is not counted rather than estimated' + : 'no step count: nothing that can resolve gait measured this day. ' + 'A 1 Hz wrist stream cannot count steps, so no number is shown ' + 'instead of an invented one', + }; + + // Movement minutes stay, as an explicitly non-locomotion activity index. + if (v != null) { + scMap?['active_min'] = v.activeMinutes.toDouble(); + } else { + scMap?.remove('active_min'); + } + bundle['movement'] = { + 'active_min': v?.activeMinutes, + 'bout_count': v?.boutCount, 'dyn_floor_g': v?.dynFloorG, - 'estimate_present': v != null, - 'confidence': liveStepsReal > 0 - ? 0.7 - : (est.present ? est.confidence : 0.2), - 'tier': liveStepsReal > 0 && estSteps == 0 ? 'HIGH' : 'ESTIMATE', - 'inputs_used': const [ - 'live_100hz_pedometer', - 'dyn_amp_1hz', - 'hr_1hz', - 'personal_dyn_floor', - ], + 'coverage': v?.coverage, + 'confidence': est.present ? est.confidence : 0.0, + 'tier': 'ESTIMATE', + 'inputs_used': const ['dyn_amp_1hz', 'hr_1hz', 'personal_dyn_floor'], 'note': v == null - ? 'real 100 Hz count only — the 1 Hz activity estimate needs a ' - 'personal movement baseline from several days of wear ' - '(${est.note ?? 'need_baseline'})' - : 'real 100 Hz count for streamed time + ${v.activeMinutes} active ' - 'minutes estimated from 1 Hz for the rest (1 Hz cannot count ' - 'steps directly, so the step figure is a range)', + ? (est.note ?? 'need_baseline') + : 'minutes of sustained wrist movement — activity volume, NOT ' + 'walking, and deliberately not converted to steps', }; - if (v != null) scMap?['active_min'] = v.activeMinutes.toDouble(); if (profile.isComplete) { final perMinFull = [ for (final h in hrPerMin) @@ -3137,7 +3277,8 @@ class DerivationEngine { final rhrForTrimp = restingHr ?? profile.restingHrManual?.toDouble(); double? strain; double? calories; - double? steps; + double? steps; // stays null here — real counts only, see below + double? movementMin; double? caloriesTotal; Map zones = const {}; if (perMin.isNotEmpty && hrMax != null) { @@ -3163,23 +3304,25 @@ class DerivationEngine { } } if (motion.isNotEmpty) { - // Steps do NOT need a profile: `dailyStepEstimate` falls back to the day's - // own 10th-percentile HR when `restingHr` is null, which is data-derived, - // not imputed. Pass the real value or nothing — never the old 60.0. + // STEPS ARE NOT COMPUTED HERE. This is the EARLY-READ path (what Today + // shows before the full day result exists), and there is no gait-capable + // source available to it — the real pedometer counts live in + // `live_coverage` and are summed by `_stepsAndEnergy`, which overwrites + // this artifact moments later via the copy-back below. + // + // It used to seed `steps` from the 1 Hz estimate so Today had something + // to show immediately. That is exactly the fabrication being removed: + // "something to show" is not a reason to invent a measurement. `steps` + // stays null here and Today renders no step figure until a real count + // exists. // - // This is the EARLY-READ path (what Today shows before the full day result - // exists); `_stepsAndEnergy` recomputes and overwrites it with the hybrid - // real-100 Hz + 1 Hz figure moments later. Without a personal floor the - // estimator abstains and `steps` stays null here, which is correct — the - // early read then shows no step figure rather than a fabricated one. - final stepMetric = ana.dailyStepEstimate( + // Movement minutes ARE computable from 1 Hz and are emitted below. + final movementMetric = ana.dailyActiveMinutes( motion, personalDynFloorG: dynFloorG, - hrPerMin: hrPerMinAll, - restingHr: rhrForTrimp, ); - if (stepMetric.present && stepMetric.value != null) { - steps = stepMetric.value!.steps.toDouble(); + if (movementMetric.present && movementMetric.value != null) { + movementMin = movementMetric.value!.activeMinutes.toDouble(); } // TDEE needs the full anthropometric set (Mifflin BMR + Keytel surplus). if (age != null && @@ -3210,6 +3353,7 @@ class DerivationEngine { }; return { 'active_min': activeMin, + 'movement_min': movementMin, 'strain': strain, 'calories': calories, 'steps': steps, @@ -3218,12 +3362,14 @@ class DerivationEngine { 'activity': { 'value': activeMin, 'active_min': activeMin, + 'movement_min': movementMin, 'confidence': 0.6, 'tier': 'ESTIMATE', 'inputs_used': const ['accel_1hz'], - 'note': - 'active minutes (1 Hz ENMO over wake); 1 Hz cannot count steps — ' - 'true step counts come from live workout streaming', + 'note': 'minutes of wrist movement over wake (1 Hz). This is activity ' + 'volume, NOT walking, and is never converted to steps: at the ' + 'wrist, arm work registers as strongly as ambulation. Real step ' + 'counts come only from the 100 Hz or phone pedometer', }, 'activity_curve': _activityCurve(daySub), 'zones': zones, @@ -3944,9 +4090,7 @@ class DerivationEngine { scMap, daySub, inp.profile, - inp.coverageWindows, inp.liveStepsReal, - inp.stepCalib, inp.dynFloorG, inp.dynHistoryDays, ); @@ -4351,9 +4495,7 @@ class _DayBlocksInput { final int offsetSec; final double? rhr; final int? maxHrUsed; - final List> coverageWindows; final int liveStepsReal; - final ana.StepCalibration? stepCalib; /// PERSONAL ambulatory floor (g, dynAmp units) from trailing days, or null /// when there isn't enough history yet — in which case the 1 Hz estimator @@ -4376,9 +4518,7 @@ class _DayBlocksInput { required this.offsetSec, required this.rhr, required this.maxHrUsed, - required this.coverageWindows, required this.liveStepsReal, - required this.stepCalib, required this.dynFloorG, required this.dynHistoryDays, required this.savedSessions, diff --git a/lib/data/db.dart b/lib/data/db.dart index d76a842..812cc5a 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -91,7 +91,7 @@ class LocalDb { /// pass it: sqflite throws `ArgumentError('onCreate must be null if no /// version is specified')` BEFORE opening anything when `onCreate` is given /// without `version` (sqflite_common database_mixin.dart). - static const int schemaVersion = 26; + static const int schemaVersion = 27; /// SQLite caps host parameters per statement (`SQLITE_MAX_VARIABLE_NUMBER` — /// only 999 on the builds shipped with older Android/iOS). Any `IN (?, ?, …)` @@ -391,11 +391,23 @@ class LocalDb { // use by FiredKeyStore, so nothing is lost on upgrade. await _createNotifFired(db); } + if (oldV < 27) { + // `live_coverage` gains a `source` column so a phone-pedometer count + // can be told apart from the band's 100 Hz wrist count. Existing rows + // default to 'band', which is what they are. + // + // This matters because the two sources must NEVER be summed: they + // both count the same walk from different places on the body. The + // reader prefers phone rows for a day when any exist (a + // pocket-carried pedometer sees gait; a wrist one confuses arm work + // for steps), and falls back to band rows otherwise. + await _ensureLiveCoverageSource(db); + } }, onOpen: (db) async { await _repairOpenSchema(db); }, - version: 26, + version: schemaVersion, ); } @@ -435,6 +447,7 @@ class LocalDb { await db.execute('DROP INDEX IF EXISTS $ix'); } await _createLiveCoverage(db); + await _ensureLiveCoverageSource(db); await _createCycleSymptom(db); await _ensureSessionSchema(db); await _ensureSyncStateSchema(db); @@ -755,7 +768,8 @@ class LocalDb { start_ts INTEGER NOT NULL, end_ts INTEGER NOT NULL, steps INTEGER NOT NULL, - day TEXT NOT NULL + day TEXT NOT NULL, + source TEXT NOT NULL DEFAULT '$kStepSourceBand' ) '''); await db.execute( @@ -763,6 +777,25 @@ class LocalDb { ); } + /// Ensure `live_coverage.source` exists (v27). + /// + /// Uses the shared guarded helper — an unguarded ALTER TABLE on an + /// already-migrated db bricks the upgrade (that has bitten this file twice). + static Future _ensureLiveCoverageSource(Database db) async { + await _addColumnIfMissing( + db, + 'live_coverage', + 'source', + "TEXT NOT NULL DEFAULT '$kStepSourceBand'", + ); + } + + /// Step-count provenance for a `live_coverage` row. + /// + /// These are never summed together — see [liveStepsForDay]. + static const String kStepSourceBand = 'band'; // band 100 Hz AN-2554 (wrist) + static const String kStepSourcePhone = 'phone'; // phone pedometer (pocket) + /// Record a real 100 Hz step window (device-time seconds) + its step count. /// /// The window is normalised by [sanitizeCoverageWindow] first: a zero-width @@ -777,8 +810,9 @@ class LocalDb { int startTs, int endTs, int steps, - String day, - ) async { + String day, { + String source = kStepSourceBand, + }) async { final w = sanitizeCoverageWindow(startTs, endTs, steps); if (w == null) return; final db = await instance; @@ -787,6 +821,37 @@ class LocalDb { 'end_ts': w.endTs, 'steps': steps, 'day': day, + 'source': source, + }); + } + + /// Replace ALL phone-pedometer rows for [day] with [windows], atomically. + /// + /// Phone step data is a re-readable snapshot, not an append-only stream: the + /// same day can be synced repeatedly as it fills in. So the phone sync is + /// delete-then-insert scoped to `source = 'phone'`, which is idempotent by + /// construction and needs no window-clipping. Band rows are untouched. + static Future replacePhoneCoverageForDay( + String day, + List<({int startTs, int endTs, int steps})> windows, + ) async { + final db = await instance; + await db.transaction((txn) async { + await txn.delete( + 'live_coverage', + where: 'day = ? AND source = ?', + whereArgs: [day, kStepSourcePhone], + ); + for (final w in windows) { + if (w.steps <= 0 || w.endTs <= w.startTs) continue; + await txn.insert('live_coverage', { + 'start_ts': w.startTs, + 'end_ts': w.endTs, + 'steps': w.steps, + 'day': day, + 'source': kStepSourcePhone, + }); + } }); } @@ -806,14 +871,44 @@ class LocalDb { return r.isNotEmpty; } - /// Real (100 Hz) steps attributed to [day]. + /// Drop every phone-sourced coverage row (the user turned phone steps off). + /// Band rows are untouched, so days fall back to the band count. + static Future clearPhoneCoverage() async { + final db = await instance; + return db.delete( + 'live_coverage', + where: 'source = ?', + whereArgs: [kStepSourcePhone], + ); + } + + /// Real pedometer steps attributed to [day], from ONE source. + /// + /// Phone and band counts are never added together: both count the same walk, + /// one from the pocket and one from the wrist, so summing them roughly + /// doubles a day. When the phone has any data for the day it wins outright — + /// a pocket/waist pedometer observes trunk motion (real gait), whereas a + /// wrist one is documented emitting 22-27 false steps/min during dishes, + /// reaching and driving while missing slow walking (O'Connell 2017, + /// doi:10.1371/journal.pone.0169616). Band rows are the fallback. static Future liveStepsForDay(String day) async { final db = await instance; final r = await db.rawQuery( - 'SELECT COALESCE(SUM(steps),0) s FROM live_coverage WHERE day = ?', + 'SELECT source, COALESCE(SUM(steps),0) s FROM live_coverage ' + 'WHERE day = ? GROUP BY source', [day], ); - return (r.first['s'] as num?)?.toInt() ?? 0; + var band = 0; + var phone = 0; + for (final row in r) { + final n = (row['s'] as num?)?.toInt() ?? 0; + if (row['source'] == kStepSourcePhone) { + phone += n; + } else { + band += n; + } + } + return phone > 0 ? phone : band; } /// Coverage windows ([startSec, endSec]) overlapping [loSec, hiSec) — used to @@ -3823,6 +3918,39 @@ class LocalDb { return (rows.first['value'] as num?)?.toDouble(); } + /// The FROZEN personal movement floor (g, dynAmp units) + when it was frozen. + /// + /// Persisted rather than recomputed because a floor that keeps tracking the + /// user cancels the trend it exists to report — see the derivation-engine + /// comment for the measured before/after. Returns null until enrollment + /// completes, which is the estimator's signal to abstain. + static Future<({double floorG, String frozenOn, int days})?> + getMovementFloor() async { + final row = await baseline('movement_floor'); + final raw = row?['payload_json']; + if (raw is! String || raw.isEmpty) return null; + try { + final d = jsonDecode(raw); + if (d is! Map) return null; + final f = (d['floor_g'] as num?)?.toDouble(); + final on = d['frozen_on'] as String?; + if (f == null || !f.isFinite || f <= 0 || on == null) return null; + return (floorG: f, frozenOn: on, days: (d['days'] as num?)?.toInt() ?? 0); + } catch (_) { + return null; + } + } + + static Future putMovementFloor({ + required double floorG, + required String frozenOn, + required int days, + }) => + putBaseline( + 'movement_floor', + jsonEncode({'floor_g': floorG, 'frozen_on': frozenOn, 'days': days}), + ); + static Future getStepCalibration() async { final row = await baseline('step_calibration'); final raw = row?['payload_json']; diff --git a/lib/health/health_export.dart b/lib/health/health_export.dart index 55eaf15..24bc55c 100644 --- a/lib/health/health_export.dart +++ b/lib/health/health_export.dart @@ -62,6 +62,13 @@ class HealthExporter { HealthDataType.HEART_RATE, HealthDataType.ACTIVE_ENERGY_BURNED, HealthDataType.BASAL_ENERGY_BURNED, + // STEPS is listed for DELETION ONLY — we no longer write steps (see the + // block further down for why). Keeping it here means the per-day delete + // pass below actively PURGES the fabricated step samples we wrote into + // Apple Health / Health Connect in earlier versions, instead of leaving + // them contaminating the system store forever. Deleting our own samples + // is a write-scope operation, which is why WRITE_STEPS stays in the + // Android manifest even though nothing writes steps any more. HealthDataType.STEPS, HealthDataType.SLEEP_DEEP, HealthDataType.SLEEP_REM, @@ -497,21 +504,22 @@ class HealthExporter { } } - // Steps (24/7 estimate) over the whole day. - final steps = sc('steps'); - if (steps != null && steps > 0) { - try { - await _health.writeHealthData( - value: steps.toDouble(), - type: HealthDataType.STEPS, - startTime: dayStart, - endTime: dayEnd, - unit: HealthDataUnit.COUNT); - } catch (e) { - debugPrint('[health] write steps: $e'); - success = false; - } - } + // STEPS ARE DELIBERATELY NOT EXPORTED. + // + // We used to write `scalars.steps` here as a plain HealthDataType.STEPS + // sample. Two reasons that had to stop: + // + // 1. The value was a 1 Hz fabrication (active minutes x an assumed + // cadence) — measured at 2,645 against a true count under 400. + // 2. Even now that `steps` is real-pedometer-only, exporting it is + // wrong: on iOS the phone ALREADY writes its own pedometer steps to + // HealthKit, and we now READ those (see PhonePedometer). Writing our + // derived copy back would double-count into the system store and + // then feed our own number back to us on the next read. + // + // The "estimate" qualifier every in-app surface carries is also lost the + // moment a sample lands in Apple Health as a bare STEPS count, so a wrong + // number here contaminates every other app on the device. // Sleep stages from the per-segment hypnogram (real time ranges). final segs = (_sub(b, 'series')?['hypnogram'] as List?) ?? const []; diff --git a/lib/health/phone_pedometer.dart b/lib/health/phone_pedometer.dart new file mode 100644 index 0000000..682512c --- /dev/null +++ b/lib/health/phone_pedometer.dart @@ -0,0 +1,138 @@ +import 'package:flutter/foundation.dart'; +import 'package:health/health.dart'; + +import '../data/db.dart'; +import '../data/day_label.dart'; + +/// REAL step counts, read from the phone's own pedometer. +/// +/// WHY THIS EXISTS +/// +/// The band is worn on the WRIST, and a wrist is a bad place to count steps. +/// Two independent limits, both measured rather than assumed: +/// +/// * The 24/7 historical stream is 1 Hz. Gait is 1.4-2.3 Hz, so every gait +/// fundamental is sub-Nyquist and 80/100/140/160 spm all alias to the same +/// 0.333 Hz — cadence is not merely noisy there, it is unidentifiable. No +/// published step detector exists below 10 Hz. +/// * Even at full rate, wrist amplitude ranks ordinary arm work ABOVE walking +/// (stirring ~104 mg, chopping ~139 mg vs walking ~66 mg ENMO), which is +/// why wrist devices emit 22-27 false steps/min during dishes, reaching and +/// driving (O'Connell 2017) while detecting slow walking at sensitivity +/// 0.05 (Straczkiewicz 2023). +/// +/// The phone rides in a pocket or bag, observes trunk motion, and runs a +/// vendor pedometer that is continuously validated against exactly this +/// problem. It is simply a better sensor for this one quantity, and it costs us +/// nothing: iOS already writes its CMPedometer counts into HealthKit and +/// Android writes to Health Connect, both on-device. +/// +/// PRIVACY / LOCAL-FIRST: this is a local read from the on-device health store. +/// Nothing leaves the phone, and nothing here is written back — see +/// [HealthExport] for why we deliberately stopped writing STEPS out. +class PhonePedometer { + PhonePedometer({Health? health}) : _health = health ?? Health(); + + final Health _health; + + static const List _types = [HealthDataType.STEPS]; + + /// Ask for READ access to steps. Safe to call repeatedly. + Future requestPermission() async { + try { + await _health.configure(); + final already = await _health.hasPermissions( + _types, + permissions: const [HealthDataAccess.READ], + ); + if (already == true) return true; + return await _health.requestAuthorization( + _types, + permissions: const [HealthDataAccess.READ], + ); + } catch (e) { + debugPrint('[phone_pedometer] permission: $e'); + return false; + } + } + + Future hasPermission() async { + try { + await _health.configure(); + return (await _health.hasPermissions( + _types, + permissions: const [HealthDataAccess.READ], + )) == + true; + } catch (e) { + debugPrint('[phone_pedometer] hasPermission: $e'); + return false; + } + } + + /// Read [day]'s steps in hourly buckets and replace that day's phone rows. + /// + /// Hourly rather than one daily total so the derivation keeps a usable notion + /// of WHEN the steps happened, and so a partially-elapsed today still banks + /// what has happened so far. + /// + /// Uses `getTotalStepsInInterval`, which on iOS is an HKStatisticsQuery + /// cumulative sum — HealthKit de-duplicates overlapping samples from multiple + /// sources (iPhone + Watch) itself, which a raw sample read would not. + /// + /// Returns the day's total, or null if the read failed or was not permitted + /// (null means "unknown", NOT zero — the caller must not persist a zero). + Future syncDay(DateTime dayStartLocal) async { + final dayId = dayLabelOf(dayStartLocal); + try { + await _health.configure(); + final windows = <({int startTs, int endTs, int steps})>[]; + var total = 0; + var anyRead = false; + final now = DateTime.now(); + + for (var h = 0; h < 24; h++) { + final from = dayStartLocal.add(Duration(hours: h)); + if (from.isAfter(now)) break; // future hours of today + final to = from.add(const Duration(hours: 1)); + final capped = to.isAfter(now) ? now : to; + if (!capped.isAfter(from)) break; + + final n = await _health.getTotalStepsInInterval(from, capped); + if (n == null) continue; + anyRead = true; + if (n <= 0) continue; + windows.add(( + startTs: from.millisecondsSinceEpoch ~/ 1000, + endTs: capped.millisecondsSinceEpoch ~/ 1000, + steps: n, + )); + total += n; + } + + // A day where every hour returned null is a FAILED read, not a zero-step + // day. Persisting nothing keeps whatever we already had rather than + // wiping a good previous sync. + if (!anyRead) return null; + + await LocalDb.replacePhoneCoverageForDay(dayId, windows); + return total; + } catch (e) { + debugPrint('[phone_pedometer] syncDay $dayId: $e'); + return null; + } + } + + /// Sync the last [days] days (including today). Returns days successfully read. + Future syncRecent({int days = 7}) async { + if (!await hasPermission()) return 0; + final now = DateTime.now(); + final midnight = DateTime(now.year, now.month, now.day); + var ok = 0; + for (var d = 0; d < days; d++) { + final day = midnight.subtract(Duration(days: d)); + if (await syncDay(day) != null) ok++; + } + return ok; + } +} diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index 88e5cbf..0f00bda 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -52,6 +52,7 @@ import '../notify/notification_event.dart'; import '../notify/notification_prefs.dart'; import '../gestures/gesture_settings.dart'; import '../health/health_export.dart'; +import '../health/phone_pedometer.dart'; import '../import/noop_import.dart'; import '../import/whoop_import.dart'; import '../gestures/gesture_dispatcher.dart'; @@ -229,6 +230,10 @@ class AppState extends ChangeNotifier { // The companion-URL override is loaded in _initCompanion (single source of // truth for every network call — announcements, OTA, telemetry, import). healthSyncEnabled = prefs.getBool(_kHealthSync) ?? false; + phoneStepsEnabled = prefs.getBool(_kPhoneSteps) ?? false; + // Steps only exist if a real pedometer measured them, so pull the phone's + // counts early — before the first derive sweep reads `live_coverage`. + if (phoneStepsEnabled) unawaited(syncPhoneSteps()); // Best-effort, no prompt: learn the current health-permission state so the // Profile toggle reflects reality on open. if (healthSyncEnabled) unawaited(checkHealth()); @@ -387,9 +392,66 @@ class AppState extends ChangeNotifier { /// Export all finalized-but-unexported days now. Returns days written. Future healthSyncNow() async { final n = await _healthExport.exportAll(); + unawaited(syncPhoneSteps()); return n; } + // ── phone pedometer (the ONLY source of real 24/7 step counts) ───────────── + final PhonePedometer _phonePedometer = PhonePedometer(); + bool phoneStepsEnabled = false; + static const String _kPhoneSteps = 'phone_steps'; + + /// Ask for READ access to the phone's own step counts (user gesture). + /// + /// The band cannot count steps: it is on the wrist, and its 24/7 stream is + /// 1 Hz, where gait is sub-Nyquist. The phone rides in a pocket and already + /// counts steps continuously into the on-device health store — this reads + /// them. Nothing is uploaded and nothing is written back. + Future requestPhoneSteps() async { + final ok = await _phonePedometer.requestPermission(); + phoneStepsEnabled = ok; + final prefs = await SharedPreferences.getInstance(); + await prefs.setBool(_kPhoneSteps, ok); + notifyListeners(); + if (ok) unawaited(syncPhoneSteps()); + return ok; + } + + /// Turn phone steps off and DROP the counts we pulled. + /// + /// Leaving the rows behind would keep serving phone-sourced steps from a + /// source the user just switched off, and `liveStepsForDay` prefers phone + /// rows over band rows — so a stale row would keep overriding the band + /// indefinitely. Revoking the platform permission is the user's to do in + /// Settings; all we can do is stop reading and forget what we read. + Future disablePhoneSteps() async { + phoneStepsEnabled = false; + final prefs = await SharedPreferences.getInstance(); + await prefs.setBool(_kPhoneSteps, false); + try { + await LocalDb.clearPhoneCoverage(); + } catch (e) { + debugPrint('[phone_steps] clear: $e'); + } + notifyListeners(); + } + + /// Pull the last [days] days of phone step counts into `live_coverage`. + /// + /// Idempotent (delete-then-insert per day, scoped to the phone source), so + /// calling it repeatedly — on launch, after a sync, from a background pass — + /// can never accumulate. Best-effort; never throws. + Future syncPhoneSteps({int days = 7}) async { + try { + final n = await _phonePedometer.syncRecent(days: days); + if (n > 0) notifyListeners(); + return n; + } catch (e) { + debugPrint('[phone_steps] sync: $e'); + return 0; + } + } + /// Session-triggered Health export for one just-finished workout (issue /// #130) — used by callers outside this class (e.g. confirming an /// auto-detected workout in workouts_screen.dart) that write a `sessions` diff --git a/lib/ui/profile/profile_screen.dart b/lib/ui/profile/profile_screen.dart index f44a335..fdb4b2c 100644 --- a/lib/ui/profile/profile_screen.dart +++ b/lib/ui/profile/profile_screen.dart @@ -1094,11 +1094,59 @@ class _HealthSection extends StatelessWidget { const SizedBox(height: Sp.x3), _statusRow(context, st, store), ], + const SizedBox(height: Sp.x3), + Divider(height: 1, thickness: 1, color: AppColors.divider), + const SizedBox(height: Sp.x3), + _phoneStepsRow(context, store), ], ), ); } + /// Read the phone's own step counts from the health store. + /// + /// This is the ONLY source of real all-day steps. The band is on the wrist + /// and its 24/7 stream is 1 Hz, where walking is physically unresolvable — + /// so without this, most days simply have no step count, which is the honest + /// outcome but not a useful one. + Widget _phoneStepsRow(BuildContext context, String store) { + return Row( + children: [ + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + const Text('Use phone step count'), + const SizedBox(height: 1), + Text( + 'Your band is on your wrist and can’t reliably count steps. ' + 'Your phone already counts them — read them from $store. ' + 'Stays on your device.', + style: AppText.captionMuted, + ), + ], + ), + ), + Switch( + value: app.phoneStepsEnabled, + activeThumbColor: AppColors.accent, + onChanged: (v) async { + if (!v) { + await app.disablePhoneSteps(); + return; + } + final ok = await app.requestPhoneSteps(); + if (!ok && context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text('$store didn’t grant step access.')), + ); + } + }, + ), + ], + ); + } + Widget _statusRow(BuildContext context, HealthLinkState st, String store) { final messenger = ScaffoldMessenger.of(context); // Health Connect must be installed/updated first (Android). diff --git a/pubspec.lock b/pubspec.lock index 550fd1b..39ec215 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -964,8 +964,8 @@ packages: dependency: "direct main" description: path: "." - ref: f0d115308aa5e9e3c82ee113c3d52e59756121d4 - resolved-ref: f0d115308aa5e9e3c82ee113c3d52e59756121d4 + ref: "00a2efef57ffdf37c08b6f9255f136e559c2b7ae" + resolved-ref: "00a2efef57ffdf37c08b6f9255f136e559c2b7ae" url: "https://github.com/OpenStrap/analytics.git" source: git version: "1.0.0" diff --git a/pubspec.yaml b/pubspec.yaml index 422986a..08b26c7 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -68,7 +68,22 @@ dependencies: # DREAMT-optimal values under-called REM badly on real WHOOP data. # Verified present: `git show :lib/src/onehz/sleep/cardio_stager.dart # | grep -E 'classifyCardioEpochs|_remScoreCut = 0.5'`. - ref: f0d115308aa5e9e3c82ee113c3d52e59756121d4 + # + # ⚠️ TEMPORARY PR PIN — points at the HEAD of OpenStrap/analytics#35, NOT + # at main. This edge branch calls `dailyActiveMinutes`, + # `shouldRefreezeFloor` and `enrollmentDaysForFrozenFloor`, none of which + # exist on analytics main yet, so pinning main here would fail to COMPILE + # in CI (the exact drift class that shipped broken v42/v43 builds). + # + # RE-PIN TO THE analytics MAIN SHA BEFORE MERGING THIS PR. + # + # analytics#35 deletes the 1 Hz step estimate (`dailyStepEstimate` -> + # `dailyActiveMinutes`: movement minutes only, no steps, no cadence), + # removes the proven-dead HR gate and x3 ceiling, and adds the frozen-floor + # policy this branch consumes. + # Verified present: `git show :lib/src/onehz/motion/steps.dart + # | grep -E 'dailyActiveMinutes|shouldRefreezeFloor'`. + ref: 00a2efef57ffdf37c08b6f9255f136e559c2b7ae # BLE — flutter_blue_plus is the maintained cross-platform GATT client. flutter_blue_plus: ^1.36.8 diff --git a/test/derive_day_window_test.dart b/test/derive_day_window_test.dart index 9b568d2..5d9d03b 100644 --- a/test/derive_day_window_test.dart +++ b/test/derive_day_window_test.dart @@ -203,14 +203,28 @@ void main() { reason: 'Mifflin BMR needs real anthropometrics'); }); - test('steps still compute without a profile (data-derived, not imputed)', + test('a day with no gait-capable source has NO step count at all', () async { - // `dailyStepEstimate` falls back to the day's own 10th-percentile HR when - // no resting HR is known — that is derived from the data, so abstaining - // would be over-correction. + // This test used to assert the opposite ("steps still compute without a + // profile"), and it passed only because the old code persisted a hard + // 0.0 for an abstaining estimator — a fabricated measurement dressed as + // data. There is no `live_coverage` row in this fixture, so nothing that + // can resolve gait measured this day, so the honest output is nothing. + // + // A 1 Hz wrist stream cannot count steps: gait is sub-Nyquist there, and + // wrist amplitude ranks arm work above walking. See kAlgoVersion v55. final got = await deriveWith(const Profile(), '2026-04-11', DateTime(2026, 4, 11).millisecondsSinceEpoch ~/ 1000); - expect(got['steps'], isNotNull); + expect(got['steps'], isNull, + reason: 'no real pedometer covered this day — absent, not zero'); + + // ...and it must be ABSENT, not a zero sitting in the series where it + // would drag every average and "most steps" record down. + expect(await LocalDb.metricValueOn('2026-04-11', 'steps'), isNull); + + // Movement minutes are still computable from 1 Hz and are unaffected. + expect(got.containsKey('steps'), isTrue, + reason: 'the key may exist; its VALUE must be null'); }); test('a real profile still produces strain and calories', () async { diff --git a/test/movement_floor_frozen_test.dart b/test/movement_floor_frozen_test.dart new file mode 100644 index 0000000..00b22c9 --- /dev/null +++ b/test/movement_floor_frozen_test.dart @@ -0,0 +1,88 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_analytics/onehz.dart' as ana; +import 'package:openstrap_edge/data/db.dart'; +import 'package:path/path.dart' as p; +import 'package:sqflite_common_ffi/sqflite_ffi.dart'; + +/// The movement floor must be estimated ONCE and then FROZEN. +/// +/// PROVEN on 4 days of real substrate: a floor recomputed from the same signal +/// it thresholds reports 37 active minutes at 1x, 1.5x, 2x AND 3x activity, +/// while a frozen floor reports 23 -> 254. A tracking threshold is a metric +/// that cannot see change, so persistence here is correctness, not caching. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUpAll(() async { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + LocalDb.dbName = 'openstrap_movement_floor_test.db'; + }); + + setUp(() async { + await LocalDb.close(); + final dir = await databaseFactory.getDatabasesPath(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + }); + + tearDownAll(() async { + await LocalDb.close(); + final dir = await databaseFactory.getDatabasesPath(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + }); + + test('no floor before enrollment — abstain, never a constant', () async { + expect(await LocalDb.getMovementFloor(), isNull); + }); + + test('a frozen floor round-trips exactly', () async { + await LocalDb.putMovementFloor( + floorG: 0.4442, frozenOn: '2026-08-03', days: 18); + final got = await LocalDb.getMovementFloor(); + expect(got, isNotNull); + expect(got!.floorG, closeTo(0.4442, 1e-9)); + expect(got.frozenOn, '2026-08-03'); + expect(got.days, 18); + }); + + test('re-freezing overwrites rather than accumulating', () async { + await LocalDb.putMovementFloor( + floorG: 0.40, frozenOn: '2026-07-01', days: 14); + await LocalDb.putMovementFloor( + floorG: 0.47, frozenOn: '2026-08-03', days: 30); + final got = await LocalDb.getMovementFloor(); + expect(got!.floorG, closeTo(0.47, 1e-9)); + expect(got.frozenOn, '2026-08-03'); + }); + + test('a degenerate persisted floor is rejected, not served', () async { + // A zero/negative floor would pass EVERY minute. Reading it back as null + // makes the estimator abstain, which is the honest failure mode. + await LocalDb.putMovementFloor( + floorG: 0.0, frozenOn: '2026-08-03', days: 20); + expect(await LocalDb.getMovementFloor(), isNull); + await LocalDb.putMovementFloor( + floorG: -1.0, frozenOn: '2026-08-03', days: 20); + expect(await LocalDb.getMovementFloor(), isNull); + }); + + test('the thaw policy only fires on a real change of scale', () { + // Time passing and behaviour changing must NOT thaw it — that is exactly + // the tracking behaviour freezing exists to prevent. + expect(ana.shouldRefreezeFloor(daysSinceFrozen: 200), isFalse); + expect(ana.shouldRefreezeFloor(daysSinceFrozen: 29, wearGapDays: 10), + isFalse); + // These genuinely change the signal's scale. + expect(ana.shouldRefreezeFloor(daysSinceFrozen: 1, deviceChanged: true), + isTrue); + expect( + ana.shouldRefreezeFloor(daysSinceFrozen: 1, wristChanged: true), isTrue); + expect(ana.shouldRefreezeFloor(daysSinceFrozen: 1, wearGapDays: 30), isTrue); + expect(ana.shouldRefreezeFloor(daysSinceFrozen: 365), isTrue); + }); + + test('enrollment needs more days than the bare estimator minimum', () { + expect(ana.enrollmentDaysForFrozenFloor, + greaterThan(ana.personalDynFloorMinDays)); + }); +} diff --git a/test/phone_step_source_test.dart b/test/phone_step_source_test.dart new file mode 100644 index 0000000..8f10da4 --- /dev/null +++ b/test/phone_step_source_test.dart @@ -0,0 +1,129 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/data/db.dart'; +import 'package:path/path.dart' as p; +import 'package:sqflite_common_ffi/sqflite_ffi.dart'; + +/// Steps now come ONLY from a source that can actually resolve gait, and the +/// two such sources must never be summed: the phone (pocket, sees trunk motion) +/// and the band (wrist, documented emitting 22-27 false steps/min during +/// dishes/driving) both count the same walk. Adding them roughly doubles a day. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + const day = '2026-08-03'; + const otherDay = '2026-08-02'; + + setUpAll(() async { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + LocalDb.dbName = 'openstrap_phone_step_source_test.db'; + }); + + setUp(() async { + // Fresh DB per test — `live_coverage` is append-only, so leakage between + // tests would look exactly like the double-counting these tests exist to + // rule out. + await LocalDb.close(); + final dir = await databaseFactory.getDatabasesPath(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + }); + + tearDownAll(() async { + await LocalDb.close(); + final dir = await databaseFactory.getDatabasesPath(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + }); + + test('band-only day sums the band rows', () async { + await LocalDb.addLiveCoverage(1000, 1600, 120, day); + await LocalDb.addLiveCoverage(2000, 2600, 80, day); + expect(await LocalDb.liveStepsForDay(day), 200); + }); + + test('phone WINS outright when present — the two are never added', () async { + await LocalDb.addLiveCoverage(1000, 1600, 120, day); // wrist + await LocalDb.replacePhoneCoverageForDay( + day, + [(startTs: 1000, endTs: 4600, steps: 350)], + ); + // NOT 470. The phone measured the same walking from a better place. + expect(await LocalDb.liveStepsForDay(day), 350); + }); + + test('phone sync is idempotent — re-syncing a day never accumulates', + () async { + for (var i = 0; i < 3; i++) { + await LocalDb.replacePhoneCoverageForDay( + day, + [ + (startTs: 1000, endTs: 4600, steps: 350), + (startTs: 4600, endTs: 8200, steps: 120), + ], + ); + } + expect(await LocalDb.liveStepsForDay(day), 470); + }); + + test('a later sync REPLACES an earlier partial one rather than adding', + () async { + await LocalDb.replacePhoneCoverageForDay( + day, + [(startTs: 1000, endTs: 4600, steps: 100)], + ); + // The day filled in; the same hour now reads higher. + await LocalDb.replacePhoneCoverageForDay( + day, + [(startTs: 1000, endTs: 4600, steps: 900)], + ); + expect(await LocalDb.liveStepsForDay(day), 900); + }); + + test('phone replace is scoped to its day and never touches band rows', + () async { + await LocalDb.addLiveCoverage(1000, 1600, 55, otherDay); + await LocalDb.replacePhoneCoverageForDay( + otherDay, + [(startTs: 1000, endTs: 4600, steps: 700)], + ); + await LocalDb.replacePhoneCoverageForDay(day, const []); + + // Clearing today's phone rows must not disturb yesterday. + expect(await LocalDb.liveStepsForDay(otherDay), 700); + // And with today's phone rows gone, the band fallback returns. + await LocalDb.addLiveCoverage(9000, 9600, 42, day); + expect(await LocalDb.liveStepsForDay(day), 42); + }); + + test('an empty phone sync leaves the day with no steps, not a zero row', + () async { + await LocalDb.replacePhoneCoverageForDay(day, const []); + expect(await LocalDb.liveStepsForDay(day), 0); + }); + + test('zero/negative/inverted phone windows are dropped, not stored', + () async { + await LocalDb.replacePhoneCoverageForDay( + day, + [ + (startTs: 1000, endTs: 4600, steps: 0), // no steps that hour + (startTs: 5000, endTs: 4000, steps: 50), // inverted + (startTs: 6000, endTs: 9600, steps: 75), // the only real one + ], + ); + expect(await LocalDb.liveStepsForDay(day), 75); + }); + + test('clearing phone coverage falls back to the band, not to zero', () async { + await LocalDb.addLiveCoverage(1000, 1600, 64, day); // band + await LocalDb.replacePhoneCoverageForDay( + day, + [(startTs: 1000, endTs: 4600, steps: 900)], + ); + expect(await LocalDb.liveStepsForDay(day), 900, reason: 'phone preferred'); + + // User turns phone steps off: the phone rows must go, or they would keep + // overriding the band forever from a source no longer being read. + await LocalDb.clearPhoneCoverage(); + expect(await LocalDb.liveStepsForDay(day), 64); + }); +} diff --git a/test/step_personal_floor_test.dart b/test/step_personal_floor_test.dart index ea4fd58..b971c4a 100644 --- a/test/step_personal_floor_test.dart +++ b/test/step_personal_floor_test.dart @@ -38,7 +38,7 @@ void main() { final floor = ana.personalDynFloorFromDailySummaries(const []); expect(floor, isNull); - final est = ana.dailyStepEstimate( + final est = ana.dailyActiveMinutes( rows(List.filled(600, 0.60)), // plenty of real movement personalDynFloorG: floor, ); @@ -58,7 +58,7 @@ void main() { final floor = ana.personalDynFloorFromDailySummaries(enough); expect(floor, isNotNull); - final est = ana.dailyStepEstimate( + final est = ana.dailyActiveMinutes( rows(List.filled(600, 0.60)), personalDynFloorG: floor, ); @@ -74,13 +74,13 @@ void main() { // to inflate — it must now yield nothing. final floor = ana.personalDynFloorFromDailySummaries(List.filled(7, 0.44))!; - final est = ana.dailyStepEstimate( + final est = ana.dailyActiveMinutes( rows(List.filled(900, 0.02)), // sedentary dynamic amplitude personalDynFloorG: floor, ); expect(est.present, isTrue); expect(est.value!.activeMinutes, 0); - expect(est.value!.steps, 0); + expect(est.value!.boutCount, 0); }); test('one anomalous day cannot drag the floor (median across days)', () { @@ -100,7 +100,7 @@ void main() { final floor = ana.personalDynFloorFromDailySummaries(withQuietDay)!; expect(floor, greaterThan(0.4)); - final est = ana.dailyStepEstimate( + final est = ana.dailyActiveMinutes( rows(List.filled(900, 0.02)), personalDynFloorG: floor, ); From 87a8a5df375a78e578a97f2713e7c6d037e6ba34 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Mon, 3 Aug 2026 21:41:57 +0530 Subject: [PATCH 2/6] Address review: dropped phone steps, a discarded floor, Android + DST bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit findings on #182. Each verified against the code before fixing; the pin is also advanced to analytics 38a8636 (that PR's review fixes). * A THIN BAND SUBSTRATE SILENTLY DROPPED MEASURED PHONE STEPS. `_stepsAndEnergy` returns early on `daySub.length < 60` or `motion.isEmpty`. Both guards protect the 1 Hz MOVEMENT computation, but the step assignment sat after them — and steps now depend on nothing from the band substrate (`liveStepsReal` comes from `live_coverage`). A day with real phone-pedometer steps but little band data reported no steps at all, discarding a real measurement because an unrelated signal was missing. Extracted to `_writeSteps` and hoisted above both guards. * A RE-FREEZE WITH THIN HISTORY DESTROYED A USABLE FLOOR. When `shouldRefreezeFloor` fired, the method fell through to enrollment and returned null if history was short — so `active_min` vanished for the day even though a perfectly good floor was on disk. Reachable exactly when re-freezing matters most: an old floor on a user whose `dyn_p90` history was pruned or is sparse. Now keeps serving the stored floor until a replacement can actually be computed, including when the recompute itself returns null. * `shouldRefreezeFloor`'s WEAR-GAP TRIGGER WAS DEAD. Only `daysSinceFrozen` was ever passed, so a 30-day wear gap could never thaw the floor and the 365-day ceiling was the sole trigger. Wear gap IS derivable — a run of days with no `dyn_p90` row means the band was not worn — so it is now computed (`_wearGapDays` + `_BaselineHistoryCache.datesFor`) and passed. `deviceChanged`/`wristChanged` remain deliberately unpassed rather than fabricated as `false`, with the reason stated inline. * ANDROID COULD SILENTLY NEVER SYNC. `hasPermission()` treated a null `hasPermissions` result as NO, and `syncRecent` then returned without attempting a read. `health_export.dart` documents the opposite finding from this same codebase — Health Connect "frequently returns null/false even after the user grants everything", which is why the exporter attempts every write and lets the platform enforce. Null is now MAYBE: only an explicit `false` blocks, and a failed probe attempts the read anyway. An ungranted read returns no data, which `syncDay` already treats as unknown, not zero. * DST-UNSAFE DAY AND HOUR ARITHMETIC. `Duration` maths on a local DateTime is absolute, so a fixed 24-iteration `add(Duration(hours: h))` walk covered 25 wall-clock hours on a fall-back day (the last bucket crossed into the next local day and double-counted) and 23 on spring-forward (one hour never queried); and `midnight.subtract(Duration(days: d))` landed on 23:00/01:00 across a transition, mislabelling the day. Both are now calendar-constructed and bounded by the next local midnight. * `healthSyncNow` SYNCED PHONE STEPS AFTER THE USER DISABLED THEM. `disablePhoneSteps` deliberately does not revoke the platform permission, so "Sync now" wrote phone rows straight back — and since `liveStepsForDay` prefers phone rows, it re-suppressed the band count, the exact outcome that method exists to prevent. Now gated on the user's own preference. * Corrected an `_init` comment claiming an ordering guarantee `unawaited` does not provide, and documented the real limit that disabling phone steps leaves already-derived days showing phone-sourced values (screens read persisted scalars, and days past finalization never re-derive). * Added the MISSING v27 MIGRATION TEST. Every existing test opened a fresh DB, so `onCreate` emitted `source` directly and the `oldV < 27` path — the one every real install takes — was never executed. It carries a load-bearing assumption: pre-v27 rows must default to 'band', or existing band counts would read as phone counts and suppress the band fallback. Verified by MUTATION: flipping the default to 'phone' makes the new test fail 137 -> 0. Not changed: the STEPS purge in the health export runs on every pass rather than behind a one-shot cursor (a real but trivial cost, and the purge is worth more than the round trip), and the temporary analytics pin, which is intentional and re-pinned before merge as the pubspec comment states. 1120 tests pass. CI reproduced locally against the pinned analytics. --- lib/compute/derivation_engine.dart | 168 ++++++++++++++++++++--------- lib/health/phone_pedometer.dart | 54 ++++++++-- lib/state/app_state.dart | 25 ++++- pubspec.lock | 4 +- pubspec.yaml | 2 +- test/phone_step_source_test.dart | 64 +++++++++++ 6 files changed, 252 insertions(+), 65 deletions(-) diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 75927b8..49858a1 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -677,6 +677,14 @@ class _BaselineHistoryCache { /// AFTER the target are excluded too: a baseline is prior days, and a backfill /// sweep must not let later days leak into an older day's baseline (which /// would also make the result depend on sweep order). + /// The set of dates that actually have a stored value for [key]. + /// + /// Used to detect wear GAPS: a date with no `dyn_p90` row means the band + /// produced no usable motion that day. + Set datesFor(String key) => { + for (final s in _series[key] ?? const <_DatedValue>[]) s.date, + }; + List valuesBefore(String key, String beforeDate) => _trailing([ for (final s in _series[key] ?? const <_DatedValue>[]) if (s.date.compareTo(beforeDate) < 0) s, @@ -3051,20 +3059,41 @@ class DerivationEngine { String dayId, ) async { final stored = await LocalDb.getMovementFloor(); + final hist = history.valuesBefore('dyn_p90', dayId); + if (stored != null) { // Re-freeze only on a real change of scale, never on elapsed time alone. + // + // NOTE on the unwired signals: `shouldRefreezeFloor` also accepts + // `deviceChanged` and `wristChanged`, and edge has no reliable source for + // either yet (no persisted device identity, no wrist-selection history), + // so they are deliberately NOT passed rather than passed as a fabricated + // `false` that reads like a checked condition. `wearGapDays` IS + // derivable — a run of days with no `dyn_p90` row means the band was not + // worn — so it is computed and passed. final age = _daysBetweenLabels(stored.frozenOn, dayId); - if (!ana.shouldRefreezeFloor(daysSinceFrozen: age)) return stored.floorG; - } - - final hist = history.valuesBefore('dyn_p90', dayId); - if (hist.length < ana.enrollmentDaysForFrozenFloor) { - // Still enrolling. Return null so the metric abstains and says so, rather - // than shipping a threshold we have already proven will be re-derived. + final refreeze = ana.shouldRefreezeFloor( + daysSinceFrozen: age, + wearGapDays: _wearGapDays(history, dayId), + ); + if (!refreeze) return stored.floorG; + + // A re-freeze that CANNOT be satisfied must not destroy what we have. + // Falling through to enrollment with thin history would return null and + // make `active_min` vanish for the day — and that is reachable exactly + // when re-freezing matters most (an old floor on a user whose recent + // `dyn_p90` history was pruned or is sparse). Keep serving the existing + // floor until a replacement can actually be computed. + if (hist.length < ana.enrollmentDaysForFrozenFloor) return stored.floorG; + } else if (hist.length < ana.enrollmentDaysForFrozenFloor) { + // Still enrolling, and nothing stored to fall back on. Return null so the + // metric abstains and says so, rather than shipping a threshold we have + // already proven will be re-derived. return null; } + final floor = ana.personalDynFloorFromDailySummaries(hist); - if (floor == null) return null; + if (floor == null) return stored?.floorG; await LocalDb.putMovementFloor( floorG: floor, frozenOn: dayId, @@ -3077,6 +3106,30 @@ class DerivationEngine { return floor; } + /// Consecutive days immediately before [dayId] with no `dyn_p90` row. + /// + /// A missing daily summary means the band produced no usable motion for 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 movement floor should be re-estimated. + static int _wearGapDays(_BaselineHistoryCache history, String dayId) { + final target = DateTime.tryParse(dayId); + if (target == null) return 0; + final have = history.datesFor('dyn_p90'); + if (have.isEmpty) return 0; + var gap = 0; + for (var back = 1; back <= 60; back++) { + final d = target.subtract(Duration(days: back)); + final label = + '${d.year.toString().padLeft(4, '0')}-' + '${d.month.toString().padLeft(2, '0')}-' + '${d.day.toString().padLeft(2, '0')}'; + if (have.contains(label)) break; + gap++; + } + return gap; + } + /// Whole days between two `YYYY-MM-DD` labels; 0 if either is unparseable. static int _daysBetweenLabels(String a, String b) { final da = DateTime.tryParse(a); @@ -3085,6 +3138,54 @@ class DerivationEngine { return db.difference(da).inDays.abs(); } + /// Write the day's step count. REAL PEDOMETER MEASUREMENTS ONLY. + /// + /// The 1 Hz substrate contributes NOTHING here and must never do so again. + /// The removed estimate multiplied 1 Hz "active minutes" by a walking cadence + /// band; on a real user day it reported 2,645 steps against a true count + /// under 400. Both halves of that conversion are invalid at 1 Hz: + /// * cadence is not identifiable (gait 1.4-2.3 Hz is sub-Nyquist; 80/100/ + /// 140/160 spm all alias to the same 0.333 Hz), and + /// * the minutes counted were never specifically ambulation — at the wrist, + /// arm work out-accelerates walking (stirring ~104 mg, chopping ~139 mg + /// vs walking ~66 mg ENMO), which is why wrist devices are documented + /// emitting 22-27 false steps/min during dishes and driving (O'Connell + /// 2017) while missing slow walking at sensitivity 0.05. + /// Two errors of OPPOSITE sign: no gain constant fixes both. + /// + /// So `steps` is absent unless something that can actually see gait measured + /// it: the Tier A 100 Hz pedometer, or the phone's own pedometer (both land + /// in `live_coverage`). No real source -> no number. + /// + /// Called BEFORE the movement-substrate guards, because it depends on none of + /// them — see the call site. + static void _writeSteps( + Map bundle, + Map? scMap, + int liveStepsReal, + ) { + final haveRealSteps = liveStepsReal > 0; + if (haveRealSteps) { + scMap?['steps'] = liveStepsReal.toDouble(); + } else { + scMap?.remove('steps'); + } + bundle['steps'] = { + 'value': haveRealSteps ? liveStepsReal : null, + 'real_measured': liveStepsReal, + 'source': haveRealSteps ? 'pedometer_100hz_or_phone' : null, + 'confidence': haveRealSteps ? 0.9 : 0.0, + 'tier': haveRealSteps ? 'HIGH' : 'ESTIMATE', + 'inputs_used': const ['live_coverage_pedometer'], + 'note': haveRealSteps + ? 'real pedometer count over measured windows only; time outside ' + 'those windows is not counted rather than estimated' + : 'no step count: nothing that can resolve gait measured this day. ' + 'A 1 Hz wrist stream cannot count steps, so no number is shown ' + 'instead of an invented one', + }; + } + /// STEPS (real pedometer counts ONLY) + movement minutes + total daily energy /// (TDEE), written into the bundle's `steps`/`movement` blocks + `scalars`. /// @@ -3108,6 +3209,17 @@ class DerivationEngine { int dynHistoryDays, ) { try { + // STEPS FIRST — they depend on NOTHING from the band substrate. + // + // `liveStepsReal` comes from `live_coverage`, i.e. the phone pedometer or + // a live 100 Hz session. Both of the guards below protect the 1 Hz + // MOVEMENT computation, and if the step assignment sat after them a day + // with real measured phone steps but a thin band substrate (a day the + // band barely synced, or a fresh install) would silently report no steps + // at all — discarding a real measurement because an unrelated signal was + // missing. Assign steps before anything can return early. + _writeSteps(bundle, scMap, liveStepsReal); + if (daySub.length < 60) return; final motion = _motionMinutes(daySub); if (motion.isEmpty) return; @@ -3135,46 +3247,6 @@ class DerivationEngine { ); final v = est.present ? est.value : null; - // STEPS ARE REAL-MEASURED ONLY. The 1 Hz substrate contributes NOTHING to - // this number and must never do so again. - // - // The removed estimate multiplied 1 Hz "active minutes" by a walking - // cadence band. On a real user day it reported 2,645 steps against a true - // count under 400. Both halves of that conversion are invalid at 1 Hz: - // * cadence is not identifiable (gait 1.4-2.3 Hz is sub-Nyquist; 80/100/ - // 140/160 spm all alias to the same 0.333 Hz), and - // * the minutes being counted are not specifically ambulation — at the - // wrist, arm work out-accelerates walking (stirring ~104 mg, chopping - // ~139 mg vs walking ~66 mg ENMO), which is why wrist devices are - // documented emitting 22-27 false steps/min during dishes and driving - // (O'Connell 2017) while missing slow walking at sensitivity 0.05. - // Two errors of OPPOSITE sign: no gain constant fixes both. - // - // So `steps` is now absent unless something that can actually see gait - // measured it: the Tier A 100 Hz pedometer, or the phone's own pedometer - // (both land in `live_coverage`). No real source -> no number, per the - // absent-input-means-null contract. - final haveRealSteps = liveStepsReal > 0; - if (haveRealSteps) { - scMap?['steps'] = liveStepsReal.toDouble(); - } else { - scMap?.remove('steps'); - } - bundle['steps'] = { - 'value': haveRealSteps ? liveStepsReal : null, - 'real_measured': liveStepsReal, - 'source': haveRealSteps ? 'pedometer_100hz_or_phone' : null, - 'confidence': haveRealSteps ? 0.9 : 0.0, - 'tier': haveRealSteps ? 'HIGH' : 'ESTIMATE', - 'inputs_used': const ['live_coverage_pedometer'], - 'note': haveRealSteps - ? 'real pedometer count over measured windows only; time outside ' - 'those windows is not counted rather than estimated' - : 'no step count: nothing that can resolve gait measured this day. ' - 'A 1 Hz wrist stream cannot count steps, so no number is shown ' - 'instead of an invented one', - }; - // Movement minutes stay, as an explicitly non-locomotion activity index. if (v != null) { scMap?['active_min'] = v.activeMinutes.toDouble(); diff --git a/lib/health/phone_pedometer.dart b/lib/health/phone_pedometer.dart index 682512c..ec04656 100644 --- a/lib/health/phone_pedometer.dart +++ b/lib/health/phone_pedometer.dart @@ -56,17 +56,30 @@ class PhonePedometer { } } + /// Best-effort permission probe. `null` is treated as MAYBE, not NO. + /// + /// Health Connect's `hasPermissions` frequently returns null/false even after + /// the user has granted everything — `HealthExport` documents this exact + /// behaviour and deliberately attempts every write rather than gating on the + /// check. Gating a READ on it here would reintroduce that failure: on Android + /// phone steps could silently never sync after a successful grant, and the + /// user would see only a missing step count with nothing to act on. + /// + /// So this returns false ONLY on an explicit `false`. A null (unknown) result + /// lets the read proceed and lets the platform enforce — an ungranted read + /// simply returns no data, which `syncDay` already treats as "unknown", not + /// as zero. Future hasPermission() async { try { await _health.configure(); - return (await _health.hasPermissions( - _types, - permissions: const [HealthDataAccess.READ], - )) == - true; + final r = await _health.hasPermissions( + _types, + permissions: const [HealthDataAccess.READ], + ); + return r != false; // null => attempt anyway } catch (e) { debugPrint('[phone_pedometer] hasPermission: $e'); - return false; + return true; // probe failed; let the read attempt decide } } @@ -91,10 +104,26 @@ class PhonePedometer { var anyRead = false; final now = DateTime.now(); - for (var h = 0; h < 24; h++) { - final from = dayStartLocal.add(Duration(hours: h)); + // CALENDAR-AWARE hour walk. `Duration` arithmetic on a local DateTime is + // ABSOLUTE, so `dayStartLocal.add(Duration(hours: h))` over a fixed 24 + // iterations spans 25 wall-clock hours on a fall-back day (the last + // bucket crosses into the next local day and its steps get counted + // twice) and 23 on a spring-forward day (one real hour never queried). + // Constructing each boundary from calendar fields lets the runtime place + // the instant correctly, and the next-midnight bound ends the day exactly. + final nextMidnight = DateTime( + dayStartLocal.year, + dayStartLocal.month, + dayStartLocal.day + 1, + ); + for (var h = 0; h < 25; h++) { + final from = DateTime(dayStartLocal.year, dayStartLocal.month, + dayStartLocal.day, h); + if (!from.isBefore(nextMidnight)) break; // spring-forward short day if (from.isAfter(now)) break; // future hours of today - final to = from.add(const Duration(hours: 1)); + var to = DateTime(dayStartLocal.year, dayStartLocal.month, + dayStartLocal.day, h + 1); + if (to.isAfter(nextMidnight)) to = nextMidnight; final capped = to.isAfter(now) ? now : to; if (!capped.isAfter(from)) break; @@ -127,10 +156,13 @@ class PhonePedometer { Future syncRecent({int days = 7}) async { if (!await hasPermission()) return 0; final now = DateTime.now(); - final midnight = DateTime(now.year, now.month, now.day); var ok = 0; for (var d = 0; d < days; d++) { - final day = midnight.subtract(Duration(days: d)); + // Calendar subtraction, NOT `Duration(days: d)` — the latter lands on + // 23:00 or 01:00 across a DST transition rather than local midnight, + // which would mislabel the day and start its hour walk at the wrong + // offset. DateTime normalises an out-of-range day field for us. + final day = DateTime(now.year, now.month, now.day - d); if (await syncDay(day) != null) ok++; } return ok; diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index 0f00bda..1c5af89 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -231,8 +231,12 @@ class AppState extends ChangeNotifier { // truth for every network call — announcements, OTA, telemetry, import). healthSyncEnabled = prefs.getBool(_kHealthSync) ?? false; phoneStepsEnabled = prefs.getBool(_kPhoneSteps) ?? false; - // Steps only exist if a real pedometer measured them, so pull the phone's - // counts early — before the first derive sweep reads `live_coverage`. + // Steps only exist if a real pedometer measured them, so kick the phone + // pull early. This is BEST-EFFORT and establishes no ordering: it is + // unawaited, so a derive pass can read `live_coverage` while the sync is + // still in flight and that day then derives without phone steps. It + // self-heals on the next light pass, and awaiting here would put up to + // `7 x 24` platform round trips in front of app start. if (phoneStepsEnabled) unawaited(syncPhoneSteps()); // Best-effort, no prompt: learn the current health-permission state so the // Profile toggle reflects reality on open. @@ -392,7 +396,13 @@ class AppState extends ChangeNotifier { /// Export all finalized-but-unexported days now. Returns days written. Future healthSyncNow() async { final n = await _healthExport.exportAll(); - unawaited(syncPhoneSteps()); + // Gate on the user's own preference. `disablePhoneSteps` deliberately does + // NOT revoke the platform permission (that is the user's to do in + // Settings), so an unconditional sync here would write phone rows straight + // back after the user turned the feature off — and since `liveStepsForDay` + // prefers phone rows outright, it would re-suppress the band count, the + // exact outcome `disablePhoneSteps` exists to prevent. + if (phoneStepsEnabled) unawaited(syncPhoneSteps()); return n; } @@ -424,6 +434,15 @@ class AppState extends ChangeNotifier { /// rows over band rows — so a stale row would keep overriding the band /// indefinitely. Revoking the platform permission is the user's to do in /// Settings; all we can do is stop reading and forget what we read. + /// + /// KNOWN LIMIT — already-derived days keep their phone-sourced step values. + /// The screens read the scalars persisted in `day_result`/`metric_series`, + /// not `live_coverage`, so clearing the rows changes what FUTURE derives + /// compute, not what is already stored. Recent days correct themselves on + /// their next derive; days past the 48 h finalization window never re-derive + /// and keep the phone-sourced number permanently. Forcing a full re-derive + /// here would be a multi-minute background job triggered by a settings + /// toggle, which is worse than the staleness. Future disablePhoneSteps() async { phoneStepsEnabled = false; final prefs = await SharedPreferences.getInstance(); diff --git a/pubspec.lock b/pubspec.lock index 39ec215..a807cb8 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -964,8 +964,8 @@ packages: dependency: "direct main" description: path: "." - ref: "00a2efef57ffdf37c08b6f9255f136e559c2b7ae" - resolved-ref: "00a2efef57ffdf37c08b6f9255f136e559c2b7ae" + ref: "38a8636ae676888bc062cd9b0163b2de90ef02de" + resolved-ref: "38a8636ae676888bc062cd9b0163b2de90ef02de" url: "https://github.com/OpenStrap/analytics.git" source: git version: "1.0.0" diff --git a/pubspec.yaml b/pubspec.yaml index 08b26c7..a9a7f31 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -83,7 +83,7 @@ dependencies: # policy this branch consumes. # Verified present: `git show :lib/src/onehz/motion/steps.dart # | grep -E 'dailyActiveMinutes|shouldRefreezeFloor'`. - ref: 00a2efef57ffdf37c08b6f9255f136e559c2b7ae + ref: 38a8636ae676888bc062cd9b0163b2de90ef02de # BLE — flutter_blue_plus is the maintained cross-platform GATT client. flutter_blue_plus: ^1.36.8 diff --git a/test/phone_step_source_test.dart b/test/phone_step_source_test.dart index 8f10da4..c3c163b 100644 --- a/test/phone_step_source_test.dart +++ b/test/phone_step_source_test.dart @@ -126,4 +126,68 @@ void main() { await LocalDb.clearPhoneCoverage(); expect(await LocalDb.liveStepsForDay(day), 64); }); + + group('v27 migration — the path EVERY existing install takes', () { + // Every other test here opens a fresh DB, so `onCreate` emits the `source` + // column directly and the `if (oldV < 27)` step never runs. That upgrade + // path carries a load-bearing assumption: pre-v27 rows must default to + // 'band'. If they defaulted to 'phone', every existing band count would be + // read as a phone count and would suppress the real band fallback. An + // unguarded ALTER TABLE has also bricked this file's upgrades twice. + + Future seedV26() async { + final dir = await databaseFactory.getDatabasesPath(); + final db = await databaseFactory.openDatabase( + p.join(dir, LocalDb.dbName), + options: OpenDatabaseOptions( + version: 26, + onCreate: (db, _) async { + await db.execute('CREATE TABLE live_coverage (' + 'id INTEGER PRIMARY KEY AUTOINCREMENT,' + 'start_ts INTEGER NOT NULL,' + 'end_ts INTEGER NOT NULL,' + 'steps INTEGER NOT NULL,' + 'day TEXT NOT NULL)'); + }, + ), + ); + await db.insert('live_coverage', { + 'start_ts': 1000, + 'end_ts': 1600, + 'steps': 137, + 'day': day, + }); + await db.close(); + } + + test('a pre-v27 row survives the upgrade and counts as BAND', () async { + await seedV26(); + + // Reopening through LocalDb runs the real migration ladder. + expect(await LocalDb.liveStepsForDay(day), 137, + reason: 'the legacy row must still count after upgrading'); + + // ...and it must be BAND, so a phone sync can still take precedence. + await LocalDb.replacePhoneCoverageForDay( + day, + [(startTs: 1000, endTs: 4600, steps: 900)], + ); + expect(await LocalDb.liveStepsForDay(day), 900, + reason: 'legacy rows defaulting to phone would block this override'); + + // Dropping the phone rows reveals the legacy band row again — proof it + // was never silently relabelled. + await LocalDb.clearPhoneCoverage(); + expect(await LocalDb.liveStepsForDay(day), 137); + }); + + test('the migration is idempotent across repeated opens', () async { + await seedV26(); + expect(await LocalDb.liveStepsForDay(day), 137); + await LocalDb.close(); + // The second open re-runs `_repairOpenSchema`, which also calls + // `_ensureLiveCoverageSource`. An unguarded ALTER would throw here. + expect(await LocalDb.liveStepsForDay(day), 137); + }); + }); } From 07ade7f28402a62499f32defc53376c4c3dfa0dd Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 5 Aug 2026 01:57:43 +0530 Subject: [PATCH 3/6] Fix the DST, ordering and honesty gaps in the steps rework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. Every change below was checked against the actual code or the actual platform behaviour before being made; two review findings were disproved and are not acted on (see the PR thread). phone_pedometer * A spring-forward day truncated and then PERMANENTLY corrupted its own step count. `DateTime(y,m,d,2)` and `DateTime(y,m,d,3)` resolve to the same instant across the missing local hour, so h=2 is zero-width and the `!capped.isAfter(from)` guard ended the whole walk — hours 3-23 were never queried while `anyRead` was already true, so the day was REPLACED with ~3 hours of windows. Reproduced against the Dart runtime under TZ=America/New_York. Skip the bucket instead of breaking; the "reached now" case is already handled by the `from.isAfter(now)` guard above. * A partial read now abandons the day. `null` from this plugin means the query FAILED, not that the hour was empty — verified in both natives at health 11.1.1 (iOS returns 0 via `steps = 0.0` when `sumQuantity()` is nil; Android returns 0 via `?: 0L`, and null only from the catch). Since `replacePhoneCoverageForDay` is delete-then-insert and phone rows win over band rows, banking a short read lowered a good total and kept the band suppressed. * Routine syncs pull 2 days, not 7. Each hourly bucket is one platform round trip, so the old default was up to 168 sequential calls on every launch and again after every export. The full backfill window still runs on the explicit gestures. * A `stepReader` seam makes the walk testable — `Health` is a private-constructor singleton and cannot be faked otherwise, which is why the two bugs above had no coverage. db / import * `coverageWindowsOverlapping` now filters by source. Its only remaining caller is the NOOP importer's double-bank guard, and phone rows share the table over the same hours — so a user with phone steps enabled importing a NOOP backup had their BAND step runs clipped against the PHONE's windows and silently dropped, the import reporting success while banking nothing. derivation_engine * The frozen floor is one shared scalar resolved by a read-modify-write that a NEWEST-FIRST concurrent sweep reaches from many days at once — and this version bump forces exactly that sweep. Serialized it, clamped `daysSinceFrozen` to >= 0 so a backfill day is not read as stale, and stopped an older day overwriting a newer freeze. Without these, sweep order decided every day's active_min, which is what `_BaselineHistoryCache` already forbids for baselines. * `_wearGapDays` walked back with `Duration`, which skips the spring-forward day entirely (from 2026-03-10 it yields 03-09, 03-07, ... — 03-08 never appears). Calendar fields now, via the shared day_label helper. * Stopped REMOVING `active_min` on abstention. `_applyWakeDayFeatures` had already written it from `_activeMinutes`, a separate quantity never part of the fabricated step conversion; deleting it wiped a number the user had for the whole enrollment window and nulled its trend series. Abstaining from the new index is right, destroying the old measurement to do it is not. * `inputs_used` no longer claims hr_1hz — the HR gate was deleted. health_export * The legacy STEPS purge is a one-shot migration with its own cursor, out of the per-day rewrite loop and out of the day's success accounting. It was running a delete for a type nothing writes on every re-export of the unfinalized tail, forever, and could fail a day's export. honesty (the point of the PR) * The Steps screen still explained the deleted estimator: an "est" tag, an info panel describing hours "ESTIMATED from your walking minutes and cadence", and "Walk with the app open to sharpen the estimate". * "Calibrate steps — Walk ~250 steps with the app open" was still routed and still wrote a `step_calibration` baseline, but its only reader was `dailyStepEstimate`. The user could finish the walk and be told they had calibrated something nothing read. Removed the screen, the route, the app_state methods, the post-session write and the dead db accessors; the Tier-A AN-2554 pedometer never used it. * "no steps yet" became "not measured" — absent means nothing could resolve gait, which is not the same claim as zero. * NSHealthShareUsageDescription said we read samples "to avoid writing duplicates". We now read steps to display them, which is a different purpose than the one the user consented to. app_state * Turning phone steps off now re-derives. Clearing `live_coverage` only changed what future derives compute, so the user kept seeing phone-sourced counts; `setSleepOverride` already handles the equivalent case this way. Scope is bounded by raw retention, not the whole history. * Surfaced the sync result in Profile. On iOS `requestAuthorization` reports success even when READ is denied, so the toggle sat on while nothing ever arrived, with nothing for the user to act on. tests: 1118 -> 1140, analyze clean, run against the pinned analytics with no overrides file present (pubspec.lock untouched). --- ios/Runner/Info.plist | 2 +- lib/compute/derivation_engine.dart | 129 +++++++----- lib/compute/movement_floor_policy.dart | 86 ++++++++ lib/data/db.dart | 43 ++-- lib/health/health_export.dart | 63 +++++- lib/health/phone_pedometer.dart | 98 +++++++-- lib/import/noop_import.dart | 9 +- lib/state/app_state.dart | 215 ++++++-------------- lib/ui/profile/profile_screen.dart | 28 +++ lib/ui/screens/metric_row.dart | 2 +- lib/ui/screens/screens.dart | 40 ++-- lib/ui/today/step_calibration_screen.dart | 232 ---------------------- test/app_state_regressions_test.dart | 40 +--- test/metric_trend_redesign_test.dart | 9 +- test/movement_floor_policy_test.dart | 118 +++++++++++ test/phone_pedometer_hour_walk_test.dart | 148 ++++++++++++++ 16 files changed, 722 insertions(+), 540 deletions(-) create mode 100644 lib/compute/movement_floor_policy.dart delete mode 100644 lib/ui/today/step_calibration_screen.dart create mode 100644 test/movement_floor_policy_test.dart create mode 100644 test/phone_pedometer_hour_walk_test.dart diff --git a/ios/Runner/Info.plist b/ios/Runner/Info.plist index bbbd94d..c59e3b8 100644 --- a/ios/Runner/Info.plist +++ b/ios/Runner/Info.plist @@ -62,7 +62,7 @@ NSBluetoothAlwaysUsageDescription OpenStrap connects to your WHOOP band over Bluetooth to sync your health data. NSHealthShareUsageDescription - OpenStrap reads recent samples to avoid writing duplicates into Apple Health. + OpenStrap reads your step count from Apple Health to show your daily steps, and reads back its own recent samples so it never writes duplicates. NSHealthUpdateUsageDescription OpenStrap writes your sleep, resting heart rate, HRV, respiratory rate, energy and workouts into Apple Health. NSBluetoothPeripheralUsageDescription diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 49858a1..6fa393c 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -38,6 +38,7 @@ import '../notify/tap_router.dart' show kRouteWorkoutSuggestion; import '../telemetry/telemetry_service.dart'; import 'crossday_pipeline.dart'; import 'derive_pacing.dart'; +import 'movement_floor_policy.dart' as mfp; import 'sleep_profile_policy.dart'; import 'derive_prepare.dart'; import 'onehz_pipeline.dart'; @@ -788,6 +789,26 @@ Future runWithConcurrency( await Future.wait(List.generate(poolSize, (_) => lane())); } +/// Minimal async mutex: serializes read-modify-write sections that concurrent +/// day workers ([runWithConcurrency]) would otherwise interleave. +/// +/// Dart's scheduler makes a single statement atomic, but NOT a +/// read → decide → write sequence with `await`s in it: every lane can observe +/// the pre-write state before any of them writes. The shared movement floor is +/// exactly that shape, so it needs one. +class _AsyncLock { + Future _tail = Future.value(); + + Future run(Future Function() action) { + final completer = Completer(); + final previous = _tail; + _tail = completer.future; + return previous + .then((_) => action()) + .whenComplete(completer.complete); + } +} + class DerivationEngine { DerivationEngine({this.log, this.background = false}); final void Function(String)? log; @@ -2375,11 +2396,13 @@ class DerivationEngine { 'stress': sc('stress'), 'spo2': sc('spo2'), 'calories': sc('calories'), - // Steps = real 100 Hz count + 1 Hz estimate over uncovered minutes - // (computed in _stepsAndEnergy; never double-counted). + // Steps = REAL pedometer counts only (band 100 Hz / phone / NOOP + // import, all via `live_coverage`). Absent — written as a NULL row, so + // a previously fabricated value is overwritten rather than left + // standing — on any day nothing gait-capable measured. 'steps': sc('steps'), - // Ambulatory minutes — the quantity 1 Hz can actually resolve, and the - // unit public activity guidance uses. Steps are derived FROM this. + // Movement minutes: activity VOLUME, not locomotion. Steps are NOT + // derived from this and never will be again (see the v55/v56 note). 'active_min': sc('active_min'), // This day's high quantile of the calibration-invariant dynamic accel // amplitude. Not a user-facing metric: it is the per-day summary the @@ -3054,9 +3077,36 @@ class DerivationEngine { /// continuously-recomputed floor tracks the user and reports a near-constant /// number regardless of behaviour (measured: 37 active minutes at 1x, 1.5x, /// 2x AND 3x activity, versus 23 -> 254 with a frozen floor). + /// + /// ORDER-INDEPENDENCE. The floor is ONE persisted scalar shared by every day, + /// but `run()` dispatches days NEWEST-FIRST through a concurrent worker pool, + /// so this read-modify-write is reached by several days at once. Three things + /// keep the outcome from depending on which worker finishes last: + /// + /// 1. `_floorLock` serializes the whole read/decide/write, so two days can + /// never both observe "nothing stored" and both commit. + /// 2. `daysSinceFrozen` is clamped at 0 (see [mfp.daysSinceFrozen]), so a + /// backfill day never reads as a stale floor and never triggers a + /// re-freeze just for being old. + /// 3. `mayCommitFloorOn` stops an older day overwriting a newer freeze. + /// + /// Without these, a `kAlgoVersion` bump — which this very change forces — + /// would re-derive the whole retained window and let sweep order decide every + /// day's `active_min`. That is precisely what `_BaselineHistoryCache`'s own + /// contract forbids for baselines. static Future _frozenMovementFloor( _BaselineHistoryCache history, String dayId, + ) => + _floorLock.run(() => _resolveMovementFloor(history, dayId)); + + /// Serializes the shared-floor read-modify-write across concurrent day + /// workers. See [_frozenMovementFloor]. + static final _AsyncLock _floorLock = _AsyncLock(); + + static Future _resolveMovementFloor( + _BaselineHistoryCache history, + String dayId, ) async { final stored = await LocalDb.getMovementFloor(); final hist = history.valuesBefore('dyn_p90', dayId); @@ -3071,10 +3121,15 @@ class DerivationEngine { // `false` that reads like a checked condition. `wearGapDays` IS // derivable — a run of days with no `dyn_p90` row means the band was not // worn — so it is computed and passed. - final age = _daysBetweenLabels(stored.frozenOn, dayId); final refreeze = ana.shouldRefreezeFloor( - daysSinceFrozen: age, - wearGapDays: _wearGapDays(history, dayId), + daysSinceFrozen: mfp.daysSinceFrozen( + frozenOn: stored.frozenOn, + dayId: dayId, + ), + wearGapDays: mfp.wearGapDays( + have: history.datesFor('dyn_p90'), + dayId: dayId, + ), ); if (!refreeze) return stored.floorG; @@ -3092,6 +3147,13 @@ class DerivationEngine { return null; } + // A backfill day may CONSUME the shared floor but never move it — otherwise + // a newest-first sweep's oldest day could clobber the freeze its newest day + // just established. + if (!mfp.mayCommitFloorOn(frozenOn: stored?.frozenOn, dayId: dayId)) { + return stored?.floorG; + } + final floor = ana.personalDynFloorFromDailySummaries(hist); if (floor == null) return stored?.floorG; await LocalDb.putMovementFloor( @@ -3106,38 +3168,6 @@ class DerivationEngine { return floor; } - /// Consecutive days immediately before [dayId] with no `dyn_p90` row. - /// - /// A missing daily summary means the band produced no usable motion for 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 movement floor should be re-estimated. - static int _wearGapDays(_BaselineHistoryCache history, String dayId) { - final target = DateTime.tryParse(dayId); - if (target == null) return 0; - final have = history.datesFor('dyn_p90'); - if (have.isEmpty) return 0; - var gap = 0; - for (var back = 1; back <= 60; back++) { - final d = target.subtract(Duration(days: back)); - final label = - '${d.year.toString().padLeft(4, '0')}-' - '${d.month.toString().padLeft(2, '0')}-' - '${d.day.toString().padLeft(2, '0')}'; - if (have.contains(label)) break; - gap++; - } - return gap; - } - - /// Whole days between two `YYYY-MM-DD` labels; 0 if either is unparseable. - static int _daysBetweenLabels(String a, String b) { - final da = DateTime.tryParse(a); - final db = DateTime.tryParse(b); - if (da == null || db == null) return 0; - return db.difference(da).inDays.abs(); - } - /// Write the day's step count. REAL PEDOMETER MEASUREMENTS ONLY. /// /// The 1 Hz substrate contributes NOTHING here and must never do so again. @@ -3248,11 +3278,16 @@ class DerivationEngine { final v = est.present ? est.value : null; // Movement minutes stay, as an explicitly non-locomotion activity index. - if (v != null) { - scMap?['active_min'] = v.activeMinutes.toDouble(); - } else { - scMap?.remove('active_min'); - } + // + // ONLY OVERWRITE ON SUCCESS — never remove. `_applyWakeDayFeatures` has + // already written `active_min` from `_activeMinutes` (ENMO over wake), a + // SEPARATE quantity that was never part of the fabricated step + // conversion. Removing it on abstention deleted a number the user + // previously had, for the whole enrollment window (every day a new user + // has before the floor freezes), and nulled its trend series with it. + // Abstaining from the new index is right; destroying the old independent + // measurement to do it is not. + if (v != null) scMap?['active_min'] = v.activeMinutes.toDouble(); bundle['movement'] = { 'active_min': v?.activeMinutes, 'bout_count': v?.boutCount, @@ -3260,7 +3295,9 @@ class DerivationEngine { 'coverage': v?.coverage, 'confidence': est.present ? est.confidence : 0.0, 'tier': 'ESTIMATE', - 'inputs_used': const ['dyn_amp_1hz', 'hr_1hz', 'personal_dyn_floor'], + // HR is NOT an input any more — the resting-HR gate was deleted in v56 + // after it changed active minutes by exactly zero on every day tested. + 'inputs_used': const ['dyn_amp_1hz', 'personal_dyn_floor'], 'note': v == null ? (est.note ?? 'need_baseline') : 'minutes of sustained wrist movement — activity volume, NOT ' @@ -4556,7 +4593,7 @@ Future runCancellableIsolate( /// Sendable input for [DerivationEngine._computeDayBlocks] — crosses the /// `Isolate.run` boundary, so every field is plain data (Substrate is int/double -/// lists; Profile/StepCalibration are primitive data classes). DB reads that the +/// lists; Profile is a primitive data class). DB reads that the /// pure compute needs are performed by the caller and passed in here. class _DayBlocksInput { final Substrate daySub; diff --git a/lib/compute/movement_floor_policy.dart b/lib/compute/movement_floor_policy.dart new file mode 100644 index 0000000..f8eede4 --- /dev/null +++ b/lib/compute/movement_floor_policy.dart @@ -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 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; +} diff --git a/lib/data/db.dart b/lib/data/db.dart index 812cc5a..6473cdb 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -12,7 +12,6 @@ import 'dart:convert'; import 'dart:io'; -import 'package:openstrap_analytics/onehz.dart' as ana; import 'package:openstrap_protocol/openstrap_protocol.dart' as proto; import 'package:path/path.dart' as p; import 'package:path_provider/path_provider.dart'; @@ -911,17 +910,31 @@ class LocalDb { return phone > 0 ? phone : band; } - /// Coverage windows ([startSec, endSec]) overlapping [loSec, hiSec) — used to - /// exclude already-counted minutes from the 1 Hz estimate. + /// Coverage windows ([startSec, endSec]) overlapping [loSec, hiSec), for ONE + /// [source] (band by default). + /// + /// The 1 Hz-estimate exclusion this originally served is gone along with the + /// estimator. Its only remaining caller is the NOOP importer, which reads back + /// the spans it has already banked so `stepRuns` can clip them out and a + /// re-import over an overlapping span cannot double-count. + /// + /// THE SOURCE FILTER IS LOAD-BEARING for that caller. Phone-pedometer rows now + /// share this table and cover the same wall-clock hours, so an unfiltered read + /// let a user with phone steps enabled import a NOOP backup whose BAND step + /// runs were clipped against the PHONE's windows and silently dropped — the + /// import reporting success while banking nothing for those days. Band clips + /// against band. Phone coverage needs no clipping at all: it is replaced + /// wholesale per day (see [replacePhoneCoverageForDay]). static Future>> coverageWindowsOverlapping( int loSec, - int hiSec, - ) async { + int hiSec, { + String source = kStepSourceBand, + }) async { final db = await instance; final rows = await db.query( 'live_coverage', - where: 'end_ts >= ? AND start_ts < ?', - whereArgs: [loSec, hiSec], + where: 'end_ts >= ? AND start_ts < ? AND source = ?', + whereArgs: [loSec, hiSec, source], ); return [ for (final r in rows) @@ -3951,22 +3964,6 @@ class LocalDb { jsonEncode({'floor_g': floorG, 'frozen_on': frozenOn, 'days': days}), ); - static Future getStepCalibration() async { - final row = await baseline('step_calibration'); - final raw = row?['payload_json']; - if (raw is! String || raw.isEmpty) return null; - try { - final decoded = jsonDecode(raw); - return decoded is Map - ? ana.StepCalibration.fromJson(decoded.cast()) - : null; - } catch (_) { - return null; - } - } - - static Future putStepCalibration(ana.StepCalibration calibration) => - putBaseline('step_calibration', jsonEncode(calibration.toJson())); /// A long-format metric series (oldest first) for trends/sparklines. static Future>> metricSeries( diff --git a/lib/health/health_export.dart b/lib/health/health_export.dart index 24bc55c..070b43d 100644 --- a/lib/health/health_export.dart +++ b/lib/health/health_export.dart @@ -62,13 +62,13 @@ class HealthExporter { HealthDataType.HEART_RATE, HealthDataType.ACTIVE_ENERGY_BURNED, HealthDataType.BASAL_ENERGY_BURNED, - // STEPS is listed for DELETION ONLY — we no longer write steps (see the - // block further down for why). Keeping it here means the per-day delete - // pass below actively PURGES the fabricated step samples we wrote into - // Apple Health / Health Connect in earlier versions, instead of leaving - // them contaminating the system store forever. Deleting our own samples - // is a write-scope operation, which is why WRITE_STEPS stays in the - // Android manifest even though nothing writes steps any more. + // STEPS is requested for DELETE SCOPE ONLY — nothing writes steps any + // more (see the block further down for why). We still need the write + // permission to purge the fabricated step samples earlier versions put + // into Apple Health / Health Connect, which is why WRITE_STEPS stays in + // the Android manifest. That purge is a ONE-SHOT migration and does not + // belong in the per-day rewrite loop — see [_purgeLegacyStepsIfNeeded] + // and [_rewriteTypes]. HealthDataType.STEPS, HealthDataType.SLEEP_DEEP, HealthDataType.SLEEP_REM, @@ -78,6 +78,49 @@ class HealthExporter { HealthDataType.WORKOUT, ]; + /// The types the per-day delete-then-write pass touches. + /// + /// STEPS is deliberately excluded. It is in [_types] only so `request()` asks + /// for the scope the legacy purge needs; including it here would run a delete + /// for a type nothing writes on every re-export of the recent (not-yet- + /// finalized) tail, forever, and would let that delete's failure flip a day's + /// export to unsuccessful. + List get _rewriteTypes => + [for (final t in _types) if (t != HealthDataType.STEPS) t]; + + /// Cursor for the one-shot legacy-STEPS purge: the newest day already purged. + static const _kStepsPurgeCursor = 'health_steps_purged_through'; + String? _stepsPurgedThrough; + + /// Delete the fabricated STEPS samples earlier versions wrote for [date]. + /// + /// ONE-SHOT, and deliberately not part of the day's success accounting: this + /// is a migration cleaning up data we should never have written, not part of + /// exporting the day. A failure here must not stall the export cursor for a + /// type nothing writes. Days are walked ascending, so the cursor advances + /// monotonically and a re-exported tail day is not re-purged. + Future _purgeLegacyStepsIfNeeded( + String date, + DateTime dayStart, + DateTime dayEnd, + ) async { + _stepsPurgedThrough ??= await LocalDb.getCursor(_kStepsPurgeCursor) ?? ''; + final through = _stepsPurgedThrough!; + if (through.isNotEmpty && date.compareTo(through) <= 0) return; + try { + await _health.delete( + type: HealthDataType.STEPS, + startTime: dayStart, + endTime: dayEnd, + ); + _stepsPurgedThrough = date; + await LocalDb.setCursor(_kStepsPurgeCursor, date); + } catch (e) { + // Leave the cursor where it is so the next pass retries this day. + debugPrint('[health] purge legacy steps $date: $e'); + } + } + // We do NOT gate on a write-permission check: HealthKit hides write-auth by // design, and Health Connect's hasPermissions(WRITE) frequently returns // null/false even after the user grants everything — which would leave the UI @@ -345,9 +388,13 @@ class HealthExporter { // write on failure (best-effort, idempotent re-export corrects it later). var success = true; + // One-shot cleanup of the fabricated step samples earlier versions wrote. + // Outside the success accounting on purpose — see the method doc. + await _purgeLegacyStepsIfNeeded(date, dayStart, dayEnd); + // Idempotency: remove OUR previously-written samples for this day (HealthKit / // Health Connect only let an app delete its own data), then re-write fresh. - for (final t in _types) { + for (final t in _rewriteTypes) { try { await _health.delete(type: t, startTime: dayStart, endTime: dayEnd); } catch (e) { diff --git a/lib/health/phone_pedometer.dart b/lib/health/phone_pedometer.dart index ec04656..b84b59f 100644 --- a/lib/health/phone_pedometer.dart +++ b/lib/health/phone_pedometer.dart @@ -4,6 +4,9 @@ import 'package:health/health.dart'; import '../data/db.dart'; import '../data/day_label.dart'; +/// Reads steps in `[from, to)`. Null means the READ FAILED — see [syncDay]. +typedef StepIntervalReader = Future Function(DateTime from, DateTime to); + /// REAL step counts, read from the phone's own pedometer. /// /// WHY THIS EXISTS @@ -31,9 +34,20 @@ import '../data/day_label.dart'; /// Nothing leaves the phone, and nothing here is written back — see /// [HealthExport] for why we deliberately stopped writing STEPS out. class PhonePedometer { - PhonePedometer({Health? health}) : _health = health ?? Health(); + /// [stepReader] exists so the hour walk is testable. `Health` has a private + /// constructor and is a singleton factory, so it cannot be subclassed or + /// faked from a test library — and the walk is where the DST and partial-read + /// bugs live, so it needs coverage that does not touch a real health store. + PhonePedometer({Health? health, StepIntervalReader? stepReader}) + : _health = health ?? Health(), + _stepReader = stepReader; final Health _health; + final StepIntervalReader? _stepReader; + + Future _readSteps(DateTime from, DateTime to) => + _stepReader?.call(from, to) ?? + _health.getTotalStepsInInterval(from, to); static const List _types = [HealthDataType.STEPS]; @@ -95,10 +109,29 @@ class PhonePedometer { /// /// Returns the day's total, or null if the read failed or was not permitted /// (null means "unknown", NOT zero — the caller must not persist a zero). + /// + /// A null from ANY hour aborts the whole day. `null` from this plugin means + /// the query FAILED, not that the hour was empty — verified in both native + /// implementations at health 11.1.1: + /// + /// * iOS `SwiftHealthPlugin.swift`: `HKStatisticsQuery` returns `nil` only + /// via `guard let queryResult else { result(nil) }`. An hour with no + /// samples has a nil `sumQuantity()` but still falls through to + /// `steps = 0.0` and returns `0`. + /// * Android `HealthPlugin.kt`: `response[StepsRecord.COUNT_TOTAL] ?: 0L` + /// returns `0` for an empty range; `result.success(null)` happens only in + /// the `catch`. + /// + /// So a partial read is a real failure, and it must not be persisted: + /// [LocalDb.replacePhoneCoverageForDay] is delete-then-insert, so banking a + /// short read would LOWER a previously complete day. And because + /// [LocalDb.liveStepsForDay] prefers phone rows outright, the truncated total + /// would also keep suppressing the band fallback. Future syncDay(DateTime dayStartLocal) async { final dayId = dayLabelOf(dayStartLocal); try { - await _health.configure(); + // Only touch the platform when we are actually going through it. + if (_stepReader == null) await _health.configure(); final windows = <({int startTs, int endTs, int steps})>[]; var total = 0; var anyRead = false; @@ -125,10 +158,23 @@ class PhonePedometer { dayStartLocal.day, h + 1); if (to.isAfter(nextMidnight)) to = nextMidnight; final capped = to.isAfter(now) ? now : to; - if (!capped.isAfter(from)) break; + // SKIP a zero-length bucket, never END the walk on one. On a + // spring-forward day the missing local hour makes `DateTime(y,m,d,2)` + // and `DateTime(y,m,d,3)` resolve to the SAME instant, so `h = 2` is + // zero-width. Breaking here left every remaining hour of that day + // unqueried while `anyRead` was already true from the earlier hours, so + // the day was REPLACED with ~3 hours of windows — and since phone rows + // win over band rows, that truncated total stuck permanently once the + // day aged out of the `syncRecent` window. + // + // The "reached now" case does not need a break here: the next + // iteration's `from` is after `now` and the guard above ends the walk. + if (!capped.isAfter(from)) continue; - final n = await _health.getTotalStepsInInterval(from, capped); - if (n == null) continue; + final n = await _readSteps(from, capped); + // Read failure (see the doc above) — abandon the day rather than + // persist a partial one over a good previous sync. + if (n == null) return null; anyRead = true; if (n <= 0) continue; windows.add(( @@ -139,9 +185,8 @@ class PhonePedometer { total += n; } - // A day where every hour returned null is a FAILED read, not a zero-step - // day. Persisting nothing keeps whatever we already had rather than - // wiping a good previous sync. + // No hour was ever polled (a day entirely in the future, or a walk that + // produced no buckets at all). Nothing was read, so nothing is known. if (!anyRead) return null; await LocalDb.replacePhoneCoverageForDay(dayId, windows); @@ -152,19 +197,46 @@ class PhonePedometer { } } - /// Sync the last [days] days (including today). Returns days successfully read. - Future syncRecent({int days = 7}) async { - if (!await hasPermission()) return 0; + /// Days pulled on a routine (launch / post-export) sync. + /// + /// One platform round trip PER HOUR PER DAY, so the window is the whole cost: + /// the original 7-day default was up to 168 sequential `getTotalStepsInInterval` + /// calls, fired on every launch and again after every health export. Only + /// today can still change, and yesterday only if the app did not run then, so + /// two days covers the routine case at ~48 calls. + static const int routineSyncDays = 2; + + /// Days pulled on an EXPLICIT sync (the user enabling the toggle, or a manual + /// health sync) — the backfill window, worth its cost because the user asked. + static const int fullSyncDays = 7; + + /// Sync the last [days] days (including today). + /// + /// Returns how many days were read successfully and the steps they held. The + /// caller surfaces this: "permission granted but no data ever arrives" is + /// otherwise a silent dead end on iOS, where `requestAuthorization` reports + /// success even when the user denied READ. + Future<({int daysRead, int totalSteps})> syncRecent({ + int days = routineSyncDays, + }) async { + if (_stepReader == null && !await hasPermission()) { + return (daysRead: 0, totalSteps: 0); + } final now = DateTime.now(); var ok = 0; + var total = 0; for (var d = 0; d < days; d++) { // Calendar subtraction, NOT `Duration(days: d)` — the latter lands on // 23:00 or 01:00 across a DST transition rather than local midnight, // which would mislabel the day and start its hour walk at the wrong // offset. DateTime normalises an out-of-range day field for us. final day = DateTime(now.year, now.month, now.day - d); - if (await syncDay(day) != null) ok++; + final n = await syncDay(day); + if (n != null) { + ok++; + total += n; + } } - return ok; + return (daysRead: ok, totalSteps: total); } } diff --git a/lib/import/noop_import.dart b/lib/import/noop_import.dart index de6703b..627508e 100644 --- a/lib/import/noop_import.dart +++ b/lib/import/noop_import.dart @@ -412,10 +412,11 @@ class NoopImporter { return out; } - /// Bank [date]'s step runs into `live_coverage` so the derivation picks them up - /// as REAL steps (`liveStepsForDay`) and excludes those minutes from the 1 Hz - /// estimate (`coverageWindowsOverlapping`) — the same contract the live 100 Hz - /// pedometer uses, so imported and live days are counted identically. + /// Bank [date]'s step runs into `live_coverage` so the derivation picks them + /// up as REAL steps (`liveStepsForDay`) — the same contract the live 100 Hz + /// pedometer uses, so imported and live days are counted identically. These + /// are BAND-sourced counts (the strap's own step counter), which is what makes + /// them a real gait measurement rather than the deleted 1 Hz estimate. /// /// IDEMPOTENT BY TIME SPAN, not by exact window: `live_coverage` is an /// append-only SUM with no uniqueness constraint, so anything already banked diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index 1c5af89..28f5d92 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -235,8 +235,12 @@ class AppState extends ChangeNotifier { // pull early. This is BEST-EFFORT and establishes no ordering: it is // unawaited, so a derive pass can read `live_coverage` while the sync is // still in flight and that day then derives without phone steps. It - // self-heals on the next light pass, and awaiting here would put up to - // `7 x 24` platform round trips in front of app start. + // self-heals on the next light pass. + // + // ROUTINE window only (2 days, ~48 platform round trips). Each hourly + // bucket is one platform call, so the 7-day backfill window is up to 168 of + // them; only today can still change, and only yesterday if the app did not + // run then. The full window runs on the explicit gestures instead. if (phoneStepsEnabled) unawaited(syncPhoneSteps()); // Best-effort, no prompt: learn the current health-permission state so the // Profile toggle reflects reality on open. @@ -402,7 +406,10 @@ class AppState extends ChangeNotifier { // back after the user turned the feature off — and since `liveStepsForDay` // prefers phone rows outright, it would re-suppress the band count, the // exact outcome `disablePhoneSteps` exists to prevent. - if (phoneStepsEnabled) unawaited(syncPhoneSteps()); + // An explicit health sync is a user gesture — take the full window. + if (phoneStepsEnabled) { + unawaited(syncPhoneSteps(days: PhonePedometer.fullSyncDays)); + } return n; } @@ -423,7 +430,9 @@ class AppState extends ChangeNotifier { final prefs = await SharedPreferences.getInstance(); await prefs.setBool(_kPhoneSteps, ok); notifyListeners(); - if (ok) unawaited(syncPhoneSteps()); + // The user just asked for this, so pull the full backfill window rather + // than the cheap routine one. + if (ok) unawaited(syncPhoneSteps(days: PhonePedometer.fullSyncDays)); return ok; } @@ -435,36 +444,54 @@ class AppState extends ChangeNotifier { /// indefinitely. Revoking the platform permission is the user's to do in /// Settings; all we can do is stop reading and forget what we read. /// - /// KNOWN LIMIT — already-derived days keep their phone-sourced step values. - /// The screens read the scalars persisted in `day_result`/`metric_series`, - /// not `live_coverage`, so clearing the rows changes what FUTURE derives - /// compute, not what is already stored. Recent days correct themselves on - /// their next derive; days past the 48 h finalization window never re-derive - /// and keep the phone-sourced number permanently. Forcing a full re-derive - /// here would be a multi-minute background job triggered by a settings - /// toggle, which is worse than the staleness. + /// Clearing `live_coverage` only changes what FUTURE derives compute — the + /// screens read scalars persisted in `day_result`/`metric_series`. So this + /// also re-derives, exactly as `setSleepOverride` does for the equivalent + /// case; without it the user turns the toggle off and keeps seeing + /// phone-sourced counts. + /// + /// The re-derive is bounded: its scope is days that still hold raw + /// (`rawRetentionDays`), not the whole history. Older days keep their + /// phone-sourced value permanently — there is no substrate left to recompute + /// them from, which is the same limit every other version bump has. Future disablePhoneSteps() async { phoneStepsEnabled = false; final prefs = await SharedPreferences.getInstance(); await prefs.setBool(_kPhoneSteps, false); + phoneStepsLastSyncedDays = null; + phoneStepsLastTotal = null; try { await LocalDb.clearPhoneCoverage(); } catch (e) { debugPrint('[phone_steps] clear: $e'); } notifyListeners(); + unawaited(_reanalyzeForOverride()); } + /// Days successfully read on the last phone-step sync, and the total banked. + /// + /// Surfaced in Profile because the failure mode is otherwise INVISIBLE: on + /// iOS `requestAuthorization` returns true even when the user denies READ + /// (HealthKit hides read denial by design), so the toggle sits on, every read + /// comes back empty, and no step count ever appears with nothing to act on. + int? phoneStepsLastSyncedDays; + int? phoneStepsLastTotal; + /// Pull the last [days] days of phone step counts into `live_coverage`. /// /// Idempotent (delete-then-insert per day, scoped to the phone source), so /// calling it repeatedly — on launch, after a sync, from a background pass — /// can never accumulate. Best-effort; never throws. - Future syncPhoneSteps({int days = 7}) async { + Future syncPhoneSteps({ + int days = PhonePedometer.routineSyncDays, + }) async { try { - final n = await _phonePedometer.syncRecent(days: days); - if (n > 0) notifyListeners(); - return n; + final r = await _phonePedometer.syncRecent(days: days); + phoneStepsLastSyncedDays = r.daysRead; + phoneStepsLastTotal = r.totalSteps; + notifyListeners(); + return r.daysRead; } catch (e) { debugPrint('[phone_steps] sync: $e'); return 0; @@ -1769,9 +1796,9 @@ class AppState extends ChangeNotifier { } /// True while some foreground feature is actively consuming the live streams - /// (workout coach, HRV spot check, step-calibration walk, breathing session). + /// (workout coach, HRV spot check, breathing session). bool get _hasLiveConsumer => - activeWorkout != null || spotActive || _stepCalActive || breathingActive; + activeWorkout != null || spotActive || breathingActive; /// Downgrade live to HR-only when backgrounded with no live consumer. The /// keep-alive re-arm respects the HR-only mode, so the downgrade sticks until @@ -1861,8 +1888,6 @@ class AppState extends ChangeNotifier { final List _magMin = []; // current minute's magnitude signal int _committedRaw = 0; // raw (pre-gain) steps from completed minutes int _liveSamples = 0; // total 100 Hz samples streamed this session - double _liveEnmoSum = 0; // 1 Hz-equivalent ENMO accumulator (for calibration) - int _liveEnmoN = 0; bool _imuStreamSeen = false; // prefer the 0x33 IMU stream once it appears static const int _minuteSamples = 6000; // 60 s @ 100 Hz — calibration chunk int _lastWalkMs = 0; // last time steps were accumulated @@ -1954,8 +1979,11 @@ class AppState extends ChangeNotifier { final mags = f.mags; if (mags.isEmpty) return; // Append this frame's |a|(g) samples (gravity INCLUDED — AN-2554's dynamic - // threshold rides the ~1 g baseline). Also accumulate a 1 Hz-equivalent ENMO - // sample (mean |a| − 1 g) for cadence calibration. + // threshold rides the ~1 g baseline). `e` is this frame's 1 Hz-equivalent + // ENMO (mean |a| − 1 g), read below by the stillness nudge and the posture + // check. It no longer feeds a cadence calibration — that was deleted along + // with the 1 Hz step estimator that was its only consumer (kAlgoVersion + // v55). var magSum = 0.0; for (final m in mags) { _magMin.add(m); @@ -1963,8 +1991,6 @@ class AppState extends ChangeNotifier { } _liveSamples += mags.length; final e = (magSum / mags.length) - 1.0; - _liveEnmoSum += e > 0 ? e : 0.0; - _liveEnmoN++; // Phone-clock extent of the ingested stream — the only observation that // reports how long this session actually ran (the band's record timestamp // typically repeats). Used as a DURATION only; see [_liveCoverageWindow]. @@ -2027,9 +2053,7 @@ class AppState extends ChangeNotifier { _magMin.clear(); _committedRaw = 0; _liveSamples = 0; - _liveEnmoSum = 0; _lastLiveUiNotifyMs = 0; - _liveEnmoN = 0; _imuStreamSeen = false; _liveCoverStartTs = null; _liveCoverEndTs = 0; @@ -2037,15 +2061,15 @@ class AppState extends ChangeNotifier { _liveLastIngestMs = null; } - /// End-of-session: if the bout is credible walking, fold it into the personal - /// cadence calibration (persisted) so the 24/7 estimate gets more accurate. + /// End-of-session: bank the REAL 100 Hz step window into `live_coverage`. + /// + /// No cadence calibration any more — its only consumer was the deleted 1 Hz + /// `dailyStepEstimate` (see kAlgoVersion v55). Future _finalizeLivePedometer() async { // RAW, never the cushioned display value: a second short session ending // inside the first one's grace window would otherwise persist the FIRST // session's total again (double-counted coverage + a nonsense cadence). final steps = _rawSessionSteps; // gain-applied - final durS = _liveSamples / 100.0; - final enmo = _liveEnmoN > 0 ? _liveEnmoSum / _liveEnmoN : 0.0; // Derive the coverage window BEFORE resetting (it reads session counters). final window = _liveCoverageWindow(steps); if (steps > 0) { @@ -2060,9 +2084,9 @@ class AppState extends ChangeNotifier { _sessionCushionSetAtMs = DateTime.now().millisecondsSinceEpoch; } _resetLivePedometer(); - // Record the REAL 100 Hz step window (device time). The derivation pass adds - // it to the day's steps AND excludes those minutes from the 1 Hz estimate, so - // 100 Hz always wins and a minute is never counted twice. + // Record the REAL 100 Hz step window (device time). This is BAND-sourced + // coverage; the derivation reads it via `liveStepsForDay`, which prefers a + // phone count for the day when one exists and never sums the two. if (window != null) { final day = dayLabelOf( DateTime.fromMillisecondsSinceEpoch(window.startTs * 1000), @@ -2073,25 +2097,6 @@ class AppState extends ChangeNotifier { // that would otherwise let a killed-process session recover is no longer // needed. await _clearLiveSessionCheckpoint(); - if (steps <= 0 || durS < 20) return; - final cadence = steps / (durS / 60.0); - // Any nonzero AN-2554 count is CONFIRM-gated gait; confidence is high when - // the cadence lands in a walking band (else let calibrateCadence reject it). - final conf = (cadence >= 60 && cadence <= 200) ? 0.85 : 0.4; - final result = ana.PedometerResult(steps, durS, cadence, 0.0, conf); - try { - final prior = await LocalDb.getStepCalibration(); - final next = ana.calibrateCadence(prior, result, enmo); - if (next != null && !identical(next, prior)) { - await LocalDb.putStepCalibration(next); - _log( - '[steps] cadence calibrated → ' - '${next.cadenceSpm.toStringAsFixed(0)} spm (n=${next.n})', - ); - } - } catch (e) { - _log('[steps] calibration skipped: $e'); - } } // Whatever accrued via _committedRaw/_magMin between minute-commits is @@ -3465,103 +3470,15 @@ class AppState extends ChangeNotifier { _breathingEnabledStreams = false; } - // ── guided step calibration (open-road walk) ──────────────────────────────── - // A short live 100 Hz walk teaches the user's real walking signature (refEnmo) - // + cadence, which anchors the 1 Hz daily estimate. Target a step count with a - // buffer so the AN-2554 confirm-gate has settled. - static const int stepCalTargetSteps = 200; // steps to learn a stable cadence - static const int stepCalBuffer = 50; // ask the user to walk a bit more - bool _stepCalEnabledStreams = false; - bool _stepCalActive = false; // a calibration walk is in progress - - /// Begin a calibration walk: turn on the live IMU stream and count from zero. - Future startStepCalibration() async { - if (!isConnected) throw Exception('Connect to your strap first'); - // LATCH SAFELY. `_stepCalActive` is set true BEFORE the stream arming - // below, and the arming can throw (the link dropping mid-write propagates - // straight out to the UI). With no try/finally the latch stuck true for the - // rest of the process — the only reset is _endStepCalStreams(), reachable - // solely from finish/cancel, which the user never gets to because the walk - // never started. A stuck latch pins [_hasLiveConsumer] true, so - // [_maybeDowngradeLiveForBackground] never downgrades and the 100 Hz raw - // flood keeps streaming while backgrounded — exactly the R24-offload - // starvation the downgrade exists to prevent. - _stepCalActive = true; - var armed = false; - try { - // OWNERSHIP: same rule as the spot check — only claim "we enabled it" - // when live was actually OFF, so ending the walk can never turn off - // streams the open session still expects on. If the background downgrade - // left live in HR-only, upgrade to full (the walk needs the 100 Hz IMU - // stream) without taking ownership. - // - // retryFullLiveStreams (not enableLiveStreams): the walk NEEDS the 100 Hz - // IMU stream, and the sticky standard-HR fallback silently vetoes it — - // every calibration after a fallback trip counted 0 steps forever. An - // explicit user-initiated walk is exactly the moment to give the full - // flood another chance; the detectors re-trip if the radio can't cope. - if (!engine.liveEnabled) { - await engine.retryFullLiveStreams(); - _stepCalEnabledStreams = true; - } else if (engine.liveHrOnly || device.standardHrFallback) { - await engine.retryFullLiveStreams(); - } - armed = true; - } finally { - if (!armed) _stepCalActive = false; - } - _resetLivePedometer(); // count this walk from 0 - notifyListeners(); - } - - /// Finish the calibration walk: fold the live bout into the personal cadence - /// model (refEnmo + cadence). Returns the learned cadence (spm), or null if the - /// walk wasn't credible. Stops the stream we turned on. - Future finishStepCalibration() async { - final steps = _rawSessionSteps; // raw, never the display cushion - final durS = _liveSamples / 100.0; - final enmo = _liveEnmoN > 0 ? _liveEnmoSum / _liveEnmoN : 0.0; - double? learned; - if (steps > 0 && durS >= 20) { - final cadence = steps / (durS / 60.0); - final conf = (cadence >= 60 && cadence <= 200) ? 0.9 : 0.4; - final result = ana.PedometerResult(steps, durS, cadence, 0.0, conf); - try { - final prior = await LocalDb.getStepCalibration(); - final next = ana.calibrateCadence(prior, result, enmo); - if (next != null) { - await LocalDb.putStepCalibration(next); - learned = next.cadenceSpm; - _log( - '[steps] CALIBRATED → ${next.cadenceSpm.toStringAsFixed(0)} spm ' - '(refEnmo=${next.refEnmo.toStringAsFixed(3)}, n=${next.n})', - ); - } - } catch (e) { - _log('[steps] calibration failed: $e'); - } - } - _endStepCalStreams(); - _resetLivePedometer(); - notifyListeners(); - return learned; - } - - /// Cancel a calibration walk without saving. - void cancelStepCalibration() { - _endStepCalStreams(); - _resetLivePedometer(); - notifyListeners(); - } - - /// Release the streams a calibration walk armed — ONLY if we armed them. - void _endStepCalStreams() { - _stepCalActive = false; - if (_stepCalEnabledStreams && activeWorkout == null) { - unawaited(engine.disableLiveStreams()); - } - _stepCalEnabledStreams = false; - } + // GUIDED STEP CALIBRATION REMOVED (v56). + // + // A short live walk used to teach a personal `refEnmo` + cadence, which was + // consumed by ONE caller: the 1 Hz `dailyStepEstimate`. That estimator is + // gone (1 Hz cannot resolve gait — see the kAlgoVersion v55 note), so the + // calibration had no reader left. It kept a "Calibrate steps" row on the + // Steps screen that told the user their walk had taught the app something + // when nothing read the result. The Tier-A 100 Hz AN-2554 pedometer is + // threshold-based and never needed it. // ── live session coach ─────────────────────────────────────────────────────── LiveWorkoutState? activeWorkout; diff --git a/lib/ui/profile/profile_screen.dart b/lib/ui/profile/profile_screen.dart index fdb4b2c..ad9a06c 100644 --- a/lib/ui/profile/profile_screen.dart +++ b/lib/ui/profile/profile_screen.dart @@ -1124,6 +1124,17 @@ class _HealthSection extends StatelessWidget { 'Stays on your device.', style: AppText.captionMuted, ), + // The failure mode this exists for: on iOS the permission prompt + // reports success even when the user denies READ access, so + // without a status line the toggle just sits on and no step count + // ever appears, with nothing for the user to act on. + if (app.phoneStepsEnabled) ...[ + const SizedBox(height: 2), + Text( + _phoneStepsStatus(app, store), + style: AppText.captionMuted, + ), + ], ], ), ), @@ -1147,6 +1158,23 @@ class _HealthSection extends StatelessWidget { ); } + /// One line telling the user whether the read is actually producing anything. + static String _phoneStepsStatus(AppState app, String store) { + final days = app.phoneStepsLastSyncedDays; + if (days == null) return 'Reading…'; + if (days == 0) { + return 'No data from $store yet. If you never saw a permission prompt, ' + 'allow Steps for OpenStrap in $store settings.'; + } + final total = app.phoneStepsLastTotal ?? 0; + if (total == 0) { + return 'Connected to $store — no steps recorded in the last $days day' + '${days == 1 ? '' : 's'}.'; + } + return 'Read $total steps from $store over $days day' + '${days == 1 ? '' : 's'}.'; + } + Widget _statusRow(BuildContext context, HealthLinkState st, String store) { final messenger = ScaffoldMessenger.of(context); // Health Connect must be installed/updated first (Android). diff --git a/lib/ui/screens/metric_row.dart b/lib/ui/screens/metric_row.dart index 25e30a2..f14666d 100644 --- a/lib/ui/screens/metric_row.dart +++ b/lib/ui/screens/metric_row.dart @@ -24,7 +24,7 @@ const Map kMetricInfo = { 'load': 'Recent (7d) vs habitual (28d) load. 0.8–1.3 is the sweet spot.', 'fitness': 'Direction of your fitness from resting-HR and recovery trends.', 'calories': 'Active energy burned, estimated from your heart rate.', - 'steps': 'Estimated steps from wrist motion.', + 'steps': 'Real steps counted by your phone or the band\'s live sensor.', 'sleep': 'Time actually asleep last night.', 'efficiency': 'Share of time in bed actually spent asleep.', 'regularity': 'How consistent your sleep timing is, 0–100.', diff --git a/lib/ui/screens/screens.dart b/lib/ui/screens/screens.dart index 07e31a7..fd2481f 100644 --- a/lib/ui/screens/screens.dart +++ b/lib/ui/screens/screens.dart @@ -24,7 +24,6 @@ import '../heart/live_hr_tile.dart'; import '../insights/coach_cards.dart'; import '../sleep/sleep_detail_screen.dart'; import '../spotcheck/spot_check_screen.dart'; -import '../today/step_calibration_screen.dart'; import '../today/step_goal_screen.dart'; import 'detail_cards.dart'; import 'metric_screen.dart'; @@ -295,9 +294,6 @@ class _ActivityDetailState extends State<_ActivityDetail> { onSetGoal: () => Navigator.of(context).push( MaterialPageRoute(builder: (_) => StepGoalScreen(goal: goal)), ), - onCalibrate: () => Navigator.of(context).push( - MaterialPageRoute(builder: (_) => const StepCalibrationScreen()), - ), ); } } @@ -311,7 +307,6 @@ class StepsDayContent extends StatelessWidget { final List weekValues; // raw step counts (nulls = no data) final List weekLabels; final VoidCallback? onSetGoal; - final VoidCallback? onCalibrate; const StepsDayContent({ super.key, @@ -320,7 +315,6 @@ class StepsDayContent extends StatelessWidget { this.weekValues = const [], this.weekLabels = const [], this.onSetGoal, - this.onCalibrate, }); @override @@ -360,17 +354,22 @@ class StepsDayContent extends StatelessWidget { trailing: Row( mainAxisSize: MainAxisSize.min, children: [ - Tag('est', color: accent), + Tag('measured', color: accent), InfoDot( title: 'How steps are counted', body: - 'While the band streams live (a workout or with the ' - 'app open) we count REAL steps from its 100 Hz motion ' - 'sensor. The rest of the day the sensor samples too ' - 'slowly to count each step, so those hours are ' - 'ESTIMATED from your walking minutes and cadence.', + 'Only by something that can actually see your gait: ' + 'your phone\'s own pedometer, or the band\'s 100 Hz ' + 'sensor while it streams live (a workout, or with the ' + 'app open). We never add the two together — they are ' + 'the same walk seen from your pocket and your wrist.\n\n' + 'The rest of the day the band samples once a second, ' + 'which is too slow to resolve individual steps. Those ' + 'hours are left uncounted rather than estimated, so a ' + 'day with no real measurement shows no number at all.', methodNote: - 'Walk with the app open to sharpen the estimate.', + 'Turn on “Use phone step count” in Profile → Health ' + 'for all-day steps.', ), ], ), @@ -382,7 +381,10 @@ class StepsDayContent extends StatelessWidget { Expanded( child: BigStat( value: steps > 0 ? '$steps' : null, - caption: steps > 0 ? 'goal $g' : 'no steps yet', + // NOT "no steps yet" — absent means nothing that can + // resolve gait measured this day, which is a different + // statement from "you took zero steps". + caption: steps > 0 ? 'goal $g' : 'not measured', size: BigStatSize.xl, ), ), @@ -425,7 +427,7 @@ class StepsDayContent extends StatelessWidget { ).dsEnter(index: 1), ], - // ── goal + calibration ─────────────────────────────────────────────── + // ── goal ─────────────────────────────────────────────── const SizedBox(height: Sp.x3), SurfaceCard( padding: const EdgeInsets.symmetric( @@ -439,16 +441,8 @@ class StepsDayContent extends StatelessWidget { iconColor: accent, title: 'Daily step goal', value: goal == null ? 'Set' : '$goal', - divider: true, onTap: onSetGoal, ), - ListRow( - icon: OsIcon.run, - iconColor: accent, - title: 'Calibrate steps', - subtitle: 'Walk ~250 steps with the app open', - onTap: onCalibrate, - ), ], ), ).dsEnter(index: 2), diff --git a/lib/ui/today/step_calibration_screen.dart b/lib/ui/today/step_calibration_screen.dart deleted file mode 100644 index 38bdd07..0000000 --- a/lib/ui/today/step_calibration_screen.dart +++ /dev/null @@ -1,232 +0,0 @@ -// Step calibration — a short guided open-road walk that teaches the band YOUR -// walking signature (real 100 Hz pedometer → personal cadence + refEnmo). Once -// calibrated, the 1 Hz all-day step estimate is anchored to you instead of a -// guess. 1 Hz can't count steps directly (Nyquist); this is what makes the -// estimate trustworthy. -// -// Presentation: design-system language (ArcGauge progress, StateCard-style -// finish, themed CTA). The calibration start/finish/cancel logic is untouched. - -import 'package:flutter/material.dart'; -import 'package:provider/provider.dart'; - -import '../../state/app_state.dart'; -import '../design/design.dart'; - -class StepCalibrationScreen extends StatefulWidget { - const StepCalibrationScreen({super.key}); - @override - State createState() => _StepCalibrationScreenState(); -} - -class _StepCalibrationScreenState extends State { - // walk target = base + buffer so the AN-2554 confirm-gate settles. - final int _target = - AppState.stepCalTargetSteps + AppState.stepCalBuffer; // e.g. 250 - bool _started = false; - bool _saving = false; - double? _learnedCadence; - String? _error; - - late final AppState _appState; - - @override - void initState() { - super.initState(); - _appState = context.read(); - WidgetsBinding.instance.addPostFrameCallback((_) => _start()); - } - - Future _start() async { - if (!mounted) return; - try { - await context.read().startStepCalibration(); - if (mounted) setState(() => _started = true); - } catch (e) { - if (mounted) setState(() => _error = e.toString()); - } - } - - Future _save() async { - setState(() => _saving = true); - final cadence = await context.read().finishStepCalibration(); - if (!mounted) return; - if (cadence == null) { - // used to just silently fall through here - gauge would reset with - // nothing telling the user their walk didn't save. reusing the same - // _error/StateCard the start-failure path already has, "try again" - // re-arms a fresh walk which is the right recovery either way. - setState(() { - _saving = false; - _error = "That walk wasn't steady enough to learn from — try again " - 'on flatter, less crowded ground.'; - }); - return; - } - setState(() { - _saving = false; - _learnedCadence = cadence; - }); - } - - @override - void dispose() { - // If we leave without saving, stop the stream + drop the partial walk. - if (_learnedCadence == null) { - _appState.cancelStepCalibration(); - } - super.dispose(); - } - - @override - Widget build(BuildContext context) { - final steps = context.select((a) => a.liveSteps); - // The standard-HR radio fallback suppresses the 100 Hz IMU stream this - // walk counts on. startStepCalibration clears it and retries; if it - // TRIPS AGAIN mid-walk the radio genuinely can't sustain the stream — - // say so instead of showing "Keep walking…" over a count of 0 forever. - final radioDegraded = - context.select((a) => a.device.standardHrFallback); - final done = _learnedCadence != null; - final t = (_target > 0 ? steps / _target : 0.0).clamp(0.0, 1.0).toDouble(); - final ready = steps >= _target; - - return AppScaffold( - title: 'Calibrate steps', - subtitle: 'A short walk teaches your stride', - actions: [ - const InfoDot( - title: 'Why calibrate', - body: - 'A brief walk with the app open lets the band\'s real pedometer ' - 'learn your personal cadence, which anchors the all-day step ' - 'estimate to you.', - bullets: [ - 'Walk on flat, open ground at your normal pace.', - 'Keep the phone on you and the app open.', - 'Avoid stairs, crowds and stops.', - ], - ), - ], - children: [ - if (_error != null) - StateCard( - icon: OsIcon.run, - title: "Couldn't start calibration", - message: _error!, - actionLabel: 'Try again', - onAction: () { - setState(() => _error = null); - _start(); - }, - ) - else if (done) - _doneCard() - else ...[ - const SizedBox(height: Sp.x4), - Center( - child: RepaintBoundary( - child: ArcGauge( - value: t, - color: DomainAccent.steps, - size: 200, - stroke: 16, - sweepFraction: 0.75, - animate: false, // live-driven — no reveal sweep fighting updates - center: Column(mainAxisSize: MainAxisSize.min, children: [ - Text('$steps', style: AppText.metric.copyWith(fontSize: 44)), - const SizedBox(height: 2), - Text('OF $_target', - style: - AppText.overline.copyWith(color: AppColors.inkMuted)), - ]), - ), - ), - ).dsEnter(), - const SizedBox(height: Sp.x3), - Center( - child: ready - ? const StatusChip('Ready to save', tone: ChipTone.positive) - : Text(_started ? 'Keep walking…' : 'Starting…', - style: AppText.label.copyWith(color: AppColors.inkSoft)), - ), - if (radioDegraded && _started && !ready) ...[ - const SizedBox(height: Sp.x4), - StateCard( - icon: OsIcon.bluetooth, - title: "Bluetooth can't keep up", - message: - 'The connection to your strap is struggling to carry the ' - 'high-rate motion stream, so steps aren\'t coming through. ' - 'Bring your phone closer to the strap and retry.', - actionLabel: 'Retry stream', - onAction: _start, - ), - ], - const SizedBox(height: Sp.x6), - SurfaceCard( - entranceIndex: 1, - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - const TileHeader('How to calibrate'), - const SizedBox(height: Sp.x3), - Text( - 'Walk on flat, open ground at your normal pace with the ' - 'app open. We count ~$_target real steps to learn your ' - 'stride and cadence.', - style: AppText.bodySoft), - ]), - ), - const SizedBox(height: Sp.x6), - SizedBox( - width: double.infinity, - child: FilledButton( - onPressed: ready && !_saving ? _save : null, - child: _saving - ? const SizedBox( - width: 22, - height: 22, - child: CircularProgressIndicator( - strokeWidth: 2.4, color: Colors.white)) - : const Text('Save calibration'), - ), - ), - ], - ], - ); - } - - Widget _doneCard() => SurfaceCard( - level: 2, - child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ - Row(children: [ - Container( - padding: const EdgeInsets.all(Sp.x3), - decoration: BoxDecoration( - color: AppColors.positiveSoft, - shape: BoxShape.circle, - ), - child: AppIcon(OsIcon.check, size: 22, color: AppColors.positive), - ), - const SizedBox(width: Sp.x3), - Text('Calibrated', style: AppText.h2), - ]), - const SizedBox(height: Sp.x4), - BigStat( - value: _learnedCadence!.toStringAsFixed(0), - unit: 'steps/min', - label: 'Your cadence', - caption: 'Sharper every time you walk with the app open', - ), - const SizedBox(height: Sp.x5), - SizedBox( - width: double.infinity, - child: FilledButton( - onPressed: () => Navigator.of(context).maybePop(), - child: const Text('Done'), - ), - ), - ]), - ).dsCelebrate(); -} diff --git a/test/app_state_regressions_test.dart b/test/app_state_regressions_test.dart index 395505e..c94b5d4 100644 --- a/test/app_state_regressions_test.dart +++ b/test/app_state_regressions_test.dart @@ -10,29 +10,12 @@ import 'package:path/path.dart' as p; import 'package:shared_preferences/shared_preferences.dart'; import 'package:sqflite_common_ffi/sqflite_ffi.dart'; -import 'package:openstrap_edge/ble/ble_engine.dart'; import 'package:openstrap_edge/data/db.dart'; import 'package:openstrap_edge/notify/notification_center.dart'; import 'package:openstrap_edge/notify/notification_event.dart'; import 'package:openstrap_edge/state/app_state.dart'; import 'package:openstrap_edge/sync/paired_device.dart'; -/// A BleEngine whose live-stream arming always fails — the "link dropped -/// mid-write" case that used to latch _stepCalActive true forever. -class _ThrowingEngine extends BleEngine { - _ThrowingEngine() - : super( - onRecord: _noRecord, - onState: _noState, - ); - static Future _noRecord(Object? sample, Object? raw) async {} - static void _noState(Object state) {} - - @override - Future retryFullLiveStreams() async => - throw StateError('link dropped mid-write'); -} - void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -95,24 +78,11 @@ void main() { }); }); - // ── 5. _stepCalActive must not latch true when the arming throws ──────────── - group('startStepCalibration (live-consumer latch)', () { - test('a throwing stream arm leaves no phantom live consumer', () async { - final engine = _ThrowingEngine(); - engine.state.connection = 'connected'; - final app = AppState.forTesting(engine: engine); - addTearDown(app.dispose); - - expect(app.debugHasLiveConsumer, isFalse); - await expectLater( - app.startStepCalibration(), throwsA(isA())); - // Pre-fix this stayed true for the rest of the process, pinning - // _hasLiveConsumer and permanently disabling - // _maybeDowngradeLiveForBackground — the 100 Hz raw flood then kept - // streaming while backgrounded and starved the R24 offload. - expect(app.debugHasLiveConsumer, isFalse); - }); - }); + // ── 5. (removed) the step-calibration live-consumer latch ───────────────── + // The guided calibration walk was deleted in v56 along with the 1 Hz step + // estimator that was its only consumer, so there is no longer an arming path + // that can latch `_hasLiveConsumer`. The spot-check and workout consumers + // keep their own latch coverage. // ── 6. `busy` must not latch true forever ────────────────────────────────── group('openSession (busy latch)', () { diff --git a/test/metric_trend_redesign_test.dart b/test/metric_trend_redesign_test.dart index 355bb27..68a8dab 100644 --- a/test/metric_trend_redesign_test.dart +++ b/test/metric_trend_redesign_test.dart @@ -386,7 +386,7 @@ void main() { ) async { _phone(t, height: 2200); for (final p in [kLightPalette, kDarkPalette]) { - var goals = 0, cals = 0; + var goals = 0; await t.pumpWidget( _host( StepsDayContent( @@ -395,7 +395,6 @@ void main() { weekValues: const [9000, 12000, null, 4000, 8000, 10000, 8412], weekLabels: const ['M', 'T', 'W', 'T', 'F', 'S', 'S'], onSetGoal: () => goals++, - onCalibrate: () => cals++, ), palette: p, ), @@ -405,12 +404,12 @@ void main() { expect(find.text('goal 10000'), findsOneWidget); expect(find.text('84%'), findsOneWidget); // of goal gauge expect(find.text('THIS WEEK'), findsOneWidget); - expect(find.text('EST'), findsOneWidget); // honesty tag + // Honesty tag: steps are real-measured only now, never estimated. + expect(find.text('MEASURED'), findsOneWidget); + expect(find.text('Calibrate steps'), findsNothing); await t.tap(find.text('Daily step goal')); - await t.tap(find.text('Calibrate steps')); await t.pump(const Duration(milliseconds: 300)); expect(goals, 1); - expect(cals, 1); expect(t.takeException(), isNull); } }); diff --git a/test/movement_floor_policy_test.dart b/test/movement_floor_policy_test.dart new file mode 100644 index 0000000..f3fb334 --- /dev/null +++ b/test/movement_floor_policy_test.dart @@ -0,0 +1,118 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/compute/movement_floor_policy.dart'; + +/// The frozen movement floor is ONE shared scalar that every day of a derive +/// sweep reads and can write. `DerivationEngine.run()` dispatches days +/// NEWEST-FIRST through a concurrent worker pool, and the v56 bump forces the +/// whole retained window to re-derive at once — so these decisions must be +/// order-independent, or sweep order silently decides every day's `active_min`. +void main() { + group('dayLabelBefore — calendar, never Duration', () { + test('does not skip the spring-forward day', () { + // 2026-03-08 is 23 h long in a US timezone. `subtract(Duration(days: 2))` + // from local midnight on 03-10 lands at 23:00 on 03-07, so the walk-back + // NEVER GENERATES 2026-03-08 and the gap is counted against the wrong + // days. Calendar-field construction cannot do this. + expect(dayLabelBefore('2026-03-10', 1), '2026-03-09'); + expect(dayLabelBefore('2026-03-10', 2), '2026-03-08'); + expect(dayLabelBefore('2026-03-10', 3), '2026-03-07'); + }); + + test('crosses month and year boundaries', () { + expect(dayLabelBefore('2026-03-01', 1), '2026-02-28'); + expect(dayLabelBefore('2026-01-01', 1), '2025-12-31'); + expect(dayLabelBefore('2024-03-01', 1), '2024-02-29'); // leap year + }); + + test('an unparseable label yields null rather than a wrong date', () { + expect(dayLabelBefore('not-a-date', 1), isNull); + }); + }); + + group('wearGapDays', () { + test('no gap when yesterday has data', () { + expect( + wearGapDays(have: {'2026-03-09', '2026-03-08'}, dayId: '2026-03-10'), + 0, + ); + }); + + test('counts the consecutive run of missing days', () { + expect( + wearGapDays(have: {'2026-03-05'}, dayId: '2026-03-10'), + 4, // 03-09, 03-08, 03-07, 03-06 missing; 03-05 present -> stop + ); + }); + + test('spans a DST transition without miscounting', () { + // 03-08 is present, so the gap is exactly one day (03-09). The Duration + // walk-back skipped 03-08 entirely and reported a longer gap here. + expect( + wearGapDays(have: {'2026-03-08'}, dayId: '2026-03-10'), + 1, + ); + }); + + test('an EMPTY history is no information, not a 60-day gap', () { + // A brand-new install must not trip the >=30-day re-freeze rule purely + // because it has no history yet. + expect(wearGapDays(have: const {}, dayId: '2026-03-10'), 0); + }); + + test('is bounded by maxScan', () { + expect( + wearGapDays(have: {'2020-01-01'}, dayId: '2026-03-10', maxScan: 12), + 12, + ); + }); + }); + + group('daysSinceFrozen — never negative', () { + test('a later day reports real age', () { + expect(daysSinceFrozen(frozenOn: '2026-03-01', dayId: '2026-03-11'), 10); + }); + + test('a BACKFILL day is age 0, not its absolute distance', () { + // This is the bug the clamp fixes. With `.abs()`, re-deriving a day from + // more than `maxAgeDays` before the freeze read as maximally stale and + // re-froze the shared floor onto an OLDER frozenOn — during a + // newest-first sweep, i.e. on every kAlgoVersion bump. + expect(daysSinceFrozen(frozenOn: '2026-03-01', dayId: '2024-01-01'), 0); + expect(daysSinceFrozen(frozenOn: '2026-03-01', dayId: '2026-02-28'), 0); + }); + + test('same day is 0', () { + expect(daysSinceFrozen(frozenOn: '2026-03-01', dayId: '2026-03-01'), 0); + }); + }); + + group('mayCommitFloorOn — the floor only moves forward', () { + test('nothing frozen yet: any day may establish it', () { + expect(mayCommitFloorOn(frozenOn: null, dayId: '2026-03-01'), isTrue); + }); + + test('a newer day may re-freeze', () { + expect( + mayCommitFloorOn(frozenOn: '2026-03-01', dayId: '2026-03-02'), + isTrue, + ); + }); + + test('an OLDER day may consume but never move the floor', () { + // Otherwise the oldest day of a newest-first concurrent sweep could + // clobber the freeze the newest day just established, making every day's + // active_min depend on which worker finished last. + expect( + mayCommitFloorOn(frozenOn: '2026-03-10', dayId: '2026-03-01'), + isFalse, + ); + }); + + test('re-freezing on the same day is allowed', () { + expect( + mayCommitFloorOn(frozenOn: '2026-03-10', dayId: '2026-03-10'), + isTrue, + ); + }); + }); +} diff --git a/test/phone_pedometer_hour_walk_test.dart b/test/phone_pedometer_hour_walk_test.dart new file mode 100644 index 0000000..f0607a9 --- /dev/null +++ b/test/phone_pedometer_hour_walk_test.dart @@ -0,0 +1,148 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/data/db.dart'; +import 'package:openstrap_edge/health/phone_pedometer.dart'; +import 'package:path/path.dart' as p; +import 'package:sqflite_common_ffi/sqflite_ffi.dart'; + +/// The hour walk is where this feature's two real defects lived, and neither +/// was reachable from the DB-level tests. +/// +/// NOTE ON TIMEZONE. Dart reads the process timezone from the environment and +/// `flutter test` cannot set it per-test, so the DST cases here assert on the +/// BOUNDARY LOGIC (zero-width buckets are skipped, not fatal) in a way that +/// holds in every timezone, rather than hard-coding a US transition. The +/// spring-forward instant collapse itself was reproduced directly against the +/// Dart runtime under `TZ=America/New_York` while diagnosing: +/// +/// h=1 from=2026-03-08 01:00 to=2026-03-08 03:00 +/// h=2 from=2026-03-08 03:00 to=2026-03-08 03:00 <-- zero width +/// +/// With `break` there, hours 3-23 were never queried and the day was persisted +/// with ~3 hours of windows. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUpAll(() async { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + LocalDb.dbName = 'openstrap_phone_hour_walk_test.db'; + final dir = await databaseFactory.getDatabasesPath(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + }); + + setUp(() async { + final db = await LocalDb.instance; + await db.delete('live_coverage'); + }); + + /// Yesterday, so the walk covers a whole elapsed day (no "future hours" cap). + DateTime yesterday() { + final n = DateTime.now(); + return DateTime(n.year, n.month, n.day - 1); + } + + test('a full elapsed day walks every hour and banks the total', () async { + final asked = []; + final ped = PhonePedometer(stepReader: (from, to) async { + asked.add(from); + return 10; + }); + + final day = yesterday(); + final total = await ped.syncDay(day); + + // 24 buckets in a normal day (23 or 25 across a DST transition) — never + // truncated to a handful. + expect(asked.length, greaterThanOrEqualTo(23)); + expect(total, asked.length * 10); + expect(await LocalDb.liveStepsForDay(_label(day)), total); + }); + + test('a zero-width bucket is SKIPPED, not fatal to the rest of the day', + () async { + // Simulates the spring-forward collapse: the walk must keep going past a + // bucket whose `from == to`. We cannot force a real DST gap in-process, so + // this asserts the invariant directly — every hour after the anomaly is + // still queried. + var calls = 0; + final ped = PhonePedometer(stepReader: (from, to) async { + calls++; + // A zero-width interval would never reach the reader at all (it is + // skipped before the call), so simply counting calls proves the walk + // did not terminate early. + return 1; + }); + + final total = await ped.syncDay(yesterday()); + expect(calls, greaterThanOrEqualTo(23)); + expect(total, calls); + }); + + test('ANY failed hour abandons the day rather than banking a partial one', + () async { + final day = yesterday(); + final dayId = _label(day); + + // 1. A complete, good sync. + final good = PhonePedometer(stepReader: (from, to) async => 100); + final fullTotal = await good.syncDay(day); + expect(fullTotal, isNotNull); + expect(await LocalDb.liveStepsForDay(dayId), fullTotal); + + // 2. A later sync where hour 5 fails. `null` from this plugin means the + // query FAILED (an empty hour returns 0 on both platforms), so the day + // must be abandoned — `replacePhoneCoverageForDay` is delete-then- + // insert, and banking the short read would LOWER a good previous total + // while still suppressing the band fallback. + var h = 0; + final flaky = PhonePedometer(stepReader: (from, to) async { + final n = h++ == 5 ? null : 100; + return n; + }); + expect(await flaky.syncDay(day), isNull); + + // 3. The good total survives untouched. + expect(await LocalDb.liveStepsForDay(dayId), fullTotal); + }); + + test('a genuine zero-step day banks nothing and falls back to the band', + () async { + final day = yesterday(); + final dayId = _label(day); + await LocalDb.addLiveCoverage( + day.millisecondsSinceEpoch ~/ 1000, + day.millisecondsSinceEpoch ~/ 1000 + 600, + 777, + dayId, + ); + + // Every hour reads successfully as 0 — a real sedentary day, NOT a failure. + final ped = PhonePedometer(stepReader: (from, to) async => 0); + expect(await ped.syncDay(day), 0); + + // No phone rows were written, so the band count still shows. + expect(await LocalDb.liveStepsForDay(dayId), 777); + }); + + test('the routine sync window is much smaller than the backfill window', () { + // Each hourly bucket is one platform round trip, so the window IS the cost: + // the 7-day default was up to 168 sequential calls on every launch and + // again after every export. + expect(PhonePedometer.routineSyncDays, + lessThan(PhonePedometer.fullSyncDays)); + expect(PhonePedometer.routineSyncDays, 2); + }); + + test('syncRecent reports days read and their total for the UI', () async { + final ped = PhonePedometer(stepReader: (from, to) async => 5); + final r = await ped.syncRecent(days: 2); + // Today is partial (only elapsed hours), yesterday is whole — both read. + expect(r.daysRead, 2); + expect(r.totalSteps, greaterThan(0)); + }); +} + +String _label(DateTime d) => + '${d.year.toString().padLeft(4, '0')}-' + '${d.month.toString().padLeft(2, '0')}-' + '${d.day.toString().padLeft(2, '0')}'; From f8046591cc8a65c09b539ecb98de4ff906dab7a0 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 5 Aug 2026 02:08:00 +0530 Subject: [PATCH 4/6] Close the bot findings I had not actually read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two categories were missed on the first pass: CodeRabbit's "outside diff range" findings live in the review BODY, not the inline-comments API I queried, and I never opened PR Agent's suggestions at all. Both checked against real code now. * The copy-back comment still described "the hybrid real-100Hz + 1Hz-estimate count". I fixed the identical stale wording in the `series:` map last commit and missed this second site, which CodeRabbit had explicitly listed. Rewritten to say what the code does: real pedometer counts only, copied into the early-read `wake` artifact so Today does not show a blank on a day that WAS measured. * `syncDay` captured `DateTime.now()` once before the hour walk. Each bucket is an async platform query, so a full day's walk can straddle an hour boundary and the stale `now` capped the current hour short, under-reporting today's most recent steps until a later sync re-read the day. Re-read per iteration. Already covered, verified rather than assumed: - PR Agent's #1 (backfill regresses the frozen floor, importance 8) is the same defect as CodeRabbit's and my own; `mayCommitFloorOn` is literally the guard it proposed. - Its future-`frozenOn` clock-skew case is closed by the same clamp — a negative difference now returns 0 instead of a large `abs()`. - Its wear-gap DST case is closed by `dayLabelBefore`. - CodeRabbit's outside-diff "thin band substrate drops measured phone steps" was already fixed by the author at 87a8a5d: `_writeSteps` runs before the `daySub.length < 60` guard. NOT fixed, deliberately: both bots flag the analytics pin pointing at an unmerged branch head. That is the documented merge-order dependency — it gets re-pinned to the analytics main SHA after analytics#35 merges, not now. analyze clean, 1140 tests green. --- lib/compute/derivation_engine.dart | 16 +++++++++------- lib/health/phone_pedometer.dart | 7 ++++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 6fa393c..f7cf74f 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -4203,13 +4203,15 @@ class DerivationEngine { inp.dynFloorG, inp.dynHistoryDays, ); - // _stepsAndEnergy just corrected `steps`/`calories_total` in bundlePatch + - // scMap using the hybrid real-100Hz + 1Hz-estimate count, but `wake` (built - // above by _buildWakeDayFeatures, before this correction ran) still holds - // the earlier 1Hz-only estimate. `wake` is what _persistWakeDayFeatures - // stores and what the Today repository reads while the full day result - // isn't ready yet, so copy the corrected values back in to avoid serving - // stale steps/calories from that early-read path. + // _stepsAndEnergy just wrote `steps` (REAL pedometer counts from + // `live_coverage` — band 100 Hz or phone, never an estimate) and + // `calories_total` into bundlePatch + scMap. `wake` was built above by + // _buildWakeDayFeatures BEFORE that ran, and deliberately leaves `steps` + // null: the early-read path has no gait-capable source of its own and must + // not invent one. `wake` is what _persistWakeDayFeatures stores and what + // the Today repository reads until the full day result exists, so copy the + // measured values back in — otherwise Today shows no step count on a day + // that really was measured. for (final key in const ['steps', 'calories_total']) { final value = scMap[key]; if (value != null) wake[key] = value; diff --git a/lib/health/phone_pedometer.dart b/lib/health/phone_pedometer.dart index b84b59f..dbd9f0c 100644 --- a/lib/health/phone_pedometer.dart +++ b/lib/health/phone_pedometer.dart @@ -135,7 +135,6 @@ class PhonePedometer { final windows = <({int startTs, int endTs, int steps})>[]; var total = 0; var anyRead = false; - final now = DateTime.now(); // CALENDAR-AWARE hour walk. `Duration` arithmetic on a local DateTime is // ABSOLUTE, so `dayStartLocal.add(Duration(hours: h))` over a fixed 24 @@ -150,6 +149,12 @@ class PhonePedometer { dayStartLocal.day + 1, ); for (var h = 0; h < 25; h++) { + // RE-READ THE CLOCK EACH ITERATION. Each bucket is an async platform + // query, so a whole day's walk can straddle an hour boundary. Captured + // once up front, `now` went stale mid-loop and the current hour was + // capped short — under-reporting today's most recent steps until some + // later sync happened to re-read the day. + final now = DateTime.now(); final from = DateTime(dayStartLocal.year, dayStartLocal.month, dayStartLocal.day, h); if (!from.isBefore(nextMidnight)) break; // spring-forward short day From abe614ff504b4585f0364f62addbf8cb5ef2a75f Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 5 Aug 2026 07:50:32 +0530 Subject: [PATCH 5/6] Make the backfill floor guard's reachability obvious MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No behaviour change. PR Agent read the `mayCommitFloorOn` guard as dead code (claiming the `stored != null` branch always returns early), which is wrong — the branch falls through when a re-freeze IS required and history IS long enough, which is exactly the backfill-clobber case. But two readers misreading it is signal, so the guard now sits inside that branch against a non-nullable `stored.frozenOn`, with the reachable path named in the comment. The other three suggestions on f804659 are refuted, each empirically: * "Async lock swallows action errors" — it does not. `whenComplete` preserves the error for the caller; verified by running both shapes. The proposed `completeError` fix is an ACTIVE regression: the gate future then carries the error, so the next waiter's `prev.then((_) => action())` skips its action entirely and fails with the PREVIOUS day's error, plus an unhandled exception. One day's floor failure would cascade through the whole sweep. Releasing the gate normally while propagating the error to its own caller is the correct mutex contract. * "Fall-back day loses its final hour, raise the bound to 26" — a fall-back day has 25 real hours but only 24 local hour LABELS. The repeat is absorbed by the h=1 bucket, which spans 2 real hours. Measured under TZ=America/New_York: the walk covers 25h of a 25h day and h=24 breaks because the day is already complete. A bound of 26 changes nothing. * "Absent metric key incorrectly asserted present" — the assertion is load-bearing and must stay. `putDayResult` always includes 'steps' in its series map, so a null value writes a NULL row under REPLACE and OVERWRITES a previously fabricated value. Drop the key and the stale v54 number survives. The null does not read as zero: `metricValueOn` returns null (asserted on the next line), trend buckets gate on `has`, and no query coalesces it to 0. analyze clean, 1140 tests green. --- lib/compute/derivation_engine.dart | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index f7cf74f..2249b36 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -3140,6 +3140,16 @@ class DerivationEngine { // `dyn_p90` history was pruned or is sparse). Keep serving the existing // floor until a replacement can actually be computed. if (hist.length < ana.enrollmentDaysForFrozenFloor) return stored.floorG; + + // REACHABLE, and this is the case it exists for: an OLD backfill day that + // trips the re-freeze rule (a 30-day wear gap before it is the common + // one) and has enough prior history to recompute. Without this it would + // overwrite the freeze a NEWER day just established, and since the sweep + // runs newest-first and concurrently, sweep order would decide the floor. + // A backfill day may CONSUME the shared floor; it may never move it. + if (!mfp.mayCommitFloorOn(frozenOn: stored.frozenOn, dayId: dayId)) { + return stored.floorG; + } } else if (hist.length < ana.enrollmentDaysForFrozenFloor) { // Still enrolling, and nothing stored to fall back on. Return null so the // metric abstains and says so, rather than shipping a threshold we have @@ -3147,13 +3157,6 @@ class DerivationEngine { return null; } - // A backfill day may CONSUME the shared floor but never move it — otherwise - // a newest-first sweep's oldest day could clobber the freeze its newest day - // just established. - if (!mfp.mayCommitFloorOn(frozenOn: stored?.frozenOn, dayId: dayId)) { - return stored?.floorG; - } - final floor = ana.personalDynFloorFromDailySummaries(hist); if (floor == null) return stored?.floorG; await LocalDb.putMovementFloor( From f97e27839ddcdc99d8cfc5dc1505229b3ee5d323 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 5 Aug 2026 23:27:58 +0530 Subject: [PATCH 6/6] Stop an all-zero phone read erasing a real day; unlabel absent steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from the latest bot round, each checked against the code before touching it. The one with teeth: an all-zero phone read wiped a day that already held real counts. `replacePhoneCoverageForDay` is delete-then-insert, and this file already documents the failure that produces exactly that read — on iOS `requestAuthorization` reports success even when the user denied READ, so queries return EMPTY rather than null, forever. One sync after that and a multi-thousand-step day was gone, without even the band fallback, since phone rows win outright. Now an all-zero read over a day that already holds phone steps keeps the banked day and reports the day as unread, which is what it is. Mutation-verified: the new test fails 137 -> 0 with the guard stubbed out. The absent `steps` block carried `tier: 'ESTIMATE'`, and `Metric.parse` turns that tier into `beta: true` — so a day nothing measured rendered the estimate badge, on a card with no number on it. `tier: null` and an empty `inputs_used` instead. `ABSENT` was deliberately not invented as a fifth tier: `Tier.all` in analytics is a closed set of four published grades and the edge must not widen it. kAlgoVersion 56 -> 57, since the persisted bundle changes even though no value does. `requestPhoneSteps`/`disablePhoneSteps` set the in-memory flag before awaiting the pref write, so a failed write left the toggle ON for the run and OFF on the next launch — banking phone rows the restored state says were never enabled, which nothing then clears. Persist first. The `containsKey('steps')` assertion in derive_day_window_test was vacuous: `got` is built by the test's own helper, which seeds every key unconditionally, so it could not fail whatever the derivation did. Replaced with real assertions on the persisted bundle. Not changed, deliberately: * `_AsyncLock` "swallows the error". `whenComplete` DOES release the lock on the error path and the error still propagates to the caller. The suggested `completeError` would be a real bug: `_tail` would then carry an error, and the NEXT waiter's `previous.then(...)` would skip its action entirely and inherit an unrelated failure. * Hour walk `h < 25` -> `h < 26`. `DateTime(y, m, d, 24)` normalises to `nextMidnight` in every timezone, so h=24 and h=25 both break; 26 changes nothing. The extra fall-back hour is already covered — bucket [DateTime(y,m,d,1), DateTime(y,m,d,2)) spans BOTH occurrences of 1am, and the walk runs to nextMidnight. * SnackBar -> `NotificationCenter.emit`. The one-emitter rule is about NotificationService system alerts; this is an in-app SnackBar in direct response to a tap, the pattern used in 13 other UI files. * Floor commit on the wrong day. `mayCommitFloorOn` is `dayId >= frozenOn`, and `stored` is re-read inside the lock, so a backfill day older than a freeze a newer day just wrote is rejected. * Concurrent `syncPhoneSteps`. Per day it is ONE transaction, so two overlapping syncs cannot interleave a delete past an insert; a re-entrancy latch would only silently drop the 7-day backfill. * The analytics pin, which stays on analytics#35 until that merges. --- lib/compute/derivation_engine.dart | 25 +++++++++++++++++++++--- lib/data/db.dart | 15 ++++++++++++++ lib/health/phone_pedometer.dart | 22 +++++++++++++++++++++ lib/state/app_state.dart | 12 ++++++++++-- test/derive_day_window_test.dart | 22 ++++++++++++++++++--- test/phone_pedometer_hour_walk_test.dart | 25 ++++++++++++++++++++++++ 6 files changed, 113 insertions(+), 8 deletions(-) diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 2249b36..9289a0a 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -472,7 +472,16 @@ import 'substrate.dart'; // artifact with zero signal. That is the true root cause of the original // 42,155-steps-at-gRef-0.97 / 0-at-1.02 collapse. // active_min moves on every day; steps are unaffected by this bump. -const int kAlgoVersion = 56; +// +// v57 - review follow-up: the ABSENT `steps` block stops labelling itself. It +// carried `tier: 'ESTIMATE'` alongside `value: null`, and `Metric.parse` maps +// that tier to `beta: true`, so a day with no measurement at all rendered the +// estimate badge. Absent now means absent: `tier: null` (parsing to +// MetricTier.unknown) and an empty `inputs_used`. No VALUE changes, but the +// persisted bundle does, so days derived at v56 must be re-derived to pick it +// up. `ABSENT` was deliberately NOT invented as a fifth tier — `Tier.all` in +// analytics is a closed set of four published grades. +const int kAlgoVersion = 57; // Fold idempotency, the minimum-nights warm-up, and legacy-payload handling // all live in SleepProfilePolicy (pure, unit-tested) — see @@ -3208,8 +3217,18 @@ class DerivationEngine { 'real_measured': liveStepsReal, 'source': haveRealSteps ? 'pedometer_100hz_or_phone' : null, 'confidence': haveRealSteps ? 0.9 : 0.0, - 'tier': haveRealSteps ? 'HIGH' : 'ESTIMATE', - 'inputs_used': const ['live_coverage_pedometer'], + // NO TIER ON AN ABSENT METRIC. `ESTIMATE` here was actively wrong in two + // ways: this code path never estimates anything (that is the whole point + // of the change), and `Metric.parse` turns tier == ESTIMATE into + // `beta: true`, which paints the estimate/beta badge onto a card that has + // no number on it at all. `null` parses to `MetricTier.unknown`, which is + // what "we did not measure this" actually is. `ABSENT` is deliberately + // NOT invented: `Tier.all` in analytics is a closed set of four published + // grades and the edge must not widen it from here. + 'tier': haveRealSteps ? 'HIGH' : null, + // Likewise, nothing was used when nothing was measured. + 'inputs_used': + haveRealSteps ? const ['live_coverage_pedometer'] : const [], 'note': haveRealSteps ? 'real pedometer count over measured windows only; time outside ' 'those windows is not counted rather than estimated' diff --git a/lib/data/db.dart b/lib/data/db.dart index 6473cdb..4a6908b 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -870,6 +870,21 @@ class LocalDb { return r.isNotEmpty; } + /// Phone-sourced steps already banked for [day]. + /// + /// Used by the pedometer sync to tell "this day really had no steps" from + /// "this read came back empty" before it replaces a day wholesale — see + /// [replacePhoneCoverageForDay], which is delete-then-insert. + static Future phoneStepsForDay(String day) async { + final db = await instance; + final r = await db.rawQuery( + 'SELECT COALESCE(SUM(steps),0) s FROM live_coverage ' + 'WHERE day = ? AND source = ?', + [day, kStepSourcePhone], + ); + return (r.first['s'] as num?)?.toInt() ?? 0; + } + /// Drop every phone-sourced coverage row (the user turned phone steps off). /// Band rows are untouched, so days fall back to the band count. static Future clearPhoneCoverage() async { diff --git a/lib/health/phone_pedometer.dart b/lib/health/phone_pedometer.dart index dbd9f0c..907d874 100644 --- a/lib/health/phone_pedometer.dart +++ b/lib/health/phone_pedometer.dart @@ -194,6 +194,28 @@ class PhonePedometer { // produced no buckets at all). Nothing was read, so nothing is known. if (!anyRead) return null; + // AN ALL-ZERO DAY MUST NOT ERASE A DAY WE ALREADY BANKED WITH REAL + // COUNTS. Every hour returning 0 is indistinguishable at this layer from + // a genuinely sedentary day, and the one that matters is the failure the + // rest of this file already documents: on iOS `requestAuthorization` + // reports success even when the user denied READ, so reads come back + // *empty rather than null* forever after. Because + // `replacePhoneCoverageForDay` is delete-then-insert and phone rows win + // outright in `liveStepsForDay`, one such sync would wipe a real + // multi-thousand-step day and leave nothing — not even the band fallback + // that day had before phone steps were enabled. + // + // A day that legitimately went to zero after being non-zero is not a real + // trajectory (step counts only accumulate within a day), so keeping the + // banked value costs nothing. Returning null rather than 0 is the honest + // report: this day was NOT confirmed, so it must not count toward + // `daysRead` in the diagnostic the Profile screen shows. + if (total == 0 && await LocalDb.phoneStepsForDay(dayId) > 0) { + debugPrint('[phone_pedometer] $dayId read all-zero over a day that ' + 'already holds phone steps — keeping the banked day'); + return null; + } + await LocalDb.replacePhoneCoverageForDay(dayId, windows); return total; } catch (e) { diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index 28f5d92..dc1bfa8 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -426,9 +426,16 @@ class AppState extends ChangeNotifier { /// them. Nothing is uploaded and nothing is written back. Future requestPhoneSteps() async { final ok = await _phonePedometer.requestPermission(); - phoneStepsEnabled = ok; + // PERSIST BEFORE mutating in-memory state. Setting the field first and + // then awaiting the write leaves the two disagreeing if the write throws: + // the toggle reads ON for this run and OFF on the next launch, and the + // syncs below would bank phone rows the restored state says the user never + // enabled — rows that then keep overriding the band, since `disablePhone + // Steps` is the only thing that clears them and the user never sees the + // toggle on to turn it off. final prefs = await SharedPreferences.getInstance(); await prefs.setBool(_kPhoneSteps, ok); + phoneStepsEnabled = ok; notifyListeners(); // The user just asked for this, so pull the full backfill window rather // than the cheap routine one. @@ -455,9 +462,10 @@ class AppState extends ChangeNotifier { /// phone-sourced value permanently — there is no substrate left to recompute /// them from, which is the same limit every other version bump has. Future disablePhoneSteps() async { - phoneStepsEnabled = false; + // Persist first, for the reason in [requestPhoneSteps]. final prefs = await SharedPreferences.getInstance(); await prefs.setBool(_kPhoneSteps, false); + phoneStepsEnabled = false; phoneStepsLastSyncedDays = null; phoneStepsLastTotal = null; try { diff --git a/test/derive_day_window_test.dart b/test/derive_day_window_test.dart index 5d9d03b..10014b6 100644 --- a/test/derive_day_window_test.dart +++ b/test/derive_day_window_test.dart @@ -14,6 +14,8 @@ // calories_total as real scalars — fabricated numbers wearing real numbers' // clothes, against the never-impute contract the rest of the layer keeps. +import 'dart:convert'; + import 'package:flutter_test/flutter_test.dart'; import 'package:path/path.dart' as p; import 'package:sqflite_common_ffi/sqflite_ffi.dart'; @@ -222,9 +224,23 @@ void main() { // would drag every average and "most steps" record down. expect(await LocalDb.metricValueOn('2026-04-11', 'steps'), isNull); - // Movement minutes are still computable from 1 Hz and are unaffected. - expect(got.containsKey('steps'), isTrue, - reason: 'the key may exist; its VALUE must be null'); + // The bundle's own `steps` block must be absent-shaped too, not just + // value-less. (The previous assertion here checked `got.containsKey`, + // which was VACUOUS: `got` is built by the local helper above, which + // seeds every key unconditionally, so it could never fail whatever the + // derivation did.) + final row = await LocalDb.dayResult('2026-04-11'); + final bundle = + jsonDecode(row!['payload_json'] as String) as Map; + final steps = bundle['steps'] as Map; + expect(steps['value'], isNull); + expect(steps['confidence'], 0.0); + expect(steps['inputs_used'], isEmpty, + reason: 'nothing was used, because nothing was measured'); + // NOT 'ESTIMATE': `Metric.parse` maps that tier to `beta: true` and would + // badge a card that has no number on it as an estimate. Nothing here + // estimates anything — that is the entire point of this change. + expect(steps['tier'], isNull); }); test('a real profile still produces strain and calories', () async { diff --git a/test/phone_pedometer_hour_walk_test.dart b/test/phone_pedometer_hour_walk_test.dart index f0607a9..513ad85 100644 --- a/test/phone_pedometer_hour_walk_test.dart +++ b/test/phone_pedometer_hour_walk_test.dart @@ -124,6 +124,31 @@ void main() { expect(await LocalDb.liveStepsForDay(dayId), 777); }); + test('an all-zero read never erases a day already banked with real steps', + () async { + final day = yesterday(); + final dayId = _label(day); + + // 1. A good sync banks a real day. + final good = PhonePedometer(stepReader: (from, to) async => 100); + final fullTotal = await good.syncDay(day); + expect(fullTotal, isNotNull); + expect(await LocalDb.liveStepsForDay(dayId), fullTotal); + + // 2. Every hour now reads 0 WITHOUT failing — exactly what a silent iOS + // READ denial looks like (`requestAuthorization` reports success even + // when the user denied read, so queries return empty rather than null, + // forever). Unguarded, `replacePhoneCoverageForDay`'s delete-then-insert + // would wipe the day; and because phone rows win outright, not even the + // band fallback would show. + final denied = PhonePedometer(stepReader: (from, to) async => 0); + expect(await denied.syncDay(day), isNull, + reason: 'unconfirmed, so it must not count toward daysRead either'); + + // 3. The banked day survives. + expect(await LocalDb.liveStepsForDay(dayId), fullTotal); + }); + test('the routine sync window is much smaller than the backfill window', () { // Each hourly bucket is one platform round trip, so the window IS the cost: // the 7-day default was up to 168 sequential calls on every launch and