From 5d601d4104b1996790931bcd5141b8c26ae78fd2 Mon Sep 17 00:00:00 2001 From: Zebsi Date: Thu, 27 Aug 2026 11:58:34 +0200 Subject: [PATCH] hrv: widen pct's multiply so multiMs stops reporting a negative percentage `deliveryHistogram` calls `pct(msToInt(multiMs), msToInt(knownMs))`, where both arguments are beat-time in milliseconds summed over a whole night rather than a row count. `part * 200` was evaluated in a 32-bit `Int` and overflows above 2147483647 / 200 = 10,737,418 ms, i.e. 2.98 h of beat-time on multi-delivery seconds; a real over-count night carries three to four times that. Kotlin's `Int` wraps silently, so `multiMs` was reported negative on every such night. It is a share of a positive subset of a positive total and cannot be negative. Swift's `Int` is 64-bit on the platforms that run it today, so the same source computed the right answer there and the two twins disagreed. Widen the operation rather than the signature, so no caller changes and the integer half-up tie behaviour is untouched. The Swift twin states `Int64` explicitly rather than inheriting the platform width, since StrandAnalytics also builds for watchOS, where `Int` is 32-bit. The same guard already exists thirty lines below in `duplicatePairRatios` and again in `V18AuxCodec`; `pct` predates both. Adds a regression case at night scale to the file that already pins `pct`. The existing tie cases are unchanged by the widening. --- .../Sources/StrandAnalytics/HRVAnalyzer.swift | 9 ++++++++- .../HRVAnalyzerSampleOrdTests.swift | 9 +++++++++ .../main/java/com/noop/analytics/HrvAnalyzer.kt | 12 ++++++++++-- .../noop/analytics/HrvAnalyzerSampleOrdTest.kt | 16 ++++++++++++++++ 4 files changed, 43 insertions(+), 3 deletions(-) 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)) + } }