From 37e866320188ca5fcc329691646fa57a2955f709 Mon Sep 17 00:00:00 2001 From: sbalint Date: Mon, 24 Aug 2026 22:19:51 -0700 Subject: [PATCH] Fix inverted time factors in mixed glucose/time unit conversions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LoopUnit.conversionFactor applied a duration-style time factor (x1/60 for seconds to minutes) to the four conversions that change both the glucose unit and the time unit. Rates convert in the opposite direction, so each of these was off by 3600x: mg/dL·s -> mmol/L·min mg/dL·min -> mmol/L·s mmol/L·s -> mg/dL·min mmol/L·min -> mg/dL·s The same-glucose-unit rate cases were already correct, so only mmol/L users were affected. Most visibly this broke the carb effect chart. Insulin counteraction effects are stored in mg/dL·s and CarbEffectChart requests them in /min, so for mmol/L users the observed series rendered 3600x too small and collapsed onto zero, while the predicted carb effect series - which never uses a rate unit - rendered correctly. Dosing is unaffected; the algorithm paths stay in mg/dL throughout and never hit a mixed conversion. The existing tests asserted the same incorrect factors, so the suite passed. Those assertions are corrected, and testConversionFactorsMatchHealthKit now checks every unit pair against HealthKit's own dimensional analysis so the table cannot silently drift again. Co-Authored-By: Claude Opus 5 --- Sources/LoopAlgorithm/LoopUnit.swift | 9 ++--- Tests/LoopAlgorithmTests/LoopUnitTests.swift | 35 +++++++++++++++++--- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/Sources/LoopAlgorithm/LoopUnit.swift b/Sources/LoopAlgorithm/LoopUnit.swift index a97f965..370334b 100644 --- a/Sources/LoopAlgorithm/LoopUnit.swift +++ b/Sources/LoopAlgorithm/LoopUnit.swift @@ -73,18 +73,19 @@ public enum LoopUnit: Sendable, CaseIterable { (.milligramsPerDeciliterPerInternationalUnit, .millimolesPerLiterPerInternationalUnit): return 1/UnitMolarMassBloodGlucoseDivisible case (.milligramsPerDeciliterPerSecond, .millimolesPerLiterPerMinute): - return 1/UnitMolarMassBloodGlucoseDivisible / 60 - case (.milligramsPerDeciliterPerMinute, .millimolesPerLiterPerSecond): + // Rate: per-second -> per-minute multiplies by 60 (opposite of a duration conversion) return 1/UnitMolarMassBloodGlucoseDivisible * 60 + case (.milligramsPerDeciliterPerMinute, .millimolesPerLiterPerSecond): + return 1/UnitMolarMassBloodGlucoseDivisible / 60 case (.millimolesPerLiter, .milligramsPerDeciliter), (.millimolesPerLiterPerSecond, .milligramsPerDeciliterPerSecond), (.millimolesPerLiterPerMinute, .milligramsPerDeciliterPerMinute), (.millimolesPerLiterPerInternationalUnit, .milligramsPerDeciliterPerInternationalUnit): return UnitMolarMassBloodGlucoseDivisible case (.millimolesPerLiterPerSecond, .milligramsPerDeciliterPerMinute): - return UnitMolarMassBloodGlucoseDivisible / 60 - case (.millimolesPerLiterPerMinute, .milligramsPerDeciliterPerSecond): return UnitMolarMassBloodGlucoseDivisible * 60 + case (.millimolesPerLiterPerMinute, .milligramsPerDeciliterPerSecond): + return UnitMolarMassBloodGlucoseDivisible / 60 case (.gram, _), (.gramsPerUnit, _), (.internationalUnit, _), diff --git a/Tests/LoopAlgorithmTests/LoopUnitTests.swift b/Tests/LoopAlgorithmTests/LoopUnitTests.swift index 94def32..8c78cfe 100644 --- a/Tests/LoopAlgorithmTests/LoopUnitTests.swift +++ b/Tests/LoopAlgorithmTests/LoopUnitTests.swift @@ -190,18 +190,45 @@ class LoopUnitTests: XCTestCase { unit1 = LoopUnit.milligramsPerDeciliterPerMinute unit2 = LoopUnit.millimolesPerLiterPerSecond - XCTAssertEqual(unit1.conversionFactor(toUnit: unit2), 1/UnitMolarMassBloodGlucoseDivisible * 60) - XCTAssertEqual(unit2.conversionFactor(toUnit: unit1), UnitMolarMassBloodGlucoseDivisible / 60) + XCTAssertEqual(unit1.conversionFactor(toUnit: unit2), 1/UnitMolarMassBloodGlucoseDivisible / 60) + XCTAssertEqual(unit2.conversionFactor(toUnit: unit1), UnitMolarMassBloodGlucoseDivisible * 60) unit1 = LoopUnit.milligramsPerDeciliterPerSecond unit2 = LoopUnit.millimolesPerLiterPerMinute - XCTAssertEqual(unit1.conversionFactor(toUnit: unit2), 1/UnitMolarMassBloodGlucoseDivisible / 60) - XCTAssertEqual(unit2.conversionFactor(toUnit: unit1), UnitMolarMassBloodGlucoseDivisible * 60) + XCTAssertEqual(unit1.conversionFactor(toUnit: unit2), 1/UnitMolarMassBloodGlucoseDivisible * 60) + XCTAssertEqual(unit2.conversionFactor(toUnit: unit1), UnitMolarMassBloodGlucoseDivisible / 60) let quantity1 = LoopQuantity(unit: .millimolesPerLiter, doubleValue: 4) let quantity2 = LoopQuantity(unit: .milligramsPerDeciliter, doubleValue: 72.06235200002165) XCTAssertEqual(quantity1, quantity2) } + + /// The conversion table is maintained by hand, so check every pair against HealthKit, + /// which does the dimensional analysis for us. This guards against transcription errors + /// like the inverted time factor that made mg/dL·s -> mmol/L·min 3600x too small. + func testConversionFactorsMatchHealthKit() { + for source in LoopUnit.allCases { + for target in LoopUnit.allCases { + let context = "\(source.unitString) -> \(target.unitString)" + let one = HKQuantity(unit: HKUnit(from: source.unitString), doubleValue: 1) + let hkTarget = HKUnit(from: target.unitString) + let convertible = one.is(compatibleWith: hkTarget) + + guard let factor = source.conversionFactor(toUnit: target) else { + XCTAssertFalse(convertible, "\(context) is convertible in HealthKit but returned nil") + continue + } + + guard convertible else { + XCTFail("\(context) returned a factor but is not convertible in HealthKit") + continue + } + + let expected = one.doubleValue(for: hkTarget) + XCTAssertEqual(factor, expected, accuracy: max(abs(expected) * 1e-12, .ulpOfOne), context) + } + } + } } extension LoopUnitTests {