Skip to content

Commit 21e65f8

Browse files
committed
Fix max lookahead sliding window bug
Carbs originally outside the lookahead window could drift into it over time and fire incorrectly. Now all future carbs are tracked from first observation, and only fire if their original distance (carbDate minus observedAt) was within the max lookahead. Stale cleanup also preserves entries whose carb still exists to prevent re-observation with a fresh timestamp.
1 parent f316fba commit 21e65f8

2 files changed

Lines changed: 117 additions & 25 deletions

File tree

LoopFollow/Alarm/AlarmCondition/FutureCarbsCondition.swift

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ import Foundation
66
/// Fires once when a future-dated carb entry's scheduled time arrives.
77
///
88
/// **How it works:**
9-
/// 1. Each alarm tick scans `recentCarbs` for entries whose `date` is in the future
10-
/// (within a configurable max lookahead window). New ones are added to a persistent
11-
/// "pending" list.
9+
/// 1. Each alarm tick scans `recentCarbs` for entries whose `date` is in the future.
10+
/// New ones are added to a persistent "pending" list regardless of lookahead distance,
11+
/// capturing the moment they were first observed (`observedAt`).
1212
/// 2. When a pending entry's `carbDate` passes (i.e. `carbDate <= now`), verify the
13-
/// carb still exists in `recentCarbs`. If so, fire the alarm. If the carb was
14-
/// deleted, silently remove it.
15-
/// 3. Stale entries (observed > 2 hours ago) are cleaned up automatically.
13+
/// carb still exists in `recentCarbs` **and** that the original distance
14+
/// (`carbDate − observedAt`) was within the max lookahead window. If both hold,
15+
/// fire the alarm. Otherwise silently remove the entry.
16+
/// 3. Stale entries (observed > 2 hours ago) whose carb no longer exists in
17+
/// `recentCarbs` are cleaned up automatically.
1618
struct FutureCarbsCondition: AlarmCondition {
1719
static let type: AlarmType = .futureCarbs
1820
init() {}
@@ -36,9 +38,11 @@ struct FutureCarbsCondition: AlarmCondition {
3638
for carb in data.recentCarbs {
3739
let carbTI = carb.date.timeIntervalSince1970
3840

39-
// Must be in the future and within the lookahead window
41+
// Must be in the future and meet the minimum grams threshold.
42+
// We track ALL future carbs (not just those within the lookahead
43+
// window) so that carbs originally outside the window cannot
44+
// drift in later with a fresh observedAt.
4045
guard carbTI > nowTI,
41-
carbTI - nowTI <= maxLookaheadSec,
4246
carb.grams >= minGrams
4347
else { continue }
4448

@@ -61,27 +65,30 @@ struct FutureCarbsCondition: AlarmCondition {
6165
var fired = false
6266

6367
pending.removeAll { entry in
64-
// Cleanup stale entries (observed > 2 hours ago)
65-
if nowTI - entry.observedAt > 7200 {
68+
let stillExists = data.recentCarbs.contains { carb in
69+
abs(carb.date.timeIntervalSince1970 - entry.carbDate) < tolerance
70+
&& carb.grams == entry.grams
71+
}
72+
73+
// Cleanup stale entries (observed > 2 hours ago) only if
74+
// the carb no longer exists — prevents eviction and
75+
// re-observation with a fresh observedAt.
76+
if nowTI - entry.observedAt > 7200, !stillExists {
6677
return true
6778
}
6879

6980
// Not yet due
7081
guard entry.carbDate <= nowTI else { return false }
7182

72-
// Due — verify carb still exists in recentCarbs
73-
let stillExists = data.recentCarbs.contains { carb in
74-
abs(carb.date.timeIntervalSince1970 - entry.carbDate) < tolerance
75-
&& carb.grams == entry.grams
76-
}
83+
// Carb was deleted — remove silently
84+
if !stillExists { return true }
7785

78-
if stillExists, !fired {
79-
fired = true
80-
return true // remove from pending after firing
81-
}
86+
// Carb was originally outside the lookahead window — remove without firing
87+
if entry.carbDate - entry.observedAt > maxLookaheadSec { return true }
8288

83-
// Carb was deleted or we already fired this tick — remove silently
84-
if !stillExists {
89+
// Fire (one per tick)
90+
if !fired {
91+
fired = true
8592
return true
8693
}
8794

Tests/AlarmConditions/FutureCarbsConditionTests.swift

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ struct FutureCarbsConditionTests {
7474
#expect(Storage.shared.pendingFutureCarbs.value.isEmpty)
7575
}
7676

77-
// MARK: - 4. Beyond lookahead — carb ignored
77+
// MARK: - 4. Beyond lookahead — tracked but does not fire
7878

79-
@Test("#beyond lookahead — carb ignored")
80-
func beyondLookaheadIgnored() {
79+
@Test("#beyond lookahead — tracked but does not fire")
80+
func beyondLookaheadTrackedButNoFire() {
8181
resetPending()
8282
let now = Date()
8383
let alarm = Alarm.futureCarbs(threshold: 45)
@@ -86,7 +86,9 @@ struct FutureCarbsConditionTests {
8686
let result = cond.evaluate(alarm: alarm, data: data, now: now)
8787

8888
#expect(!result)
89-
#expect(Storage.shared.pendingFutureCarbs.value.isEmpty)
89+
// Carb is tracked (to prevent re-observation with fresh observedAt)
90+
// but will never fire because original distance > lookahead
91+
#expect(Storage.shared.pendingFutureCarbs.value.count == 1)
9092
}
9193

9294
// MARK: - 5. Below min grams — carb ignored
@@ -212,4 +214,87 @@ struct FutureCarbsConditionTests {
212214
_ = cond.evaluate(alarm: alarm, data: data, now: now)
213215
#expect(Storage.shared.pendingFutureCarbs.value.count == 1)
214216
}
217+
218+
// MARK: - 11. Sliding window — carb outside lookahead never fires
219+
220+
@Test("#sliding window — carb outside lookahead never fires")
221+
func slidingWindowNeverFires() {
222+
resetPending()
223+
let t0 = Date()
224+
let alarm = Alarm.futureCarbs(threshold: 10) // 10-minute lookahead
225+
let carbDate = t0.addingTimeInterval(15 * 60) // 15 min in future
226+
let carbSample = CarbSample(grams: 20, date: carbDate)
227+
228+
// Tick at T+0: carb is 15 min away, outside 10-min window but tracked
229+
let data = AlarmData.withCarbs([carbSample])
230+
let r0 = cond.evaluate(alarm: alarm, data: data, now: t0)
231+
#expect(!r0)
232+
#expect(Storage.shared.pendingFutureCarbs.value.count == 1)
233+
234+
// Tick at T+5min: carb is now 10 min away (inside window), but
235+
// original distance was 15 min — must NOT fire
236+
let t1 = t0.addingTimeInterval(5 * 60)
237+
let r1 = cond.evaluate(alarm: alarm, data: data, now: t1)
238+
#expect(!r1)
239+
240+
// Tick at T+15min: carb is due — still must NOT fire
241+
let t2 = t0.addingTimeInterval(15 * 60)
242+
let r2 = cond.evaluate(alarm: alarm, data: data, now: t2)
243+
#expect(!r2)
244+
// Entry should be removed (due, outside original window)
245+
#expect(Storage.shared.pendingFutureCarbs.value.isEmpty)
246+
}
247+
248+
// MARK: - 12. Due entry outside original window removed without firing
249+
250+
@Test("#due entry outside original window removed without firing")
251+
func dueOutsideWindowRemovedNoFire() {
252+
resetPending()
253+
let now = Date()
254+
let pastDate = now.addingTimeInterval(-60) // 1 min ago
255+
256+
// Entry was observed 20 min before its carb date (outside 10-min window)
257+
Storage.shared.pendingFutureCarbs.value = [
258+
PendingFutureCarb(
259+
carbDate: pastDate.timeIntervalSince1970,
260+
grams: 20,
261+
observedAt: pastDate.timeIntervalSince1970 - 20 * 60
262+
),
263+
]
264+
265+
let alarm = Alarm.futureCarbs(threshold: 10)
266+
let data = AlarmData.withCarbs([CarbSample(grams: 20, date: pastDate)])
267+
268+
let result = cond.evaluate(alarm: alarm, data: data, now: now)
269+
270+
#expect(!result)
271+
#expect(Storage.shared.pendingFutureCarbs.value.isEmpty)
272+
}
273+
274+
// MARK: - 13. Stale entry with existing carb is not evicted
275+
276+
@Test("#stale entry with existing carb is not evicted")
277+
func staleWithExistingCarbNotEvicted() {
278+
resetPending()
279+
let now = Date()
280+
let futureDate = now.addingTimeInterval(300) // 5 min in the future
281+
282+
// Entry observed 3 hours ago, but carb still exists in recentCarbs
283+
Storage.shared.pendingFutureCarbs.value = [
284+
PendingFutureCarb(
285+
carbDate: futureDate.timeIntervalSince1970,
286+
grams: 20,
287+
observedAt: now.addingTimeInterval(-3 * 3600).timeIntervalSince1970
288+
),
289+
]
290+
291+
let alarm = Alarm.futureCarbs()
292+
let data = AlarmData.withCarbs([CarbSample(grams: 20, date: futureDate)])
293+
294+
let result = cond.evaluate(alarm: alarm, data: data, now: now)
295+
296+
#expect(!result)
297+
// Entry must survive — carb still exists, don't evict
298+
#expect(Storage.shared.pendingFutureCarbs.value.count == 1)
299+
}
215300
}

0 commit comments

Comments
 (0)