Skip to content

Effort: one resolved figure for every read-out on Today (#1001) - #1006

Merged
ryanbr merged 1 commit into
mainfrom
fix/effort-one-value
Aug 1, 2026
Merged

Effort: one resolved figure for every read-out on Today (#1001)#1006
ryanbr merged 1 commit into
mainfrom
fix/effort-one-value

Conversation

@ryanbr

@ryanbr ryanbr commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Fixes #1001.

Reported as Effort disagreeing with itself on Today: the hero ring read 2.3 while the Key Metrics tile and the HR chart's edge badge both read 0.5.

What was wrong

Effort has two sources. The stored DailyMetric.strain is rewritten only when the heavy daily pass runs; the live in-progress value recomputes today's HR from local midnight to now, and exists precisely because the stored row lags (#402).

Only the hero ring knew about the live value. The tile and the chart badge read the stored column directly. The reporter's HR trace climbs from about 09:00 with the screenshot taken at 12:22, so the row predated the entire morning — the ring counted it, the other two did not.

Recovery and Rest matched across all their read-outs precisely because neither has a live path. That asymmetry is what made the diagnosis unambiguous.

Not a 9.3.0 regression, though it surfaced there. The live hero landed in v4.0.1 on 15 June; the tile has read the stored column unchanged since 14 June. #963 made Effort track an active morning more closely, which widened the gap against a stale row until it became visible — the same change the reporter credits for Effort behaving better.

The fix

StrainScorer.effectiveEffort(live:stored:) is now the single rule, Swift and Kotlin, with twin test suites.

The max in it is not a tie-break. Effort accrues over a day and must never visibly drop; the live recompute can under-read when HR is sparse or a logged workout's load isn't in the raw stream — #489/#506 had a 5/MG user's real 38.3 replaced by a live 0. Worth naming explicitly, because preferring live is exactly what someone would "simplify" this into later.

Android resolves once in TodayScreen and threads it to MetricGrid and HeartRateTrendCard, alongside the resolved values those already accept (caloriesForDay, importedStepsForDay). The near-zero Effort caption turned out to hold a fourth hand-rolled copy of the same expression — it now uses the shared one, and still returns null for past days so the caption stays today-only.

iOS already had effortStrain() as its single point, and its gauge and near-zero note used it — but effortMarker and the Effort StatTile reached past it to the row. Both now resolve through it, and it delegates to the shared rule so the platforms cannot drift.

Verification

  • 7 new tests per platform, same cases in the same order. The Kotlin suite runs green locally against the function extracted verbatim from source: OK (7 tests). The Swift twin runs in swift-packages CI (StrandAnalytics needs macOS here).
  • Cases: live ahead of a stale row, the Import FIT activity steps #489/Restore recognizable Android dashboard metric icons #506 floor, past-day (no live), pre-score (no row), both-nil staying "No Data", a genuine zero surviving as a value rather than an absence, and equal sources being stable.
  • Kotlin: full src/main/java compile — 2517 errors on this branch and 2517 on main, 126 in TodayScreen.kt on both. No new errors.
  • swiftc -parse clean on TodayView.swift, StrainScorer.swift and the new Swift test.
  • Tools/doc_comment_lint.py and Tools/i18n_audit.py --ci both clean.

app-build.yml is disabled, so nothing compiles the iOS/macOS app target — the TodayView.swift edits are parse-checked only and want an app build before this is relied on. The shared rule and its Swift tests are covered by swift-packages.

Re-review

No new defects. What the pass added:

Verified rather than asserted. The earlier "same error count" claim is now a set comparison: the branch and main produce byte-identical Kotlin error sets, so nothing new is hiding behind an equal total. MetricGrid has exactly one call site. On iOS, effortStrain, effortMarker and keyMetricTile are all inside struct TodayView (opens line 171, no top-level close before them), so the helper is in scope at both new call sites — and keyMetricTile already opens with let d = displayDay, making the let effort = … I added the same construct that function already uses. That matters because app-build.yml is disabled and nothing compiles this file.

The chart fix reaches further than the badge. strain feeds four Effort read-outs in OverviewHRChart: the marker's visibility gate, its colour, its label, and the accessibility description, which announced "0.5 Effort now" while the ring showed 2.3. Screen-reader output was wrong in the same way and is fixed by the same change.

Known limit, deliberately not changed. The Effort tile's 14-day sparkline still comes from the stored daily series, so on today it ends at the stored value while the number above it shows the resolved one. On a 0–100 axis in a sparkline that height the difference is around 2% of range — not visible, and substituting the last point means reaching into a shared Window used by other tiles. Recovery and Rest are unaffected because neither has a live path. Left as-is, recorded here rather than discovered later.

Effort was resolved independently in several places and only the hero ring
knew about the live in-progress value. The Key Metrics tile and the HR
chart's edge badge read DailyMetric.strain straight off the daily row, which
is rewritten only when the heavy daily pass runs. The reporter's morning
showed 2.3 on the ring and 0.5 in both of the others, with the HR trace
climbing from about 09:00 and the screenshot taken at 12:22 — the row
predated the whole morning. Recovery and Rest matched everywhere precisely
because neither has a live path.

Not a 9.3.0 regression: the live hero landed in v4.0.1 on 15 June and the
tile has read the stored column unchanged since 14 June. #963 made Effort
track an active morning more closely, which widened the gap enough to see.

StrainScorer.effectiveEffort(live:stored:) is now the one rule, Swift and
Kotlin, with the twin test suites pinning it. The MAX in it is not a
tie-break but the never-drop floor from #489/#506, where a sparse-HR live
under-read replaced a real 38.3 with 0 — worth naming, because it is the
part someone would 'simplify' into preferring live.

Android: resolved once in TodayScreen and threaded to MetricGrid and
HeartRateTrendCard, alongside the resolved values those already take
(caloriesForDay, importedStepsForDay). The near-zero Effort caption had a
fourth hand-rolled copy of the same expression; it now uses the shared one
and keeps returning null for past days so the caption stays today-only.

iOS already had effortStrain() as the single point, and its gauge and
near-zero note used it; effortMarker and the Effort StatTile reached past it
to the row. Both now resolve through it, and it delegates to the shared rule.
@ryanbr
ryanbr merged commit b93479b into main Aug 1, 2026
14 checks passed
@ryanbr
ryanbr deleted the fix/effort-one-value branch August 1, 2026 00:51
DX23876 added a commit to DX23876/noop that referenced this pull request Aug 1, 2026
Upstream work landed after their 9.3.0 tag: the Effort consistency fix (ryanbr#1001,
reported by @bartmuskala), a sleep-stage wake-spelling fix, four importer and
prefs performance passes, and the tools/ -> Tools/ normalisation.

Three conflicts, all resolved by keeping both sides rather than picking one:

  StrandApp.swift / StrandiOSApp.swift
      Both sides add a one-time migration at the same launch point — the fork's
      CoachEntryPrefs.migrateIfNeeded() and upstream's
      PuffinExperiment.migrateContinuousHrvOvernightDefault() (ryanbr#1008). They are
      independent and both must run, so both are kept.

  TodayView.swift
      Upstream replaces `d?.strain` with the resolved `effort` figure so every
      Effort read-out on Today agrees — that is the whole point of ryanbr#1006, so
      upstream wins for value/caption/accent. The fork's sparkline expression
      (keyMetricsDetailed ? windowedSpark("strain") : nil) is a separate feature
      and stays.

d343243 is upstream's own tools/ -> Tools/ normalisation, whose effect this
fork had already applied by hand; the merge absorbed it cleanly, with 0 paths
left under the lowercase spelling and 79 under Tools/.

Verification: macOS universal build green; NOOPiOS build green; 1731 StrandTests
green (up from 1726 — upstream adds EffectiveEffortTests and
SleepStageVocabularyTests); StrandAnalytics 1223 green.

Not verified on hardware: nothing here touches the CoreBluetooth path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Effort on top and strain in key metrics no longer the same

1 participant