Repository navigation
test(charge): pin the gap-longer-than-the-window display as a decision - #2670
Merged
Merged
Conversation
#2525 made a Charge baseline read only nights inside `ChargeBaselines.windowDays`, so after a gap longer than that the folded history is empty, the calibration count is non-nil (0, which #335 deliberately includes) and calibrating takes precedence over the carry. The Today Charge tile therefore reads "Calibrating" on such a gap where #779 had it read "Latest sleep". That is a reasonable call, but it arrived as a side effect: the state mappers were not touched, only their input, so the existing mapper tests kept passing because they drive the count directly with synthetic values. Nothing asserted that a gap this long now yields a non-nil count, which left the display free to be flipped back silently in either direction. Each test pins BOTH displays from the SAME fourteen nights, so the contrast is the test rather than a claim about it. Fourteen matters: fewer than `minNightsSeed` banked nights calibrate under either rule and would pin nothing, so these would have been trusted by the old whole-history fold and the carry genuinely used to win. The Swift side converts the count with the same expression TodayView uses, so the nil that lets the carry through is the production one. Mutation-checked rather than assumed: neutralising `windowDays` turns the pin red (expected 0 but was 14). An earlier draft of this test used a single night, which calibrated under BOTH rules and so pinned nothing; that is why the night count is spelled out above. No production code changes.
ryanbr
force-pushed
the
test/charge-window-gap-pin
branch
from
October 3, 2026 23:22
74ba4ac to
28a1f57
Compare
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.
Follow-up to #2666, test only. No production code changes.
#2525 made a Charge baseline read only the nights inside
ChargeBaselines.windowDays, so after a gap longer than that the folded history is empty, the calibration count is non-nil (0, which #335 deliberately includes) and calibrating takes precedence over the carry. The Today Charge tile therefore reads "Calibrating" on such a gap, where #779 had it read "Latest sleep".I think that is the right call: the baseline genuinely is empty after a gap that long, the next score needs four nights either way, and the old count read "not calibrating" while the engine's baseline was already empty. The problem is that it arrived as a side effect rather than a decision. The state mappers were not touched, only their input, so the existing mapper tests kept passing: they drive the count directly with synthetic values, and nothing asserted that a gap this long now yields a non-nil count. The display was free to flip back, in either direction, with nothing going red.
Each test pins BOTH displays from the SAME input, so the contrast is the test rather than a claim about it:
nValidis 14, overminNightsSeed, the count is nil, and the carry wins as.carriedLastNight(stale: true), which is "Latest sleep".ChargeBaselines.historyempties the fold, soownValidNightsis 0, the count is 0, and the mapper resolves to calibrating with four nights remaining.The night count is load-bearing. An earlier draft of this test used a single night, which calibrates under BOTH rules because one banked night is still under
minNightsSeed, so the carry branch was never reachable and the test pinned nothing despite its name. Fourteen is pastminNightsTrust, so the old fold would have been trusted and the carry genuinely used to win. The Swift side converts the count with$0.map { max(1, Baselines.minNightsSeed - $0) }, lifted from theTodayViewcall site, because it is the nil propagating through thatmapwhich lets the carry through at all.They sit directly below the #779 stale-carry test they collide with, and say in the comment that #2525 wins over #779 here, so whoever reads #779's test sees the newer rule beside it. Each names the other as its twin.
TodayExplainabilityTest.scoreState_gapLongerThanTheChargeWindow_calibratesRatherThanCarryingTodayExplainabilityTests.testScoreState_gapLongerThanTheChargeWindow_calibratesRatherThanCarryingAlso worth recording from reviewing #2666: its parity oracle covers 40 spreads, 17 of them seeding, but contains no case where the window yields zero nights. The empty window is exactly the path this display change rides, so the oracle does not cover it on either platform. These tests close that for the display; the baseline side of it is still uncovered by the oracle.
How it was tested
testFullDebugUnitTest --no-build-cache --rerun-tasksoverTodayExplainabilityTest(53 tests, 0 failures), with the result XML timestamp checked against the clock and the new test's name grepped out of the XML to confirm it ran rather than being filtered out by the--testspattern.ChargeBaselines.windowDaysfrom 21 to 100000 turns the pin red (expected:<0> but was:<14>), so it does detect a reverted window rule. It fails at theownValidNightsassertion rather than at the display assertion, because the fourteen nights return to the fold before the display is resolved; the chain catches it either way. The production file was reverted under a shell trap and the revert verified.ChargeBaselinesTest(20),RecoveryCalibrationTest(9) andRecoveryDriversUiTest(8), all green.StrandTestsneeds macOS with Xcode, andStrandAnalyticsdoes not build in my WSL environment (CSQLitefinds nosqlite3.h), so thebuild (Strand, macOS)leg here is the first execution of the Swift twin. It was written against the signatures in the source and follows the import pattern inChargeBreakdownWiringTests.Refs #2666, #2525, #779, #335