Repository navigation
Prevent manual bolus crashes from missing carb schedule coverage - #2614
Open
damonbayer wants to merge 1 commit into
Open
damonbayer wants to merge 1 commit into
damonbayer wants to merge 1 commit into
Conversation
Collaborator
|
I want to land this, but I also really want to understand what happened, and I haven't been able to figure out a theory that explains it. |
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 and scope
A UUID-matched symbolication of a Loop 3.15.2 (69) crash identifies
CarbMath.swift:717as the immediate failure, with the stackBolusEntryViewModel.update → recommendManualBolus → LoopAlgorithm.run → generatePrediction → CarbMath.map:At least one carb entry had no preceding carb-ratio or insulin-sensitivity value. The calculation includes previously logged carbs even when the user is bolusing without adding or editing carbs.
The reported incident's underlying cause remains unresolved. The reporter clarified that the bolus view was open for only a few minutes and they believe they were bolusing without adding a carb entry. That does not establish the pending-entry aging scenario below. The IPS contains neither the offending input values nor the pending-entry state. This PR adds a recoverable coverage check that applies to stored as well as pending entries, and repairs a separate concrete pending-entry coverage defect found during the investigation. It should prevent the observed precondition trap in the manual recommendation/preview paths, but is not evidence that the incident's source of incomplete history has been identified.
Pending-entry defect found during investigation
fetchDatarequests carb-ratio history beginning atnow − 12 hours − 1 minute, and ISF/override history extends back at least that far. BothrecommendManualBolusand the glucose preview then append a pending entry after fetching that history. Its timestamp stays fixed while the history window advances.For example, select an entry 12 hours in the past and leave the bolus screen open for its five-minute refresh. The pending entry is then four minutes older than the fetched carb-ratio window.
closestPrior(to: entry.startDate)returns nil and triggers the precondition above. The earlier ISF-window fix (#2489) is already present, but only covers the normal query window.A synthetic input against the recorded algorithm revision reproduced SIGTRAP with a carb entry one second before the ratio-history boundary; the boundary itself and extended history succeeded. This establishes the coverage failure mechanism, not the actual input state of the reported incident.
Change
fetchDatato extend carb-ratio, ISF, and override history to the pending entry's timestamp. Both manual-bolus recommendations and their glucose previews supply that timestamp, including edits.SettingsStore. Its previous daily-boundary behavior could accidentally conceal this bug.Validation
Regression coverage includes a pending 12-hour-old entry followed by a five-minute clock advance, an edited entry outside the normal window, historical override application, preservation of normal coverage for recent entries, missing carb-ratio and ISF history in both recommendation and preview paths, and clearing an existing UI recommendation after a configuration error.
Passed 80 tests, 0 failures: all
LoopDataManagerTests(20) andBolusEntryViewModelTests(60), including nine new regression tests. Ran on an iPhone 16 / iOS 18.5 simulator with ad-hoc signing, against Loopnext-devat8ca3f9d8, LoopKitdad7d163, and LoopAlgorithm9628dfe6.git diff --checkalso passes. No changes to the numerical absorption model, dosing formulas, or production signing settings.