Merge pull request #614 from nightscout/oref-swift-iob-fixes

Fix testing JS for IoB and small bugs in Swift implementation

Trio/Sources/APS/OpenAPSSwift/Iob/IobGenerator.swift

@@ -9,13 +9,42 @@ struct IobGenerator {
     ) throws -> [IobResult] {
         // As a performance optimization, filter out any pump events
         // that occurred before the DIA would use it
-        let diaAgo = Double(profile.dia ?? 10) * 60 * 60
+        let durationOfInsulinActionAgo = Double(profile.dia ?? 10) * 60 * 60
         // add an extra two hours to the DIA to ensure we get all temp basals
-        let lastDia = clock - diaAgo - 2.hoursToSeconds
+        let lastDurationOfInsulinAction = clock - durationOfInsulinActionAgo - 2.hoursToSeconds
 
         // we have to keep all of our suspend/resume events due to a hardcoded
         // DIA value in dealing with suspended pumps in JS
-        let pumpHistory = history.filter({ $0.timestamp >= lastDia || $0.isSuspendOrResume() }).map({ $0.computedEvent() })
+        var pumpHistory = history.filter({ $0.timestamp >= lastDurationOfInsulinAction || $0.isSuspendOrResume() })
+            .map({ $0.computedEvent() })
+
+        // To make sure that lastTemp and lastBolusTime are filled in
+        // correctly, we need to check if there aren't any tempBasal or bolus
+        // events in the DIA-filtered list. If not, find the most recent one
+        // from the full history and add it.
+        if pumpHistory.filter({ $0.type == .tempBasal }).isEmpty {
+            // Find the most recent TempBasal event from before the DIA cutoff
+            let olderTempBasals = history.filter({ $0.type == .tempBasal && $0.timestamp < lastDurationOfInsulinAction })
+            if let lastTempBasal = olderTempBasals.max(by: { $0.timestamp < $1.timestamp }) {
+                // Find its matching TempBasalDuration (same timestamp)
+                if let matchingDuration = history
+                    .first(where: { $0.type == .tempBasalDuration && $0.timestamp == lastTempBasal.timestamp })
+                {
+                    pumpHistory.append(lastTempBasal.computedEvent())
+                    pumpHistory.append(matchingDuration.computedEvent())
+                }
+            }
+        }
+
+        // we need to check for amount != 0 to match the lastBolusTime logic
+        if pumpHistory.filter({ $0.type == .bolus && $0.amount != 0 }).isEmpty {
+            // Find the most recent Bolus event from before the DIA cutoff
+            let olderBoluses = history
+                .filter({ $0.type == .bolus && $0.amount != 0 && $0.timestamp < lastDurationOfInsulinAction })
+            if let lastBolus = olderBoluses.max(by: { $0.timestamp < $1.timestamp }) {
+                pumpHistory.append(lastBolus.computedEvent())
+            }
+        }
 
         let treatments = try IobHistory.calcTempTreatments(
             history: pumpHistory,

Trio/Sources/APS/OpenAPSSwift/Iob/IobHistory.swift

@@ -114,7 +114,11 @@ struct IobHistory {
     ///
     /// The algorithm just looks at time intervals from suspend events to resume events to calculate
     /// periods of suspension.
-    private static func getSuspends(pumpHistory: [ComputedPumpHistoryEvent], clock: Date) throws -> [PumpSuspended] {
+    private static func getSuspends(
+        pumpHistory: [ComputedPumpHistoryEvent],
+        profile: Profile,
+        clock: Date
+    ) throws -> [PumpSuspended] {
         let pumpSuspendResumeFull = pumpHistory.filter { $0.type == .pumpSuspend || $0.type == .pumpResume }
 
         // drop all repeated suspend / resume events to match JS
@@ -142,8 +146,12 @@ struct IobHistory {
         // If our first suspend/resume event is a resume, the pump is suspended
         // when our history begins
 
-        // this dia is hard-coded in Javascript, it doesn't use the profile
-        let maxDiaAgo = clock - 8.hoursToSeconds
+        var profileDia = profile.dia ?? 10
+        if profileDia < 5 {
+            profileDia = 5
+        }
+
+        let maxDiaAgo = clock - profileDia.hoursToSeconds
         if let first = pumpSuspendResume.first, first.type == .pumpResume, maxDiaAgo < first.timestamp {
             let start = maxDiaAgo
             let duration = first.timestamp.timeIntervalSince(start).secondsToMinutes
@@ -457,7 +465,7 @@ struct IobHistory {
         // ignore any records in the future and sort them
         let pumpHistory = history.filter({ $0.timestamp <= clock }).sorted { $0.timestamp < $1.timestamp }
         let tempBasals = try getTempBasals(pumpHistory: pumpHistory, clock: clock, zeroTempDuration: zeroTempDuration)
-        let suspends = try getSuspends(pumpHistory: pumpHistory, clock: clock)
+        let suspends = try getSuspends(pumpHistory: pumpHistory, profile: profile, clock: clock)
         let boluses = pumpHistory.filter({ $0.type == .bolus }).map { $0.copyWith(insulin: $0.amount) }
 
         var tempHistory: [ComputedPumpHistoryEvent]

Trio/Sources/APS/OpenAPSSwift/Logging/JSONCompare.swift

@@ -259,6 +259,7 @@ enum JSONCompare {
         var differences: [String: ValueDifference] = [:]
         let approximateKeys = function.approximateMatchingNumbers()
         let flexibleArrayKeys = function.flexibleArrayKeys()
+        let propertiesToSkip = function.propertiesToSkip()
 
         // Check all keys present in either dictionary
         Set(jsDict.keys).union(swiftDict.keys).forEach { key in
@@ -271,6 +272,7 @@ enum JSONCompare {
                 approximately: approximateKeys[key],
                 approximateKeys: approximateKeys,
                 flexibleArrayKeys: flexibleArrayKeys,
+                propertiesToSkip: propertiesToSkip,
                 currentPath: currentPath
             ) {
                 differences[currentPath] = ValueDifference(
@@ -317,6 +319,7 @@ enum JSONCompare {
         approximately: Double?,
         approximateKeys: [String: Double],
         flexibleArrayKeys: [String],
+        propertiesToSkip: Set<String>,
         currentPath: String
     ) -> Bool {
         switch (value1, value2) {
@@ -341,22 +344,28 @@ enum JSONCompare {
                 return zip(a1.prefix(shortestCount), a2.prefix(shortestCount)).allSatisfy { v1, v2 in
                     valuesAreEqual(
                         v1, v2, approximately: approximately, approximateKeys: approximateKeys,
-                        flexibleArrayKeys: flexibleArrayKeys, currentPath: currentPath
+                        flexibleArrayKeys: flexibleArrayKeys, propertiesToSkip: propertiesToSkip,
+                        currentPath: currentPath
                     )
                 }
             }
             return a1.count == a2.count && zip(a1, a2).allSatisfy { v1, v2 in
                 valuesAreEqual(
                     v1, v2, approximately: approximately, approximateKeys: approximateKeys,
-                    flexibleArrayKeys: flexibleArrayKeys, currentPath: currentPath
+                    flexibleArrayKeys: flexibleArrayKeys, propertiesToSkip: propertiesToSkip,
+                    currentPath: currentPath
                 )
             }
         case let (.object(o1), .object(o2)):
-            return o1.keys == o2.keys && o1.keys.allSatisfy { key in
+            // Filter out properties that should be skipped during comparison
+            let keys1 = Set(o1.keys).subtracting(propertiesToSkip)
+            let keys2 = Set(o2.keys).subtracting(propertiesToSkip)
+            return keys1 == keys2 && keys1.allSatisfy { key in
                 guard let v1 = o1[key], let v2 = o2[key] else { return false }
                 return valuesAreEqual(
                     v1, v2, approximately: approximateKeys[key], approximateKeys: approximateKeys,
-                    flexibleArrayKeys: flexibleArrayKeys, currentPath: "\(currentPath).\(key)"
+                    flexibleArrayKeys: flexibleArrayKeys, propertiesToSkip: propertiesToSkip,
+                    currentPath: "\(currentPath).\(key)"
                 )
             }
         default:

Trio/Sources/APS/OpenAPSSwift/Logging/OrefFunction.swift

@@ -92,10 +92,7 @@ enum OrefFunction: String, Codable {
                 "basaliob": 0.25,
                 "bolusiob": 0.25,
                 "netbasalinsulin": 0.25,
-                "bolusinsulin": 0.25,
-                // Please see this issue for context on duration:
-                // https://github.com/nightscout/Trio-dev/issues/453
-                "duration": 120
+                "bolusinsulin": 0.25
             ]
         case .meal:
             return [
@@ -147,4 +144,19 @@ enum OrefFunction: String, Codable {
             return []
         }
     }
+
+    /// Properties to skip during object comparison. Unlike keysToIgnore which filters
+    /// final differences, this skips properties during the recursive comparison itself.
+    /// This is needed for array return types where differences are recorded at the
+    /// element level rather than at individual property paths.
+    func propertiesToSkip() -> Set<String> {
+        switch self {
+        case .iob:
+            // Please see this issue for context on skipping lastTemp:
+            // https://github.com/nightscout/Trio-dev/issues/453
+            return Set(["lastTemp"])
+        default:
+            return Set()
+        }
+    }
 }

TrioTests/OpenAPSSwiftTests/IobJsonTests.swift

@@ -65,8 +65,10 @@ import Testing
             // The JS implementation of IoB when the pump is suspend is so fundamentally
             // broken that I wasn't able to fix it in JS. So we'll just skip these, but I
             // verified them by hand and the Swift implementation appears to be correct
-            if let mostRecentPumpEvent = iobInputs.history.filter({ $0.isExternal != true }).first {
-                if mostRecentPumpEvent.type == .pumpSuspend
+            if let mostRecentSuspendResumeEvent = iobInputs.history.filter({ $0.type == .pumpSuspend || $0.type == .pumpResume })
+                .first
+            {
+                if mostRecentSuspendResumeEvent.type == .pumpSuspend
                 {
                     print("Skipping, known issue with JS and currently suspended pumps")
                     continue
@@ -98,6 +100,27 @@ import Testing
             autosens: try JSONBridge.to(iobInputs.autosens)
         )
 
+        // In suspendedPrior mode (first suspend/resume event is a Resume), JS incorrectly
+        // returns pre-resume temp basals in lastTemp because history.js line 566 uses
+        // tempHistory instead of splitHistory. Swift correctly handles this case.
+        if case let .success(jsRawJson) = iobResultJavascript,
+           let jsIobEntries = try? JSONBridge.iobResult(from: jsRawJson),
+           let jsLastTempDate = jsIobEntries.first?.lastTemp?.date
+        {
+            let suspendResumeEvents = iobInputs.history
+                .filter { $0.type == .pumpSuspend || $0.type == .pumpResume }
+                .sorted { $0.timestamp < $1.timestamp }
+            if let firstEvent = suspendResumeEvents.first,
+               firstEvent.type == .pumpResume
+            {
+                let firstResumeTime = UInt64(firstEvent.timestamp.timeIntervalSince1970 * 1000)
+                if jsLastTempDate < firstResumeTime {
+                    print("Skipping, known issue with JS lastTemp in suspendedPrior mode")
+                    return
+                }
+            }
+        }
+
         let comparison = JSONCompare.createComparison(
             function: .iob,
             swift: iobResultSwift,