From a038b4e65140ffc47ca23289b09c21ee39734689 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Sat, 22 Aug 2026 09:12:49 -0700 Subject: [PATCH 1/2] fix: connecting no longer purges demo data at all (iOS parity) Seeded demo data vanished within minutes of being created whenever a ring was paired, with no user action. Reported as "reseed demo data resets when I navigate Today -> Sleep"; navigation was a red herring (four tab bounces with counts pinned at 772/84/36 and no device event firing). The cause was the reconnect cadence. Captured live while sampling the DB with the phone untouched: 08:57:53 demo=772/84/36 conn=08:52:34 09:00:52 demo=772/84/36 conn=- <- link drops 09:00:58 demo=0/0/0 conn=09:00:54 <- reconnect wipes it CONNECTED ran clearDemo() across measurements, activity_daily, sleep_sessions and sleep_stage_blocks, gated only on "this is the BLE client's own connect". That gate correctly excludes decoder Status echoes but not a first connect from the 200th, and a paired COLMI R10 re-establishes its link roughly every five minutes. The purge is now removed outright rather than narrowed to first-connect, because it was never a deliberate product decision. It is the last surviving fragment of the original "connect = clear everything and rebuild from the ring" design, which five commits have been dismantling (aefa646 sleep clear() -> clearDemo(), c3b669e gate on a real transition, e11be6e stop deleting sleep history, 7649513 make "connect deletes nothing" structural). iOS -- which this is a port of -- 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 rows are retired only from Settings -> Privacy & Data. ConnectPurge is kept, reduced to a single NOTHING member. The single-branch `when`s that switch on it read as dead code and are not: they are what makes re-introducing a connect-time delete a compile error rather than a one-line .clear(), which is the tripwire the old per-family preservesSleepOnConnect boolean never had. AGENTS.md updated to match, including a note not to "simplify" those whens away. Full suite: 1039 tests, 0 failures. --- AGENTS.md | 29 +++++-- .../service/EventPersistenceSubscriber.kt | 81 +++++++++++-------- .../service/EventPersistenceIdentityTest.kt | 29 +++++-- 3 files changed, 96 insertions(+), 43 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 45641fe..b15100b 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,31 @@ 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, and `connectPurge` is what it feeds — though +`connectPurge` now returns `ConnectPurge.NOTHING` unconditionally, so nothing is deleted on either +path. 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 **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 re-adding a connect-time delete a compile error instead of a one-line edit. +Don't "simplify" them away. + 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/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index 82ac204..4153a79 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" } @@ -677,19 +678,35 @@ 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 re-introducing any connect-time delete cannot be + * done with a one-line `.clear()`: 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. * - * 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. [eventDeviceType] is retained + * because [isConnectTransition] remains the meaningful distinction elsewhere — a real connect from + * the BLE client versus a decoder re-asserting CONNECTED from an ordinary device-info reply — and + * because any future connect-time action needs exactly that gate to be correct. + */ +@Suppress("UNUSED_PARAMETER") +internal fun connectPurge(eventDeviceType: RingDeviceType?): ConnectPurge = ConnectPurge.NOTHING internal fun shouldReplaceCompleteSleep( existingStart: Long, 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)) From 08bfab4a7c9e111c8eb57b7cbca069aee26a0202 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Sat, 22 Aug 2026 09:35:17 -0700 Subject: [PATCH 2/2] fix: readers pick real over demo now that connect purges nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing the connect-time demo purge also removed the thing that had been keeping seeded rows out of every unfiltered query. Demo and ring rows now coexist indefinitely, and Reseed Demo Data ships in Settings, so the mixed state is reachable by any user who seeds and then pairs. DemoDataPolicy states the rule the readers follow: 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. Values that are persisted or exported read *Real unconditionally, since a demo-derived number outlives the demo data behind it. - SleepInsights.collapseByDay: a ring night outranks a demo night on the same date. Summing them reported ~13h asleep and inflated every average, stage average and score built on the collapse. The seeder's guard only covers seed-after-sync, not sync-after-seed. - ViewModels: the sleep aggregate and its anchor take inRangeReal/recentReal once any night has synced; the Day-view comment claiming a day is all-ring-or-all-demo is corrected. - ViewModels.buildState: the demo-mode "chart full history" switch keys off hasReal(kind), not hasDemo(kind). One reseed used to pin every vitals chart to full history over an interleaved series, permanently. - RestingHRBaselineService, DailyCalorieEstimator: rangeReal. The seeder plants ~30d of HR inside the baseline window, and the calorie estimate is written back onto a real activity_daily row that is export-eligible. - MetricsService.isDemo compared against "mock" while the seeder writes "demo", so it never fired; DemoDataPolicy.isDemo accepts both. - CoachContextBuilder: isDemo was hardcoded false, so the demo-data warning could never be emitted while stepsWeek/hrResting fed the LLM a blend. - ConnectPurge/AGENTS.md: the tripwire only catches an author who routes a new deletion through the enum — a bare clearDemo() in the CONNECTED arm still compiles. Say so, and note isConnectTransition has no production caller left. 1044 tests, 0 failures. --- AGENTS.md | 28 +++++++++--- .../coach/context/CoachContextBuilder.kt | 23 +++++++--- .../java/com/pulseloop/data/DemoDataPolicy.kt | 32 ++++++++++++++ .../main/java/com/pulseloop/data/dao/Daos.kt | 27 ++++++++++++ .../service/DailyCalorieEstimator.kt | 6 ++- .../service/EventPersistenceSubscriber.kt | 30 ++++++++----- .../com/pulseloop/service/MetricsService.kt | 25 ++++++++--- .../service/RestingHRBaselineService.kt | 7 ++- .../com/pulseloop/service/SleepInsights.kt | 9 +++- .../com/pulseloop/ui/viewmodels/ViewModels.kt | 32 ++++++++++---- .../com/pulseloop/data/DemoDataPolicyTest.kt | 32 ++++++++++++++ .../service/SleepSegmentationTest.kt | 43 ++++++++++++++++++- 12 files changed, 255 insertions(+), 39 deletions(-) create mode 100644 app/src/main/java/com/pulseloop/data/DemoDataPolicy.kt create mode 100644 app/src/test/java/com/pulseloop/data/DemoDataPolicyTest.kt diff --git a/AGENTS.md b/AGENTS.md index b15100b..517f6b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -101,10 +101,13 @@ 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 — though -`connectPurge` now returns `ConnectPurge.NOTHING` unconditionally, so nothing is deleted on either -path. 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. @@ -123,8 +126,21 @@ 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 re-adding a connect-time delete a compile error instead of a one-line edit. -Don't "simplify" them away. +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: 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 4153a79..1ba5e92 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -661,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 @@ -682,11 +687,16 @@ internal fun isConnectTransition(eventDeviceType: RingDeviceType?): Boolean = ev * * 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 re-introducing any connect-time delete cannot be - * done with a one-line `.clear()`: 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. + * 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. * * 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 @@ -700,10 +710,10 @@ internal enum class 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. [eventDeviceType] is retained - * because [isConnectTransition] remains the meaningful distinction elsewhere — a real connect from - * the BLE client versus a decoder re-asserting CONNECTED from an ordinary device-info reply — and - * because any future connect-time action needs exactly that gate to be correct. + * 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 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/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