fix(rescore): pace a backgrounded pass under iOS CPU limits (rebase of #2280) - #2296
Merged
Merged
Conversation
…of deferring it #1538 read the killed background passes as work that could not finish inside a wake, and deferred any pass whose last completed run took over 20 s to a processing task. The on-device crash reports say otherwise: 26 `cpu_resource_fatal` kills on one iPhone in five nights ("48 seconds cpu time over 52 seconds ... exceeding limit of 80% cpu over 60 seconds"), each about 52 s into a pass. A cold pass is ~144 s of near-continuous CPU on that install (`re-score: done — scored 21 night(s) in 144311 ms`, `dayCache reused=0/21`), so every attempt in the background was killed, including the processing task it was deferred to. The deferral only moved the kill, and the night's scores reached the phone when the app was next opened. A backgrounded pass now rests after each night, in both loops, for as long as the night took (capped at 30 s), holding it near 50% CPU. Suspension between rests does not kill it; it resumes on the next wake, so a pass longer than any single wake completes. With that, how long a pass takes stops being a reason to defer an offload, and the measurement rule is gone: - a real update (an offload) runs in the background, paced; - a pass owed from a KILLED attempt still defers, as before; - a pass running in this process is not a killed one: its own started-mark reads as owed, and deferring on it recorded a newer debt the running pass could not settle (#1681), so every later offload deferred. The trigger now reaches the engine, which re-arms one follow-up pass; - the backstop tick no longer runs in the background at all, since a paced pass costs minutes and every real update runs its own; - a processing task that finds a pass already running leaves it to settle its own debt. The measured duration is still banked and logged.
4 tasks
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Sep 17, 2026
Conflict resolutions: - android/: kept deleted (the fork dropped the Android tree on 2026-08-14). - project.yml build number, altstore-source.json, i18n baseline: kept the fork's. - Rescore: kept the fork's shared background assertion, day reuse and follow-up reconciliation, and added upstream's background CPU pacing, uptime-based pass timing and the "pass already running" guard (ryanbr#2296). - WorkoutsView: took upstream's WorkoutStartControl leaf (no 1 Hz re-render of the list, ryanbr#2295) and moved the fork's session-controller start flow into it. - TrainingLoadCard: took upstream's extracted TrainingLoadChart and reapplied the fork's "Effort over time" / Baseline / Recent / Balance wording. - SleepView: kept the fork's non-observing body-clock card. - iOS app, widgets, intents, Settings: added upstream's Sync Strap shortcut, keep-screen-on setting and sync Live Activity (ryanbr#2272) beside the fork's Coach brief, Recovery Status intent and searchable settings, without the Lift Log activity the fork removed. - Localizable.xcstrings: three-way merged per key and language. Follow-ups needed to build and pass the i18n gate: - BLEManager: pendingManualSyncTTL is nonisolated (Xcode 27 rejects a main actor default value in a nonisolated function). - WorkoutsView: the dash-pattern lookup uses the interval label constants. - Translate 19 String(localized:) literals the stricter upstream scanner now sees (ryanbr#2299), since the fork keeps no i18n baseline. Analysis migration required: no (upstream's pacing and uptime timing change when and how fast a pass runs, not what it computes).
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Sep 17, 2026
The fork's DeferredRescorePlanTests still called RescoreBackgroundPolicy.decide with the pass-duration budget upstream removed, so StrandTests no longer compiled after the 11.8.0 sync. An outstanding debt is now the only thing that defers a backgrounded real update; the test pins that instead, and the two doc comments that named the removed budget now describe it as the earlier behaviour.
4 tasks
ryanbr
pushed a commit
that referenced
this pull request
Sep 18, 2026
…per night (#2318) From @Iskrata. Follow-up to #2296. A rest is a Task.sleep, and a backgrounded process that is only sleeping is exactly what iOS suspends, until the next bluetooth wake about ten minutes later. Each unit of a pass is milliseconds to a few seconds of CPU, so resting after every night meant a backgrounded pass advanced roughly one night per wake. The CPU limit was respected; wall-clock progress was not. On one phone a full-history pass took the re-score lock at 21:34 and still held it after 11:00 the next day. Every post-offload pass in between returned at `guard !computing` without a word, 26 dashboard-cache refreshes with no re-score line among them, and that morning's night was never scored: no daily row, no session. A backgrounded pass now does ten seconds of work between rests. The rest is still as long as the work it follows and still capped at 30 s, so the same ~50% ceiling holds under iOS's 80%-over-60 s kill, with one suspension opportunity per ten seconds of work rather than one per night. The accumulation is what makes that safe: the work mark is no longer reset after every unit, only after a rest actually happens or in the foreground, so short units run back to back and the elapsed time builds until it crosses a quantum. Had it kept resetting per unit the guard would never fire and the pass would run unpaced into the CPU kill. Also says, once per running pass rather than once per trigger, when a forced re-score is queued behind one: re-score: queued behind a 4000-day pass running for 812 s The log previously said nothing between "sync done" and "no score", which is why this took a database pull to find. The pass start is cleared in the same defer that clears `computing`, so a throwing pass cannot leave a stale duration behind. Nothing to port: Android has no equivalent background suspension and no twin of the pacing. Author: Iskrata
4 tasks
ksricharank
pushed a commit
to ksricharank/noop
that referenced
this pull request
Sep 19, 2026
Records the two fork features RETIRED as redundant against upstream (the coach settings move, superseded by ryanbr#2243's CoachSettingsView; the isDeviceLocked re-score rule, superseded by the fork's own sleep-window rule and built on the measured rule ryanbr#2296 deleted), and the two re-verified as still needed. The notes carry the 260919 log's two open findings — the rss. counter-namespace collision that makes two header lines print the same integer, and the uninstrumented writeBackAfterNewData path that leaves the HealthKit-writes question unanswerable — as the first two items the next log must address. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ryanbr
pushed a commit
that referenced
this pull request
Sep 19, 2026
A backgrounded offload that finds a re-score already owed, with nothing running, now defers only while the last pass STARTED less than 30 minutes ago. After that it runs, paced as usual. The owed rule dates from #1538, when a backgrounded pass was killed by the CPU limit every time and retrying on each offload was the livelock. Pacing (#2296, #2318) keeps a background pass under that limit now, but the rule still deferred on any unfinished debt with no end, until a BGProcessingTask or the next foreground. On one phone that left a pass unfinished at 11:25 deferring every offload until the app was opened at 09:26 the next morning: 19 hours, and the night in between was never scored. The start time is recorded only by a pass that is about to work. The deferral path re-marks the debt without touching it, so repeated deferrals cannot keep an old unfinished pass looking recent. Nothing to port: Android has no equivalent deferral or background suspension. By @Iskrata.
ksricharank
pushed a commit
to ksricharank/noop
that referenced
this pull request
Sep 21, 2026
…kipped The report: Charge alternating between ~36 and ~67 through the day, present already at 06:30 before the app was opened, and the home-screen widget showing 36 while the app showed 67 at the same moment. Cause: the 260906 abort gate breaks the day loop the instant the app backgrounds, leaving the pass with only the newest N of maxDays nights. On a strap-only install those N nights ARE the HRV/RHR baseline — `hist` reads the imported device id, which is empty — so pass 2 z-scored the night against a window it never scored. Being a full pass, it then wrote that value straight over what a completed pass had computed. The abandonment `return` sits ~1000 lines after the persist; it only skips the watermark and the debt settle. This is the SAME failure the light-pass guard was written for (the 22 <-> 47 flip, 260903), reaching the store by a second route the `lightPass` flag cannot see. Measured against the shipped scorer: one night reads Charge 64 folded from 4 nights and 30 folded from 21. Why it surfaced now: upstream's ryanbr#2296 (v18 uplift) deleted the measured-duration deferral, so a slow pass is no longer held back — it runs, rests, and meets the abort gate. Abandonment went from rare to routine (25 in one log). Overnight the app wakes backgrounded, where `isBackgroundedSnapshot` correctly reports background on a cold launch, abandons, and leaves a truncated Charge sitting there until the morning foreground pass corrects it. The guard is keyed on BASELINE SUFFICIENCY, not on which kind of pass this is: the invariant is "do not judge a night against a window we did not score", and `lightPass` was only ever a proxy for it. A partial pass now preserves the stored recovery via the existing lightPassMerged path. `wasAbandoned` is hoisted above the fold so the one place that needs it can see it. Three always-on ledgers, counts-only, because the store keeps only the final value and none of this was answerable after the fact: - chargeWrite: every recovery write — value, hrvNValid, pass kind, trigger. nValid is the tell; far below 21 on kind=full means truncation. - chargePublish: the widget snapshot's Charge, its anchor day (flagged when carried), and whether WidgetKit was actually asked to reload or the request was withheld by the background budget. A withheld reload leaves a correct snapshot behind a stale face, which from outside looks like a wrong value. - chargeShown: what the screen displayed and via which branch, on transitions only. A carried 36 and a mis-scored 36 were previously indistinguishable. Tests: AbandonedPassBaselineTests pins the premise against the shipped scorer (a 4-night fold must read >20 points above a 21-night fold for the same night), that a sub-seed fold refuses rather than guesses, and the four guard outcomes. Mutation-checked: reverting the guard fails two of them. Every pre-existing recovery test supplies a full baseline and asserts pure-function output — none constructs a truncated one, which is why a ~30-point error shipped green. Verified: Strand (macOS) + NOOPiOS build; full 2485-test macOS suite runs with only the 4 failures already present on a clean worktree (confirmed by stashing this change and re-running). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
A rebase of #2280 onto current main, opened here so it can land without waiting on a round trip. All the work is @Iskrata's; the commit keeps their authorship and I have changed nothing in it.
Why a new PR rather than a merge of theirs
#2280 was stacked on #2279, which landed as a SQUASH (
2eafe4b0b). Its parent commit is therefore not an ancestor of main, and the branch point was also well behind: diffed from main, #2280 read as about 2,110 deletions, removingAGENTS.md,HealthExportMerge.swift, the i18n audit rule, the Coach master switch, the motion-trace hoist and more.It was never proposing any of that, it is simply what a stale branch point looks like from main's side. But it is the shape that lands as a silent revert if merged on a green tick, and
git merge-treereported three conflicts besides.Cherry-picking the single pacing commit onto current main gives what the change actually is: 6 files, +142/-134.
The change itself
#1538 read killed background passes as work too slow for one wake, and deferred anything over 20 s to a processing task. The device's own crash reports say otherwise: it is iOS's background CPU limit. Across five nights one iPhone logged 26
cpu_resource_fatalreports, each readingagainst a cold pass of about 144 s. Deferring only moved where the kill happened; the processing-task attempts were killed on the same limit.
A backgrounded pass now rests after each night for as long as that night's work took, capped at 30 s, holding it near 50% CPU. Once a pass cannot be killed for running long, duration stops being a reason to defer, so that rule goes.
Two details worth noticing, both the contributor's: the 30 s cap exists because
uptimeNanosecondskeeps advancing while the process is merely suspended, so an uncapped rest would stall an already-idle pass; andpaceIfBackgroundedre-readsisBackgroundedper unit rather than sampling once, so a pass that starts foregrounded and is backgrounded mid-run begins pacing from that point.What is still open
My review notes on #2280 stand. The one I would most like answered from the overnight run is whether the assertion expiry handler fired: "no
cpu_resource_fatalreports" reads the same whether the pass genuinely resumed after suspension or ran on detached past its grant, and only the first means a backgrounded pass reliably finishes.Also worth watching: removing duration-based deferral takes away the safety net at the same moment the replacement arrives. If pacing does not hold the limit on a slower phone or a denser history, the symptom returns as kills rather than delays, with no fallback.
Verified here with
swiftc -parseon the four changed sources; CI covers the compile. Credit and thanks to @Iskrata.