Repository navigation
Today steps: route by source, not catalog order (completes #551) - #574
Conversation
|
Self-review notes (re-reviewed my own change adversarially, verified against Confirmed correct — no bug reintroduced. I checked whether reverting the reorder re-opens the original "steps tile → empty apple-health" bug at any bare-key push site. It doesn't: there are no other One tradeoff worth stating plainly (it's why I'm flagging, not just approving my own PR): #551's reorder had a second effect I under-weighted — it incidentally gave WHOOP 5.0/MG users their measured steps in LabBook and the Compare picker (bare-key Pre-existing, not addressed here: the Compare picker lists three entries titled "Steps" (apple-health / my-whoop / xiaomi-band) because it enumerates the whole catalog without dedup or source-filtering. Two existed pre-#551 (apple-health + xiaomi-band); this is orthogonal to routing and out of scope. Verification unchanged: syntax-parses clean; |
…alog reorder Completes #551. That PR fixed the Today steps tile tapping through to an empty apple-health/steps detail, but did it partly by declaring the measured my-whoop/steps catalog entry BEFORE apple-health so `first { key == "steps" }` resolves to my-whoop. That global reorder also changed three bare-key resolvers the PR doesn't touch — LabBookView:938 (descriptor("steps")), CompareView's picker, and the TabRoute `.metric` fallback — flipping them from apple-health to my-whoop. my-whoop/steps is empty for anyone not on a 5.0/MG, so an Apple-Health-steps user's LabBook steps correlation and Compare pick would read empty. Fix: make the routing source-explicit instead of order-dependent, then undo the reorder. - TabRoute gains `.metricSourced(key:source:)` — resolves the exact (key, source) via MetricCatalog.metric(key:source:), order-independent, with a bare-key + Health fallback. - The Today steps card routes via `.metricSourced(key: stepsDetailKey, source: "my-whoop")` (both the measured "steps" and the "steps_est" fallback are my-whoop). The compact tile was already source-explicit in #551 via todayStepsMetric/detailMetric. - The measured my-whoop/steps entry moves back to AFTER apple-health, so `first{key=="steps"}` returns apple-health again — LabBook/Compare/TabRoute keep their prior behaviour, no regression. Its catalog position no longer decides where anything taps through. - Tests: drop the assertion that cemented the reorder; add a source-resolution test and a regression guard that bare-key "steps" stays apple-health (fails loudly on a future reorder). VERIFICATION LIMIT: app-target Swift has no Linux/CI path (swift-packages covers only Packages/**, app-build.yml is disabled), and this box has no Xcode, so I could NOT type-check or run the tests. Static checks done: all four files syntax-parse (`swift -frontend -parse`); TabRoute.swift holds the only exhaustive switch over the enum (its unique cases appear nowhere else), so the new case is handled; cardLink takes a TabRoute; metric(key:source:) is static. Needs an `xcodebuild -scheme Strand test` or on-demand app-build.yml run before merge — same gap that applies to #551 itself.
7634b7f to
c92330c
Compare
#377) (#575) iOS Today steps showed the WHOOP measured count ?: the motion ESTIMATE — with no tier for an imported Apple Health count. A WHOOP 4.0 user (no @57 measured steps) who imports Apple Health therefore saw the approximate motion estimate on Today instead of their real imported count, while Android (TodayScreen #377) blends `real ?: imported ?: estimate` and shows the import. Adds the imported tier on iOS, in BOTH the displayed value and the tap-through source so they stay consistent (the invariant #551/#574 established): - stepCount: displayDay.steps ?: importedStepsDay ?: stepsEst. importedStepsDay is the selected day's Apple Health steps (max across rows), loaded from repo.appleDailyRows() in the same per-day load as stepsEst. Health Connect is Android-only, so apple-health is the sole import source on iOS. Mirrors Android `stepsForDay`. - todayStepsMetric gains hasImportedSteps (defaulted false, so existing callers/tests are unchanged): measured -> my-whoop:steps, imported -> apple-health:steps, else my-whoop:steps_est. - the steps card routes `.metricSourced(key: stepsDetailKey, source: stepsDetailSource)` so an imported value taps through to the apple-health:steps history (exploreSeries routes apple-health via series(), so that detail is populated), not an empty my-whoop detail. - tests: measured wins over imported; imported wins over estimate. KNOWN MINOR (not fixed here): the tile's mini-sparkline keys on the "steps" series = measured DailyMetric.steps, so an import-only user's value + detail are correct but the tiny trend line stays flat. Blending the trail would need an imported-steps sparkline series; left as a follow-up to keep this change minimal. VERIFICATION LIMIT: app-target Swift, no Linux/CI path, no Xcode on this box -> could NOT type-check or run. Static: all three files syntax-parse; appleDailyRows()/AppleDaily(.day/.steps) exist; types line up ([AppleDaily].compactMap{steps}.max() -> Int?). Needs an `xcodebuild -scheme Strand test` / app-build.yml run before merge.
…alog reorder (ryanbr#574) Completes ryanbr#551. That PR fixed the Today steps tile tapping through to an empty apple-health/steps detail, but did it partly by declaring the measured my-whoop/steps catalog entry BEFORE apple-health so `first { key == "steps" }` resolves to my-whoop. That global reorder also changed three bare-key resolvers the PR doesn't touch — LabBookView:938 (descriptor("steps")), CompareView's picker, and the TabRoute `.metric` fallback — flipping them from apple-health to my-whoop. my-whoop/steps is empty for anyone not on a 5.0/MG, so an Apple-Health-steps user's LabBook steps correlation and Compare pick would read empty. Fix: make the routing source-explicit instead of order-dependent, then undo the reorder. - TabRoute gains `.metricSourced(key:source:)` — resolves the exact (key, source) via MetricCatalog.metric(key:source:), order-independent, with a bare-key + Health fallback. - The Today steps card routes via `.metricSourced(key: stepsDetailKey, source: "my-whoop")` (both the measured "steps" and the "steps_est" fallback are my-whoop). The compact tile was already source-explicit in ryanbr#551 via todayStepsMetric/detailMetric. - The measured my-whoop/steps entry moves back to AFTER apple-health, so `first{key=="steps"}` returns apple-health again — LabBook/Compare/TabRoute keep their prior behaviour, no regression. Its catalog position no longer decides where anything taps through. - Tests: drop the assertion that cemented the reorder; add a source-resolution test and a regression guard that bare-key "steps" stays apple-health (fails loudly on a future reorder). VERIFICATION LIMIT: app-target Swift has no Linux/CI path (swift-packages covers only Packages/**, app-build.yml is disabled), and this box has no Xcode, so I could NOT type-check or run the tests. Static checks done: all four files syntax-parse (`swift -frontend -parse`); TabRoute.swift holds the only exhaustive switch over the enum (its unique cases appear nowhere else), so the new case is handled; cardLink takes a TabRoute; metric(key:source:) is static. Needs an `xcodebuild -scheme Strand test` or on-demand app-build.yml run before merge — same gap that applies to ryanbr#551 itself.
ryanbr#377) (ryanbr#575) iOS Today steps showed the WHOOP measured count ?: the motion ESTIMATE — with no tier for an imported Apple Health count. A WHOOP 4.0 user (no @57 measured steps) who imports Apple Health therefore saw the approximate motion estimate on Today instead of their real imported count, while Android (TodayScreen ryanbr#377) blends `real ?: imported ?: estimate` and shows the import. Adds the imported tier on iOS, in BOTH the displayed value and the tap-through source so they stay consistent (the invariant ryanbr#551/ryanbr#574 established): - stepCount: displayDay.steps ?: importedStepsDay ?: stepsEst. importedStepsDay is the selected day's Apple Health steps (max across rows), loaded from repo.appleDailyRows() in the same per-day load as stepsEst. Health Connect is Android-only, so apple-health is the sole import source on iOS. Mirrors Android `stepsForDay`. - todayStepsMetric gains hasImportedSteps (defaulted false, so existing callers/tests are unchanged): measured -> my-whoop:steps, imported -> apple-health:steps, else my-whoop:steps_est. - the steps card routes `.metricSourced(key: stepsDetailKey, source: stepsDetailSource)` so an imported value taps through to the apple-health:steps history (exploreSeries routes apple-health via series(), so that detail is populated), not an empty my-whoop detail. - tests: measured wins over imported; imported wins over estimate. KNOWN MINOR (not fixed here): the tile's mini-sparkline keys on the "steps" series = measured DailyMetric.steps, so an import-only user's value + detail are correct but the tiny trend line stays flat. Blending the trail would need an imported-steps sparkline series; left as a follow-up to keep this change minimal. VERIFICATION LIMIT: app-target Swift, no Linux/CI path, no Xcode on this box -> could NOT type-check or run. Static: all three files syntax-parse; appleDailyRows()/AppleDaily(.day/.steps) exist; types line up ([AppleDaily].compactMap{steps}.max() -> Int?). Needs an `xcodebuild -scheme Strand test` / app-build.yml run before merge.
…alog reorder (ryanbr#574) Completes ryanbr#551. That PR fixed the Today steps tile tapping through to an empty apple-health/steps detail, but did it partly by declaring the measured my-whoop/steps catalog entry BEFORE apple-health so `first { key == "steps" }` resolves to my-whoop. That global reorder also changed three bare-key resolvers the PR doesn't touch — LabBookView:938 (descriptor("steps")), CompareView's picker, and the TabRoute `.metric` fallback — flipping them from apple-health to my-whoop. my-whoop/steps is empty for anyone not on a 5.0/MG, so an Apple-Health-steps user's LabBook steps correlation and Compare pick would read empty. Fix: make the routing source-explicit instead of order-dependent, then undo the reorder. - TabRoute gains `.metricSourced(key:source:)` — resolves the exact (key, source) via MetricCatalog.metric(key:source:), order-independent, with a bare-key + Health fallback. - The Today steps card routes via `.metricSourced(key: stepsDetailKey, source: "my-whoop")` (both the measured "steps" and the "steps_est" fallback are my-whoop). The compact tile was already source-explicit in ryanbr#551 via todayStepsMetric/detailMetric. - The measured my-whoop/steps entry moves back to AFTER apple-health, so `first{key=="steps"}` returns apple-health again — LabBook/Compare/TabRoute keep their prior behaviour, no regression. Its catalog position no longer decides where anything taps through. - Tests: drop the assertion that cemented the reorder; add a source-resolution test and a regression guard that bare-key "steps" stays apple-health (fails loudly on a future reorder). VERIFICATION LIMIT: app-target Swift has no Linux/CI path (swift-packages covers only Packages/**, app-build.yml is disabled), and this box has no Xcode, so I could NOT type-check or run the tests. Static checks done: all four files syntax-parse (`swift -frontend -parse`); TabRoute.swift holds the only exhaustive switch over the enum (its unique cases appear nowhere else), so the new case is handled; cardLink takes a TabRoute; metric(key:source:) is static. Needs an `xcodebuild -scheme Strand test` or on-demand app-build.yml run before merge — same gap that applies to ryanbr#551 itself.
ryanbr#377) (ryanbr#575) iOS Today steps showed the WHOOP measured count ?: the motion ESTIMATE — with no tier for an imported Apple Health count. A WHOOP 4.0 user (no @57 measured steps) who imports Apple Health therefore saw the approximate motion estimate on Today instead of their real imported count, while Android (TodayScreen ryanbr#377) blends `real ?: imported ?: estimate` and shows the import. Adds the imported tier on iOS, in BOTH the displayed value and the tap-through source so they stay consistent (the invariant ryanbr#551/ryanbr#574 established): - stepCount: displayDay.steps ?: importedStepsDay ?: stepsEst. importedStepsDay is the selected day's Apple Health steps (max across rows), loaded from repo.appleDailyRows() in the same per-day load as stepsEst. Health Connect is Android-only, so apple-health is the sole import source on iOS. Mirrors Android `stepsForDay`. - todayStepsMetric gains hasImportedSteps (defaulted false, so existing callers/tests are unchanged): measured -> my-whoop:steps, imported -> apple-health:steps, else my-whoop:steps_est. - the steps card routes `.metricSourced(key: stepsDetailKey, source: stepsDetailSource)` so an imported value taps through to the apple-health:steps history (exploreSeries routes apple-health via series(), so that detail is populated), not an empty my-whoop detail. - tests: measured wins over imported; imported wins over estimate. KNOWN MINOR (not fixed here): the tile's mini-sparkline keys on the "steps" series = measured DailyMetric.steps, so an import-only user's value + detail are correct but the tiny trend line stays flat. Blending the trail would need an imported-steps sparkline series; left as a follow-up to keep this change minimal. VERIFICATION LIMIT: app-target Swift, no Linux/CI path, no Xcode on this box -> could NOT type-check or run. Static: all three files syntax-parse; appleDailyRows()/AppleDaily(.day/.steps) exist; types line up ([AppleDaily].compactMap{steps}.max() -> Int?). Needs an `xcodebuild -scheme Strand test` / app-build.yml run before merge.
…, not a hardcoded "My WHOOP" (ryanbr#2274) The source row under the day stepper was a literal on both platforms — `Text("My WHOOP")` / `R.string.timeline_my_whoop` — written for ryanbr#574 when the strap was the only possible owned source. `Repository.timelineSeries` reads the ACTIVE device id first, so with an Oura ring active the chart plotted the ring's series under a WHOOP label. The row now shows the active device's registry display name (nickname, else "Brand Model") via the existing `PairedDevice.displayName` / `displayName(PairedDeviceRow)` twins — the same name the Devices screen uses. No registry row (the pre-registry seeded strap) keeps the legacy "My WHOOP", so no new strings and no locale work. Verified: macOS `Strand` build, iOS `NOOPiOS` build, Android `compileFullDebugKotlin`, `doc_comment_lint.py`, `i18n_audit.py --ci` all green. Refs ryanbr#2274 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XQZpEYdY41SRMZ48HJdrGD
…, not a hardcoded "My WHOOP" (ryanbr#2274) The source row under the day stepper was a literal on both platforms — `Text("My WHOOP")` / `R.string.timeline_my_whoop` — written for ryanbr#574 when the strap was the only possible owned source. `Repository.timelineSeries` reads the ACTIVE device id first, so with an Oura ring active the chart plotted the ring's series under a WHOOP label. The row now shows the active device's registry display name (nickname, else "Brand Model") via the existing `PairedDevice.displayName` / `displayName(PairedDeviceRow)` twins — the same name the Devices screen uses. No registry row (the pre-registry seeded strap) keeps the legacy "My WHOOP", so no new strings and no locale work. Verified: macOS `Strand` build, iOS `NOOPiOS` build, Android `compileFullDebugKotlin`, `doc_comment_lint.py`, `i18n_audit.py --ci` all green. Refs ryanbr#2274 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XQZpEYdY41SRMZ48HJdrGD
…#2276) The Deep Timeline's source row was a literal on both platforms, written for #574 when the strap was the only possible owned source. `Repository.timelineSeries` reads the ACTIVE device id first, so with an Oura ring active the chart plotted the ring's series under a WHOOP label. See #2274. The row now shows the active device's registry display name through the existing `PairedDevice.displayName` twins: nickname if set, else "Brand Model", the same name the Devices screen and the "make X your active strap" prompts already use. An active id with no registry row (the pre-registry seeded strap) keeps the legacy "My WHOOP", so the existing translations stay in use for the fallback and no new strings are needed. Reads the registry rather than comparing a brand string, which is what makes an active Oura ring read "Oura ..." instead of the strap label this row shipped with. Visible side effect, accepted deliberately: a WHOOP strap that HAS a registry row now reads "WHOOP 4.0" or its nickname instead of "My WHOOP". That is consistent with the Devices screen and is what a multi-WHOOP setup needs anyway (#1300, #1304). The author offered a brand-only gate to keep the old label; merged without it. The nickname is user data, not copy, so the Swift side renders it through `Text(verbatim:)` and cannot mistake a device name for a localization key. Both platforms mirrored, same fallback semantics on each. Thanks @pipiche38. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XQZpEYdY41SRMZ48HJdrGD
Builds on and completes #551 (@exzanimo) — includes that PR's commit plus one fix on top. Merging this closes #551; if #551 lands first, this reduces to just the follow-up commit.
What #551 got right, and the one thing it left
#551 correctly fixes the Today steps tile tapping through to an empty
apple-health/stepsdetail. But it does so partly by declaring the measuredmy-whoop/stepscatalog entry beforeapple-health/steps, soMetricCatalog.all.first { $0.key == "steps" }now resolves to my-whoop. That lookup is used by three bare-key resolvers the PR doesn't touch:LabBookView:938descriptor("steps")(confirmed reachable — steps is in its analysed set)CompareView's picker / legacy resolutionTabRoute.metricfallbackSince
my-whoop/stepsis empty for anyone not on a 5.0/MG, an Apple-Health-steps user (e.g. a WHOOP 4.0 owner who imports Health) would now get an empty LabBook steps correlation and Compare series where they previously saw their data. The reorder is load-bearing for #551's dashboard-card path (which routes by bare key), so it can't simply be dropped — the card needs the my-whoop entry to win.The fix: route by source, not by catalog order
TabRoute.metricSourced(key:source:)— a new route that resolves the exact(key, source)viaMetricCatalog.metric(key:source:), order-independent, with a bare-key → Health fallback so a stale route can't dead-end..metricSourced(key: stepsDetailKey, source: "my-whoop")(both the measuredstepsand thesteps_estfallback are my-whoop). The compact tile was already source-explicit in Fix Today steps analytics source #551 viatodayStepsMetric/detailMetric.apple-health, sofirst { key == "steps" }returns apple-health again and LabBook / Compare / TabRoute keep their prior behaviour. No regression, and the entry's catalog position no longer decides where anything taps through.stepsstays apple-health — so a future reorder fails loudly instead of silently emptying those screens.Net result: every steps surface that needs my-whoop reaches it explicitly by source; every surface that resolves by bare key is byte-for-byte unchanged from before #551.
Same gap #551 has, stated plainly: this is app-target Swift (
Strand/,StrandTests/), which has no Linux/CI path —swift-packages.ymlcovers onlyPackages/**andapp-build.ymlis disabled. This box has no Xcode, so I could not type-check or run the tests. What I did verify statically:swift -frontend -parse(syntax clean);TabRoute.swiftholds the only exhaustive switch over the enum (its unique cases.fullDayChart/.metricExplorerappear in no other file), so the new.metricSourcedcase is fully handled — no other switch breaks;cardLink(_:)takes aTabRoute;MetricCatalog.metric(key:source:)isstaticand reachable;MetricDetailView(metric:)is the existing initialiser.Please run
xcodebuild -project Strand.xcodeproj -scheme Strand test(or dispatchapp-build.yml) before merging — that exercisesMetricCatalogStepsTestsand the compile that neither #551 nor this PR has had.Not included (separate follow-up)
The card-level Apple-Health steps parity gap — Android's #377 blends imported Apple/Health-Connect steps into the Today steps value (
real ?: imported ?: estimate) for both card and detail, while iOS omits the imported tier entirely. This PR keeps the iOS detail consistent with the iOS card (both my-whoop-then-estimate); closing the imported-tier gap touchesstepCountitself and belongs in its own PR.