Conversation
lowState, urgentLowState, highState and predictedLowInEpisode were in-memory only, so every launch reset lastFiredAt to nil. Acknowledge a High Glucose alert, restart the app, and the next in-boundary reading re-alerts it. The same applied to Urgent Low, and a restart also defeated an active snooze, since the repeat interval is enforced purely through lastFiredAt. Encode the four as a single blob in UserDefaults, written from a didSet on each and restored in init.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Acknowledge a High Glucose alert, restart the app, and the same alert fires again.
GlucoseAlertManagerpersists profiles, active profile, override and sounds, but not its hysteresis state:decideAndUpdatefires wheneverstate.lastFiredAt == nil, so a relaunch makes an ongoing episode look new:lastFiredAtsetAlertManagerrecords it;GlucoseAlertManagernever learns, it doesn't observe acknowledgementhighStateis back toAlertState()lastFiredAt == nil, so it fires againThis is a fresh issuance, not AlertStore playback: these alerts are
.immediateand acknowledged, so both playback queries exclude them.Two consequences beyond the reported symptom:
.critical.lastFiredAt.With the high-delay option enabled the re-alert arrives one delay period after launch rather than on the first reading, which is probably why it looks intermittent.
Fix
AlertStatebecomesCodableand the four values are stored as oneEpisodeStateblob inUserDefaults. AdidSeton each property persists — that covers every mutation site without touching callers, sincedecideAndUpdateandtakeRetractIDtake theminoutand the writeback fires the observer.initrestores if present.State is persisted as-is with no aging. If the app is closed across a full high → normal → high cycle, the restored
lastFiredAtsuppresses the new episode until the repeat interval elapses; recovery detection handles the ordinary case, since the first in-range reading after launch resets the state and retracts.Testing
Three tests added to
GlucoseAlertManagerTests, simulating a restart by constructing a second manager over the same injectedUserDefaults:testHighAlertDoesNotRepeatAfterRelaunch— the reported bugtestRecoveryAfterRelaunchRetractsHigh—inBoundarysurvives, not justlastFiredAttestHighAlertsAgainInNewEpisodeAfterRelaunch— a genuine new episode still alerts7 tests pass. Removing the restore from
initmakes the suite fail, so these do catch the bug.