diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/HRVAnalyzer.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/HRVAnalyzer.swift index 99eb32fbc3..603aff60c4 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/HRVAnalyzer.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/HRVAnalyzer.swift @@ -808,8 +808,15 @@ public enum HRVAnalyzer { static func msToInt(_ ms: Double) -> Int { ms > 0 ? Int(ms + 0.5) : 0 } /// Whole-percent, integer half-up, so both platforms round a tie the same way. 0 when `total` is 0. + /// + /// The arithmetic is widened to `Int64` because `deliveryHistogram` passes BEAT-TIME IN + /// MILLISECONDS summed over a whole night, not a row count. `part * 200` needs more than 32 bits + /// above 10,737,418 ms (2.98 h of beat-time on multi-delivery seconds) and a real over-count night + /// carries 3-4x that. `Int` is 64-bit on iOS/macOS so this platform read correctly while the Kotlin + /// twin wrapped to a negative percentage; it is 32-bit on `arm64_32`, and StrandAnalytics already + /// builds for watchOS, so the width is stated rather than inherited. Twin of Kotlin `pct`. static func pct(_ part: Int, _ total: Int) -> Int { - total > 0 ? (part * 200 + total) / (total * 2) : 0 + total > 0 ? Int((Int64(part) * 200 + Int64(total)) / (Int64(total) * 2)) : 0 } /// #1008: a compact, deterministic RAW-ROW sample of the beats around the DENSEST second, for the diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HRVAnalyzerSampleOrdTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HRVAnalyzerSampleOrdTests.swift index c17a9cb062..83d6e8c6a7 100644 --- a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HRVAnalyzerSampleOrdTests.swift +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HRVAnalyzerSampleOrdTests.swift @@ -82,4 +82,13 @@ final class HRVAnalyzerSampleOrdTests: XCTestCase { XCTAssertEqual(HRVAnalyzer.pct(3, 8), 38) // 37.5 -> 38 XCTAssertEqual(HRVAnalyzer.pct(0, 0), 0) } + + /// Twin of Kotlin `pctSurvivesAWholeNightOfBeatTime`. `deliveryHistogram` passes a whole night's + /// beat-time in milliseconds, so `part * 200` needs more than 32 bits. `Int` is 64-bit here, so this + /// passed before the widening as well - it is a parity pin and a guard for `arm64_32`, where + /// StrandAnalytics also builds and where the Kotlin failure would reproduce exactly. + func testPctSurvivesAWholeNightOfBeatTime() { + XCTAssertEqual(HRVAnalyzer.pct(15_264_177, 35_498_088), 43) + XCTAssertEqual(HRVAnalyzer.pct(36_296_594, 39_886_368), 91) + } } diff --git a/android/app/src/main/java/com/noop/analytics/HrvAnalyzer.kt b/android/app/src/main/java/com/noop/analytics/HrvAnalyzer.kt index 686831112a..c82530ff4a 100644 --- a/android/app/src/main/java/com/noop/analytics/HrvAnalyzer.kt +++ b/android/app/src/main/java/com/noop/analytics/HrvAnalyzer.kt @@ -772,9 +772,17 @@ object HrvAnalyzer { */ internal fun msToInt(ms: Double): Int = if (ms > 0) (ms + 0.5).toInt() else 0 - /** Whole-percent, integer half-up, so both platforms round a tie the same way. 0 when [total] is 0. */ + /** Whole-percent, integer half-up, so both platforms round a tie the same way. 0 when [total] is 0. + * + * The arithmetic is widened to `Long` because [deliveryHistogram] passes BEAT-TIME IN MILLISECONDS + * summed over a whole night, not a row count. `part * 200` overflows a 32-bit `Int` above + * 2147483647 / 200 = 10,737,418 ms, i.e. 2.98 h of beat-time on multi-delivery seconds, and a real + * over-count night carries 3-4x that. Kotlin's `Int` wraps silently, so `multiMs` was reported + * NEGATIVE on every such night while the Swift twin (64-bit `Int`) reported the right value from the + * same source - the parity these two are supposed to hold. Widening the operation rather than the + * signature keeps every caller and the half-up tie behaviour untouched. */ internal fun pct(part: Int, total: Int): Int = - if (total > 0) (part * 200 + total) / (total * 2) else 0 + if (total > 0) ((part.toLong() * 200 + total) / (total.toLong() * 2)).toInt() else 0 /** * #1008: a compact, deterministic RAW-ROW sample of the beats around the DENSEST second, for the diff --git a/android/app/src/test/java/com/noop/analytics/HrvAnalyzerSampleOrdTest.kt b/android/app/src/test/java/com/noop/analytics/HrvAnalyzerSampleOrdTest.kt index 1b5bc4d3f3..6549aacbf1 100644 --- a/android/app/src/test/java/com/noop/analytics/HrvAnalyzerSampleOrdTest.kt +++ b/android/app/src/test/java/com/noop/analytics/HrvAnalyzerSampleOrdTest.kt @@ -102,4 +102,20 @@ class HrvAnalyzerSampleOrdTest { assertEquals(38, HrvAnalyzer.pct(3, 8)) assertEquals(0, HrvAnalyzer.pct(0, 0)) } + + /** + * `deliveryHistogram` feeds `pct` a whole night's BEAT-TIME IN MILLISECONDS, not a row count, and + * `part * 200` needs more than 32 bits past 10,737,418 ms. The numbers below are one real MG night + * (34,002 beats, meanNN 1044 ms, multiSec 23%): before the widening `part * 200` wrapped and this + * returned -16, a percentage of a positive subset of a positive total that cannot be negative. + * + * The existing cases above cannot reach it - the largest `rrMs` anywhere in this file is 1309.0 - so + * the overflow sat behind a green suite on Android while the 64-bit Swift twin read correctly. + */ + @Test fun pctSurvivesAWholeNightOfBeatTime() { + assertEquals(43, HrvAnalyzer.pct(15_264_177, 35_498_088)) + // The widest multi-delivery share in the same capture: 91% of a 39.9 M ms night (42,981 beats, + // meanNN 928 ms). Also wrapped to -16, which is what the log reported for that night. + assertEquals(91, HrvAnalyzer.pct(36_296_594, 39_886_368)) + } }