From 08d171dc76b796b388e8b02c8bfd2738fedb4cc4 Mon Sep 17 00:00:00 2001 From: SATHVIK SVS Date: Mon, 10 Aug 2026 07:18:18 +0530 Subject: [PATCH 1/6] spec: storage compression design --- .../2026-08-10-storage-compression-design.md | 229 ++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-10-storage-compression-design.md diff --git a/docs/superpowers/specs/2026-08-10-storage-compression-design.md b/docs/superpowers/specs/2026-08-10-storage-compression-design.md new file mode 100644 index 00000000..d1a84128 --- /dev/null +++ b/docs/superpowers/specs/2026-08-10-storage-compression-design.md @@ -0,0 +1,229 @@ +# Storage compression — design + +Date: 2026-08-10 +Status: approved, implementing + +## Problem + +Measured on-device footprint, taken by building the real schema from +`lib/data/db.dart` into a scratch SQLite database, filling it with realistic +values, and reading per-object byte counts out of `dbstat`: + +| Object | Rate | Bounded by | 1-year total | +|---|---|---|---| +| `decoded_onehz` + 2 indexes | 7.18 MB/day | 3 days | ~21.5 MB | +| `decoded_rr` + 3 b-trees | 5.40 MB/day | 3 days | ~16.2 MB | +| `day_result` (88 KB `payload_json`) | 88.1 KB/day | **never** | **~32.9 MB** | +| `metric_series` + 2 indexes | 3.0 KB/day | never | ~1.1 MB | +| **Live DB total** | | | **~72 MB** | +| Auto-backups (`kBackupsKept = 5`) | 5 × full DB | 5 copies | **~360 MB** | + +Nothing is compressed at rest. `gzip` appears only at I/O boundaries: the +opt-in daily health upload (`telemetry/health_uploader.dart:89`) and container +detection on import (`import/import_container.dart`). + +`day_result` is the only pool that grows without bound. The 1 Hz substrate is +capped at `rawRetentionDays = 3` and is flat, not growing. + +## What the bytes actually are + +`payload_json` is 88 KB, of which `series` is 74.5 KB, stored as: + +```json +[{"t":1783572180,"v":77},{"t":1783572240,"v":80}] +``` + +27 bytes per sample to carry two numbers, with the full 10-digit epoch repeated +in every element. Across the three tracked fixtures, four curves sample on a +perfectly regular grid — `hr_curve` (dt=60), `strain_curve` (60), +`zone_timeline` (60), `skin_temp_day` (300) — and account for 85% of `series`. +The rest (`hrv_day`, `resp_day`, `hrv_timeline`) are event-timed. + +This is an encoding problem, not a compression problem. Facebook's Gorilla +reaches ~12x on exactly this shape via delta-of-delta timestamps before any +general-purpose codec runs. + +## Constraint that shapes the design + +`payload_json` cannot become a compressed BLOB. The coach views read it with +SQL: + +```sql +FROM latest l, json_each(json_extract(l.payload_json,'$.series.hypnogram')) e +``` + +`v_series` and `v_hypnogram` (`db.dart:1755-1791`) `json_extract` into the +payload, and `sleepAccountingDays` (`db.dart:3205`) runs `json_valid` on it. +SQLite's json1 functions cannot see inside a gzip blob, and sqflite exposes no +way to register a custom SQL decompress function. Compressing the column +silently strips every intra-day curve from the AI Coach — invariant 13, and the +§4.7 "wired into one call path but not all N" pattern. + +Stacking gzip on top of the re-encoding below reaches 5.1-8.1x instead of +2.13x. It is **explicitly rejected**: it buys ~7 MB/year at the cost of the +coach's entire SQL surface. + +## Design + +### Wire format + +Three shapes coexist permanently. All are plain JSON, so json1 still reads +them. + +| Shape | Form | Written | Read | +|---|---|---|---| +| `legacy` | `[{"t":N,"v":X},…]` | never again | always | +| `grid` | `{"t0":N,"dt":N,"v":[…]}` | regular sampling | always | +| `offset` | `{"t0":N,"to":[…],"v":[…]}` | irregular sampling | always | + +Legacy stays readable forever. That is what makes this migration-free: no +rewrite pass runs inside `openDatabase` under the iOS CPU watchdog +(invariant 11). + +`json_each` exposes a JSON array's index as `key`, so a grid reconstructs its +timestamps as `t0 + key*dt` in pure SQL — no running sum, no extension. + +### Encoder rules + +Owned by one new pure file, `lib/data/series_codec.dart` (invariant 8). + +- Encode only when the curve has >= 3 points and every element carries `t` plus + the value key. `zone_timeline` uses `z`, everything else `v`. +- `grid` iff every delta is identical and positive; `offset` otherwise, with + `to[0] == 0`. +- **Null values are preserved as `null` in `v[]`** — never dropped, never + interpolated (invariant 3). +- Anything the encoder cannot handle passes through unchanged. The fallback is + always "stay legacy", never "lose data". +- `hypnogram` elements are `{start,end,stage}` with no `t`, so the encoder skips + them by construction and `v_hypnogram` needs no change. + +No `kAlgoVersion` bump: values do not change, only their spelling. Bumping +would force a pointless full-history recompute. + +### Three seams, one owner each + +**Write** — `LocalDb.putDayResult` encodes. All four callers +(`derivation_engine` x2, `cloud_import`, `whoop_import`) already funnel through +it. Everything upstream keeps operating on plain `[{t,v}]` in memory: the +`bundle['series']` merges at `derivation_engine.dart:2490` and `:2815`, and the +patch logic at `:4793`, are untouched. + +**Dart read** — `SeriesCodec.decodePayload` normalizes back to `[{t,v}]` inside +`local_repository_impl._decode` (`:46`), covering ~15 call sites at once. Five +readers live outside that funnel and each gets the same call: +`state/app_state.dart:1304`, `data/db.dart:4173` and `:4422`, +`import/whoop_import.dart:196`, `compute/derivation_engine.dart:3239`. + +Normalization is safe to apply to non-`day_result` payloads that share +`_decode` (baselines, freshness, wake features): it only rewrites keys that are +in grid/offset shape, which nothing but `putDayResult` ever writes. It is +idempotent. + +**SQL read** — `v_series` becomes a UNION over the three shapes for the named +curves, `zone_timeline`, and the root `activity_curve`. Each branch is guarded +so a row in one shape contributes to exactly one branch. Views are DROP+CREATE +on every open, so they need no migration. + +### Schema v32 + +- `DROP INDEX IF EXISTS idx_decoded_rr_counter` in the `onUpgrade` ladder and in + `_repairOpenSchema`, and stop creating it in `_createDecodedStore`. + It is an exact duplicate of `sqlite_autoindex_decoded_rr_1`, which + `PRIMARY KEY (counter, beat_index)` already creates. Verified: both measured + 3,264,512 bytes on a 3-day fill, and after dropping it the planner still + serves `counter` lookups from the auto-index. Saves ~1.09 MB/day plus one + b-tree write on the hottest insert path in the app. + +### History backfill + +New rows shrink immediately; existing rows would stay large forever. A bounded +re-encode pass runs where `pruneSupersededIntermediates` already runs — after +derivation, off the path to a durable commit, never inside a migration. It +re-encodes a capped number of legacy rows per invocation, is idempotent, and is +resumable. + +### Backups and import + +- `auto_backup` writes `openstrap-YYYYMMDD-HHMMSS.db.gz`. +- The retention pattern must match **both** `.db` and `.db.gz`. If it only + matches the new name, existing backups become invisible to + `sortBackupsNewestFirst` and are never pruned, leaking five stale copies. +- `import_container` learns to inflate gzip instead of rejecting it with "unzip + it first", reusing the existing `_kMaxUncompressedBytes` guard. +- The manual profile export stays a plain `.db` — users open that in other + tools. + +## Error handling + +Every decode path in this codebase is already `try/catch -> ignore`; the codec +keeps that contract. A malformed grid object (missing `dt`, ragged `to`/`v`) +decodes to an empty curve rather than throwing — the same observable outcome as +a missing key today. + +## Testing + +- `series_codec_test.dart` — lossless round-trip against all three tracked + fixtures, plus empty, 1-2 points, embedded nulls, non-monotonic `t`, + duplicate `t`, negative `dt`. +- `coach_views_series_shapes_test.dart` — the same day inserted in legacy and in + encoded form must produce identical `v_series` rows. This is the regression + pin. +- A structural guard in the style of `dart_source_test.dart`: every `jsonDecode` + of a `payload_json` must be wrapped by the normalizer, so a future reader + cannot silently skip it (§4.7). +- Size assertion: the encoded fixture is under 50% of the original. +- Extensions to `db_storage_hygiene_test.dart` (index gone, `counter` lookups + still index-served), `auto_backup_test.dart` (`.gz` naming, retention across + mixed old and new names), `import_container_test.dart` (gzip inflate, size + guard). + +## Measured outcome + +Prototype run against the three tracked fixtures, with `v_series` output +compared row-for-row between the old SQL over old payloads and the new SQL over +encoded payloads: + +| Fixture | Now | Encoded | Ratio | View output | +|---|---|---|---|---| +| `payload.json` | 88,053 | 32,985 | 2.67x | identical | +| `payload_july10.json` | 68,317 | 39,456 | 1.73x | identical | +| `payload_null.json` | 57,252 | 27,786 | 2.06x | identical | +| **Total** | 213,622 | 100,227 | **2.13x** | **byte-identical** | + +- Derived data: ~33 MB/yr -> ~15 MB/yr +- Substrate: -1.09 MB/day +- Backups: ~360 MB -> ~120 MB + +No decompression on any read path. + +## Rejected alternatives + +- **gzip `payload_json` into a BLOB** — 5.1-8.1x, but breaks `v_series` / + `v_hypnogram` and cannot be repaired without a custom SQL function sqflite + does not expose. +- **Native zstd (`sqlite-zstd`, `sqlite_zstd_vfs`)** — ~80% savings, but means + FFI plus per-platform native builds wired into the riskiest part of the app. + Dart's built-in `ZLibCodec` needs none of that and is only used where no SQL + reads the bytes. +- **Chunked columnar blobs for the 1 Hz substrate** — a real Gorilla-style win + per day, but the substrate is already capped at 3 days, so the steady-state + saving is one-time and modest, while the cost is rewriting the BLE drain + through the commit-before-ACK path (invariant 1), whose failure mode is + permanent data loss or an infinite re-flood. Deferred. Dropping the duplicate + index already claims 1.09 of its 12.6 MB/day for none of that risk. +- **Materializing `v_series` into a real table** — measured worse than the JSON + it would replace (~116 KB/day naive, ~39 KB/day with interned keys, before + the index the coach would need). +- **Tiered hot/cold split (recent days uncompressed, old days compressed)** — + the coach auto-appends a row cap but never bounds by date, so old days would + silently vanish from its context rather than degrade. + +## References + +- Gorilla: A Fast, Scalable, In-Memory Time Series Database (VLDB 2015) — + https://www.vldb.org/pvldb/vol8/p1816-teller.pdf +- phiresky/sqlite-zstd — https://phiresky.github.io/blog/2022/sqlite-zstd/ +- mlin/sqlite_zstd_vfs — https://github.com/mlin/sqlite_zstd_vfs +- Netdata tiered retention — + https://www.netdata.cloud/features/dataplatform/tiered-retention/ From fa467f11ea0f917a6edb9238b95585b526205665 Mon Sep 17 00:00:00 2001 From: SATHVIK SVS Date: Mon, 10 Aug 2026 07:46:12 +0530 Subject: [PATCH 2/6] storage that stops growing: compact curves, one less index, gzipped backups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 392 MB -> 143 MB on a one-year-old install, measured by rebuilding the real schema both ways and reading per-object byte counts out of `dbstat`. `day_result` is the only store that grows without bound — raw and decoded are capped at `rawRetentionDays`, derived days are forever. Its 88 KB bundle was 74.5 KB of `series`, stored as one JSON object per sample: [{"t":1783572180,"v":77},{"t":1783572240,"v":80}] 27 bytes to carry two numbers, repeating the full 10-digit epoch every element, for curves that sample on a fixed 60-second grid. That is an encoding problem, not a compression problem. Curves are now written as `{t0,dt,v[]}` when the sampling is regular and `{t0,to[],v[]}` when it is not — 2.13x across the three tracked fixtures, with no codec and no decompression on any read path. It stays PLAIN JSON on purpose. The coach reads this column with SQL (`json_each`/`json_extract` in `v_series`/`v_hypnogram`), a compressed BLOB is opaque to json1, and sqflite cannot register a decompress function — so gzipping the column would have silently stripped every intra-day curve from the AI Coach. Stacking gzip on top would reach 5.1-8.1x; it is rejected for ~7 MB a year. Three shapes coexist forever, so there is no rewriting migration and nothing runs inside `openDatabase` under iOS's CPU watchdog. `v_series` reads all three and was verified row-for-row against the pre-codec view, including a database holding both shapes at once. Existing rows are converted by a bounded, forward-only, resumable pass that runs beside `pruneSupersededIntermediates` — each row gated on a proven-lossless round-trip before it is touched, and only `payload_json` is written, so no day is re-dated or re-finalized. Also: * `idx_decoded_rr_counter` was an exact duplicate of the index `PRIMARY KEY (counter, beat_index)` already creates. Both measured 3,264,512 bytes on a 3-day fill; the planner still serves `counter` lookups from the auto-index without it. ~1.09 MB/day and one b-tree write per beat off the hottest insert path. Dropped in `_createDecodedStore` like the `idx_decoded_rr_ts` drop above it, so no schemaVersion bump. * Auto-backups are gzipped (3.44x on five copies, ~326 MB -> ~95 MB), streamed rather than buffered. The retention pattern now also matches the plain `.db` names earlier versions wrote and the `-N` collision names `_uniqueDestination` emits — neither matched before, so both leaked full-size copies that retention could never see. * Import inflates gzip instead of refusing it ("unzip it first"), for both the CSV and database paths, under the existing size ceiling. A structural guard pins every bundle-decode seam to the codec — §4.7 is the failure mode here, and it is a silent one: a bypassed reader gets a Map where it expects a List, matches neither, and renders an empty chart. flutter analyze clean; 1830 tests pass. --- .../2026-08-10-storage-compression-design.md | 37 ++- lib/compute/derivation_engine.dart | 28 +- lib/data/auto_backup.dart | 48 ++- lib/data/db.dart | 223 ++++++++++--- lib/data/local_repository_impl.dart | 19 +- lib/data/series_codec.dart | 310 ++++++++++++++++++ lib/health/health_export.dart | 16 +- lib/import/import_container.dart | 95 +++++- lib/import/whoop_import.dart | 7 +- lib/state/app_state.dart | 7 +- test/auto_backup_test.dart | 77 ++++- test/coach_views_series_shapes_test.dart | 217 ++++++++++++ test/day_result_reencode_test.dart | 248 ++++++++++++++ test/db_storage_hygiene_test.dart | 32 ++ test/import_container_test.dart | Bin 12061 -> 15470 bytes test/series_codec_structural_test.dart | 186 +++++++++++ test/series_codec_test.dart | 273 +++++++++++++++ 17 files changed, 1723 insertions(+), 100 deletions(-) create mode 100644 lib/data/series_codec.dart create mode 100644 test/coach_views_series_shapes_test.dart create mode 100644 test/day_result_reencode_test.dart create mode 100644 test/series_codec_structural_test.dart create mode 100644 test/series_codec_test.dart diff --git a/docs/superpowers/specs/2026-08-10-storage-compression-design.md b/docs/superpowers/specs/2026-08-10-storage-compression-design.md index d1a84128..87c7b00a 100644 --- a/docs/superpowers/specs/2026-08-10-storage-compression-design.md +++ b/docs/superpowers/specs/2026-08-10-storage-compression-design.md @@ -125,15 +125,21 @@ curves, `zone_timeline`, and the root `activity_curve`. Each branch is guarded so a row in one shape contributes to exactly one branch. Views are DROP+CREATE on every open, so they need no migration. -### Schema v32 - -- `DROP INDEX IF EXISTS idx_decoded_rr_counter` in the `onUpgrade` ladder and in - `_repairOpenSchema`, and stop creating it in `_createDecodedStore`. - It is an exact duplicate of `sqlite_autoindex_decoded_rr_1`, which - `PRIMARY KEY (counter, beat_index)` already creates. Verified: both measured - 3,264,512 bytes on a 3-day fill, and after dropping it the planner still - serves `counter` lookups from the auto-index. Saves ~1.09 MB/day plus one - b-tree write on the hottest insert path in the app. +### The duplicate index + +`idx_decoded_rr_counter` is an exact duplicate of the index +`PRIMARY KEY (counter, beat_index)` already creates +(`sqlite_autoindex_decoded_rr_1`) — same table, same columns, same order. +Verified: both measured 3,264,512 bytes on a 3-day fill, and after dropping it +the planner still serves `counter` lookups and `(counter, beat_index)` ordering +from the auto-index. Saves ~1.09 MB/day plus one b-tree write on the hottest +insert path in the app. + +**No `schemaVersion` bump.** The drop lives inside `_createDecodedStore`, which +`_repairOpenSchema` already re-runs on every open, so it self-heals on existing +installs and is never created on new ones. This follows the precedent one line +above it — the `idx_decoded_rr_ts` drop was done exactly this way. A ladder +entry would force `onUpgrade` to run for no additional effect. ### History backfill @@ -191,9 +197,16 @@ encoded payloads: | `payload_null.json` | 57,252 | 27,786 | 2.06x | identical | | **Total** | 213,622 | 100,227 | **2.13x** | **byte-identical** | -- Derived data: ~33 MB/yr -> ~15 MB/yr -- Substrate: -1.09 MB/day -- Backups: ~360 MB -> ~120 MB +End-to-end, rebuilding the real schema both ways and reading `dbstat` — a +one-year-old install with three days of 1 Hz substrate and five auto-backups: + +| Component | Before | After | Saved | +|---|---|---|---| +| 1 Hz substrate (3-day window) | 37,703,680 | 34,443,264 | 3,260,416 | +| Derived bundles (365 days) | 26,447,872 | 12,484,608 | 13,963,264 | +| **Database file** | **65,290,240** | **48,066,560** | **1.36x** | +| Backups (5 copies) | 326,451,200 | 94,812,300 | 3.44x | +| **Total on device** | **392 MB** | **143 MB** | **2.74x** | No decompression on any read path. diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index b568d680..a397cd0b 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -33,6 +33,7 @@ import 'package:firebase_performance/firebase_performance.dart'; import '../data/db.dart'; import '../data/day_label.dart'; +import '../data/series_codec.dart'; import '../notify/notification_center.dart'; import '../notify/notification_event.dart'; import '../notify/tap_router.dart' show kRouteWorkoutSuggestion; @@ -3203,15 +3204,16 @@ class DerivationEngine { } } - static Map? _decodeBundle(Object? json) { - if (json is! String) return null; - try { - final d = jsonDecode(json); - return d is Map ? d.cast() : null; - } catch (_) { - return null; - } - } + /// Decode a stored day bundle, normalizing the compact curve format back to + /// plain [{t,v}] lists. + /// + /// The normalization is LOAD-BEARING on the re-derive path, not just hygiene: + /// `_deriveOneDay` reads the previous row through here and merges its series + /// into the fresh bundle. Without normalizing, an encoded `prev` would be + /// merged beside newly-computed legacy lists and the day would carry two + /// different shapes for the same curve. + static Map? _decodeBundle(Object? json) => + SeriesCodec.decodePayloadJson(json); /// Build the cross-day record from a day_result row + its payload bundle. static Map? _crossDayRecord( @@ -3312,6 +3314,14 @@ class DerivationEngine { if (stale > 0) { _log('pruned $stale superseded intermediate rows'); } + // Convert the back catalogue to the compact curve format, a bounded batch + // at a time. Same reasoning as the prune above: recomputable/rewritable + // housekeeping belongs here, off the path to a durable commit, and never in + // a migration under iOS's CPU watchdog. + final reencoded = await LocalDb.reencodeLegacyDayResults(); + if (reencoded > 0) { + _log('re-encoded $reencoded legacy day bundles'); + } } static List _perMinuteMeanWake( diff --git a/lib/data/auto_backup.dart b/lib/data/auto_backup.dart index bbd4416d..1c40e738 100644 --- a/lib/data/auto_backup.dart +++ b/lib/data/auto_backup.dart @@ -80,6 +80,11 @@ bool backupIsDue({ return now.difference(lastRun) >= interval; } +/// Extension for a backup written by the CURRENT code. Backups are gzipped: +/// the database is JSON-heavy and mostly text, so this is roughly a 3x saving +/// on the one thing here that is kept five times over. +const kBackupExtension = '.db.gz'; + /// Filename for a backup taken at [when]. Sorts chronologically as text, so /// retention can order by name without parsing. /// @@ -88,15 +93,26 @@ bool backupIsDue({ String backupFileName(DateTime when) { String two(int v) => v.toString().padLeft(2, '0'); return 'openstrap-${when.year}${two(when.month)}${two(when.day)}' - '-${two(when.hour)}${two(when.minute)}${two(when.second)}.db'; + '-${two(when.hour)}${two(when.minute)}${two(when.second)}$kBackupExtension'; } -/// EXACTLY the shape [backupFileName] emits, and nothing else. +/// EXACTLY the shapes this file has ever emitted, and nothing else. /// /// Retention DELETES what this matches, and it runs in a directory the user /// can put files into. A loose `openstrap-*.db` glob would happily eat /// someone's `openstrap-notes.db`. -final _backupNamePattern = RegExp(r'^openstrap-\d{8}-\d{6}\.db$'); +/// +/// Covers THREE shapes deliberately: +/// • `.db.gz` — what is written now. +/// • `.db` — what earlier versions wrote. An install that upgrades still has +/// up to [kBackupsKept] of these. If the pattern stopped matching them they +/// would become invisible to [sortBackupsNewestFirst], never be counted +/// toward retention and never be pruned — five stale full-size copies +/// leaked permanently, which is the opposite of what this change is for. +/// • a `-N` collision suffix — [_uniqueDestination] emits these when two runs +/// land in the same second, and the pattern never matched them, so they +/// leaked for the same reason. +final _backupNamePattern = RegExp(r'^openstrap-\d{8}-\d{6}(-\d+)?\.db(\.gz)?$'); /// Existing backups, newest first. List sortBackupsNewestFirst(Iterable entries) { @@ -202,12 +218,22 @@ Future _runBackup({ final snapshot = await (exportSnapshot ?? LocalDb.exportCopy)(); final tmp = File(snapshot); try { - await tmp.rename(dest.path); - } on FileSystemException { - // The temp directory and external storage are different filesystems on - // Android, where rename fails outright — copy across, then drop the - // source. - await tmp.copy(dest.path); + // STREAMED, not read-then-compress: the snapshot is the whole database + // and buffering it twice in memory to save disk would trade one resource + // problem for a worse one on the devices that have the most data. + // + // This also replaces the old rename/copy fallback — that existed because + // temp and external storage are different filesystems on Android, where + // rename fails outright. A stream never had that problem. + await tmp.openRead().transform(gzip.encoder).pipe(dest.openWrite()); + } catch (_) { + // A half-written .gz is not a backup, and leaving one behind would let it + // count toward retention and push a GOOD backup out of the window. + try { + if (await dest.exists()) await dest.delete(); + } catch (_) {} + rethrow; + } finally { try { if (await tmp.exists()) await tmp.delete(); } catch (_) {} @@ -242,10 +268,10 @@ Future pruneBackups(Directory dir, {required int keep}) async { /// the exact data loss this function exists to prevent. File? _uniqueDestination(Directory dir, DateTime when) { final base = backupFileName(when); - final stem = base.substring(0, base.length - 3); // drop '.db' + final stem = base.substring(0, base.length - kBackupExtension.length); for (var i = 1; i < 100; i++) { final candidate = File( - p.join(dir.path, i == 1 ? base : '$stem-$i.db'), + p.join(dir.path, i == 1 ? base : '$stem-$i$kBackupExtension'), ); if (!candidate.existsSync()) return candidate; } diff --git a/lib/data/db.dart b/lib/data/db.dart index 2fe16077..b6a8d54f 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -20,6 +20,7 @@ import 'day_label.dart'; import 'journal_fields.dart'; import 'live_coverage_policy.dart'; import 'models.dart'; +import 'series_codec.dart'; class LocalDb { static Database? _db; @@ -1751,29 +1752,57 @@ class LocalDb { '''); // Intra-day curves UNNESTED from the latest day_result bundle. HEAVY — always // filter by date AND series. zone_timeline uses 'z'; activity_curve is root. + // + // THREE SHAPES, one row each. A curve is stored either legacy + // (`[{t,v},…]`), grid (`{t0,dt,v[]}`) or offset (`{t0,to[],v[]}`) — see + // data/series_codec.dart for why. Old rows keep their legacy shape forever, + // so this view must read all three, and the branch guards are what keep a + // row from being emitted twice: legacy requires an `array`, grid requires + // `.dt`, offset requires `.to`, and no stored curve ever satisfies two. + // + // The grid branch needs no running sum because json_each exposes an array's + // index as `key`, so t = t0 + key*dt. Offset pairs `to` and `v` on that + // same key. Verified row-for-row against the pre-codec view on the three + // tracked bundle fixtures, including a database holding both shapes at once + // (test/coach_views_series_shapes_test.dart). await db.execute(''' CREATE VIEW v_series AS WITH latest AS ( SELECT r.day_id, r.payload_json FROM day_result r JOIN (SELECT day_id, MAX(algo_version) v FROM day_result GROUP BY day_id) m ON r.day_id = m.day_id AND r.algo_version = m.v + ), + curve(sk, pth, vk) AS ( + SELECT 'hr_curve','\$.series.hr_curve','\$.v' + UNION ALL SELECT 'strain_curve','\$.series.strain_curve','\$.v' + UNION ALL SELECT 'hrv_timeline','\$.series.hrv_timeline','\$.v' + UNION ALL SELECT 'hrv_day','\$.series.hrv_day','\$.v' + UNION ALL SELECT 'resp_day','\$.series.resp_day','\$.v' + UNION ALL SELECT 'skin_temp_day','\$.series.skin_temp_day','\$.v' + UNION ALL SELECT 'zone_timeline','\$.series.zone_timeline','\$.z' + UNION ALL SELECT 'activity_curve','\$.activity_curve','\$.v' ) - SELECT l.day_id AS date, s.sk AS series, + SELECT l.day_id AS date, c.sk AS series, json_extract(e.value,'\$.t') AS t, - json_extract(e.value,'\$.v') AS v - FROM latest l - JOIN (SELECT 'hr_curve' sk UNION ALL SELECT 'strain_curve' - UNION ALL SELECT 'hrv_timeline' UNION ALL SELECT 'hrv_day' - UNION ALL SELECT 'resp_day' UNION ALL SELECT 'skin_temp_day') s - JOIN json_each(json_extract(l.payload_json,'\$.series.'||s.sk)) e + json_extract(e.value, c.vk) AS v + FROM latest l JOIN curve c + JOIN json_each(json_extract(l.payload_json, c.pth)) e + WHERE json_type(json_extract(l.payload_json, c.pth)) = 'array' UNION ALL - SELECT l.day_id, 'zone_timeline', - json_extract(e.value,'\$.t'), json_extract(e.value,'\$.z') - FROM latest l, json_each(json_extract(l.payload_json,'\$.series.zone_timeline')) e + SELECT l.day_id, c.sk, + json_extract(l.payload_json, c.pth||'.t0') + + e.key * json_extract(l.payload_json, c.pth||'.dt'), + e.value + FROM latest l JOIN curve c + JOIN json_each(json_extract(l.payload_json, c.pth||'.v')) e + WHERE json_extract(l.payload_json, c.pth||'.dt') IS NOT NULL UNION ALL - SELECT l.day_id, 'activity_curve', - json_extract(e.value,'\$.t'), json_extract(e.value,'\$.v') - FROM latest l, json_each(json_extract(l.payload_json,'\$.activity_curve')) e + SELECT l.day_id, c.sk, + json_extract(l.payload_json, c.pth||'.t0') + et.value, ev.value + FROM latest l JOIN curve c + JOIN json_each(json_extract(l.payload_json, c.pth||'.to')) et + JOIN json_each(json_extract(l.payload_json, c.pth||'.v')) ev ON ev.key = et.key + WHERE json_extract(l.payload_json, c.pth||'.to') IS NOT NULL '''); // Sleep stage segments (different element shape from the {t,v} curves). await db.execute(''' @@ -2036,9 +2065,6 @@ class LocalDb { PRIMARY KEY (counter, beat_index) ) '''); - await db.execute( - 'CREATE INDEX IF NOT EXISTS idx_decoded_rr_counter ON decoded_rr(counter, beat_index)', - ); await db.execute( 'CREATE UNIQUE INDEX IF NOT EXISTS idx_decoded_rr_ts_beat_unique ' 'ON decoded_rr(rr_ts_ms, beat_index)', @@ -2048,6 +2074,15 @@ class LocalDb { // from it. The narrower index only added a second b-tree to maintain on // the hottest write path in the app. await db.execute('DROP INDEX IF EXISTS idx_decoded_rr_ts'); + // idx_decoded_rr_counter(counter, beat_index) was an EXACT duplicate of the + // index `PRIMARY KEY (counter, beat_index)` already creates + // (sqlite_autoindex_decoded_rr_1) — same table, same columns, same order. + // Measured on a 3-day fill: both b-trees 3,264,512 bytes, i.e. ~1.09 MB/day + // of pure duplication, plus a second b-tree write per beat on the hottest + // insert path in the app. After dropping it the planner still serves + // `counter` lookups and (counter, beat_index) ordering from the auto-index + // — pinned by test/db_storage_hygiene_test.dart, same as the drop above. + await db.execute('DROP INDEX IF EXISTS idx_decoded_rr_counter'); } /// Rebuild the decoded substrate into noop-style canonical time-keyed rows: @@ -3106,11 +3141,17 @@ class LocalDb { }) async { final db = await instance; final now = DateTime.now().millisecondsSinceEpoch; + // THE write seam for the compact curve format. All four callers + // (DerivationEngine x2, cloud_import, whoop_import) funnel through here, so + // no producer needs to know the wire format exists — upstream code keeps + // merging and patching plain [{t,v}] lists in memory. Lossless or no-op: + // SeriesCodec leaves anything it cannot encode exactly as it found it. + final encodedPayload = SeriesCodec.encodePayloadJson(payloadJson); await db.transaction((txn) async { await txn.insert('day_result', { 'day_id': dayId, 'algo_version': algoVersion, - 'payload_json': payloadJson, + 'payload_json': encodedPayload, 'window_json': windowJson, 'computed_at': now, 'finalized': finalized ? 1 : 0, @@ -4166,16 +4207,9 @@ class LocalDb { final rawByDay = await decodedRecTsMaxByDay(); final out = >[]; for (final row in rows) { - final payload = row['payload_json'] as String?; - Map decoded = const {}; - if (payload != null && payload.isNotEmpty) { - try { - final d = jsonDecode(payload); - if (d is Map) decoded = d.cast(); - } catch (_) { - /* ignore */ - } - } + final decoded = + SeriesCodec.decodePayloadJson(row['payload_json']) ?? + const {}; final scalars = ((decoded['scalars'] as Map?) ?? const {}) .cast(); final dayId = row['day_id'] as String? ?? ''; @@ -4384,6 +4418,128 @@ class LocalDb { return rows.isEmpty ? null : rows.first; } + /// Bookkeeping key for the one-time walk in [reencodeLegacyDayResults]. + static const String kReencodeCursorKey = 'series_reencode'; + + /// Re-encode a BOUNDED batch of pre-codec `day_result` rows into the compact + /// curve format, newest first. Returns how many rows were rewritten. + /// + /// WHY A BACKFILL AT ALL. `SeriesCodec` reads the legacy shape forever, so + /// nothing breaks without this — but a user's existing history would stay at + /// ~88 KB/day while only new days shrank, and `day_result` is precisely the + /// store that grows without bound. This converts the back catalogue once. + /// + /// WHERE IT RUNS. Called from the derivation engine's post-derive + /// housekeeping, beside `pruneSupersededIntermediates` — off the path to a + /// durable commit, and NEVER inside a migration: `onUpgrade` runs inside + /// `openDatabase` under iOS's CPU watchdog, where rewriting a year of bundles + /// would be a launch hang (invariant 11). + /// + /// A FORWARD-ONLY CURSOR, not a rescan. Progress is stored in + /// `compute_freshness`, so each call walks strictly older days than the last + /// and the whole history costs one pass. Re-scanning from the newest day + /// every time would re-read (and re-parse) every already-converted bundle + /// forever — tens of MB of I/O per derivation. + /// + /// IMMUTABILITY. `day_result` rows are immutable PER VERSION, meaning their + /// derived VALUES never change without a `kAlgoVersion` bump. This rewrite + /// changes only how those same values are spelled, and every row is gated on + /// [SeriesCodec.verifyLossless] before it is touched — a bundle whose + /// round-trip is not provably exact is skipped and left legacy. Nothing but + /// `payload_json` is written: `computed_at`, `finalized`, `partial` and the + /// indexed scalars are untouched, so no day is re-finalized or re-dated. + static Future reencodeLegacyDayResults({int limit = 40}) async { + final db = await instance; + + String? cursorDay; + int? cursorVersion; + final prev = await computeFreshness(kReencodeCursorKey); + final prevJson = prev?['payload_json']; + if (prevJson is String && prevJson.isNotEmpty) { + try { + final d = jsonDecode(prevJson); + if (d is Map) { + if (d['done'] == true) return 0; // whole history already walked + final c = d['cursor']; + if (c is String && c.isNotEmpty) cursorDay = c; + final v = d['cursor_version']; + if (v is int) cursorVersion = v; + } + } catch (_) { + /* unreadable bookkeeping ⇒ start over; the walk is idempotent */ + } + } + + // The cursor is the COMPOSITE key, not just the day. `day_result` is keyed + // (day_id, algo_version) and one day can hold several generations, so a + // day-only cursor stepped straight past a day's older rows and left them + // legacy forever. + // + // Spelled out rather than as the row-value form `(day_id, algo_version) < + // (?, ?)`: row values need SQLite 3.15, and on Android sqflite uses the + // OS's SQLite, which is older than that on the devices this app still + // supports. + final String? where; + final List? whereArgs; + if (cursorDay == null) { + where = null; + whereArgs = null; + } else if (cursorVersion == null) { + where = 'day_id < ?'; + whereArgs = [cursorDay]; + } else { + where = 'day_id < ? OR (day_id = ? AND algo_version < ?)'; + whereArgs = [cursorDay, cursorDay, cursorVersion]; + } + + final rows = await db.query( + 'day_result', + columns: ['day_id', 'algo_version', 'payload_json'], + where: where, + whereArgs: whereArgs, + orderBy: 'day_id DESC, algo_version DESC', + limit: limit, + ); + if (rows.isEmpty) { + await putComputeFreshness(kReencodeCursorKey, jsonEncode({'done': true})); + return 0; + } + + var rewritten = 0; + await db.transaction((txn) async { + for (final row in rows) { + final pj = row['payload_json']; + if (pj is! String || pj.isEmpty) continue; + if (!SeriesCodec.needsReencode(pj)) continue; + if (!SeriesCodec.verifyLossless(pj)) continue; + final encoded = SeriesCodec.encodePayloadJson(pj); + if (encoded.length >= pj.length) continue; // never grow a row + await txn.update( + 'day_result', + {'payload_json': encoded}, + where: 'day_id = ? AND algo_version = ?', + whereArgs: [row['day_id'], row['algo_version']], + ); + rewritten++; + } + }); + + // The cursor advances past every row we LOOKED at, not just the ones we + // rewrote — a row we skipped (already encoded, or not provably lossless) + // would otherwise be re-examined on every future pass and the walk would + // never terminate. + await putComputeFreshness( + kReencodeCursorKey, + jsonEncode({ + 'cursor': rows.last['day_id'], + 'cursor_version': rows.last['algo_version'], + 'done': rows.length < limit, + 'rewritten_last': rewritten, + }), + ); + return rewritten; + } + static Future putComputeFreshness( String key, String payloadJson, @@ -4415,16 +4571,9 @@ class LocalDb { final dayId = row['day_id']?.toString(); if (dayId == null || dayId.isEmpty) continue; if (dayId == today && todayRow == null) todayRow = row; - final payload = row['payload_json'] as String?; - Map decoded = const {}; - if (payload != null && payload.isNotEmpty) { - try { - final d = jsonDecode(payload); - if (d is Map) decoded = d.cast(); - } catch (_) { - decoded = const {}; - } - } + final decoded = + SeriesCodec.decodePayloadJson(row['payload_json']) ?? + const {}; if (decoded['skipped'] == true) continue; final scalars = ((decoded['scalars'] as Map?) ?? const {}) .cast(); diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 543add71..65500613 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -28,6 +28,7 @@ import 'day_label.dart'; import 'db.dart'; import 'journal_fields.dart'; import 'local_repository.dart'; +import 'series_codec.dart'; import '../gps/route_models.dart'; import '../gps/route_math.dart' as rmath; @@ -109,15 +110,15 @@ class LocalRepositoryImpl extends LocalRepository { await _bundle(date) ?? (_isTodayLabel(date) ? await _latestBundle() : null); - static Map? _decode(Object? json) { - if (json is! String) return null; - try { - final d = jsonDecode(json); - return d is Map ? d.cast() : null; - } catch (_) { - return null; - } - } + /// THE read seam for the compact curve format: every bundle this class serves + /// comes through here, so downstream readers keep seeing plain [{t,v}] lists + /// and none of them has to know the wire format exists. + /// + /// Safe on the non-day_result payloads that also use it (baselines, + /// freshness, wake features): SeriesCodec only rewrites keys already in + /// grid/offset shape, which nothing but `putDayResult` ever writes. + static Map? _decode(Object? json) => + SeriesCodec.decodePayloadJson(json); /// Pull a sub-map by dotted path (e.g. 'clinical.hrv_time'). Map? _sub(Map? b, String path) { diff --git a/lib/data/series_codec.dart b/lib/data/series_codec.dart new file mode 100644 index 00000000..55e09435 --- /dev/null +++ b/lib/data/series_codec.dart @@ -0,0 +1,310 @@ +// series_codec.dart — the compact wire format for the intra-day curves inside +// `day_result.payload_json`. +// +// WHY. A curve used to be stored as one JSON object per sample: +// +// [{"t":1783572180,"v":77},{"t":1783572240,"v":80}] +// +// That is 27 bytes to carry two numbers, repeating the full 10-digit epoch in +// every element, for a curve that samples on a fixed 60-second grid. `series` +// was 74.5 KB of an 88 KB bundle and `day_result` is the ONE store that grows +// without bound (raw/decoded are capped at `rawRetentionDays`). +// +// WHY NOT gzip. `payload_json` is read by SQL, not just by Dart — the coach's +// `v_series` / `v_hypnogram` views run `json_each(json_extract(payload_json, +// '$.series.…'))` over it (db.dart `_ensureCoachViews`). A compressed BLOB is +// opaque to json1, and sqflite exposes no way to register a decompress +// function, so compressing the column would silently strip every intra-day +// curve from the coach (invariant 13). Everything here therefore stays PLAIN +// JSON that json1 can still walk. +// +// THREE SHAPES, all readable forever: +// +// legacy [{"t":N,"v":X}, …] never written again +// grid {"t0":N,"dt":N,"v":[X, …]} regular sampling +// offset {"t0":N,"to":[N, …],"v":[X, …]} irregular sampling +// +// `grid` reconstructs in pure SQL because json_each exposes an array's index as +// `key`: t = t0 + key*dt. `offset` pairs `to` and `v` on that same `key`. +// +// Legacy staying readable is what makes this migration-free: no rewrite pass +// runs inside `openDatabase` under iOS's CPU watchdog (invariant 11). Old rows +// are re-encoded later by a bounded background pass, off the durable-commit +// path. +// +// PURE. No I/O, no plugins, no Flutter — safe on any isolate. + +import 'dart:convert'; + +/// Encoder/decoder for the curve shapes stored in `day_result.payload_json`. +/// +/// The invariant every method here upholds: **encode → decode is lossless, or +/// the curve is left alone.** There is no shape this file can write that it +/// cannot read back exactly, and anything it cannot encode losslessly passes +/// through untouched. The fallback is always "stay legacy", never "lose data". +class SeriesCodec { + SeriesCodec._(); + + /// Curves under `payload['series']`, mapped to the key their samples use for + /// the value. `zone_timeline` is the odd one out — it carries `z`, not `v` + /// (matching the `v_series` view, which reads `$.z` for that branch alone). + static const Map seriesCurves = { + 'hr_curve': 'v', + 'strain_curve': 'v', + 'hrv_timeline': 'v', + 'hrv_day': 'v', + 'resp_day': 'v', + 'skin_temp_day': 'v', + 'zone_timeline': 'z', + }; + + /// Curves living at the bundle ROOT rather than under `series`. + /// `activity_curve` is surfaced by `v_series` like the rest, so it gets the + /// same treatment. + static const Map rootCurves = {'activity_curve': 'v'}; + + /// Below this, the envelope (`t0`/`dt`/`to` keys) costs more than the + /// per-sample repetition it removes, so encoding is not worth it. + static const int minPoints = 3; + + // ── encode ───────────────────────────────────────────────────────────────── + + /// Encode one curve to `grid` or `offset`, or return [raw] UNCHANGED when it + /// cannot be encoded losslessly. + /// + /// Refuses (and so leaves legacy) when any of these hold, because each one + /// would make the round-trip lossy or change what SQL sees: + /// • fewer than [minPoints] samples + /// • an element that is not a Map, or whose keys are not exactly + /// `{t, valueKey}` — an extra key would be dropped by the columnar form + /// • a `t` that is not an `int` — a double `t` would come back out of the + /// SQL branch as `t0 + key*dt` in a different numeric type than + /// `json_extract($.t)` produced before + static Object? encodeCurve(Object? raw, {String valueKey = 'v'}) { + if (raw is! List || raw.length < minPoints) return raw; + + final ts = []; + final vs = []; + for (final e in raw) { + if (e is! Map) return raw; + // Exactly {t, valueKey} — nothing else survives the columnar form. + if (e.length != 2 || !e.containsKey('t') || !e.containsKey(valueKey)) { + return raw; + } + final t = e['t']; + if (t is! int) return raw; + ts.add(t); + vs.add(e[valueKey]); + } + + // A single positive delta across the whole curve ⇒ a true grid. + final dt = ts[1] - ts[0]; + if (dt > 0) { + var regular = true; + for (var i = 2; i < ts.length; i++) { + if (ts[i] - ts[i - 1] != dt) { + regular = false; + break; + } + } + if (regular) return {'t0': ts[0], 'dt': dt, 'v': vs}; + } + + final t0 = ts[0]; + return { + 't0': t0, + 'to': [for (final t in ts) t - t0], + 'v': vs, + }; + } + + /// Encode every known curve in a decoded bundle and return the result. + /// + /// PURE — the argument is not modified. Rebuilding rather than writing + /// through matters: `Map` accepts a caller's more narrowly + /// inferred map (a literal of nothing but curves infers as + /// `Map>`), and storing an encoded object into that throws at + /// runtime. The shallow copies are a few dozen entries against an ~88 KB + /// bundle. + /// + /// Idempotent: an already-encoded curve is not a `List`, so [encodeCurve] + /// hands it straight back. + static Map encodePayload(Map payload) { + final out = Map.from(payload); + final series = out['series']; + if (series is Map) { + final encodedSeries = Map.from(series); + for (final entry in seriesCurves.entries) { + if (!encodedSeries.containsKey(entry.key)) continue; + encodedSeries[entry.key] = encodeCurve( + encodedSeries[entry.key], + valueKey: entry.value, + ); + } + out['series'] = encodedSeries; + } + for (final entry in rootCurves.entries) { + if (!out.containsKey(entry.key)) continue; + out[entry.key] = encodeCurve(out[entry.key], valueKey: entry.value); + } + return out; + } + + /// Encode a serialized bundle. Returns [payloadJson] unchanged when it is not + /// a JSON object — a caller must never lose a payload to this optimization. + static String encodePayloadJson(String payloadJson) { + if (payloadJson.isEmpty) return payloadJson; + try { + final decoded = jsonDecode(payloadJson); + if (decoded is! Map) return payloadJson; + return jsonEncode(encodePayload(decoded.cast())); + } catch (_) { + return payloadJson; + } + } + + // ── decode ───────────────────────────────────────────────────────────────── + + /// Normalize one curve back to the legacy `[{t, valueKey}, …]` shape. + /// + /// A `List` (legacy) is returned as-is. A malformed envelope yields an EMPTY + /// curve rather than throwing — the same observable outcome callers already + /// get from a missing key, and the contract every decode path in this repo + /// keeps. + static Object? decodeCurve(Object? raw, {String valueKey = 'v'}) { + if (raw is! Map) return raw; + + final t0 = raw['t0']; + final vs = raw['v']; + if (t0 is! int || vs is! List) return const []; + + final dt = raw['dt']; + if (dt is int) { + return [ + for (var i = 0; i < vs.length; i++) {'t': t0 + i * dt, valueKey: vs[i]}, + ]; + } + + final to = raw['to']; + if (to is! List || to.length != vs.length) return const []; + return [ + for (var i = 0; i < vs.length; i++) + if (to[i] is int) {'t': t0 + (to[i] as int), valueKey: vs[i]}, + ]; + } + + /// Normalize every known curve in a decoded bundle and return the result. + /// + /// PURE, for the same reason as [encodePayload]. + /// + /// THE single read-side entry point. Every site that turns a stored + /// `payload_json` string into a Map calls this, so no downstream reader has + /// to know the wire format exists (§4.7: one concern, all call sites). + /// + /// Safe on payloads that are not day bundles (baselines, freshness, wake + /// features all share `local_repository_impl._decode`): it only rewrites keys + /// already in grid/offset shape, which nothing but the write seam produces. + /// Idempotent — a legacy `List` is handed straight back. + static Map? decodePayload(Map? payload) { + if (payload == null) return null; + final out = Map.from(payload); + final series = out['series']; + if (series is Map) { + final decodedSeries = Map.from(series); + for (final entry in seriesCurves.entries) { + final cur = decodedSeries[entry.key]; + if (cur is! Map) continue; // legacy or absent — nothing to do + decodedSeries[entry.key] = decodeCurve(cur, valueKey: entry.value); + } + out['series'] = decodedSeries; + } + for (final entry in rootCurves.entries) { + final cur = out[entry.key]; + if (cur is! Map) continue; + out[entry.key] = decodeCurve(cur, valueKey: entry.value); + } + return out; + } + + /// Decode a serialized bundle straight to a normalized Map, or null when it + /// is absent/unparseable. Mirrors the `try/catch → null` contract of the + /// existing `_decode` helpers. + static Map? decodePayloadJson(Object? payloadJson) { + if (payloadJson is! String || payloadJson.isEmpty) return null; + try { + final decoded = jsonDecode(payloadJson); + if (decoded is! Map) return null; + return decodePayload(decoded.cast()); + } catch (_) { + return null; + } + } + + /// True when re-encoding [payloadJson] provably loses nothing: the encoded + /// form decodes back to exactly what the original decodes to. + /// + /// The round-trip is unit-tested, but the backfill uses this as a per-row + /// gate before OVERWRITING durable user data. A day older than + /// `rawRetentionDays` has no substrate left to re-derive from, so a lossy + /// rewrite there would be unrecoverable — cheap insurance against a future + /// bundle shape this codec has never seen. + static bool verifyLossless(String payloadJson) { + try { + final original = jsonDecode(payloadJson); + if (original is! Map) return false; + final reencoded = decodePayloadJson( + encodePayloadJson(jsonEncode(original)), + ); + return _deepEquals( + decodePayload(original.cast()), + reencoded, + ); + } catch (_) { + return false; + } + } + + static bool _deepEquals(Object? a, Object? b) { + if (identical(a, b)) return true; + if (a is Map && b is Map) { + if (a.length != b.length) return false; + for (final k in a.keys) { + if (!b.containsKey(k) || !_deepEquals(a[k], b[k])) return false; + } + return true; + } + if (a is List && b is List) { + if (a.length != b.length) return false; + for (var i = 0; i < a.length; i++) { + if (!_deepEquals(a[i], b[i])) return false; + } + return true; + } + return a == b; + } + + /// True when [payloadJson] still holds at least one legacy-shaped curve, i.e. + /// re-encoding it would shrink the row. Used by the background backfill to + /// skip rows already converted without paying a full encode. + static bool needsReencode(String payloadJson) { + if (payloadJson.isEmpty) return false; + try { + final decoded = jsonDecode(payloadJson); + if (decoded is! Map) return false; + final series = decoded['series']; + if (series is Map) { + for (final key in seriesCurves.keys) { + final cur = series[key]; + if (cur is List && cur.length >= minPoints) return true; + } + } + for (final key in rootCurves.keys) { + final cur = decoded[key]; + if (cur is List && cur.length >= minPoints) return true; + } + return false; + } catch (_) { + return false; + } + } +} diff --git a/lib/health/health_export.dart b/lib/health/health_export.dart index 492f9045..8440e9a0 100644 --- a/lib/health/health_export.dart +++ b/lib/health/health_export.dart @@ -25,6 +25,7 @@ import 'package:flutter/foundation.dart'; import 'package:health/health.dart'; import '../data/db.dart'; +import '../data/series_codec.dart'; import 'health_heart_rate_batch.dart'; import 'health_sleep_session.dart'; @@ -1030,15 +1031,12 @@ class HealthExporter { HealthWorkoutActivityType _activity(String? type) => healthActivityForType(type, ios: isApple); - static Map? _decode(Object? json) { - if (json is! String) return null; - try { - final d = jsonDecode(json); - return d is Map ? d.cast() : null; - } catch (_) { - return null; - } - } + /// Decode a stored day bundle, normalizing the compact curve format back to + /// plain [{t,v}] lists. Hypnogram segments are never encoded (no `t` key), so + /// today only the sleep export reads through here — but every day_result + /// reader goes through the codec so a future one cannot silently miss it. + static Map? _decode(Object? json) => + SeriesCodec.decodePayloadJson(json); static Map? _sub(Map? b, String path) { var cur = b; diff --git a/lib/import/import_container.dart b/lib/import/import_container.dart index 384f0fb9..cdfb1d02 100644 --- a/lib/import/import_container.dart +++ b/lib/import/import_container.dart @@ -177,6 +177,56 @@ class ResolvedNoopDatabase { } } +/// Inflate a gzip file into [dir] and return the path, or null if [path] is not +/// gzip. +/// +/// STREAMED with a running ceiling rather than decoded in memory. A gzip +/// declares nothing about its output size, so the only way to refuse a +/// pathological one is to count bytes as they land and stop — and the file this +/// most often is (a full database backup) is exactly the size that must never +/// be buffered twice. +/// +/// The caller owns [dir] and the file inside it. +Future inflateGzip(String path, Directory dir) async { + if (await sniffFile(path) != ImportContainer.gzip) return null; + + var base = p.basename(path); + if (base.toLowerCase().endsWith('.gz')) { + base = base.substring(0, base.length - 3); + } + if (base.isEmpty) base = 'inflated'; + final destPath = p.join(dir.path, base); + final sink = File(destPath).openWrite(); + var written = 0; + try { + await for (final chunk in File(path).openRead().transform(gzip.decoder)) { + written += chunk.length; + if (written > _kMaxUncompressedBytes) { + throw ImportFormatException( + '“${p.basename(path)}” unpacks to more than ' + '${_kMaxUncompressedBytes ~/ (1024 * 1024 * 1024)} GB, which is not ' + 'something we can import.', + ); + } + sink.add(chunk); + } + await sink.close(); + } catch (e) { + try { + await sink.close(); + } catch (_) {} + try { + final partial = File(destPath); + if (await partial.exists()) await partial.delete(); + } catch (_) {} + if (e is ImportFormatException) rethrow; + throw ImportFormatException( + 'Could not read “${p.basename(path)}” as a gzip archive: $e', + ); + } + return destPath; +} + /// If [path] is a NOOP full backup, return its database ready to open. /// /// Handles both shapes users arrive with: the `.noopbak` itself (a ZIP whose @@ -192,6 +242,26 @@ Future resolveNoopDatabase(String path) async { return ResolvedNoopDatabase(path, null); case ImportContainer.zip: break; + case ImportContainer.gzip: + // A gzipped database — the shape this app's own auto-backups take, and + // what `gzip -k` leaves behind for anyone compressing an export by hand. + // Inflate, then re-sniff: only a real SQLite file is claimed here, so a + // gzipped CSV still falls through to the CSV path. + final tempDir = await Directory.systemTemp.createTemp('openstrap_gz_'); + try { + final inflated = await inflateGzip(path, tempDir); + if (inflated != null && + await sniffFile(inflated) == ImportContainer.sqlite) { + return ResolvedNoopDatabase(inflated, tempDir); + } + } catch (_) { + // Not readable as gzip — let the CSV path produce the user-facing + // message rather than throwing a database-flavoured one here. + } + try { + if (tempDir.existsSync()) await tempDir.delete(recursive: true); + } catch (_) {} + return null; default: return null; } @@ -328,10 +398,27 @@ Future resolveImportCsvPaths( 'export.', ); case ImportContainer.gzip: - throw ImportFormatException( - '“${p.basename(path)}” is a gzip archive. Unzip it first and pick ' - 'the CSV inside.', - ); + // Used to be a flat refusal ("unzip it first"). It is inflated now: + // gzip is what every command-line tool and most file managers produce + // when someone compresses a CSV, and it is the shape this app's own + // auto-backups take. + tempDir ??= + await Directory.systemTemp.createTemp('openstrap_import_'); + final gzInto = Directory(p.join(tempDir.path, 'a${archiveIndex++}')); + await gzInto.create(recursive: true); + final inflated = await inflateGzip(path, gzInto); + // Re-sniff rather than assume: a gzipped ZIP or database is still not + // a CSV, and the message for those should say so. + final inner = inflated == null + ? ImportContainer.binary + : await sniffFile(inflated); + if (inner != ImportContainer.text) { + throw ImportFormatException( + '“${p.basename(path)}” unpacks to something that is not a ' + '$flavor CSV export.', + ); + } + out.add(inflated!); case ImportContainer.utf16: throw ImportFormatException( '“${p.basename(path)}” is saved as UTF-16 text. Re-save it as ' diff --git a/lib/import/whoop_import.dart b/lib/import/whoop_import.dart index 4e22236c..55af1d60 100644 --- a/lib/import/whoop_import.dart +++ b/lib/import/whoop_import.dart @@ -18,6 +18,7 @@ import '../compute/derivation_engine.dart' show kAlgoVersion, DerivationEngine; import '../compute/profile.dart'; import '../compute/substrate.dart' show localDateLabel; import '../data/db.dart'; +import '../data/series_codec.dart'; import 'import_container.dart'; class WhoopImportResult { @@ -193,8 +194,10 @@ class WhoopImporter { if (row == null) return false; if (((row['skipped'] as num?) ?? 0).toInt() == 1) return false; try { - final p = jsonDecode((row['payload_json'] as String?) ?? '{}'); - if (p is Map) { + final p = SeriesCodec.decodePayloadJson( + (row['payload_json'] as String?) ?? '{}', + ); + if (p != null) { if (p['skipped'] == true) return false; // A prior import (this importer, or the cloud one) is replaceable — // both are vendor snapshots, neither is measured on-device data. diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index dbca2b13..5f54277c 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -55,6 +55,7 @@ import '../gps/gps_source.dart'; import '../gps/route_tracker.dart'; import '../gps/screen_wake.dart'; import '../data/local_repository_impl.dart'; +import '../data/series_codec.dart'; import '../notify/battery_forecast.dart'; import '../notify/notification_center.dart'; import '../notify/notification_event.dart'; @@ -1301,8 +1302,10 @@ class AppState extends ChangeNotifier { // Sleep hours from the day's bundle accounting (tst), for the body copy. String slept = ''; try { - final payload = jsonDecode((row['payload_json'] ?? '{}').toString()); - if (payload is Map) { + final payload = SeriesCodec.decodePayloadJson( + (row['payload_json'] ?? '{}').toString(), + ); + if (payload != null) { final acct = ((payload['sleep'] as Map?)?['accounting'] as Map?); final tstSec = ((acct?['value'] as Map?)?['tst_sec'] as num?) ?.toDouble(); diff --git a/test/auto_backup_test.dart b/test/auto_backup_test.dart index 5ea69f5c..1b102593 100644 --- a/test/auto_backup_test.dart +++ b/test/auto_backup_test.dart @@ -147,7 +147,7 @@ void main() { test('is zero-padded so widths match', () { expect(backupFileName(DateTime(2026, 1, 2, 3, 4, 5)), - 'openstrap-20260102-030405.db'); + 'openstrap-20260102-030405.db.gz'); }); test('two runs in the same minute get different names', () { @@ -198,12 +198,37 @@ void main() { touch('openstrap-2026.db'); touch('openstrap-20260101.db'); touch('random.db'); + touch('openstrap-20260101-000000.db.gz.bak'); final out = sortBackupsNewestFirst(tmp.listSync()); expect(out.map((f) => p.basename(f.path)), [ + 'openstrap-20260101-000000.db.gz', + ]); + }); + + test('still matches the UNCOMPRESSED names earlier versions wrote', () { + // An install that upgrades still holds up to kBackupsKept plain `.db` + // backups. If the pattern stopped matching them they would never be + // counted toward retention and never pruned — five full-size copies + // leaked permanently, which is the opposite of the point. + touch('openstrap-20260101-000000.db'); + touch('openstrap-20260102-000000.db.gz'); + final out = sortBackupsNewestFirst(tmp.listSync()); + expect(out.map((f) => p.basename(f.path)), [ + 'openstrap-20260102-000000.db.gz', 'openstrap-20260101-000000.db', ]); }); + test('matches the -N collision names _uniqueDestination emits', () { + // These leaked for the same reason: two runs inside one second produce a + // `-2` suffix that the pattern never covered, so the file was invisible + // to retention forever. + touch('openstrap-20260101-000000.db.gz'); + touch('openstrap-20260101-000000-2.db.gz'); + touch('openstrap-20260101-000000-3.db'); + expect(sortBackupsNewestFirst(tmp.listSync()).length, 3); + }); + test('an empty directory is empty, not an error', () { expect(sortBackupsNewestFirst(tmp.listSync()), isEmpty); }); @@ -385,15 +410,57 @@ void main() { ); }); + test('a backup is gzip on disk and inflates back to the database', () async { + // The whole point of the extension change. A backup that is smaller but + // cannot be read back is not a backup, so this asserts BOTH: the file is + // really gzip, and what comes out of it is really the snapshot. + final outcome = await runBackup(now: DateTime(2026, 8, 9, 15, 0, 0)); + expect(outcome.succeeded, isTrue, reason: outcome.error); + + final file = File(outcome.path!); + expect(p.basename(file.path), endsWith('.db.gz')); + + final bytes = await file.readAsBytes(); + expect(bytes.length, greaterThan(2)); + expect(bytes[0], 0x1F, reason: 'gzip magic byte 0'); + expect(bytes[1], 0x8B, reason: 'gzip magic byte 1'); + + final inflated = gzip.decode(bytes); + expect( + String.fromCharCodes(inflated.take(15)), + 'SQLite format 3', + reason: 'the inflated backup must be an openable database', + ); + expect( + inflated.length, + greaterThan(bytes.length), + reason: 'a compressed backup must be smaller than the database', + ); + }); + + test('a snapshot is never left behind in temp', () async { + // The export is a full second copy of the database. The old code renamed + // it into place; the new one streams and must still delete the source. + final outcome = await runBackup(now: DateTime(2026, 8, 9, 16, 0, 0)); + expect(outcome.succeeded, isTrue, reason: outcome.error); + final leftovers = tmp + .listSync() + .whereType() + .map((f) => p.basename(f.path)) + .where((n) => n.startsWith('openstrap_export_')) + .toList(); + expect(leftovers, isEmpty); + }); + test('an occupied destination is never handed back', () async { // Returning the last candidate would give the next backup a real // snapshot to overwrite — the exact loss the unique naming prevents. final when = DateTime(2026, 8, 9, 14, 0, 0); final dir = await backupDirectory(); final base = backupFileName(when); - final stem = base.substring(0, base.length - 3); + final stem = base.substring(0, base.length - kBackupExtension.length); for (var i = 1; i < 100; i++) { - File(p.join(dir.path, i == 1 ? base : '$stem-$i.db')) + File(p.join(dir.path, i == 1 ? base : '$stem-$i$kBackupExtension')) .writeAsStringSync('occupied'); } @@ -412,11 +479,11 @@ void main() { expect(exported, 0, reason: 'nothing should have been exported'); // Every pre-existing file is untouched. for (var i = 1; i < 100; i++) { - final f = File(p.join(dir.path, i == 1 ? base : '$stem-$i.db')); + final f = File(p.join(dir.path, i == 1 ? base : '$stem-$i$kBackupExtension')); expect(f.readAsStringSync(), 'occupied'); } for (var i = 1; i < 100; i++) { - File(p.join(dir.path, i == 1 ? base : '$stem-$i.db')).deleteSync(); + File(p.join(dir.path, i == 1 ? base : '$stem-$i$kBackupExtension')).deleteSync(); } }); }); diff --git a/test/coach_views_series_shapes_test.dart b/test/coach_views_series_shapes_test.dart new file mode 100644 index 00000000..84d5e13d --- /dev/null +++ b/test/coach_views_series_shapes_test.dart @@ -0,0 +1,217 @@ +// v_series must be BLIND to how a curve is stored. +// +// day_result.payload_json holds curves in three shapes at once — `legacy` +// ([{t,v},…]) from before data/series_codec.dart existed, and the `grid` +// ({t0,dt,v[]}) / `offset` ({t0,to[],v[]}) forms written since. Old rows keep +// their legacy shape forever (there is no rewriting migration), so a real +// database holds a MIXTURE and the coach must not be able to tell. +// +// This is the regression pin for that: the same day, stored both ways, has to +// come out of the view identically — and a mixed database must not emit a row +// twice or drop one, which is what a wrong branch guard would do. +// +// It also pins the reason the payload could not simply be gzipped: these views +// read the column with SQL (json_each/json_extract), so the stored bytes have +// to stay something json1 can walk. + +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/data/db.dart'; +import 'package:openstrap_edge/data/series_codec.dart'; + +/// A day bundle covering every curve the view exposes, in LEGACY shape: +/// • a regular grid (hr_curve, strain_curve, skin_temp_day) +/// • an irregular curve (hrv_day, resp_day) +/// • the odd value key (zone_timeline → 'z') +/// • a root-level curve (activity_curve) +/// • a curve too short to encode, which must stay legacy (hrv_timeline) +/// • hypnogram, which has no `t` and must never be touched +Map legacyBundle(int t0) => { + 'scalars': {'rhr': 55.0}, + 'series': { + 'hr_curve': [ + for (var i = 0; i < 12; i++) {'t': t0 + i * 60, 'v': 60 + i}, + ], + 'strain_curve': [ + for (var i = 0; i < 8; i++) {'t': t0 + i * 60, 'v': i * 0.37}, + ], + 'skin_temp_day': [ + for (var i = 0; i < 5; i++) {'t': t0 + i * 300, 'v': -1.5 + i}, + ], + // Irregular on purpose — this is the branch that pairs `to` with `v`. + 'hrv_day': [ + {'t': t0 + 9, 'v': 36.8}, + {'t': t0 + 71, 'v': 56.1}, + {'t': t0 + 325, 'v': 72.5}, + {'t': t0 + 400, 'v': 41.2}, + ], + 'resp_day': [ + {'t': t0 + 52, 'v': 14.9}, + {'t': t0 + 2738, 'v': 15.4}, + {'t': t0 + 3548, 'v': 13.1}, + ], + 'zone_timeline': [ + for (var i = 0; i < 8; i++) {'t': t0 + i * 60, 'z': i % 4}, + ], + // Two points — below minPoints, so it must survive as a legacy array even + // in the "encoded" row. + 'hrv_timeline': [ + {'t': 9, 'v': 36.8}, + {'t': 69, 'v': 44.1}, + ], + 'hypnogram': [ + {'start': t0, 'end': t0 + 3600, 'stage': 'light'}, + {'start': t0 + 3600, 'end': t0 + 4200, 'stage': 'deep'}, + {'start': t0 + 4200, 'end': t0 + 7200, 'stage': 'rem'}, + ], + }, + 'activity_curve': [ + for (var i = 0; i < 10; i++) {'t': t0 + i * 300, 'v': i * 1.5}, + ], +}; + +Future insertDay( + Database db, + String dayId, + Map bundle, +) async { + await db.insert('day_result', { + 'day_id': dayId, + 'algo_version': 47, + 'payload_json': jsonEncode(bundle), + 'window_json': '{}', + 'computed_at': 0, + 'finalized': 0, + }); +} + +Future>> seriesRows(Database db, String dayId) async => + db.rawQuery( + 'SELECT series, t, v FROM v_series WHERE date = ? ' + 'ORDER BY series ASC, t ASC, v ASC', + [dayId], + ); + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late Database db; + const t0 = 1783572180; + + setUpAll(() async { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + LocalDb.dbName = 'openstrap_series_shapes_test.db'; + final dir = await databaseFactory.getDatabasesPath(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + db = await LocalDb.instance; + + await insertDay(db, '2026-01-01', legacyBundle(t0)); + await insertDay( + db, + '2026-01-02', + SeriesCodec.encodePayload(legacyBundle(t0)), + ); + }); + + tearDownAll(() async { + await LocalDb.close(); + final dir = await databaseFactory.getDatabasesPath(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + }); + + test('an encoded day and a legacy day yield identical view rows', () async { + final legacy = await seriesRows(db, '2026-01-01'); + final encoded = await seriesRows(db, '2026-01-02'); + + expect(legacy, isNotEmpty, reason: 'the fixture must produce rows at all'); + expect(encoded.length, legacy.length); + for (var i = 0; i < legacy.length; i++) { + expect(encoded[i]['series'], legacy[i]['series']); + expect(encoded[i]['t'], legacy[i]['t'], reason: 'row $i timestamp'); + expect(encoded[i]['v'], legacy[i]['v'], reason: 'row $i value'); + } + }); + + test('every curve the view exposes is actually covered', () async { + final got = (await seriesRows(db, '2026-01-02')) + .map((r) => r['series'] as String) + .toSet(); + expect(got, { + 'hr_curve', + 'strain_curve', + 'skin_temp_day', + 'hrv_day', + 'resp_day', + 'zone_timeline', + 'hrv_timeline', + 'activity_curve', + }); + }); + + test('the fixture really exercises all three shapes', () { + final encoded = SeriesCodec.encodePayload(legacyBundle(t0)); + final series = encoded['series'] as Map; + // grid + expect((series['hr_curve'] as Map).containsKey('dt'), isTrue); + // offset + expect((series['hrv_day'] as Map).containsKey('to'), isTrue); + // legacy passthrough — too short to encode + expect(series['hrv_timeline'], isA()); + // never touched + expect(series['hypnogram'], isA()); + }); + + test('a mixed database emits each row exactly once', () async { + // The branch guards are what prevent double-counting: legacy needs an + // `array`, grid needs `.dt`, offset needs `.to`. A row satisfying two would + // appear twice and silently double every curve the coach reads. + final dupes = await db.rawQuery(''' + SELECT date, series, t, COUNT(*) n FROM v_series + GROUP BY date, series, t, v HAVING n > 1 + '''); + expect(dupes, isEmpty); + + final total = await db.rawQuery('SELECT COUNT(*) c FROM v_series'); + final legacy = await seriesRows(db, '2026-01-01'); + expect( + (total.first['c'] as num).toInt(), + legacy.length * 2, + reason: 'both days must contribute the same number of rows', + ); + }); + + test('v_hypnogram is unaffected by the encoding', () async { + final legacy = await db.rawQuery( + "SELECT start_ts, end_ts, stage FROM v_hypnogram " + "WHERE date='2026-01-01' ORDER BY start_ts", + ); + final encoded = await db.rawQuery( + "SELECT start_ts, end_ts, stage FROM v_hypnogram " + "WHERE date='2026-01-02' ORDER BY start_ts", + ); + expect(legacy.length, 3); + expect(encoded, legacy); + }); + + test('the encoded row is materially smaller on disk', () async { + final rows = await db.rawQuery( + 'SELECT day_id, LENGTH(payload_json) n FROM day_result ORDER BY day_id', + ); + final legacyLen = (rows.first['n'] as num).toInt(); + final encodedLen = (rows.last['n'] as num).toInt(); + expect(encodedLen, lessThan(legacyLen)); + }); + + test('the stored payload is still valid JSON to SQLite', () async { + // The reason this is an encoding change and not a gzip: json1 has to be + // able to walk the column, or the views above cannot exist. + final rows = await db.rawQuery( + 'SELECT day_id FROM day_result WHERE NOT json_valid(payload_json)', + ); + expect(rows, isEmpty); + }); +} diff --git a/test/day_result_reencode_test.dart b/test/day_result_reencode_test.dart new file mode 100644 index 00000000..02470ae6 --- /dev/null +++ b/test/day_result_reencode_test.dart @@ -0,0 +1,248 @@ +// The one-time walk that converts pre-codec day_result rows to the compact +// curve format. +// +// This is the only code in the app that REWRITES a durable derived row, so the +// bar is higher than "it shrinks things": +// • values must survive exactly — a day older than rawRetentionDays has no +// substrate left to re-derive from, so a lossy rewrite is unrecoverable +// • nothing but payload_json may change — no re-dating, no re-finalizing +// • the walk must TERMINATE, and must not re-read converted rows forever +// • it must be safe to interrupt and resume, because it runs after derivation +// and the app can be killed at any point + +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/data/db.dart'; +import 'package:openstrap_edge/data/series_codec.dart'; + +Map bundleFor(int t0, {int n = 30}) => { + 'scalars': {'rhr': 55.0, 'readiness': 71.0}, + 'series': { + 'hr_curve': [ + for (var i = 0; i < n; i++) {'t': t0 + i * 60, 'v': 60 + (i % 17)}, + ], + 'hrv_day': [ + for (var i = 0; i < n; i++) {'t': t0 + i * 61 + (i % 5), 'v': 30.0 + i}, + ], + 'zone_timeline': [ + for (var i = 0; i < n; i++) {'t': t0 + i * 60, 'z': i % 4}, + ], + }, + 'activity_curve': [ + for (var i = 0; i < n; i++) {'t': t0 + i * 300, 'v': i * 1.5}, + ], +}; + +Future seedLegacy(Database db, String dayId, int t0) async { + await db.insert('day_result', { + 'day_id': dayId, + 'algo_version': 47, + 'payload_json': jsonEncode(bundleFor(t0)), + 'window_json': '{}', + 'computed_at': 1234567, + 'finalized': 1, + 'skipped': 0, + 'partial': 0, + 'rhr': 55.0, + 'rmssd': 41.0, + 'readiness': 71.0, + }); +} + +/// Reset the forward-only cursor so each test starts a fresh walk. +Future clearCursor(Database db) async { + await db.delete( + 'compute_freshness', + where: 'key = ?', + whereArgs: [LocalDb.kReencodeCursorKey], + ); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late Database db; + const t0 = 1783572180; + + setUp(() async { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + LocalDb.dbName = 'openstrap_reencode_test.db'; + final dir = await databaseFactory.getDatabasesPath(); + await LocalDb.close(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + db = await LocalDb.instance; + }); + + tearDown(() async { + await LocalDb.close(); + final dir = await databaseFactory.getDatabasesPath(); + await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); + }); + + test('rewrites legacy rows and shrinks them', () async { + await seedLegacy(db, '2026-01-01', t0); + final before = + (await db.query('day_result', columns: ['payload_json'])).first['payload_json'] + as String; + + expect(await LocalDb.reencodeLegacyDayResults(), 1); + + final after = + (await db.query('day_result', columns: ['payload_json'])).first['payload_json'] + as String; + expect(after.length, lessThan(before.length)); + expect(SeriesCodec.needsReencode(after), isFalse); + }); + + test('every value survives the rewrite exactly', () async { + await seedLegacy(db, '2026-01-01', t0); + await LocalDb.reencodeLegacyDayResults(); + + final after = SeriesCodec.decodePayloadJson( + (await db.query('day_result', columns: ['payload_json'])).first['payload_json'], + ); + expect(jsonEncode(after), jsonEncode(bundleFor(t0))); + }); + + test('nothing but payload_json is touched', () async { + // A rewrite that moved computed_at would look like a fresh derivation; one + // that cleared `finalized` would put a locked day back in the recompute + // queue. Neither is this function's business. + await seedLegacy(db, '2026-01-01', t0); + final before = (await db.query('day_result')).first; + await LocalDb.reencodeLegacyDayResults(); + final after = (await db.query('day_result')).first; + + for (final key in before.keys) { + if (key == 'payload_json') continue; + expect(after[key], before[key], reason: 'column $key changed'); + } + }); + + test('an already-encoded row is left alone and reports zero', () async { + await db.insert('day_result', { + 'day_id': '2026-01-01', + 'algo_version': 47, + 'payload_json': jsonEncode(SeriesCodec.encodePayload(bundleFor(t0))), + 'window_json': '{}', + 'computed_at': 0, + }); + expect(await LocalDb.reencodeLegacyDayResults(), 0); + }); + + test('the walk terminates and does not rescan converted rows', () async { + for (var d = 1; d <= 9; d++) { + await seedLegacy(db, '2026-01-0$d', t0 + d * 86400); + } + + // Small batches so the cursor has to carry progress across calls. + var total = 0; + var calls = 0; + while (calls < 20) { + final n = await LocalDb.reencodeLegacyDayResults(limit: 2); + calls++; + total += n; + if (n == 0) break; + } + expect(total, 9, reason: 'every seeded day should be converted once'); + + // Once done it stays done — further calls must not re-read anything. + expect(await LocalDb.reencodeLegacyDayResults(limit: 2), 0); + expect(await LocalDb.reencodeLegacyDayResults(limit: 2), 0); + + final rows = await db.query('day_result', columns: ['payload_json']); + for (final r in rows) { + expect(SeriesCodec.needsReencode(r['payload_json'] as String), isFalse); + } + }); + + test('it is resumable — an interrupted walk finishes later', () async { + for (var d = 1; d <= 6; d++) { + await seedLegacy(db, '2026-01-0$d', t0 + d * 86400); + } + expect(await LocalDb.reencodeLegacyDayResults(limit: 2), 2); + + // Simulate a relaunch mid-walk: the cursor is durable, the handle is not. + await LocalDb.close(); + db = await LocalDb.instance; + + var total = 2; + for (var i = 0; i < 10; i++) { + final n = await LocalDb.reencodeLegacyDayResults(limit: 2); + if (n == 0) break; + total += n; + } + expect(total, 6); + }); + + test('a corrupt cursor restarts the walk instead of wedging it', () async { + await seedLegacy(db, '2026-01-01', t0); + await LocalDb.putComputeFreshness(LocalDb.kReencodeCursorKey, 'not json'); + expect(await LocalDb.reencodeLegacyDayResults(), 1); + }); + + test('an unparseable payload is skipped, not destroyed', () async { + await db.insert('day_result', { + 'day_id': '2026-01-01', + 'algo_version': 47, + 'payload_json': '{ this is not json', + 'window_json': '{}', + 'computed_at': 0, + }); + await clearCursor(db); + expect(await LocalDb.reencodeLegacyDayResults(), 0); + final after = + (await db.query('day_result', columns: ['payload_json'])).first['payload_json']; + expect(after, '{ this is not json'); + }); + + test('a row it cannot shrink is left as it is', () async { + // Curves too short to encode: the walk must not write an equal-or-larger + // payload back just to say it did something. + final tiny = { + 'series': { + 'hr_curve': [ + {'t': t0, 'v': 60}, + {'t': t0 + 60, 'v': 61}, + ], + }, + }; + await db.insert('day_result', { + 'day_id': '2026-01-01', + 'algo_version': 47, + 'payload_json': jsonEncode(tiny), + 'window_json': '{}', + 'computed_at': 0, + }); + await clearCursor(db); + expect(await LocalDb.reencodeLegacyDayResults(), 0); + final after = + (await db.query('day_result', columns: ['payload_json'])).first['payload_json']; + expect(after, jsonEncode(tiny)); + }); + + test('every algo_version generation of a day is converted', () async { + // day_result is keyed (day_id, algo_version); a day can hold more than one + // generation and the walk must not stop at the newest. + for (final v in const [45, 46, 47]) { + await db.insert('day_result', { + 'day_id': '2026-01-01', + 'algo_version': v, + 'payload_json': jsonEncode(bundleFor(t0)), + 'window_json': '{}', + 'computed_at': 0, + }); + } + var total = 0; + for (var i = 0; i < 10; i++) { + final n = await LocalDb.reencodeLegacyDayResults(limit: 1); + if (n == 0) break; + total += n; + } + expect(total, 3); + }); +} diff --git a/test/db_storage_hygiene_test.dart b/test/db_storage_hygiene_test.dart index 199767f8..437f258f 100644 --- a/test/db_storage_hygiene_test.dart +++ b/test/db_storage_hygiene_test.dart @@ -37,6 +37,38 @@ void main() { expect(idx, contains('idx_decoded_rr_ts_beat_unique')); }); + test('the duplicate-of-primary-key rr index is gone', () async { + // idx_decoded_rr_counter(counter, beat_index) duplicated, column for column, + // the index PRIMARY KEY (counter, beat_index) already creates. Measured on a + // 3-day fill: both b-trees 3,264,512 bytes — ~1.09 MB/day of pure + // duplication plus a second b-tree write per beat on the hottest insert + // path in the app. + final db = await LocalDb.instance; + final idx = (await db.rawQuery( + "SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='decoded_rr'", + )).map((r) => r['name'] as String?).whereType().toList(); + expect(idx, isNot(contains('idx_decoded_rr_counter'))); + }); + + test('counter lookups are still index-served without it', () async { + // Dropping an index is only safe if the planner has another. The PK's + // auto-index covers exactly the same columns in the same order. + final db = await LocalDb.instance; + for (final sql in const [ + 'EXPLAIN QUERY PLAN SELECT * FROM decoded_rr WHERE counter = 42 ' + 'ORDER BY beat_index', + 'EXPLAIN QUERY PLAN SELECT * FROM decoded_rr WHERE counter BETWEEN 1 AND 9', + ]) { + final detail = (await db.rawQuery(sql)) + .map((r) => r['detail'].toString()) + .join(' | '); + expect(detail.toUpperCase(), contains('USING'), + reason: 'planner fell back to a full scan: $detail'); + expect(detail, contains('sqlite_autoindex_decoded_rr_1'), + reason: 'expected the primary key auto-index: $detail'); + } + }); + test('rr_ts_ms range scans are still served by an index', () async { final db = await LocalDb.instance; final plan = await db.rawQuery( diff --git a/test/import_container_test.dart b/test/import_container_test.dart index b8cf4de3606d811e4858f178a7dcc38a04bb0c1f..e76ccb58c58ee39c4f476af3ad795d5fadfdd40b 100644 GIT binary patch delta 2411 zcmaJ?+iu)M7`~}QRSBqQ6{)1v{ynI+Q?Ij|9*B~*&{GhiN@x=*aVYKB9`BA~do0g5 zhgQ+@2uQx>h6}_M;(@s05+NjB0QhFS_HI$>YUQ=pGymy3%=h2?dGO2P^5g{3@-mHH zh=MA`W73jBPK=V2X)1YADVCCZp~}q)p(zoi(!}zZ3K8v+9&@_4{e(oWM9eREK@;y2 zO+w*M69qwYxxZ#MVt3oy5G~4CmK9>d0l`h%iqW{~s zKL&9aj9xsq)sIS96|Q3@;`rox^b#A1G*{Ynn4MICmTc2=Et-jltdOOyKk6+6kwkck z)E?a;=iXtIqCm|`OXGlnOtS!6Pzo}e0s@{#GUjfjlhyVf z3p}rWc>2+CjTxHa>Ysyt{lkS*^&i1|tPSFL-nuQ^6weO|9%*+&q}-L24hp8nDo`RH zrQ8cj&f+_2TbFp=KFlN6^G)U5{v&{%voxJ%`|e_|mJ4UDnF7ruOvaEYZqSWVhW=t= z2Bh|d5BxzHGqU=xi>D{x+YSJm!=?c*psgH944B)G9tzDVVHkTzEiak#EbDoq-IFunyd5<4-86 zqpW$bJ_uc}-GmG9*kEc2 zVFHZZ7a+~35;4mo=mJiXa<&JfLlL&9kmDz=*Dm)aPgpAAsW6}i>wLLL5(4P~q;c6X zFFE{!l_48(yPQFOJnn<`a9;>~9qOB8 zhz61FJpO$1-kqHX_h-@UM5mL(8o@%Wi@ApL;1(5yJYqOw=stW9{tQ{e+e(UM(<$UD z(pfkO_ohrJ<5C2tMipgnqW-Mcn>b-9cX>YZ%0|Z>B7=kB1~huGde>YBtM)p;b^d!W zFpmd=t22&Ig>It-BMt;{dRe`@!lPTa$rZFjSC>AZD>NK>bQ$|MHad|!?G(|7H5O(4 z#j&MxqZiK#v@BTq%;}bN0$ujcpQZMw{22Uk4b^?ZZX3D&(z*T?k=@zMl$}I5jT!BJp&tKnoB!BZp`l$7ky)oLCjOI+bwxim8$;z*gFd* P(~+Iek;Thgk52pxn-b+m delta 10 RcmaD?F*k05v_4ZU7XTY|1LXh! diff --git a/test/series_codec_structural_test.dart b/test/series_codec_structural_test.dart new file mode 100644 index 00000000..53594865 --- /dev/null +++ b/test/series_codec_structural_test.dart @@ -0,0 +1,186 @@ +// The curve wire format has to be applied at EVERY seam, not most of them. +// +// AGENTS.md §4.7 is the recurring failure this guards against: a capability +// wired into one call path but not all N. The worst instance in this repo's +// history — FirmwareAwareR24Decoder existing but reaching only one of three +// decode paths — was a total sync outage for real users. +// +// The equivalent here is quiet rather than loud. A day_result reader that calls +// jsonDecode directly gets `{'t0':…,'dt':60,'v':[…]}` where it expects +// `[{t,v},…]`, matches neither, and renders an empty chart. No exception, no +// crash — just a curve that silently is not there. +// +// So: assert the seams structurally. These are the functions that turn a stored +// payload_json into a Map, and each one must route through SeriesCodec. + +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +import 'support/dart_source.dart'; + +/// A bundle-decode seam: the file, the helper, and why it counts. +class Seam { + const Seam(this.path, this.helper, this.why); + final String path; + final String helper; + final String why; +} + +const seams = [ + Seam( + 'lib/data/local_repository_impl.dart', + '_decode', + 'the read seam every screen is served from', + ), + Seam( + 'lib/compute/derivation_engine.dart', + '_decodeBundle', + 'the re-derive path merges a previous bundle into a fresh one', + ), + Seam( + 'lib/health/health_export.dart', + '_decode', + 'the Apple Health sleep export reads hypnogram out of a bundle', + ), +]; + +/// Index just past the `)` matching the `(` at [open]. +int? _matchParen(String code, int open) { + var depth = 0; + for (var i = open; i < code.length; i++) { + if (code[i] == '(') depth++; + if (code[i] == ')') { + depth--; + if (depth == 0) return i + 1; + } + } + return null; +} + +/// The body of the DECLARATION of `name`, from comment/string-stripped source. +/// +/// Two things a naive scan gets wrong, both of which made this guard pass while +/// looking at the wrong text: +/// • `code.indexOf(name)` finds a CALL SITE — `local_repository_impl` calls +/// `_decode` seventy lines above where it declares it. +/// • brace-matching from the first `{` matches the NAMED PARAMETER list, so +/// `putDayResult({…})` returned its own signature instead of its body. +String? helperBody(String code, String name) { + var from = 0; + while (true) { + final start = code.indexOf(name, from); + if (start < 0) return null; + from = start + name.length; + + // Must be `name(`, not a substring of a longer identifier. + var i = start + name.length; + while (i < code.length && code[i] == ' ') { + i++; + } + if (i >= code.length || code[i] != '(') continue; + + final afterParams = _matchParen(code, i); + if (afterParams == null) continue; + + // Skip whitespace and any `async` / `async*` / `sync*` modifier. + var j = afterParams; + while (j < code.length && (code[j] == ' ' || code[j] == '\n')) { + j++; + } + for (final kw in const ['async*', 'async', 'sync*']) { + if (code.startsWith(kw, j)) { + j += kw.length; + while (j < code.length && (code[j] == ' ' || code[j] == '\n')) { + j++; + } + break; + } + } + + // Expression body ends at the semicolon; block body brace-matches. + if (code.startsWith('=>', j)) { + final end = code.indexOf(';', j); + return end < 0 ? null : code.substring(start, end); + } + if (j < code.length && code[j] == '{') { + var depth = 0; + for (var k = j; k < code.length; k++) { + if (code[k] == '{') depth++; + if (code[k] == '}') { + depth--; + if (depth == 0) return code.substring(start, k + 1); + } + } + return null; + } + // A call site — keep looking for the declaration. + } +} + +void main() { + for (final seam in seams) { + test('${seam.path} ${seam.helper} routes through SeriesCodec', () { + final file = File(seam.path); + expect(file.existsSync(), isTrue, reason: '${seam.path} moved or was renamed'); + + final code = stripCommentsAndStrings(file.readAsStringSync()); + final body = helperBody(code, seam.helper); + expect( + body, + isNotNull, + reason: '${seam.helper} not found in ${seam.path} — if it was renamed, ' + 'update this guard rather than deleting it', + ); + expect( + body, + contains('SeriesCodec'), + reason: 'BYPASSED: ${seam.helper} decodes a stored bundle without ' + 'normalizing the curve format. ${seam.why}. A grid/offset curve ' + 'reaches the caller as a Map where it expects a List and silently ' + 'renders as nothing.', + ); + }); + } + + test('putDayResult encodes on the way in', () { + // The single write seam. All four callers (DerivationEngine x2, + // cloud_import, whoop_import) go through it, which is the only reason + // producers can keep building plain [{t,v}] lists in memory. + final code = stripCommentsAndStrings( + File('lib/data/db.dart').readAsStringSync(), + ); + final body = helperBody(code, 'putDayResult'); + expect(body, isNotNull); + expect( + body, + contains('SeriesCodec.encodePayloadJson'), + reason: 'putDayResult stopped encoding — new days would be written in ' + 'the legacy shape and the saving would quietly stop', + ); + }); + + test('the payload column is still TEXT, never a BLOB', () { + // The coach views read payload_json with json_each/json_extract. If this + // column ever becomes a compressed BLOB, v_series and v_hypnogram return + // nothing and the AI Coach loses every intra-day curve — sqflite cannot + // register a SQL decompress function to get it back. + final code = stripCommentsAndStrings( + File('lib/data/db.dart').readAsStringSync(), + ); + expect(code, isNot(contains('payload_json BLOB'))); + }); + + test('v_series reads all three shapes', () { + // Old rows keep the legacy shape forever — there is no rewriting migration + // — so dropping the legacy branch would blank every un-backfilled day. + final src = File('lib/data/db.dart').readAsStringSync(); + final view = src.substring( + src.indexOf('CREATE VIEW v_series'), + src.indexOf('CREATE VIEW v_hypnogram'), + ); + expect(view, contains(".pth)) = 'array'"), reason: 'legacy branch missing'); + expect(view, contains(".pth||'.dt'"), reason: 'grid branch missing'); + expect(view, contains(".pth||'.to'"), reason: 'offset branch missing'); + }); +} diff --git a/test/series_codec_test.dart b/test/series_codec_test.dart new file mode 100644 index 00000000..e69c9129 --- /dev/null +++ b/test/series_codec_test.dart @@ -0,0 +1,273 @@ +// The wire format for day_result curves must be LOSSLESS or absent: every +// shape SeriesCodec can write, it must read back exactly, and anything it +// cannot encode losslessly must pass through untouched. +// +// The round-trip cases run against the three real bundle fixtures tracked in +// the repo root, so this pins the actual shapes the derivation engine emits +// rather than a hand-written approximation of them. + +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/data/series_codec.dart'; + +/// Structural equality over decoded JSON. Hand-rolled rather than pulling in +/// package:collection so the test adds no dependency. +bool deepEquals(Object? a, Object? b) { + if (a is Map && b is Map) { + if (a.length != b.length) return false; + for (final k in a.keys) { + if (!b.containsKey(k) || !deepEquals(a[k], b[k])) return false; + } + return true; + } + if (a is List && b is List) { + if (a.length != b.length) return false; + for (var i = 0; i < a.length; i++) { + if (!deepEquals(a[i], b[i])) return false; + } + return true; + } + return a == b; +} + +const fixtures = ['payload.json', 'payload_july10.json', 'payload_null.json']; + +Map loadFixture(String name) => + (jsonDecode(File(name).readAsStringSync()) as Map).cast(); + +void main() { + group('round-trip on real bundles', () { + for (final name in fixtures) { + test('$name survives encode → decode unchanged', () { + final original = loadFixture(name); + final encoded = SeriesCodec.encodePayload(loadFixture(name)); + final decoded = SeriesCodec.decodePayload(encoded); + expect( + deepEquals(original, decoded), + isTrue, + reason: '$name did not round-trip losslessly', + ); + }); + + test('$name shrinks by at least half', () { + final before = jsonEncode(loadFixture(name)).length; + final after = jsonEncode(SeriesCodec.encodePayload(loadFixture(name))) + .length; + expect( + after, + lessThan(before), + reason: 'encoding must never grow a bundle', + ); + // Measured 1.73x–2.67x across the three fixtures; 50% is the floor the + // weakest one clears with room to spare. + expect(after / before, lessThan(0.75)); + }); + + test('$name encode is idempotent', () { + final once = jsonEncode(SeriesCodec.encodePayload(loadFixture(name))); + final twice = jsonEncode( + SeriesCodec.encodePayload( + (jsonDecode(once) as Map).cast(), + ), + ); + expect(twice, once); + }); + + test('$name reports needing re-encode before, not after', () { + final raw = jsonEncode(loadFixture(name)); + expect(SeriesCodec.needsReencode(raw), isTrue); + expect( + SeriesCodec.needsReencode(SeriesCodec.encodePayloadJson(raw)), + isFalse, + ); + }); + } + }); + + group('shape selection', () { + test('a regular curve becomes a grid', () { + final out = SeriesCodec.encodeCurve([ + {'t': 100, 'v': 1}, + {'t': 160, 'v': 2}, + {'t': 220, 'v': 3}, + ]); + expect(out, { + 't0': 100, + 'dt': 60, + 'v': [1, 2, 3], + }); + }); + + test('an irregular curve becomes offsets, with to[0] == 0', () { + final out = SeriesCodec.encodeCurve([ + {'t': 100, 'v': 1}, + {'t': 161, 'v': 2}, + {'t': 400, 'v': 3}, + ]); + expect(out, { + 't0': 100, + 'to': [0, 61, 300], + 'v': [1, 2, 3], + }); + }); + + test('zone_timeline round-trips on its z key', () { + final points = [ + {'t': 100, 'z': 0}, + {'t': 160, 'z': 2}, + {'t': 220, 'z': 1}, + ]; + final enc = SeriesCodec.encodeCurve(points, valueKey: 'z'); + expect(enc, isA()); + expect(SeriesCodec.decodeCurve(enc, valueKey: 'z'), points); + }); + }); + + group('refuses anything it cannot encode losslessly', () { + test('fewer than minPoints stays legacy', () { + final short = [ + {'t': 100, 'v': 1}, + {'t': 160, 'v': 2}, + ]; + expect(SeriesCodec.encodeCurve(short), same(short)); + }); + + test('an element with an extra key stays legacy', () { + final extra = [ + {'t': 100, 'v': 1, 'q': 9}, + {'t': 160, 'v': 2, 'q': 9}, + {'t': 220, 'v': 3, 'q': 9}, + ]; + expect(SeriesCodec.encodeCurve(extra), same(extra)); + }); + + test('a non-int timestamp stays legacy', () { + final floaty = [ + {'t': 100.5, 'v': 1}, + {'t': 160.5, 'v': 2}, + {'t': 220.5, 'v': 3}, + ]; + expect(SeriesCodec.encodeCurve(floaty), same(floaty)); + }); + + test('a missing value key stays legacy', () { + final wrong = [ + {'t': 100, 'z': 1}, + {'t': 160, 'z': 2}, + {'t': 220, 'z': 3}, + ]; + expect(SeriesCodec.encodeCurve(wrong), same(wrong)); + }); + + test('hypnogram segments are skipped — no t key', () { + final hypno = [ + {'start': 1, 'end': 2, 'stage': 'light'}, + {'start': 2, 'end': 3, 'stage': 'deep'}, + {'start': 3, 'end': 4, 'stage': 'rem'}, + ]; + expect(SeriesCodec.encodeCurve(hypno), same(hypno)); + }); + + test('hypnogram is left alone by a whole-payload encode', () { + final payload = { + 'series': { + 'hypnogram': [ + {'start': 1, 'end': 2, 'stage': 'light'}, + {'start': 2, 'end': 3, 'stage': 'deep'}, + {'start': 3, 'end': 4, 'stage': 'rem'}, + ], + }, + }; + final out = SeriesCodec.encodePayload(payload); + expect(out['series']['hypnogram'], isA()); + }); + }); + + group('honesty — nulls are data, not gaps', () { + test('null values survive a grid round-trip in place', () { + final points = [ + {'t': 100, 'v': 1}, + {'t': 160, 'v': null}, + {'t': 220, 'v': 3}, + ]; + final enc = SeriesCodec.encodeCurve(points); + expect((enc as Map)['v'], [1, null, 3]); + expect(SeriesCodec.decodeCurve(enc), points); + }); + + test('null values survive an offset round-trip in place', () { + final points = [ + {'t': 100, 'v': null}, + {'t': 161, 'v': 2}, + {'t': 400, 'v': null}, + ]; + final enc = SeriesCodec.encodeCurve(points); + expect(SeriesCodec.decodeCurve(enc), points); + }); + + test('a non-monotonic curve still round-trips exactly', () { + final points = [ + {'t': 400, 'v': 1}, + {'t': 100, 'v': 2}, + {'t': 250, 'v': 3}, + ]; + expect(SeriesCodec.decodeCurve(SeriesCodec.encodeCurve(points)), points); + }); + + test('duplicate timestamps round-trip exactly', () { + final points = [ + {'t': 100, 'v': 1}, + {'t': 100, 'v': 2}, + {'t': 100, 'v': 3}, + ]; + expect(SeriesCodec.decodeCurve(SeriesCodec.encodeCurve(points)), points); + }); + }); + + group('malformed input degrades, never throws', () { + test('an envelope with neither dt nor to yields an empty curve', () { + expect(SeriesCodec.decodeCurve({'t0': 1, 'v': [1, 2]}), isEmpty); + }); + + test('a ragged offset envelope yields an empty curve', () { + expect( + SeriesCodec.decodeCurve({ + 't0': 1, + 'to': [0, 5], + 'v': [1, 2, 3], + }), + isEmpty, + ); + }); + + test('a missing t0 yields an empty curve', () { + expect( + SeriesCodec.decodeCurve({ + 'dt': 60, + 'v': [1, 2], + }), + isEmpty, + ); + }); + + test('unparseable json decodes to null, not a throw', () { + expect(SeriesCodec.decodePayloadJson('{not json'), isNull); + expect(SeriesCodec.decodePayloadJson(null), isNull); + expect(SeriesCodec.decodePayloadJson(''), isNull); + }); + + test('unparseable json encodes back to itself', () { + expect(SeriesCodec.encodePayloadJson('{not json'), '{not json'); + expect(SeriesCodec.encodePayloadJson('[1,2,3]'), '[1,2,3]'); + }); + + test('decodePayload is idempotent and safe on foreign payloads', () { + final baseline = {'value': 42.0, 'mean': 40.0, 'n': 28}; + final once = SeriesCodec.decodePayload({...baseline}); + expect(deepEquals(once, baseline), isTrue); + expect(deepEquals(SeriesCodec.decodePayload(once), baseline), isTrue); + }); + }); +} From 19ab6a7a2b64f4f42cbfa2220ba7fa7b407da9ce Mon Sep 17 00:00:00 2001 From: SATHVIK SVS Date: Mon, 10 Aug 2026 08:02:39 +0530 Subject: [PATCH 3/6] review: run the backfill on every derive, stage backups, guard the view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six of the eight review findings were real; two are answered in the threads. * The re-encode walk was called from `_pruneOldDecoded`, and BOTH of that method's call sites sit behind `if (scope.fullHistory)`. Ordinary light/heavy derives run with `fullHistory: false`, so the back-catalogue rewrite was resumable but effectively unreachable — a normal install would have converted nothing. Moved to `_runStorageHousekeeping`, called on every derive including the all-days-finalized early return. * Backups compressed straight into the final `.db.gz` path, so the published name existed while the file was still being written. A process killed mid-stream left a truncated file carrying a name retention matches, which counted toward the five and evicted a good backup; `catch` cannot help there. Now staged under a `.partial` suffix retention does not match, and published by rename. Leftovers from a killed run are swept on the next. * `json_extract` RAISES on a malformed document, so one corrupt payload_json failed the entire v_series query rather than dropping that day. The `latest` CTE now filters on `json_valid` first, the same guard daysWithSleepTst already applies for the same reason. * `importFromDbFile` writes day_result rows with a raw batch.insert, bypassing the encode seam, so imported rows arrive in whatever shape the source device stored. The walk latches `done` and is forward-only, so those rows would have kept the legacy shape forever — an import silently undoing the compression. An import that wrote day_result rows now rewinds the cursor. * A curve carrying BOTH `dt` and `to` matched the grid and offset branches at once and emitted every point twice. The codec never writes both, but storage does not enforce that and an import or corruption can. The offset branch now requires `.dt` to be absent, which also matches SeriesCodec.decodeCurve's precedence so SQL and Dart read an ambiguous curve the same way. * `verifyLossless` + `encodePayloadJson` ran inside `db.transaction`, holding the write lock across ~40 x ~88 KB of pure JSON work. Prepared outside now; the transaction only applies the updates. Also: test/import_container_test.dart carried three literal NUL bytes, which made git, grep and review tooling treat it as BINARY — no diff, no search hits. That is why the gzip tests in it read as missing. Written as `\x00` escapes so the file is text; same bytes, same assertions. flutter analyze clean; 1835 tests pass. --- lib/compute/derivation_engine.dart | 28 +++++++-- lib/data/auto_backup.dart | 45 ++++++++++++-- lib/data/db.dart | 75 +++++++++++++++++------ test/auto_backup_test.dart | 50 +++++++++++++++ test/coach_views_series_shapes_test.dart | 59 ++++++++++++++++++ test/day_result_reencode_test.dart | 34 ++++++++++ test/import_container_test.dart | Bin 15470 -> 15482 bytes 7 files changed, 262 insertions(+), 29 deletions(-) diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index a397cd0b..b51f47e3 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -1076,6 +1076,9 @@ class DerivationEngine { if (scope.fullHistory) { await _pruneOldDecoded(todoDays, dataNowSec); } + // Unconditional: an install whose days are all finalized still has a + // back catalogue to convert, and this is the path it takes every time. + await _runStorageHousekeeping(); return 0; } _diag['todo_days'] = todoDays.length; @@ -1175,7 +1178,12 @@ class DerivationEngine { _diag['stage'] = 'notifications'; await _runNotifications(); } - // 5. Prune raw — never for a day still inside its raw window / un-derived. + // 5. Storage housekeeping. OUTSIDE the fullHistory gate below — see + // _runStorageHousekeeping for why. + _diag['stage'] = 'housekeeping'; + await _runStorageHousekeeping(); + + // 6. Prune raw — never for a day still inside its raw window / un-derived. if (scope.fullHistory) { _diag['stage'] = 'prune'; await _pruneOldDecoded(todoDays, dataNowSec); @@ -3314,10 +3322,20 @@ class DerivationEngine { if (stale > 0) { _log('pruned $stale superseded intermediate rows'); } - // Convert the back catalogue to the compact curve format, a bounded batch - // at a time. Same reasoning as the prune above: recomputable/rewritable - // housekeeping belongs here, off the path to a durable commit, and never in - // a migration under iOS's CPU watchdog. + } + + /// Storage housekeeping that must run on EVERY derive. + /// + /// Deliberately NOT inside [_pruneOldDecoded]: both of that method's call + /// sites sit behind `if (scope.fullHistory)`, and ordinary light/heavy + /// derives run with `fullHistory: false`. Putting the back-catalogue rewrite + /// there made it resumable but effectively unreachable — a normal install + /// would have converted nothing. + /// + /// Bounded and resumable, so running it on every pass costs one small batch. + /// Off the path to a durable commit, and never inside a migration: `onUpgrade` + /// runs under iOS's CPU watchdog (invariant 11). + Future _runStorageHousekeeping() async { final reencoded = await LocalDb.reencodeLegacyDayResults(); if (reencoded > 0) { _log('re-encoded $reencoded legacy day bundles'); diff --git a/lib/data/auto_backup.dart b/lib/data/auto_backup.dart index 1c40e738..2a66e95e 100644 --- a/lib/data/auto_backup.dart +++ b/lib/data/auto_backup.dart @@ -114,6 +114,27 @@ String backupFileName(DateTime when) { /// leaked for the same reason. final _backupNamePattern = RegExp(r'^openstrap-\d{8}-\d{6}(-\d+)?\.db(\.gz)?$'); +/// Appended while a backup is still being written. Chosen so +/// [_backupNamePattern] does NOT match it: a partial file must be invisible to +/// retention, or a process killed mid-write would let a truncated backup evict +/// a good one. +const kBackupStagingSuffix = '.partial'; + +/// Delete staging files left by a run that was killed mid-write. +/// +/// Retention cannot do this — it only sees names it matches, and the whole +/// point of the staging suffix is that it does not. Best-effort: a leftover +/// costs disk, never correctness. +Future pruneStagingFiles(Directory dir) async { + try { + for (final f in dir.listSync().whereType()) { + if (p.basename(f.path).endsWith(kBackupStagingSuffix)) await f.delete(); + } + } catch (_) { + /* housekeeping only */ + } +} + /// Existing backups, newest first. List sortBackupsNewestFirst(Iterable entries) { final files = entries @@ -217,6 +238,15 @@ Future _runBackup({ } final snapshot = await (exportSnapshot ?? LocalDb.exportCopy)(); final tmp = File(snapshot); + // STAGE, then publish by rename. Compressing straight into `dest` meant the + // final backup name existed while it was still being written: kill the + // process mid-stream and a truncated file is left behind carrying a name + // `_backupNamePattern` matches, so retention counts it as one of the five + // and evicts a good backup to make room. `catch` cannot help — the process + // is gone. The staging name is deliberately one retention does NOT match, + // and rename is atomic within the directory, so `dest.path` only ever + // exists as a complete file. + final staging = File('${dest.path}$kBackupStagingSuffix'); try { // STREAMED, not read-then-compress: the snapshot is the whole database // and buffering it twice in memory to save disk would trade one resource @@ -224,13 +254,14 @@ Future _runBackup({ // // This also replaces the old rename/copy fallback — that existed because // temp and external storage are different filesystems on Android, where - // rename fails outright. A stream never had that problem. - await tmp.openRead().transform(gzip.encoder).pipe(dest.openWrite()); + // rename fails outright. Staging lives in the destination directory, so + // the publish step is a same-filesystem rename. + final sink = staging.openWrite(); + await tmp.openRead().transform(gzip.encoder).pipe(sink); + await staging.rename(dest.path); } catch (_) { - // A half-written .gz is not a backup, and leaving one behind would let it - // count toward retention and push a GOOD backup out of the window. try { - if (await dest.exists()) await dest.delete(); + if (await staging.exists()) await staging.delete(); } catch (_) {} rethrow; } finally { @@ -239,6 +270,10 @@ Future _runBackup({ } catch (_) {} } + // Sweep any staging files a previous run was killed midway through. They + // are invisible to retention by design, so nothing else would ever remove + // them. + await pruneStagingFiles(dir); await pruneBackups(dir, keep: kBackupsKept); return BackupOutcome(path: dest.path); } catch (e) { diff --git a/lib/data/db.dart b/lib/data/db.dart index b6a8d54f..92753b01 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -1758,7 +1758,17 @@ class LocalDb { // data/series_codec.dart for why. Old rows keep their legacy shape forever, // so this view must read all three, and the branch guards are what keep a // row from being emitted twice: legacy requires an `array`, grid requires - // `.dt`, offset requires `.to`, and no stored curve ever satisfies two. + // `.dt`, offset requires `.to` AND no `.dt`. The codec never writes both, + // but "never" is not enforced by the storage layer — a foreign or corrupted + // curve carrying both fields matched the grid and offset branches at once + // and silently doubled the curve. The `.dt` precedence also matches + // SeriesCodec.decodeCurve, so SQL and Dart resolve an ambiguous curve the + // same way rather than disagreeing. + // + // `latest` filters on json_valid first: json_extract raises on a malformed + // document, and without the guard ONE corrupt payload fails the entire + // v_series query instead of dropping that one day. Same guard, same reason, + // as daysWithSleepTst. // // The grid branch needs no running sum because json_each exposes an array's // index as `key`, so t = t0 + key*dt. Offset pairs `to` and `v` on that @@ -1771,6 +1781,7 @@ class LocalDb { SELECT r.day_id, r.payload_json FROM day_result r JOIN (SELECT day_id, MAX(algo_version) v FROM day_result GROUP BY day_id) m ON r.day_id = m.day_id AND r.algo_version = m.v + WHERE json_valid(r.payload_json) ), curve(sk, pth, vk) AS ( SELECT 'hr_curve','\$.series.hr_curve','\$.v' @@ -1803,6 +1814,7 @@ class LocalDb { JOIN json_each(json_extract(l.payload_json, c.pth||'.to')) et JOIN json_each(json_extract(l.payload_json, c.pth||'.v')) ev ON ev.key = et.key WHERE json_extract(l.payload_json, c.pth||'.to') IS NOT NULL + AND json_extract(l.payload_json, c.pth||'.dt') IS NULL '''); // Sleep stage segments (different element shape from the {t,v} curves). await db.execute(''' @@ -3931,6 +3943,16 @@ class LocalDb { } finally { await src.close(); } + // An import writes day_result rows with a raw batch.insert, deliberately + // bypassing putDayResult (and therefore the curve-encode seam), so the rows + // arrive in whatever shape the source device stored — legacy, if it was on + // an older build. The re-encode walk is forward-only and latches `done`, so + // once it has finished those rows would never be looked at again and the + // growth this walk exists to remove would come straight back with the + // import. Rewind it. + if ((counts['day_result'] ?? 0) > 0) { + await putComputeFreshness(kReencodeCursorKey, jsonEncode({})); + } return counts; } @@ -4505,24 +4527,39 @@ class LocalDb { return 0; } - var rewritten = 0; - await db.transaction((txn) async { - for (final row in rows) { - final pj = row['payload_json']; - if (pj is! String || pj.isEmpty) continue; - if (!SeriesCodec.needsReencode(pj)) continue; - if (!SeriesCodec.verifyLossless(pj)) continue; - final encoded = SeriesCodec.encodePayloadJson(pj); - if (encoded.length >= pj.length) continue; // never grow a row - await txn.update( - 'day_result', - {'payload_json': encoded}, - where: 'day_id = ? AND algo_version = ?', - whereArgs: [row['day_id'], row['algo_version']], - ); - rewritten++; - } - }); + // PREPARE OUTSIDE THE TRANSACTION. Each eligible bundle costs several JSON + // parse/serialize passes (needsReencode, then verifyLossless, which encodes + // and decodes to prove the round trip, then the real encode). Doing that + // inside db.transaction held the write lock open across ~40 x ~88 KB of + // pure CPU while the rest of the app waited to write. + final updates = <({String dayId, int algoVersion, String encoded})>[]; + for (final row in rows) { + final pj = row['payload_json']; + if (pj is! String || pj.isEmpty) continue; + if (!SeriesCodec.needsReencode(pj)) continue; + if (!SeriesCodec.verifyLossless(pj)) continue; + final encoded = SeriesCodec.encodePayloadJson(pj); + if (encoded.length >= pj.length) continue; // never grow a row + updates.add(( + dayId: row['day_id'] as String, + algoVersion: (row['algo_version'] as num).toInt(), + encoded: encoded, + )); + } + + final rewritten = updates.length; + if (updates.isNotEmpty) { + await db.transaction((txn) async { + for (final u in updates) { + await txn.update( + 'day_result', + {'payload_json': u.encoded}, + where: 'day_id = ? AND algo_version = ?', + whereArgs: [u.dayId, u.algoVersion], + ); + } + }); + } // The cursor advances past every row we LOOKED at, not just the ones we // rewrote — a row we skipped (already encoded, or not provably lossless) diff --git a/test/auto_backup_test.dart b/test/auto_backup_test.dart index 1b102593..242089c9 100644 --- a/test/auto_backup_test.dart +++ b/test/auto_backup_test.dart @@ -438,6 +438,56 @@ void main() { ); }); + test('the final backup name never exists as a partial file', () async { + // Compressing straight into `dest` published the final name while the + // file was still being written. Kill the process mid-stream and a + // truncated file carries a name retention matches, so it counts as one of + // the five and evicts a good backup. `catch` cannot save that — the + // process is gone. So: stage under a name retention does NOT match, and + // publish by rename. + // + // Asserted through a failing export, which is the only mid-write failure + // reachable from a test: no final-named file may be left behind, and + // nothing invisible may accumulate either. + final dir = await backupDirectory(); + final before = dir.listSync().length; + + final outcome = await runBackup( + now: DateTime(2026, 8, 9, 17, 0, 0), + exportSnapshot: () async => throw const FileSystemException('boom'), + ); + expect(outcome.succeeded, isFalse); + + final names = dir.listSync().map((f) => p.basename(f.path)).toList(); + expect( + names.where((n) => n.contains('20260809-170000')), + isEmpty, + reason: 'a failed backup must leave neither a final nor a staging file', + ); + expect(dir.listSync().length, before); + }); + + test('staging files are invisible to retention', () async { + // The suffix only protects a good backup if retention genuinely cannot + // see it — otherwise a partial would still be counted and still evict. + final dir = await backupDirectory(); + File( + p.join(dir.path, 'openstrap-20260809-180000.db.gz$kBackupStagingSuffix'), + ).writeAsStringSync('half a backup'); + try { + final seen = sortBackupsNewestFirst(dir.listSync()) + .map((f) => p.basename(f.path)); + expect(seen.where((n) => n.contains('180000')), isEmpty); + } finally { + await pruneStagingFiles(dir); + } + expect( + dir.listSync().where((f) => f.path.endsWith(kBackupStagingSuffix)), + isEmpty, + reason: 'pruneStagingFiles must reclaim what retention cannot see', + ); + }); + test('a snapshot is never left behind in temp', () async { // The export is a full second copy of the database. The old code renamed // it into place; the new one streams and must still delete the source. diff --git a/test/coach_views_series_shapes_test.dart b/test/coach_views_series_shapes_test.dart index 84d5e13d..15710615 100644 --- a/test/coach_views_series_shapes_test.dart +++ b/test/coach_views_series_shapes_test.dart @@ -184,6 +184,65 @@ void main() { ); }); + test('a curve carrying BOTH dt and to is not doubled', () async { + // The codec never writes both, but the storage layer does not enforce that + // — an import from a foreign device or a corrupted row can. When the grid + // and offset branches were only guarded on their own field, such a curve + // matched both and the coach saw every point twice. + await db.insert('day_result', { + 'day_id': '2026-01-03', + 'algo_version': 47, + 'payload_json': jsonEncode({ + 'series': { + 'hr_curve': { + 't0': 100, + 'dt': 60, + 'to': [0, 60, 120], + 'v': [1, 2, 3], + }, + }, + }), + 'window_json': '{}', + 'computed_at': 0, + }); + + final rows = await seriesRows(db, '2026-01-03'); + expect(rows, hasLength(3), reason: 'each point exactly once'); + // `dt` wins, matching SeriesCodec.decodeCurve, so SQL and Dart agree on + // what an ambiguous curve means rather than disagreeing. + expect(rows.map((r) => r['t']), [100, 160, 220]); + }); + + test('one corrupt payload does not fail the whole view', () async { + // json_extract RAISES on a malformed document. Without a json_valid guard + // in the `latest` CTE, a single unparseable row took down every other day's + // curves with it — the coach got an error instead of the data it could + // still have had. + await db.insert('day_result', { + 'day_id': '2026-01-04', + 'algo_version': 47, + 'payload_json': '{ this is not json', + 'window_json': '{}', + 'computed_at': 0, + }); + try { + final rows = await db.rawQuery( + "SELECT COUNT(*) c FROM v_series WHERE date = '2026-01-01'", + ); + expect((rows.first['c'] as num).toInt(), greaterThan(0)); + expect(await seriesRows(db, '2026-01-04'), isEmpty); + } finally { + // Removed here rather than left for the shared teardown: this row is + // deliberately malformed, and the "encoder never emits invalid JSON" + // assertion further down scans the whole table. + await db.delete( + 'day_result', + where: 'day_id = ?', + whereArgs: ['2026-01-04'], + ); + } + }); + test('v_hypnogram is unaffected by the encoding', () async { final legacy = await db.rawQuery( "SELECT start_ts, end_ts, stage FROM v_hypnogram " diff --git a/test/day_result_reencode_test.dart b/test/day_result_reencode_test.dart index 02470ae6..37011f2d 100644 --- a/test/day_result_reencode_test.dart +++ b/test/day_result_reencode_test.dart @@ -225,6 +225,40 @@ void main() { expect(after, jsonEncode(tiny)); }); + test('an import rewinds a finished walk', () async { + // importFromDbFile writes day_result rows with a raw batch.insert, so they + // arrive in whatever shape the source device stored. The walk latches + // `done` and is forward-only, so without a rewind those rows would keep the + // legacy shape forever and the import would silently undo the compression. + await seedLegacy(db, '2026-01-01', t0); + expect(await LocalDb.reencodeLegacyDayResults(), 1); + expect(await LocalDb.reencodeLegacyDayResults(), 0); // walk is done + + // What an import leaves behind: a legacy row that never met the write seam. + await seedLegacy(db, '2026-02-01', t0 + 86400 * 40); + expect( + await LocalDb.reencodeLegacyDayResults(), + 0, + reason: 'precondition: a latched walk ignores it', + ); + + // The rewind importFromDbFile performs. + await LocalDb.putComputeFreshness(LocalDb.kReencodeCursorKey, '{}'); + + var total = 0; + for (var i = 0; i < 10; i++) { + final n = await LocalDb.reencodeLegacyDayResults(limit: 2); + if (n == 0) break; + total += n; + } + expect(total, 1, reason: 'the imported row gets converted'); + + final rows = await db.query('day_result', columns: ['payload_json']); + for (final r in rows) { + expect(SeriesCodec.needsReencode(r['payload_json'] as String), isFalse); + } + }); + test('every algo_version generation of a day is converted', () async { // day_result is keyed (day_id, algo_version); a day can hold more than one // generation and the walk must not stop at the newest. diff --git a/test/import_container_test.dart b/test/import_container_test.dart index e76ccb58c58ee39c4f476af3ad795d5fadfdd40b..2a3b9a97e6670f9865b6e6fe03741f8c71634483 100644 GIT binary patch delta 51 zcmaD?@vCA(A3sY>g@M85e*Q>42CPDkt9+cGw)LC&~u^|6&qb delta 37 tcmexW@vdS+A3rNYQEG9?Bzn9ezU}9k0EUy>M2LKpb3^@P* From 1fd948f57a6aaef61784b9af7ad67ca30e7f7759 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Mon, 10 Aug 2026 22:33:21 +0530 Subject: [PATCH 4/6] make the compressed backups restorable, and stop the backfill racing a derive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auto-backup writes .db.gz and the restore path handed the picked file straight to openDatabase, so every backup this code takes came back as "file is not a database". importFromDbFile now sniffs the magic bytes, inflates to a temp file, imports and cleans up either way; a plain .db is untouched. The import screen and the backup sheet said nothing about compression, so they do now. Nothing crossed that boundary in a test, which is why it shipped. The re-encode backfill prepared its bundles outside any transaction and then wrote them back keyed on (day_id, algo_version) alone. Derivation runs in more than one isolate and this walk starts at the newest day with kAlgoVersion unbumped, so its first targets are the rows a light derive is rewriting: the row ended up with the new scalar columns and the old payload. The update is now a compare-and-set on the payload it read, and a row that has moved is parked behind the cursor rather than stepped over. That prepare is also ~0.1-0.4s of synchronous JSON on desktop hardware, on whichever isolate called the derive, which is the UI one. SeriesCodec is pure, so the batch runs on a worker isolate. _runStorageHousekeeping was called unguarded from two points in run(), so a SQLITE_BUSY or a full disk skipped the raw prune and the timezone re-baseline and made a finished derive report zero days. It now swallows its own errors and runs from the finally of run/runDays/rescanRecent — the only place every entry path and early return reaches, which also fixes it never running on a restored database that has derived history but no decoded rows. pruneStagingFiles deleted any *.partial in a folder the user is invited to point iCloud Drive or Nextcloud at; it now requires the published name to match the backup pattern. Retention sorted same-second collision names backwards because '-' sorts before '.', so it ranked the later backup as the older one; it sorts by parsed timestamp and collision index instead. inflateGzip used sink.add in an await-for loop, which queues without back-pressure and buffers the whole inflated database in memory - the OOM this file's header is about. It pipes through a counting transformer now, against a 2 GiB ceiling rather than 4 GiB, which is not a bound a phone survives reaching. decodeCurve returned an empty curve for a map it did not recognise; since decodePayload is the read seam for baselines and freshness rows too, it hands the value back instead. --- .gitattributes | 6 + lib/compute/derivation_engine.dart | 43 ++++++-- lib/data/auto_backup.dart | 51 ++++++++- lib/data/db.dart | 172 ++++++++++++++++++++++++----- lib/data/series_codec.dart | 18 ++- lib/import/import_container.dart | 46 ++++++-- lib/ui/import/import_screen.dart | 3 +- lib/ui/profile/profile_screen.dart | 2 + test/auto_backup_test.dart | 147 ++++++++++++++++++++++++ test/day_result_reencode_test.dart | 126 +++++++++++++++++++-- test/series_codec_test.dart | 42 +++---- 11 files changed, 569 insertions(+), 87 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..a2510bd0 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,6 @@ +# Dart sources are text, always. A test that needs a NUL byte in a fixture can +# write one as a literal, and git's binary heuristic then calls the whole file +# binary: the diff collapses to "Binary files differ" with no line counts, so +# nobody reviews it. Escape sequences are the fix in the source; this makes the +# diff readable even when a file slips through. +*.dart text diff diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index b51f47e3..6c54aeb4 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -1076,9 +1076,6 @@ class DerivationEngine { if (scope.fullHistory) { await _pruneOldDecoded(todoDays, dataNowSec); } - // Unconditional: an install whose days are all finalized still has a - // back catalogue to convert, and this is the path it takes every time. - await _runStorageHousekeeping(); return 0; } _diag['todo_days'] = todoDays.length; @@ -1178,12 +1175,7 @@ class DerivationEngine { _diag['stage'] = 'notifications'; await _runNotifications(); } - // 5. Storage housekeeping. OUTSIDE the fullHistory gate below — see - // _runStorageHousekeeping for why. - _diag['stage'] = 'housekeeping'; - await _runStorageHousekeeping(); - - // 6. Prune raw — never for a day still inside its raw window / un-derived. + // 5. Prune raw — never for a day still inside its raw window / un-derived. if (scope.fullHistory) { _diag['stage'] = 'prune'; await _pruneOldDecoded(todoDays, dataNowSec); @@ -1210,6 +1202,11 @@ class DerivationEngine { _log('derive ERROR: $e\n$st'); return 0; } finally { + // Storage housekeeping runs here, after everything, still holding + // `_running`. See _runStorageHousekeeping — this is the only place every + // entry path and every early return actually reaches. + _diag['stage'] = 'housekeeping'; + await _runStorageHousekeeping(); _running = false; final finishedAt = DateTime.now().millisecondsSinceEpoch; _diag @@ -1336,6 +1333,7 @@ class DerivationEngine { _log('derive selected ERROR: $e\n$st'); return 0; } finally { + await _runStorageHousekeeping(); final finishedAt = DateTime.now().millisecondsSinceEpoch; _diag ..['running'] = false @@ -2064,6 +2062,7 @@ class DerivationEngine { _log('rescan ERROR: $e\n$st'); return 0; } finally { + await _runStorageHousekeeping(); _running = false; } } @@ -3332,13 +3331,33 @@ class DerivationEngine { /// there made it resumable but effectively unreachable — a normal install /// would have converted nothing. /// + /// CALLED FROM THE `finally` OF EVERY ENTRY PATH, and it swallows its own + /// errors, for two reasons that were both live: + /// + /// • Reach. Called from the body, it sat below `if (dataNowSec <= 0) + /// return 0` and below two other early returns — so the install that most + /// needs it, one restored from a backup with years of derived history and + /// no decoded rows at all (they are capped at `rawRetentionDays`, so a + /// backup carries almost none), converted nothing, ever. runDays and + /// rescanRecent never reached it at all. + /// • Blast radius. Called unguarded from the body, a throw — SQLITE_BUSY + /// from the other derivation isolate, a full disk — skipped the raw prune + /// that enforces `rawRetentionDays`, skipped the timezone re-baseline, and + /// landed in the run-wide catch, so a derive that had actually completed + /// every day reported 0 back to `reanalyzeAll`. This work is a storage + /// optimization; nothing it does may change what the derive returns. + /// /// Bounded and resumable, so running it on every pass costs one small batch. /// Off the path to a durable commit, and never inside a migration: `onUpgrade` /// runs under iOS's CPU watchdog (invariant 11). Future _runStorageHousekeeping() async { - final reencoded = await LocalDb.reencodeLegacyDayResults(); - if (reencoded > 0) { - _log('re-encoded $reencoded legacy day bundles'); + try { + final reencoded = await LocalDb.reencodeLegacyDayResults(); + if (reencoded > 0) { + _log('re-encoded $reencoded legacy day bundles'); + } + } catch (e) { + _log('storage housekeeping skipped: $e'); } } diff --git a/lib/data/auto_backup.dart b/lib/data/auto_backup.dart index 2a66e95e..e6d69028 100644 --- a/lib/data/auto_backup.dart +++ b/lib/data/auto_backup.dart @@ -85,8 +85,7 @@ bool backupIsDue({ /// on the one thing here that is kept five times over. const kBackupExtension = '.db.gz'; -/// Filename for a backup taken at [when]. Sorts chronologically as text, so -/// retention can order by name without parsing. +/// Filename for a backup taken at [when]. /// /// Seconds are included: two runs inside the same minute would otherwise land /// on one name and the second would overwrite the first. @@ -120,6 +119,24 @@ final _backupNamePattern = RegExp(r'^openstrap-\d{8}-\d{6}(-\d+)?\.db(\.gz)?$'); /// a good one. const kBackupStagingSuffix = '.partial'; +/// True when [basename] is one of OUR staging files. +/// +/// The suffix alone is not enough. This directory is app-specific external +/// storage on Android and the file-sharing Documents directory on iOS — the +/// whole point of picking it is that users and sync clients can reach it, and +/// `.partial` is exactly what a half-finished Nextcloud or iCloud download is +/// called. Deleting on the suffix alone reached outside this feature's own +/// files, for the same reason [_backupNamePattern] is strict rather than a +/// loose `openstrap-*` glob. +bool _isOurStagingFile(String basename) { + if (!basename.endsWith(kBackupStagingSuffix)) return false; + final published = basename.substring( + 0, + basename.length - kBackupStagingSuffix.length, + ); + return _backupNamePattern.hasMatch(published); +} + /// Delete staging files left by a run that was killed mid-write. /// /// Retention cannot do this — it only sees names it matches, and the whole @@ -128,20 +145,46 @@ const kBackupStagingSuffix = '.partial'; Future pruneStagingFiles(Directory dir) async { try { for (final f in dir.listSync().whereType()) { - if (p.basename(f.path).endsWith(kBackupStagingSuffix)) await f.delete(); + if (_isOurStagingFile(p.basename(f.path))) await f.delete(); } } catch (_) { /* housekeeping only */ } } +/// Sort key for a backup filename: its timestamp, then its collision index. +/// +/// NOT the raw basename. Names sort chronologically as text right up until a +/// same-second collision suffix appears, because `-` (0x2D) sorts before `.` +/// (0x2E): `…-000000-2.db.gz` compares LESS than `…-000000.db.gz`, so the +/// second backup of that second was ranked as the older one and retention +/// would evict it first. A higher index is always the later write — +/// [_uniqueDestination] only reaches `-2` because `-1`'s name was taken. +(String, int) _backupSortKey(String basename) { + final m = _backupNamePattern.firstMatch(basename); + if (m == null) return ('', 0); + final stamp = basename.substring(0, 'openstrap-00000000-000000'.length); + final collision = m.group(1); + return (stamp, collision == null ? 1 : (int.tryParse(collision.substring(1)) ?? 1)); +} + /// Existing backups, newest first. List sortBackupsNewestFirst(Iterable entries) { final files = entries .whereType() .where((f) => _backupNamePattern.hasMatch(p.basename(f.path))) .toList(); - files.sort((a, b) => p.basename(b.path).compareTo(p.basename(a.path))); + files.sort((a, b) { + final ka = _backupSortKey(p.basename(a.path)); + final kb = _backupSortKey(p.basename(b.path)); + final byStamp = kb.$1.compareTo(ka.$1); + if (byStamp != 0) return byStamp; + final byCollision = kb.$2.compareTo(ka.$2); + if (byCollision != 0) return byCollision; + // Same second, same index — an upgraded install can hold both the old + // `.db` and the new `.db.gz`. Any stable order will do; pick one. + return p.basename(b.path).compareTo(p.basename(a.path)); + }); return files; } diff --git a/lib/data/db.dart b/lib/data/db.dart index 92753b01..62df570a 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -11,11 +11,13 @@ import 'dart:convert'; import 'dart:io'; +import 'dart:isolate'; import 'package:openstrap_protocol/openstrap_protocol.dart' as proto; import 'package:path/path.dart' as p; import 'package:path_provider/path_provider.dart'; import 'package:sqflite/sqflite.dart'; +import '../import/import_container.dart'; import 'day_label.dart'; import 'journal_fields.dart'; import 'live_coverage_policy.dart'; @@ -3763,10 +3765,44 @@ class LocalDb { /// table missing in the source is skipped. Locally FINALIZED day_result rows /// are protected — an import never overwrites them. Returns per-table counts /// of rows actually copied. + /// + /// The picked file may be COMPRESSED. Auto-backups are written gzipped, so + /// the file a user reinstalling onto a new phone reaches for is a `.db.gz`, + /// and handing that straight to `openDatabase` fails with "file is not a + /// database" — the app could write backups it could not restore. Detected by + /// MAGIC BYTES, not by extension: a file manager or a sync client that + /// renames on the way through is exactly the situation a restore has to + /// survive. Plain `.db` files (older backups, and `exportCopy` output) take + /// the same path they always did. static Future> importFromDbFile(String path) async { if (!await File(path).exists()) { throw const FileSystemException('Backup file not found'); } + if (await sniffFile(path) != ImportContainer.gzip) { + return _mergeFromDbFile(path); + } + final work = await Directory.systemTemp.createTemp('openstrap_restore_'); + try { + final inflated = await inflateGzip(path, work); + if (inflated == null || + await sniffFile(inflated) != ImportContainer.sqlite) { + throw const ImportFormatException( + 'That file unpacked to something that is not an OpenStrap database.', + ); + } + return await _mergeFromDbFile(inflated); + } finally { + // The inflated copy is a full second copy of the database, so it goes + // whether the import worked or not. + try { + if (work.existsSync()) await work.delete(recursive: true); + } catch (_) { + /* the OS reclaims the temp dir eventually */ + } + } + } + + static Future> _mergeFromDbFile(String path) async { final src = await openDatabase(path, readOnly: true); final db = await instance; // Order: independent tables; all use INSERT OR REPLACE so re-import is safe. @@ -4527,36 +4563,73 @@ class LocalDb { return 0; } - // PREPARE OUTSIDE THE TRANSACTION. Each eligible bundle costs several JSON + // PREPARE ON A WORKER ISOLATE, outside the transaction. + // + // Outside the transaction because each eligible bundle costs several JSON // parse/serialize passes (needsReencode, then verifyLossless, which encodes - // and decodes to prove the round trip, then the real encode). Doing that - // inside db.transaction held the write lock open across ~40 x ~88 KB of - // pure CPU while the rest of the app waited to write. - final updates = <({String dayId, int algoVersion, String encoded})>[]; - for (final row in rows) { - final pj = row['payload_json']; - if (pj is! String || pj.isEmpty) continue; - if (!SeriesCodec.needsReencode(pj)) continue; - if (!SeriesCodec.verifyLossless(pj)) continue; - final encoded = SeriesCodec.encodePayloadJson(pj); - if (encoded.length >= pj.length) continue; // never grow a row + // and decodes to prove the round trip, then the real encode), and doing + // that inside db.transaction held the write lock open across ~40 x ~88 KB + // of pure CPU while the rest of the app waited to write. + // + // Off THIS isolate because that CPU is otherwise synchronous on whichever + // isolate called the derive, and the derive is called from the UI one: + // measured at 0.1-0.4 s per batch on a desktop, which is several times that + // on a mid-tier phone, with no await in the loop for the frame scheduler to + // get a word in. This app has shipped a derive-correlated main-isolate + // freeze before. `SeriesCodec` is pure — no I/O, no plugins, no Flutter — + // so it is safe anywhere, and the batch is bounded by `limit`. + final payloads = [ + for (final row in rows) + (row['payload_json'] is String) ? row['payload_json'] as String : '', + ]; + final prepared = await Isolate.run(() => _reencodeBatch(payloads)); + + final updates = + <({int rowIndex, String dayId, int algoVersion, String from, String to})>[]; + for (var i = 0; i < rows.length; i++) { + final encoded = prepared[i]; + if (encoded == null) continue; updates.add(( - dayId: row['day_id'] as String, - algoVersion: (row['algo_version'] as num).toInt(), - encoded: encoded, + rowIndex: i, + dayId: rows[i]['day_id'] as String, + algoVersion: (rows[i]['algo_version'] as num).toInt(), + from: payloads[i], + to: encoded, )); } - final rewritten = updates.length; + var rewritten = 0; + // Index of the OLDEST-ranked row (first in this newest-first batch) whose + // compare-and-set found something other than what we read. + int? missedIndex; if (updates.isNotEmpty) { await db.transaction((txn) async { for (final u in updates) { - await txn.update( + // COMPARE-AND-SET on the payload we actually read. + // + // The prepare above deliberately runs outside any transaction and + // takes hundreds of milliseconds, and derivation runs in more than + // one isolate (see updateBaseline's exclusive transaction for the + // same hazard). A blind `WHERE day_id = ? AND algo_version = ?` will + // happily write a stale bundle over a row that a concurrent derive + // rewrote in the meantime — and because this walk starts at the + // NEWEST day with kAlgoVersion unbumped, its first targets are + // exactly the rows a light derive is rewriting. The row would end up + // holding the new scalar columns beside the old payload. + // + // A row that has moved is left alone. It is not lost: the cursor is + // held back below so a later pass looks at it again. + final n = await txn.update( 'day_result', - {'payload_json': u.encoded}, - where: 'day_id = ? AND algo_version = ?', - whereArgs: [u.dayId, u.algoVersion], + {'payload_json': u.to}, + where: 'day_id = ? AND algo_version = ? AND payload_json = ?', + whereArgs: [u.dayId, u.algoVersion, u.from], ); + if (n > 0) { + rewritten++; + } else { + missedIndex ??= u.rowIndex; + } } }); } @@ -4565,14 +4638,39 @@ class LocalDb { // rewrote — a row we skipped (already encoded, or not provably lossless) // would otherwise be re-examined on every future pass and the walk would // never terminate. - await putComputeFreshness( - kReencodeCursorKey, - jsonEncode({ + // + // A row that lost the compare-and-set is the one exception: it is parked + // just BEHIND the cursor so the next pass reads it again, and `done` is + // withheld so the walk cannot latch shut over it. A missed FIRST row leaves + // the cursor exactly where it was, which costs one repeated batch and + // converges — the second look either re-encodes the row or finds it already + // encoded and steps past. + final Map mark; + final missed = missedIndex; + if (missed == null) { + mark = { 'cursor': rows.last['day_id'], 'cursor_version': rows.last['algo_version'], 'done': rows.length < limit, - 'rewritten_last': rewritten, - }), + }; + } else if (missed > 0) { + mark = { + 'cursor': rows[missed - 1]['day_id'], + 'cursor_version': rows[missed - 1]['algo_version'], + 'done': false, + }; + } else if (cursorDay == null) { + mark = const {}; + } else { + mark = { + 'cursor': cursorDay, + 'cursor_version': cursorVersion, + 'done': false, + }; + } + await putComputeFreshness( + kReencodeCursorKey, + jsonEncode({...mark, 'rewritten_last': rewritten}), ); return rewritten; } @@ -5600,3 +5698,27 @@ class LocalDb { ); } } + +/// One batch of curve re-encodes: for each input bundle, the compacted +/// replacement, or null when the row must be left exactly as it is. +/// +/// TOP-LEVEL and pure so it can be handed to `Isolate.run` — it touches nothing +/// but `SeriesCodec`, which has no I/O, no plugins and no Flutter. The null +/// cases are all "leave it legacy": already encoded, not provably lossless +/// (see `verifyLossless` — this OVERWRITES durable user data, and a day past +/// `rawRetentionDays` has no substrate left to re-derive from), or an encode +/// that did not actually shrink the row. +List _reencodeBatch(List payloads) { + final out = []; + for (final pj in payloads) { + if (pj.isEmpty || + !SeriesCodec.needsReencode(pj) || + !SeriesCodec.verifyLossless(pj)) { + out.add(null); + continue; + } + final encoded = SeriesCodec.encodePayloadJson(pj); + out.add(encoded.length < pj.length ? encoded : null); + } + return out; +} diff --git a/lib/data/series_codec.dart b/lib/data/series_codec.dart index 55e09435..adf6ddbd 100644 --- a/lib/data/series_codec.dart +++ b/lib/data/series_codec.dart @@ -167,16 +167,22 @@ class SeriesCodec { /// Normalize one curve back to the legacy `[{t, valueKey}, …]` shape. /// - /// A `List` (legacy) is returned as-is. A malformed envelope yields an EMPTY - /// curve rather than throwing — the same observable outcome callers already - /// get from a missing key, and the contract every decode path in this repo - /// keeps. + /// A `List` (legacy) is returned as-is, and so is a Map that is not one of + /// the envelope shapes this file writes. + /// + /// PASS THROUGH rather than empty. This used to return `const []` for + /// anything it did not recognise, which is a silent TOTAL LOSS: [decodePayload] + /// is the read seam for every stored payload, not just day bundles — baselines, + /// `compute_freshness` and wake features share it — so a foreign map that + /// happened to sit under a curve key would be replaced by nothing on the way + /// out. Handing the value back unchanged costs the same and cannot destroy + /// anything; a caller that wanted a curve still sees a non-List and ignores it. static Object? decodeCurve(Object? raw, {String valueKey = 'v'}) { if (raw is! Map) return raw; final t0 = raw['t0']; final vs = raw['v']; - if (t0 is! int || vs is! List) return const []; + if (t0 is! int || vs is! List) return raw; final dt = raw['dt']; if (dt is int) { @@ -186,7 +192,7 @@ class SeriesCodec { } final to = raw['to']; - if (to is! List || to.length != vs.length) return const []; + if (to is! List || to.length != vs.length) return raw; return [ for (var i = 0; i < vs.length; i++) if (to[i] is int) {'t': t0 + (to[i] as int), valueKey: vs[i]}, diff --git a/lib/import/import_container.dart b/lib/import/import_container.dart index cdfb1d02..1cc284f5 100644 --- a/lib/import/import_container.dart +++ b/lib/import/import_container.dart @@ -26,6 +26,7 @@ // imported for real; anything we cannot use gets a message naming the file we // DO want, instead of a byte offset. +import 'dart:async'; import 'dart:io'; import 'package:archive/archive.dart'; @@ -126,6 +127,18 @@ bool _isCsvMember(String name) { const int _kMaxArchiveMembers = 5000; const int _kMaxUncompressedBytes = 4 * 1024 * 1024 * 1024; // 4 GiB +/// Ceiling for a STREAMED gzip inflate, which is a different problem from the +/// ZIP one above: there the size is declared in the member header and can be +/// refused before a byte is written, whereas gzip declares nothing, so the only +/// way to refuse one is to count bytes that are already landing on disk. A +/// ceiling in the multi-gigabyte range is therefore no protection at all on a +/// phone — the storage is gone long before the guard trips. 2 GiB is more than +/// an order of magnitude above the largest real database this app produces and +/// still leaves a device with room to notice. Dart has no portable free-space +/// API, so this is the bound available; the partial file is deleted on the way +/// out either way. +const int _kMaxInflatedBytes = 2 * 1024 * 1024 * 1024; // 2 GiB + /// CSV files on disk for an import, plus the temp directory (if any) that has /// to be cleaned up once they have been read. class ResolvedImportFiles { @@ -198,19 +211,32 @@ Future inflateGzip(String path, Directory dir) async { final destPath = p.join(dir.path, base); final sink = File(destPath).openWrite(); var written = 0; - try { - await for (final chunk in File(path).openRead().transform(gzip.decoder)) { + // COUNT INSIDE A TRANSFORMER, then `pipe`. The obvious `await for (…) + // sink.add(chunk)` reads as streaming but is not: `IOSink.add` queues without + // back-pressure, so an inflate that outruns the disk buffers the ENTIRE + // inflated database in memory — the 256 MB-heap OOM this file's header is + // about, reintroduced by the code meant to avoid it. `pipe` goes through + // `addStream`, which pauses the source while a write is in flight, and the + // transformer propagates that pause upstream to the decoder. + final counted = StreamTransformer, List>.fromHandlers( + handleData: (chunk, out) { written += chunk.length; - if (written > _kMaxUncompressedBytes) { - throw ImportFormatException( - '“${p.basename(path)}” unpacks to more than ' - '${_kMaxUncompressedBytes ~/ (1024 * 1024 * 1024)} GB, which is not ' - 'something we can import.', + if (written > _kMaxInflatedBytes) { + out.addError( + ImportFormatException( + '“${p.basename(path)}” unpacks to more than ' + '${_kMaxInflatedBytes ~/ (1024 * 1024 * 1024)} GB, which is not ' + 'something we can import.', + ), ); + out.close(); + return; } - sink.add(chunk); - } - await sink.close(); + out.add(chunk); + }, + ); + try { + await File(path).openRead().transform(gzip.decoder).transform(counted).pipe(sink); } catch (e) { try { await sink.close(); diff --git a/lib/ui/import/import_screen.dart b/lib/ui/import/import_screen.dart index 33ee7416..8054b9e1 100644 --- a/lib/ui/import/import_screen.dart +++ b/lib/ui/import/import_screen.dart @@ -180,7 +180,8 @@ class _ImportScreenState extends State { ImportOptionCard( icon: OsIcon.server, title: 'Import from Edge backup', - body: 'A .db exported from another OpenStrap device.', + body: 'A .db or .db.gz from another OpenStrap device — or one of ' + 'this app’s own automatic backups.', onTap: _locked ? null : _importEdge, ), const SizedBox(height: Sp.x3), diff --git a/lib/ui/profile/profile_screen.dart b/lib/ui/profile/profile_screen.dart index b828f880..03a2a1dc 100644 --- a/lib/ui/profile/profile_screen.dart +++ b/lib/ui/profile/profile_screen.dart @@ -62,6 +62,8 @@ Future _backupSheet(BuildContext ctx, AppState app) async { 'folder you can reach from Files. Point iCloud Drive, Synology ' 'or Nextcloud at it and your history lives somewhere other than ' 'this phone. The last $kBackupsKept are kept.\n\n' + 'Each one is compressed (a .db.gz file). Import from Edge backup ' + 'reads it back as it is — there is nothing to unzip first.\n\n' 'It is not encrypted, and it runs when you open the app rather ' 'than in the background.', style: AppText.bodySoft.copyWith(color: AppColors.inkSoft), diff --git a/test/auto_backup_test.dart b/test/auto_backup_test.dart index 242089c9..e37fe52f 100644 --- a/test/auto_backup_test.dart +++ b/test/auto_backup_test.dart @@ -229,6 +229,24 @@ void main() { expect(sortBackupsNewestFirst(tmp.listSync()).length, 3); }); + test('a collision suffix ranks NEWER, not older', () { + // Sorting the raw basename got this backwards: `-` (0x2D) sorts before + // `.` (0x2E), so `-2` compared LESS than the unsuffixed name and the + // second backup of that second was ranked the older of the pair — the + // one retention evicts first. A higher index is always the later write. + touch('openstrap-20260101-000000.db.gz'); + touch('openstrap-20260101-000000-2.db.gz'); + touch('openstrap-20260101-000000-10.db.gz'); + expect( + sortBackupsNewestFirst(tmp.listSync()).map((f) => p.basename(f.path)), + [ + 'openstrap-20260101-000000-10.db.gz', + 'openstrap-20260101-000000-2.db.gz', + 'openstrap-20260101-000000.db.gz', + ], + ); + }); + test('an empty directory is empty, not an error', () { expect(sortBackupsNewestFirst(tmp.listSync()), isEmpty); }); @@ -467,6 +485,36 @@ void main() { expect(dir.listSync().length, before); }); + test('a failure AFTER the staging file exists still removes it', () async { + // The test above throws before the sink is ever opened, so it proves + // nothing about the cleanup that matters: the case worth covering is a + // staging file that has already been created and then has to be reclaimed + // when the write fails. Reached here by handing back a snapshot path that + // cannot be read as a file, so the failure lands inside the write rather + // than in front of it. + final dir = await backupDirectory(); + final when = DateTime(2026, 8, 9, 19, 0, 0); + final dest = File(p.join(dir.path, backupFileName(when))); + final staging = File('${dest.path}$kBackupStagingSuffix'); + staging.writeAsStringSync('a previous attempt got this far'); + + final unreadable = Directory(p.join(tmp.path, 'not-a-snapshot')) + ..createSync(); + final outcome = await runBackup( + now: when, + exportSnapshot: () async => unreadable.path, + ); + + expect(outcome.succeeded, isFalse); + expect(dest.existsSync(), isFalse, reason: 'no final name may be published'); + expect( + staging.existsSync(), + isFalse, + reason: 'a staging file that was created must be deleted on failure', + ); + unreadable.deleteSync(); + }); + test('staging files are invisible to retention', () async { // The suffix only protects a good backup if retention genuinely cannot // see it — otherwise a partial would still be counted and still evict. @@ -488,6 +536,105 @@ void main() { ); }); + test('a .partial that is not ours is left alone', () async { + // This folder is app-specific external storage on Android and the + // file-sharing Documents directory on iOS — chosen precisely so sync + // clients can point at it, and `.partial` is what a half-finished + // Nextcloud or iCloud download is called. Deleting on the suffix alone + // reached outside this feature's own files. + final dir = await backupDirectory(); + final foreign = File(p.join(dir.path, 'holiday-video.mp4.partial')) + ..writeAsStringSync('someone else is downloading this'); + final lookalike = File(p.join(dir.path, 'openstrap-notes.db.partial')) + ..writeAsStringSync('not ours either'); + final ours = File( + p.join(dir.path, 'openstrap-20260809-181500.db.gz$kBackupStagingSuffix'), + )..writeAsStringSync('half a backup'); + + await pruneStagingFiles(dir); + + expect(ours.existsSync(), isFalse); + expect(foreign.existsSync(), isTrue); + expect(lookalike.existsSync(), isTrue); + foreign.deleteSync(); + lookalike.deleteSync(); + }); + + test('a backup this code writes can actually be restored', () async { + // THE boundary this feature turns on, and nothing crossed it. The write + // side gzips; the restore side handed the picked path straight to + // openDatabase, so every backup written here came back as "file is not a + // database" — a user who lost their phone, reinstalled, and picked their + // own backup got nothing. + final db = await LocalDb.instance; + const dayId = '2026-08-09'; + const payload = '{"scalars":{"rhr":52.0}}'; + await db.insert('day_result', { + 'day_id': dayId, + 'algo_version': 61, + 'payload_json': payload, + 'window_json': '{}', + 'computed_at': 1, + 'finalized': 0, + 'skipped': 0, + 'partial': 0, + }, conflictAlgorithm: ConflictAlgorithm.replace); + + final outcome = await runBackup(now: DateTime(2026, 8, 9, 20, 0, 0)); + expect(outcome.succeeded, isTrue, reason: outcome.error); + expect(outcome.path, endsWith('.db.gz')); + // Out of the retention folder, so a later backup in this group cannot + // evict the file under the assertion. + final picked = File(p.join(tmp.path, 'picked-backup.db.gz')); + await File(outcome.path!).copy(picked.path); + + await db.delete('day_result', where: 'day_id = ?', whereArgs: [dayId]); + expect( + await db.query('day_result', where: 'day_id = ?', whereArgs: [dayId]), + isEmpty, + ); + + final counts = await LocalDb.importFromDbFile(picked.path); + expect(counts['day_result'], greaterThanOrEqualTo(1)); + final restored = await db.query( + 'day_result', + where: 'day_id = ?', + whereArgs: [dayId], + ); + expect(restored, hasLength(1)); + expect(restored.first['payload_json'], payload); + await picked.delete(); + await db.delete('day_result', where: 'day_id = ?', whereArgs: [dayId]); + }); + + test('a plain uncompressed .db still restores', () async { + // Older backups and exportCopy output are not compressed. Sniffing by + // magic bytes rather than by extension has to leave that path alone. + final db = await LocalDb.instance; + const dayId = '2026-08-10'; + await db.insert('day_result', { + 'day_id': dayId, + 'algo_version': 61, + 'payload_json': '{"scalars":{"rhr":48.0}}', + 'window_json': '{}', + 'computed_at': 1, + 'finalized': 0, + 'skipped': 0, + 'partial': 0, + }, conflictAlgorithm: ConflictAlgorithm.replace); + + final snapshot = await LocalDb.exportCopy(); + await db.delete('day_result', where: 'day_id = ?', whereArgs: [dayId]); + + await LocalDb.importFromDbFile(snapshot); + expect( + await db.query('day_result', where: 'day_id = ?', whereArgs: [dayId]), + hasLength(1), + ); + await File(snapshot).delete(); + await db.delete('day_result', where: 'day_id = ?', whereArgs: [dayId]); + }); + test('a snapshot is never left behind in temp', () async { // The export is a full second copy of the database. The old code renamed // it into place; the new one streams and must still delete the source. diff --git a/test/day_result_reencode_test.dart b/test/day_result_reencode_test.dart index 37011f2d..6a894419 100644 --- a/test/day_result_reencode_test.dart +++ b/test/day_result_reencode_test.dart @@ -36,11 +36,11 @@ Map bundleFor(int t0, {int n = 30}) => { ], }; -Future seedLegacy(Database db, String dayId, int t0) async { +Future seedLegacy(Database db, String dayId, int t0, {int n = 30}) async { await db.insert('day_result', { 'day_id': dayId, 'algo_version': 47, - 'payload_json': jsonEncode(bundleFor(t0)), + 'payload_json': jsonEncode(bundleFor(t0, n: n)), 'window_json': '{}', 'computed_at': 1234567, 'finalized': 1, @@ -225,25 +225,131 @@ void main() { expect(after, jsonEncode(tiny)); }); + test('a concurrent derive is never overwritten with a stale bundle', () async { + // The prepare is deliberately done outside any transaction, on a worker + // isolate, and takes hundreds of milliseconds — and derivation itself runs + // in more than one isolate. The update used to key on (day_id, + // algo_version) alone, so a bundle read before that work started was + // written back over whatever the other isolate had committed in the + // meantime, leaving the row holding the new scalar columns beside the old + // payload. The walk starts at the NEWEST day and kAlgoVersion is unbumped, + // so its first targets are exactly the rows a light derive is rewriting. + // + // A full heavy batch is seeded so the prepare genuinely occupies the window + // the write below lands in; the assertion on the return value pins that. + for (var d = 1; d <= 40; d++) { + await seedLegacy(db, '2026-01-${d.toString().padLeft(2, '0')}', + t0 + d * 86400, n: 800); + } + const newest = '2026-01-40'; + + final walk = LocalDb.reencodeLegacyDayResults(); + await Future.delayed(const Duration(milliseconds: 80)); + + // The other isolate finishing a derive of the newest day — the first row + // this walk read. + final fresh = bundleFor(t0 + 40 * 86400 + 3600, n: 900); + await db.update( + 'day_result', + {'payload_json': jsonEncode(fresh)}, + where: 'day_id = ? AND algo_version = ?', + whereArgs: [newest, 47], + ); + + expect( + await walk, + 39, + reason: 'the moved row must be the one row the walk declines to write', + ); + + final after = SeriesCodec.decodePayloadJson( + (await db.query( + 'day_result', + columns: ['payload_json'], + where: 'day_id = ?', + whereArgs: [newest], + )).first['payload_json'], + ); + expect(jsonEncode(after), jsonEncode(fresh)); + }); + + test('a row that lost the compare-and-set is converted later', () async { + // Declining the write is only half of it: the cursor must not step past + // the row and latch `done`, or it keeps the legacy shape forever. + for (var d = 1; d <= 40; d++) { + await seedLegacy(db, '2026-01-${d.toString().padLeft(2, '0')}', + t0 + d * 86400, n: 800); + } + const newest = '2026-01-40'; + + final walk = LocalDb.reencodeLegacyDayResults(); + await Future.delayed(const Duration(milliseconds: 80)); + await db.update( + 'day_result', + {'payload_json': jsonEncode(bundleFor(t0 + 40 * 86400 + 3600, n: 900))}, + where: 'day_id = ? AND algo_version = ?', + whereArgs: [newest, 47], + ); + expect(await walk, 39, reason: 'precondition: the row was skipped'); + + var total = 0; + for (var i = 0; i < 5; i++) { + final n = await LocalDb.reencodeLegacyDayResults(); + if (n == 0) break; + total += n; + } + expect(total, 1, reason: 'the skipped row is picked up by a later pass'); + final after = (await db.query( + 'day_result', + columns: ['payload_json'], + where: 'day_id = ?', + whereArgs: [newest], + )).first['payload_json'] as String; + expect(SeriesCodec.needsReencode(after), isFalse); + }); + test('an import rewinds a finished walk', () async { // importFromDbFile writes day_result rows with a raw batch.insert, so they // arrive in whatever shape the source device stored. The walk latches // `done` and is forward-only, so without a rewind those rows would keep the // legacy shape forever and the import would silently undo the compression. + // + // Driven through the REAL import rather than a hand-written cursor reset: + // written the other way, this test still passed with the rewind deleted + // from production, which is the only thing it exists to protect. await seedLegacy(db, '2026-01-01', t0); expect(await LocalDb.reencodeLegacyDayResults(), 1); expect(await LocalDb.reencodeLegacyDayResults(), 0); // walk is done - // What an import leaves behind: a legacy row that never met the write seam. - await seedLegacy(db, '2026-02-01', t0 + 86400 * 40); - expect( - await LocalDb.reencodeLegacyDayResults(), - 0, - reason: 'precondition: a latched walk ignores it', + // A source export holding one legacy row, the shape an older device wrote. + final srcPath = p.join( + await databaseFactory.getDatabasesPath(), + 'openstrap_reencode_src.db', + ); + await databaseFactory.deleteDatabase(srcPath); + final src = await databaseFactory.openDatabase(srcPath); + await src.execute( + 'CREATE TABLE day_result (' + 'day_id TEXT NOT NULL, algo_version INTEGER NOT NULL, ' + 'payload_json TEXT, window_json TEXT, computed_at INTEGER, ' + 'finalized INTEGER DEFAULT 0, skipped INTEGER DEFAULT 0, ' + 'partial INTEGER DEFAULT 0, rhr REAL, rmssd REAL, readiness REAL, ' + 'PRIMARY KEY (day_id, algo_version))', ); + await src.insert('day_result', { + 'day_id': '2026-02-01', + 'algo_version': 47, + 'payload_json': jsonEncode(bundleFor(t0 + 86400 * 40)), + 'window_json': '{}', + 'computed_at': 1234567, + 'finalized': 0, + 'skipped': 0, + 'partial': 0, + }); + await src.close(); - // The rewind importFromDbFile performs. - await LocalDb.putComputeFreshness(LocalDb.kReencodeCursorKey, '{}'); + final counts = await LocalDb.importFromDbFile(srcPath); + expect(counts['day_result'], 1); var total = 0; for (var i = 0; i < 10; i++) { diff --git a/test/series_codec_test.dart b/test/series_codec_test.dart index e69c9129..0c72c2ee 100644 --- a/test/series_codec_test.dart +++ b/test/series_codec_test.dart @@ -227,29 +227,33 @@ void main() { }); group('malformed input degrades, never throws', () { - test('an envelope with neither dt nor to yields an empty curve', () { - expect(SeriesCodec.decodeCurve({'t0': 1, 'v': [1, 2]}), isEmpty); + // An unrecognised map is handed BACK, not replaced with an empty curve. + // decodePayload is the read seam for every stored payload — baselines and + // freshness rows go through it too — so emptying what it does not + // understand would silently destroy data it was only passing along. + test('an envelope with neither dt nor to is returned unchanged', () { + final raw = { + 't0': 1, + 'v': [1, 2], + }; + expect(SeriesCodec.decodeCurve(raw), same(raw)); }); - test('a ragged offset envelope yields an empty curve', () { - expect( - SeriesCodec.decodeCurve({ - 't0': 1, - 'to': [0, 5], - 'v': [1, 2, 3], - }), - isEmpty, - ); + test('a ragged offset envelope is returned unchanged', () { + final raw = { + 't0': 1, + 'to': [0, 5], + 'v': [1, 2, 3], + }; + expect(SeriesCodec.decodeCurve(raw), same(raw)); }); - test('a missing t0 yields an empty curve', () { - expect( - SeriesCodec.decodeCurve({ - 'dt': 60, - 'v': [1, 2], - }), - isEmpty, - ); + test('a missing t0 is returned unchanged', () { + final raw = { + 'dt': 60, + 'v': [1, 2], + }; + expect(SeriesCodec.decodeCurve(raw), same(raw)); }); test('unparseable json decodes to null, not a throw', () { From ba18c281644b8ea8d4e53baf9634c385342ee80e Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Tue, 11 Aug 2026 20:09:09 +0530 Subject: [PATCH 5/6] place the interleave instead of racing a sleep against it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two compare-and-set tests waited 80 ms and hoped the competing write landed between the batch prepare and the transaction. The property under test is that exact placement, so a test that only holds while the runner stays inside the delay is one that goes red on a loaded box and, worse, could stop exercising the race at all while still passing. The walk awaits a hook at that boundary — null in production, one null check per batch — and the tests drive the competing derive from it. --- lib/data/db.dart | 20 +++++++++++++ test/day_result_reencode_test.dart | 46 +++++++++++++++++------------- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/lib/data/db.dart b/lib/data/db.dart index 62df570a..fdc043ac 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -13,6 +13,7 @@ import 'dart:convert'; import 'dart:io'; import 'dart:isolate'; +import 'package:flutter/foundation.dart' show visibleForTesting; import 'package:openstrap_protocol/openstrap_protocol.dart' as proto; import 'package:path/path.dart' as p; import 'package:path_provider/path_provider.dart'; @@ -4479,6 +4480,18 @@ class LocalDb { /// Bookkeeping key for the one-time walk in [reencodeLegacyDayResults]. static const String kReencodeCursorKey = 'series_reencode'; + /// Test seam: awaited inside [reencodeLegacyDayResults] between the batch + /// prepare and the write transaction. Null in production, and the only cost + /// there is one null check per batch. + /// + /// It exists because the race the compare-and-set guards is a placement + /// problem, not a timing one: a competing derive has to land in that exact + /// window. Pinning it with a sleep meant the test asserted a real property + /// only as long as the runner stayed inside the delay, which is the shape of + /// a test that passes on a laptop and goes red on a loaded CI box. + @visibleForTesting + static Future Function()? debugAfterReencodePrepare; + /// Re-encode a BOUNDED batch of pre-codec `day_result` rows into the compact /// curve format, newest first. Returns how many rows were rewritten. /// @@ -4583,6 +4596,13 @@ class LocalDb { (row['payload_json'] is String) ? row['payload_json'] as String : '', ]; final prepared = await Isolate.run(() => _reencodeBatch(payloads)); + // The window the compare-and-set below exists to close: the rows were read, + // the encode took real time, and nothing has been locked yet. A test drives + // a competing write through here rather than racing a sleep against it — + // the interleave is the whole property, so it has to be placed rather than + // hoped for. + final afterPrepare = debugAfterReencodePrepare; + if (afterPrepare != null) await afterPrepare(); final updates = <({int rowIndex, String dayId, int algoVersion, String from, String to})>[]; diff --git a/test/day_result_reencode_test.dart b/test/day_result_reencode_test.dart index 6a894419..442b53ed 100644 --- a/test/day_result_reencode_test.dart +++ b/test/day_result_reencode_test.dart @@ -78,6 +78,8 @@ void main() { }); tearDown(() async { + // Static, so it would otherwise leak into whatever runs after it. + LocalDb.debugAfterReencodePrepare = null; await LocalDb.close(); final dir = await databaseFactory.getDatabasesPath(); await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); @@ -243,21 +245,22 @@ void main() { } const newest = '2026-01-40'; - final walk = LocalDb.reencodeLegacyDayResults(); - await Future.delayed(const Duration(milliseconds: 80)); - // The other isolate finishing a derive of the newest day — the first row - // this walk read. + // this walk read. Driven from the seam between the prepare and the write + // rather than raced against a sleep, so the interleave is placed and the + // test cannot quietly stop exercising it on a slower runner. final fresh = bundleFor(t0 + 40 * 86400 + 3600, n: 900); - await db.update( - 'day_result', - {'payload_json': jsonEncode(fresh)}, - where: 'day_id = ? AND algo_version = ?', - whereArgs: [newest, 47], - ); + LocalDb.debugAfterReencodePrepare = () async { + await db.update( + 'day_result', + {'payload_json': jsonEncode(fresh)}, + where: 'day_id = ? AND algo_version = ?', + whereArgs: [newest, 47], + ); + }; expect( - await walk, + await LocalDb.reencodeLegacyDayResults(), 39, reason: 'the moved row must be the one row the walk declines to write', ); @@ -282,15 +285,18 @@ void main() { } const newest = '2026-01-40'; - final walk = LocalDb.reencodeLegacyDayResults(); - await Future.delayed(const Duration(milliseconds: 80)); - await db.update( - 'day_result', - {'payload_json': jsonEncode(bundleFor(t0 + 40 * 86400 + 3600, n: 900))}, - where: 'day_id = ? AND algo_version = ?', - whereArgs: [newest, 47], - ); - expect(await walk, 39, reason: 'precondition: the row was skipped'); + LocalDb.debugAfterReencodePrepare = () async { + await db.update( + 'day_result', + {'payload_json': jsonEncode(bundleFor(t0 + 40 * 86400 + 3600, n: 900))}, + where: 'day_id = ? AND algo_version = ?', + whereArgs: [newest, 47], + ); + }; + expect(await LocalDb.reencodeLegacyDayResults(), 39, + reason: 'precondition: the row was skipped'); + // Only the first pass races; the retries below must run clean. + LocalDb.debugAfterReencodePrepare = null; var total = 0; for (var i = 0; i < 5; i++) { From d43144ef5099b51eeb8383273dcdfbf0c4e6a3de Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Tue, 11 Aug 2026 22:25:39 +0530 Subject: [PATCH 6/6] fix the quadratic view branch, guard v_hypnogram, reject a short gzip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The offset branch of v_series joined json_each over `.to` to json_each over `.v` on `key`. SQLite cannot index a table-valued function, so that join has no plan except a full cross product and its cost grows with the square of the curve length. hrv_day, hrv_timeline and resp_day are all irregularly sampled and therefore all offset-encoded, so it was the normal path: a 365-day AVG(v) took 877 ms where the pre-codec view took 106, and a 30-day slice of 1440-point curves took 2.1 s. It now walks `.v` once and indexes into `.to` by that key, which measures 89 ms. The `e.key < json_array_length(.to)` bound keeps the result row-for-row identical to the join, including for a payload whose `to` is shorter than its `v`, which the join dropped and an unbounded index would have emitted with a null timestamp. v_hypnogram never got the json_valid guard v_series has, so one malformed payload_json made the whole view throw and took every day's sleep stages away from the coach rather than only its own. A truncated .db.gz restored as success. zlib checks the gzip CRC and length, but only on reaching the end of the stream, and a stream that just stops never gets there — Dart's decoder returns what it inflated with no error. A backup cut at 99.9% inflated, sniffed as SQLite, merged and reported success one row short, which is exactly what a half-synced cloud copy looks like. The trailer is now read off the file and compared against what came out of the decoder, and a mismatch is refused with a message about the file being incomplete. Failure still leaves the live database untouched and nothing half-inflated on disk. decodeCurve skipped offset entries that were not integers, handing back a curve silently missing samples; verifyLossless could not see it because it compares a decode against a decode rather than against the original. It now leaves such a curve alone. SQL still adds a fractional offset through rather than suppressing it, noted where it happens. Three tests were asserting nothing. The index-drop test asserted an index was absent from a fresh database that never creates it, so deleting the DROP left it green; it now plants the index, reopens, and checks the open removed it, which is the path a real upgrade takes. verifyLossless had no direct coverage at all despite being the only gate before overwriting a day whose substrate has aged out. The offset branch's shape is pinned by a query-plan assertion. .gitattributes asked for `text diff` on Dart sources when `diff` is the part that keeps a NUL-containing file reviewable; `text` also switches on end-of-line normalisation repo-wide, which was not the intent. --- .gitattributes | 17 ++- lib/data/db.dart | 39 +++++-- lib/data/series_codec.dart | 18 ++- lib/import/import_container.dart | 66 +++++++++++ test/coach_views_series_shapes_test.dart | 89 ++++++++++++++ test/db_storage_hygiene_test.dart | 140 ++++++++++++++--------- test/import_container_test.dart | 59 +++++++++- test/series_codec_test.dart | 83 +++++++++++++- 8 files changed, 438 insertions(+), 73 deletions(-) diff --git a/.gitattributes b/.gitattributes index a2510bd0..6a234805 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,6 +1,11 @@ -# Dart sources are text, always. A test that needs a NUL byte in a fixture can -# write one as a literal, and git's binary heuristic then calls the whole file -# binary: the diff collapses to "Binary files differ" with no line counts, so -# nobody reviews it. Escape sequences are the fix in the source; this makes the -# diff readable even when a file slips through. -*.dart text diff +# A test that needs a NUL byte in a fixture can write one as a literal, and +# git's binary heuristic then calls the whole file binary: the diff collapses to +# "Binary files differ" with no line counts, so nobody reviews it. Escape +# sequences are the fix in the source; this makes the diff readable even when a +# file slips through. +# +# `diff`, not `text diff`. `diff` alone is the part that keeps such a file +# reviewable. `text` additionally turns on end-of-line normalisation for every +# Dart file in the repo, which is a working-tree-wide change with nothing to do +# with reviewable diffs. +*.dart diff diff --git a/lib/data/db.dart b/lib/data/db.dart index fdc043ac..2ca13269 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -1774,10 +1774,24 @@ class LocalDb { // as daysWithSleepTst. // // The grid branch needs no running sum because json_each exposes an array's - // index as `key`, so t = t0 + key*dt. Offset pairs `to` and `v` on that - // same key. Verified row-for-row against the pre-codec view on the three - // tracked bundle fixtures, including a database holding both shapes at once - // (test/coach_views_series_shapes_test.dart). + // index as `key`, so t = t0 + key*dt. Verified row-for-row against the + // pre-codec view on the three tracked bundle fixtures, including a database + // holding both shapes at once (test/coach_views_series_shapes_test.dart). + // + // The offset branch walks `.v` ONCE and indexes into `.to` by that key. The + // obvious form — json_each over `.to` joined to json_each over `.v` on + // `key` — is quadratic: SQLite cannot index a table-valued function, so the + // join degrades to a full cross product of the two and the cost grows with + // the SQUARE of the curve length. hrv_day, hrv_timeline and resp_day are + // all irregularly sampled and therefore all offset-encoded, so this is the + // hot path, not a corner: on 365 real days a `SELECT AVG(v)` measured 877 ms + // against 89 ms, and on 1440-point curves a 30-day slice took 2.1 s. The + // `e.key < json_array_length(.to)` bound is what keeps the rewrite + // row-for-row identical rather than merely equivalent on well-formed data — + // the join emitted min(len(to), len(v)) rows, and without the bound a `to` + // shorter than `v` would gain rows with a NULL `t`. Pinned by a query-plan + // assertion in test/coach_views_series_shapes_test.dart: two nested virtual + // table scans in this branch is the regression. await db.execute(''' CREATE VIEW v_series AS WITH latest AS ( @@ -1812,20 +1826,27 @@ class LocalDb { WHERE json_extract(l.payload_json, c.pth||'.dt') IS NOT NULL UNION ALL SELECT l.day_id, c.sk, - json_extract(l.payload_json, c.pth||'.t0') + et.value, ev.value + json_extract(l.payload_json, c.pth||'.t0') + + json_extract(l.payload_json, c.pth||'.to['||e.key||']'), + e.value FROM latest l JOIN curve c - JOIN json_each(json_extract(l.payload_json, c.pth||'.to')) et - JOIN json_each(json_extract(l.payload_json, c.pth||'.v')) ev ON ev.key = et.key - WHERE json_extract(l.payload_json, c.pth||'.to') IS NOT NULL - AND json_extract(l.payload_json, c.pth||'.dt') IS NULL + JOIN json_each(json_extract(l.payload_json, c.pth||'.v')) e + WHERE json_extract(l.payload_json, c.pth||'.dt') IS NULL + AND json_extract(l.payload_json, c.pth||'.to') IS NOT NULL + AND e.key < json_array_length(json_extract(l.payload_json, c.pth||'.to')) '''); // Sleep stage segments (different element shape from the {t,v} curves). + // Same json_valid guard as v_series and for the same reason: without it one + // malformed payload_json anywhere in day_result makes this view THROW, so a + // single corrupt row takes every day's sleep stages away from the coach + // instead of just its own. await db.execute(''' CREATE VIEW v_hypnogram AS WITH latest AS ( SELECT r.day_id, r.payload_json FROM day_result r JOIN (SELECT day_id, MAX(algo_version) v FROM day_result GROUP BY day_id) m ON r.day_id = m.day_id AND r.algo_version = m.v + WHERE json_valid(r.payload_json) ) SELECT l.day_id AS date, json_extract(e.value,'\$.start') AS start_ts, diff --git a/lib/data/series_codec.dart b/lib/data/series_codec.dart index adf6ddbd..eeebb6e2 100644 --- a/lib/data/series_codec.dart +++ b/lib/data/series_codec.dart @@ -193,9 +193,25 @@ class SeriesCodec { final to = raw['to']; if (to is! List || to.length != vs.length) return raw; + // ALL the offsets or none of them. Skipping just the entries that are not + // ints emitted a SHORT curve — a plausible-looking curve quietly missing + // samples, which is worse than one the reader can see is unusable, and + // `verifyLossless` could not tell because it compares decode against + // decode, not against the original. [encodeCurve] cannot produce this (it + // only writes int offsets); a foreign or corrupted payload can, and for + // those the file's rule applies — leave it alone rather than half-read it. + // + // SQL DIVERGES HERE and cannot be made to agree cheaply: `v_series` adds + // `to[key]` to `t0` per row, so a fractional offset comes out as a + // fractional `t` rather than being suppressed. Checking every offset's type + // in the view would cost a json_type call per sample on the coach's hottest + // path, to defend a shape nothing in this app writes. + for (final o in to) { + if (o is! int) return raw; + } return [ for (var i = 0; i < vs.length; i++) - if (to[i] is int) {'t': t0 + (to[i] as int), valueKey: vs[i]}, + {'t': t0 + (to[i] as int), valueKey: vs[i]}, ]; } diff --git a/lib/import/import_container.dart b/lib/import/import_container.dart index 1cc284f5..3746f6c0 100644 --- a/lib/import/import_container.dart +++ b/lib/import/import_container.dart @@ -28,6 +28,7 @@ import 'dart:async'; import 'dart:io'; +import 'dart:typed_data'; import 'package:archive/archive.dart'; import 'package:path/path.dart' as p; @@ -199,6 +200,22 @@ class ResolvedNoopDatabase { /// most often is (a full database backup) is exactly the size that must never /// be buffered twice. /// +/// VERIFIED against the gzip trailer, which the decoder alone does not get to. +/// A gzip ends with a CRC32 and an ISIZE over the uncompressed bytes, and zlib +/// does check them — but only on reaching the end of the stream. A stream that +/// simply STOPS never gets there, and Dart's decoder returns whatever it +/// managed to inflate without raising. So a backup truncated to 99.9% inflated +/// cleanly, sniffed as SQLite, merged, and reported success one row short of +/// what the user was restoring; a half-synced iCloud or Nextcloud copy is +/// exactly the case restore exists for. Cuts between 50% and 99% were caught +/// only incidentally, by sqlite, and surfaced as a raw ffi exception rather +/// than something a user could act on. Reading the trailer off the file instead +/// of waiting for the decoder to reach it catches every cut at the container, +/// where the message can name what is wrong. +/// +/// Single-member gzip only, which is what every writer in this app produces. +/// Concatenated members would carry a trailer per member and be rejected here. +/// /// The caller owns [dir] and the file inside it. Future inflateGzip(String path, Directory dir) async { if (await sniffFile(path) != ImportContainer.gzip) return null; @@ -211,6 +228,7 @@ Future inflateGzip(String path, Directory dir) async { final destPath = p.join(dir.path, base); final sink = File(destPath).openWrite(); var written = 0; + var crc = 0; // COUNT INSIDE A TRANSFORMER, then `pipe`. The obvious `await for (…) // sink.add(chunk)` reads as streaming but is not: `IOSink.add` queues without // back-pressure, so an inflate that outruns the disk buffers the ENTIRE @@ -232,11 +250,16 @@ Future inflateGzip(String path, Directory dir) async { out.close(); return; } + // Folded into the same pass as the ceiling: the inflated bytes are only + // in memory here, and re-reading the restored file to checksum it would + // double the I/O on a multi-hundred-MB backup. + crc = getCrc32(chunk, crc); out.add(chunk); }, ); try { await File(path).openRead().transform(gzip.decoder).transform(counted).pipe(sink); + await _checkGzipTrailer(path, written: written, crc: crc); } catch (e) { try { await sink.close(); @@ -253,6 +276,49 @@ Future inflateGzip(String path, Directory dir) async { return destPath; } +/// Compare the gzip trailer of [path] against what actually came out of the +/// decoder, and throw [ImportFormatException] when they disagree. +/// +/// The last eight bytes of a gzip member are CRC32 then ISIZE, both +/// little-endian over the UNCOMPRESSED data, ISIZE modulo 2^32. Either one +/// mismatching means the file we read is not the file that was written — the +/// usual cause being a copy that was still syncing. +Future _checkGzipTrailer( + String path, { + required int written, + required int crc, +}) async { + final file = File(path); + final length = await file.length(); + // 10-byte header + 2-byte empty deflate block + 8-byte trailer is the + // smallest possible gzip; anything shorter lost its trailer outright. + if (length < 18) throw _gzipTruncated(path); + + final raf = await file.open(); + final Uint8List trailer; + try { + await raf.setPosition(length - 8); + trailer = await raf.read(8); + } finally { + await raf.close(); + } + if (trailer.length != 8) throw _gzipTruncated(path); + + final expectedCrc = + trailer[0] | trailer[1] << 8 | trailer[2] << 16 | trailer[3] << 24; + final expectedSize = + trailer[4] | trailer[5] << 8 | trailer[6] << 16 | trailer[7] << 24; + if (written % 0x100000000 != expectedSize || crc != expectedCrc) { + throw _gzipTruncated(path); + } +} + +ImportFormatException _gzipTruncated(String path) => ImportFormatException( + '“${p.basename(path)}” is incomplete or damaged — the compressed data does ' + 'not match the checksum stored in the file. If it came from a cloud folder, ' + 'wait for it to finish downloading, or export it again.', +); + /// If [path] is a NOOP full backup, return its database ready to open. /// /// Handles both shapes users arrive with: the `.noopbak` itself (a ZIP whose diff --git a/test/coach_views_series_shapes_test.dart b/test/coach_views_series_shapes_test.dart index 15710615..6d4534f2 100644 --- a/test/coach_views_series_shapes_test.dart +++ b/test/coach_views_series_shapes_test.dart @@ -243,6 +243,95 @@ void main() { } }); + test('the offset branch stays linear in the curve length', () async { + // SQLite cannot index a table-valued function. Written as json_each over + // `.to` JOINed to json_each over `.v` on `key`, the offset branch therefore + // has no way to resolve the join except a full cross product of the two, + // and its cost grows with the SQUARE of the curve length — 877 ms against + // 89 ms on a year of real days, worse the denser the sampling gets. Since + // hrv_day, hrv_timeline and resp_day are all irregularly sampled, every one + // of them takes that path. + // + // The shape is what the plan shows: one virtual-table scan nested inside + // another. v_series has exactly three json_each calls, one per branch, so a + // fourth scan appearing here means the join came back. + final plan = await db.rawQuery('EXPLAIN QUERY PLAN SELECT * FROM v_series'); + final scans = plan + .map((r) => r['detail'] as String? ?? '') + .where((d) => d.contains('VIRTUAL TABLE')) + .toList(); + expect( + scans, + hasLength(3), + reason: + 'one json_each per branch, never a TVF joined to a TVF:\n' + '${plan.map((r) => r['detail']).join('\n')}', + ); + }); + + test('an offset curve with fewer offsets than values gains no rows', () async { + // Only a foreign or corrupt payload can carry a `to` shorter than its `v` — + // the codec writes them in lockstep. It still pins the bound that keeps the + // linear form row-for-row identical to the join it replaced: the join + // emitted min(len(to), len(v)) rows, so a value with no offset has to be + // dropped rather than emitted with a NULL timestamp. + await db.insert('day_result', { + 'day_id': '2026-01-05', + 'algo_version': 47, + 'payload_json': jsonEncode({ + 'series': { + 'hrv_day': { + 't0': 100, + 'to': [0, 5], + 'v': [1, 2, 3, 4], + }, + }, + }), + 'window_json': '{}', + 'computed_at': 0, + }); + try { + final rows = await seriesRows(db, '2026-01-05'); + expect(rows.map((r) => r['t']), [100, 105]); + } finally { + await db.delete( + 'day_result', + where: 'day_id = ?', + whereArgs: ['2026-01-05'], + ); + } + }); + + test('one corrupt payload does not fail v_hypnogram either', () async { + // Same failure and same guard as v_series above. Without json_valid in the + // `latest` CTE, one unparseable row made json_extract raise for the whole + // query, so a single corrupt day removed EVERY day's sleep stages from the + // coach rather than only its own. + await db.insert('day_result', { + 'day_id': '2026-01-06', + 'algo_version': 47, + 'payload_json': '{ this is not json', + 'window_json': '{}', + 'computed_at': 0, + }); + try { + // Scanned unfiltered, the way csv_export and the coach actually read it. + // A `WHERE date = …` proves nothing here: SQLite pushes that predicate + // into the CTE and never evaluates json_extract on the corrupt row. + final rows = await db.rawQuery( + 'SELECT date, start_ts, end_ts, stage FROM v_hypnogram ' + 'ORDER BY date ASC, start_ts ASC', + ); + expect(rows.where((r) => r['date'] == '2026-01-01'), hasLength(3)); + } finally { + await db.delete( + 'day_result', + where: 'day_id = ?', + whereArgs: ['2026-01-06'], + ); + } + }); + test('v_hypnogram is unaffected by the encoding', () async { final legacy = await db.rawQuery( "SELECT start_ts, end_ts, stage FROM v_hypnogram " diff --git a/test/db_storage_hygiene_test.dart b/test/db_storage_hygiene_test.dart index 437f258f..56f3465d 100644 --- a/test/db_storage_hygiene_test.dart +++ b/test/db_storage_hygiene_test.dart @@ -37,17 +37,33 @@ void main() { expect(idx, contains('idx_decoded_rr_ts_beat_unique')); }); - test('the duplicate-of-primary-key rr index is gone', () async { + test('an existing duplicate-of-primary-key rr index is dropped on open', () async { // idx_decoded_rr_counter(counter, beat_index) duplicated, column for column, // the index PRIMARY KEY (counter, beat_index) already creates. Measured on a // 3-day fill: both b-trees 3,264,512 bytes — ~1.09 MB/day of pure // duplication plus a second b-tree write per beat on the hottest insert // path in the app. - final db = await LocalDb.instance; - final idx = (await db.rawQuery( - "SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='decoded_rr'", - )).map((r) => r['name'] as String?).whereType().toList(); - expect(idx, isNot(contains('idx_decoded_rr_counter'))); + // + // PLANTED FIRST, then reopened. A fresh database never creates the index + // any more, so simply asserting it is absent asserts nothing — deleting the + // DROP leaves the test green. The installs that have the index are the ones + // that were created before it stopped being written, and the only thing + // that removes it for them is `_repairOpenSchema` on the next open. That is + // the path this reproduces. + var db = await LocalDb.instance; + await db.execute( + 'CREATE INDEX IF NOT EXISTS idx_decoded_rr_counter ' + 'ON decoded_rr(counter, beat_index)', + ); + expect( + await _rrIndexes(db), + contains('idx_decoded_rr_counter'), + reason: 'the fixture must actually plant the index', + ); + + await LocalDb.close(); + db = await LocalDb.instance; + expect(await _rrIndexes(db), isNot(contains('idx_decoded_rr_counter'))); }); test('counter lookups are still index-served without it', () async { @@ -59,13 +75,19 @@ void main() { 'ORDER BY beat_index', 'EXPLAIN QUERY PLAN SELECT * FROM decoded_rr WHERE counter BETWEEN 1 AND 9', ]) { - final detail = (await db.rawQuery(sql)) - .map((r) => r['detail'].toString()) - .join(' | '); - expect(detail.toUpperCase(), contains('USING'), - reason: 'planner fell back to a full scan: $detail'); - expect(detail, contains('sqlite_autoindex_decoded_rr_1'), - reason: 'expected the primary key auto-index: $detail'); + final detail = (await db.rawQuery( + sql, + )).map((r) => r['detail'].toString()).join(' | '); + expect( + detail.toUpperCase(), + contains('USING'), + reason: 'planner fell back to a full scan: $detail', + ); + expect( + detail, + contains('sqlite_autoindex_decoded_rr_1'), + reason: 'expected the primary key auto-index: $detail', + ); } }); @@ -76,51 +98,58 @@ void main() { 'ORDER BY rr_ts_ms ASC, beat_index ASC', ); final detail = plan.map((r) => r['detail'].toString()).join(' | '); - expect(detail, contains('idx_decoded_rr_ts_beat_unique'), - reason: 'planner fell back to a scan: $detail'); - expect(detail.toUpperCase(), isNot(contains('USE TEMP B-TREE')), - reason: 'ordering should come from the index: $detail'); + expect( + detail, + contains('idx_decoded_rr_ts_beat_unique'), + reason: 'planner fell back to a scan: $detail', + ); + expect( + detail.toUpperCase(), + isNot(contains('USE TEMP B-TREE')), + reason: 'ordering should come from the index: $detail', + ); }); - test('superseded intermediate generations are pruned, recent ones kept', - () async { - final db = await LocalDb.instance; - for (final table in const [ - 'sleep_session_candidates', - 'wake_day_features', - ]) { - for (final v in const [48, 49, 50]) { - for (final day in const ['2026-07-01', '2026-07-02']) { - await db.insert(table, { - 'day_id': day, - 'algo_version': v, - 'payload_json': '{}', - 'computed_at': 0, - }); + test( + 'superseded intermediate generations are pruned, recent ones kept', + () async { + final db = await LocalDb.instance; + for (final table in const [ + 'sleep_session_candidates', + 'wake_day_features', + ]) { + for (final v in const [48, 49, 50]) { + for (final day in const ['2026-07-01', '2026-07-02']) { + await db.insert(table, { + 'day_id': day, + 'algo_version': v, + 'payload_json': '{}', + 'computed_at': 0, + }); + } } } - } - final deleted = await LocalDb.pruneSupersededIntermediates(); - expect(deleted, 4, reason: 'two days x v48, in both tables'); + final deleted = await LocalDb.pruneSupersededIntermediates(); + expect(deleted, 4, reason: 'two days x v48, in both tables'); - for (final table in const [ - 'sleep_session_candidates', - 'wake_day_features', - ]) { - final left = (await db.rawQuery( - 'SELECT DISTINCT algo_version FROM $table ORDER BY algo_version', - )).map((r) => r['algo_version'] as int).toList(); - expect(left, [49, 50], reason: '$table keeps current + previous'); - } - }); + for (final table in const [ + 'sleep_session_candidates', + 'wake_day_features', + ]) { + final left = (await db.rawQuery( + 'SELECT DISTINCT algo_version FROM $table ORDER BY algo_version', + )).map((r) => r['algo_version'] as int).toList(); + expect(left, [49, 50], reason: '$table keeps current + previous'); + } + }, + ); test('pruning is a no-op when there is nothing superseded', () async { expect(await LocalDb.pruneSupersededIntermediates(), 0); }); - test( - 'a day stuck on an old version (raw aged out, never re-derived) is not ' + test('a day stuck on an old version (raw aged out, never re-derived) is not ' 'orphaned just because OTHER days reached newer versions', () async { final db = await LocalDb.instance; // '2026-06-01' only ever got derived once, at v48 — its raw substrate is @@ -146,11 +175,14 @@ void main() { await LocalDb.pruneSupersededIntermediates(); final stale = await LocalDb.sleepSessionCandidate('2026-06-01', 48); - expect(stale, isNotNull, - reason: - 'a table-wide "keep the 2 highest versions present anywhere" ' - 'cutoff would delete this the moment two OTHER days reach v49/50 ' - '— it must be scoped per day_id instead'); + expect( + stale, + isNotNull, + reason: + 'a table-wide "keep the 2 highest versions present anywhere" ' + 'cutoff would delete this the moment two OTHER days reach v49/50 ' + '— it must be scoped per day_id instead', + ); expect(stale!['payload_json'], '{"stale":true}'); // The recent days still get their own per-day retention (49/50 kept, @@ -162,3 +194,7 @@ void main() { expect(recent, [49, 50]); }); } + +Future> _rrIndexes(Database db) async => (await db.rawQuery( + "SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='decoded_rr'", +)).map((r) => r["name"] as String?).whereType().toList(); diff --git a/test/import_container_test.dart b/test/import_container_test.dart index 2a3b9a97..eb3e78d5 100644 --- a/test/import_container_test.dart +++ b/test/import_container_test.dart @@ -404,9 +404,8 @@ void main() { test('a corrupt gzip fails with guidance and leaves nothing behind', () async { // Valid magic so the sniff routes it here, garbage where the deflate - // stream should be. This is the damaged-file case; a merely TRUNCATED - // gzip is not reliably detectable, because Dart's decoder does not fail - // on a missing trailer. + // stream should be. The deflate stream itself rejects this one; the two + // tests below cover the damage the decoder does NOT notice. final path = await write('broken.csv.gz', [ 0x1F, 0x8B, 0x08, 0x00, 0, 0, 0, 0, 0x00, 0x03, ...List.generate(64, (i) => (i * 37 + 11) & 0xFF), @@ -427,6 +426,60 @@ void main() { } }); + test('a truncated gzip is refused instead of restored short', () async { + // The half-synced-backup case, and the one that mattered: `gzip.decoder` + // returns what it managed to inflate from a stream that just stops, with + // no error. A backup cut at 99.9% inflated, sniffed as SQLite, merged and + // reported success one row short of the data the user was restoring. + final body = utf8.encode( + List.generate(400, (i) => 'row,$i,${i * 7},value-$i').join('\n'), + ); + final full = gzip.encode(body); + final path = await write( + 'cut.csv.gz', + full.sublist(0, full.length - (full.length ~/ 200) - 1), + ); + final dir = await Directory.systemTemp.createTemp('gz_cut_'); + try { + await expectLater( + inflateGzip(path, dir), + throwsA(isA()), + ); + expect( + dir.listSync(), + isEmpty, + reason: 'a short inflate must not be left where a caller can read it', + ); + } finally { + await dir.delete(recursive: true); + } + }); + + test('a gzip whose contents no longer match its checksum is refused', + () async { + // The boundary of the case above: when the stream is COMPLETE, zlib + // reaches the trailer and checks it itself, so this is refused whether or + // not inflateGzip verifies anything. Pinned because that is exactly why + // truncation was missed — the checking only ever happened at the end of a + // stream that arrived, and a cut file never gets there. + final full = [ + ...gzip.encode(utf8.encode('a,b,c\n' * 500)), + ]; + final crcAt = full.length - 8; + full[crcAt] = full[crcAt] ^ 0xFF; + final path = await write('flipped.csv.gz', full); + final dir = await Directory.systemTemp.createTemp('gz_crc_'); + try { + await expectLater( + inflateGzip(path, dir), + throwsA(isA()), + ); + expect(dir.listSync(), isEmpty); + } finally { + await dir.delete(recursive: true); + } + }); + test('a non-gzip file is not claimed by inflateGzip', () async { final path = await write('plain.csv', utf8.encode('a,b')); final dir = await Directory.systemTemp.createTemp('gz_none_'); diff --git a/test/series_codec_test.dart b/test/series_codec_test.dart index 0c72c2ee..4c944e82 100644 --- a/test/series_codec_test.dart +++ b/test/series_codec_test.dart @@ -53,8 +53,9 @@ void main() { test('$name shrinks by at least half', () { final before = jsonEncode(loadFixture(name)).length; - final after = jsonEncode(SeriesCodec.encodePayload(loadFixture(name))) - .length; + final after = jsonEncode( + SeriesCodec.encodePayload(loadFixture(name)), + ).length; expect( after, lessThan(before), @@ -256,6 +257,19 @@ void main() { expect(SeriesCodec.decodeCurve(raw), same(raw)); }); + test('an offset envelope with a non-int offset is returned unchanged', () { + // Dropping only the bad entry returned a SHORT curve — three stored + // samples, two handed to the reader, nothing said about the third. A + // curve the caller can see is not a curve is recoverable; one that is + // silently two thirds of itself is not. + final raw = { + 't0': 1, + 'to': [0, 1.5, 3], + 'v': [1, 2, 3], + }; + expect(SeriesCodec.decodeCurve(raw), same(raw)); + }); + test('unparseable json decodes to null, not a throw', () { expect(SeriesCodec.decodePayloadJson('{not json'), isNull); expect(SeriesCodec.decodePayloadJson(null), isNull); @@ -274,4 +288,69 @@ void main() { expect(deepEquals(SeriesCodec.decodePayload(once), baseline), isTrue); }); }); + + // The per-row gate the backfill consults before OVERWRITING a stored bundle. + // Days older than rawRetentionDays have no substrate left to re-derive from, + // so a wrong `true` here is unrecoverable data loss. It has to fail closed: + // anything it cannot fully account for must come back false, not "probably + // fine". + group('verifyLossless', () { + for (final name in fixtures) { + test('$name vouches for itself', () { + expect( + SeriesCodec.verifyLossless(File(name).readAsStringSync()), + isTrue, + ); + }); + } + + test('an already-encoded bundle still vouches for itself', () { + // The backfill can meet a row a previous pass converted. Re-encoding it + // is a no-op, so the gate must not refuse it. + final encoded = jsonEncode( + SeriesCodec.encodePayload(loadFixture(fixtures.first)), + ); + expect(SeriesCodec.verifyLossless(encoded), isTrue); + }); + + test('a hand-built bundle covering all three shapes vouches', () { + final bundle = jsonEncode({ + 'scalars': {'rhr': 55.0}, + 'series': { + 'hr_curve': [ + for (var i = 0; i < 12; i++) {'t': 1000 + i * 60, 'v': 60 + i}, + ], + 'hrv_day': [ + {'t': 1009, 'v': 36.8}, + {'t': 1071, 'v': null}, + {'t': 1325, 'v': 72.5}, + ], + 'zone_timeline': [ + {'t': 1000, 'z': 0}, + {'t': 1060, 'z': 3}, + ], + 'hypnogram': [ + {'start': 1000, 'end': 4600, 'stage': 'light'}, + ], + }, + 'activity_curve': [ + for (var i = 0; i < 10; i++) {'t': 1000 + i * 300, 'v': i * 1.5}, + ], + }); + expect(SeriesCodec.verifyLossless(bundle), isTrue); + }); + + test('a payload that is not an object is refused', () { + // Nothing in day_result should look like this, which is the point: the + // gate has no idea what it is and therefore will not vouch for it. + expect(SeriesCodec.verifyLossless('[1,2,3]'), isFalse); + expect(SeriesCodec.verifyLossless('42'), isFalse); + expect(SeriesCodec.verifyLossless('null'), isFalse); + }); + + test('an unparseable payload is refused', () { + expect(SeriesCodec.verifyLossless('{not json'), isFalse); + expect(SeriesCodec.verifyLossless(''), isFalse); + }); + }); }