diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 16eb8578b..c8a9d78ed 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -7312,7 +7312,7 @@ class DerivationEngine { // one layer up on the output. final builtForDay = LocalDb.localDayLabelNow(); final builtAtEpoch = DateTime.now().millisecondsSinceEpoch ~/ 1000; - final (bundleJson, dropped) = await _runIsolateCancellable( + final (bundleJson, dropped, needMin) = await _runIsolateCancellable( () { final bundle = buildCrossDayBundle( @@ -7331,7 +7331,9 @@ class DerivationEngine { final paths = []; final safe = sanitizeForJson(bundle, paths) as Map; if (paths.isNotEmpty) safe['encode_dropped_fields'] = paths; - return (jsonEncode(safe), paths); + // Read here, not by re-decoding bundleJson on the main isolate (the + // hang the comment above is about). + return (jsonEncode(safe), paths, crossDayNeedMin(safe)); }, _crossDayTimeout, label: 'crossday', @@ -7342,7 +7344,15 @@ class DerivationEngine { _log('crossday: zone changed during the build; not stored'); return false; } - if (!await LocalDb.putReviewedBaseline('crossday', bundleJson, reviewRevision)) { + // `series`: the need this build computed, kept per day: the artifact + // only ever holds the latest one, so without this every past night was + // read against TODAY's need (#619). Same transaction as the artifact, + // so the day's row always matches the artifact that day last served; a + // later build the same day replaces both. null when the need abstained, + // so a past day reads "—", never a stale value. No backfill: an earlier + // day's need was never stored anywhere. + if (!await LocalDb.putReviewedBaseline('crossday', bundleJson, reviewRevision, + series: (date: builtForDay, key: 'sleep_need_min', value: needMin))) { _log('crossday: newer activity review; leaving its refresh queued'); return false; } @@ -7403,6 +7413,16 @@ class DerivationEngine { return out; } + /// The learned sleep need in a crossday bundle, in whole minutes, or null + /// when `sleep_coach.need` abstained or is missing — the same field and + /// rounding `LocalRepositoryImpl._sleepNeedMin` reads. + static double? crossDayNeedMin(Map bundle) => + switch (bundle['sleep_coach']) { + {'need': {'value': {'need_sec': final num s}}} => + (s / 60).roundToDouble(), + _ => null, + }; + /// The phone zone a `crossday_input` artifact was built in. Its records are /// zone-RELATIVE (`clock_shift_sec` is "this night's clock minus the zone the /// record was built in"), so an artifact built before a flight, served after diff --git a/lib/data/db.dart b/lib/data/db.dart index c3ab9f1f6..947b1e8b2 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -13341,13 +13341,22 @@ class LocalDb { } /// A calculation started before a review must never publish over it. - static Future putReviewedBaseline(String key, String payloadJson, int expectedRevision) async { + /// [series], when given, is one `metric_series` row derived from this same + /// payload, written in the same transaction so the two can never disagree + /// (derivation runs in more than one isolate, so two builds can interleave). + static Future putReviewedBaseline(String key, String payloadJson, int expectedRevision, + {({String date, String key, double? value})? series}) async { return (await instance).transaction((tx) async { final rows = await tx.query('activity_review_meta', columns: ['revision']); final revision = rows.isEmpty ? 0 : rows.single['revision'] as int; if (revision != expectedRevision) return false; await tx.insert('baselines', {'key': key, 'payload_json': payloadJson, 'updated_at': DateTime.now().millisecondsSinceEpoch}, conflictAlgorithm: ConflictAlgorithm.replace); + if (series != null) { + await tx.insert('metric_series', + {'date': series.date, 'key': series.key, 'value': series.value}, + conflictAlgorithm: ConflictAlgorithm.replace); + } return true; }); } diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index f74dba334..2146c8f55 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -1096,7 +1096,12 @@ class LocalRepositoryImpl extends LocalRepository { return v == null ? null : (v / 60).round(); } - final needMin = _sleepNeedMin(await _crossDay()); + // Today: the live estimate. A past night: the need computed on that day + // (written per day by the crossday pass), absent if none was — never + // today's estimate, which postdates the night (#619). + final needMin = _isTodayLabel(date) + ? _sleepNeedMin(await _crossDay()) + : (await LocalDb.metricValueOn(date, 'sleep_need_min'))?.round(); final sleepConf = _sub(b, 'sleep.accounting')?['confidence'] as num?; // Sleep periods (main + naps) for the periods screen. The main period is // enriched HERE with the hypnogram + stage minutes: derivation builds the @@ -1166,9 +1171,9 @@ class LocalRepositoryImpl extends LocalRepository { // field getDayHeart already read; no new computation. 'spo2': b['spo2'], // Sleep need, and the debt against it — both only when the need was - // actually learned (`sleep_coach.need`, see [_sleepNeedMin]; it is the - // current personal estimate, and the artifact behind it is refused once - // it is more than a week old). There is no 8 h default: it made the gauge + // actually learned (`sleep_coach.need`, see [_sleepNeedMin]; today's is + // the current personal estimate, a past night's the one stored for that + // day — see `needMin` above). There is no 8 h default: it made the gauge // "always read", which is the point, since what it read was a population // figure presented as this user's own. Absent keys leave the gauge in its // honest empty state. diff --git a/test/sleep_need_per_night_test.dart b/test/sleep_need_per_night_test.dart new file mode 100644 index 000000000..a9ed97859 --- /dev/null +++ b/test/sleep_need_per_night_test.dart @@ -0,0 +1,178 @@ +// #619: a past night is read against the sleep need the app had computed ON +// that day, not against today's estimate. The crossday artifact only ever holds +// the latest need, so `_runCrossDay` also files it per day under +// `metric_series` `sleep_need_min`; `getDaySleepV2` reads that row for any day +// but today, and reads nothing (no need, no debt) when there is none. + +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'; + +import 'package:openstrap_edge/compute/derivation_engine.dart'; +import 'package:openstrap_edge/data/day_label.dart'; +import 'package:openstrap_edge/data/db.dart'; +import 'package:openstrap_edge/data/local_repository_impl.dart'; + +void main() { + group('crossDayNeedMin', () { + test('reads sleep_coach.need in whole minutes', () { + expect( + DerivationEngine.crossDayNeedMin({ + 'sleep_coach': { + 'need': { + 'value': {'need_sec': 462 * 60 + 29}, + 'confidence': 0.7, + }, + }, + }), + 462.0, + ); + }); + + test('is null when the need abstained or the block is missing', () { + expect(DerivationEngine.crossDayNeedMin({}), isNull); + expect( + DerivationEngine.crossDayNeedMin({ + 'sleep_coach': { + 'need': {'value': null, 'reason': 'need_nights'}, + }, + }), + isNull, + ); + expect( + DerivationEngine.crossDayNeedMin({'sleep_coach': 'garbage'}), + isNull, + ); + }); + }); + + group('getDaySleepV2 sleep need per night', () { + late LocalRepositoryImpl repo; + late String today; + late String pastDay; + + String label(DateTime d) => dayLabelOf(d); + + String bundle(String day) => jsonEncode({ + 'date': day, + 'sleep': { + 'accounting': { + 'value': {'tst_sec': 420 * 60, 'efficiency_pct': 90.0}, + }, + }, + }); + + setUpAll(() async { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + LocalDb.dbName = 'openstrap_sleep_need_per_night_test.db'; + await databaseFactory.deleteDatabase( + p.join(await databaseFactory.getDatabasesPath(), LocalDb.dbName), + ); + repo = LocalRepositoryImpl(getProfileMap: () => const {}); + }); + + tearDownAll(() async { + await LocalDb.close(); + await databaseFactory.deleteDatabase( + p.join(await databaseFactory.getDatabasesPath(), LocalDb.dbName), + ); + }); + + setUp(() async { + // Per test, not once: a run that crosses local midnight would otherwise + // read a fixture "today" as a past day. + today = todayLabel(); + final t = DateTime.parse(today); + pastDay = label(DateTime(t.year, t.month, t.day - 3)); + final db = await LocalDb.instance; + await db.delete('day_result'); + await db.delete('baselines'); + await db.delete('metric_series'); + for (final day in [pastDay, today]) { + await LocalDb.putDayResult( + dayId: day, + algoVersion: kAlgoVersion, + payloadJson: bundle(day), + windowJson: '{}', + ); + } + // Today's crossday artifact says 480 min. + await LocalDb.putBaseline( + 'crossday', + jsonEncode({ + 'algo_version': kAlgoVersion, + 'built_for_day': today, + 'sleep_coach': { + 'need': { + 'value': {'need_sec': 480 * 60}, + 'confidence': 0.7, + 'tier': 'ESTIMATE', + }, + }, + }), + ); + }); + + test('a past night uses the need stored for that day, not today\'s', + () async { + await LocalDb.putMetricSeriesValue(pastDay, 'sleep_need_min', 450); + + final night = await repo.getDaySleepV2(pastDay); + expect(night['need_min'], 450); + expect(night['debt_min'], 30); // 450 need - 420 asleep + }); + + test('a past night with no stored need has no need and no debt', () async { + final night = await repo.getDaySleepV2(pastDay); + expect(night['has_sleep'], isNot(false)); + expect(night.containsKey('need_min'), isFalse); + expect(night.containsKey('debt_min'), isFalse); + }); + + test('a need that abstained later the same day clears the stored one', + () async { + await LocalDb.putMetricSeriesValue(pastDay, 'sleep_need_min', 450); + await LocalDb.putMetricSeriesValue(pastDay, 'sleep_need_min', null); + + final rows = await (await LocalDb.instance).query('metric_series', + where: 'date = ? AND key = ?', + whereArgs: [pastDay, 'sleep_need_min']); + expect(rows, hasLength(1)); + expect((await repo.getDaySleepV2(pastDay)).containsKey('need_min'), + isFalse); + }); + + test('the crossday write stores the artifact and its day\'s need together, ' + 'or neither', () async { + Future stored() => LocalDb.metricValueOn(today, 'sleep_need_min'); + final revision = await LocalDb.activityReviewRevision(); + + expect( + await LocalDb.putReviewedBaseline('crossday', '{"v":1}', revision, + series: (date: today, key: 'sleep_need_min', value: 455.0)), + isTrue); + expect(await stored(), 455.0); + expect((await LocalDb.baseline('crossday'))?['payload_json'], '{"v":1}'); + + // A stale build (a newer activity review landed) writes nothing. + expect( + await LocalDb.putReviewedBaseline('crossday', '{"v":2}', revision + 1, + series: (date: today, key: 'sleep_need_min', value: 400.0)), + isFalse); + expect(await stored(), 455.0); + expect((await LocalDb.baseline('crossday'))?['payload_json'], '{"v":1}'); + }); + + test('today still reads the live crossday need', () async { + // A stored row for today must not override the live estimate. + await LocalDb.putMetricSeriesValue(today, 'sleep_need_min', 450); + + final night = await repo.getDaySleepV2(today); + expect(night['need_min'], 480); + expect(night['debt_min'], 60); + }); + }); +}