fix: stamp a device as seen when it connects, not when it was added (#1527)

Packages/WhoopStore/Sources/WhoopStore/DeviceRegistryStore.swift

@@ -76,6 +76,20 @@ public struct DeviceRegistryStore: Sendable {
         }
     }
 
+    /// Stamp a row as seen right now. Called when a strap actually connects or drops, because nothing
+    /// else in the BLE path writes `lastSeenAt` — before #1527 it was only ever set when the row was
+    /// created or promoted to active, so the Devices card reported time-since-ADDED and a strap syncing
+    /// every day could read "Last seen 45 d ago".
+    ///
+    /// Archived rows are excluded: "Removed - data kept" is a deliberate resting state, and a stray
+    /// connect must not quietly resurrect one into looking live.
+    public func touchLastSeen(_ id: String, at ts: Int) throws {
+        try dbQueue.write { db in
+            try db.execute(sql: "UPDATE pairedDevice SET lastSeenAt = ? WHERE id = ? AND status != 'archived'",
+                           arguments: [ts, id])
+        }
+    }
+
     /// Adopt (or clear) the stable BLE identity for a registry row. `peripheralId` is the
     /// CBPeripheral.identifier.uuidString on iOS/Mac; passing nil un-adopts it.
     public func setPeripheralId(_ id: String, peripheralId: String?) throws {

Strand/BLE/SourceCoordinator.swift

@@ -489,6 +489,16 @@ final class SourceCoordinator: ObservableObject {
 
     // MARK: - Identity adoption
 
+    /// Stamp the row belonging to the strap that just DISCONNECTED, resolved by the identity it adopted
+    /// rather than by whatever is active now. On a multi-WHOOP install those are not the same row: make
+    /// another strap active while this one is live, and stamping "the active row" would record a sighting
+    /// of a strap that was never connected — the same mis-mapping the peripheralId guard above exists to
+    /// prevent. A uuid no row has adopted stamps nothing. (#1527)
+    private func touchLastSeen(forStrap uuid: String) {
+        guard let device = registry.device(forPeripheralId: uuid) else { return }
+        registry.touchLastSeen(device.id)
+    }
+
     /// The BLE engine connected to a WHOOP peripheral (`uuid`). Persist that stable identity onto the
     /// CURRENTLY ACTIVE device when it's a WHOOP and hasn't adopted one yet — so the legacy "my-whoop"
     /// learns its strap's id on first connect, and a freshly-paired WHOOP confirms its identity.
@@ -508,8 +518,16 @@ final class SourceCoordinator: ObservableObject {
         // Track the live strap's uuid for the WHOOP->WHOOP adopt-in-place skip (#74). nil is a
         // disconnect/never-connected republish: clear it so a later make-active can't wrongly match a stale
         // link, then fall through to the existing ignore.
+        let previouslyConnectedTo = connectedWhoopUuid
         connectedWhoopUuid = uuid
-        guard let uuid else { return }
+        guard let uuid else {
+            // Disconnect edge. The strap was in hand right up to this instant, so stamp it NOW rather than
+            // leaving the connect-time value: after a ten-hour overnight session that would have the card
+            // reading "Last seen 10 h ago" the moment the link dropped. Only when we were actually
+            // connected — a nil republish with no prior link is not a sighting. (#1527)
+            if let previouslyConnectedTo { touchLastSeen(forStrap: previouslyConnectedTo) }
+            return
+        }
 
         let activeId = registry.activeDeviceId
         guard isWhoop(activeId),
@@ -519,8 +537,9 @@ final class SourceCoordinator: ObservableObject {
         case .none:
             // First connect for this WHOOP row → adopt the strap's stable identity.
             registry.setPeripheralId(activeId, peripheralId: uuid)
+            registry.touchLastSeen(activeId)
         case .some(uuid):
-            break                               // already adopted this exact strap → nothing to do
+            registry.touchLastSeen(activeId)    // already adopted this exact strap → only the sighting is new
         case .some(let existing):
             // A DIFFERENT strap connected under this WHOOP row. Re-adopt ONLY when this is the #52 stale-pin
             // handoff — i.e. the engine is genuinely encrypted-bonded to the strap whose id just arrived.
@@ -531,6 +550,7 @@ final class SourceCoordinator: ObservableObject {
             if live.encryptedBond {
                 live.append(log: "Multi-WHOOP (#52): active device \(activeId) was pinned to strap \(existing) which refused to bond — re-adopting the working strap \(uuid).")
                 registry.setPeripheralId(activeId, peripheralId: uuid)
+                registry.touchLastSeen(activeId)
             } else {
                 live.append(log: "Multi-WHOOP: active device \(activeId) is registered to strap \(existing) but \(uuid) connected — not overwriting.")
             }

Strand/Data/DeviceRegistry.swift

@@ -125,6 +125,13 @@ final class DeviceRegistry: ObservableObject {
         reload()
     }
 
+    /// Stamp a device as seen right now — a real connect or disconnect, not every inbound packet, which
+    /// would be a write per second for no more truth. Refreshes the published list. Best-effort. (#1527)
+    func touchLastSeen(_ id: String, at ts: Int = Int(Date().timeIntervalSince1970)) {
+        try? store.touchLastSeen(id, at: ts)
+        reload()
+    }
+
     /// Find the paired device that has adopted a given BLE peripheral, if any. A plain read of the
     /// store (no reload) — returns nil on any error or when no row has adopted that peripheral yet.
     func device(forPeripheralId peripheralId: String) -> PairedDevice? {

android/app/src/main/java/com/noop/ble/SourceCoordinator.kt

@@ -212,6 +212,23 @@ class SourceCoordinator(
             }
         }
 
+    /** Stamp the row belonging to the strap that just DISCONNECTED, resolved by the identity it adopted
+     *  rather than by whatever is active now. On a multi-WHOOP install those are not the same row: make
+     *  another strap active while this one is live, and stamping "the active row" would record a sighting of
+     *  a strap that was never connected — the same mis-mapping the peripheralId guard below exists to
+     *  prevent. An address no row has adopted stamps nothing.
+     *
+     *  Matched the way the connect path below matches — `ignoreCase` — and NOT through
+     *  [DeviceRegistry.deviceForPeripheralId], whose `WHERE peripheralId = ?` is case-SENSITIVE in SQLite.
+     *  A stored address differing only in case is the same strap to the guard below, so resolving it more
+     *  strictly here would adopt on connect and then silently stamp nothing on disconnect — the same
+     *  quiet staleness this whole change exists to remove. The Swift twin keeps its exact lookup because
+     *  its connect path compares exactly too; each side matches ITS OWN adopt rule. (#1527) */
+    private suspend fun touchLastSeenForStrap(address: String) {
+        val row = registry.all().firstOrNull { it.peripheralId.equals(address, ignoreCase = true) } ?: return
+        registry.touchLastSeen(row.id)
+    }
+
     /**
      * The BLE engine connected to a WHOOP strap at [address] (null on disconnect). Persist that stable
      * identity onto the CURRENTLY ACTIVE device when it's a WHOOP and hasn't adopted one yet — so the
@@ -230,8 +247,18 @@ class SourceCoordinator(
         // Track the live strap's address for the WHOOP->WHOOP adopt-in-place skip (#74). A null address is a
         // disconnect/never-connected republish: clear it so a later make-active can't wrongly match a stale
         // link, then fall through to the existing ignore.
+        val previouslyConnectedTo = connectedWhoopAddress
         connectedWhoopAddress = address
-        if (address == null) return
+        if (address == null) {
+            // Disconnect edge. The strap was in hand right up to this instant, so stamp it NOW rather than
+            // leaving the connect-time value: after a ten-hour overnight session that would have the card
+            // reading "Last seen 10 h ago" the moment the link dropped. Only when we were actually
+            // connected — a null republish with no prior link is not a sighting. (#1527)
+            if (previouslyConnectedTo != null) {
+                scope.launch { runCatching { touchLastSeenForStrap(previouslyConnectedTo) } }
+            }
+            return
+        }
         scope.launch {
             val activeId = registry.activeDeviceId() ?: return@launch
             val devices = registry.all()
@@ -240,11 +267,14 @@ class SourceCoordinator(
 
             val existing = row.peripheralId
             when {
-                existing == null ->
+                existing == null -> {
                     // First connect for this WHOOP row → adopt the strap's stable identity (its address).
                     registry.setPeripheralId(activeId, address)
+                    registry.touchLastSeen(activeId)
+                }
                 existing.equals(address, ignoreCase = true) -> {
-                    // Already adopted this exact strap → nothing to do.
+                    // Already adopted this exact strap → only the sighting is new. (#1527)
+                    registry.touchLastSeen(activeId)
                 }
                 else ->
                     // A DIFFERENT strap connected under this WHOOP row. Never silently overwrite — that would

android/app/src/main/java/com/noop/data/DeviceRegistry.kt

@@ -121,6 +121,11 @@ class DeviceRegistry(
         }
     }
 
+    /** Stamp a device as seen right now — a real connect or disconnect, not every inbound packet, which
+     *  would be a write per second for no more truth. Twin of Swift `DeviceRegistry.touchLastSeen`. (#1527) */
+    suspend fun touchLastSeen(id: String, now: Long = System.currentTimeMillis() / 1000) =
+        dao.touchLastSeen(id, now)
+
     /** Archive a device — keeps its row and samples (invariant I4). */
     suspend fun archive(id: String) = dao.archiveDevice(id)
 

android/app/src/main/java/com/noop/data/DeviceRegistryDao.kt

@@ -34,6 +34,15 @@ interface DeviceRegistryDao {
     @Query("UPDATE pairedDevice SET status = 'active', lastSeenAt = :now WHERE id = :id")
     suspend fun promote(id: String, now: Long)
 
+    /** Stamp a device as seen right now. Nothing in the BLE path wrote `lastSeenAt` before #1527 — it was
+     *  set only when the row was created or promoted to active — so the Devices card reported
+     *  time-since-ADDED, and a strap syncing daily could read "Last seen 45 d ago".
+     *
+     *  Archived rows are excluded: "Removed - data kept" is a deliberate resting state and a stray connect
+     *  must not quietly resurrect one into looking live. Twin of the Swift store's `touchLastSeen`. */
+    @Query("UPDATE pairedDevice SET lastSeenAt = :now WHERE id = :id AND status != 'archived'")
+    suspend fun touchLastSeen(id: String, now: Long)
+
     /** Archive a device (keeps the row + its samples — invariant I4). */
     @Query("UPDATE pairedDevice SET status = 'archived' WHERE id = :id")
     suspend fun archiveDevice(id: String)

android/app/src/test/java/com/noop/analytics/RegistryDayOwnerSourceTest.kt

@@ -44,6 +44,12 @@ class RegistryDayOwnerSourceTest {
             devices[id]?.let { devices[id] = it.copy(model = model) }
         }
         override suspend fun renameDevice(id: String, nickname: String?) {}
+        override suspend fun touchLastSeen(id: String, now: Long) {
+            // #1527, mirrors the query's `AND status != 'archived'`.
+            devices[id]?.let {
+                if (it.status != DeviceStatus.archived.name) devices[id] = it.copy(lastSeenAt = now)
+            }
+        }
         override suspend fun setPeripheralId(id: String, peripheralId: String?) {
             devices[id]?.let { devices[id] = it.copy(peripheralId = peripheralId) }
         }

android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt

@@ -60,6 +60,12 @@ class SourceCoordinatorAdoptionTest {
         override suspend fun setPeripheralId(id: String, peripheralId: String?) {
             devices[id]?.let { devices[id] = it.copy(peripheralId = peripheralId) }
         }
+        override suspend fun touchLastSeen(id: String, now: Long) {
+            // Reproduces the query's `AND status != 'archived'` guard.
+            devices[id]?.let {
+                if (it.status != DeviceStatus.archived.name) devices[id] = it.copy(lastSeenAt = now)
+            }
+        }
         override suspend fun deviceForPeripheralId(peripheralId: String): PairedDeviceRow? =
             devices.values.firstOrNull { it.peripheralId == peripheralId }
         override suspend fun setDayOwner(row: DayOwnershipRow) { owners[row.day] = row }
@@ -384,4 +390,152 @@ class SourceCoordinatorAdoptionTest {
         assertEquals("a different WHOOP must drop the current link", 1, stops)
         assertEquals("a different WHOOP must reconnect", 1, starts)
     }
+
+    // MARK: - "Last seen" is stamped by the link, not by the wizard (#1527)
+
+    /**
+     * The regression: a strap that connects must move `lastSeenAt`. Nothing in the BLE path wrote the
+     * column before this, so the Devices card reported time-since-ADDED and a strap syncing every day
+     * read "Last seen 45 d ago".
+     */
+    @Test
+    fun aConnectStampsTheActiveRowAsSeen() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01")
+        }
+        coordinatorOver(dao).connectedPeripheralChanged("AA:BB:CC:DD:EE:01")
+        assertTrue("a connect must move lastSeenAt off its seeded value",
+                   dao.devices["my-whoop"]!!.lastSeenAt > 100)
+    }
+
+    /** A first connect adopts the identity AND records the sighting — the two are the same event. */
+    @Test
+    fun aFirstConnectAdoptsTheIdentityAndStamps() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["my-whoop"] = whoopRow("my-whoop", peripheralId = null)
+        }
+        coordinatorOver(dao).connectedPeripheralChanged("AA:BB:CC:DD:EE:01")
+        assertEquals("AA:BB:CC:DD:EE:01", dao.devices["my-whoop"]!!.peripheralId)
+        assertTrue(dao.devices["my-whoop"]!!.lastSeenAt > 100)
+    }
+
+    /**
+     * The disconnect edge stamps too. Without it the card would report the time of the CONNECT, so an
+     * overnight session would read "Last seen 10 h ago" the instant the link dropped. The connect stamp is
+     * rewound here so the assertion can only pass on the disconnect's own write.
+     */
+    @Test
+    fun aDisconnectStampsTheRowAsSeenRatherThanLeavingTheConnectTime() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01")
+        }
+        val coordinator = coordinatorOver(dao)
+        coordinator.connectedPeripheralChanged("AA:BB:CC:DD:EE:01")
+        dao.devices["my-whoop"] = dao.devices["my-whoop"]!!.copy(lastSeenAt = 100)   // rewind the connect stamp
+        coordinator.connectedPeripheralChanged(null)
+        assertTrue("the disconnect edge must stamp the row itself",
+                   dao.devices["my-whoop"]!!.lastSeenAt > 100)
+    }
+
+    /** A null republish with no live link is not a sighting — nothing was ever connected to record. */
+    @Test
+    fun aNullWithNoPriorConnectIsNotASighting() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01")
+        }
+        coordinatorOver(dao).connectedPeripheralChanged(null)
+        assertEquals(100, dao.devices["my-whoop"]!!.lastSeenAt)
+    }
+
+    /**
+     * The guard that matters on a multi-WHOOP install: a DIFFERENT strap connecting under this row must
+     * neither overwrite its identity nor claim to be a sighting OF IT. Same reasoning as the peripheralId
+     * guard above — that strap is not this row.
+     */
+    @Test
+    fun aDifferentStrapDoesNotStampThisRow() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01")
+        }
+        coordinatorOver(dao).connectedPeripheralChanged("AA:BB:CC:DD:EE:99")
+        assertEquals("AA:BB:CC:DD:EE:01", dao.devices["my-whoop"]!!.peripheralId)
+        assertEquals(100, dao.devices["my-whoop"]!!.lastSeenAt)
+    }
+
+    /**
+     * The disconnect stamps the strap that was CONNECTED, not whatever is active by the time it drops.
+     * Make another WHOOP active while this one is live — a multi-WHOOP install can — and resolving "the
+     * active row" would record a sighting of a strap nobody connected. That is the same mis-mapping the
+     * peripheralId guard exists to prevent, so the disconnect resolves by adopted identity instead.
+     */
+    @Test
+    fun theDisconnectStampsTheStrapThatWasConnectedNotWhateverIsActiveNow() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["whoop-a"] = whoopRow("whoop-a", peripheralId = "AA:BB:CC:DD:EE:01")
+            devices["whoop-b"] = whoopRow("whoop-b", peripheralId = "AA:BB:CC:DD:EE:02")
+                .copy(status = DeviceStatus.paired.name)
+        }
+        val coordinator = coordinatorOver(dao)
+        coordinator.connectedPeripheralChanged("AA:BB:CC:DD:EE:01")
+
+        // whoop-b becomes active while whoop-a is still the live link; rewind both stamps so the
+        // assertions can only be satisfied by the disconnect's own write.
+        dao.devices["whoop-a"] = dao.devices["whoop-a"]!!.copy(
+            status = DeviceStatus.paired.name, lastSeenAt = 100)
+        dao.devices["whoop-b"] = dao.devices["whoop-b"]!!.copy(
+            status = DeviceStatus.active.name, lastSeenAt = 100)
+
+        coordinator.connectedPeripheralChanged(null)
+
+        assertTrue("the strap that was connected must be the one stamped",
+                   dao.devices["whoop-a"]!!.lastSeenAt > 100)
+        assertEquals("a strap that was never connected must not be stamped",
+                     100, dao.devices["whoop-b"]!!.lastSeenAt)
+    }
+
+    /**
+     * The disconnect must resolve identity the way the connect path does. That comparison is
+     * `ignoreCase` — a stored address differing only in case is the SAME strap to the adopt guard — so a
+     * stricter lookup here would adopt on connect and then quietly stamp nothing on disconnect.
+     */
+    @Test
+    fun aDisconnectResolvesTheStrapCaseInsensitivelyLikeTheAdoptGuard() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "aa:bb:cc:dd:ee:01")
+        }
+        val coordinator = coordinatorOver(dao)
+        coordinator.connectedPeripheralChanged("AA:BB:CC:DD:EE:01")   // same strap, upper-cased
+        dao.devices["my-whoop"] = dao.devices["my-whoop"]!!.copy(lastSeenAt = 100)
+        coordinator.connectedPeripheralChanged(null)
+        assertTrue("a case-different address is the same strap to the adopt guard, so it must stamp",
+                   dao.devices["my-whoop"]!!.lastSeenAt > 100)
+    }
+
+    /** An address no row has adopted stamps nothing rather than guessing at the active row. */
+    @Test
+    fun aDisconnectFromAnUnadoptedStrapStampsNothing() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01")
+        }
+        val coordinator = coordinatorOver(dao)
+        coordinator.connectedPeripheralChanged("AA:BB:CC:DD:EE:77")   // different strap, never adopted
+        dao.devices["my-whoop"] = dao.devices["my-whoop"]!!.copy(lastSeenAt = 100)
+        coordinator.connectedPeripheralChanged(null)
+        assertEquals(100, dao.devices["my-whoop"]!!.lastSeenAt)
+    }
+
+    /**
+     * "Removed - data kept" is a deliberate resting state: a stray connect must not quietly resurrect an
+     * archived row into looking live. The real guard is the query's `AND status != 'archived'`; the fake
+     * reproduces it, so this pins the CONTRACT callers rely on rather than Room's SQL.
+     */
+    @Test
+    fun anArchivedRowIsNotStamped() = runBlocking {
+        val dao = FakeRegistryDao().apply {
+            devices["old-whoop"] = whoopRow("old-whoop", peripheralId = "AA:BB:CC:DD:EE:02")
+                .copy(status = DeviceStatus.archived.name)
+        }
+        registryWith(dao).touchLastSeen("old-whoop")
+        assertEquals(100, dao.devices["old-whoop"]!!.lastSeenAt)
+    }
 }