Integrate upstream 12.1.0 prerelease into the Apple test version

Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift

@@ -201,6 +201,68 @@ public enum Whoop5EcgProbe {
         }
     }
 
+    /// How long one run listens, in seconds, chosen by what started it.
+    ///
+    /// `capture` is 60 rather than 30 because of @meta1971's field timing in #891: across five MG
+    /// sessions the strap's terminal frame (classifier state 2, progress 100) landed 38 to 39 s after the
+    /// first live frame, so a 30 s window could not reach a completed reading on ANY strap or firmware.
+    /// The verdict sheet landing at 30 s made that worse than a missed deadline: Stop is the next control
+    /// a person reaches for, so the window trained people to end the session 8 to 9 s before the
+    /// classifier would have answered, and the probe then reported the silence it had caused.
+    ///
+    /// `selectOrStop` stays at 30. Those runs wait for a COMMAND_RESPONSE rather than for a reading, so
+    /// the longer window would be dead time in front of an answer that already arrived.
+    ///
+    /// `terminalGrace` is how long a run keeps listening AFTER the terminal frame instead of ending on
+    /// it. @meta1971 measured the variability field sitting at `0xffff` for 2 to 9 s past that frame
+    /// before it settled, so a probe that stopped on the terminal frame would pin the unset value as the
+    /// reading's own. It also means a run that ends early reports early: a terminal frame at 12 s renders
+    /// the verdict at 22 s rather than holding the sheet for the rest of the window.
+    public enum Window {
+        public static let selectOrStop = 30
+        public static let capture = 60
+        public static let terminalGrace = 10
+    }
+
+    /// Cap on the per-packet candidate lines one run records. Each line is rendered into both the report
+    /// and the strap log, so a chatty stream must not grow either without bound.
+    public static let maxCandidateLines = 12
+
+    /// Slots a run keeps in reserve for TERMINAL frames once `maxCandidateLines` is spent.
+    ///
+    /// R17 arrives at about one frame a second, so a flat cap keeps seconds 1 to 12 and discards
+    /// everything after. Per #891 the result, average HR and reason mask populate only on the terminal
+    /// frame, which means the flat cap threw away the single frame carrying the answer and kept twelve
+    /// that could not. A reserve rather than a bigger cap: the point is not more evidence, it is the RIGHT
+    /// evidence, and the ceiling stays bounded at `maxCandidateLines + maxTerminalCandidateLines`.
+    public static let maxTerminalCandidateLines = 4
+
+    /// Whether a decoded frame's line is kept, given what the run has already recorded.
+    ///
+    /// A terminal frame inside the first `maxCandidateLines` is kept by the ordinary rule and still spends
+    /// one reserve slot, so a run can never record more than the ceiling either way.
+    ///
+    /// Kotlin twin: `Whoop5EcgProbe.retainsCandidate`.
+    public static func retainsCandidate(linesKept: Int,
+                                        terminalLinesKept: Int,
+                                        isTerminal: Bool) -> Bool {
+        if linesKept < maxCandidateLines { return true }
+        return isTerminal && terminalLinesKept < maxTerminalCandidateLines
+    }
+
+    /// Seconds to wait before rendering the verdict once a terminal frame lands, or nil to leave the
+    /// run's existing deadline alone.
+    ///
+    /// ONCE per run (`alreadyExtended`), because the strap sends more than one terminal frame: extending
+    /// on each would let a terminal stream push the verdict out indefinitely and the report would never
+    /// render.
+    ///
+    /// Kotlin twin: `Whoop5EcgProbe.terminalGraceSeconds`.
+    public static func terminalGraceSeconds(isTerminal: Bool, alreadyExtended: Bool) -> Int? {
+        guard isTerminal, !alreadyExtended else { return nil }
+        return Window.terminalGrace
+    }
+
     /// Classify the run.
     ///
     /// Order matters, and it puts the ATTESTED signals first: the COMMAND_RESPONSE result codes are

Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgProbeWindowTests.swift

@@ -0,0 +1,57 @@
+import XCTest
+@testable import WhoopProtocol
+
+/// #891 — the two rules that decide whether a run can OBSERVE a completed reading at all.
+///
+/// Both defects they pin were invisible to every existing test, because the probe's own report looked
+/// correct: the packet count was honest, the verdict wording was honest, and the run simply never
+/// reached the frame that carries the answer. @meta1971's field timing (terminal frame at 38 to 39 s
+/// across five MG sessions, variability unset for 2 to 9 s after it) is the evidence behind the numbers.
+///
+/// The Kotlin twin pins the same cases in `Whoop5EcgProbeWindowTest`.
+final class Whoop5EcgProbeWindowTests: XCTestCase {
+
+    func testCaptureWindowOutlastsTheMeasuredTerminalFrame() {
+        // The whole defect in one assertion: 30 s cannot reach a frame that lands at 38 to 39 s.
+        XCTAssertGreaterThan(Whoop5EcgProbe.Window.capture, 39)
+        XCTAssertEqual(Whoop5EcgProbe.Window.capture, 60)
+        // A run waiting on a COMMAND_RESPONSE is not waiting on a reading, so it keeps the short window.
+        XCTAssertEqual(Whoop5EcgProbe.Window.selectOrStop, 30)
+        // Long enough to outlast the measured 2-to-9 s unset variability window after the terminal frame.
+        XCTAssertGreaterThan(Whoop5EcgProbe.Window.terminalGrace, 9)
+        XCTAssertEqual(Whoop5EcgProbe.Window.terminalGrace, 10)
+    }
+
+    func testOrdinaryFramesFillTheCapAndThenStop() {
+        XCTAssertTrue(Whoop5EcgProbe.retainsCandidate(linesKept: 0, terminalLinesKept: 0,
+                                                      isTerminal: false))
+        XCTAssertTrue(Whoop5EcgProbe.retainsCandidate(linesKept: 11, terminalLinesKept: 0,
+                                                      isTerminal: false))
+        XCTAssertFalse(Whoop5EcgProbe.retainsCandidate(linesKept: 12, terminalLinesKept: 0,
+                                                       isTerminal: false))
+        XCTAssertFalse(Whoop5EcgProbe.retainsCandidate(linesKept: 99, terminalLinesKept: 0,
+                                                       isTerminal: false))
+    }
+
+    func testTerminalFramesAreKeptPastTheCapButStayBounded() {
+        // THE #891 CASE. R17 arrives about once a second, so at the terminal frame the ordinary cap has
+        // long since filled with seconds 1 to 12 — the twelve frames that carry no result.
+        XCTAssertTrue(Whoop5EcgProbe.retainsCandidate(linesKept: 12, terminalLinesKept: 0,
+                                                      isTerminal: true))
+        XCTAssertTrue(Whoop5EcgProbe.retainsCandidate(linesKept: 40, terminalLinesKept: 3,
+                                                      isTerminal: true))
+        // Bounded: the reserve is four slots, not an exemption.
+        XCTAssertFalse(Whoop5EcgProbe.retainsCandidate(linesKept: 40, terminalLinesKept: 4,
+                                                       isTerminal: true))
+        XCTAssertEqual(Whoop5EcgProbe.maxCandidateLines + Whoop5EcgProbe.maxTerminalCandidateLines, 16)
+    }
+
+    func testTheGraceExtensionHappensOncePerRun() {
+        XCTAssertEqual(Whoop5EcgProbe.terminalGraceSeconds(isTerminal: true, alreadyExtended: false), 10)
+        // A terminal STREAM must not be able to postpone the verdict indefinitely.
+        XCTAssertNil(Whoop5EcgProbe.terminalGraceSeconds(isTerminal: true, alreadyExtended: true))
+        // An ordinary frame leaves the run's deadline alone.
+        XCTAssertNil(Whoop5EcgProbe.terminalGraceSeconds(isTerminal: false, alreadyExtended: false))
+        XCTAssertNil(Whoop5EcgProbe.terminalGraceSeconds(isTerminal: false, alreadyExtended: true))
+    }
+}

Strand/BLE/BLEManager.swift

@@ -1093,10 +1093,21 @@ public final class BLEManager: NSObject, ObservableObject {
     /// Structural-triage hits: the empirical search for the packet TYPE these records arrive under.
     private var ecgProbeCandidates: [String] = []
     private var ecgProbePacketsSeen = 0
-    /// Non-nil while the listen window is open; drives `ecgProbeArmed`.
+    /// Non-nil while the listen window is open; drives `ecgProbeArmed`. A MARKER, not a deadline:
+    /// nothing compares the stamp to the clock, and the window's length lives only in the verdict
+    /// timer, so do not start reading this as the moment a run ends.
     private var ecgProbeDeadline: Date?
     /// Supersedes a previous run's pending verdict timer when the user taps again.
     private var ecgProbeRunToken = 0
+    /// When the run in flight began, so the report states the time actually listened rather than the
+    /// window asked for. A terminal frame ends a run early (`Whoop5EcgProbe.terminalGraceSeconds`), so
+    /// those two stopped being the same number.
+    private var ecgProbeStartedAt: Date?
+    /// Whether this run already pushed its deadline out on a terminal frame. Once only, or a terminal
+    /// stream would postpone the verdict forever.
+    private var ecgProbeTerminalExtended = false
+    /// How many TERMINAL candidate lines this run has kept, against the reserve the cap holds back.
+    private var ecgProbeTerminalLinesKept = 0
     private var clockRequested = false
     /// #700: retry count for GET_CLOCK when no correlation establishes before backfill. Capped at 3.
     private var clockRetries = 0
@@ -4299,10 +4310,12 @@ public final class BLEManager: NSObject, ObservableObject {
 
     /// Sentinel shown while a probe run is in flight (twin of the #592/#690 constants).
     public static let ecgProbeWaiting = "__waiting__"
-    /// How long the probe listens for ECG-shaped packets before rendering its verdict.
-    private static let ecgProbeWindow: TimeInterval = 30
-    /// Cap on recorded candidate-frame lines, so a chatty stream can't grow the report without bound.
-    private static let ecgProbeMaxCandidates = 12
+    /// How long a run that is waiting on a COMMAND_RESPONSE listens before rendering its verdict.
+    /// Derived from `Whoop5EcgProbe.Window` so the two platforms cannot hold different numbers: that
+    /// they both held 30 independently is how the window outlived the evidence against it (#891).
+    private static let ecgProbeWindow = TimeInterval(Whoop5EcgProbe.Window.selectOrStop)
+    /// The CAPTURE run's window. Longer, because a reading's terminal frame lands at 38 to 39 s.
+    private static let ecgProbeCaptureWindow = TimeInterval(Whoop5EcgProbe.Window.capture)
     /// Cap on recorded steps. A run sends at most three, so the headroom is for UNSOLICITED replies —
     /// which are inbound-driven and therefore need the same bound as the candidate list.
     private static let ecgProbeMaxSteps = 12
@@ -4452,7 +4465,7 @@ public final class BLEManager: NSObject, ObservableObject {
         sendEcgCommand(.toggleLabradorRawSave, arg: 1)
         ecgSendAbortHistorical()
         sendEcgCommand(.toggleLabradorDataGeneration, arg: Whoop5Ecg.ControlSignal.start.rawValue)
-        scheduleEcgProbeVerdict()
+        scheduleEcgProbeVerdict(after: BLEManager.ecgProbeCaptureWindow)
     }
 
     /// The START list's first member: ABORT_HISTORICAL_TRANSMITS (20), immediately ahead of `124`.
@@ -4511,7 +4524,15 @@ public final class BLEManager: NSObject, ObservableObject {
         }
         ecgStopOverride = true
         defer { ecgStopOverride = false }
-        if reportsResult { beginEcgProbeRun(clearingSteps: true) } else { ecgProbeSteps = [] }
+        // Read BEFORE the run is re-armed: an open window means the capture's verdict has not rendered
+        // yet, so its evidence has never been reported and must survive into this run. Once the verdict
+        // has rendered the lines are already out, and a later stop is a genuinely separate run.
+        let captureStillListening = ecgProbeArmed
+        if reportsResult {
+            beginEcgProbeRun(clearingSteps: true, keepingEvidence: captureStillListening)
+        } else {
+            ecgProbeSteps = []
+        }
         log("ECG probe: stopping ECG data generation and both streams")
         sendEcgCommand(.toggleLabradorDataGeneration, arg: Whoop5Ecg.ControlSignal.stop.rawValue)
         sendEcgCommand(.toggleLabradorRawSave, arg: 0)
@@ -4523,28 +4544,55 @@ public final class BLEManager: NSObject, ObservableObject {
     /// Clear the probe result (dialog dismissed).
     public func clearEcgProbe() { state.ecgProbe = nil }
 
-    private func beginEcgProbeRun(clearingSteps: Bool) {
+    /// `keepingEvidence` carries a capture's collected R17 lines, packet count and start time into the
+    /// STOP run that follows it, instead of clearing them with the steps.
+    ///
+    /// Without it the stop discards exactly what this probe exists to retain. The reading completes at
+    /// 38 to 39 s (#891) while the sheet still reads "waiting", so the natural next action is to stop a
+    /// session the user can tell has finished — and that wiped every candidate line, the terminal frame
+    /// among them. The 30 s window hid this by rendering the verdict before most people got there.
+    ///
+    /// The START TIME travels with the packets on purpose. The report states "packets seen in Ns", so
+    /// keeping a count from a 47 s capture under a 30 s stop window would attribute those packets to a
+    /// span that did not collect them.
+    private func beginEcgProbeRun(clearingSteps: Bool, keepingEvidence: Bool = false) {
         if clearingSteps {
             ecgProbeSteps = []
-            ecgProbeCandidates = []
-            ecgProbePacketsSeen = 0
+            if !keepingEvidence {
+                ecgProbeCandidates = []
+                ecgProbePacketsSeen = 0
+            }
         }
         ecgProbeRunToken &+= 1
-        ecgProbeDeadline = Date().addingTimeInterval(BLEManager.ecgProbeWindow)
+        if !keepingEvidence { ecgProbeStartedAt = Date() }
+        ecgProbeTerminalExtended = false
+        ecgProbeTerminalLinesKept = 0
+        // Armed from HERE rather than from `scheduleEcgProbeVerdict`, because the sends in between can
+        // draw a COMMAND_RESPONSE and the triage is gated on this. The VALUE is a marker only: nothing
+        // compares it to the clock, and the length of the window lives in the verdict timer alone, so
+        // there is one number to get right instead of two that can disagree.
+        ecgProbeDeadline = Date()
         state.ecgProbe = BLEManager.ecgProbeWaiting
     }
 
     /// Render the verdict once the listen window closes. BLE callbacks and this timer both run on the
     /// main queue, so the token check is race-free without a lock.
-    private func scheduleEcgProbeVerdict() {
+    private func scheduleEcgProbeVerdict(after window: TimeInterval = BLEManager.ecgProbeWindow) {
+        // Renews the armed marker as well as the timer: a terminal frame re-arms through here, and the
+        // two must not be able to drift apart.
+        ecgProbeDeadline = Date()
         let token = ecgProbeRunToken
-        DispatchQueue.main.asyncAfter(deadline: .now() + BLEManager.ecgProbeWindow) { [weak self] in
+        DispatchQueue.main.asyncAfter(deadline: .now() + window) { [weak self] in
             guard let self, self.ecgProbeRunToken == token else { return }
             self.ecgProbeDeadline = nil
+            // The time LISTENED, not the window asked for. A terminal frame cuts a run short, and the
+            // verdict text quotes this number ("no ECG packet arrived in Ns"), so quoting the request
+            // would have the report claim a window it did not run.
+            let listened = Int((Date().timeIntervalSince(self.ecgProbeStartedAt ?? Date())).rounded())
             let text = Whoop5EcgProbe.report(steps: self.ecgProbeSteps,
                                              ecgPacketsSeen: self.ecgProbePacketsSeen,
                                              candidateFrames: self.ecgProbeCandidates,
-                                             windowSeconds: Int(BLEManager.ecgProbeWindow))
+                                             windowSeconds: listened)
             self.log("ECG probe:\n\(text)")
             self.state.ecgProbe = text
         }
@@ -4617,7 +4665,25 @@ public final class BLEManager: NSObject, ObservableObject {
     private func noteEcgProbeCandidate(_ frame: [UInt8]) {
         guard frame.count >= 12, let packet = Whoop5Ecg.r17FromFrame(frame) else { return }
         ecgProbePacketsSeen += 1
-        guard ecgProbeCandidates.count < BLEManager.ecgProbeMaxCandidates else { return }
+        // A TERMINAL frame is kept even past the cap, and ends the run on a grace period rather than on
+        // the frame itself. Both halves come from #891: the result, average HR and reason mask populate
+        // only on that frame, so the old flat cap kept seconds 1 to 12 and discarded the one frame that
+        // carried the answer; and the variability field reads 0xffff for several seconds after it, so
+        // stopping on the frame would pin the unset value as the reading's own.
+        if let grace = Whoop5EcgProbe.terminalGraceSeconds(isTerminal: packet.isTerminal,
+                                                           alreadyExtended: ecgProbeTerminalExtended) {
+            ecgProbeTerminalExtended = true
+            // Re-arm rather than cancel: bumping the token strands the pending verdict timer, which is
+            // the same mechanism a second user tap already uses.
+            ecgProbeRunToken &+= 1
+            log("ECG probe: terminal frame (state=\(packet.classifierState) progress="
+                + "\(packet.progress.raw)) — listening \(grace)s more before the verdict")
+            scheduleEcgProbeVerdict(after: TimeInterval(grace))
+        }
+        guard Whoop5EcgProbe.retainsCandidate(linesKept: ecgProbeCandidates.count,
+                                              terminalLinesKept: ecgProbeTerminalLinesKept,
+                                              isTerminal: packet.isTerminal) else { return }
+        if packet.isTerminal { ecgProbeTerminalLinesKept += 1 }
         // The classifier byte is logged as a NUMBER, never as its token name: a strap log is a shareable
         // artefact, and no line in it should read like a clinical finding. The decoder maps the value
         // offline; the raw byte is lossless.
@@ -6193,6 +6259,9 @@ extension BLEManager: @preconcurrency CBCentralManagerDelegate {
         ecgProbeSteps = []
         ecgProbeCandidates = []
         ecgProbePacketsSeen = 0
+        ecgProbeStartedAt = nil
+        ecgProbeTerminalExtended = false
+        ecgProbeTerminalLinesKept = 0
         // The DIS strings belong to the link that just dropped; a stale variant must not keep an MG-only
         // capability unlocked for whatever connects next.
         state.whoop5Variant = nil