Repository navigation
Say a Rest score is provisional instead of hiding it - #2016
Merged
Merged
Conversation
Reported on 11.1.1 Android: at 16:50, for a night that ended at 07:20, the Rest hero read "Pending sync / strap history still offloading" with no number. It reads as the app deciding to recalculate a settled night. It is not doing that, but the state is wrong for a different reason. #1164 asked for a provisional Rest to be surfaced honestly rather than shown as a confident number that later jumps. The implementation withheld the number entirely, on both surfaces: the hero ring drew an empty track with a two-line overlay, and the Rest tile rendered a bare dash. So a user who had slept, and whose score had been computed, saw nothing at all for as long as the strap had anything left to send. On a strap that banks continuously that is most of the day, every day. Hiding the value is a stronger response than the problem needs. A number that may still move is not the same as no number, and it is the one the user came to the screen for. Both surfaces now render the score and SAY it is provisional beside it: the hero carries a one-line caption under its domain label, and the tile keeps its existing caption slot with the number restored above it. That also fixes what the overlay did to its own explanation. It was laid out against the ring's width, so it spilled past the vessel's edge and ellipsised mid-word, reaching the reporter as "strap history still o...". The caption sits under the label at the COLUMN's width, where the text has room, and it cannot cover the score by construction. The predicate is untouched. `restPendingSync` still means what it meant, is still today-only, and is still gated on a score existing. Its TRIGGER is a separate question and deliberately not changed here: it fires on "the strap is ahead of us at all" or "an offload is running", neither of which is scoped to the night being scored, so post-wake daytime records can raise it. Which of the two fired in this report cannot be told from a screenshot, and narrowing it wants a strap log first. Swift resolves the state once in `restIsPendingSync` and both surfaces read it, so the hero and the tile can no longer disagree about the same moment. No new strings on either platform: the caption reuses the existing "Pending sync" key, and the tile keeps the longer one it already had. Android UI suites 984 green, i18n audit clean. On-device QA still wanted for the caption's fit at large font scales and in the longer locales.
5 tasks done
…ocs teaching the old rule Two things from re-reviewing the change. The caption sat between the domain label and the provenance badge, so a Rest column carrying one would have pushed its badge a line below Charge's. The badges read as a row across the three columns; one of them dropping is exactly the kind of drift the caption is not worth. It now renders last, under the badge. Both rows are top-aligned and self-sizing, so a caption grows the row and leaves every ring and badge where it was. Android was already unaffected, since its source badge is owned by the hero card rather than the column, but the ordering now matches. The larger one: `restPendingSync`'s own documentation still described what it used to do. It said surfacing the state "rather than a confident number", and the mirrored tests were named and commented around suppressing a number and never showing a score. Nothing failed, because the predicate itself is unchanged. But that is how the behaviour comes back: the next person to touch this reads that the flag means withhold, and restores it. Both doc comments and both test suites now say what it means, that true captions a score and never hides one, and why. Android UI suites 984 green; doc lint and the i18n gate clean.
…t this reverses #1164 Two more from re-reviewing. The caption was left to size itself. Android's three hero columns are laid out at a fixed width, so a caption there can only fill its own column; these SwiftUI columns take the width of what is in them, so a longer translation of "Pending sync" would have widened the Rest column and tipped the trio off centre. It is now bounded to the ring's width, two lines, with the same shrink allowance the domain label above it already uses. The second is about the description of the change rather than the code. The PR body said the earlier work "withheld the number entirely" as though that overshot what was asked. It did not: #1164's proposal was to surface Rest as pending sync RATHER THAN a provisional number, and that is what shipped. This commit's parent reverses that decision, and the PR now says so in those words, with the reasoning and the alternative, so it is a call the maintainer makes rather than one smuggled in as a defect report. The reasoning, briefly: the approach was sound for a state expected to be brief, and the trigger turned out not to be brief. It fires on "the strap is ahead of us at all" or "an offload is running", so on a continuously banking strap it holds for most of the day, and hiding a computed score for most of the day is a worse failure than a number that moves once with a note saying it might. If the trigger is narrowed to the night being scored, hiding becomes defensible again, and this is reversible.
… whole sentence on its tile Found by auditing the two platforms against each other. The hero caption was rendered with AutoSizeValue, which is maxLines = 1 with soft wrap off and an ellipsis. It shrinks to its floor and then cuts the text off mid-word. That is exactly what the old in-ring overlay did, and exactly what moving the caption out from over the ring was meant to stop, so it would have come straight back through the component on a longer translation. "Pending sync" fits in English at any scale, which is why it was invisible here. It is now a plain wrapping Text with two lines, matching the iOS side, which already wraps. The Rest tile said only "strap history still offloading" where iOS says "Pending sync · strap history still offloading". The second half alone reads as a fragment, and more so now that a real number sits above it rather than a dash. Composed from the two strings already shipped rather than adding a third that would need four translations. Not changed, and not mine: the Android Rest tile still has no caption at all when it is NOT pending, where iOS shows the night's duration and efficiency. That gap predates this work and wants its own issue. Android UI suites 984 green; the i18n gate and doc lint clean. Android-only, so the Swift build already running on the parent covers the Swift half of this branch unchanged.
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Sep 16, 2026
Thirty real conflicts. The Android tree is dropped again per FORK_GUIDE; every other hunk was resolved by asking which mechanism this fork actually runs, not by preferring a side. Taken from upstream: • Protocol: ryanbr#2017/ryanbr#2018/ryanbr#2020 — the 5/MG historical optical and raw-IMU layouts, and the v26 window's ABSOLUTE base code beside its deltas. Its migration lands as `v57-ppg-waveform-base-code` (upstream's v44), since this fork's sequence is nine ahead of theirs. • BLE diagnostics ryanbr#1998-ryanbr#2003: a bond end that says WHY, named BLE integers, a session-budgeted hex dump, and "we hold the link" only when the OS says so. • Battery: ryanbr#1951 stop asking a 5/MG for a reading it refuses; ryanbr#1959 poll 0x2A19 for an unbonded one. • Scoring: ryanbr#1978/ryanbr#1982/ryanbr#1990 the RHR/HRV baseline gates, ryanbr#1977 the strain denominator's domain, RMSSD withheld on an over-counted night, ryanbr#1943 the resting-HR bin gate, the closed final HR/HRV window, HR-recovery requiring one CONTINUOUS effort, ryanbr#1984 nights bridged under minSleepMin, ryanbr#2015 motion re-folded only when that day's gravity moved, ryanbr#2016 a provisional Rest. • Charts ryanbr#2004/ryanbr#2005/ryanbr#2007/ryanbr#2009/ryanbr#2011 and ryanbr#1995's hero-ring detail route, ryanbr#1164's "Pending sync" caption under Rest. • Widgets: the heart-rate widget with its trace (ryanbr#1957/ryanbr#1964) and the widget cost counters. • Health: ryanbr#1503 — the Apple Health external UUID drops the strap id, so a re-pair no longer strands every prior record, plus the one-off sweep that reclaims the stranded ones; ryanbr#1205 GPS routes read back from Apple Health. • Storage: rolling retention for ppgWaveformSample, the skin-temp absolute backfill outside the 21-night window, per-table byte estimates. • Units split (body vs exercise distance), per-day wake overrides, the wind-down nudge pinned to its own day, and the German Sie→du conversion (29 entries, merged per LOCALE so the fork's own de edits stayed). Kept on the fork's side, with the reason: • ryanbr#1005's in-memory `dayScanCache` stays out, again: the persisted `DayScanFingerprint` survives relaunch and its key folds the per-day weight and max HR upstream's does not. • The steps-motion cache stays PERSISTED for the same reason. Upstream's failed-read-vs-empty-day distinction IS adopted into it — on a persisted cache a stale zero from a read error would outlive the process that made it. • Every stream read in the day loop keeps `limit: Int.max`. All five are uncapped together; taking StreamReadCap for three of them would leave the loop half-capped, which is worse than either choice. • The coach engine. Upstream's K1-K15 layer is not adopted: its single 40-row `coachMessage` table, its tool-less pre-baked context and its own brief are behind what this fork already ships. Its ADDITIVE half is kept — the on-device voice input, the Siri "Ask Coach" intent, the brief scheduler and brief widget, and streaming for the OpenAI-compatible providers this fork's Anthropic-only StreamingToolClient does not cover. They are wired up in the next commit; `CoachBriefWidget` is deliberately not registered until then. • Liquid's `DomainTheme` labels, value-sampled Charge tint, `LiquidScoreGauge` and `TodayCardSurface`; classic Today's ring order and `LiveEffort.today`. • RootTabView's four-tab shell and `MoreCatalog`: Coach hangs on content here, not on a tab. Two duplicate mechanisms collapsed rather than carried: • `ChargeBreakdownWiring` (upstream) and `ChargeBreakdownFormat` (fork) are the same extraction done twice. The fork's survives, Coupled now reads it, and upstream's TEST is kept — it is coverage this path never had. • Upstream's `CoachMessageStore` is dropped; `CoachConversationStore` already autosaves the whole multi-conversation history. One defect found while resolving, fixed here: `LiveEffort.today` read HR with no explicit limit, so `hrSamples`' 8000-row default truncated it — at ~18k rows a day that covered about ten hours and the live Effort silently stopped climbing. It failed safe (`effectiveEffort` takes the max), which is why it went unseen. `Live5RestFrozenTests` is removed by upstream, which replaces it with `Live5NoGravityRestChainTests`. `local_day_windows_oracle.json` moves out of upstream's android path into `Tests/StrandAnalyticsTests/Oracles/` — NOT `Fixtures/`, which this repo's .gitignore excludes everywhere. Verified: StrandAnalytics 2233, WhoopStore 627, WhoopProtocol 700, StrandImport 293, StrandDesign 120, NoopLocalAccess 14, SemanticMemory 19, OuraProtocol 209 and PolarProtocol 25 all green; macOS and iOS both build; doc_comment_lint clean; `i18n_audit --ci` byte-identical to its pre-merge output (it is red on main for reasons that predate this, and the seven new widget literals were given all nine locales so the gate did not get worse). Six StrandTests were ALREADY failing on main before this merge and still are — `CoachMyLogsAndZonesTests` (the tool census pin was not bumped when the Hevy tools landed) and five in `RawPhysiologyUnionTests`. Every input to them — CoachTools.swift, ToolConsent.swift, Repository.swift and both test files — is byte-identical to main, so this merge neither caused nor fixed them. They went unnoticed because app-build.yml last ran on 2026-08-25. Analysis migration required: YES. The scoring changes above make stored derived values stale; the recipe bump and its resumable migration are the next commit.
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
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.
Reported in #2012.
What was happening
At 16:50, for a night that ended at 07:20, the Rest hero read "Pending sync / strap history still
offloading" with no number. It reads as the app deciding to recalculate a settled night. It is not
doing that, and nothing is re-derived by that message, but the state is wrong for a different reason.
#1164's proposal was to "surface Rest as pending sync rather than a provisional number", and that
is exactly what shipped: the number is withheld entirely, on both surfaces:
So a user who had slept, and whose score had been computed, saw nothing at all for as long as the
strap had anything left to send. On a strap that banks continuously that is most of the day, every
day.
This reverses #1164's chosen approach, deliberately
Worth stating plainly rather than filing as a defect. Withholding the number was the right answer for
a state expected to be brief. The trigger turned out not to be brief: it fires on "the strap is ahead
of us at all" or "an offload is running", so on a continuously banking strap it holds for most of the
day. Hiding a computed score for most of the day is a worse failure than a number that moves once
with a note saying it might.
If the trigger is narrowed to the night being scored (see below), the state becomes short again and
withholding is defensible once more. This change is reversible on its own if you would rather keep
#1164's approach and wait for a log to narrow the trigger first.
Change
Hiding the value is a stronger response than the problem needs. A number that may still move is not
the same as no number, and it is the one the user opened the screen for. Both surfaces now render
the score and say it is provisional beside it:
translation cannot widen the column and tip the three rings off centre;
That also fixes what the overlay did to its own explanation. It was laid out against the ring's
width, so it spilled past the vessel's edge and ellipsised mid-word. The reporter's screenshot shows
it arriving as "strap history still o...". The caption sits under the label at the column's width,
where the text has room, and it cannot cover the score by construction.
Swift resolves the state once in
restIsPendingSync, read by both surfaces, so the hero and the tilecan no longer disagree about the same moment. Android already passes one
restPendingSyncvalue toboth.
What is deliberately NOT changed
restPendingSyncitself. It still means what it meant, is still today-only, and is still gated on ascore existing. Its trigger is a separate defect and wants evidence first: it fires on
historyPendingSync("the strap has banked HR more than five minutes newer than our newest localrow") or
backfilling("an offload is running"). Neither is scoped to the night being scored, sorecords banked long after wake can raise it, and nothing offloaded after 07:20 can change a score for
a night that ended then.
Which of the two fired here cannot be told from a screenshot. Live HR is clearly streaming in it
(101 bpm, "Now"), which should keep the HR frontier at roughly now and close
historyPendingSync'sgap, so
backfillingis the likelier of the two, but that is inference. Narrowing the triggerproperly means comparing our ingest coverage against the scored night's end, and it should be done
against a log rather than a guess. Asked for one on the issue.
Verification
compileFullDebugKotlinclean.Tools/i18n_audit.py --ci origin/mainclean;Tools/doc_comment_lint.pyclean.tile keeps the longer string it already had, and both remain referenced.
Reported by @bartmuskala.