From bfc6f19e2a546b85cbcef85344b9475fa315c202 Mon Sep 17 00:00:00 2001 From: uvtqt Date: Tue, 10 Nov 2026 13:20:14 +0100 Subject: [PATCH 1/2] Read a past night against its own sleep need, not today's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The crossday artifact only ever holds the latest sleep need, and _daySleep read it for every date, so any past night's need and debt were measured against today's estimate (#619). The crossday pass now also files the need it computed under metric_series `sleep_need_min` for the day it was built (REPLACE per day; null when the need abstained). _daySleep keeps the live estimate for today and reads the stored row for past dates; with no row, a past night carries no need_min/debt_min and the ring shows "—". No backfill: an earlier day's need was never stored, so it is not reconstructed. The need is read inside the crossday isolate (crossDayNeedMin) rather than by re-decoding the bundle on the main isolate. No existing number changes, so no kAlgoVersion bump. --- lib/compute/derivation_engine.dart | 22 +++- lib/data/local_repository_impl.dart | 13 ++- test/sleep_need_per_night_test.dart | 155 ++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 6 deletions(-) create mode 100644 test/sleep_need_per_night_test.dart diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 16eb8578b..c393240d9 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', @@ -7346,6 +7348,12 @@ class DerivationEngine { _log('crossday: newer activity review; leaving its refresh queued'); return false; } + // The need this build computed, kept per day: the artifact above only + // ever holds the latest one, so without this every past night was read + // against TODAY's need (#619). A later build the same day replaces it; + // 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. + await LocalDb.putMetricSeriesValue(builtForDay, 'sleep_need_min', needMin); if (dropped.isNotEmpty) { // Loud, not debug-only: a dropped field is a metric the user will see // as absent, and the reason lives here and nowhere else. @@ -7403,6 +7411,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/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..cf4d65251 --- /dev/null +++ b/test/sleep_need_per_night_test.dart @@ -0,0 +1,155 @@ +// #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 {}); + today = todayLabel(); + final t = DateTime.parse(today); + pastDay = label(DateTime(t.year, t.month, t.day - 3)); + }); + + tearDownAll(() async { + await LocalDb.close(); + await databaseFactory.deleteDatabase( + p.join(await databaseFactory.getDatabasesPath(), LocalDb.dbName), + ); + }); + + setUp(() async { + 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('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); + }); + }); +} From 5bd81c952d57988e841e0e029641425baa8ce19d Mon Sep 17 00:00:00 2001 From: uvtqt Date: Tue, 10 Nov 2026 17:01:20 +0100 Subject: [PATCH 2/2] Write the day's sleep need in the crossday artifact's transaction Derivation can run in more than one isolate, so two crossday builds on the same day could interleave their two separate writes and leave the artifact from one and the per-day need from the other. putReviewedBaseline takes an optional metric_series row and writes it in the same transaction (and not at all when the review revision has moved on), so the day's row always matches the artifact it came from. Test: the day's "today" is taken per test, not once in setUpAll, so a run crossing local midnight doesn't read today's fixture as a past day. --- lib/compute/derivation_engine.dart | 16 +++++++++------- lib/data/db.dart | 11 ++++++++++- test/sleep_need_per_night_test.dart | 29 ++++++++++++++++++++++++++--- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index c393240d9..c8a9d78ed 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -7344,16 +7344,18 @@ 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; } - // The need this build computed, kept per day: the artifact above only - // ever holds the latest one, so without this every past night was read - // against TODAY's need (#619). A later build the same day replaces it; - // 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. - await LocalDb.putMetricSeriesValue(builtForDay, 'sleep_need_min', needMin); if (dropped.isNotEmpty) { // Loud, not debug-only: a dropped field is a metric the user will see // as absent, and the reason lives here and nowhere else. 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/test/sleep_need_per_night_test.dart b/test/sleep_need_per_night_test.dart index cf4d65251..a9ed97859 100644 --- a/test/sleep_need_per_night_test.dart +++ b/test/sleep_need_per_night_test.dart @@ -72,9 +72,6 @@ void main() { p.join(await databaseFactory.getDatabasesPath(), LocalDb.dbName), ); repo = LocalRepositoryImpl(getProfileMap: () => const {}); - today = todayLabel(); - final t = DateTime.parse(today); - pastDay = label(DateTime(t.year, t.month, t.day - 3)); }); tearDownAll(() async { @@ -85,6 +82,11 @@ void main() { }); 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'); @@ -143,6 +145,27 @@ void main() { 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);