Measure where a re-score pass actually goes, before optimising the reads - #1559
Conversation
Pricing the "make the pass cheaper" option on #1538 got as far as static analysis can and then hit a wall: the pass has only ever timed itself end to end, so nobody knows whether ~22.6 s per night is spent materialising rows or inside analyzeDay. That single ratio decides whether narrowing the read windows is worth building at all, and no amount of reading the code answers it. What the static analysis DID settle, and why the question matters. Each day reads a 54-hour night window (dayStart-30h to dayStart+24h) on a 24-hour stride, so consecutive windows overlap by 30 hours and every row is materialised about 2.25 times per pass -- 1134 window-hours read against 534 distinct. 53% of the read volume is redundant. Nine windowed store reads per day survive; the day-window reads (dayHr/daySteps/dayGrav) were already sliced out of the night lists by #997/#1346, so that part is done. The obvious fix does NOT work: reading the whole 21-day span once and slicing would hold ~1.8M HR rows in memory, and OOM on big-import libraries is a known failure of this path. A sliding window would -- the loop already walks newest-to-oldest, so it could read only the incremental 24 hours per step and drop the tail, keeping today's peak memory while materialising 2.25x fewer rows. That is a real design, but it is only worth building if reads actually dominate. So: time the two phases. `prep` brackets the nine windowed reads plus the session matching between them; `score` brackets analyzeDay. They deliberately do not sum to the pass total -- pass 2, the baseline folds and the reconciliation are all outside this loop -- so the line is to be read as a RATIO, which is the only thing the question needs. A day skipped for too few HR samples still counts its prep, or the tally would under-report exactly the sparse-history installs where reads dominate most. Instrumentation only: no behaviour change, no scoring change, one new diagnostic line per pass on both platforms, byte-identical between them. This is the instrument-first pattern the repo already uses for questions it cannot answer from a desk (#1344 for the pass duration, #688 for the REM funnel). Verified: Android 4229 tests, 0 failures (--no-build-cache --rerun-tasks), matching main; doc lint clean. app-build to follow for the app-target Swift.
|
Re-reviewed the code. No changes — the timer accounting verified complete, and two limitations recorded rather than papered over. Accounting is closed on both platformsThe risk in an instrument like this is a path that starts a timer and never accounts for it, which silently under-reports one phase and skews the very ratio the PR exists to produce. Traced every exit:
So a reused day contributes nothing to either phase (it does no reads and no scoring — correct), a day skipped for too few samples contributes its read time but no score time (correct — it paid for the read), and no day can start a timer that goes unrecorded. The emitted strings are byte-identical across platforms, both truncating rather than rounding. Two limitations, deliberately not changedDifferent clock sources. Kotlin uses No denominator on the cost line. Neither is worth uncompiled churn while the PR is green and waiting on a hardware measurement. Unchanged from beforeAndroid CI built and tested this ( |
Curated notes for docs/releases/v10.6.0.md covering the 83 merged PRs since v10.5.0, then Tools/appchangelog-gen.py run over them to insert the entry into both AppChangelog.swift and AppChangelog.kt and localize the card title into all six Android locales. Generated rather than hand-written on purpose: the release workflow only runs appchangelog-gen when it performs the version bump itself, so running it here means the in-app card can be reviewed and lint-checked before anything is published instead of arriving as a CI warning nobody reads. Headline items are the selectable Banister TRIMP Effort scale (#1562, #1563), the background re-score that can now finish rather than restarting forever (#1557, #1559), the Oura ring being handed out of daytime-HR mode so its own sleep suite can run (#1526, #1550), ramp-aware sleep with the night's HR line (#1551, #1552), live-workout pause/discard plus SDNN on Android (#1533, #1535), and the WHOOP 4.0 device key no longer reaching a shared strap log (#1610). Third-party contributors credited by handle per CLAUDE.md, covering both merged PRs and the reports behind the fixes. The version itself is deliberately NOT bumped here: the release workflow skips appchangelog-gen when the source version already equals the target, so pre-bumping would ship a release whose in-app What's New still advertised the previous one. Verified: compileFullDebugKotlin, lintVitalFullRelease -PstagingRelease (the ExtraTranslation gate that compile and the i18n audit both miss), i18n_audit --ci, doc_comment_lint, and the 108 Tools tests.
…he cache Three diagnostics, no behaviour change. Every later commit in the analyze-pass-cost plan is gated on what these measure — the two prior plans in this area both correctly refused to optimise an unprofiled hotspot, and this is what stops that refusal from blocking a third time. Ports the Swift half of two upstream changes this fork never took, plus one addition of our own: * `analyzeRecent cost prep=Nms score=Nms` (upstream ryanbr#1559, 25+/0-, ported near-verbatim). The pass has only ever timed itself end to end, so whether the ~33 s/night measured on this device is spent in the windowed store reads or inside analyzeDay is unknown — and that ratio is what decides whether narrowing the 54h-window-on-a-24h-stride reads is worth building. Keeps upstream's sparse-day accounting: the `hr.count >= 200` early `continue` folds its elapsed prep in before continuing, because 13 of the 21 day slots on this device take that path and would otherwise vanish from the tally. A cache hit contributes to neither phase — the brackets start after the reuse `continue`, which is the point of a hit. * An honest reuse denominator (upstream ryanbr#1556's other half): the ratio is now `reused/(reused+cacheable)` with `days=` alongside. `maxDays` counted loop iterations, so a store holding 8 real nights in a 21-day window could never report better than 8/21 — which reads as a broken cache and is in fact a healthy one. That misreading already cost one investigation. Upstream's attribution half is deliberately NOT ported: it infers the trigger from two booleans, where we already carry a real AnalyzeTrigger. * `analyzeRecent dayCache DROPPED — sig changed: <names>` (ours). The 2026-08-26 device log shows a full cold pass (775 s) followed by a second full cold pass (266 s) whose 7 cached days were byte-identical, then a warm pass reusing 7 of 8. Only a whole-cache drop explains the middle one, but the log could not say which of the 14 signature components moved — or whether it was a signature drop at all rather than per-day eligibility. The signature is now carried as (name, value) pairs and the drop names the components that changed. NAMES ONLY, never values: a component can carry profile data (age, sex). A cold process is reported as such rather than listing all 14. `eligible=` and `ownerFamilyNil=` on the reuse line settle the eligibility half — upstream hit the same class of silent registry absence in ryanbr#1567 and reached for the same remedy. The signature's joined value is byte-identical to before (same components, same order, same separator), so this restructure invalidates no cache. Verification: `xcodegen generate` + `xcodebuild -scheme NOOPiOS -destination generic/platform=iOS build` — BUILD SUCCEEDED. No CI covers these targets (app-build.yml triggers only on pull_request to main, and work here merges locally), so the local build is the gate. Not yet observed on device; the next morning's log is what these lines exist to produce. Kotlin twin: not required. Diagnostic strings on the iOS log sink — docs/CROSS_PLATFORM.md binds decoders, analytics formulas, migrations and stored values, and a log line is none of those. Same reasoning and precedent as the rescore-floor plan's Commit 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017g6WqitM2Y76kkYsHzpXab
Pricing the "make the pass cheaper" option on #1538 got as far as static analysis can, then hit a wall. Instrumentation only — no behaviour change, no scoring change.
What the static analysis settled
Each day reads a 54-hour night window (
dayStart − 30h→dayStart + 24h) on a 24-hour stride. Consecutive windows overlap by 30 hours, so every row is materialised about 2.25× per pass:limit: 200_000— 97% of capNine windowed store reads per day survive. The day-window reads (
dayHr/daySteps/dayGrav) are already sliced out of the night lists by #997/#1346, so that part of the work is done and shouldn't be re-done.What it could not settle
Whether reads or
analyzeDaydominate. The pass has only ever timed itself end to end (re-score: done … in N ms), so the ~22.6 s per night is an undifferentiated number. That single ratio decides whether narrowing the windows is worth building, and no amount of reading the code answers it.The design this would unlock, and the one it rules out
The obvious fix does not work: reading the whole 21-day span once and slicing would hold ~1.8M HR rows in memory, and OOM on big-import libraries is a known failure of this exact path.
A sliding window would. The loop already walks newest→oldest, so it could read only the incremental 24 hours per step and drop the tail — same peak memory as today, 2.25× fewer rows materialised. That is a real design, but it is only worth building if reads actually dominate.
What this PR adds
One diagnostic line per pass, on both platforms, byte-identical:
prepbrackets the nine windowed store reads plus the session matching between them.scorebracketsanalyzeDay.They deliberately do not sum to the pass total — pass 2, the baseline folds and the reconciliation all sit outside this loop — so the line is to be read as a ratio, which is the only thing the question needs. A day skipped for too few HR samples still counts its
prep, or the tally would under-report exactly the sparse-history installs where reads dominate most.This is the instrument-first pattern the repo already uses for questions it cannot answer from a desk — #1344 for the pass duration, #688 for the REM funnel.
How to read the result
prep≫score→ build the sliding window; the ceiling is ~53% of read time.score≫prep→ narrowing windows is a dead end whatever the row counts suggest, and the persistent day cache is the only remaining lever.Verification
Android 4,229 tests, 0 failures (
--no-build-cache --rerun-tasks), matching main. Doc lint clean.app-builddispatched for the app-target Swift.Not run on hardware — which is the entire point: this exists so a real device can answer a question a desk cannot. Partial work on #1538; that issue stays open.