Repository navigation
feat(charts): space points by date, so a gap takes the width it spans - #2007
Merged
Merged
Conversation
#2004 broke the line across a missing day, but points are positioned by reading index, so a gap has ZERO width. The break then reads as a chopped line and an isolated day as an orphan dot floating in space, which is what the reporter saw on the month range. The break only means something once the gap has width. LineChart already accepted per-point timestamps for its scrub read-out. Those now position the points too, so a four-day gap occupies four days of width. Opt-in: without timestamps every existing chart keeps the even spacing it has always had. It refuses rather than guesses. A length mismatch, a zero span, or a non-ascending sequence all fall back to index spacing, and the day-key conversion is all-or-nothing, because positioning some points by time and the rest by a fallback is a worse lie than either rule applied consistently. The tap and drag hit-testing now takes the SAME fractions the geometry used. Deriving the nearest index from a uniform step, which is what it did, would have selected the wrong reading the moment spacing stopped being uniform: the chart would have highlighted one day and labelled another. That also retires the count-based helper, which nothing calls now. The positions are computed once and remembered rather than per gesture event: the drag handler fires every frame and this allocates a list the length of the series. The gesture handlers are keyed on that value too, because keying them on the values alone would let a timestamp change leave them hit-testing against stale positions. MultiLineChart keeps index spacing and says so: its series are index-aligned to each other rather than to a shared clock.
ryanbr
force-pushed
the
feat/chart-date-spacing
branch
from
September 8, 2026 11:57
45cfdb2 to
9e5f4ca
Compare
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.
feat(charts): space points by date, so a gap takes the width it spans
#2004 broke the line across a missing day, but points are positioned by reading
index, so a gap has ZERO width. The break then reads as a chopped line and an
isolated day as an orphan dot floating in space, which is what the reporter saw on
the month range. The break only means something once the gap has width.
LineChart already accepted per-point timestamps for its scrub read-out. Those now
position the points too, so a four-day gap occupies four days of width. Opt-in:
without timestamps every existing chart keeps the even spacing it has always had.
It refuses rather than guesses. A length mismatch, a zero span, or a non-ascending
sequence all fall back to index spacing, and the day-key conversion is
all-or-nothing, because positioning some points by time and the rest by a fallback
is a worse lie than either rule applied consistently.
The tap and drag hit-testing now takes the SAME fractions the geometry used.
Deriving the nearest index from a uniform step, which is what it did, would have
selected the wrong reading the moment spacing stopped being uniform: the chart
would have highlighted one day and labelled another. That also retires the
count-based helper, which nothing calls now.
MultiLineChart keeps index spacing and says so: its series are index-aligned to
each other rather than to a shared clock.
Why this is the missing half
Breaking the stroke on a missing day is only legible if the gap has width. Shipping the break first made
the month range look worse, not better, and that is on me: I deferred spacing as "a bigger change" and
then shipped the thing that depends on it.
Tests
+9. The reported shape is pinned directly: 3 Sep, 5 Sep, 6 Sep puts the first point at 0, the second at
two-thirds, the third at 1, so the two-day gap is twice the one-day step. Plus all three refusal paths, the
all-or-nothing day conversion, and that fractions always span the full width in ascending order.
674 classes / 5714 tests, 0 failures. Android-only: the iOS TrendChart already positions by date.