Conversation
The forecast series starts at the current glucose value rather than the first future point, so iterating ceil(predictiveMinutes / 5) entries stopped one step short of the requested horizon. A 20-minute setting only reached 15 minutes ahead, and a 15-minute one reached 10. At every setting the dial behaved as though it were one notch lower. Take one more point so the setting matches its label, and raise the alarm forecast cap from 12 points to 13 so the 60-minute maximum is reachable instead of capping at 55. loopHorizonReachesRequestedMinutes is the regression test: the dip sits at index 4, so a 20-minute look-ahead has to reach it and a 15-minute one must not. The other added cases cover bounds safety when the horizon exceeds the published series and pin the existing behaviour of following the longest forecast rather than the first to run out.
LowBGCondition owns the horizon arithmetic: forecastPoints(forMinutes:) converts a look-ahead in minutes into a point count, and maxPredictiveMinutes is the longest look-ahead the alarm editor offers. Both alarmForecastPointCap and the editor's Predictive stepper range derive from these, keeping the formula, the cap, and the UI bound in step. trioShortForecastKeepsHorizon keeps every combined point before index 5 above the threshold, so its assertion holds only when the 25-minute look-ahead reaches the full horizon. loopSinglePointForecast pins both sides of the current-value convention: a lone high point stays silent and a lone low point fires.
This was referenced Sep 18, 2026
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
2 participants
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.
Supersedes #732 by @JustMaier. His commit is carried over unchanged, followed by the follow-up commit proposed in his fork and a merge of current
devso the required SwiftFormat check can run. The fork is organization-owned, so GitHub cannot grant maintainer edits on the original PR branch.Summary
The predictive low alarm looks ahead one 5-minute step less than the setting says. A 20-minute setting behaves like a 15-minute one, a 15 like a 10, and so on at every value.
The forecast series that
LowBGConditionwalks starts at the current glucose value. Both backends build it that way:DeviceStatusLoopstampsprediction[0]atlastLoopTime, and the OpenAPSpredBGscurves all begin at the cycle's ownbg. LookingpredictiveMinutesahead therefore takesceil(predictiveMinutes / 5) + 1points, and the alarm now examines exactly that many.The horizon arithmetic lives in one place,
LowBGCondition.forecastPoints(forMinutes:), andalarmForecastPointCapderives from it using the editor's 60-minute maximum, so the longest setting the editor offers can actually be satisfied.For every value the UI allows, the corrected look-ahead fires on the same cycles the previous code fired on at the next setting up. Anyone who prefers their current behaviour can select one step lower and get it back exactly. Users who leave the setting alone will see more predictive alerts, and the added far-horizon alerts are less precise than the near ones. See #732 for the replay figures behind that tradeoff.
For the Trio path,
lowestForecasttakes the longest of the four forecast curves so one curve running short does not shorten the look-ahead.Tests cover the boundary in both directions, a horizon longer than the published series, and the case where one of the four forecasts runs short.