From 6db169b167558f7f551ca77cc0a582307b30e515 Mon Sep 17 00:00:00 2001 From: vishk23 <119831996+vishk23@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:24:42 -0400 Subject: [PATCH 1/4] fix(protocol): keep the Android v18 record_index in the unsigned domain `record_index@11` is an unsigned 32-bit field. The Kotlin decoder narrowed it with `toInt()`, and Kotlin's Int is 32-bit where Swift's is 64-bit, so a value with bit 31 set decoded to -1_062_772_323 on Android and 3_232_194_973 on Swift/macOS from byte-identical bytes. The per-platform fixture-hex tests cannot see this: the wire bytes are the same on both platforms and each suite asserts its own platform's answer, so the divergence lives entirely downstream of the bytes. PR #848 hit the same 32-vs-64-bit split in the v18 storage codec and had to re-widen there; this is the decoder key itself. Carried as a Long, matching the u32 reader's own return type. No main-code consumer reads the key (the only readers are tests and the field census), so the change is confined to the decoded map and its assertion. --- .../src/main/java/com/noop/protocol/HistoricalStreams.kt | 8 +++++++- .../java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/android/app/src/main/java/com/noop/protocol/HistoricalStreams.kt b/android/app/src/main/java/com/noop/protocol/HistoricalStreams.kt index 30f7ca7bb9..34a9885ea7 100644 --- a/android/app/src/main/java/com/noop/protocol/HistoricalStreams.kt +++ b/android/app/src/main/java/com/noop/protocol/HistoricalStreams.kt @@ -313,7 +313,13 @@ private fun decodeWhoop5Historical(frame: ByteArray): Map? { out["hist_version"] = version // @11 a per-record counter: +1 every record, independent of unix (advances across gaps); seen on // two straps. @11 is only the low byte — read the full u32 LE. - frame.histU32(11)?.let { out["record_index"] = it.toInt() } + // Emitted as a Long, NOT narrowed to Int: this is an UNSIGNED 32-bit field and Kotlin's Int is + // 32-bit while Swift's is 64-bit, so `toInt()` made a value with bit 31 set decode to + // -1_062_772_323 here and 3_232_194_973 in Swift from byte-identical bytes — a cross-platform + // decode divergence the per-platform fixture-hex tests cannot see (same bytes, each suite asserting + // its own answer). PR #848 hit the same 32-vs-64-bit split in the storage codec. Pinned on both + // platforms by the `whoop5_v18_synthetic_record_index_high_bit` fixture in decoder_oracle.json. + frame.histU32(11)?.let { out["record_index"] = it } frame.histU32(15)?.let { out["unix"] = it.toInt() } frame.histU8(22)?.let { out["heart_rate"] = it } val rrn = frame.histU8(23) ?: 0 diff --git a/android/app/src/test/java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt b/android/app/src/test/java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt index 2b0166be1c..ac7b6e8cc7 100644 --- a/android/app/src/test/java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt +++ b/android/app/src/test/java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt @@ -86,7 +86,10 @@ class Whoop5HistoricalDecodeTest { // Fields read off the same real worn frame, justified by observed behaviour (parity with the // Swift Whoop5HistoricalTests.testHistoricalV18ObservedFields). val p = decodeHistorical(bytes(wornV18), DeviceFamily.WHOOP5)!! - assertEquals(25443699, p["record_index"]) // @11 per-record counter + // Long, not Int: record_index is an unsigned u32 and Kotlin's Int is 32-bit while Swift's is + // 64-bit, so it is carried as a Long to keep the two decoders' output identical (see the + // decoder comment and the high-bit oracle fixture). + assertEquals(25443699L, p["record_index"]) // @11 per-record counter assertEquals(141, p["hr_quality_flags"]) // @36 flag byte (0x8D), NOT a fixed-point HR assertEquals(101, p["heart_rate_alt"]) // @37 duplicate HR (hr@22 = 102) assertEquals(170, p["step_cadence"]) // @59 cadence-like byte (raw) From 4dcd61119c00a5b854d78db20df9d64957ca7677 Mon Sep 17 00:00:00 2001 From: vishk23 <119831996+vishk23@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:25:00 -0400 Subject: [PATCH 2/4] test(protocol): extend the golden decoder oracle past frame decode `decoder_oracle.json` already made both decoders prove the same output against the same bytes, but it stopped at frame decode. It now pins three layers, each with a twin test in `DecoderOracleTests.swift` and `DecoderOracleTest.kt`, and the file stays byte-identical across the two copies. 1. Decoded VALUES, including the 32-vs-64-bit boundary. Adds `whoop5_v18_synthetic_record_index_high_bit` - a real worn frame with only the u32 `record_index@11` replaced by 0xC0A7619D and the CRC32 recomputed. Bit 31 is where Swift and Kotlin stop agreeing, and no real capture reaches it (observed lifetime counters are ~8-25 million), so it has to be synthesised to be covered at all. 2. Stream ASSEMBLY (`stream_batches`). Six batches run named fixture frames through `extractHistoricalStreams` and pin every stream's row count - zeros included, so a stream materialising on one platform only is a failure - plus the emptiness verdict and the #547 dropped-record counter. Frame decode agreeing does not imply assembly agrees. Because a batch fixture can only cover the streams it happens to populate, a constructive test also builds a one-stream batch per stream and asserts each is non-empty: `insert` early-returns on an empty batch, so a stream missing from the verdict is silent data loss on that platform. 3. A `coverage` manifest, so the oracle cannot quietly stop covering something. It pins every fixture and batch name, every stream, and every asserted decoder key mapped to HOW MANY frames assert it - the count, not just the name, so dropping a key from one frame while another still carries it is also a failure. Every non-derived key must additionally appear in a real decode, the check PR #848 needed after a rename silently broke an extractor. Both suites also assert the stream list equals the array-typed fields `Streams`/`StreamBatch` actually declare, so a new stream added on one platform without oracle coverage fails there. Batches use identity clock refs (the archive-replay seam), so no clock correction applies and they stay reproducible. Refs #647, #775. --- .../DecoderOracleTests.swift | 218 +++++++++++++++ .../Resources/decoder_oracle.json | 193 ++++++++++++- .../com/noop/protocol/DecoderOracleTest.kt | 257 ++++++++++++++++++ .../src/test/resources/decoder_oracle.json | 193 ++++++++++++- docs/CONTRIBUTING.md | 9 + 5 files changed, 868 insertions(+), 2 deletions(-) diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift index d68981a9ab..e97fe41b3c 100644 --- a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift @@ -13,12 +13,75 @@ import XCTest /// The fixture was seeded from existing in-repo test vectors (Whoop4HistoricalV24HardwareTests, /// Whoop4HistoricalV25Tests, Whoop5HistoricalTests and their Kotlin twins), so every expected value is /// already independently grounded , this test only proves the two decoders agree on it. +/// +/// The oracle pins THREE layers, each with a twin test in `DecoderOracleTest.kt` (#647, #775): +/// 1. `frames` , decoded VALUES. A per-platform fixture-hex test cannot catch a 32-vs-64-bit +/// or signedness divergence: the wire bytes are identical on both platforms and +/// each suite asserts its own answer. Comparing the decoded NUMBERS against one +/// shared expectation is what catches it (PR #848: a u32 with bit 31 set read +/// 3232194973 on Swift and -1062772323 on Kotlin). +/// 2. `stream_batches` , the ASSEMBLED shape. Frame decode agreeing does not imply assembly agrees , +/// PR #848 also shipped a Kotlin `StreamBatch.isEmpty` that omitted a stream +/// Swift's `Streams.isEmpty` included, and `insert` early-returns on empty, so +/// that batch banked nothing on Android alone. +/// 3. `coverage` , the self-defence manifest, so the oracle cannot silently stop covering +/// something. Deleting an `expect` key, a frame or a batch fails a test. final class DecoderOracleTests: XCTestCase { // Mirrors the JSON shape. `expect` is a heterogeneous map decoded leniently below. private struct Oracle: Decodable { let tolerance: Double + let coverage: Coverage let frames: [Frame] + let streamBatches: [StreamBatchCase] + private enum CodingKeys: String, CodingKey { + case tolerance, coverage, frames + case streamBatches = "stream_batches" + } + } + /// The self-defence manifest , see the `coverage._comment` block in the JSON. + private struct Coverage: Decodable { + /// Pinned decoder key -> how many fixture frames assert it. The COUNT, not just the name: a set + /// alone would let one frame quietly drop a key another frame still carries. + let fields: [String: Int] + let derivedFields: [String] + let frames: [String] + let batches: [String] + let streams: [String] + let nonStreamLists: [String] + private enum CodingKeys: String, CodingKey { + case fields, frames, batches, streams + case derivedFields = "derived_fields" + case nonStreamLists = "non_stream_lists" + } + } + /// One pinned stream-assembly case: named fixture frames in, one pinned `Streams` shape out. + private struct StreamBatchCase: Decodable { + let name: String + let family: String + let frames: [String] + let deviceClockRef: Int + let wallClockRef: Int + let sessionOldestUnix: Int? + let sessionNewestUnix: Int? + let expect: BatchExpect + private enum CodingKeys: String, CodingKey { + case name, family, frames, expect + case deviceClockRef = "device_clock_ref" + case wallClockRef = "wall_clock_ref" + case sessionOldestUnix = "session_oldest_unix" + case sessionNewestUnix = "session_newest_unix" + } + } + private struct BatchExpect: Decodable { + let isEmpty: Bool + let droppedImplausible: Int + let counts: [String: Int] + private enum CodingKeys: String, CodingKey { + case counts + case isEmpty = "is_empty" + case droppedImplausible = "dropped_implausible" + } } private struct Frame: Decodable { let name: String @@ -106,6 +169,161 @@ final class DecoderOracleTests: XCTestCase { } } + // MARK: - Layer 2: stream assembly + + /// Per-stream row counts + the emptiness verdict for each pinned batch. Frame decode agreeing does + /// not imply assembly agrees: `extractHistoricalStreams` gates each stream separately (bpm 0 writes + /// no HR row, skin temp has a thermal gate, v25 carries gravity but no HR), and the emptiness verdict + /// is what `insert` early-returns on. Every stream is pinned INCLUDING the zeros, so a stream that + /// materialises on one platform only is a failure rather than an unasserted extra. + func testOracleStreamBatchesAssembleToExpectedShape() throws { + let oracle = try loadOracle() + XCTAssertGreaterThan(oracle.streamBatches.count, 0, "no oracle stream batches loaded") + var byName: [String: Frame] = [:] + for f in oracle.frames { byName[f.name] = f } + + for batch in oracle.streamBatches { + let family: DeviceFamily = batch.family == "whoop5" ? .whoop5 : .whoop4 + let parsed: [ParsedFrame] = try batch.frames.map { name in + let f = try XCTUnwrap(byName[name], "\(batch.name): unknown fixture frame '\(name)'") + return parseFrame(hexToBytes(f.hex), family: family) + } + let s = extractHistoricalStreams(parsed, deviceClockRef: batch.deviceClockRef, + wallClockRef: batch.wallClockRef, + sessionOldestUnix: batch.sessionOldestUnix, + sessionNewestUnix: batch.sessionNewestUnix) + let got = Self.streamCounts(s) + for (stream, want) in batch.expect.counts { + let have = try XCTUnwrap(got[stream], "\(batch.name): unknown stream '\(stream)'") + XCTAssertEqual(have, want, "\(batch.name).\(stream) row count") + } + XCTAssertEqual(s.isEmpty, batch.expect.isEmpty, "\(batch.name): emptiness verdict") + XCTAssertEqual(s.droppedImplausible, batch.expect.droppedImplausible, + "\(batch.name): #547 dropped-record count") + // The verdict must also AGREE with the counts, so a platform whose isEmpty forgets a stream + // is caught even on a batch that wasn't written to target that stream. + XCTAssertEqual(s.isEmpty, got.values.allSatisfy { $0 == 0 }, + "\(batch.name): isEmpty disagrees with the per-stream counts") + } + } + + /// Row count per stream, keyed by the oracle's (and `Streams.CodingKeys`') wire names. + private static func streamCounts(_ s: Streams) -> [String: Int] { + [ + "hr": s.hr.count, "rr": s.rr.count, "spo2": s.spo2.count, "skin_temp": s.skinTemp.count, + "resp": s.resp.count, "gravity": s.gravity.count, "steps": s.steps.count, + "sleep_state": s.sleepState.count, "ppg_hr": s.ppgHr.count, + "ppg_waveform": s.ppgWaveform.count, "events": s.events.count, "battery": s.battery.count, + ] + } + + /// EVERY stream on its own makes the batch non-empty. This is the exhaustive form of the #848 bug: + /// Kotlin's `StreamBatch.isEmpty` omitted one stream, and because `insert` early-returns on an empty + /// batch, a batch carrying ONLY that stream banked nothing on Android. A batch fixture can only catch + /// that for the streams it happens to populate; constructing a one-stream batch per stream catches it + /// for all of them. The cases are keyed by the oracle's stream names and cross-checked against the + /// manifest, so a stream added to `Streams` without a case here fails `testDeclaredStreams…` below. + func testEmptinessVerdictCoversEveryStream() throws { + let oracle = try loadOracle() + let oneOf: [String: Streams] = [ + "hr": Streams(hr: [HRSample(ts: 1, bpm: 60)]), + "rr": Streams(rr: [RRInterval(ts: 1, rrMs: 900)]), + "spo2": Streams(spo2: [SpO2Sample(ts: 1, red: 1, ir: 1)]), + "skin_temp": Streams(skinTemp: [SkinTempSample(ts: 1, raw: 3000)]), + "resp": Streams(resp: [RespSample(ts: 1, raw: 3000)]), + "gravity": Streams(gravity: [GravitySample(ts: 1, x: 0, y: 0, z: 1)]), + "steps": Streams(steps: [StepSample(ts: 1, counter: 1)]), + "sleep_state": Streams(sleepState: [SleepStateSample(ts: 1, state: 2)]), + "ppg_hr": Streams(ppgHr: [PpgHrSample(ts: 1, bpm: 60, conf: 0.5)]), + "ppg_waveform": Streams(ppgWaveform: [PpgWaveformSample(ts: 1, samples: [1, 2])]), + "events": Streams(events: [WhoopEvent(ts: 1, kind: "BOOT", payload: [:])]), + "battery": Streams(battery: [BatterySample(ts: 1, soc: 50, mv: 3900)]), + ] + XCTAssertEqual(Set(oneOf.keys), Set(oracle.coverage.streams), + "one-stream cases and the oracle's stream manifest disagree") + XCTAssertTrue(Streams().isEmpty, "a Streams with no rows must be empty") + for (stream, s) in oneOf { + XCTAssertFalse(s.isEmpty, "Streams carrying only '\(stream)' must NOT be empty , " + + "isEmpty gates the insert, so a stream missing from the verdict is silent data loss") + XCTAssertEqual(Self.streamCounts(s).values.reduce(0, +), 1, + "'\(stream)' case must populate exactly one row in exactly one stream") + } + } + + // MARK: - Layer 3: the oracle defends itself + + /// The manifest and the fixtures must agree EXACTLY, in both directions. Without this, deleting an + /// `expect` key, a whole frame or a whole batch passes both suites and coverage silently shrinks. + /// Precedent: PR #848 needed a test that every storage slot's decoder key exists in a real decode, + /// because a rename had broken an extractor and nothing failed. + func testOracleCoverageManifestMatchesFixtures() throws { + let oracle = try loadOracle() + let cov = oracle.coverage + + XCTAssertEqual(oracle.frames.map(\.name), cov.frames, + "coverage.frames must list every fixture frame, in order") + XCTAssertEqual(Set(oracle.streamBatches.map(\.name)), Set(cov.batches), + "coverage.batches must list every stream batch") + + // Field -> how many frames assert it. Counting (not just naming) is what makes EVERY individual + // assertion load-bearing: dropping `heart_rate` from one frame leaves the key set unchanged. + var asserted: [String: Int] = [:] + for f in oracle.frames { for key in f.expect.keys { asserted[key, default: 0] += 1 } } + XCTAssertEqual(asserted, cov.fields, + "coverage.fields must name every 'expect' key and how many frames assert it") + XCTAssertTrue(Set(cov.derivedFields).isSubset(of: Set(cov.fields.keys)), + "coverage.derived_fields must be a subset of coverage.fields") + + // Every pinned batch pins EVERY stream, zeros included , otherwise "absent on one platform" + // and "not asserted" are indistinguishable. + for batch in oracle.streamBatches { + XCTAssertEqual(Set(batch.expect.counts.keys), Set(cov.streams), + "\(batch.name): counts must pin every stream in coverage.streams") + XCTAssertEqual(batch.expect.isEmpty, batch.expect.counts.values.allSatisfy { $0 == 0 }, + "\(batch.name): pinned is_empty contradicts the pinned counts") + } + + // Every non-derived pinned field must appear in at least one REAL decode. A field that no + // decoder emits any more would otherwise sit in the manifest looking covered. + let derived = Set(cov.derivedFields) + var seen = Set() + for f in oracle.frames { + let family: DeviceFamily = f.family == "whoop5" ? .whoop5 : .whoop4 + let parsed = parseFrame(hexToBytes(f.hex), family: family).parsed + seen.formUnion(parsed.keys) + } + for field in cov.fields.keys where !derived.contains(field) { + XCTAssertTrue(seen.contains(field), + "coverage.fields lists '\(field)' but no fixture decode emits that key") + } + } + + /// The oracle's stream manifest must equal the array-typed fields `Streams` actually declares. This + /// is the direction the manifest alone cannot cover: a NEW stream added to `Streams` (and to the + /// emptiness verdict) but never given oracle counts would otherwise be invisible to every check + /// above. Diagnostics that are arrays but deliberately excluded from the verdict are listed in + /// `coverage.non_stream_lists`. The Kotlin twin reflects `StreamBatch` the same way. + func testDeclaredStreamsMatchOracleManifest() throws { + let oracle = try loadOracle() + let declared = Mirror(reflecting: Streams()).children.compactMap { child -> String? in + guard let label = child.label, child.value as? [Any] != nil else { return nil } + return Self.snakeCased(label) + } + XCTAssertEqual(Set(declared), + Set(oracle.coverage.streams).union(oracle.coverage.nonStreamLists), + "Streams declares array fields the oracle does not account for (or vice versa) , " + + "add the new stream to coverage.streams and to every batch's counts") + } + + /// `sleepState` -> `sleep_state`. Digits never take a separator, so `spo2` stays `spo2`. + private static func snakeCased(_ s: String) -> String { + var out = "" + for ch in s { + if ch.isUppercase { out.append("_"); out.append(Character(ch.lowercased())) } else { out.append(ch) } + } + return out + } + /// The Swift and Android copies of the oracle MUST be byte-identical, so neither platform can edit /// its fixture without the other. Compares the bundled Swift resource against the Android source /// copy (located relative to this test file). Skips gracefully if the Android tree isn't present diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json index 92da383d30..61bbfde271 100644 --- a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json @@ -13,9 +13,70 @@ "android/app/src/test/resources/decoder_oracle.json and a test in each suite asserts the two", "byte streams match. Doubles are compared within 1e-4; ints exact. A field absent from a record's", "'expect' map is simply not asserted (decoders legitimately emit supersets per family).", - "Frames carry only biometrics + timestamp (no serial / name / token), so they are safe to commit." + "Frames carry only biometrics + timestamp (no serial / name / token), so they are safe to commit.", + "", + "THIS FILE PINS THREE THINGS, each with a test in BOTH suites (issues #647, #775):", + " 1. 'frames' - the DECODED VALUES of a fixture frame. Same bytes in, same numbers out.", + " A fixture-hex test cannot catch a 32-vs-64-bit or signedness divergence,", + " because the wire bytes are identical on both platforms and each suite", + " asserts its own answer; comparing the decoded NUMBERS against one shared", + " expectation is what catches it.", + " 2. 'stream_batches' - the ASSEMBLED stream shape for a set of fixture frames: per-stream row", + " counts, the emptiness verdict, and the #547 dropped-record counter. Frame", + " decode agreeing does not imply assembly agrees.", + " 3. 'coverage' - the self-defence manifest. See its own comment." ], "tolerance": 1e-4, + "coverage": { + "_comment": [ + "SELF-DEFENCE MANIFEST. Every name this oracle claims to pin, listed once. Both suites assert", + "the manifest and the fixtures agree EXACTLY, in both directions, so silently deleting an", + "'expect' key, a frame or a batch fails a test instead of quietly shrinking coverage; adding one", + "requires listing it here. Precedent: PR #848 needed a test that every storage slot's decoder key", + "exists in a real decode, because a rename had silently broken an extractor and nothing failed.", + " fields - every key used in any frame's 'expect', mapped to HOW MANY fixture frames", + " assert it. The count, not just the name: a set would let one frame quietly", + " drop a key that another frame still carries, which is the same silent loss", + " of coverage one frame smaller. Each key must also appear in at least one", + " real decode (the #848 check), except the derived ones below.", + " derived_fields - names asserted here that are NOT decoder keys: 'gravity_mag' is computed", + " from gravity_x/y/z because the components are family-specific.", + " frames / batches - every fixture / batch name, so removing one is a failure.", + " streams - the persisted streams that define the emptiness verdict. Both suites also", + " assert this list equals the array-typed fields actually declared on", + " Streams (Swift) / StreamBatch (Kotlin), so a NEW stream added on one", + " platform without oracle coverage fails there.", + " non_stream_lists - array-typed fields on those types that are diagnostics, deliberately", + " excluded from the emptiness verdict." + ], + "fields": { + "activity_class": 1, "aux_byte_82": 1, "cardiac_flags": 1, "cardiac_status": 1, + "dynamic_acceleration": 6, "gravity_mag": 7, "heart_rate": 9, "heart_rate_alt": 4, + "hist_version": 12, "hr_quality_flags": 4, "motion_wear_quality": 1, "onwrist": 2, + "optical_amp_a": 4, "optical_amp_b": 4, "optical_baseline_a": 4, "optical_baseline_b": 4, + "record_index": 2, "resp_rate_raw": 1, "rr_count": 5, "rr_intervals": 8, "rr_packed": 1, + "skin_temp_raw": 5, "sleep_state": 2, "spo2_ir": 1, "spo2_red": 1, "status_word": 1, + "status_word_1": 2, "status_word_2": 2, "step_cadence": 1, "step_motion_counter": 1, + "temp_aux_1_raw": 2, "temp_aux_2_raw": 2, "unix": 12, "unknown_f32_113": 1, + "wake_quality": 2 + }, + "derived_fields": ["gravity_mag"], + "frames": [ + "whoop4_v24_real_worn", "whoop4_v24_synthetic_anchor", "whoop4_v25_real_a", "whoop4_v25_real_b", + "whoop4_v25_real_c", "whoop5_v18_real_worn", "whoop5_v18_real_one_rr", + "whoop5_v18_real_offwrist", "whoop5_v18_real_ack_capture", "whoop5_v18_real_device2_hr57", + "whoop5_v18_real_device2_hr63", "whoop5_v18_synthetic_record_index_high_bit" + ], + "batches": [ + "empty_input", "whoop4_v24_single", "whoop4_v25_gravity_only", "whoop5_v18_worn_pair", + "whoop5_v18_offwrist_single", "whoop5_v18_outside_session_range" + ], + "streams": [ + "hr", "rr", "spo2", "skin_temp", "resp", "gravity", "steps", "sleep_state", "ppg_hr", + "ppg_waveform", "events", "battery" + ], + "non_stream_lists": ["dropped_rtc_events"] + }, "frames": [ { "name": "whoop4_v24_real_worn", @@ -215,6 +276,136 @@ "heart_rate": 63, "dynamic_acceleration": 0.008421 } + }, + { + "name": "whoop5_v18_synthetic_record_index_high_bit", + "family": "whoop5", + "synthetic": true, + "note": [ + "whoop5_v18_real_worn with ONLY the u32 record_index@11 replaced by 0xC0A7619D (bit 31 set)", + "and the CRC32 trailer recomputed; every other byte is the real capture, which is why the", + "unix/heart_rate anchors below are identical to that frame's.", + "WHY: a u32 whose high bit is set is where Swift and Kotlin stop agreeing. Swift's Int is", + "64-bit, Kotlin's is 32-bit, so a decoder that narrows the value reads 3232194973 on one", + "platform and -1062772323 on the other from byte-identical bytes. That divergence is invisible", + "to a per-platform fixture-hex test (the wire bytes match and each suite asserts its own", + "answer) and it shipped in PR #848's first pass. Asserting the same decimal literal on both", + "sides is what catches it. No real capture reaches bit 31 (the observed lifetime counters are", + "~8-25 million), so the value has to be synthesised to be covered at all." + ], + "hex": "aa01740001003fb12f12809d61a7c0b69f266a66460066025a0265020000000000007b0a8d656463ff0012163cf6a439bf2924fd3ed763fe3e3200aa000000000000000000f7000901f10b0007010c020c00000000000000000000000000000000000000000000000100656f1e1e0000009d61a7c000000074fc28e0", + "expect": { + "record_index": 3232194973, + "hist_version": 18, + "unix": 1780916150, + "heart_rate": 102, + "rr_count": 2, + "rr_intervals": [602, 613] + } + } + ], + "stream_batches": [ + { + "_comment": [ + "STREAM ASSEMBLY. Each batch runs the named fixture frames through extractHistoricalStreams", + "and pins the resulting Streams (Swift) / StreamBatch (Kotlin): every stream's row count", + "(zeros included, so a stream appearing on one platform only is a failure), the emptiness", + "verdict, and the #547 dropped-record counter. Agreement on frame decode does not imply", + "agreement on assembly: PR #848 shipped a Kotlin StreamBatch.isEmpty that omitted a stream", + "Swift's included, and insert() early-returns on empty, so that batch banked nothing on", + "Android alone. Both platforms use identity clock refs (device_clock_ref = wall_clock_ref = 0,", + "the archive-replay seam) so no clock correction is applied and the batches stay reproducible." + ], + "name": "empty_input", + "family": "whoop5", + "frames": [], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": true, + "dropped_implausible": 0, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 0, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop4_v24_single", + "family": "whoop4", + "frames": ["whoop4_v24_real_worn"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 1, "rr": 2, "spo2": 1, "skin_temp": 1, "resp": 1, "gravity": 1, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop4_v25_gravity_only", + "family": "whoop4", + "frames": ["whoop4_v25_real_a", "whoop4_v25_real_b", "whoop4_v25_real_c"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 3, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop5_v18_worn_pair", + "family": "whoop5", + "frames": ["whoop5_v18_real_worn", "whoop5_v18_real_one_rr"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 2, "rr": 3, "spo2": 0, "skin_temp": 2, "resp": 0, "gravity": 2, + "steps": 2, "sleep_state": 2, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop5_v18_offwrist_single", + "family": "whoop5", + "frames": ["whoop5_v18_real_offwrist"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 1, "resp": 0, "gravity": 1, + "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop5_v18_outside_session_range", + "family": "whoop5", + "frames": ["whoop5_v18_real_worn", "whoop5_v18_real_one_rr"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "session_oldest_unix": 1700000000, + "session_newest_unix": 1700100000, + "expect": { + "is_empty": true, + "dropped_implausible": 2, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 0, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } } ] } diff --git a/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt b/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt index 2035267c35..28e8b9264c 100644 --- a/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt +++ b/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt @@ -1,7 +1,21 @@ package com.noop.protocol +import com.noop.data.BatteryRow +import com.noop.data.EventEntry +import com.noop.data.GravityRow +import com.noop.data.HrRow +import com.noop.data.PpgHrRow +import com.noop.data.PpgWaveformRow +import com.noop.data.RespRow +import com.noop.data.RrRow +import com.noop.data.SkinTempRow +import com.noop.data.SleepStateRow +import com.noop.data.Spo2Row +import com.noop.data.StepRow +import com.noop.data.StreamBatch import org.json.JSONObject import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Test @@ -21,6 +35,19 @@ import kotlin.math.sqrt * The fixture was seeded from existing in-repo test vectors (Whoop4HistoricalV25Test, * Whoop5HistoricalDecodeTest and their Swift twins), so every expected value is already independently * grounded , this test only proves the two decoders agree on it. + * + * The oracle pins THREE layers, each with a twin test in `DecoderOracleTests.swift` (#647, #775): + * 1. `frames` , decoded VALUES. A per-platform fixture-hex test cannot catch a 32-vs-64-bit or + * signedness divergence: the wire bytes are identical on both platforms and each + * suite asserts its own answer. Comparing the decoded NUMBERS against one shared + * expectation is what catches it (PR #848: a u32 with bit 31 set read + * -1062772323 here and 3232194973 in Swift). + * 2. `stream_batches` , the ASSEMBLED shape. Frame decode agreeing does not imply assembly agrees , + * PR #848 also shipped a [StreamBatch.isEmpty] that omitted a stream Swift's + * `Streams.isEmpty` included, and `insert` early-returns on empty, so that batch + * banked nothing on Android alone. + * 3. `coverage` , the self-defence manifest, so the oracle cannot silently stop covering + * something. Deleting an `expect` key, a frame or a batch fails a test. */ class DecoderOracleTest { @@ -91,6 +118,236 @@ class DecoderOracleTest { } } + // MARK: - Layer 2: stream assembly + + /** + * Per-stream row counts + the emptiness verdict for each pinned batch. Frame decode agreeing does + * not imply assembly agrees: [extractHistoricalStreams] gates each stream separately (bpm 0 writes + * no HR row, skin temp has a thermal gate, v25 carries gravity but no HR), and the emptiness verdict + * is what `insert` early-returns on. Every stream is pinned INCLUDING the zeros, so a stream that + * materialises on one platform only is a failure rather than an unasserted extra. + */ + @Test + fun oracleStreamBatchesAssembleToExpectedShape() { + val oracle = loadOracle() + val batches = oracle.getJSONArray("stream_batches") + assertTrue("no oracle stream batches loaded", batches.length() > 0) + val framesByName = HashMap() + val frames = oracle.getJSONArray("frames") + for (i in 0 until frames.length()) { + val f = frames.getJSONObject(i) + framesByName[f.getString("name")] = f + } + + for (i in 0 until batches.length()) { + val spec = batches.getJSONObject(i) + val name = spec.getString("name") + val family = + if (spec.getString("family") == "whoop5") DeviceFamily.WHOOP5 else DeviceFamily.WHOOP4 + val names = spec.getJSONArray("frames") + val raw = (0 until names.length()).map { + val fname = names.getString(it) + val f = framesByName[fname] ?: throw AssertionError("$name: unknown fixture frame '$fname'") + hexToBytes(f.getString("hex")) + } + val batch = extractHistoricalStreams( + rawFrames = raw, + deviceClockRef = spec.getInt("device_clock_ref"), + wallClockRef = spec.getInt("wall_clock_ref"), + family = family, + sessionOldestUnix = if (spec.has("session_oldest_unix")) spec.getLong("session_oldest_unix") else null, + sessionNewestUnix = if (spec.has("session_newest_unix")) spec.getLong("session_newest_unix") else null, + ) + + val expect = spec.getJSONObject("expect") + val counts = expect.getJSONObject("counts") + val got = streamCounts(batch) + for (stream in counts.keys()) { + val have = got[stream] ?: throw AssertionError("$name: unknown stream '$stream'") + assertEquals("$name.$stream row count", counts.getInt(stream), have) + } + assertEquals("$name: emptiness verdict", expect.getBoolean("is_empty"), batch.isEmpty) + assertEquals( + "$name: #547 dropped-record count", + expect.getInt("dropped_implausible"), + batch.droppedImplausibleTs, + ) + // The verdict must also AGREE with the counts, so a platform whose isEmpty forgets a stream + // is caught even on a batch that wasn't written to target that stream. + assertEquals( + "$name: isEmpty disagrees with the per-stream counts", + got.values.all { it == 0 }, + batch.isEmpty, + ) + } + } + + /** Row count per stream, keyed by the oracle's (and Swift `Streams.CodingKeys`') wire names. */ + private fun streamCounts(b: StreamBatch): Map = mapOf( + "hr" to b.hr.size, "rr" to b.rr.size, "spo2" to b.spo2.size, "skin_temp" to b.skinTemp.size, + "resp" to b.resp.size, "gravity" to b.gravity.size, "steps" to b.steps.size, + "sleep_state" to b.sleepState.size, "ppg_hr" to b.ppgHr.size, + "ppg_waveform" to b.ppgWaveform.size, "events" to b.events.size, "battery" to b.battery.size, + ) + + /** + * EVERY stream on its own makes the batch non-empty. This is the exhaustive form of the #848 bug: + * [StreamBatch.isEmpty] omitted one stream, and because `insert` early-returns on an empty batch, a + * batch carrying ONLY that stream banked nothing on Android. A batch fixture can only catch that for + * the streams it happens to populate; constructing a one-stream batch per stream catches it for all + * of them. The cases are keyed by the oracle's stream names and cross-checked against the manifest, + * so a stream added to [StreamBatch] without a case here fails `declaredStreams…` below. + */ + @Test + fun emptinessVerdictCoversEveryStream() { + val oracle = loadOracle() + val oneOf = mapOf( + "hr" to StreamBatch(hr = listOf(HrRow(1L, 60))), + "rr" to StreamBatch(rr = listOf(RrRow(1L, 900))), + "spo2" to StreamBatch(spo2 = listOf(Spo2Row(1L, 1, 1))), + "skin_temp" to StreamBatch(skinTemp = listOf(SkinTempRow(1L, 3000))), + "resp" to StreamBatch(resp = listOf(RespRow(1L, 3000))), + "gravity" to StreamBatch(gravity = listOf(GravityRow(1L, 0.0, 0.0, 1.0))), + "steps" to StreamBatch(steps = listOf(StepRow(1L, 1))), + "sleep_state" to StreamBatch(sleepState = listOf(SleepStateRow(1L, 2))), + "ppg_hr" to StreamBatch(ppgHr = listOf(PpgHrRow(1L, 60, 0.5))), + "ppg_waveform" to StreamBatch(ppgWaveform = listOf(PpgWaveformRow(1L, listOf(1, 2)))), + "events" to StreamBatch(events = listOf(EventEntry(1L, "BOOT", "{}"))), + "battery" to StreamBatch(battery = listOf(BatteryRow(1L, 50.0, 3900))), + ) + assertEquals( + "one-stream cases and the oracle's stream manifest disagree", + stringSet(oracle.getJSONObject("coverage").getJSONArray("streams")), + oneOf.keys.toSortedSet(), + ) + assertTrue("a StreamBatch with no rows must be empty", StreamBatch().isEmpty) + for ((stream, batch) in oneOf) { + assertFalse( + "StreamBatch carrying only '$stream' must NOT be empty , isEmpty gates the insert, " + + "so a stream missing from the verdict is silent data loss", + batch.isEmpty, + ) + assertEquals( + "'$stream' case must populate exactly one row in exactly one stream", + 1, + streamCounts(batch).values.sum(), + ) + } + } + + // MARK: - Layer 3: the oracle defends itself + + /** + * The manifest and the fixtures must agree EXACTLY, in both directions. Without this, deleting an + * `expect` key, a whole frame or a whole batch passes both suites and coverage silently shrinks. + * Precedent: PR #848 needed a test that every storage slot's decoder key exists in a real decode, + * because a rename had broken an extractor and nothing failed. + */ + @Test + fun oracleCoverageManifestMatchesFixtures() { + val oracle = loadOracle() + val cov = oracle.getJSONObject("coverage") + val frames = oracle.getJSONArray("frames") + val batches = oracle.getJSONArray("stream_batches") + + val frameNames = (0 until frames.length()).map { frames.getJSONObject(it).getString("name") } + assertEquals( + "coverage.frames must list every fixture frame, in order", + (0 until cov.getJSONArray("frames").length()).map { cov.getJSONArray("frames").getString(it) }, + frameNames, + ) + assertEquals( + "coverage.batches must list every stream batch", + stringSet(cov.getJSONArray("batches")), + (0 until batches.length()).map { batches.getJSONObject(it).getString("name") }.toSortedSet(), + ) + + // Field -> how many frames assert it. Counting (not just naming) is what makes EVERY individual + // assertion load-bearing: dropping `heart_rate` from one frame leaves the key set unchanged. + val asserted = sortedMapOf() + for (i in 0 until frames.length()) { + for (key in frames.getJSONObject(i).getJSONObject("expect").keys()) { + asserted[key] = (asserted[key] ?: 0) + 1 + } + } + val fieldsObj = cov.getJSONObject("fields") + val fields = fieldsObj.keys().asSequence().associateWithTo(sortedMapOf()) { fieldsObj.getInt(it) } + assertEquals( + "coverage.fields must name every 'expect' key and how many frames assert it", + fields, + asserted, + ) + val derived = stringSet(cov.getJSONArray("derived_fields")) + assertTrue("coverage.derived_fields must be a subset of coverage.fields", fields.keys.containsAll(derived)) + + // Every pinned batch pins EVERY stream, zeros included , otherwise "absent on one platform" and + // "not asserted" are indistinguishable. + val streams = stringSet(cov.getJSONArray("streams")) + for (i in 0 until batches.length()) { + val spec = batches.getJSONObject(i) + val name = spec.getString("name") + val expect = spec.getJSONObject("expect") + val counts = expect.getJSONObject("counts") + assertEquals( + "$name: counts must pin every stream in coverage.streams", + streams, + counts.keys().asSequence().toSortedSet(), + ) + assertEquals( + "$name: pinned is_empty contradicts the pinned counts", + counts.keys().asSequence().all { counts.getInt(it) == 0 }, + expect.getBoolean("is_empty"), + ) + } + + // Every non-derived pinned field must appear in at least one REAL decode. A field that no + // decoder emits any more would otherwise sit in the manifest looking covered. + val seen = HashSet() + for (i in 0 until frames.length()) { + val f = frames.getJSONObject(i) + val family = + if (f.getString("family") == "whoop5") DeviceFamily.WHOOP5 else DeviceFamily.WHOOP4 + decodeHistorical(hexToBytes(f.getString("hex")), family)?.let { seen.addAll(it.keys) } + } + for (field in fields.keys - derived) { + assertTrue( + "coverage.fields lists '$field' but no fixture decode emits that key", + seen.contains(field), + ) + } + } + + /** + * The oracle's stream manifest must equal the list-typed fields [StreamBatch] actually declares. + * This is the direction the manifest alone cannot cover: a NEW stream added to [StreamBatch] (and to + * the emptiness verdict) but never given oracle counts would otherwise be invisible to every check + * above. Diagnostics that are lists but deliberately excluded from the verdict are listed in + * `coverage.non_stream_lists`. The Swift twin reflects `Streams` the same way. + */ + @Test + fun declaredStreamsMatchOracleManifest() { + val cov = loadOracle().getJSONObject("coverage") + val declared = StreamBatch::class.java.declaredFields + .filter { List::class.java.isAssignableFrom(it.type) } + .map { snakeCased(it.name) } + .toSortedSet() + val expected = (stringSet(cov.getJSONArray("streams")) + + stringSet(cov.getJSONArray("non_stream_lists"))).toSortedSet() + assertEquals( + "StreamBatch declares list fields the oracle does not account for (or vice versa) , " + + "add the new stream to coverage.streams and to every batch's counts", + expected, + declared, + ) + } + + private fun stringSet(a: org.json.JSONArray): java.util.SortedSet = + (0 until a.length()).map { a.getString(it) }.toSortedSet() + + /** `sleepState` -> `sleep_state`. Digits never take a separator, so `spo2` stays `spo2`. */ + private fun snakeCased(s: String): String = + s.map { if (it.isUpperCase()) "_${it.lowercaseChar()}" else "$it" }.joinToString("") + /** * The Android and Swift copies of the oracle MUST be byte-identical, so neither platform can edit * its fixture without the other. We can read the Android copy off the test classpath; the Swift diff --git a/android/app/src/test/resources/decoder_oracle.json b/android/app/src/test/resources/decoder_oracle.json index 92da383d30..61bbfde271 100644 --- a/android/app/src/test/resources/decoder_oracle.json +++ b/android/app/src/test/resources/decoder_oracle.json @@ -13,9 +13,70 @@ "android/app/src/test/resources/decoder_oracle.json and a test in each suite asserts the two", "byte streams match. Doubles are compared within 1e-4; ints exact. A field absent from a record's", "'expect' map is simply not asserted (decoders legitimately emit supersets per family).", - "Frames carry only biometrics + timestamp (no serial / name / token), so they are safe to commit." + "Frames carry only biometrics + timestamp (no serial / name / token), so they are safe to commit.", + "", + "THIS FILE PINS THREE THINGS, each with a test in BOTH suites (issues #647, #775):", + " 1. 'frames' - the DECODED VALUES of a fixture frame. Same bytes in, same numbers out.", + " A fixture-hex test cannot catch a 32-vs-64-bit or signedness divergence,", + " because the wire bytes are identical on both platforms and each suite", + " asserts its own answer; comparing the decoded NUMBERS against one shared", + " expectation is what catches it.", + " 2. 'stream_batches' - the ASSEMBLED stream shape for a set of fixture frames: per-stream row", + " counts, the emptiness verdict, and the #547 dropped-record counter. Frame", + " decode agreeing does not imply assembly agrees.", + " 3. 'coverage' - the self-defence manifest. See its own comment." ], "tolerance": 1e-4, + "coverage": { + "_comment": [ + "SELF-DEFENCE MANIFEST. Every name this oracle claims to pin, listed once. Both suites assert", + "the manifest and the fixtures agree EXACTLY, in both directions, so silently deleting an", + "'expect' key, a frame or a batch fails a test instead of quietly shrinking coverage; adding one", + "requires listing it here. Precedent: PR #848 needed a test that every storage slot's decoder key", + "exists in a real decode, because a rename had silently broken an extractor and nothing failed.", + " fields - every key used in any frame's 'expect', mapped to HOW MANY fixture frames", + " assert it. The count, not just the name: a set would let one frame quietly", + " drop a key that another frame still carries, which is the same silent loss", + " of coverage one frame smaller. Each key must also appear in at least one", + " real decode (the #848 check), except the derived ones below.", + " derived_fields - names asserted here that are NOT decoder keys: 'gravity_mag' is computed", + " from gravity_x/y/z because the components are family-specific.", + " frames / batches - every fixture / batch name, so removing one is a failure.", + " streams - the persisted streams that define the emptiness verdict. Both suites also", + " assert this list equals the array-typed fields actually declared on", + " Streams (Swift) / StreamBatch (Kotlin), so a NEW stream added on one", + " platform without oracle coverage fails there.", + " non_stream_lists - array-typed fields on those types that are diagnostics, deliberately", + " excluded from the emptiness verdict." + ], + "fields": { + "activity_class": 1, "aux_byte_82": 1, "cardiac_flags": 1, "cardiac_status": 1, + "dynamic_acceleration": 6, "gravity_mag": 7, "heart_rate": 9, "heart_rate_alt": 4, + "hist_version": 12, "hr_quality_flags": 4, "motion_wear_quality": 1, "onwrist": 2, + "optical_amp_a": 4, "optical_amp_b": 4, "optical_baseline_a": 4, "optical_baseline_b": 4, + "record_index": 2, "resp_rate_raw": 1, "rr_count": 5, "rr_intervals": 8, "rr_packed": 1, + "skin_temp_raw": 5, "sleep_state": 2, "spo2_ir": 1, "spo2_red": 1, "status_word": 1, + "status_word_1": 2, "status_word_2": 2, "step_cadence": 1, "step_motion_counter": 1, + "temp_aux_1_raw": 2, "temp_aux_2_raw": 2, "unix": 12, "unknown_f32_113": 1, + "wake_quality": 2 + }, + "derived_fields": ["gravity_mag"], + "frames": [ + "whoop4_v24_real_worn", "whoop4_v24_synthetic_anchor", "whoop4_v25_real_a", "whoop4_v25_real_b", + "whoop4_v25_real_c", "whoop5_v18_real_worn", "whoop5_v18_real_one_rr", + "whoop5_v18_real_offwrist", "whoop5_v18_real_ack_capture", "whoop5_v18_real_device2_hr57", + "whoop5_v18_real_device2_hr63", "whoop5_v18_synthetic_record_index_high_bit" + ], + "batches": [ + "empty_input", "whoop4_v24_single", "whoop4_v25_gravity_only", "whoop5_v18_worn_pair", + "whoop5_v18_offwrist_single", "whoop5_v18_outside_session_range" + ], + "streams": [ + "hr", "rr", "spo2", "skin_temp", "resp", "gravity", "steps", "sleep_state", "ppg_hr", + "ppg_waveform", "events", "battery" + ], + "non_stream_lists": ["dropped_rtc_events"] + }, "frames": [ { "name": "whoop4_v24_real_worn", @@ -215,6 +276,136 @@ "heart_rate": 63, "dynamic_acceleration": 0.008421 } + }, + { + "name": "whoop5_v18_synthetic_record_index_high_bit", + "family": "whoop5", + "synthetic": true, + "note": [ + "whoop5_v18_real_worn with ONLY the u32 record_index@11 replaced by 0xC0A7619D (bit 31 set)", + "and the CRC32 trailer recomputed; every other byte is the real capture, which is why the", + "unix/heart_rate anchors below are identical to that frame's.", + "WHY: a u32 whose high bit is set is where Swift and Kotlin stop agreeing. Swift's Int is", + "64-bit, Kotlin's is 32-bit, so a decoder that narrows the value reads 3232194973 on one", + "platform and -1062772323 on the other from byte-identical bytes. That divergence is invisible", + "to a per-platform fixture-hex test (the wire bytes match and each suite asserts its own", + "answer) and it shipped in PR #848's first pass. Asserting the same decimal literal on both", + "sides is what catches it. No real capture reaches bit 31 (the observed lifetime counters are", + "~8-25 million), so the value has to be synthesised to be covered at all." + ], + "hex": "aa01740001003fb12f12809d61a7c0b69f266a66460066025a0265020000000000007b0a8d656463ff0012163cf6a439bf2924fd3ed763fe3e3200aa000000000000000000f7000901f10b0007010c020c00000000000000000000000000000000000000000000000100656f1e1e0000009d61a7c000000074fc28e0", + "expect": { + "record_index": 3232194973, + "hist_version": 18, + "unix": 1780916150, + "heart_rate": 102, + "rr_count": 2, + "rr_intervals": [602, 613] + } + } + ], + "stream_batches": [ + { + "_comment": [ + "STREAM ASSEMBLY. Each batch runs the named fixture frames through extractHistoricalStreams", + "and pins the resulting Streams (Swift) / StreamBatch (Kotlin): every stream's row count", + "(zeros included, so a stream appearing on one platform only is a failure), the emptiness", + "verdict, and the #547 dropped-record counter. Agreement on frame decode does not imply", + "agreement on assembly: PR #848 shipped a Kotlin StreamBatch.isEmpty that omitted a stream", + "Swift's included, and insert() early-returns on empty, so that batch banked nothing on", + "Android alone. Both platforms use identity clock refs (device_clock_ref = wall_clock_ref = 0,", + "the archive-replay seam) so no clock correction is applied and the batches stay reproducible." + ], + "name": "empty_input", + "family": "whoop5", + "frames": [], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": true, + "dropped_implausible": 0, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 0, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop4_v24_single", + "family": "whoop4", + "frames": ["whoop4_v24_real_worn"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 1, "rr": 2, "spo2": 1, "skin_temp": 1, "resp": 1, "gravity": 1, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop4_v25_gravity_only", + "family": "whoop4", + "frames": ["whoop4_v25_real_a", "whoop4_v25_real_b", "whoop4_v25_real_c"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 3, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop5_v18_worn_pair", + "family": "whoop5", + "frames": ["whoop5_v18_real_worn", "whoop5_v18_real_one_rr"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 2, "rr": 3, "spo2": 0, "skin_temp": 2, "resp": 0, "gravity": 2, + "steps": 2, "sleep_state": 2, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop5_v18_offwrist_single", + "family": "whoop5", + "frames": ["whoop5_v18_real_offwrist"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 1, "resp": 0, "gravity": 1, + "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop5_v18_outside_session_range", + "family": "whoop5", + "frames": ["whoop5_v18_real_worn", "whoop5_v18_real_one_rr"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "session_oldest_unix": 1700000000, + "session_newest_unix": 1700100000, + "expect": { + "is_empty": true, + "dropped_implausible": 2, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 0, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } } ] } diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 0a7e9e9ae8..3aa1634918 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -500,6 +500,15 @@ Schema lives in `Packages/WhoopStore/Sources/WhoopStore/Database.swift` as a **v workout detection), and the CSV / Apple Health importers (including real-export tests). - **`Fixtures/`** holds a sample WHOOP export for the import tests; `StrandImport` test resources are bundled via the package's `Package.swift`. +- **The golden decoder oracle is where cross-platform decode parity is pinned.** `decoder_oracle.json` + lives in two byte-identical copies (`Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/` + and `android/app/src/test/resources/`) and both `DecoderOracleTests.swift` and `DecoderOracleTest.kt` + run the *same* assertions against it: decoded field VALUES per fixture frame, and the assembled + `Streams`/`StreamBatch` shape (per-stream row counts + the emptiness verdict) per fixture batch. + Pinning values rather than bytes is the point — the wire bytes are identical on both platforms, so + a 32-vs-64-bit or signedness split is invisible to a per-platform fixture-hex test. **Extend the + oracle rather than adding a parallel mechanism**; a `coverage` manifest in the file makes silently + dropping an assertion a test failure, so adding one means listing it there too. - **Prefer pure tests.** Because `WhoopProtocol`, `StrandAnalytics`, and `FrameRouter` are framework-free, you can (and should) cover new decode/routing/math with captured frames and fixtures rather than requiring a strap. From 5ad9bc45a602fcf0511dcd4b69d105a68efaf331 Mon Sep 17 00:00:00 2001 From: vishk23 <119831996+vishk23@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:52:35 -0400 Subject: [PATCH 3/4] fix(protocol): keep the Android historical unix in the unsigned domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `unix` is an unsigned 32-bit field at three separate decode sites (WHOOP 4 v24/v5 @11, WHOOP 4 v25 @11, WHOOP 5/MG v18 @15). All three narrowed it with `toInt()`, and Kotlin's Int is 32-bit where Swift's is 64-bit, so from 2038-01-19 — the second bit 31 first sets — Android decodes a NEGATIVE timestamp where macOS/iOS decodes 2147483648 from byte-identical bytes. For `unix` that is worse than a wrong number. The negative value fails the #547 `MIN_PLAUSIBLE_UNIX` floor in `extractHistoricalStreams`, so the record is dropped: every offloaded record disappears on Android while macOS/iOS keeps ingesting the same bytes, with the UI still reporting a healthy sync. That violates the cross-platform parity contract, and it is silent. Carried as a Long, matching the u32 reader's own return type. The widening alone is NOT sufficient: `intOrNull`'s `is Long -> v.toInt()` branch would have re-narrowed it right back at the read site, so this adds `longOrNull` and reads `unix` through it. Two downstream consumers move with it — `Backfiller`'s SpO2 RE dump (an `as? Int` cast that would otherwise miss on every record and silently stop dumping) and `Spo2ReTrace.recordLine`, whose Swift twin already takes a 64-bit Int, so widening restores the byte-identical-line promise rather than breaking it. The v26 PPG path is widened for consistency only; it was never wrong, because its single consumer re-widened with `and 0xFFFFFFFFL`. That mask is now unnecessary and is removed. It remains the one site the oracle cannot pin: v26 returns null from `decodeHistorical`, so no v26 frame can live in the fixture set. Pinned on both platforms by three new synthetic oracle frames, one per decode site, each its real source capture with only the four `unix` bytes replaced by 0x80000000 and the CRC32 trailer recomputed (the two families checksum different ranges). Three paired stream batches assert the record still assembles into rows instead of vanishing, which is the actual user-visible bug; they need a `wall_now` override because the records are ~12 years in the future, so this also adds that test seam to Swift's `extractHistoricalStreams` (Android already had it). Every production caller passes nil and behaviour is byte-identical. Verified: swift test 350 passed; ./gradlew testFullDebugUnitTest 3096 passed (SyncChipStateTest fails on pristine upstream/main, unrelated). Each of the three decoder lines and the `longOrNull` read was reverted in isolation to confirm the matching fixture fails. Co-Authored-By: Claude Opus 5 --- .../WhoopProtocol/HistoricalStreams.swift | 13 +- .../DecoderOracleTests.swift | 8 +- .../Resources/decoder_oracle.json | 149 +++++++++++++++++- .../java/com/noop/analytics/Spo2ReTrace.kt | 10 +- .../src/main/java/com/noop/ble/Backfiller.kt | 4 +- .../com/noop/protocol/HistoricalStreams.kt | 40 ++++- .../main/java/com/noop/protocol/Streams.kt | 13 ++ .../com/noop/protocol/DecoderOracleTest.kt | 9 +- .../noop/protocol/Whoop4HistoricalV25Test.kt | 10 +- .../protocol/Whoop5HistoricalDecodeTest.kt | 10 +- .../src/test/resources/decoder_oracle.json | 149 +++++++++++++++++- 11 files changed, 380 insertions(+), 35 deletions(-) diff --git a/Packages/WhoopProtocol/Sources/WhoopProtocol/HistoricalStreams.swift b/Packages/WhoopProtocol/Sources/WhoopProtocol/HistoricalStreams.swift index 5314674c6b..470b633cb3 100644 --- a/Packages/WhoopProtocol/Sources/WhoopProtocol/HistoricalStreams.swift +++ b/Packages/WhoopProtocol/Sources/WhoopProtocol/HistoricalStreams.swift @@ -122,7 +122,16 @@ public func extractHistoricalStreams(_ parsed: [ParsedFrame], // The pure package can't read prefs, so the app-layer caller (Backfiller / // archive replay) reads PuffinExperiment.ppgHrSubLagInterpEnabled and passes // it. Default false = byte-identical to today. Mirrors the Android arg. - subLagInterp: Bool = false) -> Streams { + subLagInterp: Bool = false, + // TEST SEAM (#547 gate's "now"): overrides the live-clock upper bound + // resolved just below. nil everywhere in production — every caller keeps + // the live clock and today's behaviour is byte-identical. It exists so a + // fixture can pin a record whose own timestamp is in the future relative + // to the machine running the test, which is the only way to cover the + // post-2038 (bit-31-set) unix domain at all. Android's + // `extractHistoricalStreams` already had this parameter; this is Swift + // catching up, so the two oracle harnesses can drive the gate identically. + wallNow wallNowOverride: Int? = nil) -> Streams { func wall(_ deviceTs: Int?) -> Int? { guard let d = deviceTs else { return nil } return wallClockRef + (d - deviceClockRef) @@ -144,7 +153,7 @@ public func extractHistoricalStreams(_ parsed: [ParsedFrame], // nor a paused live clock wrongly rejects a real record. The MIN_PLAUSIBLE_UNIX floor is unconditional // and still catches the far-past garbage in every caller. Genuine future garbage (pikapik's records // dated beyond now, the field's year-2081 overshoot) is still > now + FUTURE_MARGIN → dropped. (#547) - let wallNow = max(wallClockRef, Int(Date().timeIntervalSince1970)) + let wallNow = wallNowOverride ?? max(wallClockRef, Int(Date().timeIntervalSince1970)) // PRIMARY FIX (#547): a record's own decoded ts must be near "now". A bad-clock strap emits records // whose unix is scattered garbage (far-past, a bogus 2027, even future dates); trusted verbatim, one // polluted block was re-attributed to every day and a future row surfaced as "last night". Returns diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift index e97fe41b3c..daf3640ac0 100644 --- a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift @@ -64,6 +64,10 @@ final class DecoderOracleTests: XCTestCase { let wallClockRef: Int let sessionOldestUnix: Int? let sessionNewestUnix: Int? + /// Optional #547 future-bound override. Absent → the production default (the live clock). + /// A batch whose records are post-2038 MUST set it, or the live clock rejects them for an + /// unrelated reason and the batch asserts nothing. Kotlin passes the same value. + let wallNow: Int? let expect: BatchExpect private enum CodingKeys: String, CodingKey { case name, family, frames, expect @@ -71,6 +75,7 @@ final class DecoderOracleTests: XCTestCase { case wallClockRef = "wall_clock_ref" case sessionOldestUnix = "session_oldest_unix" case sessionNewestUnix = "session_newest_unix" + case wallNow = "wall_now" } } private struct BatchExpect: Decodable { @@ -191,7 +196,8 @@ final class DecoderOracleTests: XCTestCase { let s = extractHistoricalStreams(parsed, deviceClockRef: batch.deviceClockRef, wallClockRef: batch.wallClockRef, sessionOldestUnix: batch.sessionOldestUnix, - sessionNewestUnix: batch.sessionNewestUnix) + sessionNewestUnix: batch.sessionNewestUnix, + wallNow: batch.wallNow) let got = Self.streamCounts(s) for (stream, want) in batch.expect.counts { let have = try XCTUnwrap(got[stream], "\(batch.name): unknown stream '\(stream)'") diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json index 61bbfde271..c56fb043a3 100644 --- a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json @@ -51,13 +51,13 @@ ], "fields": { "activity_class": 1, "aux_byte_82": 1, "cardiac_flags": 1, "cardiac_status": 1, - "dynamic_acceleration": 6, "gravity_mag": 7, "heart_rate": 9, "heart_rate_alt": 4, - "hist_version": 12, "hr_quality_flags": 4, "motion_wear_quality": 1, "onwrist": 2, + "dynamic_acceleration": 6, "gravity_mag": 9, "heart_rate": 11, "heart_rate_alt": 4, + "hist_version": 15, "hr_quality_flags": 4, "motion_wear_quality": 1, "onwrist": 2, "optical_amp_a": 4, "optical_amp_b": 4, "optical_baseline_a": 4, "optical_baseline_b": 4, - "record_index": 2, "resp_rate_raw": 1, "rr_count": 5, "rr_intervals": 8, "rr_packed": 1, + "record_index": 3, "resp_rate_raw": 1, "rr_count": 7, "rr_intervals": 11, "rr_packed": 1, "skin_temp_raw": 5, "sleep_state": 2, "spo2_ir": 1, "spo2_red": 1, "status_word": 1, "status_word_1": 2, "status_word_2": 2, "step_cadence": 1, "step_motion_counter": 1, - "temp_aux_1_raw": 2, "temp_aux_2_raw": 2, "unix": 12, "unknown_f32_113": 1, + "temp_aux_1_raw": 2, "temp_aux_2_raw": 2, "unix": 15, "unknown_f32_113": 1, "wake_quality": 2 }, "derived_fields": ["gravity_mag"], @@ -65,11 +65,15 @@ "whoop4_v24_real_worn", "whoop4_v24_synthetic_anchor", "whoop4_v25_real_a", "whoop4_v25_real_b", "whoop4_v25_real_c", "whoop5_v18_real_worn", "whoop5_v18_real_one_rr", "whoop5_v18_real_offwrist", "whoop5_v18_real_ack_capture", "whoop5_v18_real_device2_hr57", - "whoop5_v18_real_device2_hr63", "whoop5_v18_synthetic_record_index_high_bit" + "whoop5_v18_real_device2_hr63", "whoop5_v18_synthetic_record_index_high_bit", + "whoop4_v24_synthetic_unix_high_bit", "whoop4_v25_synthetic_unix_high_bit", + "whoop5_v18_synthetic_unix_high_bit" ], "batches": [ "empty_input", "whoop4_v24_single", "whoop4_v25_gravity_only", "whoop5_v18_worn_pair", - "whoop5_v18_offwrist_single", "whoop5_v18_outside_session_range" + "whoop5_v18_offwrist_single", "whoop5_v18_outside_session_range", + "whoop4_v24_high_bit_unix_survives", "whoop4_v25_high_bit_unix_survives", + "whoop5_v18_high_bit_unix_survives" ], "streams": [ "hr", "rr", "spo2", "skin_temp", "resp", "gravity", "steps", "sleep_state", "ppg_hr", @@ -302,6 +306,77 @@ "rr_count": 2, "rr_intervals": [602, 613] } + }, + { + "name": "whoop4_v24_synthetic_unix_high_bit", + "family": "whoop4", + "synthetic": true, + "note": [ + "whoop4_v24_real_worn with ONLY the u32 unix@11 replaced by 0x80000000 = 2147483648 and the", + "CRC32 trailer recomputed (whoop4 checksums frame[4:length] into a trailer at frame[length]);", + "every other byte is the real capture, which is why the heart_rate/R-R/gravity anchors below", + "are identical to that frame's.", + "WHY 0x80000000: it is 2038-01-19T03:14:08Z, the exact second bit 31 of a unix u32 first sets.", + "That is the boundary where a decoder narrowing the u32 to Kotlin's 32-bit Int reads -2147483648", + "while Swift's 64-bit Int reads 2147483648, from byte-identical bytes. For `unix` specifically", + "the consequence is worse than a wrong number: the negative value fails the #547 MIN_PLAUSIBLE_UNIX", + "floor, so extractHistoricalStreams DROPS the record and the strap's entire banked history goes", + "silently missing on Android while macOS/iOS keeps ingesting it. The paired stream batch", + "`whoop4_v24_high_bit_unix_survives` pins that consequence; this frame pins the decoded value.", + "No real capture can reach bit 31 before 2038, so the value has to be synthesised to be covered." + ], + "hex": "aa6400a12f18054c1c0a02000000805037805418016d022b0234020000000000006b07ff0085593c1f65cebed7b3e63eb85a5f3f000080401f65cebed7b3e63eb85a5f3f500264025d03640229014009010c020c00000000000f0001c402000000000000ee3e8ed0", + "expect": { + "hist_version": 24, + "unix": 2147483648, + "heart_rate": 109, + "rr_count": 2, + "rr_intervals": [555, 564], + "gravity_mag": 1.0 + } + }, + { + "name": "whoop4_v25_synthetic_unix_high_bit", + "family": "whoop4", + "synthetic": true, + "note": [ + "whoop4_v25_real_a with ONLY the u32 unix@11 replaced by 0x80000000 and the CRC32 trailer", + "recomputed; every other byte is the real capture. Same boundary and same failure mode as", + "whoop4_v24_synthetic_unix_high_bit — this one covers the SEPARATE v25 decode path, which reads", + "unix at its own offset in its own branch and would have to be narrowed (or widened) on its own.", + "v25 stores no per-second HR (it is PPG-derived on-device), so gravity is the only stream it", + "yields; the paired batch `whoop4_v25_high_bit_unix_survives` pins that it still yields it." + ], + "hex": "aa50000c2f1900006800000000008020430900433103007e026502ba026c022eff70f996f879fad6fd8300d6017e0267027201be00290258030e05c507f00c030ead11cb15791500d2553c90030000000d237e0c", + "expect": { + "hist_version": 25, + "unix": 2147483648, + "rr_intervals": [], + "gravity_mag": 1.0 + } + }, + { + "name": "whoop5_v18_synthetic_unix_high_bit", + "family": "whoop5", + "synthetic": true, + "note": [ + "whoop5_v18_real_worn with ONLY the u32 unix@15 replaced by 0x80000000 and the CRC32 trailer", + "recomputed (whoop5 checksums frame[8:payloadEnd], payloadEnd = declaredLength + 8 - 4 — a", + "DIFFERENT range from whoop4's, which is why each family needs its own patched frame).", + "Same boundary and same failure mode as the two whoop4 frames; this covers the third and last", + "narrowing site, the WHOOP 5/MG v18 decoder. record_index below is the frame's REAL value,", + "unchanged, so this frame also proves the patch touched unix@15 and nothing else — the two u32", + "fields sit four bytes apart and a decoder reading the wrong one would surface here." + ], + "hex": "aa01740001003fb12f1280733d84010000008066460066025a0265020000000000007b0a8d656463ff0012163cf6a439bf2924fd3ed763fe3e3200aa000000000000000000f7000901f10b0007010c020c00000000000000000000000000000000000000000000000100656f1e1e0000009d61a7c000000074c9fb37", + "expect": { + "hist_version": 18, + "unix": 2147483648, + "record_index": 25443699, + "heart_rate": 102, + "rr_count": 2, + "rr_intervals": [602, 613] + } } ], "stream_batches": [ @@ -406,6 +481,68 @@ "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 } } + }, + { + "_comment": [ + "THE POST-2038 BATCHES. These three run the high-bit-unix frames through the SAME assembly the", + "batches above use, and they are where the u32-narrowing bug becomes visible as data loss rather", + "than as a wrong number. With `unix` narrowed to a 32-bit Int the timestamp goes negative, fails", + "the #547 MIN_PLAUSIBLE_UNIX floor, and every record is dropped: all counts 0, is_empty true,", + "dropped_implausible equal to the frame count. Carried as a Long it decodes to 2147483648 and the", + "record assembles normally — the counts below are exactly the ones the same frames produce today", + "with their real timestamps (whoop4_v24_single, whoop4_v25_gravity_only, and whoop5_v18_worn_pair", + "minus its second frame), because only the timestamp changed.", + "`wall_now` pins the #547 gate's future bound. Without it the bound is the live clock, every one", + "of these records is ~12 years in the future, and the batch would assert nothing about narrowing", + "— it would drop the records on BOTH platforms for an unrelated reason and pass. It is a test-only", + "seam on extractHistoricalStreams (nil/absent in production on both platforms)." + ], + "name": "whoop4_v24_high_bit_unix_survives", + "family": "whoop4", + "frames": ["whoop4_v24_synthetic_unix_high_bit"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "wall_now": 2147500000, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 1, "rr": 2, "spo2": 1, "skin_temp": 1, "resp": 1, "gravity": 1, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop4_v25_high_bit_unix_survives", + "family": "whoop4", + "frames": ["whoop4_v25_synthetic_unix_high_bit"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "wall_now": 2147500000, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 1, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop5_v18_high_bit_unix_survives", + "family": "whoop5", + "frames": ["whoop5_v18_synthetic_unix_high_bit"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "wall_now": 2147500000, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 1, "rr": 2, "spo2": 0, "skin_temp": 1, "resp": 0, "gravity": 1, + "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } } ] } diff --git a/android/app/src/main/java/com/noop/analytics/Spo2ReTrace.kt b/android/app/src/main/java/com/noop/analytics/Spo2ReTrace.kt index 643a17861c..b8fa40805e 100644 --- a/android/app/src/main/java/com/noop/analytics/Spo2ReTrace.kt +++ b/android/app/src/main/java/com/noop/analytics/Spo2ReTrace.kt @@ -30,12 +30,16 @@ object Spo2ReTrace { * One record's RE line: the mapped SpO2 channels + timestamp + layout version, then the FULL frame * hex (no prefix cap - a v24 record is ~84 B and the unmapped tail is exactly where a banked SpO2 * would sit). Absent channels render "null" so a channel-less record still proves what it lacks. - * Takes already-extracted ints (ConnectionTrace's primitive style, matching the Swift signature); + * Takes already-extracted numbers (ConnectionTrace's primitive style, matching the Swift signature); * the caller reads them off its decoded record map. + * + * [unix] is a LONG here where the other fields are Int: it is an unsigned u32 off the wire, and the + * Swift twin's `Int` is 64-bit, so narrowing it on this side would render a NEGATIVE unix in the dump + * past 2038-01-19 and break the byte-identical-line promise above. See `histU32` in HistoricalStreams. */ - fun recordLine(frame: ByteArray, version: Int?, unix: Int?, red: Int?, ir: Int?, skinRaw: Int?): String { + fun recordLine(frame: ByteArray, version: Int?, unix: Long?, red: Int?, ir: Int?, skinRaw: Int?): String { val hex = frame.joinToString("") { String.format("%02x", it.toInt() and 0xFF) } - fun f(v: Int?): String = v?.toString() ?: "null" + fun f(v: Any?): String = v?.toString() ?: "null" return "spo2re v=${f(version)} unix=${f(unix)} red=${f(red)} ir=${f(ir)} " + "skinRaw=${f(skinRaw)} len=${frame.size} raw=$hex" } diff --git a/android/app/src/main/java/com/noop/ble/Backfiller.kt b/android/app/src/main/java/com/noop/ble/Backfiller.kt index ec6888cf2e..24b4baecc9 100644 --- a/android/app/src/main/java/com/noop/ble/Backfiller.kt +++ b/android/app/src/main/java/com/noop/ble/Backfiller.kt @@ -409,7 +409,9 @@ class Backfiller( for (f in frames) { if (spo2Dumped >= com.noop.analytics.Spo2ReTrace.MAX_SAMPLES) break val d = decodeHistorical(f, family) ?: continue - val recUnix = d["unix"] as? Int ?: continue + // `as? Long`, not `as? Int`: the decoder carries unix in the unsigned domain, so an + // Int cast would miss on EVERY record and silently stop the dump. See `histU32`. + val recUnix = d["unix"] as? Long ?: continue connectionLog( com.noop.analytics.Spo2ReTrace.recordLine( frame = f, diff --git a/android/app/src/main/java/com/noop/protocol/HistoricalStreams.kt b/android/app/src/main/java/com/noop/protocol/HistoricalStreams.kt index 34a9885ea7..59aea6603e 100644 --- a/android/app/src/main/java/com/noop/protocol/HistoricalStreams.kt +++ b/android/app/src/main/java/com/noop/protocol/HistoricalStreams.kt @@ -106,6 +106,22 @@ private fun ByteArray.histI16(off: Int): Int? { return if (u >= 0x8000) u - 0x10000 else u } +/** + * Unsigned little-endian u32 -> Long. null when out of range. + * + * THE RETURN TYPE IS LOAD-BEARING: keep a u32 in the Long (unsigned) domain all the way to the + * decoded map. Kotlin's Int is 32-bit where Swift's is 64-bit, so `toInt()` on a value with bit 31 + * set decodes to a NEGATIVE number here and the full unsigned value in Swift from byte-identical + * bytes — a cross-platform divergence the per-platform fixture-hex tests cannot see (the wire bytes + * match and each suite asserts its own platform's answer). PR #848 hit this in the v18 storage codec + * and PR #869 in `record_index@11`; for `unix` the consequence is worse than a wrong number, because + * a negative timestamp fails the #547 `plausible()` floor and [extractHistoricalStreams] drops the + * record outright — total, silent history loss on Android only, from 2038-01-19 onward. + * + * Read a u32 into an Int ONLY where the field is provably bounded below 2^31; otherwise carry the + * Long. Reading it out of a parsed map needs `longOrNull`, NOT `intOrNull` — the latter re-narrows + * with `toInt()` and silently undoes this. + */ private fun ByteArray.histU32(off: Int): Long? { if (off + 4 > size) return null return (this[off].toLong() and 0xFFL) or @@ -224,7 +240,8 @@ fun decodeHistorical(frame: ByteArray, family: DeviceFamily = DeviceFamily.WHOOP if (version == 25 && frame.size >= 79) { val out = LinkedHashMap() out["hist_version"] = version - frame.histU32(11)?.let { out["unix"] = it.toInt() } + // Long, not Int — see [histU32]. Pinned by `whoop4_v25_synthetic_unix_high_bit`. + frame.histU32(11)?.let { out["unix"] = it } fun grav(off: Int): Double? { val u = frame.histU16(off) ?: return null return (if (u >= 32768) u - 65536 else u).toDouble() / 16384.0 // i16 LE, ±2 g full-scale @@ -255,7 +272,8 @@ fun decodeHistorical(frame: ByteArray, family: DeviceFamily = DeviceFamily.WHOOP out["hist_version"] = version // unix is the record's REAL unix seconds (no clock offset needed for type-47). - frame.histU32(layout.unixOff)?.let { out["unix"] = it.toInt() } + // Long, not Int — see [histU32]. Pinned by `whoop4_v24_synthetic_unix_high_bit`. + frame.histU32(layout.unixOff)?.let { out["unix"] = it } frame.histU8(layout.hrOff)?.let { out["heart_rate"] = it } val rrn = frame.histU8(layout.rrCountOff) ?: 0 out["rr_count"] = rrn @@ -320,7 +338,10 @@ private fun decodeWhoop5Historical(frame: ByteArray): Map? { // its own answer). PR #848 hit the same 32-vs-64-bit split in the storage codec. Pinned on both // platforms by the `whoop5_v18_synthetic_record_index_high_bit` fixture in decoder_oracle.json. frame.histU32(11)?.let { out["record_index"] = it } - frame.histU32(15)?.let { out["unix"] = it.toInt() } + // Same unsigned-domain rule, and here it is not merely a wrong number: a narrowed `unix` goes + // NEGATIVE past 2038-01-19, fails the #547 plausibility floor, and the record is dropped — see + // [histU32]. Pinned by `whoop5_v18_synthetic_unix_high_bit`. + frame.histU32(15)?.let { out["unix"] = it } frame.histU8(22)?.let { out["heart_rate"] = it } val rrn = frame.histU8(23) ?: 0 out["rr_count"] = rrn @@ -465,12 +486,14 @@ private fun decodeWhoop5Historical(frame: ByteArray): Map? { * the raw per-burst counter @21 — `burst_index`, NOT a channel id; PR#553) and the footer after [75] * are intentionally not mapped here: the Android offload path needs only [unix] + the waveform for HR. */ -private data class V26Record(val unix: Int, val samples: List) +private data class V26Record(val unix: Long, val samples: List) private fun decodeWhoop5HistoricalV26(frame: ByteArray): V26Record? { if (frame.histU8(8) != PacketType.HISTORICAL_DATA.rawValue) return null if (frame.histU8(9) != 26) return null - val unix = frame.histU32(15)?.toInt() ?: return null + // Long, not Int — see [histU32]. This path was never actually wrong (its one consumer re-widened + // with `and 0xFFFFFFFFL`), but carrying the reader's own type removes the mask and the trap. + val unix = frame.histU32(15) ?: return null val samples = ArrayList(24) var off = 27 while (off < 75) { @@ -718,7 +741,7 @@ fun extractHistoricalStreams( decodeWhoop5HistoricalV26(frame)?.let { rec -> // #547: skip a v26 PPG buffer whose unix is implausible (correctedWall → null) so a // bad-clock strap can't seed the derived-HR estimator with garbage-timestamped samples. - val baseTs = correctedWall(rec.unix.toLong() and 0xFFFFFFFFL) + val baseTs = correctedWall(rec.unix) if (baseTs != null) { for (v in rec.samples) ppgSamples.add(PpgHr.Sample(ts = baseTs, value = v)) // Persist the raw waveform itself too (#156 follow-up), keyed on the record's @@ -734,7 +757,10 @@ fun extractHistoricalStreams( // #547: correctedWall is now nullable — it returns null for an implausible (far-past / // future-dated) record, so the `?: continue` below skips a bad-clock record entirely // instead of letting its garbage `unix` enter the DB and pollute the day-windowed analytics. - val ts = (p.intOrNull("unix")?.toLong())?.let { correctedWall(it) } ?: continue + // longOrNull, NOT intOrNull: the latter's `is Long -> v.toInt()` branch would re-narrow + // the u32 the decoder just carried in the unsigned domain, sending a post-2038 record + // negative and straight into the #547 drop below — silently, on Android only. See [histU32]. + val ts = p.longOrNull("unix")?.let { correctedWall(it) } ?: continue // skip startup hr=0 (matches Swift `bpm != 0`). p.intOrNull("heart_rate")?.let { bpm -> if (bpm != 0) hr.add(HrRow(ts, bpm)) } diff --git a/android/app/src/main/java/com/noop/protocol/Streams.kt b/android/app/src/main/java/com/noop/protocol/Streams.kt index 83e1c1862c..410e143413 100644 --- a/android/app/src/main/java/com/noop/protocol/Streams.kt +++ b/android/app/src/main/java/com/noop/protocol/Streams.kt @@ -275,6 +275,19 @@ internal fun Map.intOrNull(key: String): Int? = when (val v = this else -> null } +/** + * Read a parsed-map value in the LONG (unsigned-u32-safe) domain. Use this, never [intOrNull], for a + * field the decoder read with an unsigned u32 reader: [intOrNull]'s `is Long -> v.toInt()` branch + * narrows to Kotlin's 32-bit Int, which sends any value with bit 31 set negative and re-opens the + * cross-platform split against Swift's 64-bit Int. Swift needs no analogue — its `intValue` is + * already 64-bit, which is exactly why the divergence is Android-only. + */ +internal fun Map.longOrNull(key: String): Long? = when (val v = this[key]) { + is Long -> v + is Int -> v.toLong() + else -> null +} + internal fun Map.doubleOrNull(key: String): Double? = when (val v = this[key]) { is Double -> v is Int -> v.toDouble() diff --git a/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt b/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt index 28e8b9264c..8bc34f1c76 100644 --- a/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt +++ b/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt @@ -150,11 +150,18 @@ class DecoderOracleTest { val f = framesByName[fname] ?: throw AssertionError("$name: unknown fixture frame '$fname'") hexToBytes(f.getString("hex")) } + val wallClockRef = spec.getInt("wall_clock_ref") val batch = extractHistoricalStreams( rawFrames = raw, deviceClockRef = spec.getInt("device_clock_ref"), - wallClockRef = spec.getInt("wall_clock_ref"), + wallClockRef = wallClockRef, family = family, + // Optional #547 future-bound override. Absent → reproduce the production default + // verbatim, so the batches that don't set it behave exactly as before. A batch whose + // records are post-2038 MUST set it, or the live clock rejects them for an unrelated + // reason and the batch asserts nothing. Swift passes the same value. + wallNow = if (spec.has("wall_now")) spec.getLong("wall_now") + else maxOf(wallClockRef.toLong(), System.currentTimeMillis() / 1000L), sessionOldestUnix = if (spec.has("session_oldest_unix")) spec.getLong("session_oldest_unix") else null, sessionNewestUnix = if (spec.has("session_newest_unix")) spec.getLong("session_newest_unix") else null, ) diff --git a/android/app/src/test/java/com/noop/protocol/Whoop4HistoricalV25Test.kt b/android/app/src/test/java/com/noop/protocol/Whoop4HistoricalV25Test.kt index 7afb6592ab..8160527714 100644 --- a/android/app/src/test/java/com/noop/protocol/Whoop4HistoricalV25Test.kt +++ b/android/app/src/test/java/com/noop/protocol/Whoop4HistoricalV25Test.kt @@ -27,9 +27,11 @@ class Whoop4HistoricalV25Test { val p = decodeHistorical(rec, DeviceFamily.WHOOP4) assertNotNull("v25 record must decode (not rejected)", p) assertEquals(25, p!!["hist_version"]) - val unix = p["unix"] as? Int + // Long, not Int: `unix` is an unsigned u32 carried in the unsigned domain so a post-2038 + // value cannot go negative on Kotlin's 32-bit Int (see `histU32`). + val unix = p["unix"] as? Long assertNotNull(unix) - assertTrue(unix!! > 1_781_000_000) + assertTrue(unix!! > 1_781_000_000L) val gx = p["gravity_x"] as? Double assertNotNull("v25 must decode gravity (the sleep-staging input)", gx) val gy = (p["gravity_y"] as? Double) ?: 0.0 @@ -38,7 +40,7 @@ class Whoop4HistoricalV25Test { assertTrue("|gravity| ~1 g, got $mag", mag in 0.8..1.2) } // unix increments 1 Hz across the three. - val ts = records.map { decodeHistorical(it, DeviceFamily.WHOOP4)!!["unix"] as Int } + val ts = records.map { decodeHistorical(it, DeviceFamily.WHOOP4)!!["unix"] as Long } assertEquals(listOf(ts[0], ts[0] + 1, ts[0] + 2), ts) } @@ -48,7 +50,7 @@ class Whoop4HistoricalV25Test { } @Test fun v25ProducesGravityStream() { - val ref = decodeHistorical(records[0], DeviceFamily.WHOOP4)!!["unix"] as Int + val ref = (decodeHistorical(records[0], DeviceFamily.WHOOP4)!!["unix"] as Long).toInt() val streams = extractHistoricalStreams(records, deviceClockRef = ref, wallClockRef = ref) assertEquals(records.size, streams.gravity.size) } diff --git a/android/app/src/test/java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt b/android/app/src/test/java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt index ac7b6e8cc7..b2fe817e3e 100644 --- a/android/app/src/test/java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt +++ b/android/app/src/test/java/com/noop/protocol/Whoop5HistoricalDecodeTest.kt @@ -32,7 +32,9 @@ class Whoop5HistoricalDecodeTest { assertNotNull(p) p!! assertEquals(18, p["hist_version"]) - assertEquals(1780916150, p["unix"]) + // Long, not Int: `unix` is an unsigned u32 kept in the unsigned domain so a post-2038 value + // cannot go negative on Kotlin's 32-bit Int and get dropped by the #547 gate (see `histU32`). + assertEquals(1780916150L, p["unix"]) assertEquals(102, p["heart_rate"]) assertEquals(2, p["rr_count"]) assertEquals(listOf(602, 613), p["rr_intervals"]) @@ -236,7 +238,7 @@ class Whoop5HistoricalDecodeTest { assertNotNull(p) p!! assertEquals(18, p["hist_version"]) - assertEquals(1781047486, p["unix"]) + assertEquals(1781047486L, p["unix"]) assertEquals(66, p["heart_rate"]) assertEquals(3238, p["skin_temp_raw"]) // 32.38 °C on the wrist val gx = p["gravity_x"] as Double @@ -256,8 +258,8 @@ class Whoop5HistoricalDecodeTest { @Test fun decodesV18FromASecondDevice() { for ((hex, expectHR, expectUnix) in listOf( - Triple(secondDeviceHR57, 57, 1781120109), - Triple(secondDeviceHR63, 63, 1781124622), + Triple(secondDeviceHR57, 57, 1781120109L), + Triple(secondDeviceHR63, 63, 1781124622L), )) { val p = decodeHistorical(bytes(hex), DeviceFamily.WHOOP5) assertNotNull(p); p!! diff --git a/android/app/src/test/resources/decoder_oracle.json b/android/app/src/test/resources/decoder_oracle.json index 61bbfde271..c56fb043a3 100644 --- a/android/app/src/test/resources/decoder_oracle.json +++ b/android/app/src/test/resources/decoder_oracle.json @@ -51,13 +51,13 @@ ], "fields": { "activity_class": 1, "aux_byte_82": 1, "cardiac_flags": 1, "cardiac_status": 1, - "dynamic_acceleration": 6, "gravity_mag": 7, "heart_rate": 9, "heart_rate_alt": 4, - "hist_version": 12, "hr_quality_flags": 4, "motion_wear_quality": 1, "onwrist": 2, + "dynamic_acceleration": 6, "gravity_mag": 9, "heart_rate": 11, "heart_rate_alt": 4, + "hist_version": 15, "hr_quality_flags": 4, "motion_wear_quality": 1, "onwrist": 2, "optical_amp_a": 4, "optical_amp_b": 4, "optical_baseline_a": 4, "optical_baseline_b": 4, - "record_index": 2, "resp_rate_raw": 1, "rr_count": 5, "rr_intervals": 8, "rr_packed": 1, + "record_index": 3, "resp_rate_raw": 1, "rr_count": 7, "rr_intervals": 11, "rr_packed": 1, "skin_temp_raw": 5, "sleep_state": 2, "spo2_ir": 1, "spo2_red": 1, "status_word": 1, "status_word_1": 2, "status_word_2": 2, "step_cadence": 1, "step_motion_counter": 1, - "temp_aux_1_raw": 2, "temp_aux_2_raw": 2, "unix": 12, "unknown_f32_113": 1, + "temp_aux_1_raw": 2, "temp_aux_2_raw": 2, "unix": 15, "unknown_f32_113": 1, "wake_quality": 2 }, "derived_fields": ["gravity_mag"], @@ -65,11 +65,15 @@ "whoop4_v24_real_worn", "whoop4_v24_synthetic_anchor", "whoop4_v25_real_a", "whoop4_v25_real_b", "whoop4_v25_real_c", "whoop5_v18_real_worn", "whoop5_v18_real_one_rr", "whoop5_v18_real_offwrist", "whoop5_v18_real_ack_capture", "whoop5_v18_real_device2_hr57", - "whoop5_v18_real_device2_hr63", "whoop5_v18_synthetic_record_index_high_bit" + "whoop5_v18_real_device2_hr63", "whoop5_v18_synthetic_record_index_high_bit", + "whoop4_v24_synthetic_unix_high_bit", "whoop4_v25_synthetic_unix_high_bit", + "whoop5_v18_synthetic_unix_high_bit" ], "batches": [ "empty_input", "whoop4_v24_single", "whoop4_v25_gravity_only", "whoop5_v18_worn_pair", - "whoop5_v18_offwrist_single", "whoop5_v18_outside_session_range" + "whoop5_v18_offwrist_single", "whoop5_v18_outside_session_range", + "whoop4_v24_high_bit_unix_survives", "whoop4_v25_high_bit_unix_survives", + "whoop5_v18_high_bit_unix_survives" ], "streams": [ "hr", "rr", "spo2", "skin_temp", "resp", "gravity", "steps", "sleep_state", "ppg_hr", @@ -302,6 +306,77 @@ "rr_count": 2, "rr_intervals": [602, 613] } + }, + { + "name": "whoop4_v24_synthetic_unix_high_bit", + "family": "whoop4", + "synthetic": true, + "note": [ + "whoop4_v24_real_worn with ONLY the u32 unix@11 replaced by 0x80000000 = 2147483648 and the", + "CRC32 trailer recomputed (whoop4 checksums frame[4:length] into a trailer at frame[length]);", + "every other byte is the real capture, which is why the heart_rate/R-R/gravity anchors below", + "are identical to that frame's.", + "WHY 0x80000000: it is 2038-01-19T03:14:08Z, the exact second bit 31 of a unix u32 first sets.", + "That is the boundary where a decoder narrowing the u32 to Kotlin's 32-bit Int reads -2147483648", + "while Swift's 64-bit Int reads 2147483648, from byte-identical bytes. For `unix` specifically", + "the consequence is worse than a wrong number: the negative value fails the #547 MIN_PLAUSIBLE_UNIX", + "floor, so extractHistoricalStreams DROPS the record and the strap's entire banked history goes", + "silently missing on Android while macOS/iOS keeps ingesting it. The paired stream batch", + "`whoop4_v24_high_bit_unix_survives` pins that consequence; this frame pins the decoded value.", + "No real capture can reach bit 31 before 2038, so the value has to be synthesised to be covered." + ], + "hex": "aa6400a12f18054c1c0a02000000805037805418016d022b0234020000000000006b07ff0085593c1f65cebed7b3e63eb85a5f3f000080401f65cebed7b3e63eb85a5f3f500264025d03640229014009010c020c00000000000f0001c402000000000000ee3e8ed0", + "expect": { + "hist_version": 24, + "unix": 2147483648, + "heart_rate": 109, + "rr_count": 2, + "rr_intervals": [555, 564], + "gravity_mag": 1.0 + } + }, + { + "name": "whoop4_v25_synthetic_unix_high_bit", + "family": "whoop4", + "synthetic": true, + "note": [ + "whoop4_v25_real_a with ONLY the u32 unix@11 replaced by 0x80000000 and the CRC32 trailer", + "recomputed; every other byte is the real capture. Same boundary and same failure mode as", + "whoop4_v24_synthetic_unix_high_bit — this one covers the SEPARATE v25 decode path, which reads", + "unix at its own offset in its own branch and would have to be narrowed (or widened) on its own.", + "v25 stores no per-second HR (it is PPG-derived on-device), so gravity is the only stream it", + "yields; the paired batch `whoop4_v25_high_bit_unix_survives` pins that it still yields it." + ], + "hex": "aa50000c2f1900006800000000008020430900433103007e026502ba026c022eff70f996f879fad6fd8300d6017e0267027201be00290258030e05c507f00c030ead11cb15791500d2553c90030000000d237e0c", + "expect": { + "hist_version": 25, + "unix": 2147483648, + "rr_intervals": [], + "gravity_mag": 1.0 + } + }, + { + "name": "whoop5_v18_synthetic_unix_high_bit", + "family": "whoop5", + "synthetic": true, + "note": [ + "whoop5_v18_real_worn with ONLY the u32 unix@15 replaced by 0x80000000 and the CRC32 trailer", + "recomputed (whoop5 checksums frame[8:payloadEnd], payloadEnd = declaredLength + 8 - 4 — a", + "DIFFERENT range from whoop4's, which is why each family needs its own patched frame).", + "Same boundary and same failure mode as the two whoop4 frames; this covers the third and last", + "narrowing site, the WHOOP 5/MG v18 decoder. record_index below is the frame's REAL value,", + "unchanged, so this frame also proves the patch touched unix@15 and nothing else — the two u32", + "fields sit four bytes apart and a decoder reading the wrong one would surface here." + ], + "hex": "aa01740001003fb12f1280733d84010000008066460066025a0265020000000000007b0a8d656463ff0012163cf6a439bf2924fd3ed763fe3e3200aa000000000000000000f7000901f10b0007010c020c00000000000000000000000000000000000000000000000100656f1e1e0000009d61a7c000000074c9fb37", + "expect": { + "hist_version": 18, + "unix": 2147483648, + "record_index": 25443699, + "heart_rate": 102, + "rr_count": 2, + "rr_intervals": [602, 613] + } } ], "stream_batches": [ @@ -406,6 +481,68 @@ "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 } } + }, + { + "_comment": [ + "THE POST-2038 BATCHES. These three run the high-bit-unix frames through the SAME assembly the", + "batches above use, and they are where the u32-narrowing bug becomes visible as data loss rather", + "than as a wrong number. With `unix` narrowed to a 32-bit Int the timestamp goes negative, fails", + "the #547 MIN_PLAUSIBLE_UNIX floor, and every record is dropped: all counts 0, is_empty true,", + "dropped_implausible equal to the frame count. Carried as a Long it decodes to 2147483648 and the", + "record assembles normally — the counts below are exactly the ones the same frames produce today", + "with their real timestamps (whoop4_v24_single, whoop4_v25_gravity_only, and whoop5_v18_worn_pair", + "minus its second frame), because only the timestamp changed.", + "`wall_now` pins the #547 gate's future bound. Without it the bound is the live clock, every one", + "of these records is ~12 years in the future, and the batch would assert nothing about narrowing", + "— it would drop the records on BOTH platforms for an unrelated reason and pass. It is a test-only", + "seam on extractHistoricalStreams (nil/absent in production on both platforms)." + ], + "name": "whoop4_v24_high_bit_unix_survives", + "family": "whoop4", + "frames": ["whoop4_v24_synthetic_unix_high_bit"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "wall_now": 2147500000, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 1, "rr": 2, "spo2": 1, "skin_temp": 1, "resp": 1, "gravity": 1, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop4_v25_high_bit_unix_survives", + "family": "whoop4", + "frames": ["whoop4_v25_synthetic_unix_high_bit"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "wall_now": 2147500000, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 1, + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } + }, + { + "name": "whoop5_v18_high_bit_unix_survives", + "family": "whoop5", + "frames": ["whoop5_v18_synthetic_unix_high_bit"], + "device_clock_ref": 0, + "wall_clock_ref": 0, + "wall_now": 2147500000, + "expect": { + "is_empty": false, + "dropped_implausible": 0, + "counts": { + "hr": 1, "rr": 2, "spo2": 0, "skin_temp": 1, "resp": 0, "gravity": 1, + "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + } + } } ] } From b5cefb72bc436c74d6258057cba4c15a082b34aa Mon Sep 17 00:00:00 2001 From: vishk23 <119831996+vishk23@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:49:11 -0400 Subject: [PATCH 4/4] test(oracle): account for the v18_aux stream the manifest was missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `testDeclaredStreamsMatchOracleManifest` has been red since 2026-07-27. #848 added `v18_aux` to `Streams` / `StreamBatch` and to both platforms' emptiness verdicts, but not to this branch's oracle manifest, so the check that exists to catch exactly that — a new stream with no oracle counts — caught it. The manifest is the fix, not the test. `v18_aux` belongs in `coverage.streams` rather than `non_stream_lists` because both `Streams.isEmpty` (Streams.swift:646) and `StreamBatch.isEmpty` (WhoopRepository.kt:99) include it: it gates the insert, so a batch carrying only v18_aux rows must not read as empty. Counts are MEASURED, not chosen — each one is what `extractHistoricalStreams` actually produced over the pinned fixture frames: whoop5_v18_worn_pair 2 whoop5_v18_offwrist_single 1 whoop5_v18_high_bit_unix_survives 1 the other six batches 0 Both `streamCounts` helpers gain the stream, so the counts are compared against a real row count rather than silently absent. Both `emptinessVerdictCovers…` one-stream cases gain a `v18_aux` case, which is the half of the #848 defence that a fixture batch cannot provide. The one-stream case carries `recordIndex: 1` rather than a bare `ts`: `V18AuxSample.isEmpty` / `V18AuxRow.isEmpty` is "every slot is null", so an all-null row would still make the list non-empty and pass the check while proving nothing about a row that carries data. Both oracle copies stay byte-identical (`testOracleCopiesAreIdentical`). Swift: 445 tests, 0 failures. Kotlin: 3225 tests, 0 failures, 0 errors, 5 skipped across 393 classes (JUnit XML). DecoderOracleTest 6/6 on each side. --- .../DecoderOracleTests.swift | 7 ++++++- .../Resources/decoder_oracle.json | 20 +++++++++---------- .../com/noop/protocol/DecoderOracleTest.kt | 8 +++++++- .../src/test/resources/decoder_oracle.json | 20 +++++++++---------- 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift index daf3640ac0..ad9c36c3ce 100644 --- a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/DecoderOracleTests.swift @@ -219,7 +219,8 @@ final class DecoderOracleTests: XCTestCase { "hr": s.hr.count, "rr": s.rr.count, "spo2": s.spo2.count, "skin_temp": s.skinTemp.count, "resp": s.resp.count, "gravity": s.gravity.count, "steps": s.steps.count, "sleep_state": s.sleepState.count, "ppg_hr": s.ppgHr.count, - "ppg_waveform": s.ppgWaveform.count, "events": s.events.count, "battery": s.battery.count, + "ppg_waveform": s.ppgWaveform.count, "v18_aux": s.v18Aux.count, + "events": s.events.count, "battery": s.battery.count, ] } @@ -242,6 +243,10 @@ final class DecoderOracleTests: XCTestCase { "sleep_state": Streams(sleepState: [SleepStateSample(ts: 1, state: 2)]), "ppg_hr": Streams(ppgHr: [PpgHrSample(ts: 1, bpm: 60, conf: 0.5)]), "ppg_waveform": Streams(ppgWaveform: [PpgWaveformSample(ts: 1, samples: [1, 2])]), + // Carries a real slot value rather than a bare `ts`: `V18AuxSample.isEmpty` is "every slot is + // nil", so an all-nil sample would still make the ARRAY non-empty and pass this check while + // saying nothing about a row that carries data. + "v18_aux": Streams(v18Aux: [V18AuxSample(ts: 1, recordIndex: 1)]), "events": Streams(events: [WhoopEvent(ts: 1, kind: "BOOT", payload: [:])]), "battery": Streams(battery: [BatterySample(ts: 1, soc: 50, mv: 3900)]), ] diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json index c56fb043a3..2ac5e1297e 100644 --- a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/decoder_oracle.json @@ -77,7 +77,7 @@ ], "streams": [ "hr", "rr", "spo2", "skin_temp", "resp", "gravity", "steps", "sleep_state", "ppg_hr", - "ppg_waveform", "events", "battery" + "ppg_waveform", "v18_aux", "events", "battery" ], "non_stream_lists": ["dropped_rtc_events"] }, @@ -401,7 +401,7 @@ "dropped_implausible": 0, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 0, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -416,7 +416,7 @@ "dropped_implausible": 0, "counts": { "hr": 1, "rr": 2, "spo2": 1, "skin_temp": 1, "resp": 1, "gravity": 1, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -431,7 +431,7 @@ "dropped_implausible": 0, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 3, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -446,7 +446,7 @@ "dropped_implausible": 0, "counts": { "hr": 2, "rr": 3, "spo2": 0, "skin_temp": 2, "resp": 0, "gravity": 2, - "steps": 2, "sleep_state": 2, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 2, "sleep_state": 2, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 2, "events": 0, "battery": 0 } } }, @@ -461,7 +461,7 @@ "dropped_implausible": 0, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 1, "resp": 0, "gravity": 1, - "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 1, "events": 0, "battery": 0 } } }, @@ -478,7 +478,7 @@ "dropped_implausible": 2, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 0, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -508,7 +508,7 @@ "dropped_implausible": 0, "counts": { "hr": 1, "rr": 2, "spo2": 1, "skin_temp": 1, "resp": 1, "gravity": 1, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -524,7 +524,7 @@ "dropped_implausible": 0, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 1, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -540,7 +540,7 @@ "dropped_implausible": 0, "counts": { "hr": 1, "rr": 2, "spo2": 0, "skin_temp": 1, "resp": 0, "gravity": 1, - "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 1, "events": 0, "battery": 0 } } } diff --git a/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt b/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt index 8bc34f1c76..2cd58b2511 100644 --- a/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt +++ b/android/app/src/test/java/com/noop/protocol/DecoderOracleTest.kt @@ -13,6 +13,7 @@ import com.noop.data.SleepStateRow import com.noop.data.Spo2Row import com.noop.data.StepRow import com.noop.data.StreamBatch +import com.noop.data.V18AuxRow import org.json.JSONObject import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -194,7 +195,8 @@ class DecoderOracleTest { "hr" to b.hr.size, "rr" to b.rr.size, "spo2" to b.spo2.size, "skin_temp" to b.skinTemp.size, "resp" to b.resp.size, "gravity" to b.gravity.size, "steps" to b.steps.size, "sleep_state" to b.sleepState.size, "ppg_hr" to b.ppgHr.size, - "ppg_waveform" to b.ppgWaveform.size, "events" to b.events.size, "battery" to b.battery.size, + "ppg_waveform" to b.ppgWaveform.size, "v18_aux" to b.v18Aux.size, + "events" to b.events.size, "battery" to b.battery.size, ) /** @@ -219,6 +221,10 @@ class DecoderOracleTest { "sleep_state" to StreamBatch(sleepState = listOf(SleepStateRow(1L, 2))), "ppg_hr" to StreamBatch(ppgHr = listOf(PpgHrRow(1L, 60, 0.5))), "ppg_waveform" to StreamBatch(ppgWaveform = listOf(PpgWaveformRow(1L, listOf(1, 2)))), + // Carries a real slot value rather than a bare ts: V18AuxRow.isEmpty is "every slot is null", + // so an all-null row would still make the LIST non-empty and pass this check while saying + // nothing about a row that carries data. + "v18_aux" to StreamBatch(v18Aux = listOf(V18AuxRow(1L, recordIndex = 1L))), "events" to StreamBatch(events = listOf(EventEntry(1L, "BOOT", "{}"))), "battery" to StreamBatch(battery = listOf(BatteryRow(1L, 50.0, 3900))), ) diff --git a/android/app/src/test/resources/decoder_oracle.json b/android/app/src/test/resources/decoder_oracle.json index c56fb043a3..2ac5e1297e 100644 --- a/android/app/src/test/resources/decoder_oracle.json +++ b/android/app/src/test/resources/decoder_oracle.json @@ -77,7 +77,7 @@ ], "streams": [ "hr", "rr", "spo2", "skin_temp", "resp", "gravity", "steps", "sleep_state", "ppg_hr", - "ppg_waveform", "events", "battery" + "ppg_waveform", "v18_aux", "events", "battery" ], "non_stream_lists": ["dropped_rtc_events"] }, @@ -401,7 +401,7 @@ "dropped_implausible": 0, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 0, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -416,7 +416,7 @@ "dropped_implausible": 0, "counts": { "hr": 1, "rr": 2, "spo2": 1, "skin_temp": 1, "resp": 1, "gravity": 1, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -431,7 +431,7 @@ "dropped_implausible": 0, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 3, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -446,7 +446,7 @@ "dropped_implausible": 0, "counts": { "hr": 2, "rr": 3, "spo2": 0, "skin_temp": 2, "resp": 0, "gravity": 2, - "steps": 2, "sleep_state": 2, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 2, "sleep_state": 2, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 2, "events": 0, "battery": 0 } } }, @@ -461,7 +461,7 @@ "dropped_implausible": 0, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 1, "resp": 0, "gravity": 1, - "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 1, "events": 0, "battery": 0 } } }, @@ -478,7 +478,7 @@ "dropped_implausible": 2, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 0, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -508,7 +508,7 @@ "dropped_implausible": 0, "counts": { "hr": 1, "rr": 2, "spo2": 1, "skin_temp": 1, "resp": 1, "gravity": 1, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -524,7 +524,7 @@ "dropped_implausible": 0, "counts": { "hr": 0, "rr": 0, "spo2": 0, "skin_temp": 0, "resp": 0, "gravity": 1, - "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 0, "sleep_state": 0, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 0, "events": 0, "battery": 0 } } }, @@ -540,7 +540,7 @@ "dropped_implausible": 0, "counts": { "hr": 1, "rr": 2, "spo2": 0, "skin_temp": 1, "resp": 0, "gravity": 1, - "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "events": 0, "battery": 0 + "steps": 1, "sleep_state": 1, "ppg_hr": 0, "ppg_waveform": 0, "v18_aux": 1, "events": 0, "battery": 0 } } }