Conversation
caa9fd4 to
5ef4775
Compare
|
Thanks @Iskrata, this is careful work: a measured problem, a named cost, both What holds upThe habitual-sleep change is parity-correct. Both platforms filter on The load cache key is complete. The owner set is encoded in the witness The profile interpolation is value-based, which was my main worry. Swift The empty-witness path is consistent. When One code noteThe key's coverage of the profile rests on the DEFAULT description of both types. One thing worth namingThis is filed as performance, and it is, but learning only from finished nights also What is blocking it, which is not your doingThis branch is based on That is the failure mode the workflow header calls out, and the scheduled run on main Nothing above needs action from you yet beyond the profile-key note, which is your |
The scheduled parity-governance run on main went red this morning on test_checked_metadata_is_compact_v3_and_expands_losslessly: the checked-in authority no longer reproduced from the current sources. The cause was #2348, which added a Swift helper inside a governed root without a re-derive. Nothing went red at the time because the workflow deliberately excludes product source from its path filters, so the merge that caused the drift is never the one that reports it. The schedule exists for exactly that, and it did its job. The numbers move by that one function: functions 4453 to 4454, unpaired_functions 4107 to 4108. The baseline file is untouched and the refresh reports the same 300 known findings, so no debt changed hands. --refresh-derived alone refuses here, because the base's stored manifest cannot be reproduced at all, and --repair-stale-base cannot help either: repair is for a base whose governed state matches and whose metadata drifted, which is not this. So this is --refresh-derived --migrate-authority, which waives only the reproducibility of the base manifest and no semantic debt. Every one-sided declaration still needs its own issue-bound disposition, and the tool refuses to migrate onto anything but an exactly derived current authority. This was blocking an outside contributor: PR #2293 branched before #2348 and carries a correct authority for its own three additions, so on a rebase it would have reported a mismatch of one function it never touched. That is the failure the workflow header warns about, and this clears it. Verification: the parity-governance suite CI runs, 124 tests, OK.
|
Main is repaired, so this is unblocked. Reporting the numbers as promised.
The remedy was What that means for this branch: your 4453 to 4456 was correct for the three The review above stands unchanged: the two changes are sound, the twins agree, and |
|
Reviewed at 5ef4775. Three things in here looked wrong on a first read and turned out to be fine, so let me record those first, because the names invite the wrong conclusion and the next reader will land in the same places.
Also verified: Swift declines the cache when a fingerprint read failed ( 1. This is a behaviour change, not only a performance one
I think the change is right. A night still being slept is not a habit, and learning from a session that every sync re-banks while it grows is what produced the churn in the first place. Your doc comments are honest about the mechanism. But the PR is typed "Bug fix (performance / battery)" and the body reads as a pure speedup, and this moves three user-visible numbers. Worth saying so in the description so it is on the record rather than discovered later. Note #2350's open anchor half touches 2. Worth connecting to #2360That is an open background-battery report on a 4.0, and the two costs there are distinct: the radio side, which is the history-offload protocol gap, and the app side. Your 8 813 s and 2 345 s passes are the app side, and 96 s down to 2.9 s on an unchanged re-pass is the most concrete battery improvement anyone has put up this week. The replay methodology, same database copy before and after, with the caveat that it is a Debug build so the numbers are comparable to each other and not to a phone, is exactly the right shape for this claim. 3. Minor
HousekeepingDirty, 32 behind, carrying a One pattern across your three open PRs: this, #2284 and #2285 all report the same two Approving after the rebase, with point 1 in the description. |
…ync stops dropping the day cache
…ies instead of re-reading its heart rate every pass
… and reuse a closed cycle's Effort and calories
…scription String(describing:) of the struct was value-sensitive but its format is not a contract and it ran reflection per cycle per pass. UserProfile.cacheKey names every stored field (bit patterns for doubles) on both platforms, with a test per field.
5ef4775 to
73a6004
Compare
|
Rebased onto main (
On Locally: |
2 participants
What this PR does
Two changes stop the post-sync re-score from repeating work whose inputs did not change. Most of the cost of a backgrounded pass came from these two places.
1. Sleep habits are learned from finished nights only.
sleepNeedHours,sleepConsistencyandhabitualMidsleepSecare all in the day-cache config signature, andcomputeHabitualSleepread them from every banked session up tonow. That includes tonight's session, which every sync re-banks while it is still growing. So each sync through a morning moved the consistency and midsleep, the signature changed, and the whole 21-night cache was dropped (configDropped(sleepNeedHours+sleepConsistency+habitualMidsleep)).computeHabitualSleepnow takesfinishedBefore:and the pass passes its local midnight. A night joins the learned history the day after it ends, when the window rolls anyway.2. A closed sleep-to-sleep cycle keeps its Effort and calories. In
.sleepOnsetmode (the default),DayCycleIntelligenceIntegration.computere-read each cycle's full day of 1 Hz heart rate from every owner on every pass. It then re-scored Effort and calories for all 21 cycles, although only the open cycle gains samples. Step counts already had a per-cycle cache. Effort and calories now have one too, keyed on the index-onlyhrFingerprint(count and newest timestamp) per owner, the same witness the day cache uses, plus resting HR, HRmax, method and profile.Why
A phone log showed backgrounded passes of 8 813 s and 2 345 s running back to back, with scores arriving hours late. A backgrounded app only gets short execution windows, so a pass that costs seconds of CPU in the foreground takes hours of wall-clock time in the background.
I measured this by replaying
analyzeRecenton a Mac against a copy of that phone's database (21 nights, Debug build, so compare the numbers with each other rather than with a phone):reused=0/21,configDropped(sleepConsistency+habitualMidsleep))reused=21/21)In the last row, the
score2phase went from 5.9 s to 0.3 s. The time before the scoring loop was nearly allDayCycleIntelligenceIntegration.compute(6–12 s, measured with temporary timers). The detected nights, stages, and the resulting sleep sessions are unchanged by either commit.Behaviour change
This is not only a speed-up. Learning from finished nights moves three user-visible numbers:
sleepNeedHours,sleepConsistencyandhabitualMidsleepSecwill differ slightly from what the same database produced before, for everyone, on upgrade. The most recently finished night is held out of the learned history for up to about a day, until the window rolls past it. The reasoning is that a night still being slept is not a habit yet, and learning from a session every sync re-banks while it grows is what produced the churn. Anyone picking up the open anchor half of #2350 should knowhabitualMidsleepSecmoved underneath it.The cycle load key names the profile's fields explicitly (
UserProfile.cacheKeyon both platforms, a test per field) rather than relying onString(describing:)/toString, whose format is not a contract.Type of change
How it was tested
StrandTests/HabitualSleepFinishedNightsTests: tonight's session, synced first to 04:00 and then to 09:30, does not change the learned midsleep or nightly hours.doc_comment_lint.pyis clean, andparity_ratchet.py --base upstream/main --offlinereports 0 errors.Android. The Kotlin twin is in this PR:
IntelligenceEngine.computeHabitualSleep(finishedBefore)learns from finished nights only, throughfinishedSessions.PhysiologicalStepCycleEnginekeeps a per-cycle Effort and calories cache keyed byloadCacheKey, with the index-onlyWhoopRepository.hrUnionFingerprintas the HR witness.RescoreUnchangedInputsTestcovers both changes. The full suite has 6287 tests with 2 failures, both inRecoveryDriversTest(a half-tie rounding assertion), and they fail identically on unmodifiedmainon this Mac.platform_specificinparity_dispositions.json, because their Swift twins sit in the app layer (Strand/Data/), outside the governed roots.parity_ratchet.py --offlinereports 0 errors and the parity-governance suite passes under Python 3.12, as CI runs it.Checklist
docs/CONTRIBUTING.mdStrand.xcodeproj/) or any secrets/keystores