diff --git a/AGENTS.md b/AGENTS.md index 45641fe..517f6b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,8 +55,9 @@ See `docs/qring-ble-adoption.md` §5a for the full history and the decompiled so **Read this before touching `EventPersistenceSubscriber`'s `DeviceStateChanged` branch.** **No ring re-supplies more history than its own buffer holds, so the app's copy is the only durable -one.** A connect may retire *demo* rows and nothing else. This is not a style preference — it was a -data-loss bug twice, in two different shapes (issue #43, and the sync-pass variant before it). +one.** A connect may delete **nothing at all** — not stored history, and (since the iOS-parity fix) +not demo rows either. This is not a style preference — it was a data-loss bug three times, in three +different shapes (issue #43, the sync-pass variant before it, and the demo-row purge below). The original design deleted all sleep on connect and re-pulled it, carving YCBT out via `preservesSleepOnConnect` because YCBT re-asserts CONNECTED mid-history. That premise was false for @@ -100,13 +101,47 @@ transition: status packets. `runStartup` re-sends them, and `runStartup` is also the ~30-minute background sync — so they recur for the whole life of a connection. -`isConnectTransition(event.deviceType)` is that gate, and `connectPurge` is what it feeds. Be precise -about its scope, because it is narrower than it looks: it decides only what a CONNECTED event may -*delete*. The row write below it — `stateRaw = "CONNECTED"`, `lastConnectedAt`, `lastSyncAt` — is +`isConnectTransition(event.deviceType)` is that gate. It used to feed `connectPurge`; now that a +connect deletes nothing, `connectPurge` ignores its argument and returns `ConnectPurge.NOTHING` +unconditionally, so `isConnectTransition` has **no production caller left** — only +`EventPersistenceIdentityTest`. It is kept because the distinction is real and gets re-derived +wrongly each time someone needs it, and any future connect-time action wants exactly it. Be precise +about the gate's scope, because it is narrower than it looks: it decides only what a CONNECTED +event may *delete*. The row write below it — `stateRaw = "CONNECTED"`, `lastConnectedAt`, `lastSyncAt` — is **outside** the gate and still runs for every decoder `Status`, so a jring `0x0C` reply does still restamp the device row as freshly connected on each sync pass. That is harmless today; it is not something the gate prevents, so don't cite it as if it were. +**Demo rows are not purged on connect either — match iOS, don't "clean up" for it.** The demo +purge was the last surviving fragment of the original "connect = clear everything and rebuild" +design, and it was gated only on "this is the BLE client's own connect". A paired ring +re-establishes its link constantly — measured at roughly one reconnect per five minutes on a COLMI +R10 — so every one of those re-ran `clearDemo()` and seeded demo data could never survive alongside +a paired ring. Captured live at 09:00:54 with the phone untouched: 772 measurements / 84 activity +days / 36 sleep sessions to zero. **iOS has no connect-time demo purge anywhere** — its only +deletion of seeded rows is user-initiated inside `SeedData`, and it handles the demo/real mix by +*detecting* it (`isDemo`, `source == "mock"`, `DataFreshness.demo`) and adapting the UI. Demo data +coexisting with a real ring is an expected state. Demo rows are retired only from +Settings → Privacy & Data → Clear Demo Data. + +`ConnectPurge` now has exactly **one** member, `NOTHING`, and the single-branch `when`s that switch +on it in `EventPersistenceSubscriber` and `EventPersistenceIdentityTest` read as dead code but are +not: they are what makes *widening* what a connect deletes a compile error instead of a one-line +edit. Don't "simplify" them away. Know the limit, though — it only catches an author who routes the +new deletion through the enum; a bare `clearDemo()` dropped into the CONNECTED arm still compiles. +The two tests named below are the actual enforcement. + +**Readers must choose between demo and real rows, because nothing separates them any more.** The +purge was also, accidentally, the thing keeping seeded rows out of every unfiltered query. With it +gone the two coexist indefinitely, so `DemoDataPolicy` (`data/DemoDataPolicy.kt`) states the rule: +**real wins** — a reader surfaces demo rows only while the corresponding real series is empty, and +switches to the `*Real` DAO queries the moment the ring has synced anything. Derived values that +are *persisted or exported* — `hrRestingBaseline`, `estimatedActiveCalories`, Health Connect +records — read `*Real` unconditionally, since a demo-derived number outlives the demo data behind +it. When you add a query over `measurements`, `activity_daily`, or `sleep_sessions`, decide which +of those two it is; "it's just a read" was how a seeded 56 bpm night became a real user's auto HR +zone floor. + Two related things worth knowing before changing this area: - **Nothing bulk-deletes sleep any more, anywhere in the app.** That connect path was the only diff --git a/app/src/main/java/com/pulseloop/coach/context/CoachContextBuilder.kt b/app/src/main/java/com/pulseloop/coach/context/CoachContextBuilder.kt index d6b185e..51959a6 100644 --- a/app/src/main/java/com/pulseloop/coach/context/CoachContextBuilder.kt +++ b/app/src/main/java/com/pulseloop/coach/context/CoachContextBuilder.kt @@ -32,21 +32,29 @@ object CoachContextBuilder { val latestHr = db.measurementDao().latest(MeasurementKind.HEART_RATE.name, now) val latestSpo2 = db.measurementDao().latest(MeasurementKind.SPO2.name, now) - // 7-day trends + // 7-day trends. Real wins over seeded rows once the ring has synced (`DemoDataPolicy`): + // a connect no longer purges demo data, so without this the coach LLM is handed a + // demo/real blend — and, via `isDemo` below, told it is live. + val activityHasReal = db.activityDailyDao().hasReal() val sevenDaysAgo = todayStart - 7 * 24 * 3600_000L - val activityWeek = db.activityDailyDao().recent(7).filter { it.date >= sevenDaysAgo } + val activityWeek = (if (activityHasReal) db.activityDailyDao().recentReal(7) + else db.activityDailyDao().recent(7)).filter { it.date >= sevenDaysAgo } val stepsWeek = activityWeek.map { it.steps.toDouble() } val daysAvailable = activityWeek.count { it.steps > 0 } // Resting HR: average of HR values in the last 24h while at rest (approximated as lowest 25th percentile) - val hr24h = db.measurementDao().range(MeasurementKind.HEART_RATE.name, now - 24 * 3600_000L, now) + val hr24h = if (db.measurementDao().hasReal(MeasurementKind.HEART_RATE.name)) + db.measurementDao().rangeReal(MeasurementKind.HEART_RATE.name, now - 24 * 3600_000L, now) + else db.measurementDao().range(MeasurementKind.HEART_RATE.name, now - 24 * 3600_000L, now) val restingHr = if (hr24h.isNotEmpty()) { val sorted = hr24h.map { it.value }.sorted() sorted[(sorted.size * 0.25).toInt().coerceAtMost(sorted.size - 1)] } else null // Sleep - val latestSleep = db.sleepSessionDao().recent(1).firstOrNull() + val sleepHasReal = db.sleepSessionDao().hasReal() + val latestSleep = (if (sleepHasReal) db.sleepSessionDao().recentReal(1) + else db.sleepSessionDao().recent(1)).firstOrNull() val completeness = profileCompleteness(profile) val warnings = DataQualityAnalyzer.warnings( @@ -55,7 +63,12 @@ object CoachContextBuilder { daysAvailable = daysAvailable, hasSleep = latestSleep != null, lastSyncAt = device?.lastSyncAt, - isDemo = false, + // Hardcoded false meant the "This is demo/sample data, not live readings from the + // ring." warning could never be emitted. Everything above falls back to seeded + // rows exactly when the ring has synced nothing — which is what demo mode IS. + isDemo = !activityHasReal && !sleepHasReal && + !db.measurementDao().hasReal(MeasurementKind.HEART_RATE.name) && + (activityWeek.isNotEmpty() || latestSleep != null || hr24h.isNotEmpty()), ), now = now, ) diff --git a/app/src/main/java/com/pulseloop/data/DemoDataPolicy.kt b/app/src/main/java/com/pulseloop/data/DemoDataPolicy.kt new file mode 100644 index 0000000..f736cc9 --- /dev/null +++ b/app/src/main/java/com/pulseloop/data/DemoDataPolicy.kt @@ -0,0 +1,32 @@ +package com.pulseloop.data + +/** + * How readers treat seeded demo rows now that a connect deletes nothing (PR #52). + * + * Before #52, `EventPersistenceSubscriber` purged demo rows on every CONNECTED transition, so + * "demo rows and a paired ring's real rows in the same table" was a state that only existed for + * the few seconds between a reseed and the next reconnect. Removing that purge — correctly, it + * was wiping seeded data every ~5 minutes under a paired R10 — makes the mixed state permanent, + * and "Reseed Demo Data" ships in Settings → Privacy & Data, so any user can reach it. + * + * The rule, mirroring iOS (which detects the mix rather than cleaning it up): **real data wins.** + * A reader surfaces demo rows only while the corresponding real series is empty. As soon as a ring + * has synced anything for that series, demo rows are excluded rather than blended in — blending + * is what produced 13-hour "nights" (demo night + real night collapsed onto one date), demo-fed + * resting-HR baselines, and demo-inflated calorie estimates written onto real rows. + * + * Derived values that are persisted or exported (`hrRestingBaseline`, `estimatedActiveCalories`, + * Health Connect records) go further: they read the `*Real` queries unconditionally, since a + * demo-derived number outlives the demo data that produced it. + */ +object DemoDataPolicy { + /** Every `source` / `sourceRaw` value that marks a row as seeded rather than synced. */ + val DEMO_SOURCES = setOf("demo", "mock") + + /** + * The seeder writes `"demo"`; `"mock"` is the iOS spelling and is accepted alongside it. + * Checking only one of the two is how `MetricsService.isDemo` silently reported seeded data + * as live. + */ + fun isDemo(source: String?): Boolean = source != null && source in DEMO_SOURCES +} diff --git a/app/src/main/java/com/pulseloop/data/dao/Daos.kt b/app/src/main/java/com/pulseloop/data/dao/Daos.kt index e2645f0..8303f16 100644 --- a/app/src/main/java/com/pulseloop/data/dao/Daos.kt +++ b/app/src/main/java/com/pulseloop/data/dao/Daos.kt @@ -64,6 +64,12 @@ interface MeasurementDao { @Query("SELECT EXISTS(SELECT 1 FROM measurements WHERE kindRaw = :kind AND sourceRaw = 'demo')") suspend fun hasDemo(kind: String): Boolean + /** Whether the ring has ever synced this kind. The demo/real chooser (`DemoDataPolicy`): + * once this is true a reader takes the `*Real` query and stops charting seeded rows, instead + * of interleaving the two into one series. */ + @Query("SELECT EXISTS(SELECT 1 FROM measurements WHERE kindRaw = :kind AND sourceRaw NOT IN ('demo','mock'))") + suspend fun hasReal(kind: String): Boolean + @Insert suspend fun insert(measurement: MeasurementEntity) @@ -129,6 +135,14 @@ interface ActivityDailyDao { @Query("SELECT * FROM activity_daily ORDER BY date DESC LIMIT :limit") fun recentFlow(limit: Int = 7): Flow> + /** Synced days only — the trend leg of the demo/real chooser (`DemoDataPolicy`). */ + @Query("SELECT * FROM activity_daily WHERE source NOT IN ('demo','mock') ORDER BY date DESC LIMIT :limit") + suspend fun recentReal(limit: Int = 7): List + + /** Whether the ring has ever synced a day. */ + @Query("SELECT EXISTS(SELECT 1 FROM activity_daily WHERE source NOT IN ('demo','mock'))") + suspend fun hasReal(): Boolean + @Upsert suspend fun upsert(entry: ActivityDailyEntity) @@ -290,6 +304,19 @@ interface SleepSessionDao { @Query("SELECT * FROM sleep_sessions WHERE date BETWEEN :start AND :end ORDER BY date ASC") suspend fun inRange(start: Long, end: Long): List + /** Synced sessions in a window — the aggregate leg of the demo/real chooser + * (`DemoDataPolicy`). Blending demo and ring nights across a week skews every average, and + * two sessions on the SAME date collapse into one impossible ~13h night. */ + @Query("SELECT * FROM sleep_sessions WHERE date BETWEEN :start AND :end AND sourceRaw NOT IN ('demo','mock') ORDER BY date ASC") + suspend fun inRangeReal(start: Long, end: Long): List + + @Query("SELECT * FROM sleep_sessions WHERE sourceRaw NOT IN ('demo','mock') ORDER BY date DESC LIMIT :limit") + suspend fun recentReal(limit: Int = 7): List + + /** Whether the ring has ever synced a night. */ + @Query("SELECT EXISTS(SELECT 1 FROM sleep_sessions WHERE sourceRaw NOT IN ('demo','mock'))") + suspend fun hasReal(): Boolean + /** * Health Connect export selection (Phase 2): sessions committed after [watermark], by * `updatedAt` — sleep is a mutable group (plan §3): a re-synced night must re-upsert the SAME diff --git a/app/src/main/java/com/pulseloop/service/DailyCalorieEstimator.kt b/app/src/main/java/com/pulseloop/service/DailyCalorieEstimator.kt index c26946a..f36776d 100644 --- a/app/src/main/java/com/pulseloop/service/DailyCalorieEstimator.kt +++ b/app/src/main/java/com/pulseloop/service/DailyCalorieEstimator.kt @@ -129,7 +129,11 @@ object DailyCalorieEstimator { val row = db.activityDailyDao().byDay(dayStart) ?: return val dayEnd = dayStart + DAY_MS - val hrSamples = db.measurementDao().range(MeasurementKind.HEART_RATE.name, dayStart, dayEnd) + // `rangeReal`: the estimate is written back onto the day's REAL `activity_daily` row + // below, where it drives the calorie goal ring and is eligible for Health Connect export + // (that export filters on the row's own source, which is not demo). Seeded HR for today + // would inflate a number that outlives the demo data behind it (`DemoDataPolicy`). + val hrSamples = db.measurementDao().rangeReal(MeasurementKind.HEART_RATE.name, dayStart, dayEnd) val buckets = db.activityBucketDao().byDay(dayStart) val workouts = db.activitySessionDao().recent(WORKOUT_SCAN_LIMIT).filter { it.statusRaw == "finished" && it.endedAt != null && diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index 82ac204..1ba5e92 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -83,31 +83,32 @@ class EventPersistenceSubscriber( val device = existing ?: DeviceEntity() val state = when (event.state) { RingConnectionState.CONNECTED -> { - // Connecting retires demo rows so a real ring's data replaces the seeded - // preview. It destroys nothing else. Gated to the client's own connect (see - // [isConnectTransition]) because decoders re-assert CONNECTED mid-session - // from ordinary device-info replies — jring `0x0C`, LuckRing dev-info, YCBT - // status packets — which `runStartup` re-sends on every sync pass. + // Connecting deletes NOTHING — not stored history, not demo rows. Demo + // data is retired only from Settings → Privacy & Data, matching iOS, which + // has no connect-time purge at all and instead *detects* seeded rows + // (`isDemo`, `source == "mock"`, `DataFreshness.demo`) and adapts the UI. + // Demo data coexisting with a paired ring is an expected state, not a mess + // to clean up. // - // This used to clear *all* sleep for every family except YCBT and rebuild it - // from the ring. No ring re-supplies more than its own buffer, and the two - // smallest re-supply a single day — CRP sends `queryHistorySleep(daysAgo=0)`, - // jring `makeHistoryQueryCommand()` with its 1-day default (JringDriver.kt:105, - // NOT `syncWindowDays`, which only sizes the progress bar) — so every connect - // destroyed each night older - // than that, and a new night replaced the last one instead of joining it - // (issue #43, zaggash's R11). The rebuild was never load-bearing: - // [upsertSleepSessionAtomic] reconciles one waking day at a time, - // idempotently, and re-points legacy mis-keyed blocks itself — which was the - // blanket clear's stated reason for existing. + // The `when` below is the guard, and it is exhaustive on purpose — see + // [ConnectPurge]. It reads as dead code and is not: it is what makes + // re-introducing a connect-time delete a compile error rather than a + // one-line edit. + // + // History, because this keeps getting re-added: connect used to clear *all* + // sleep for every family except YCBT and rebuild it from the ring. No ring + // re-supplies more than its own buffer, and the two smallest re-supply a + // single day — CRP sends `queryHistorySleep(daysAgo=0)`, jring + // `makeHistoryQueryCommand()` with its 1-day default (JringDriver.kt:105, + // NOT `syncWindowDays`, which only sizes the progress bar) — so every + // connect destroyed each night older than that, and a new night replaced + // the last one instead of joining it (issue #43, zaggash's R11). The + // rebuild was never load-bearing: [upsertSleepSessionAtomic] reconciles one + // waking day at a time, idempotently, and re-points legacy mis-keyed blocks + // itself — which was the blanket clear's stated reason for existing. The + // demo purge was the last surviving fragment of that same model. when (connectPurge(event.deviceType)) { ConnectPurge.NOTHING -> {} - ConnectPurge.DEMO_ROWS -> { - db.measurementDao().clearDemo() - db.activityDailyDao().clearDemo() - db.sleepStageBlockDao().clearDemo() - db.sleepSessionDao().clearDemo() - } } "CONNECTED" } @@ -660,7 +661,12 @@ internal fun historyMeasurementId(kind: MeasurementKind, timestamp: Long): Strin /** * True when a `DeviceStateChanged(CONNECTED, …)` is a real connection transition rather than a - * mid-session re-assertion, and may therefore retire the demo rows in [EventPersistenceSubscriber]. + * mid-session re-assertion. + * + * **No production caller.** It gated the connect-time demo purge; with that purge gone a connect + * does nothing worth gating, so this is currently exercised only by `EventPersistenceIdentityTest`. + * It is kept because the distinction it draws is real, non-obvious, and re-derived wrongly every + * time someone needs it — any future connect-time action wants exactly this gate. * * Exactly two things publish a CONNECTED event, and `deviceType` separates them cleanly: * - `RingBLEClient`'s own connect always passes `deviceType = activeCoordinator.deviceType`, which @@ -677,19 +683,40 @@ internal fun historyMeasurementId(kind: MeasurementKind, timestamp: Long): Strin internal fun isConnectTransition(eventDeviceType: RingDeviceType?): Boolean = eventDeviceType != null /** - * What a CONNECTED event is allowed to remove. + * What a CONNECTED event is allowed to remove: **nothing**. + * + * The enum has exactly one member, and that is the point. Connecting must never delete stored + * history (issue #43) and — since the iOS-parity fix — must not delete demo rows either. Encoding + * that as a *type* rather than a comment means widening what a connect may delete cannot be done + * by editing one line: it needs a new member here, which fails to compile against the exhaustive + * `when`s in [EventPersistenceSubscriber] and in `EventPersistenceIdentityTest`. That is the + * tripwire the deleted `preservesSleepOnConnect` boolean never had — it made destructive clearing + * a per-family *option*, and every family took it but one. + * + * Its limit, stated plainly so nobody trusts it further than it goes: it forces a compile error + * only for an author who routes the new deletion through this enum. A bare + * `db.measurementDao().clearDemo()` dropped straight into the CONNECTED arm still compiles. The + * type documents and channels the invariant; the tests below it are what actually enforce it. * - * There is deliberately **no member meaning "real rows"**. Connecting must never delete stored - * history (issue #43), and encoding that as a type instead of a comment means re-introducing the old - * behaviour takes more than adding a `.clear()` call: it needs a new member here, which fails to - * compile against the exhaustive `when`s in [EventPersistenceSubscriber] and in - * `EventPersistenceIdentityTest`. That is the tripwire the deleted `preservesSleepOnConnect` never - * had — it made destructive clearing a per-family *option*, and every family took it but one. + * A `DEMO_ROWS` member used to live here. It was the last fragment of the old "connect = clear + * everything and rebuild from the ring" design, and it fired on every reconnect — measured at + * roughly one every five minutes on a COLMI R10 — so seeded demo data could not survive alongside + * a paired ring for more than a few minutes. iOS never had a connect-time purge at all, so this + * now matches it. */ -internal enum class ConnectPurge { NOTHING, DEMO_ROWS } +internal enum class ConnectPurge { NOTHING } -internal fun connectPurge(eventDeviceType: RingDeviceType?): ConnectPurge = - if (isConnectTransition(eventDeviceType)) ConnectPurge.DEMO_ROWS else ConnectPurge.NOTHING +/** + * Always [ConnectPurge.NOTHING]. + * + * Kept as a function rather than inlined so the guard has one named place to sit, and so the + * exhaustive `when`s that enforce it have something to switch on. The result does not depend on + * [eventDeviceType] — nothing is deleted for any family — but the parameter is retained so that a + * future connect-time action has the device in hand and reaches for [isConnectTransition] rather + * than firing on decoder status echoes as well as real connects. + */ +@Suppress("UNUSED_PARAMETER") +internal fun connectPurge(eventDeviceType: RingDeviceType?): ConnectPurge = ConnectPurge.NOTHING internal fun shouldReplaceCompleteSleep( existingStart: Long, diff --git a/app/src/main/java/com/pulseloop/service/MetricsService.kt b/app/src/main/java/com/pulseloop/service/MetricsService.kt index e27f212..fde3504 100644 --- a/app/src/main/java/com/pulseloop/service/MetricsService.kt +++ b/app/src/main/java/com/pulseloop/service/MetricsService.kt @@ -1,5 +1,6 @@ package com.pulseloop.service +import com.pulseloop.data.DemoDataPolicy import com.pulseloop.data.PulseLoopDatabase import com.pulseloop.data.entity.* import com.pulseloop.ring.MeasurementKind @@ -39,9 +40,17 @@ object MetricsService { val todayActivity = db.activityDailyDao().byDay(todayStart) val device = db.deviceDao().current() - val hr24h = db.measurementDao().range(MeasurementKind.HEART_RATE.name, now - 24 * 3600_000L, now) - val spo24h = db.measurementDao().range(MeasurementKind.SPO2.name, now - 24 * 3600_000L, now) - val recent7 = db.activityDailyDao().recent(7) + // Real wins over seeded rows once the ring has synced the series (`DemoDataPolicy`) — + // a connect no longer purges demo data, so the two coexist indefinitely. + val dayAgo = now - 24 * 3600_000L + val hr24h = if (db.measurementDao().hasReal(MeasurementKind.HEART_RATE.name)) + db.measurementDao().rangeReal(MeasurementKind.HEART_RATE.name, dayAgo, now) + else db.measurementDao().range(MeasurementKind.HEART_RATE.name, dayAgo, now) + val spo24h = if (db.measurementDao().hasReal(MeasurementKind.SPO2.name)) + db.measurementDao().rangeReal(MeasurementKind.SPO2.name, dayAgo, now) + else db.measurementDao().range(MeasurementKind.SPO2.name, dayAgo, now) + val recent7 = if (db.activityDailyDao().hasReal()) db.activityDailyDao().recentReal(7) + else db.activityDailyDao().recent(7) val latestHr = db.measurementDao().latest(MeasurementKind.HEART_RATE.name, now)?.toInt() val latestSpo2 = db.measurementDao().latest(MeasurementKind.SPO2.name, now)?.toInt() @@ -61,7 +70,9 @@ object MetricsService { restingHeartRate = restingHr, peakHeartRate = hrValues.maxOrNull()?.toInt(), batteryPercent = device?.batteryPercent, - isDemo = todayActivity?.source == "mock", + // The seeder writes source = "demo"; comparing against "mock" (the iOS spelling) + // alone meant this never fired, so seeded steps/HR were reported as live data. + isDemo = DemoDataPolicy.isDemo(todayActivity?.source), stepsTrend = recent7.map { it.steps.toDouble() }, hrTrend24h = hrValues, spo2Trend24h = spoValues, @@ -82,8 +93,10 @@ object MetricsService { ): List { val now = System.currentTimeMillis() val cutoff = now - hours * 3600_000L - return db.measurementDao().range(kind.name, cutoff, now) - .map { MetricSample(it.timestamp, it.value) } + val rows = if (db.measurementDao().hasReal(kind.name)) + db.measurementDao().rangeReal(kind.name, cutoff, now) // real wins (`DemoDataPolicy`) + else db.measurementDao().range(kind.name, cutoff, now) + return rows.map { MetricSample(it.timestamp, it.value) } } // ── Device Capabilities ────────────────────────────────────────────── diff --git a/app/src/main/java/com/pulseloop/service/RestingHRBaselineService.kt b/app/src/main/java/com/pulseloop/service/RestingHRBaselineService.kt index dc7562c..f24cce5 100644 --- a/app/src/main/java/com/pulseloop/service/RestingHRBaselineService.kt +++ b/app/src/main/java/com/pulseloop/service/RestingHRBaselineService.kt @@ -37,7 +37,12 @@ object RestingHRBaselineService { val now = System.currentTimeMillis() val start = now - BASELINE_DAYS * 86_400_000L // Room suspend queries already run off the main thread on their own dispatcher. - val samples = db.measurementDao().range(MeasurementKind.HEART_RATE.name, start, now) + // `rangeReal`, not `range`: the seeder plants ~30 days of HR — overnight ~56 bpm, workout + // spikes at 142/152 — squarely inside this window, and since PR #52 a connect no longer + // clears it. A demo-derived p10 is then PERSISTED to `hrRestingBaseline` and drives the + // `"auto"` HR zones applied to real readings, surviving a Clear Demo Data by up to the + // 6-hour refresh interval (`DemoDataPolicy`). + val samples = db.measurementDao().rangeReal(MeasurementKind.HEART_RATE.name, start, now) if (samples.size < MIN_SAMPLES) return val values = samples.map { it.value }.sorted() diff --git a/app/src/main/java/com/pulseloop/service/SleepInsights.kt b/app/src/main/java/com/pulseloop/service/SleepInsights.kt index 3197041..caa956b 100644 --- a/app/src/main/java/com/pulseloop/service/SleepInsights.kt +++ b/app/src/main/java/com/pulseloop/service/SleepInsights.kt @@ -1,5 +1,6 @@ package com.pulseloop.service +import com.pulseloop.data.DemoDataPolicy import com.pulseloop.data.entity.SleepSessionEntity import com.pulseloop.data.entity.SleepStageBlockEntity import com.pulseloop.ring.SleepStage @@ -173,7 +174,13 @@ object SleepInsights { sessions: List, blocksBySession: (String) -> List, ): List = - sessions.groupBy { it.date }.map { (day, daySessions) -> + sessions.groupBy { it.date }.map { (day, all) -> + // A ring night and a seeded demo night can share a date now that a connect deletes + // nothing (`DemoDataPolicy`) — the seeder's "skip days that already have ring data" + // guard only covers seed-after-sync, not sync-after-seed. Summing the two would + // report ~13h asleep and inflate every average built on top. Real wins per day. + val ring = all.filterNot { DemoDataPolicy.isDemo(it.sourceRaw) } + val daySessions = if (ring.isNotEmpty()) ring else all if (daySessions.size == 1) { val only = daySessions.first() return@map DaySleep(only, blocksBySession(only.id)) diff --git a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt index 3e9e9b6..fc448de 100644 --- a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt +++ b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt @@ -2,6 +2,7 @@ package com.pulseloop.ui.viewmodels import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.pulseloop.data.DemoDataPolicy import com.pulseloop.data.PulseLoopDatabase import com.pulseloop.data.dao.Bucket import com.pulseloop.data.entity.* @@ -273,9 +274,11 @@ class SleepViewModel(private val db: PulseLoopDatabase) : ViewModel() { val offset = _state.value.dayOffset val shownNow = System.currentTimeMillis() - offset * 86_400_000L val shownReference = TimeUtil.referenceNightLocal(shownNow) - // A day is either all-ring or all-demo (the seeder skips days with ring data); prefer ring. + // A day can hold BOTH a ring night and a seeded demo one — the seeder's guard only skips + // days that already have ring data, and since PR #52 a connect no longer purges demo rows + // afterwards. Real wins (`DemoDataPolicy`). val shownAll = db.sleepSessionDao().allByDay(shownReference).filter { it.totalMinutes > 0 } - val shownRing = shownAll.filter { it.sourceRaw != "demo" } + val shownRing = shownAll.filterNot { DemoDataPolicy.isDemo(it.sourceRaw) } val daySessions = (if (shownRing.isNotEmpty()) shownRing else shownAll).sortedBy { it.startAt } val dayBlocks = daySessions.associate { it.id to blocksFor(it.id) } // The primary (longest) session drives the scripted day-level coach fallback. @@ -293,12 +296,18 @@ class SleepViewModel(private val db: PulseLoopDatabase) : ViewModel() { SleepRangeKey.MONTH -> 30 SleepRangeKey.YEAR -> 365 } + // Once the ring has synced any night, the aggregate is built from ring nights only — + // averaging seeded nights in alongside them reports a history the user never slept + // (`DemoDataPolicy`). With no ring data yet, demo nights are the whole picture. + val sleepHasReal = db.sleepSessionDao().hasReal() val anchor = if (range == SleepRangeKey.DAY) shownReference - else db.sleepSessionDao().recent(1).firstOrNull()?.let { TimeUtil.startOfDayLocal(it.date) } + else (if (sleepHasReal) db.sleepSessionDao().recentReal(1) else db.sleepSessionDao().recent(1)) + .firstOrNull()?.let { TimeUtil.startOfDayLocal(it.date) } ?: TimeUtil.startOfTodayLocal() val start = anchor - (expected - 1) * 86_400_000L val end = anchor + 86_400_000L - 1 - val sessions = db.sleepSessionDao().inRange(start, end) + val sessions = if (sleepHasReal) db.sleepSessionDao().inRangeReal(start, end) + else db.sleepSessionDao().inRange(start, end) sessions.forEach { blocksFor(it.id) } // warm cache for the collapse lookup below val realLookup: (String) -> List = { blocksCache[it] ?: emptyList() } // Collapse each waking day (main night + naps) into one combined summary so a night plus @@ -577,11 +586,18 @@ class VitalsViewModel(private val db: PulseLoopDatabase, private val apiKeyStore // iOS `rangeSamples`: demo mode charts the FULL seeded history (per kind) instead of the // 24h window — that's what makes sparse demo series (daily HRV/temp) render as the // month-long scatter the Simulator shows. Real ring data keeps the 24h window. - suspend fun series(kind: MeasurementKind): List = - if (db.measurementDao().hasDemo(kind.name)) + // + // Which mode applies is decided by whether the ring has synced THIS kind, not by whether + // demo rows exist (`DemoDataPolicy`). Keying it off `hasDemo` alone was fine while a + // connect purged demo rows; since PR #52 it isn't — one reseed would otherwise pin every + // chart to demo-mode full history, over a demo/real interleaved series, forever. + suspend fun series(kind: MeasurementKind): List = when { + db.measurementDao().hasReal(kind.name) -> + db.measurementDao().rangeReal(kind.name, twentyFourHoursAgo, now) + db.measurementDao().hasDemo(kind.name) -> db.measurementDao().range(kind.name, 0, Long.MAX_VALUE) - else - db.measurementDao().range(kind.name, twentyFourHoursAgo, now) + else -> emptyList() + } val hr = series(MeasurementKind.HEART_RATE) val spo2 = series(MeasurementKind.SPO2) diff --git a/app/src/test/java/com/pulseloop/data/DemoDataPolicyTest.kt b/app/src/test/java/com/pulseloop/data/DemoDataPolicyTest.kt new file mode 100644 index 0000000..8714d2d --- /dev/null +++ b/app/src/test/java/com/pulseloop/data/DemoDataPolicyTest.kt @@ -0,0 +1,32 @@ +package com.pulseloop.data + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The demo/real predicate the readers share now that a connect deletes nothing (PR #52). + * + * The regression this pins: `MetricsService` compared `source == "mock"` — the iOS spelling — + * while `DemoDataSeeder` writes `"demo"`, so `TodaySummary.isDemo` was false for every seeded row + * and demo steps/HR were reported as live readings. + */ +class DemoDataPolicyTest { + @Test + fun `the seeder's own spelling counts as demo`() { + assertTrue(DemoDataPolicy.isDemo("demo")) + } + + @Test + fun `the iOS spelling counts as demo`() { + assertTrue(DemoDataPolicy.isDemo("mock")) + } + + @Test + fun `synced and unknown sources are not demo`() { + assertFalse(DemoDataPolicy.isDemo("ring")) + assertFalse(DemoDataPolicy.isDemo("manual")) + assertFalse(DemoDataPolicy.isDemo(null)) + assertFalse(DemoDataPolicy.isDemo("")) + } +} diff --git a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt index 3e8f984..e9f3c57 100644 --- a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt +++ b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt @@ -26,19 +26,36 @@ class EventPersistenceIdentityTest { * (`makeHistoryQueryCommand()`'s 1-day default at `JringDriver.kt:105`). * * The `when` below is exhaustive on purpose: it is the actual guard. Adding a [ConnectPurge] - * member that deletes real rows stops this file compiling, which is the tripwire the old + * member that deletes anything stops this file compiling, which is the tripwire the old * per-family `preservesSleepOnConnect` boolean never provided. */ @Test - fun `no connect event may purge anything but demo rows, for any family`() { + fun `no connect event may purge anything, for any family`() { val everyOrigin: List = RingDeviceType.entries + null for (origin in everyOrigin) { - val purge = connectPurge(origin) - val deletesRealRows = when (purge) { + val deletesAnything = when (connectPurge(origin)) { ConnectPurge.NOTHING -> false - ConnectPurge.DEMO_ROWS -> false } - assertFalse("connect purge $purge (origin $origin) must not delete real rows", deletesRealRows) + assertFalse("connect (origin $origin) must not delete anything", deletesAnything) + } + } + + /** + * iOS parity. iOS has no connect-time demo purge anywhere in its source — its only deletion of + * seeded rows is user-initiated, inside `SeedData` — and it handles the demo/real mix by + * *detecting* it (`isDemo`, `source == "mock"`, `DataFreshness.demo`) rather than cleaning it + * up. Android used to clear demo rows on every CONNECTED, which on a paired ring meant every + * reconnect: measured at roughly one per five minutes on a COLMI R10, and captured live at + * 09:00:54 taking 772 measurements / 84 activity days / 36 sleep sessions to zero with the + * phone untouched. + */ + @Test + fun `connecting never retires demo rows, first connect or reconnect`() { + for (family in RingDeviceType.entries) { + assertEquals( + "$family must not purge on its first connect", + ConnectPurge.NOTHING, connectPurge(family), + ) } // The decoder-Status case — the one that used to fire all session long — purges nothing. assertEquals(ConnectPurge.NOTHING, connectPurge(null)) diff --git a/app/src/test/java/com/pulseloop/service/SleepSegmentationTest.kt b/app/src/test/java/com/pulseloop/service/SleepSegmentationTest.kt index c25b310..99e4738 100644 --- a/app/src/test/java/com/pulseloop/service/SleepSegmentationTest.kt +++ b/app/src/test/java/com/pulseloop/service/SleepSegmentationTest.kt @@ -80,10 +80,13 @@ class SleepSegmentationTest { // ── collapseByDay ──────────────────────────────────────────────────── - private fun session(id: String, day: Long, startMin: Int, durMin: Int, score: Int?) = + private fun session( + id: String, day: Long, startMin: Int, durMin: Int, score: Int?, source: String = "ring", + ) = SleepSessionEntity( id = id, date = day, startAt = day + startMin * minute, endAt = day + (startMin + durMin) * minute, totalMinutes = durMin, score = score, + sourceRaw = source, ) @Test @@ -127,6 +130,44 @@ class SleepSegmentationTest { assertNull(collapsed[0].session.score) } + // A connect no longer purges demo rows (PR #52), so a seeded night and a synced night can + // share a date — the seeder's guard only skips days that ALREADY have ring data. Summing the + // two reported ~13h asleep and inflated every average built on the collapse. + @Test + fun `a ring night outranks a demo night on the same day`() { + val day = 1_000_000L + val demo = session("demo-sleep-$day", day, 0, 420, 90, source = "demo") + val ring = session("ring", day, 30, 360, 70) + val collapsed = SleepInsights.collapseByDay(listOf(demo, ring)) { emptyList() } + assertEquals(1, collapsed.size) + val c = collapsed[0].session + assertEquals("ring", c.id) // passes through as a single-session day + assertEquals(360, c.totalMinutes) // NOT 420 + 360 + assertEquals(70, c.score) + } + + @Test + fun `demo nights still collapse normally when no ring night shares the day`() { + val day = 1_000_000L + val night = session("demo-sleep-$day", day, 0, 420, 80, source = "demo") + val nap = session("demo-nap", day, 900, 30, 60, source = "demo") + val collapsed = SleepInsights.collapseByDay(listOf(night, nap)) { emptyList() } + assertEquals(1, collapsed.size) + assertEquals(450, collapsed[0].session.totalMinutes) + assertEquals("demo", collapsed[0].session.sourceRaw) + } + + @Test + fun `demo and ring nights on different days both survive`() { + val d1 = 1_000_000L + val d2 = d1 + 86_400_000L + val collapsed = SleepInsights.collapseByDay( + listOf(session("demo-sleep-$d1", d1, 0, 420, 80, source = "demo"), + session("ring", d2, 0, 400, 70)), + ) { emptyList() } + assertEquals(2, collapsed.size) + } + @Test fun `distinct days stay distinct and sorted`() { val d1 = 1_000_000L