diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 03bfc53..6dff31c 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -1836,13 +1836,16 @@ class DerivationEngine { rangePages: rangePages, rangeRows: rangeRows, ); - final firstCounter = (decodedRows.first['counter'] as num?)?.toInt(); - final lastCounter = (decodedRows.last['counter'] as num?)?.toInt(); - final rrRows = firstCounter == null || lastCounter == null + // The page is ordered rec_ts ASC, so first = min second, last = max. + // decoded_rr shares the rec_ts key, so this pulls exactly the page's + // beats — no counter span (which broke across the strap's reboot reset). + final firstRecTs = (decodedRows.first['rec_ts'] as num?)?.toInt(); + final lastRecTs = (decodedRows.last['rec_ts'] as num?)?.toInt(); + final rrRows = firstRecTs == null || lastRecTs == null ? const >[] - : await LocalDb.decodedRrByCounterRange( - fromCounter: firstCounter, - toCounter: lastCounter, + : await LocalDb.decodedRrByRecTsRange( + fromRecTs: firstRecTs, + toRecTs: lastRecTs, ); worker.send({'type': 'page', 'frames': decodedRows, 'rr': rrRows}); final last = decodedRows.last; diff --git a/lib/compute/derive_prepare.dart b/lib/compute/derive_prepare.dart index cd02dc7..2e016d3 100644 --- a/lib/compute/derive_prepare.dart +++ b/lib/compute/derive_prepare.dart @@ -431,11 +431,14 @@ class _PrepareAccumulator { List> rrRows, ) { if (frames.isEmpty) return; - final rrByCounter = >>{}; + // Associate beats to frames by rec_ts (their shared key). The strap's counter + // resets on reboot, so grouping by counter mis-joined two seconds that reused + // one counter within a page. + final rrByRecTs = >>{}; for (final row in rrRows) { - final counter = _num(row['counter'])?.toInt(); - if (counter == null) continue; - rrByCounter.putIfAbsent(counter, () => >[]).add(row); + final recTs = _num(row['rec_ts'])?.toInt(); + if (recTs == null) continue; + rrByRecTs.putIfAbsent(recTs, () => >[]).add(row); } for (final row in frames) { final recTs = _num(row['rec_ts'])?.toInt(); @@ -454,9 +457,7 @@ class _PrepareAccumulator { // tsSec is what lets `Substrate.fromJson` tell "absent" (empty ⇒ // zero-filled) from "present but zero". skinContact.add(_num(row['skin_contact'])?.toInt() ?? 0); - final counter = _num(row['counter'])?.toInt(); - if (counter == null) continue; - final beats = rrByCounter[counter]; + final beats = rrByRecTs[recTs]; if (beats == null) continue; for (final beat in beats) { final rr = _num(beat['rr_ms'])?.toDouble(); diff --git a/lib/data/db.dart b/lib/data/db.dart index 2ca1326..a842fc5 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -1,8 +1,8 @@ // Local raw-first storage (SQLite via sqflite). // // Durable storage layers: -// decoded_onehz — canonical per-second decoded substrate, deduped by rec_ts. -// decoded_rr — sparse RR beats for that substrate, deduped by (rr_ts_ms, beat_index). +// decoded_onehz — canonical per-second decoded substrate, keyed by rec_ts. +// decoded_rr — sparse RR beats for that substrate, keyed by (rec_ts, beat_index). // samples — legacy header cache kept only for backward-compat fallback. // // `counter` (u32 @[3:7]) is still kept as the strap's record id, but analytics @@ -95,7 +95,7 @@ class LocalDb { /// pass it: sqflite throws `ArgumentError('onCreate must be null if no /// version is specified')` BEFORE opening anything when `onCreate` is given /// without `version` (sqflite_common database_mixin.dart). - static const int schemaVersion = 31; + static const int schemaVersion = 33; /// SQLite caps host parameters per statement (`SQLITE_MAX_VARIABLE_NUMBER` — /// only 999 on the builds shipped with older Android/iOS). Any `IN (?, ?, …)` @@ -326,7 +326,11 @@ class LocalDb { await _createCycleSymptom(db); } if (oldV < 19) { - await _createDecodedStore(db); + // The v17 step (or a v11-16 origin) may leave OLD counter-keyed decoded + // tables here; the backfill below writes through the rec_ts-keyed + // _queueDecodedOneHz, so convert to the current schema first (preserving + // any existing rows), then reconstruct the rest from raw_records. + await _rekeyDecodedStoreByRecTs(db); await _backfillDecodedStore(db); await _dropRawStore(db); await _ensureSessionSchema(db); // adds hrr_bpm @@ -430,6 +434,16 @@ class LocalDb { // output is untouched and the edits replay over it. await _createSleepNap(db); } + // NOTE: base is origin/main at schemaVersion 31. PR #231 (pending) bumps + // to 32 with a raw_archive migration; this fix uses 33 so both land — a + // trivial schemaVersion rebase is expected when they merge. + if (oldV < 33) { + // RE-KEY the decoded ledger off the volatile record `counter` onto + // rec_ts. The counter resets to ~0 on every reboot, so counter-as-PK + // let a post-reboot second REPLACE-evict a pre-reboot one — silently, + // unrecoverably deleting a 1 Hz row (raw_records is dropped). + await _rekeyDecodedStoreByRecTs(db); + } }, onOpen: (db) async { await _repairOpenSchema(db); @@ -2072,10 +2086,19 @@ class LocalDb { // analytics: one row per real second (`rec_ts`) plus sparse RR beats for that // second. raw_records stays as the replay/debug ledger and upgrade fallback. static Future _createDecodedStore(Database db) async { + // KEYED BY rec_ts, NOT the band's record `counter`. The strap resets its + // per-record counter to ~0 on every reboot, so `counter INTEGER PRIMARY KEY` + // let a post-reboot record (counter=c, rec_ts=T2) REPLACE-evict a still-present + // pre-reboot row (counter=c, rec_ts=T1) — silently and UNRECOVERABLY deleting + // T1's only decoded 1 Hz row (raw_records is DROPped, so this store is the sole + // system of record). rec_ts is unique per real second, so newest-wins REPLACE + // on rec_ts is safe. `counter` is demoted to a NOT NULL forensic column (also + // the keyset-cursor tiebreak in decodedOneHzBatchByRecTsRange, which never + // fires now that rec_ts is unique). await db.execute(''' CREATE TABLE IF NOT EXISTS decoded_onehz ( - counter INTEGER PRIMARY KEY, - rec_ts INTEGER NOT NULL, + rec_ts INTEGER PRIMARY KEY, + counter INTEGER NOT NULL, hr INTEGER NOT NULL, ax REAL NOT NULL, ay REAL NOT NULL, @@ -2085,40 +2108,25 @@ class LocalDb { skin_temp_raw INTEGER NOT NULL ) '''); + // Forensic-only lookup by the raw counter; not on any read path. await db.execute( - 'CREATE INDEX IF NOT EXISTS idx_decoded_onehz_rects ON decoded_onehz(rec_ts, counter)', - ); - await db.execute( - 'CREATE UNIQUE INDEX IF NOT EXISTS idx_decoded_onehz_rec_ts_unique ' - 'ON decoded_onehz(rec_ts)', + 'CREATE INDEX IF NOT EXISTS idx_decoded_onehz_counter ON decoded_onehz(counter)', ); + // decoded_rr shares the rec_ts key with its parent: PRIMARY KEY (rec_ts, + // beat_index). Parent and child now delete/replace by the SAME key, so no + // orphan guard is needed. rr_ts_ms (= rec_ts*1000) stays as the per-beat + // timestamp the compute worker reads. No secondary index: the rec_ts-range + // read path is served by the PK, and the old UNIQUE(rr_ts_ms, beat_index) is + // now implied by the PK (rr_ts_ms is rec_ts*1000). await db.execute(''' CREATE TABLE IF NOT EXISTS decoded_rr ( - counter INTEGER NOT NULL, + rec_ts INTEGER NOT NULL, beat_index INTEGER NOT NULL, rr_ts_ms INTEGER NOT NULL, rr_ms INTEGER NOT NULL, - PRIMARY KEY (counter, beat_index) + PRIMARY KEY (rec_ts, 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)', - ); - // idx_decoded_rr_ts(rr_ts_ms) was a strict prefix of the unique index - // above, so SQLite could already serve every rr_ts_ms lookup and ordering - // 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: @@ -2126,12 +2134,32 @@ class LocalDb { /// (second, beat_index). Older duplicate counters remain in raw_records for /// forensics, but analytics no longer sees them. static Future _rebuildCanonicalDecodedStore(Database db) async { - // Guarantee the source tables exist before we SELECT from them. On upgrade - // paths from before the decoded store landed, decoded_onehz/decoded_rr were - // never created in the migration chain, so this rebuild threw "no such table: - // decoded_onehz" — failing openDatabase on every launch (stuck at loading). - // Creating them (empty) here makes the dedup/rebuild a safe no-op in that case. - await _createDecodedStore(db); + // FROZEN v17 step: it dedups the OLD counter-keyed decoded tables by rec_ts + // via a `decoded_rr.counter` join. If the store is ALREADY rec_ts-keyed (the + // ladder created it fresh at v11 with the current schema, so decoded_rr has + // no `counter` column), it is already canonical — this rebuild is impossible + // and unnecessary, so skip it. A genuinely old (counter-keyed) store still + // gets the original rebuild here, and the v33 re-key converts it afterward. + final rrCols = await db.rawQuery('PRAGMA table_info(decoded_rr)'); + if (rrCols.isNotEmpty && !rrCols.any((c) => c['name'] == 'counter')) return; + // Guarantee the OLD-schema source tables exist before we SELECT from them. + // On upgrade paths from before the decoded store landed they were never + // created, so this rebuild threw "no such table" and bricked openDatabase. + await db.execute(''' + CREATE TABLE IF NOT EXISTS decoded_onehz ( + counter INTEGER PRIMARY KEY, rec_ts INTEGER NOT NULL, + hr INTEGER NOT NULL, ax REAL NOT NULL, ay REAL NOT NULL, az REAL NOT NULL, + spo2_red_raw INTEGER NOT NULL, spo2_ir_raw INTEGER NOT NULL, + skin_temp_raw INTEGER NOT NULL + ) + '''); + await db.execute(''' + CREATE TABLE IF NOT EXISTS decoded_rr ( + counter INTEGER NOT NULL, beat_index INTEGER NOT NULL, + rr_ts_ms INTEGER NOT NULL, rr_ms INTEGER NOT NULL, + PRIMARY KEY (counter, beat_index) + ) + '''); await db.execute('DROP TABLE IF EXISTS _decoded_onehz_new'); await db.execute('DROP TABLE IF EXISTS _decoded_rr_new'); // Drop any leftover temp-named indexes BEFORE recreating them. SQLite index @@ -2216,6 +2244,75 @@ class LocalDb { await db.execute('ALTER TABLE _decoded_rr_new RENAME TO decoded_rr'); } + /// v33: re-key the decoded store off the volatile record `counter` onto rec_ts. + /// + /// Rebuilds BOTH decoded tables FROM THE EXISTING decoded tables ONLY. It must + /// NOT backfill from raw_records (that table is DROPped — a raw-backfill would + /// zero the store, total loss). Existing rows have a unique rec_ts, so the copy + /// loses nothing; any pre-fix duplicate counters collapse to newest-wins per + /// rec_ts. Idempotent: a crash mid-migration re-runs cleanly (the temp tables + /// are dropped up front, and every write is INSERT OR REPLACE keyed on identity). + /// + /// All copies are INSERT ... SELECT (server-side, ZERO host-bound variables), + /// so the iOS SQLITE_MAX_VARIABLE_NUMBER (999) never applies — no chunking is + /// needed. Mirrors [_rebuildCanonicalDecodedStore]'s rename-aside shape. + static Future _rekeyDecodedStoreByRecTs(Database db) async { + // The source tables may not exist on a pre-decoded-store upgrade path; a + // create (new schema, IF NOT EXISTS) makes the copy a safe no-op there. On a + // normal path the OLD-schema tables already exist and this is a no-op — the + // columns we SELECT (rec_ts, counter, hr, …; beat_index, rr_ts_ms, rr_ms) + // are present in both the old and new decoded schemas. + await _createDecodedStore(db); + await db.execute('DROP TABLE IF EXISTS _decoded_onehz_v33'); + await db.execute('DROP TABLE IF EXISTS _decoded_rr_v33'); + await db.execute(''' + CREATE TABLE _decoded_onehz_v33 ( + rec_ts INTEGER PRIMARY KEY, + counter INTEGER NOT NULL, + hr INTEGER NOT NULL, + ax REAL NOT NULL, + ay REAL NOT NULL, + az REAL NOT NULL, + spo2_red_raw INTEGER NOT NULL, + spo2_ir_raw INTEGER NOT NULL, + skin_temp_raw INTEGER NOT NULL + ) + '''); + await db.execute(''' + CREATE TABLE _decoded_rr_v33 ( + rec_ts INTEGER NOT NULL, + beat_index INTEGER NOT NULL, + rr_ts_ms INTEGER NOT NULL, + rr_ms INTEGER NOT NULL, + PRIMARY KEY (rec_ts, beat_index) + ) + '''); + // Deterministic newest-wins: ORDER BY rec_ts, counter so INSERT OR REPLACE on + // the rec_ts PK keeps the highest-counter (latest-offloaded) row per second. + await db.execute( + 'INSERT OR REPLACE INTO _decoded_onehz_v33 ' + '(rec_ts, counter, hr, ax, ay, az, spo2_red_raw, spo2_ir_raw, skin_temp_raw) ' + 'SELECT rec_ts, counter, hr, ax, ay, az, spo2_red_raw, spo2_ir_raw, skin_temp_raw ' + 'FROM decoded_onehz ORDER BY rec_ts ASC, counter ASC', + ); + // rec_ts derived from rr_ts_ms (= rec_ts*1000 by construction). Pre-fix orphan + // beats (owning row evicted) re-home onto their real second here. + await db.execute( + 'INSERT OR REPLACE INTO _decoded_rr_v33 (rec_ts, beat_index, rr_ts_ms, rr_ms) ' + 'SELECT rr_ts_ms / 1000, beat_index, rr_ts_ms, rr_ms ' + 'FROM decoded_rr ORDER BY rr_ts_ms ASC, beat_index ASC', + ); + await db.execute('DROP TABLE IF EXISTS decoded_rr'); + await db.execute('DROP TABLE IF EXISTS decoded_onehz'); + await db.execute('ALTER TABLE _decoded_onehz_v33 RENAME TO decoded_onehz'); + await db.execute('ALTER TABLE _decoded_rr_v33 RENAME TO decoded_rr'); + // The rec_ts PK auto-indexes; add back the forensic counter index (the temp + // tables carried no named secondary indexes, so nothing leaked onto rename). + await db.execute( + 'CREATE INDEX IF NOT EXISTS idx_decoded_onehz_counter ON decoded_onehz(counter)', + ); + } + // raw_records — keyed by the band's per-record u32 `counter` (the natural // idempotency key; re-draining the same flash region inserts nothing new). Only // the 1 Hz historical substrate (0x2F / R24) is persisted here — LIVE high-rate @@ -2363,71 +2460,28 @@ class LocalDb { } } - /// THE orphan guard for an INSERT-OR-REPLACE into `decoded_onehz`. - /// - /// Queue this onto [batch] IMMEDIATELY BEFORE writing the row for [counter] @ - /// [recTs] — every write path into `decoded_onehz` must go through it, or it - /// strands `decoded_rr` beats (see [_queueDecodedOneHz] for the full - /// derivation of both eviction cases). Returns the number of ops queued. - static int _queueOrphanGuard( - Batch batch, { - required int counter, - required int recTs, - }) { - batch.rawDelete( - 'DELETE FROM decoded_rr WHERE ' - // (a) UNIQUE(rec_ts) eviction — the LOSER counter's beats. - 'counter IN ' - '(SELECT counter FROM decoded_onehz WHERE rec_ts = ? AND counter != ?) ' - // (b) counter-PK eviction — stale-timestamped beats under OUR counter. - 'OR (counter = ? AND rr_ts_ms != ?)', - [recTs, counter, counter, recTs * 1000], - ); - return 1; - } - - /// Queues the decoded_onehz + decoded_rr (+ orphan-guard delete) writes for - /// one raw onto [batch]. Returns the number of batch operations added, so a - /// caller committing a large offload can chunk the batch to bound the native - /// argument-list size (see [commitSyncBatch]). + /// Queues the decoded_onehz + decoded_rr writes for one raw onto [batch]. + /// Returns the number of batch operations added, so a caller committing a + /// large offload can chunk the batch to bound the native argument-list size + /// (see [commitSyncBatch]). static int _queueDecodedOneHz(Batch batch, RawRecord raw, Sample? sample) { final decoded = _decodeOneHzSample(raw, preferred: sample); if (decoded == null) return 0; final recTs = raw.recTs ?? decoded.tsEpoch; // TIME-KEYED, NEWEST-WINS (noop/WHOOP-4 model: dedupe records by their - // embedded timestamp, not by a counter). decoded_onehz has a UNIQUE(rec_ts) - // index and decoded_rr a UNIQUE(rr_ts_ms, beat_index). We use REPLACE, not - // IGNORE: the strap's record `counter` RESETS to ~0 on every reboot, so a - // post-reboot record whose second already had a row would be SILENTLY DROPPED - // under IGNORE — quarantining everything after a reboot (observed: whole days - // present in raw_records but absent from the decoded substrate the engine - // reads → "not worn / metrics still computing / strain –"). REPLACE lets the - // freshly-offloaded record for a given second win, which is what we want. - // - // ORPHAN GUARD: decoded_rr rows are keyed by their record's own counter. When - // the REPLACE below evicts a DIFFERENT counter's row for this second, that - // loser's RR beats would stay behind under a counter with no decoded_onehz - // row — invisible to the counter-joined prune (permanent leak). The winner's - // REPLACE on UNIQUE(rr_ts_ms, beat_index) only overwrites overlapping beat - // indexes, so delete the evicted counter's beats explicitly, in the same - // batch/transaction (mirrors the v17 rebuild's decoded_onehz join). + // embedded timestamp, not by the volatile counter). decoded_onehz is keyed + // by rec_ts and decoded_rr by (rec_ts, beat_index). We use REPLACE, not + // IGNORE: a freshly-offloaded record for a given second should win over a + // stale one. Because rec_ts is the key, the strap's per-reboot counter reset + // can no longer make one second's record evict another's (the pre-fix + // counter-PK eviction that silently, unrecoverably deleted 1 Hz rows). // - // …AND the COUNTER-PK eviction, which the guard used to miss entirely. - // `decoded_onehz` is `counter INTEGER PRIMARY KEY` as well as - // UNIQUE(rec_ts), and (per the comment above) the strap's counter RESETS to - // ~0 on every reboot — so this same REPLACE also silently DELETES the row - // of an OLDER SECOND that happened to reuse this counter. That older - // second's beats live under OUR counter carrying ITS rr_ts_ms, and only the - // overlapping beat_indexes get overwritten below: any beat at an index past - // the new record's beat count SURVIVES, still stamped days earlier. Neither - // prune path can ever see it (the counter-join finds a fresh rec_ts; the - // orphan sweep finds the counter present), so a later page's RR series was - // polluted with beats from another day — silently wrecking RMSSD/HRV. - // Drop every beat under this counter that is not stamped with THIS second. - var ops = _queueOrphanGuard(batch, counter: raw.counter, recTs: recTs); + // Clear this second's RR beats before reinserting so a SHRINKING beat count + // can't strand stale high-index beats — the parent+child share the rec_ts + // key, so this single DELETE replaces the old counter-based orphan guard. batch.insert('decoded_onehz', { - 'counter': raw.counter, 'rec_ts': recTs, + 'counter': raw.counter, 'hr': decoded.hr, 'ax': decoded.ax ?? 0, 'ay': decoded.ay ?? 0, @@ -2436,12 +2490,14 @@ class LocalDb { 'spo2_ir_raw': decoded.spo2IrRaw ?? 0, 'skin_temp_raw': decoded.skinTempRaw ?? 0, }, conflictAlgorithm: ConflictAlgorithm.replace); - ops++; // the decoded_onehz insert + var ops = 1; // the decoded_onehz insert + batch.rawDelete('DELETE FROM decoded_rr WHERE rec_ts = ?', [recTs]); + ops++; for (var i = 0; i < decoded.rrIntervalsMs.length; i++) { final rr = decoded.rrIntervalsMs[i]; if (rr <= 0) continue; batch.insert('decoded_rr', { - 'counter': raw.counter, + 'rec_ts': recTs, 'beat_index': i, 'rr_ts_ms': recTs * 1000, 'rr_ms': rr, @@ -3061,92 +3117,28 @@ class LocalDb { ); } - /// How many times [decodedRrByCounterRange]'s degraded counter-span fallback - /// hit its row cap and therefore returned an INCOMPLETE set of beats. Any - /// value above zero means some window's HRV was computed from truncated - /// input; it should stay at zero in normal operation. - static int decodedRrFallbackTruncations = 0; - - /// Sparse RR beats for one contiguous decoded 1 Hz page. - /// - /// [fromCounter] / [toCounter] are the page's FIRST and LAST row counters, as - /// returned by [decodedOneHzBatchByRecTsRange] (which orders `rec_ts ASC, - /// counter ASC`). They are page ENDPOINTS, **not** a monotonic counter span: - /// the strap's counter resets to ~0 on every reboot, so a page straddling a - /// reboot has first = a pre-reboot high and last = a post-reboot low. The old - /// `WHERE counter >= ? AND counter <= ?` then read `>= 1200000 AND <= 5` and - /// returned ZERO rows — the entire page's RR beats vanished with no error, so - /// that window silently produced no RMSSD/HRV at all. - /// - /// Selection is therefore by the page's real TIME window, resolved from those - /// two endpoint counters. `decoded_onehz` is UNIQUE(rec_ts), so - /// `[first.rec_ts, last.rec_ts]` contains exactly the page's rows — no - /// over-fetch — and the join to `decoded_onehz` additionally keeps orphaned - /// beats (whose owning row was evicted) out of the read path. + /// Sparse RR beats for one contiguous decoded 1 Hz page, by its rec_ts window. /// - /// When the endpoints are NOT real rows the caller is asking for a plain - /// counter span (e.g. `0 .. 1<<30` = "everything"); that falls back to a - /// NORMALIZED counter range so an inverted pair still can't return nothing. - static Future>> decodedRrByCounterRange({ - required int fromCounter, - required int toCounter, + /// [fromRecTs] / [toRecTs] are the page's first and last record seconds (the + /// page is ordered `rec_ts ASC`, so first = min, last = max). decoded_rr shares + /// the rec_ts key with decoded_onehz, so `[fromRecTs, toRecTs]` on the PK + /// contains exactly the page's beats — bounded, indexed, and immune to the + /// strap's reboot counter reset (the old counter-span read could degenerate to + /// `counter >= high AND counter <= low` = zero rows, silently dropping a whole + /// page's RR). + static Future>> decodedRrByRecTsRange({ + required int fromRecTs, + required int toRecTs, }) async { final db = await instance; - final bounds = (await db.rawQuery( - 'SELECT COUNT(*) AS n, MIN(rec_ts) AS lo, MAX(rec_ts) AS hi ' - 'FROM decoded_onehz WHERE counter IN (?, ?)', - [fromCounter, toCounter], - )).first; - final n = (bounds['n'] as num?)?.toInt() ?? 0; - final want = fromCounter == toCounter ? 1 : 2; - if (n == want) { - return db.rawQuery( - 'SELECT rr.counter AS counter, rr.beat_index AS beat_index, ' - ' rr.rr_ts_ms AS rr_ts_ms, rr.rr_ms AS rr_ms ' - 'FROM decoded_rr rr ' - 'JOIN decoded_onehz d ON d.counter = rr.counter ' - 'WHERE d.rec_ts >= ? AND d.rec_ts <= ? ' - 'ORDER BY d.rec_ts ASC, rr.beat_index ASC', - [bounds['lo'], bounds['hi']], - ); - } - final lo = fromCounter <= toCounter ? fromCounter : toCounter; - final hi = fromCounter <= toCounter ? toCounter : fromCounter; - // BOUNDED. This branch is reached when an endpoint row is not in - // `decoded_onehz` — a prune, or an import's REPLACE + orphan-guard DELETE - // landing between the frame-page read and this call. The caller's counters - // are then just a span, and because the strap's counter resets on reboot a - // reboot-straddling page degenerates to `0 .. ~1200000`, i.e. effectively - // the whole table. Unbounded, that is a hundreds-of-MB platform-heap read - // on the same Java heap that OOMed the import path. A page is 2000 frames - // and a second rarely carries more than a handful of beats, so this cap is - // orders of magnitude above any legitimate page — reaching it means the - // degraded path is being used for a range it was never meant to serve. - const fallbackBeatCap = 200000; - final rows = await db.query( - 'decoded_rr', - columns: ['counter', 'beat_index', 'rr_ts_ms', 'rr_ms'], - where: 'counter >= ? AND counter <= ?', - whereArgs: [lo, hi], - orderBy: 'counter ASC, beat_index ASC', - limit: fallbackBeatCap, + final lo = fromRecTs <= toRecTs ? fromRecTs : toRecTs; + final hi = fromRecTs <= toRecTs ? toRecTs : fromRecTs; + return db.rawQuery( + 'SELECT rec_ts, beat_index, rr_ts_ms, rr_ms FROM decoded_rr ' + 'WHERE rec_ts >= ? AND rec_ts <= ? ' + 'ORDER BY rec_ts ASC, beat_index ASC', + [lo, hi], ); - // Never truncate silently — a short read here means missing beats, which - // shows up downstream as understated HRV rather than as an error. db.dart - // deliberately takes no telemetry dependency, so the fact is recorded as a - // plain counter the Diagnostics screen can surface. - // - // A static field is sound HERE specifically: every sqflite call needs the - // root isolate's platform channel, and this method's only caller - // (`DerivationEngine._prepare`) reads on the main isolate and ships each - // page to the compute worker with `worker.send`. Increments therefore land - // in the same isolate that reads them. Move this read into an isolate and - // the counter silently stops working — pass the count back over the port - // instead of reaching for a static. - if (rows.length >= fallbackBeatCap) { - decodedRrFallbackTruncations++; - } - return rows; } // ── VERSIONED DERIVED STORE I/O (day_result; main isolate only) ───────────── @@ -3589,20 +3581,20 @@ class LocalDb { where: 'rec_ts >= ? AND rec_ts < ?', whereArgs: [startSec, endSec], onPage: (page) async { - final counters = [ + final recTsList = [ for (final row in page) - if (row['counter'] != null) row['counter'], + if (row['rec_ts'] != null) row['rec_ts'], ]; - if (counters.isEmpty) return; - // CHUNKED `IN (…)`: even one page's counters can approach + if (recTsList.isEmpty) return; + // CHUNKED `IN (…)`: even one page's seconds can approach // SQLITE_MAX_VARIABLE_NUMBER, and a full day is 86,400 — two orders // of magnitude past it, so one giant statement could never bind. - // (This never surfaced only because the missing `version:` above - // aborted the export earlier.) - for (final chunk in _sqlVarChunks(counters)) { + // Keyed on rec_ts (decoded_rr's key), which pulls exactly this page's + // beats — a counter `IN` could over-match a reboot-reused counter. + for (final chunk in _sqlVarChunks(recTsList)) { final placeholders = List.filled(chunk.length, '?').join(','); final rr = await src.rawQuery( - 'SELECT * FROM decoded_rr WHERE counter IN ($placeholders)', + 'SELECT * FROM decoded_rr WHERE rec_ts IN ($placeholders)', chunk, ); if (rr.isEmpty) continue; @@ -3712,8 +3704,7 @@ class LocalDb { final (startSec, endSec) = _localDayWindow(dayId); deleted += await txn.delete( 'decoded_rr', - where: - 'counter IN (SELECT counter FROM decoded_onehz WHERE rec_ts >= ? AND rec_ts < ?)', + where: 'rec_ts >= ? AND rec_ts < ?', whereArgs: [startSec, endSec], ); deleted += await txn.delete( @@ -3970,18 +3961,15 @@ class LocalDb { )) { continue; // locally finalized — never overwritten by an import } - // ORPHAN GUARD ON THE IMPORT PATH. A plain replace-insert into - // decoded_onehz bypasses _queueDecodedOneHz entirely, so a - // foreign row colliding on UNIQUE(rec_ts) (different counter) or - // on the `counter` PRIMARY KEY (different second) evicted a local - // row and stranded its decoded_rr beats — the exact leak the - // ingest path is guarded against, wide open here. Queue the SAME - // guard, in the same batch/transaction, right before the row. - if (t == 'decoded_onehz') { - final counter = (row['counter'] as num?)?.toInt(); - final recTs = (row['rec_ts'] as num?)?.toInt(); - if (counter == null || recTs == null) continue; - ops += _queueOrphanGuard(batch, counter: counter, recTs: recTs); + // Both decoded tables are now keyed by rec_ts, so a plain + // replace-insert merges cleanly (foreign-wins on a rec_ts + // collision) — no orphan guard needed. A LEGACY export's + // decoded_rr carries no rec_ts column; derive it from rr_ts_ms + // (= rec_ts*1000) so the NOT NULL PK column is always populated. + if (t == 'decoded_rr' && + row['rec_ts'] == null && + row['rr_ts_ms'] != null) { + row['rec_ts'] = ((row['rr_ts_ms'] as num).toInt()) ~/ 1000; } batch.insert(t, row, conflictAlgorithm: ConflictAlgorithm.replace); copied++; @@ -5630,10 +5618,12 @@ class LocalDb { // caller's `if (deleted > 0) log(...)` never fired even on a real prune. int deleted = 0; await db.transaction((txn) async { + // decoded_rr shares the rec_ts key, so a plain rec_ts range delete covers + // every beat in the window — no counter subquery, no orphan sweep (there + // are no counter-orphans once parent and child are keyed the same way). deleted += await txn.delete( 'decoded_rr', - where: - 'counter IN (SELECT counter FROM decoded_onehz WHERE rec_ts < ?)', + where: 'rec_ts < ?', whereArgs: [cutoffSec], ); deleted += await txn.delete( @@ -5641,18 +5631,6 @@ class LocalDb { where: 'rec_ts < ?', whereArgs: [cutoffSec], ); - // ORPHAN SWEEP: pre-guard builds could leave decoded_rr beats whose - // owning counter lost a rec_ts collision (REPLACE evicted its - // decoded_onehz row) — the counter-joined delete above never selects - // those. Their rr_ts_ms is the colliding second, so once the window is - // pruned they're strictly before the cutoff; delete any beat in the - // pruned window whose counter no longer exists in decoded_onehz. - deleted += await txn.delete( - 'decoded_rr', - where: - 'rr_ts_ms < ? AND counter NOT IN (SELECT counter FROM decoded_onehz)', - whereArgs: [cutoffSec * 1000], - ); deleted += await txn.delete('samples', where: 'ts < ?', whereArgs: [cutoffSec]); deleted += diff --git a/test/db_integrity_test.dart b/test/db_integrity_test.dart index 69b360b..06a9cdf 100644 --- a/test/db_integrity_test.dart +++ b/test/db_integrity_test.dart @@ -1,12 +1,10 @@ // DB integrity regressions, run against the REAL LocalDb over sqflite_ffi: // -// 1. decoded_rr ORPHAN GUARD — a post-reboot rec_ts collision (two counters, -// one second) must not strand the evicted counter's RR beats under a -// counter with no decoded_onehz row (the counter-joined prune can never -// select those → permanent leak, and the loser's extra beat indexes would -// survive the winner's UNIQUE(rr_ts_ms, beat_index) REPLACE). -// 2. prune ORPHAN SWEEP — pre-existing orphans (written by pre-guard builds) -// are cleaned by pruneRawBeforeRecTs once their window is pruned. +// 1. decoded_rr REC_TS KEY — a post-reboot rec_ts collision (two counters, one +// second) keeps exactly the winner's beats; the write path DELETEs the +// second's beats before reinserting, so a shrinking beat count strands none. +// 2. prune BY REC_TS — pruneDecodedBeforeRecTs deletes decoded_rr by its rec_ts +// key (no counter subquery, no orphan sweep) and keeps recent beats. // 3. importFromDbFile FINALIZED protection — a foreign export never overwrites // a locally-finalized (day_id, algo_version) day_result row; non-finalized // rows keep the merge-REPLACE behavior. @@ -62,14 +60,13 @@ void main() { expect(health['ok'], isTrue, reason: '$health'); }); - test('rec_ts collision leaves no orphaned decoded_rr beats', () async { + test('rec_ts collision leaves exactly the winner\'s beats', () async { const ts = 1780000000; - // Pre-reboot record: high counter, THREE beats. + // Two records for the SAME second, different counters (a reboot straddle). + // THREE beats then TWO. REPLACE on the rec_ts key keeps the newest row; the + // write path DELETEs the second's beats before reinserting, so no stale + // high-index beat survives. await LocalDb.insertRecord(_raw(ts, 100), _sample(ts, 100, [800, 810, 820])); - // Post-reboot record for the SAME second: counter reset low, TWO beats. - // REPLACE on UNIQUE(rec_ts) evicts counter 100's decoded_onehz row; without - // the guard its beats (incl. beat_index 2, which the winner's two-beat - // REPLACE never touches) would be orphaned forever. await LocalDb.insertRecord(_raw(ts, 5), _sample(ts, 5, [900, 910])); final db = await LocalDb.instance; @@ -77,30 +74,30 @@ void main() { expect(onehz.length, 1); expect(onehz.first['counter'], 5); // newest-wins - // No RR beats survive under the evicted counter… - final loserBeats = - await db.query('decoded_rr', where: 'counter = ?', whereArgs: [100]); - expect(loserBeats, isEmpty); - // …and globally: zero orphans (every beat's counter owns a decoded row). + // Only the winner's two beats remain, keyed by rec_ts. + final beats = await db.query('decoded_rr', + where: 'rec_ts = ?', whereArgs: [ts], orderBy: 'beat_index ASC'); + expect([for (final b in beats) b['rr_ms']], [900, 910]); + + // Globally: zero orphans (every beat's rec_ts owns a decoded row). final orphans = await db.rawQuery( 'SELECT COUNT(*) c FROM decoded_rr ' - 'WHERE counter NOT IN (SELECT counter FROM decoded_onehz)', + 'WHERE rec_ts NOT IN (SELECT rec_ts FROM decoded_onehz)', ); expect(orphans.first['c'], 0); - // The RR read path (decodedRrByCounterRange, joined to frames by counter in - // derive_prepare.addDecodedPage) sees ONLY the winner's beats. - final rr = await LocalDb.decodedRrByCounterRange(fromCounter: 0, toCounter: 1 << 30); + // The RR read path sees ONLY the winner's beats. + final rr = await LocalDb.decodedRrByRecTsRange(fromRecTs: ts, toRecTs: ts); expect([for (final r in rr) r['rr_ms']], [900, 910]); - expect({for (final r in rr) r['counter']}, {5}); }); - test('prune sweeps pre-existing decoded_rr orphans', () async { + test('prune deletes decoded_rr by rec_ts, keeps recent beats', () async { const oldTs = 1700000000; // strictly before the cutoff below final db = await LocalDb.instance; - // Simulate a pre-guard leak: an RR beat whose counter has no decoded row. + // An old beat (its owning row absent — e.g. a leftover) is still deleted by + // the plain rec_ts-range prune; no counter subquery, no orphan sweep needed. await db.insert('decoded_rr', { - 'counter': 999999, + 'rec_ts': oldTs, 'beat_index': 0, 'rr_ts_ms': oldTs * 1000, 'rr_ms': 850, @@ -111,9 +108,9 @@ void main() { final deleted = await LocalDb.pruneDecodedBeforeRecTs(oldTs + 1000); - final orphan = await db.query('decoded_rr', where: 'counter = ?', whereArgs: [999999]); - expect(orphan, isEmpty, reason: 'orphan sweep must clean the leaked beat'); - final kept = await db.query('decoded_rr', where: 'counter = ?', whereArgs: [7]); + final old = await db.query('decoded_rr', where: 'rec_ts = ?', whereArgs: [oldTs]); + expect(old, isEmpty, reason: 'the pruned window\'s beats must be deleted'); + final kept = await db.query('decoded_rr', where: 'rec_ts = ?', whereArgs: [keepTs]); expect(kept.length, 1); // used to always come back 0 even when rows genuinely got pruned - none // of the txn.delete() counts were ever added up. diff --git a/test/db_migration_ladder_test.dart b/test/db_migration_ladder_test.dart index 4b471d5..bf48e9a 100644 --- a/test/db_migration_ladder_test.dart +++ b/test/db_migration_ladder_test.dart @@ -46,6 +46,29 @@ const _v6RawDdl = ''' ) '''; +/// The origin/main (pre-v33) COUNTER-keyed decoded store, exactly as a user who +/// installed at v19..31 has it — and by then raw_records is already DROPPED, so +/// the v33 rekey is the ONLY copy of their 1 Hz data (no raw-backfill safety +/// net). This is the highest-risk path the re-key touches. +const _counterKeyedDecodedDdl = [ + ''' + CREATE TABLE decoded_onehz ( + counter INTEGER PRIMARY KEY, rec_ts INTEGER NOT NULL, + hr INTEGER NOT NULL, ax REAL NOT NULL, ay REAL NOT NULL, az REAL NOT NULL, + spo2_red_raw INTEGER NOT NULL, spo2_ir_raw INTEGER NOT NULL, + skin_temp_raw INTEGER NOT NULL) +''', + 'CREATE UNIQUE INDEX idx_decoded_onehz_rec_ts_unique ON decoded_onehz(rec_ts)', + ''' + CREATE TABLE decoded_rr ( + counter INTEGER NOT NULL, beat_index INTEGER NOT NULL, + rr_ts_ms INTEGER NOT NULL, rr_ms INTEGER NOT NULL, + PRIMARY KEY (counter, beat_index)) +''', + 'CREATE UNIQUE INDEX idx_decoded_rr_ts_beat_unique ' + 'ON decoded_rr(rr_ts_ms, beat_index)', +]; + /// The v5-era derived tables, so step 9's derived_day → day_result copy is real. const _v5DerivedDdl = [ ''' @@ -417,4 +440,85 @@ void main() { expect(health['ok'], isTrue, reason: '$health'); }, ); + + test( + 'v33 re-key converts a v31 COUNTER-keyed decoded store to rec_ts LOSSLESSLY ' + '— raw_records is already dropped, so the rekey is the only copy', + () async { + const name = 'migrate_from_v31_counterkeyed_test.db'; + created.add(name); + await _seedOldDb( + name, + 31, + [..._counterKeyedDecodedDdl, ..._v5DerivedDdl], + seedRows: (db) async { + // Three distinct seconds, distinct counters (valid old-schema data). + for (final r in const [ + [100, 1785000000, 60], + [101, 1785000001, 61], + [102, 1785000002, 62], + ]) { + await db.insert('decoded_onehz', { + 'counter': r[0], 'rec_ts': r[1], 'hr': r[2], + 'ax': 0.0, 'ay': 0.0, 'az': 0.0, + 'spo2_red_raw': 0, 'spo2_ir_raw': 0, 'skin_temp_raw': 0, + }); + } + // Beats under counter 100 (two) and 102 (one). + await db.insert('decoded_rr', { + 'counter': 100, 'beat_index': 0, + 'rr_ts_ms': 1785000000 * 1000, 'rr_ms': 800, + }); + await db.insert('decoded_rr', { + 'counter': 100, 'beat_index': 1, + 'rr_ts_ms': 1785000000 * 1000, 'rr_ms': 810, + }); + await db.insert('decoded_rr', { + 'counter': 102, 'beat_index': 0, + 'rr_ts_ms': 1785000002 * 1000, 'rr_ms': 900, + }); + }, + ); + + expect(await _openThroughLocalDb(name), LocalDb.schemaVersion); + final db = await LocalDb.instance; + + // Every second survives; counter is preserved as the forensic column. + final oh = await db.query('decoded_onehz', orderBy: 'rec_ts ASC'); + expect([for (final r in oh) r['rec_ts']], + [1785000000, 1785000001, 1785000002]); + expect([for (final r in oh) r['counter']], [100, 101, 102]); + // PK is now rec_ts, not counter. + final ohInfo = await db.rawQuery('PRAGMA table_info(decoded_onehz)'); + expect(ohInfo.firstWhere((c) => c['name'] == 'rec_ts')['pk'], 1); + expect(ohInfo.firstWhere((c) => c['name'] == 'counter')['pk'], 0); + + // Beats re-home onto their real second; decoded_rr loses its counter col. + final rr = + await db.query('decoded_rr', orderBy: 'rec_ts ASC, beat_index ASC'); + expect([for (final r in rr) r['rec_ts']], + [1785000000, 1785000000, 1785000002]); + expect([for (final r in rr) r['rr_ms']], [800, 810, 900]); + final rrInfo = await db.rawQuery('PRAGMA table_info(decoded_rr)'); + expect(rrInfo.any((c) => c['name'] == 'counter'), isFalse); + + // No stranded beats, no cross-stamped timestamps, no leaked temp tables. + expect( + (await db.rawQuery( + 'SELECT COUNT(*) c FROM decoded_rr WHERE rr_ts_ms != rec_ts * 1000', + )).first['c'], + 0, + ); + expect( + await db.rawQuery( + "SELECT name FROM sqlite_master WHERE name LIKE '%\\_v33' ESCAPE '\\' " + "OR name LIKE '%\\_new' ESCAPE '\\'", + ), + isEmpty, + ); + + final health = await LocalDb.schemaHealth(); + expect(health['ok'], isTrue, reason: '$health'); + }, + ); } diff --git a/test/db_p0_fixes_test.dart b/test/db_p0_fixes_test.dart index 1fa07cd..adc79ae 100644 --- a/test/db_p0_fixes_test.dart +++ b/test/db_p0_fixes_test.dart @@ -88,49 +88,60 @@ void main() { if (await tmp.exists()) await tmp.delete(recursive: true); }); - // ── fix 3 ──────────────────────────────────────────────────────────────── + // ── fix 3: THE re-key regression ───────────────────────────────────────── test( - 'a REUSED counter (reboot reset) leaves no stale-timestamped decoded_rr ' - 'beats behind — the counter-PK eviction is guarded too', + 'a REUSED counter (reboot reset) KEEPS both seconds — the counter-PK ' + 'eviction that silently, unrecoverably deleted a 1 Hz row is gone', () async { - const older = 1785000000; - const newer = 1785000600; // a DIFFERENT second, same counter + const older = 1785000000; // pre-reboot, counter 777 + const newer = 1785000600; // post-reboot, SAME counter 777 const counter = 777; await LocalDb.insertRecord( _raw(older, counter), _sample(older, counter, [800, 810, 820]), // THREE beats ); - // Post-reboot the counter is handed out again, now for a later second. - // The INSERT-OR-REPLACE evicts the older second's decoded_onehz row via - // the `counter` PRIMARY KEY; only beat_index 0 and 1 are overwritten, so - // beat_index 2 used to SURVIVE still stamped with `older` — invisible to - // both prune paths, and it polluted every later RR read of that counter. + // Pre-fix: `counter` was decoded_onehz's PRIMARY KEY, so this REPLACE + // evicted the `older` row entirely — and raw_records is dropped, so that + // 1 Hz second was gone for good. Under the rec_ts key both seconds live. await LocalDb.insertRecord( _raw(newer, counter), _sample(newer, counter, [900, 910]), // TWO beats ); final db = await LocalDb.instance; - final beats = await db.query( - 'decoded_rr', - where: 'counter = ?', - whereArgs: [counter], - orderBy: 'beat_index ASC', + final rows = await db.query( + 'decoded_onehz', + where: 'rec_ts IN (?, ?)', + whereArgs: [older, newer], + orderBy: 'rec_ts ASC', ); - expect(beats, hasLength(2), reason: 'the third beat must not survive'); - expect( - beats.every((b) => b['rr_ts_ms'] == newer * 1000), - isTrue, - reason: 'no beat may carry the evicted second\'s timestamp: $beats', + expect(rows, hasLength(2), + reason: 'the pre-reboot second must NOT be evicted'); + expect([for (final r in rows) r['counter']], [counter, counter]); + + // Each second keeps its own beats, keyed by its rec_ts. + final oldBeats = await db.query('decoded_rr', + where: 'rec_ts = ?', whereArgs: [older], orderBy: 'beat_index ASC'); + expect([for (final b in oldBeats) b['rr_ms']], [800, 810, 820]); + final newBeats = await db.query('decoded_rr', + where: 'rec_ts = ?', whereArgs: [newer], orderBy: 'beat_index ASC'); + expect([for (final b in newBeats) b['rr_ms']], [900, 910]); + + // Re-offloading `older` with FEWER beats must not strand the third — + // the write path DELETEs the second's beats before reinserting. + await LocalDb.insertRecord( + _raw(older, counter), + _sample(older, counter, [850]), // ONE beat now ); - expect([for (final b in beats) b['rr_ms']], [900, 910]); + final reBeats = await db.query('decoded_rr', + where: 'rec_ts = ?', whereArgs: [older], orderBy: 'beat_index ASC'); + expect([for (final b in reBeats) b['rr_ms']], [850], + reason: 'stale high-index beats must not survive a shrink'); - // And globally: no orphans, no cross-second contamination. + // No beat carries the wrong second's timestamp, anywhere. final stale = await db.rawQuery( - 'SELECT COUNT(*) c FROM decoded_rr rr ' - 'JOIN decoded_onehz d ON d.counter = rr.counter ' - 'WHERE rr.rr_ts_ms != d.rec_ts * 1000', + 'SELECT COUNT(*) c FROM decoded_rr WHERE rr_ts_ms != rec_ts * 1000', ); expect(stale.first['c'], 0); }, @@ -138,8 +149,8 @@ void main() { // ── fix 4 ──────────────────────────────────────────────────────────────── test( - 'decodedRrByCounterRange returns a page spanning a counter RESET — the ' - 'endpoints are page bounds, not a monotonic counter span', + 'decodedRrByRecTsRange returns a page spanning a counter RESET — the ' + 'window is by rec_ts, immune to the reboot counter reset', () async { const t0 = 1785100000; // Pre-reboot: high counter. Post-reboot: the counter restarts near zero, @@ -161,16 +172,19 @@ void main() { toRecTs: t0 + 1, ); expect(page, hasLength(2)); - final first = (page.first['counter'] as num).toInt(); - final last = (page.last['counter'] as num).toInt(); - expect(first, 1200000); - expect(last, 5, reason: 'the page really does end on a LOWER counter'); - - final rr = await LocalDb.decodedRrByCounterRange( - fromCounter: first, - toCounter: last, + final first = (page.first['rec_ts'] as num).toInt(); + final last = (page.last['rec_ts'] as num).toInt(); + expect(first, t0); + expect(last, t0 + 1); + // Sanity: the page really does end on a LOWER counter (reboot straddle). + expect((page.first['counter'] as num).toInt(), 1200000); + expect((page.last['counter'] as num).toInt(), 5); + + final rr = await LocalDb.decodedRrByRecTsRange( + fromRecTs: first, + toRecTs: last, ); - // `counter >= 1200000 AND counter <= 5` used to match nothing at all — + // A counter span (`>= 1200000 AND <= 5`) used to match nothing at all — // the window silently produced no RR beats, so no RMSSD/HRV, no error. expect(rr, hasLength(4)); expect([for (final r in rr) r['rr_ms']], [800, 805, 900, 905]); @@ -318,27 +332,28 @@ void main() { // ── fix 8 ──────────────────────────────────────────────────────────────── test( - 'importFromDbFile routes decoded_onehz through the orphan guard', + 'importFromDbFile merges a LEGACY (counter-keyed, no rec_ts) decoded export ' + 'cleanly under the rec_ts key — foreign-wins, no stranded beats', () async { final db = await LocalDb.instance; await db.delete('decoded_onehz'); await db.delete('decoded_rr'); - const collideTs = 1786000000; // rec_ts collision, different counter - const reuseCounter = 8003; // counter collision, different rec_ts - const localReuseTs = 1786000500; - const foreignReuseTs = 1786009999; + const collideTs = 1786000000; // rec_ts present locally AND in the foreign + const t2 = 1786000500; // local-only second + const t3 = 1786009999; // foreign-only second await LocalDb.insertRecord( _raw(collideTs, 8002), _sample(collideTs, 8002, [700, 710, 720]), ); await LocalDb.insertRecord( - _raw(localReuseTs, reuseCounter), - _sample(localReuseTs, reuseCounter, [600, 610, 620]), + _raw(t2, 8003), + _sample(t2, 8003, [600, 610]), ); - // A foreign export that collides both ways. + // A foreign export in the OLD schema: decoded_rr has a `counter` column + // and NO rec_ts (the import must derive rec_ts from rr_ts_ms). final dir = await databaseFactory.getDatabasesPath(); final srcPath = p.join(dir, 'p0_foreign_export.db'); await databaseFactory.deleteDatabase(srcPath); @@ -378,38 +393,39 @@ void main() { } } - await foreign(8001, collideTs, [500]); // same second, other counter - await foreign(reuseCounter, foreignReuseTs, [400]); // same counter, other second + // Same second as local, fully overwriting its 3 beats; plus a new second. + await foreign(8001, collideTs, [500, 505, 510]); + await foreign(9999, t3, [400]); await src.close(); await LocalDb.importFromDbFile(srcPath); - // (a) UNIQUE(rec_ts) eviction: the local counter's beats went with it. - expect( - await db.query('decoded_rr', where: 'counter = ?', whereArgs: [8002]), - isEmpty, - reason: 'the evicted counter\'s beats must not be stranded', - ); - // (b) counter-PK eviction: no beat under the reused counter still carries - // the local second's timestamp. - final reused = await db.query( - 'decoded_rr', - where: 'counter = ?', - whereArgs: [reuseCounter], - ); - expect(reused, hasLength(1)); - expect(reused.first['rr_ts_ms'], foreignReuseTs * 1000); + // Foreign wins the rec_ts collision; the other two seconds are untouched. + final onehz = await db.query('decoded_onehz', orderBy: 'rec_ts ASC'); + expect([for (final r in onehz) r['rec_ts']], [collideTs, t2, t3]); + final collided = + onehz.firstWhere((r) => r['rec_ts'] == collideTs); + expect(collided['counter'], 8001, reason: 'foreign row won'); + expect(collided['hr'], 61); + + // The collided second's beats are the foreign set (no stale local beat). + final b1 = await db.query('decoded_rr', + where: 'rec_ts = ?', whereArgs: [collideTs], orderBy: 'beat_index ASC'); + expect([for (final b in b1) b['rr_ms']], [500, 505, 510]); + // The foreign-only second imported with rec_ts derived from rr_ts_ms. + final b3 = await db.query('decoded_rr', + where: 'rec_ts = ?', whereArgs: [t3]); + expect([for (final b in b3) b['rr_ms']], [400]); + expect(b3.first['rr_ts_ms'], t3 * 1000); // Nothing orphaned, nothing cross-stamped, anywhere. final orphans = await db.rawQuery( 'SELECT COUNT(*) c FROM decoded_rr ' - 'WHERE counter NOT IN (SELECT counter FROM decoded_onehz)', + 'WHERE rec_ts NOT IN (SELECT rec_ts FROM decoded_onehz)', ); expect(orphans.first['c'], 0); final stale = await db.rawQuery( - 'SELECT COUNT(*) c FROM decoded_rr rr ' - 'JOIN decoded_onehz d ON d.counter = rr.counter ' - 'WHERE rr.rr_ts_ms != d.rec_ts * 1000', + 'SELECT COUNT(*) c FROM decoded_rr WHERE rr_ts_ms != rec_ts * 1000', ); expect(stale.first['c'], 0); }, diff --git a/test/db_paged_import_export_test.dart b/test/db_paged_import_export_test.dart index 4575824..ae968ca 100644 --- a/test/db_paged_import_export_test.dart +++ b/test/db_paged_import_export_test.dart @@ -169,10 +169,10 @@ void main() { )).first['c']; expect(distinct, rows); - // Nothing stranded: the orphan guard still runs per row under paging. + // Nothing stranded: every beat's rec_ts owns a decoded_onehz row. final orphans = (await db.rawQuery( 'SELECT COUNT(*) c FROM decoded_rr ' - 'WHERE counter NOT IN (SELECT counter FROM decoded_onehz)', + 'WHERE rec_ts NOT IN (SELECT rec_ts FROM decoded_onehz)', )).first['c']; expect(orphans, 0); }); @@ -277,9 +277,4 @@ void main() { } }); }); - - test('the degraded RR fallback truncation counter starts clean', () async { - // A non-zero value means some window computed HRV from truncated beats. - expect(LocalDb.decodedRrFallbackTruncations, 0); - }); } diff --git a/test/db_storage_hygiene_test.dart b/test/db_storage_hygiene_test.dart index 56f3465..bdc3f62 100644 --- a/test/db_storage_hygiene_test.dart +++ b/test/db_storage_hygiene_test.dart @@ -1,7 +1,6 @@ // Storage hygiene: -// 1. decoded_rr's rr_ts_ms lookups are still index-served after dropping -// idx_decoded_rr_ts, which was a strict prefix of the (rr_ts_ms, -// beat_index) unique index and only added write cost. +// 1. decoded_rr is keyed (rec_ts, beat_index) and carries no redundant +// secondary index; rec_ts-range reads are served by the PK auto-index. // 2. Superseded generations of the recomputable per-day intermediates are // pruned. They are keyed (day_id, algo_version), so every kAlgoVersion // bump wrote a whole new generation beside the old one and nothing @@ -28,85 +27,40 @@ void main() { await databaseFactory.deleteDatabase(p.join(dir, LocalDb.dbName)); }); - test('the redundant single-column rr index is gone', () async { + test('decoded_rr carries no redundant secondary index', () async { 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_ts'))); - expect(idx, contains('idx_decoded_rr_ts_beat_unique')); - }); - - 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. - // - // 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)', - ); + final idx = await _rrIndexes(db); + // The (rec_ts, beat_index) PRIMARY KEY auto-indexes; nothing else should be + // maintained on this hot insert path. expect( - await _rrIndexes(db), - contains('idx_decoded_rr_counter'), - reason: 'the fixture must actually plant the index', + idx.where((n) => !n.startsWith('sqlite_autoindex')), + isEmpty, + reason: 'unexpected secondary index on decoded_rr: $idx', ); - - 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 { - // 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. + test('rec_ts-range reads on decoded_rr are served by the PK auto-index', () async { + // decoded_rr shares the rec_ts key with decoded_onehz, so the derive read + // path (decodedRrByRecTsRange) is a PK range scan — never a full-table read. 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( - 'EXPLAIN QUERY PLAN SELECT * FROM decoded_rr WHERE rr_ts_ms < 1000 ' - 'ORDER BY rr_ts_ms ASC, beat_index ASC', + final detail = (await db.rawQuery( + 'EXPLAIN QUERY PLAN SELECT * FROM decoded_rr WHERE rec_ts BETWEEN 1 AND 9 ' + 'ORDER BY rec_ts ASC, beat_index ASC', + )).map((r) => r['detail'].toString()).join(' | '); + expect( + detail.toUpperCase(), + contains('USING'), + reason: 'planner fell back to a full scan: $detail', ); - 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', + contains('sqlite_autoindex_decoded_rr_1'), + reason: 'expected the primary key auto-index: $detail', ); expect( detail.toUpperCase(), isNot(contains('USE TEMP B-TREE')), - reason: 'ordering should come from the index: $detail', + reason: 'ordering should come from the PK: $detail', ); }); diff --git a/test/local_persistence_test.dart b/test/local_persistence_test.dart index 380bcbf..835b86d 100644 --- a/test/local_persistence_test.dart +++ b/test/local_persistence_test.dart @@ -243,9 +243,9 @@ void main() { expect(frames.first['hr'], 61); expect(frames.first['spo2_red_raw'], 1234); - final rr = await LocalDb.decodedRrByCounterRange( - fromCounter: 424242, - toCounter: 424242, + final rr = await LocalDb.decodedRrByRecTsRange( + fromRecTs: startSec, + toRecTs: startSec, ); expect(rr, hasLength(2)); expect(rr.first['rr_ms'], 980);