Skip to content

Feat/UI rebuild - #253

Open
abdulsaheel wants to merge 104 commits into
mainfrom
feat/ui-rebuild
Open

Feat/UI rebuild#253
abdulsaheel wants to merge 104 commits into
mainfrom
feat/ui-rebuild

Conversation

@abdulsaheel

@abdulsaheel abdulsaheel commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

User description

rebuilt entire ui


PR Type

Enhancement, Bug fix, Tests


Description

  • Complete UI replacement: lib/ui removed, lib/ui2 introduced with full metric fidelity (confidence, tier, absence handling)

    • Fixed Metric tier parsing ('AUTH' not 'AUTHORITATIVE'), Poincaré row rendering, flat-series chart floor, and gen5 step counter wiring
    • Workout avg HR now banked to sessions.avg_hr; crossDayArtifactUsableToday now respects algo_version
    • Circadian rhythm screen wired to hourly HR means (raw accel pruned at 3 days); unproducible metrics (blood pressure, power zones) deleted
  • Schema bumped to v38, kAlgoVersion bumped to 66

  • New HrSensorLink: GATT Heart Rate Service (0x180D) support for session-scoped external HR sensors, stored in new external_hr table, never merged into baseline analytics

  • LiveDraft persists session state (sets, laps, score, pause clock) to Prefs so minimising or killing the app cannot lose typed workout data


Diagram Walkthrough

flowchart TD
  OldUI["lib/ui (removed)"]
  NewUI["lib/ui2 (new)"]
  LiveDraft["LiveDraft\n(Prefs-backed session state)"]
  LiveShell["LiveShell\n(session clock + transport)"]
  ActivityHost["ActivityHost\n(app seam for live screens)"]
  HrSensor["HrSensorLink\n(GATT 0x180D)"]
  ExternalHrTable["external_hr table\n(new, schema v38)"]
  DB["LocalDb / db.dart"]
  DE["DerivationEngine\n(kAlgoVersion 66)"]

  OldUI -- "replaced by" --> NewUI
  NewUI -- "uses" --> LiveShell
  LiveShell -- "reads/writes" --> LiveDraft
  LiveShell -- "wired via" --> ActivityHost
  ActivityHost -- "feeds" --> HrSensor
  HrSensor -- "flushes to" --> ExternalHrTable
  ExternalHrTable -- "owned by" --> DB
  DB -- "schema v38 migration" --> DE
Loading

File Walkthrough

Relevant files
Enhancement
14 files
live.dart
New live session screens with honest absence and draft persistence
+2231/-0
hr_sensor.dart
New GATT Heart Rate Service link for session-scoped external sensors
+314/-0 
grammar.dart
Shared UI grammar primitives for ui2 screens                         
+2297/-0
theme.dart
Design tokens and theme for ui2                                                   
[link]   
home_screen.dart
Rebuilt home screen wired to full metric fields                   
+1599/-0
health_screen.dart
Health screen with confidence-aware metric rows                   
+1413/-0
readiness_detail.dart
Readiness detail screen with tier and confidence display 
+414/-0 
metric_detail.dart
Generic metric detail screen for trend and confidence       
+1267/-0
summary.dart
Activity summary screen with retry-save and enriched result
+1863/-0
catalogue.dart
Activity catalogue and archetype definitions                         
+324/-0 
db.dart
Schema v38 migration: external_hr table and sessions.avg_hr column
+2926/-430
nutrition_store.dart
Nutrition store for food logging persistence                         
+575/-0 
off_lookup.dart
Off-wrist lookup helper for substrate filtering                   
+472/-0 
coach_actions.dart
Coach action definitions for AI coach integration               
+397/-0 
Bug fix
4 files
charts.dart
Chart widgets with correct flat-series and absence rendering
+1222/-0
sleep_detail.dart
Sleep detail screen with honest stage absence handling     
+1650/-0
workout_screen.dart
Workout screen wired to sessions.avg_hr and full result   
+1783/-0
derivation_engine.dart
kAlgoVersion bumped to 66; crossDay artifact algo_version fix
+1996/-249
Tests
2 files
ui2_activity_test.dart
Tests for live activity screens and draft persistence       
+1555/-0
ui2_wiring_r2_test.dart
Wiring tests for ui2 metric field propagation                       
+1321/-0
Additional files
101 files
build.yml +21/-14 
PRIVACY.md +116/-49
UI_AUDIT.html +0/-762 
AndroidManifest.xml +43/-5   
MainActivity.kt +14/-0   
NativeChannels.kt +55/-0   
OpenStrapBatteryWidgetProvider.kt +29/-12 
OpenStrapWidgetProvider.kt +47/-27 
PhoneStepCounter.kt +368/-0 
StrapWidgets.kt +96/-21 
launch_background.xml +4/-9     
launch_background.xml +4/-9     
widget_bg_char.xml +2/-2     
widget_bg_paper.xml +2/-2     
widget_openstrap_nodata.xml +35/-0   
colors.xml +5/-0     
styles.xml +1/-1     
colors.xml +8/-1     
styles.xml +1/-1     
widget_strings.xml +2/-2     
data_extraction_rules.xml +29/-0   
notice.html +22/-0   
privacy.html +43/-3   
TASKER_INTEGRATION.md +38/-1   
OpenStrapIntents.swift +44/-11 
OpenStrapWatchApp.swift +122/-65
WatchMetrics.swift +66/-40 
WatchStore.swift +11/-11 
OpenStrapWatchWidgetBundle.swift +0/-146 
OpenStrapBatteryWidget.swift +116/-77
OpenStrapBreathingLiveActivity.swift +27/-17 
OpenStrapWidget.entitlements +0/-10   
OpenStrapWidget.swift +272/-176
OpenStrapWidgetBundle.swift +0/-1     
OpenStrapWidgetControl.swift +0/-54   
OpenStrapWidgetLiveActivity.swift +58/-30 
project.pbxproj +52/-18 
Runner.xcscheme +18/-0   
AppDelegate.swift +165/-3 
Contents.json +116/-0 
Contents.json +6/-6     
Contents.json +0/-23   
README.md +0/-5     
LaunchScreen.storyboard +14/-11 
HealthRoutes.swift +173/-0 
Info.plist +14/-1   
WatchBridge.swift +15/-15 
ai_prefs.dart +22/-0   
briefing.dart +14/-3   
briefing_engine.dart +155/-33
nightly_sweep.dart +203/-0 
reminder_plan.dart +17/-7   
app.dart +447/-266
ble_engine.dart +729/-175
ble_state.dart +361/-3 
live_cadence.dart +68/-0   
live_step_runs.dart +199/-0 
proximity_policy.dart +0/-262 
cloud_import.dart +14/-3   
coach_engine.dart +160/-2 
coach_prompt.dart +122/-100
background_derivation.dart +8/-3     
crossday_pipeline.dart +509/-83
derive_prepare.dart +79/-11 
findings.dart +184/-0 
hr_max.dart +100/-0 
manual_session.dart +51/-30 
onehz_pipeline.dart +584/-100
profile.dart +11/-4   
strain_backfill.dart +73/-30 
substrate.dart +338/-25
csv_export.dart +192/-11
journal_fields.dart +79/-0   
live_coverage_policy.dart +166/-0 
local_repository.dart +87/-36 
local_repository_impl.dart +1579/-828
med_store.dart +399/-0 
models.dart +125/-2 
route_types.dart +38/-0   
health_export.dart +33/-16 
health_import_state.dart +88/-0   
health_measurement_import.dart +144/-0 
health_rhr_seed.dart +281/-0 
health_workout_import.dart +313/-0 
phone_pedometer.dart +139/-73
backup_crypto.dart +349/-0 
journal_csv_import.dart +271/-0 
whoop_import.dart +13/-34 
metric.dart +106/-0 
payloads.dart +27/-2   
battery_forecast.dart +27/-5   
device_alerts.dart +101/-29
notification_center.dart +147/-88
notification_event.dart +53/-0   
notification_prefs.dart +34/-7   
notification_service.dart +117/-29
tap_router.dart +30/-1   
water_buzzer.dart +10/-7   
app_icon.dart +86/-0   
tasker_bridge.dart +53/-0   
Additional files not shown

lib/ui is gone, lib/ui2 replaces it. the old one threw away five of the six
fields analytics emits, so every number rendered bare — no confidence, no
tier, no reason when it was absent.

real bugs this turned up, all live before now:
- metric.dart matched 'AUTHORITATIVE' but analytics emits 'AUTH', so that
  tier always parsed to unknown
- every poincare row rendered empty (heart['irregular'] is a plain map read
  through envValue, and the fixture had the wrong shape so it hid it)
- workout avg hr was lost after 3 days, recomputed off the pruned substrate.
  banked to sessions.avg_hr now
- crossDayArtifactUsableToday ignored its own algo_version stamp
- a flat series drew pinned to the floor of the card
- gen5 stepMotionCounter never reached Substrate

circadian was written, tested and called by nothing. wired it on hourly hr
means (raw accel is pruned at 3 days) and said so in the envelope note.

things the app can't actually produce are deleted rather than explained —
blood pressure, whole-day energy, power zones. docs/internal/UI_ROADMAP.md
has what each would need.

schema 38, algo 66.
MetricRow only drew ConfDots when there was no sparkline, and when it did it
drew a hardcoded Conf.none instead of the conf the caller passed. so every
health row — resting hr, hrv, the lot — showed three empty dots no matter what
ConfX.of(m) actually said. two goldens had baked that in.

conf is nullable now. null means confidence doesn't apply to this row, which
is the honest answer for anything the user typed in themselves; three green
dots on a hand-logged meal is a claim we can't make.

also noted in the golden test that they were baked on flutter 3.44.9 — an
older sdk anti-aliases hairlines differently and fails a few of them on
nothing but sub-pixel blend.
seven agents audited edge, analytics and protocol; this is the edge half.
findings are in docs/internal/sweep/.

decoded_onehz's sensor columns were NOT NULL, so a missing sample had nowhere
to live and got written as a real zero. absent accelerometer became (0,0,0),
which enmo, immobility and auto-workout all read as a perfectly still wrist —
accelPresentAt existed and had exactly one consumer. schema 39 makes the six
sensor columns nullable and the readers absence-aware. the optical and thermal
columns were the same shape of bug but less harmful in practice, since every
adc reader already gated on v > 0.

a single nan was destroying the whole crossday bundle for every user, days 3
to 8. readiness_glassbox emitted percentileOfYou = nan under 7 days of history,
jsonEncode threw, and the catch dropped illness, anomaly, ctl/atl/tsb,
chronotype, sleep coach, vo2max and every percentile, leaving a debug line.
sanitizeForJson drops the offending key now, records it, and the catch is loud.

the rollup carried no algo_version and no date and nothing checked either, so
weeks-old numbers under an older algorithm looked exactly like today's. it's
stamped on write and gated on read, and withheld is said out loud rather than
dressed up as a cold start.

charts were drawing pictures the data didn't support. nothing carried a
timestamp, so a 30-day view over 22 stored days drew 22 evenly spaced points
and ran the line straight through the missing week — series are dense now, one
slot per calendar day, null for a day that didn't derive. AxisSpec.of looped
forever on a non-finite value. a flat week drew pinned to the card floor. fill
over a min-anchored scale turned 58 to 60 bpm into a mountain. four painters
still take compacted series; that's tracked, not done.

dangerousCmds was only enforced in _send, so nine call sites reached _write
straight past it. the block lives in _write now with one audited opt-in —
setFfValue, which the R22 sequence bypasses deliberately.

the ui rebuild had orphaned working features: the alarm had no screen while the
band still fired it off its own rtc, the widget lost its foreground refresh,
four notification routes landed on home, and swiping back out of a live session
threw away every logged set and then blocked all future workouts.

the app also explained itself far more than it said anything — 69 statuscards
against one trendcard and zero recommendations. cycle was fully built in the
data layer with no ui at all. nutrition couldn't delete. that's started, not
finished.

22 notification kinds down to the three we actually sanctioned. headless gate
has a ceiling. two derivations can no longer race.

schema 39, algo 67. needs the new analytics and protocol shas pinned.
six more agents, an adversarial pass over the last round, and the findings in
docs/internal/sweep/. the adversarial pass refuted 18 of 44 claims from round
one, which is roughly the rate i'd expect and the reason it ran at all.

privacy first, because it's the only place the code contradicted a public
promise. PRIVACY.md claimed a settings toggle for health-data contribution and
setHealthShareConsent had zero callers — its ui died with lib/ui. fresh
installs default off and were safe; an upgrade carrying true uploaded the whole
gzipped db daily with no way to stop it. and refreshAppStatus GET'd the
companion server on every launch and every foreground, gated by nothing, with
COMPANION_URL baked into releases. there's a toggle now, an update-checks
switch, fetchStatus returns null unless sideload ota is on, and PRIVACY.md says
what the code actually does.

"reset all data" left about twenty tables standing and cleared two prefs.
wipeAll enumerates sqlite_master now, so the next table someone adds can't
escape it. export existed with zero callers while the reset dialog said "export
first" — it's in settings now, csv and whole-db, with the exclusions listed on
screen rather than silently dropped.

two ways to brick the install: _init had no try/catch and was called unawaited,
so any throw left a spinner forever with no retry; and the migration ladder was
one transaction with no guard, where a failure meant reinstall. init failure is
a state you can recover from now, and an unopenable db is quarantined under a
unique name, rebuilt, and merged back — hand-typed tables first, since those
are the ones nothing can recompute. if the salvage also fails the original is
renamed back.

the app knew what was wrong and threw it away: lastError, spotError and six ble
flags rendered by nothing, a bluetooth permission denial reported as "no band
in range", a db read failure shown as "nothing derived yet" on nine screens.
there's one BandStatus now, built by one pure function, so the phone, pairing
and devices rows can't drift into three answers.

it also grew forever. the 3-day prune only ran under scope.fullHistory, which
is the manual re-analyze button — 12.3 mb/day, about 1.1 gb at 90 days. and
sqlite hands freed pages to the freelist, not the filesystem, so there's a
vacuum behind a freelist threshold. substrate moved to Float64List: 83 to 48.6
mb held, bit-exact, no derived number moves. health export held 400 decoded
bundles at once — measured 298 mb — and now holds one.

accessibility: three cards overflowed at 2.0x in the exact places goldens
existed, passing only because every fixture was two characters. fixtures are
realistic now and there's a non-golden sweep to 3.1x that fails on any
overflow. chart palettes went through the contrast solver instead of raw
pigment — the hypnogram's light lane measured 1.67:1. expand:false silently
dropped the 44pt guarantee on twelve controls including a destructive one.

widget extension was pinned to ios 26.5 with 16.0-era code, so widgets and live
activities were dark for nearly everyone. four different readiness bandings
across phone, widget and watch — 65 was green, orange and yellow depending on
where you looked. one banding now, read from the phone.

324 of 348 goldens moved. p.on solves twice now, against card2 and against
wash(accent) over it — five of six accents measured 4.30-4.49 on the tinted
surface nothing was checking.

algo 67 and schema 39 both unchanged; everything here is bit-exact or
scheduling. still needs the analytics and protocol shas pinned.
five agents. the widget contract, the live session, the band's own failures,
and every word on screen.

the widget was shipping grey. a native pass had correctly deleted all four
duplicated readiness-banding tables — 65 rendered green on the phone, orange on
the widget and yellow on the watch — but the dart side that publishes
readiness_tier and readiness_band didn't exist yet, so every surface fell back
to unlabelled. it publishes both now, key names checked against all four
readers.

the widget also couldn't go stale. has_data is a bool frozen at push time, so a
phone that hadn't synced in a week showed week-old readiness looking like this
morning's, and updated_at was published and read by nobody. freshness is
computed at render time now, and the timeline emits a second entry at the
deadline so widgetkit flips it with no process wake. 26h for metrics, 24h for
battery — a readiness score describes a night that stays true all day, a
battery percentage describes now. also fixed a 2038 bug on the way: updated_at
read through Int overflows negative, which would have made every widget
permanently stale.

the live session re-laid-out its whole body, RouteMap included, every second of
a workout: 192 rebuilt widgets per tick, now 21. strength opts out entirely,
but its one live element asks for the tick itself so heart rate doesn't freeze.
the picker built 67 activity rows nobody was looking at.

the app knew what was wrong with the band and threw it away. one BandStatus
built by one pure function now, so devices, pairing and home can't drift into
three answers. pairing checks the blocker BEFORE the string matcher, because
adapterOff matches none of its phrases and a switched-off radio was being
demoted to a band fault. lastError and spotError deleted — computed, rendered
nowhere.

copy: always-on prose down 20%, absence copy down 41%, statuscard "why" from
17.6 words to 9.7 across 74 cards. but the real fix wasn't editing text: _Cta
drew a tappable arrow whether or not onFix existed, which is why 60% of cards
had a dead link and why the prose had stretched to compensate. one bool.
settled the vocabulary too — band not strap, readiness not recovery, logged
when you type it and recorded when the sensor catches it.

three bugs this round were invisible because the fixture was too tidy:
SourceRow overflowed for any status longer than "Connected" and every fixture
was connected:true. same shape as the two-character cards and the evening-only
greeting. fixtures get the longest realistic value and the unhappy branch now.

also: go() was a raw PageRouteBuilder, which has no back-gesture machinery, so
ios edge-swipe-back was dead on about twenty screens.
you looked at them on the phone and said no. gone: the widget, the Conf enum,
the ConfX tier mapping, the conf parameter on SignalCard/TrendCard/MetricRow/
ChartFrame, and all 32 render sites.

the tier and confidence stay in the envelope — this was only ever a rendering
choice, and lib/data, lib/compute and lib/models are untouched.

taking the parameter out (rather than just the render) exposed a chain of dead
code behind it: _confOf, confOfEnv, chronotypeConf, cosinorConf, and
MetricData.latest — which was the only caller of todayMetric(), which was the
only reason metric detail did a getToday() fetch at all. that fetch is gone
too.

absence is a separate rule and is untouched. still no bare em-dash, still a
StatusCard with a reason. one place needed care: in readiness_detail the dots
WERE the absence affordance for a missing contribution, so that now renders
nothing rather than a dash — the sub-line above it already names the case.

the prose caveats matter more now that nothing else carries the distinction, so
they stay: skin temp isn't calibrated so there's no °C, deep is the weakest of
the three stages, MET is estimated.

194 goldens moved, 4 deleted. 2358 tests.
removed, all your call:
- the export-exclusions paragraph on the data screen
- "everything stays on this device" on profile
- the workouts / records / days / sources tile
- activity history (it was a count with no destination anyway)
- edit profile from more settings — it lives in quick access now, one door
- "stages are a low-confidence estimate" and its "how this is computed" link

taking the tile out made four ProfileStats fields dead, and they weren't free:
they cost a getRecords() and a whole year-of-workouts query every time the
profile opened. both gone with them.

the staging caveat still exists where it belongs — metric_detail's method text
says a wrist sensor can't see slow-wave activity, and that's the place someone
goes when they want to know.
one question — how did my night go — answered in three seconds, then revealed
by scrolling. total sleep, onset to wake, in bed and how much of it you were
asleep. then the hypnogram, then stages, then how it sat against your own
nights, then anything that stood out, then the signals underneath, then one
thing to do tonight.

no sleep score. there's no composite in the pipeline and inventing one here
would mean picking weights in a ui file. what replaces it is the comparison
every "86" is a lossy summary of anyway: last night against the middle half of
your own recent nights, per measure, with the night count attached. a quartile
band needs no thresholds and no population norms — it's your own distribution,
so it can't be wrong about somebody it was never fitted to. 7 nights before it
compares at all, 14 before it calls anything extreme, and last night is cut out
of its own baseline.

dropped from the brief: the score and the quality bar, confidence dots (you had
those removed a commit ago), sleep replay, spo2 (decoded but never validated,
every stored value is null), the data-quality block, and possible contributors
— getJournalInsights correlates day-level readiness against journal tags at a
4-day floor, so there's nothing per-night that would survive being asked how it
knows.

merged the brief's sections 4 and 5: "how did the night go" and "what was
different" were the same four comparisons twice. the bar is the delta and the
sentence is the verdict.

two real bugs on the way. the breathing lane was dotted on every single night —
the grid was sized to 1-minute heart rate while breathing and temperature are
5-minute, so real samples read as dropouts. and the hypnogram drew one rect per
column with a 0.8pt gap between neighbours, so a solid two-hour stretch of
light sleep came out as a picket fence; it draws runs now, with a riser at each
transition, which is what a hypnogram actually is.

history is three metric_series scalar reads and one window query, not 28 day
bundles.
the overnight footnote is gone. the legend already names each lane and its
unit, and the lanes are visibly separate — the paragraph was describing the
picture instead of letting it do its job.

steps: a real 0 already rendered as 0 (isEmpty is value == null, so an unmoved
counter is a reading, not an absence). what you were seeing was genuinely
nothing recorded — gen4 has no pedometer, so it's phone steps or nothing. it no
longer swaps the tile for a paragraph about wrist motion: the tile stays and
says None / NOT RECORDED. the answer to "how many steps" is short either way.
you asked why 71.6 never moves when you switch 7/30 days, and whether there's a
today. i had an agent trace it with a real harness rather than by reading:
7 days of synthetic 1hz raw, today truncated mid-afternoon like a real
mid-day look, through the actual DerivationEngine. today derived clean —
partial:0, and the same 24 metric_series keys every other day gets. all four
structural suspects ruled out: today isn't filed partial, it's scheduled FIRST
when it has raw, metric_series has no version/finalized/skipped column to
filter it out, and the local-noon stamp lands it in the right slot.

so it was never a data bug. both symptoms were the same presentational one: the
headline was UNDATED. it named its day only when it was stale, so a fresh
number sitting under 7/30 tabs told you neither that it was today's nor that it
wasn't a summary of the range you'd picked. it now always says "Latest reading ·
Today", or "· 3 days ago" when that's the truth. the range still changes the
chart and the lowest/typical/highest block, which is what it was always doing.

also fixed the x-axis off-by-one — slot 0 of a 30-slot window is 29 days back,
not 30.

71.6 was false precision. one metricValue helper now: bpm, ms and % whole,
br/min and degrees to a tenth, minutes as 7h 30m, steps and kcal grouped.
metric_detail and health_screen both route through it, so the same reading
can't read 71.6 on one screen and 72 on the card that links to it. a delta that
rounds to nothing says "in line with your 28-day average" instead of drawing an
arrow over "0 bpm above".
you were right and i was wrong twice.

first: the headline. i told you it correctly stays fixed because it's the
latest reading. that's a bad design, not a defence — a big number under 7/30
tabs is read as a summary of the range you picked, and on steps it was worse
than confusing: today's 43 under a "30 days" tab reads as a month's total. the
headline is the window's daily average now, and it carries the day count —
"1 of 30 days" — which is also the honest explanation for the case that looked
broken. with one day of history, seven days and thirty days really do average
to the same number. the latest reading is still there, on its own line, dated.

second: the flaky test. two agents reported workout_reliability flaking and i
told you it was contention from eight of them hammering one laptop. it wasn't.
it fails about one run in four on an idle machine, in the full parallel suite,
and passes 6/6 alone.

the cause: the test slept 150ms and asserted runs == 0. that was flaky AND
weaker than it looked — zero is also what you see when nothing was ever
queued, so it would have passed before the enqueue landed. it waits for
pending_light now, which is the actual parked state, and since _arm() returns
early while a workout is live there's no timer at all — so once the job is
parked, runs cannot advance however long anything takes. deterministic instead
of merely patient. 5/5 green.
"what is it right now" and "what has it been lately" are different questions,
and a range list starting at 7 days made the first one unanswerable.

Today is its own window. on it the headline is today's reading labelled Today,
not a one-day average, and there's no chart — these series carry one value per
day, so a one-day window is a single point, and a single point drawn on an axis
is a shape pretending to be a trend. "your normal range" reads the whole
history there instead of the window, because otherwise its lowest, typical and
highest would all be the same number: today's.

on why 7 days showed you today's steps and nothing else — that's not the screen
picking today, it's that today is the only day with a steps row. gen4 has no
pedometer, so steps only exist from the day phone steps started reporting. the
headline says "1 of 7 days" now, which is the part that was missing: the number
was right, it just never told you how thin it was.

nothing recorded today gets its own words too, rather than "no history yet" —
one is a gap, the other is a new install.
alignment: the name is the only flexible part of a label→value row; the
measurement keeps its natural width and sits flush right. two flex children
splitting the row by ratio is why every value block started and ended at its
own x — the lab rows already did it the right way, which is why those were the
ones that looked correct to you.

_vital turned out to be a private re-implementation of MetricRow and is gone.
its value/unit stack shrink-wrapped to the QUALIFIER, so "bpm today" and "SD
from your own nights" were setting each row's width. a qualifier isn't a unit;
it sits under the name now, and 48-142 bpm / 14.2 br/min / +0.31 SD line up.
that also caught a shipped bug: respiratory rate on Overview rendered as
"1… br/min" — a Flexible gave the measurement a quarter of the row and
ellipsised it.

splits: the tab is per-archetype now, so flow, match and basic don't get one at
all. a route with no gps still gets "no splits for this session" — that's
expected-absent, which is different. share had four style names per archetype
where ShareCard only branches on one, so three of every four were labels for an
identical card; two now, and the second only when there's art to draw.

RR, since you asked: it's real, and the two tabs disagreed. getToday gated it
on resp['rsa'] being a Map while getDayLungs called the same function with no
such gate — so Overview could say "no respiratory rate" on a day whose Vitals
tab printed 14.2 br/min from the same bundle. the rsa block only supplies a
confidence and _respObj already falls back to 0.5, so the gate was suppressing
real readings. removed. it still needs a scored night, and the copy says which
of "no night" or "too noisy" applies.

also removed the skin-temp caveat card and the sleep architecture deep dive
from vitals, and "what interrupts you" is "manage notifications".

gallery: seven taps on the version row in settings. 41 components, live, with
text scale and light/dark switches — the two dimensions every layout bug we've
shipped actually lived in. goldenCases() moved into it, so the goldens now
photograph the same list the gallery renders and the two can't drift.
you said it reads "beat timing was too noisy to recover one". that sentence was
written by hand in the ui and shown for every kind of failure — it happened to
be one of four possible reasons.

the estimator already records which gate it failed, in respiration.rsa: too few
beats for an RSA spectral estimate (needs 20), artifact fraction over the gate,
no stable HF respiratory peak resolved, or a peak that moved between spectral
resolutions. that envelope was being read for its confidence and nothing else —
when there was no value it returned a bare null and the screen fell back to the
hand-written guess.

the note comes through now, so the card tells you which gate your night
actually failed. that's also the answer to your question: RR needs beat timing
clean enough over the sleep window, and the card will now name the specific
thing that wasn't.
the workflow died on the usage limit before it could fix anything, so these are
by hand, each re-verified against the code first.

home was painting a held-over prior night's readiness under today's date and
calling it "Today's readiness". getToday holds the last scored night over until
today's settles — every morning before the first sync, and the whole of any gap
after one — so readiness, sleep, rhr, hrv and skin temp described an older night
while steps and energy beside them described today. the deleted ui had a gate
for exactly this (settledReadinessScore, still pinned by a regression test) and
ui2 never read it. the skeptic found the hunter had UNDERSTATED it too:
_latestBundle reads 14 ROWS, not 14 days, so the number has no age cap.
withholding it would be wrong — it's real and it's the most recent one there is
— so it now names its night: "Readiness · 16 May".

the workout tab loaded once and cached for the life of the widget, so a session
you'd just finished was missing from history, this week, tracked and the weekly
load until you restarted the app. insightsRevision now ticks when a session is
durably written, and the screen re-reads when it moves — which covers the
manual finish, the gesture path and the live activity alike.

pause didn't pause HIIT. the interval timer had no pause check at all: it kept
counting down, buzzing and announcing work/rest at a phone in your pocket, and
BANKING rounds you never performed. a five-minute pause wrote four rounds of
45s work into the summary while the elapsed clock, which does subtract pauses,
said the session was shorter than the rounds it listed. round hr samples are
skipped while paused too — standing still would drag the mean toward resting.

wellness wrote the whole day's journal from a snapshot taken when the tab last
loaded, and putJournalMetrics deletes the day before re-inserting. so: open
wellness, write your journal from compose, come back, tick one habit — journal
gone. it re-reads the day before merging now.

three confirmed S1s left: the live summary's compacted hr curve, ChartFrame's
excludeSemantics eating the scrubber, and import reporting rows as days.
the live summary drew a band dropout as a continuous line. perMinuteHr only
grew when a minute HAD samples, so a 40-minute session with a ten-minute
dropout produced a 30-entry list; the summary maps index to x, so minute 9 was
joined straight to minute 21 and every later reading was drawn ten minutes
early — while the SAME session reopened from history was dense and showed the
gap correctly. the accumulator is dense now, indexed by session minute, and
splits into two readers: perMinuteHrDense for the chart, which needs the time
axis, and perMinuteHr for strain and the mean, which want the readings. avgHr
divides by the minutes that measured something rather than by the slots.

import reported a ROW count collapsed to the literal 1, so restoring a year of
history said "1 day imported". day_result's primary key is (day_id,
algo_version), so rows were never days anyway — a history through two algo
bumps has two rows per day. it asks the source file for a distinct day count
now, and reports nothing rather than a wrong number if the table can't be read.

ChartFrame used excludeSemantics to stop a screen reader announcing the bare
axis ticks and the doubled header. it did that, and it also deleted every
descendant semantics node — including child, which on the sleep screen is the
Scrubber, so the one interactive control inside a chart was invisible to
VoiceOver and Switch Control. the decoration is excluded piece by piece now and
child is left alone. explicitChildNodes matters too: a container without it
merges its descendants, which loses the control the same way by another route
— the test caught exactly that on the first attempt.

that's all 7 confirmed S1s. 2388 tests.
the first hunt's skeptics for wiring and analytics all died on the usage limit,
so those 80 findings were one agent's opinion with no second pass. re-ran it:
one skeptic per module told to refute, and anything it couldn't settle went to
a second reviewer told to REPRODUCE it instead — opposite priors on the same
claim. 67 survived, 5 died there.

the worst ones, all S1:

after the band drops mid-workout, the 1hz tick kept billing device.liveHr as a
live measurement into zone-seconds, calories and strain, and persisted it on
stop. nothing nulls that field on an unintentional disconnect — disableLiveStreams
only runs on the intentional path — and the freshness stamp that exists,
liveHrAt, had zero readers. the stale value also suppressed the honest "band is
not connected" card, because that only shows when hr is null.

the offloaded second half recomputed headline strain from scalars['rhr'] — the
general resting hr that deliberately falls back to DAYTIME hr — and overwrote
scalars['strain'] with it, so a day where the pure pipeline abstained still
published a strain built on an awake "resting" heart rate. reproduced against a
real export: two days with clinical.strain absent, scalars.strain 6.0 and 8.7,
rhr ~84 against the same user's real nocturnal 59-65.

needsRepairGuide could be set by one failed createBond or two quick
disconnects, and the only line that clears it was unreachable unless
autoReconnectPaused happened to be true — so a working, syncing band told you
permanently to forget and re-pair it.

nutrition's 7-day macro means averaged FLOOR totals as if exact: a day counted
toward the average on energy completeness alone, and the mean then admitted any
non-null protein sum including one that summed past occasions carrying no macro
figure. the energy card says "at least"; the macro means had no such channel.

medication adherence counted scheduled doses on days BEFORE the medication
existed as missed, because nothing bounded schedule resolution by the def's
creation date. add a med at 3pm and it immediately read "0 of 7".

the widget, watch and siri still published the hardcoded 480-minute sleep need
that the phone's own screens refuse to show.

also removed, each a validated finding rather than a cleanup i asked for:
proximity_policy (262 lines instantiated by nothing but its own test, fed by
readRssi which has no caller either) and LiveWorkoutState.firedMilestones (a
set documented as suppressing a banner, haptic and confetti that do not exist).
insightsRevision was on that same dead-symbol list and is NOT removed — it has
a listener now, added two commits ago when the workout tab stopped reloading.
all 83 from docs/internal/sweep/SLOP.md, the 15 hedged ones included. no
replacement copy — the instruction was removal, not rewriting.

the worst of them: "a signal worth waking you for is worth waking you for", a
tautology sitting where a reason should be. "no account to sign out of."
"eight hours is a slogan, not your need." a devices card restating the ladder
directly above it, and a second card printing the identical string a row ten
lines up already printed. "no sensor can see a pool wall", twice in one
session summary.

twelve rows would have left a card with a heading and nothing under it, so
those cards went whole — and their sections with them where the card was the
only content. that rule went too far in three places and i put them back,
because the prose was slop and the affordance was not:

- "allow location" on the gps-denied live card. denied is the ONE branch a
  user can resolve from that screen, and deleting the card left them with a
  missing map and no way to fix it. the other three branches kept theirs.
- "add weight in profile" on workout. without a weight there is no calorie
  estimate at all, and that was the only place saying so and offering the door.
- "3 of 7 nights so far" on the sleep comparison. that's a count, which the
  brief protected explicitly — it says the comparison is coming and when.

all three are title + action now, no body. StatusCard renders no empty Text
when why is blank, which makes "title and a button" a legitimate card rather
than a broken one — that IS the minimal-text ideal. what a card must never be
is a title alone.

dead code out with the strings: kCalorieNeedsWeight, an unused profile import.
_routeIssueCard, _versusUsual and _stateDetail are nullable now and
Observation.advice is optional, so nothing renders an empty string.

124 goldens moved. one sleep assertion repointed: the section is present with
its count, not absent.
the gallery only had about two thirds of the vocabulary in it. added the
readiness card, the share card, the live session pieces, the settings /
source / activity rows, legend, mono table, unlock contract and the import
report.

pulled ReadinessHero out of home_screen so it could go in at all — it was
inline in a ListView behind the db.

putting them in the sweep found four things overflowing at 3.1x that are
live right now: home's readiness card, the live hr block, every settings
row and the activity list. fixed. first attempt at the row ones used
Flexible and quietly shifted every settings value right, so they use the
row rule instead and the value drops under the title at big text.

added a test so the gallery can't fall behind again — a new component has
to be in it or on the not-a-component list.
the muscle map was never a measurement. it was the exercise→group table in
catalogue.dart times the volume you typed, painted onto a body — which is
the shape a scan produces. gone from the post-activity breakdown and from
the share card too, since that one leaves the phone. painter and muscleLoad
deleted; the group name stays as the picker label, which is all it ever
honestly was.

a lift now shows what it knows: volume, sets, top set, and every set as
logged. empty session still explains itself.

gallery: added the second state of every card — absent, over target, down
and bad, a series with a gap in it, title-only status, zero and full
consistency, every pill colour — plus a share card per archetype. that is
the state a screenshot never catches because a demo phone always has data.
summary.dart already refuses this in words — the live ring is a pacer you
breathe along with, so a finished session has no phase to draw and a static
ring at .8 is decoration shaped like a measurement. the share card was
drawing it anyway, and that is the card that leaves the phone.

a flow session shares as the minimal card now, same as a lift.
running and gps sessions get a third share style. 4:5 poster, your photo
across the top if you add one (from this phone, nothing uploaded, gradient
if you don't), the stats you picked, and the route drawn on actual
openstreetmap tiles underneath.

the map is fetched and stitched rather than a live map widget — the card is
exported with toImage, and an async tile layer exports blank about half the
time. every tile is awaited before anything is drawn, so what you see is
what gets sent. no tiles = the plain route on the card, never a half-loaded
one.

osm tile policy: real user-agent, disk cache, hard cap of 24 tiles per card,
one zoom, and the attribution is ON the map so a crop can't separate them.
tiles are tinted to the card rather than left as a white slab.

no cadence, no weather, no training-load band — three things every running
card shows and this stack doesn't measure for a gen4 band.

carried real lat/lng through to ActivityResult.geo; the painters keep using
the normalised box.

tests pin the projection against london/sydney/null island, because a wrong
constant there still draws a tidy route, just in the wrong country. the
overflow test runs without the real font on purpose — it found one, in the
footer.
you can't see the map without a run, so the gallery gets a real one: a lap
of cubbon park with real coordinates, a load-the-map button and a photo
slot. sits at the top of the gallery.

not in galleryCases on purpose — every case in that map is shot by the
goldens and swept at five text scales, and one that reaches the network
would make both a function of the wifi. this is screen-only and fetches
nothing until you tap.
i built a stack — photo band on top, stat strip, map at the bottom. the
reference is side by side: photo full bleed across the whole card, scrim
fading left to right, everything in a column down the left.

so: wordmark, activity in accent caps, the big number, a VERTICAL stat list
with ringed icons and hairlines, the map, date footer. route runs a pace
gradient now, green fast to red slow, with real pins instead of dots.

four stat rows, not three. three silently dropped the fourth and the share
sheet preselects four — a card that quietly loses a number you ticked is
the one thing this card isn't allowed to do. trimmed the map and the
padding to make room.

PosterStatRow is public so the workout screens can use the same row.

gallery preview gets a share button that actually shares — you can't test
sharing without a run otherwise.

still no cadence, weather or load band. not measured.
…here

you were right about the map. in the reference it isn't a boxed widget, it's
dissolved into the picture with only the route and the streets around it
showing. so it's a full-bleed layer now, on the photo and under the scrim,
box and corner radius gone.

the dissolve is a mask built from the route itself — a very wide, heavily
blurred stroke along the path, applied as dstIn. what survives is the block
either side of where you ran. over a photo a second mask fades it upward
into the picture; with no photo it keeps the whole card, because then it IS
the background. the line stays full strength on top: the tiles are texture,
the line is the measurement.

had to widen the scrim's hold to the full column — a route runs diagonally
across the whole card and was crossing HEART RATE at .62.

workout summary and history now use the same ringed rows as the poster. the
three-across strip is gone, so the 4th and 5th stats stop being dropped by a
layout and pace can print its own unit.

three things found on the way, all live before today:
- the summary said green = SLOWEST and the poster said green = FASTEST. same
  run, two maps, opposite meanings. summary now matches the card.
- InlineMetrics ellipsized measurements: the journey card printed '+642…' for
  +642 m, health trends printed '78 / …' for 78 / 100. scaleDown now — a
  shrunk measurement is still the measurement, a truncated one is a
  different number.
- the summary hero had a Flexible next to a Spacer, so two flex children
  split the row by ratio and a lift's volume printed '1,…' at 2x.

ring scales with text now, clamped at 2x — it's on real screens, not just
the fixed-size card.
the share sheet asked three questions and only one was mine to answer.
style list gone — a lift never had a texture to pick. stat picker gone —
it was asking which of my own measurements to leave off a card with room
for all of them. what's left is a photo and a share button, plus
post/story so instagram stops cropping it.

the basemap was at zoom 2 on every card ever shared. the tile budget
loop stepped the zoom down while the count was over its cap, but the
frame is a fixed pixel size, so the count doesn't move with zoom —
900x1200 is 25 tiles against a cap of 24 and it ran to the floor every
time. the corridor mask hid it, which is why removing the mask is what
found it.

then it was invisible instead. osm's raster has almost no dark pixels,
so mapping 0..1 onto the two ends crushed land, water and roads on top
of each other. stretch the window the source actually uses.

hero and grid were both printing the distance now that nothing is
hand-picked, so the grid drops whichever stat the hero already is.

gallery: flows tab walks every activity in the catalogue through the
real screens. its light tab was showing dark and its dark tab light, so
every review through it was of the palette i hadn't picked. dropped
powercurve and movementmap, nothing drew them.
the three that matter most:

the share sheet fetched osm tiles. so opening it on a gps session sent that
session's location to tile.openstreetmap.org, in an app whose whole claim is
that nothing leaves the device — up to forty requests, undisclosed, on a
screen you open to look at a picture. it's behind an explicit default-off
opt-in now that says plainly what gets asked for, and without it the card
still works: your photo, the route line, plain ground.

"private session" was a toggle that did nothing. the flag never left the
draft, so a session you'd been promised was hidden from summaries and exports
was sitting in both. it's a column now.

and the pin. kAlgoVersion spans both the pinned analytics and the local one,
so a day derived against different analytics at the same version can never be
invalidated — which is exactly why nobody noticed that v67 and v68 cite six
analytics behaviours the pinned sha doesn't contain, in the paragraph that
warns about v43. repinned, and the version test reads pubspec.yaml now, so
the next drift is a red test instead of a shipped wrong number.

the rest, by shape:

fabricated numbers. active_min written twice by two algorithms over two
windows under one key. a confident 0 active minutes from a day with no
gravity vector, and 0 restless minutes from a night with no accel. wear
coverage divided by the span of the records that exist instead of the length
of the day. a 100% coverage invented from the existence of one hr sample.

the wrong day. notifications stamped with the newest derived day instead of
today, so importing old history fired an alert about a night in may.
emitOncePerDay keyed its guard on the day the data came from and never
checked that day was today. home named the held-over night on readiness only,
so the sleep and heart-rate cards beside it still presented that same older
night as last night's.

forms that ate what you typed. a typo wiped a stored height. a comma turned
a macro into a missing one. a lab result vanished with the dialog. every
outcome on your data, thrown exceptions included, rendered as a green done.
blank and unparseable are different things now, in the one helper they all
share.

things you couldn't undo, and things you couldn't do. a medication could be
added and never removed. a period start was deleted by one unconfirmed tap.
pause didn't pause anything but the clock — calories, strain, zone minutes,
gps and max hr all kept accruing.

and the migration ladder. rung 19 backfilled columns into a table it had just
rebuilt without them, so an upgrade from schema 18 or older threw inside the
one exclusive transaction and got the database quarantined. the test that
should have caught it seeded a row whose hex doesn't decode.

schema 40, algo 69, 2447 tests. the 20 summary_* goldens were already red.
schema 41. device_family lands on decoded_onehz, decoded_rr and sessions,
stamped at ingest from the generation the link pinned at service discovery —
never inferred from the data, never backfilled from the currently paired
device, because what's paired today says nothing about which strap wrote a row
in march. it carries into Substrate as ONE value for the window: a window that
mixed two straps has no answer and resolves to unknown, and unknown refuses
rather than quietly reading as gen4.

that refusal is the point. every existing install reads NULL until it next
syncs, so a metric moved onto the seam goes absent for historical days. that's
visible and it's correct — the alternative is one strap's calibration silently
answering for another's.

it also found a live one on the way in: naming the new column unconditionally
in the insert map bricked the v6 upgrade, because _backfillDecodedStore runs
mid-ladder long before the rung that adds the column, and a throw inside
onUpgrade's single exclusive transaction quarantines the database. the write
path only names the column when it has something to say now.

sleep_detail's stage catch-all was `_ => SleepStage.light`, so every second the
band never recorded was drawn AND tallied as light sleep — a three-hour hole
came out as three hours of sleep. unobserved is not a stage; it draws as a gap,
and an unrecognised label from an older bundle fails the same way.

the fertile window is gone. it was mean_cycle_length minus 14 — a textbook
constant printed as her personal date with "not contraception" underneath. the
prediction is a median-and-MAD range with n stated now, and it is embarrassingly
wide for irregular cyclers, which is the finding.

vo2max and fitness age deleted. the water buzzer deleted — complete, armed on
every launch, 24 real notification slots, held off only by a pref with no
toggle. water_ml stays as a passive logged signal. spo2 is a permanent refusal
with the ir-red reason written down, and the "temporarily disabled pending
hardware-verified decoding" strings are gone; they were a promise to a future
contributor that it comes back.

then the things that were already computed and never shown: day strain off the
curve that was persisted for nobody, the readiness absence diagnostic that only
ever went to a firebase breadcrumb, deceleration capacity, the rhythm screen's
history, daytime hrv with the motion gate it never had, sleep onset on forced
windows only, wake-up runs, the caffeine timing that was dropped three lines
before the analysis.

verified against real gen4, gen5 and MG exports — all three are old-schema, all
three migrate to 41 with zero rows dropped and integrity ok. 2503 tests. the 20
summary_* goldens were already red and are now stale for a second reason.

algo bump and the repin come after the next batch lands.
the biggest one is the smallest diff. decoded_onehz.hr was NOT NULL, and 0 was
the off-skin sentinel, which is why every v25 record — 28,395 of them in a
two-week gen4 export — went to raw_archive instead of the substrate. the column
is nullable now and nothing was turned into a null on the way: hr IS NULL is
zero rows on all three real databases after the migration. the audit was the
work, not the alter; the risk was a reader treating missing as 0 and scoring
those seconds off-wrist.

three universal 220-age paths were still handing out a heart-rate ceiling while
hr_max refused for an unknown strap, so the same session could band zone_min on
one ceiling and anchor its trimp on another. one function now, dispatched by
family, and where no ceiling can be established the session says so instead of
inventing 190 for anyone who skipped the age field. at 30 the ceiling moves
190 to 187, which moves band membership — 150 bpm was z3 and is z4. that is the
bug, not a regression.

'fat burn' is 'easy'. which fuel you oxidise in z2 needs respiratory exchange
and no wrist sensor produces it.

mid-sleep is persisted, unwrapped, because 23:30 to 01:30 is +2h forwards and
-22h numerically and a change-point detector run on the wrapped series finds a
modulo artifact. only future nights get the key.

hr_valid reaches Substrate. the ambient-light channel finally has a producer —
one line, and 0 still means absent, not darkness. pages_behind writes to the
table that was created for it instead of a single ledger row that only ever
held the last reading. the coach stops being told about odi_per_hour, a column
that can never have data again.

encrypted backup says both halves in the same breath: unreadable without the
passphrase, and gone forever if you forget it, because there is no server
holding a key. no hint field, no recovery code — both weaken it to feel safer.

GATES.md answers four questions that were marked blocked on hardware nobody
had. the headline: gen5 skinTempC is populated on 1,035 of 1,035 real records,
28.8-38.3 degrees, 397 distinct values, 98-100% coverage inside real sleep
windows. the coverage gate that was supposed to kill four features didn't.

the refusals are in the code now, at the sites someone reaches for them, rather
than in a planning doc nobody opens.

repinned to analytics 391ede4. 2598 tests, the 20 summary_* goldens still red
from before all this.
schema 44. a lot of correct analytics landed last wave and then stopped at the
repo boundary — the functions existed, nothing called them, so nothing computed
them and the ui agents had nothing to render and rightly refused to fake it.
this closes that seam.

the observed hr ceiling scores sessions ONE AT A TIME rather than as a
concatenated day, because stitching two sessions invents a hold across the gap
between them. it needs the 15s hold plus corroborating motion, and the motion
floor is per-family: gen4 0.10g, gen5 0.04g, unknown refuses.

sri was going to ship an empty list forever. the pairs were serialized, but the
analytics floor is half a clock day and this caller's valid mask is only set
inside a hypnogram segment — an 8h night marks about 480 minutes, so no pair
could ever clear 720. it would have rendered as "no irregular nights" for
everyone. the published sri number doesn't move; every accepted epoch still
counts.

the day breathing curve gets its stillness gate, and the measurement is the
point: gen4 attempts 1,573 windows and 201 survive, gen5 617 and 552 survive.
so on gen4 the triple lomb-scargle now never runs on 87% of candidates. that
also means resp_day changed meaning — it's the day's resting floor now, and it
will be empty on most days, which is the honest output.

nightly hrv shape runs over the SAME cleaned nn the headline rmssd uses, so the
curve and the number can't disagree, and every bin ships lo/hi so it draws as a
band. session cost and the overreaching conjunction read outputs that already
ran daily. the conjunction is two facts and no verb, and it is structurally
kept out of the notification collector rather than just left out of it.

and 867 seconds of real MG data came back. 1,035 v18 frames were archived as
undecodable before this month's decoder fixes and decode fine now; the re-drive
is idempotent, verified by running it twice on the real export, and it never
consumes or relabels the archived bytes. the other 168 already had a row and
the standing row won.

MT-05 stays refused, and now it's refused with a number: the minute-majority
split eats 17-21% of a labelled 83-minute walk and 22% of a labelled 50-minute
run, measured against real sessions. those are exactly the minutes it would
have subtracted from active energy.
the png had no transparency at all — a flat (249,249,249) plate at alpha
128 over the whole canvas, so dropped on the purple card it was a grey
square. flood-filled from the edges instead of keying near-white, because
the character is cream and a colour key eats it. opaque pixel count is
unchanged, so nothing got cut off it. 279kb -> 42kb across 1x/2x/3x.

card is full width, mascot bottom right, copy and start on the left. it's
a row, not a stack — stacked they fought for the same pixels. height is a
floor now, not fixed, or the button clips off at 2x text.

two grounds instead of one purple: a cream mascot disappears on a pale
card, so light goes deeper rather than lighter.

rings draw solid when measured. calibrating keeps the fade on purpose —
if everything's bright you can't tell "this is real" from "this is still
filling".
the overflowbox did it. inside a listview the main axis is unbounded, an
overflowbox takes whatever it's handed, and it took the whole list down
with it. sorry — that shipped without me rendering it in a scroll view.

full bleed is the list's job now: it drops its side padding and pads
every other child, so the hero is just the one child that doesn't get
any. ordinary layout, can't blank anything.

card moved to start_card.dart and parameterised so wellness can use the
same one with its own mascot and colour.
the method prose was sitting on the screens themselves — how karvonen works,
what the alertness model can't see, what prsa averages, why cold shuts the
sensor. gone. kept the absence reasons, the one-clause honesty lines and the
medical copy, and used plainer words for the rest.
the meditating mascot came in already transparent, so it just needed
cropping. 810kb -> 30kb across the three densities.

teal because it's C.domMind, the tab's own colour, and the coral cushion
and headband sit against green better than they do against purple.

count is 3 — kBreathPatterns, what the picker actually offers. the paced
breathing ActionCard is gone; this is the same door with the same
last-sitting line on it.
Expanded and Flexible both default to flex: 1, so a row with one of each
split it 50/50 — that's why "see all" started in the middle of the line
and stopped wherever its own text ended. same shape in three shared
components, so it was everywhere at once.

fix is spaceBetween with both sides shrinkable. making the right side a
plain child anchors it too but overflows on a long left side — day
strain caught that at 15px.

and the mood buttons: a container with a child and no alignment sizes to
the child, and the constraints coming down are loose, so they rendered as
narrow pills instead of the fifth of the row the Expanded had paid for.
five sub-tabs never fit. on a 390pt phone Cycle shows 3pt and Labs 19pt,
on a 360 both show nothing at all, so the row reads as if it ends at the
fourth one — which is how the cycle and labs screens went unfound.

ScrollHint fades the tail and puts a chevron at the edge, both only while
there is genuinely more, both gone at the end of the scroll. wire it into
SubTabs separately.
they're sweep reports and measurement runs written against the real
exports, so they quote dated readings — a resting heart rate on a named
day, an irregular-rhythm flag with its date. this repo is public.

they stay on disk. this only takes them off the tip: they're still in 24
commits of branch history, which has to be purged before any push.
it printed the onboarding weight scalar and that was it. the ask was a
bar saying whether your weight is normal for your intake and burn, and i
can't build that one honestly:

- no food log in any real db i have, and a logged day is partial by
  design anyway (an occasion with no kcal is a valid entry, so the day's
  intake is a floor). self-report is under by 20-30% on top.
- calories_total is an estimate at 0.5 confidence and swings 2454-4545
  across a fortnight on my own export. a barely-worn day still prints a
  confident 1715.
- weight here is one profile number, not a series, so there's no usual to
  compare against. the honest trend already exists in journal
  (weightTrendEwma, gaps left as gaps).

reason is in the code where the section was so nobody rebuilds it.
the fifth tab is off the edge on every phone we ship to — at 360pt it's
fully off-screen in both tab sets, and above 1.0x text every set
overflows even a 430pt screen. can't be made to fit. hint draws nothing
while the row fits and scales with what's left to scroll.

icon is the colourful one now, bw set alongside it, and the launch
screen stops flashing the old mark before dart starts.
416 pngs, 27mb, none of them on main. they're machine-specific — the two
sdks here already disagree on antialiasing — so committing them is a diff
nobody can review and a conflict nobody can resolve.

test code stays, pictures don't. regenerate with --update-goldens.

still in branch history, purged along with docs/internal before any push.
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 235 files, which is 135 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 324d9725-30ff-4867-86c8-79670b68f167

📥 Commits

Reviewing files that changed from the base of the PR and between 1c425ed and cadde68.

⛔ Files ignored due to path filters (266)
  • android/app/src/main/AndroidManifest.xml is excluded by !android/**
  • android/app/src/main/kotlin/wtf/openstrap/openstrap_edge/MainActivity.kt is excluded by !android/**
  • android/app/src/main/kotlin/wtf/openstrap/openstrap_edge/NativeChannels.kt is excluded by !android/**
  • android/app/src/main/kotlin/wtf/openstrap/openstrap_edge/OpenStrapBatteryWidgetProvider.kt is excluded by !android/**
  • android/app/src/main/kotlin/wtf/openstrap/openstrap_edge/OpenStrapWidgetProvider.kt is excluded by !android/**
  • android/app/src/main/kotlin/wtf/openstrap/openstrap_edge/PhoneStepCounter.kt is excluded by !android/**
  • android/app/src/main/kotlin/wtf/openstrap/openstrap_edge/StrapWidgets.kt is excluded by !android/**
  • android/app/src/main/res/drawable-hdpi/ic_launcher_foreground.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/drawable-mdpi/ic_launcher_foreground.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/drawable-v21/launch_background.xml is excluded by !android/**
  • android/app/src/main/res/drawable-xhdpi/ic_launcher_foreground.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/drawable-xxhdpi/ic_launcher_foreground.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/drawable-xxxhdpi/ic_launcher_foreground.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/drawable/launch_background.xml is excluded by !android/**
  • android/app/src/main/res/drawable/widget_bg_char.xml is excluded by !android/**
  • android/app/src/main/res/drawable/widget_bg_paper.xml is excluded by !android/**
  • android/app/src/main/res/layout/widget_openstrap_nodata.xml is excluded by !android/**
  • android/app/src/main/res/mipmap-hdpi/launcher_icon.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/mipmap-mdpi/launcher_icon.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/mipmap-xhdpi/launcher_icon.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/mipmap-xxhdpi/launcher_icon.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/mipmap-xxxhdpi/launcher_icon.png is excluded by !**/*.png, !android/**
  • android/app/src/main/res/values-night/colors.xml is excluded by !android/**
  • android/app/src/main/res/values-night/styles.xml is excluded by !android/**
  • android/app/src/main/res/values/colors.xml is excluded by !android/**
  • android/app/src/main/res/values/styles.xml is excluded by !android/**
  • android/app/src/main/res/values/widget_strings.xml is excluded by !android/**
  • android/app/src/main/res/xml/data_extraction_rules.xml is excluded by !android/**
  • assets/images/2.0x/mascot_wellness.png is excluded by !**/*.png, !assets/**
  • assets/images/2.0x/mascot_workout.png is excluded by !**/*.png, !assets/**
  • assets/images/3.0x/mascot_wellness.png is excluded by !**/*.png, !assets/**
  • assets/images/3.0x/mascot_workout.png is excluded by !**/*.png, !assets/**
  • assets/images/icon.png is excluded by !**/*.png, !assets/**
  • assets/images/icon_bw.png is excluded by !**/*.png, !assets/**
  • assets/images/mascot_wellness.png is excluded by !**/*.png, !assets/**
  • assets/images/mascot_workout.png is excluded by !**/*.png, !assets/**
  • assets/launcher/icon_adaptive.png is excluded by !**/*.png, !assets/**
  • assets/splash/splashscreen.mp4 is excluded by !**/*.mp4, !assets/**
  • ios/OpenStrapIntents.swift is excluded by !ios/**
  • ios/OpenStrapWatch Watch App/OpenStrapWatchApp.swift is excluded by !ios/**
  • ios/OpenStrapWatch Watch App/WatchMetrics.swift is excluded by !ios/**
  • ios/OpenStrapWatch Watch App/WatchStore.swift is excluded by !ios/**
  • ios/OpenStrapWatchWidget/OpenStrapWatchWidgetBundle.swift is excluded by !ios/**
  • ios/OpenStrapWidget/OpenStrapBatteryWidget.swift is excluded by !ios/**
  • ios/OpenStrapWidget/OpenStrapBreathingLiveActivity.swift is excluded by !ios/**
  • ios/OpenStrapWidget/OpenStrapWidget.entitlements is excluded by !ios/**
  • ios/OpenStrapWidget/OpenStrapWidget.swift is excluded by !ios/**
  • ios/OpenStrapWidget/OpenStrapWidgetBundle.swift is excluded by !ios/**
  • ios/OpenStrapWidget/OpenStrapWidgetControl.swift is excluded by !ios/**
  • ios/OpenStrapWidget/OpenStrapWidgetLiveActivity.swift is excluded by !ios/**
  • ios/Podfile.lock is excluded by !**/*.lock, !ios/**
  • ios/Runner.xcodeproj/project.pbxproj is excluded by !ios/**
  • ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme is excluded by !ios/**
  • ios/Runner/AppDelegate.swift is excluded by !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-1024x1024@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-50x50@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-50x50@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-57x57@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-57x57@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-72x72@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-72x72@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Contents.json is excluded by !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-20x20@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-20x20@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-20x20@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-29x29@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-29x29@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-29x29@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-40x40@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-40x40@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-40x40@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-60x60@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-60x60@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-76x76@1x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-76x76@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/AppIconBW.appiconset/Icon-BW-83.5x83.5@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/LaunchBackground.colorset/Contents.json is excluded by !ios/**
  • ios/Runner/Assets.xcassets/LaunchImage.imageset/Contents.json is excluded by !ios/**
  • ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage@2x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage@3x.png is excluded by !**/*.png, !ios/**
  • ios/Runner/Assets.xcassets/LaunchImage.imageset/README.md is excluded by !ios/**
  • ios/Runner/Base.lproj/LaunchScreen.storyboard is excluded by !ios/**
  • ios/Runner/HealthRoutes.swift is excluded by !ios/**
  • ios/Runner/Info.plist is excluded by !ios/**
  • ios/WatchBridge.swift is excluded by !ios/**
  • pubspec.lock is excluded by !**/*.lock
  • test/absence_and_offload_guards_test.dart is excluded by !test/**
  • test/absence_reason_test.dart is excluded by !test/**
  • test/absence_routing_test.dart is excluded by !test/**
  • test/absent_not_zero_test.dart is excluded by !test/**
  • test/ai_breakdown_widget_test.dart is excluded by !test/**
  • test/ai_briefing_test.dart is excluded by !test/**
  • test/ai_reminder_plan_test.dart is excluded by !test/**
  • test/ai_screen_async_guards_test.dart is excluded by !test/**
  • test/app_state_regressions_test.dart is excluded by !test/**
  • test/background_sync_staleness_test.dart is excluded by !test/**
  • test/backup_crypto_test.dart is excluded by !test/**
  • test/band_battery_event_test.dart is excluded by !test/**
  • test/band_step_counter_test.dart is excluded by !test/**
  • test/battery_forecast_test.dart is excluded by !test/**
  • test/beat_timestamps_test.dart is excluded by !test/**
  • test/ble_engine_test.dart is excluded by !test/**
  • test/ble_repair_guide_test.dart is excluded by !test/**
  • test/ble_safe_trim_test.dart is excluded by !test/**
  • test/ble_state_test.dart is excluded by !test/**
  • test/boot_splash_test.dart is excluded by !test/**
  • test/bottom_gutter_test.dart is excluded by !test/**
  • test/breath_phases_test.dart is excluded by !test/**
  • test/breathing_session_effect_test.dart is excluded by !test/**
  • test/calm_breathing_view_test.dart is excluded by !test/**
  • test/calorie_heatmap_test.dart is excluded by !test/**
  • test/coach_actions_test.dart is excluded by !test/**
  • test/coach_local_endpoint_test.dart is excluded by !test/**
  • test/core_screens_test.dart is excluded by !test/**
  • test/crash_regressions_test.dart is excluded by !test/**
  • test/crossday_artifact_freshness_test.dart is excluded by !test/**
  • test/crossday_circadian_test.dart is excluded by !test/**
  • test/crossday_pipeline_test.dart is excluded by !test/**
  • test/csv_export_test.dart is excluded by !test/**
  • test/daily_energy_consistency_test.dart is excluded by !test/**
  • test/data_ownership_test.dart is excluded by !test/**
  • test/day_nav_test.dart is excluded by !test/**
  • test/day_timeline_test.dart is excluded by !test/**
  • test/day_window_dst_test.dart is excluded by !test/**
  • test/db_integrity_test.dart is excluded by !test/**
  • test/db_migration_ladder_test.dart is excluded by !test/**
  • test/db_p0_fixes_test.dart is excluded by !test/**
  • test/db_serve_version_and_reads_test.dart is excluded by !test/**
  • test/db_strength_schema_test.dart is excluded by !test/**
  • test/db_v42_retention_and_provenance_test.dart is excluded by !test/**
  • test/db_v43_nullable_hr_test.dart is excluded by !test/**
  • test/derivation_pipeline_test.dart is excluded by !test/**
  • test/derive_day_window_test.dart is excluded by !test/**
  • test/design_redesign_test.dart is excluded by !test/**
  • test/design_system_test.dart is excluded by !test/**
  • test/device_alerts_test.dart is excluded by !test/**
  • test/device_family_migration_test.dart is excluded by !test/**
  • test/disclosure_test.dart is excluded by !test/**
  • test/driver_breakdown_test.dart is excluded by !test/**
  • test/edit_profile_import_test.dart is excluded by !test/**
  • test/fake_route_fixture_test.dart is excluded by !test/**
  • test/find_band_view_test.dart is excluded by !test/**
  • test/findings_history_test.dart is excluded by !test/**
  • test/flow_screens_redesign_test.dart is excluded by !test/**
  • test/gait_step_types_test.dart is excluded by !test/**
  • test/gen5_decoded_onehz_persistence_test.dart is excluded by !test/**
  • test/gen5_sample_mapping_test.dart is excluded by !test/**
  • test/gen5_wiring_test.dart is excluded by !test/**
  • test/get_sessions_merge_test.dart is excluded by !test/**
  • test/headless_gate_test.dart is excluded by !test/**
  • test/health_import_state_test.dart is excluded by !test/**
  • test/health_measurement_import_test.dart is excluded by !test/**
  • test/health_profile_import_test.dart is excluded by !test/**
  • test/health_rhr_seed_test.dart is excluded by !test/**
  • test/health_workout_import_test.dart is excluded by !test/**
  • test/history_screens_redesign_test.dart is excluded by !test/**
  • test/hr_ceiling_zones_test.dart is excluded by !test/**
  • test/hr_sensor_parse_test.dart is excluded by !test/**
  • test/interactive_screens_test.dart is excluded by !test/**
  • test/journal_csv_import_test.dart is excluded by !test/**
  • test/journal_fields_test.dart is excluded by !test/**
  • test/live_cadence_test.dart is excluded by !test/**
  • test/live_coverage_window_test.dart is excluded by !test/**
  • test/live_hr_curve_density_test.dart is excluded by !test/**
  • test/live_rescore_calorie_parity_test.dart is excluded by !test/**
  • test/live_session_layout_test.dart is excluded by !test/**
  • test/live_step_runs_test.dart is excluded by !test/**
  • test/live_strain_convergence_test.dart is excluded by !test/**
  • test/local_repository_p0_test.dart is excluded by !test/**
  • test/lookback_gate_test.dart is excluded by !test/**
  • test/manual_session_test.dart is excluded by !test/**
  • test/manual_workout_repo_test.dart is excluded by !test/**
  • test/metric_trend_redesign_test.dart is excluded by !test/**
  • test/month_grid_test.dart is excluded by !test/**
  • test/mt12_dyn_accel_test.dart is excluded by !test/**
  • test/nap_credit_test.dart is excluded by !test/**
  • test/nav_back_swipe_test.dart is excluded by !test/**
  • test/night_beats_repo_test.dart is excluded by !test/**
  • test/nightly_sweep_test.dart is excluded by !test/**
  • test/noop_backup_import_test.dart is excluded by !test/**
  • test/noop_schema_drift_test.dart is excluded by !test/**
  • test/notification_center_test.dart is excluded by !test/**
  • test/notification_day_guard_test.dart is excluded by !test/**
  • test/notification_dedupe_test.dart is excluded by !test/**
  • test/nutrition_med_model_test.dart is excluded by !test/**
  • test/nutrition_med_schema_test.dart is excluded by !test/**
  • test/off_lookup_test.dart is excluded by !test/**
  • test/os_icons_wiring_test.dart is excluded by !test/**
  • test/phone_pedometer_hour_walk_test.dart is excluded by !test/**
  • test/poster_map_test.dart is excluded by !test/**
  • test/proximity_policy_test.dart is excluded by !test/**
  • test/raw_archive_test.dart is excluded by !test/**
  • test/raw_prune_gating_test.dart is excluded by !test/**
  • test/readiness_saturation_test.dart is excluded by !test/**
  • test/resting_hr_nocturnal_only_test.dart is excluded by !test/**
  • test/rough_night_test.dart is excluded by !test/**
  • test/route_types_match_catalogue_test.dart is excluded by !test/**
  • test/screen_loader_test.dart is excluded by !test/**
  • test/sleep_alarm_caption_test.dart is excluded by !test/**
  • test/sleep_clock_offset_test.dart is excluded by !test/**
  • test/sleep_naps_visible_test.dart is excluded by !test/**
  • test/sleep_windows_and_session_avg_test.dart is excluded by !test/**
  • test/start_session_card_test.dart is excluded by !test/**
  • test/startup_and_rebuild_recovery_test.dart is excluded by !test/**
  • test/step_source_ladder_test.dart is excluded by !test/**
  • test/strain_bonus_test.dart is excluded by !test/**
  • test/strain_rescale_backfill_test.dart is excluded by !test/**
  • test/strain_resting_hr_source_test.dart is excluded by !test/**
  • test/substrate_accel_absence_test.dart is excluded by !test/**
  • test/substrate_hr_valid_test.dart is excluded by !test/**
  • test/sync_dot_test.dart is excluded by !test/**
  • test/sync_policy_test.dart is excluded by !test/**
  • test/tap_router_test.dart is excluded by !test/**
  • test/timeline_scrub_test.dart is excluded by !test/**
  • test/ui2_a11y_chartframe_test.dart is excluded by !test/**
  • test/ui2_activity_test.dart is excluded by !test/**
  • test/ui2_alarm_test.dart is excluded by !test/**
  • test/ui2_beats_test.dart is excluded by !test/**
  • test/ui2_charts_test.dart is excluded by !test/**
  • test/ui2_contrast_test.dart is excluded by !test/**
  • test/ui2_cycle_history_test.dart is excluded by !test/**
  • test/ui2_day_nav_test.dart is excluded by !test/**
  • test/ui2_day_steps_test.dart is excluded by !test/**
  • test/ui2_gallery_test.dart is excluded by !test/**
  • test/ui2_golden_test.dart is excluded by !test/**
  • test/ui2_home_health_golden_test.dart is excluded by !test/**
  • test/ui2_onboarding_profile_golden_test.dart is excluded by !test/**
  • test/ui2_paint_activity_test.dart is excluded by !test/**
  • test/ui2_router_test.dart is excluded by !test/**
  • test/ui2_scroll_hint_test.dart is excluded by !test/**
  • test/ui2_sleep_detail_test.dart is excluded by !test/**
  • test/ui2_tokens_test.dart is excluded by !test/**
  • test/ui2_wiring_r2_test.dart is excluded by !test/**
  • test/ui_kit_new_widgets_test.dart is excluded by !test/**
  • test/units_controller_test.dart is excluded by !test/**
  • test/v25_refusal_test.dart is excluded by !test/**
  • test/wear_coverage_test.dart is excluded by !test/**
  • test/wear_gap_reason_test.dart is excluded by !test/**
  • test/week_view_feed_test.dart is excluded by !test/**
  • test/widget_service_sentinels_test.dart is excluded by !test/**
  • test/workout_calorie_anchors_test.dart is excluded by !test/**
  • test/workout_enrichment_test.dart is excluded by !test/**
  • test/workout_filter_test.dart is excluded by !test/**
  • test/workout_health_mapping_test.dart is excluded by !test/**
  • test/workout_max_hr_spike_test.dart is excluded by !test/**
  • test/workout_reliability_test.dart is excluded by !test/**
  • test/workout_share_card_test.dart is excluded by !test/**
  • test/workout_sleep_redesign_test.dart is excluded by !test/**
  • test/workout_stop_durability_test.dart is excluded by !test/**
  • test/workout_types_label_test.dart is excluded by !test/**
  • test/workouts_header_actions_test.dart is excluded by !test/**
📒 Files selected for processing (235)
  • .coderabbit.yaml
  • .github/workflows/build.yml
  • .gitignore
  • PRIVACY.md
  • UI_AUDIT.html
  • docs/notice.html
  • docs/privacy.html
  • guides/TASKER_INTEGRATION.md
  • lib/ai/ai_prefs.dart
  • lib/ai/briefing.dart
  • lib/ai/briefing_engine.dart
  • lib/ai/nightly_sweep.dart
  • lib/ai/reminder_plan.dart
  • lib/app.dart
  • lib/ble/ble_engine.dart
  • lib/ble/ble_state.dart
  • lib/ble/hr_sensor.dart
  • lib/ble/live_cadence.dart
  • lib/ble/live_step_runs.dart
  • lib/ble/proximity_policy.dart
  • lib/cloud/cloud_import.dart
  • lib/coach/coach_actions.dart
  • lib/coach/coach_config.dart
  • lib/coach/coach_engine.dart
  • lib/coach/coach_prompt.dart
  • lib/compute/background_derivation.dart
  • lib/compute/crossday_pipeline.dart
  • lib/compute/derivation_engine.dart
  • lib/compute/derive_prepare.dart
  • lib/compute/findings.dart
  • lib/compute/hr_max.dart
  • lib/compute/manual_session.dart
  • lib/compute/onehz_pipeline.dart
  • lib/compute/profile.dart
  • lib/compute/strain_backfill.dart
  • lib/compute/substrate.dart
  • lib/data/csv_export.dart
  • lib/data/db.dart
  • lib/data/journal_fields.dart
  • lib/data/live_coverage_policy.dart
  • lib/data/local_repository.dart
  • lib/data/local_repository_impl.dart
  • lib/data/med_store.dart
  • lib/data/models.dart
  • lib/data/nutrition_store.dart
  • lib/data/off_lookup.dart
  • lib/gps/route_types.dart
  • lib/health/health_export.dart
  • lib/health/health_import_state.dart
  • lib/health/health_measurement_import.dart
  • lib/health/health_profile_import.dart
  • lib/health/health_rhr_seed.dart
  • lib/health/health_workout_import.dart
  • lib/health/phone_pedometer.dart
  • lib/import/backup_crypto.dart
  • lib/import/journal_csv_import.dart
  • lib/import/whoop_import.dart
  • lib/models/metric.dart
  • lib/models/payloads.dart
  • lib/notify/battery_forecast.dart
  • lib/notify/device_alerts.dart
  • lib/notify/notification_center.dart
  • lib/notify/notification_event.dart
  • lib/notify/notification_prefs.dart
  • lib/notify/notification_service.dart
  • lib/notify/tap_router.dart
  • lib/notify/water_buzzer.dart
  • lib/platform/app_icon.dart
  • lib/platform/tasker_bridge.dart
  • lib/state/app_state.dart
  • lib/state/prefs.dart
  • lib/state/units_controller.dart
  • lib/stress/breath_phases.dart
  • lib/stress/session_effect.dart
  • lib/sync/background_sync.dart
  • lib/sync/band_ownership.dart
  • lib/sync/headless_boot.dart
  • lib/sync/headless_gate.dart
  • lib/sync/ios_bg_task.dart
  • lib/sync/sync_policy.dart
  • lib/sync/update_service.dart
  • lib/telemetry/health_uploader.dart
  • lib/ui/activity/live_session_screen.dart
  • lib/ui/activity/strain_detail_screen.dart
  • lib/ui/activity/workout_share_card.dart
  • lib/ui/ai/ai_breakdown_screen.dart
  • lib/ui/ai/ai_settings_screen.dart
  • lib/ui/coach/ai_coach_screen.dart
  • lib/ui/coach/coach_chart.dart
  • lib/ui/coach/coach_render.dart
  • lib/ui/coach/coach_screen.dart
  • lib/ui/coach/coach_settings_screen.dart
  • lib/ui/cycle/cycle_screen.dart
  • lib/ui/design/ai_hero.dart
  • lib/ui/design/app_scaffold.dart
  • lib/ui/design/arc_gauge.dart
  • lib/ui/design/bento.dart
  • lib/ui/design/big_stat.dart
  • lib/ui/design/controls.dart
  • lib/ui/design/design.dart
  • lib/ui/design/disclosure.dart
  • lib/ui/design/domains.dart
  • lib/ui/design/fake_route_fixture.dart
  • lib/ui/design/gallery_screen.dart
  • lib/ui/design/hypnogram.dart
  • lib/ui/design/info_sheet.dart
  • lib/ui/design/metric_card.dart
  • lib/ui/design/motion.dart
  • lib/ui/design/nav_pill.dart
  • lib/ui/design/orbit_score.dart
  • lib/ui/design/pressable.dart
  • lib/ui/design/recap_card.dart
  • lib/ui/design/ring_week.dart
  • lib/ui/design/rows.dart
  • lib/ui/design/spark.dart
  • lib/ui/design/state_chips.dart
  • lib/ui/design/surface.dart
  • lib/ui/design/sync_dot.dart
  • lib/ui/find/find_band_screen.dart
  • lib/ui/heart/live_hr_tile.dart
  • lib/ui/import/import_screen.dart
  • lib/ui/insights/coach_cards.dart
  • lib/ui/journal/custom_journal_field_sheet.dart
  • lib/ui/journal/journal_compose_screen.dart
  • lib/ui/journal/journal_metric_editor.dart
  • lib/ui/journal/journal_screen.dart
  • lib/ui/journey/day_nav.dart
  • lib/ui/journey/journey_screen.dart
  • lib/ui/kit/charts.dart
  • lib/ui/kit/kit.dart
  • lib/ui/kit/os_icons.dart
  • lib/ui/kit/route_map.dart
  • lib/ui/kit/share_origin.dart
  • lib/ui/kit/skeleton.dart
  • lib/ui/kit/state_card.dart
  • lib/ui/labs/lab_entry_sheet.dart
  • lib/ui/labs/labs_screen.dart
  • lib/ui/onboarding/welcome_screen.dart
  • lib/ui/pairing_screen.dart
  • lib/ui/profile/about_screen.dart
  • lib/ui/profile/advanced_data_screen.dart
  • lib/ui/profile/data_history_screen.dart
  • lib/ui/profile/gesture_section.dart
  • lib/ui/profile/notification_relay_section.dart
  • lib/ui/profile/notification_settings_screen.dart
  • lib/ui/profile/profile_screen.dart
  • lib/ui/profile_setup_screen.dart
  • lib/ui/recap/recap_screen.dart
  • lib/ui/records/records_screen.dart
  • lib/ui/screens/detail_cards.dart
  • lib/ui/screens/metric_row.dart
  • lib/ui/screens/metric_screen.dart
  • lib/ui/screens/screens.dart
  • lib/ui/screens/trend_screen.dart
  • lib/ui/sleep/sleep_detail_screen.dart
  • lib/ui/sleep/sleep_periods_screen.dart
  • lib/ui/splash/boot_splash.dart
  • lib/ui/spotcheck/spot_check_screen.dart
  • lib/ui/stress/calm_breathing_screen.dart
  • lib/ui/stress/interval_timer_screen.dart
  • lib/ui/stress/stress_screen.dart
  • lib/ui/timeline/timeline_screen.dart
  • lib/ui/today/ai_summary_card.dart
  • lib/ui/today/step_goal_screen.dart
  • lib/ui/today/today_screen.dart
  • lib/ui/widgets/async_guards.dart
  • lib/ui/widgets/screen_loader.dart
  • lib/ui/widgets/status_banner.dart
  • lib/ui/workouts/calorie_heatmap.dart
  • lib/ui/workouts/manual_workout_screen.dart
  • lib/ui/workouts/workout_filter.dart
  • lib/ui/workouts/workout_filter_sheet.dart
  • lib/ui/workouts/workout_types.dart
  • lib/ui/workouts/workouts_screen.dart
  • lib/ui2/README.md
  • lib/ui2/activity/catalogue.dart
  • lib/ui2/activity/day_strain.dart
  • lib/ui2/activity/live.dart
  • lib/ui2/activity/picker.dart
  • lib/ui2/activity/poster.dart
  • lib/ui2/activity/setup.dart
  • lib/ui2/activity/share.dart
  • lib/ui2/activity/summary.dart
  • lib/ui2/activity/tiles.dart
  • lib/ui2/activity/zones.dart
  • lib/ui2/app_shell.dart
  • lib/ui2/charts.dart
  • lib/ui2/grammar.dart
  • lib/ui2/onboarding/pairing.dart
  • lib/ui2/onboarding/profile_setup.dart
  • lib/ui2/onboarding/splash.dart
  • lib/ui2/onboarding/welcome.dart
  • lib/ui2/paint_activity.dart
  • lib/ui2/profile/alarm.dart
  • lib/ui2/profile/data.dart
  • lib/ui2/profile/devices.dart
  • lib/ui2/profile/gallery.dart
  • lib/ui2/profile/phone_import.dart
  • lib/ui2/profile/profile.dart
  • lib/ui2/profile/settings.dart
  • lib/ui2/screens/ai_briefing.dart
  • lib/ui2/screens/beats.dart
  • lib/ui2/screens/calm_breathing.dart
  • lib/ui2/screens/circadian_detail.dart
  • lib/ui2/screens/coach.dart
  • lib/ui2/screens/coach_figures.dart
  • lib/ui2/screens/cycle_screen.dart
  • lib/ui2/screens/day_steps.dart
  • lib/ui2/screens/day_timeline.dart
  • lib/ui2/screens/driver_breakdown.dart
  • lib/ui2/screens/findings_log.dart
  • lib/ui2/screens/health_screen.dart
  • lib/ui2/screens/home_screen.dart
  • lib/ui2/screens/investigate.dart
  • lib/ui2/screens/journal_compose.dart
  • lib/ui2/screens/log_food.dart
  • lib/ui2/screens/log_water.dart
  • lib/ui2/screens/metric_detail.dart
  • lib/ui2/screens/month_grid.dart
  • lib/ui2/screens/naps.dart
  • lib/ui2/screens/nutrition_screen.dart
  • lib/ui2/screens/readiness_detail.dart
  • lib/ui2/screens/rough_night.dart
  • lib/ui2/screens/scan_barcode.dart
  • lib/ui2/screens/screens.dart
  • lib/ui2/screens/sleep_detail.dart
  • lib/ui2/screens/start_card.dart
  • lib/ui2/screens/wellness_screen.dart
  • lib/ui2/screens/what_changed.dart
  • lib/ui2/screens/workout_screen.dart
  • lib/ui2/scroll_hint.dart
  • lib/ui2/theme.dart
  • lib/ui2/ui2.dart
  • lib/widget/widget_service.dart
  • pubspec.yaml

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit aa7f80e)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 5 🔵🔵🔵🔵🔵
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

UTC-vs-local day boundary

_loadWorkoutData computes end as DateTime(now.year, now.month, now.day) — a local midnight — and then weekStart as end.subtract(Motion.tick * 86400 * (end.weekday - 1)). Subtracting a fixed 86400-second multiple across a DST boundary produces the wrong local date (the day after a spring-forward is 23 h, so subtracting 86400 s lands in the previous day). _daySlot and lastSevenDays both use UTC midnights to avoid exactly this, but weekStart does not, so sessions on the first day of the week can be miscounted or dropped from thisWeek, weekDays, and weekImported on DST-transition days. AGENTS.md §3.7 and §4.8 flag this pattern explicitly.

final weekStart = end.subtract(Motion.tick * 86400 * (end.weekday - 1));
final thisWeek = [for (final w in past) if (w.start.isAfter(weekStart)) w];
Sticky boolean latch

_importing is set to true at the top of _importWorkouts and cleared in the finally block only when mounted is true. If the widget is disposed while the import is in flight, mounted is false, the finally branch is skipped, and _importing stays true permanently. A subsequent tap on the Import button is silently ignored (if (_importing) return). The fix is to clear _importing unconditionally in finally (the field write is safe even after dispose; only setState needs the guard). AGENTS.md §4.3 identifies this exact pattern as a recurring wedge.

} finally {
  if (mounted) {
    setState(() {
      _importing = false;
      _load = _loadWorkoutData(context.read<AppState>());
    });
  }
}
Absent-input fabrication

In _loadWorkoutData, when getWorkouts returns a row whose type field does not match any catalogue entry, the code substitutes a hardcoded Activity('Workout', LucideIcons.activity, C.purple, Track.duration, 5.0) — including a MET value of 5.0. That MET is used downstream for calorie estimation. A session type the catalogue does not know should produce null calories (or no calorie line), not a confident figure derived from an arbitrary MET. AGENTS.md §3.3 and §4.1 prohibit exactly this substitution.

final a = activityByName(r['type'] as String?) ??
    const Activity('Workout', LucideIcons.activity, C.purple,
        Track.duration, 5.0);
past.add(_PastWorkout(

⚠️ Review coverage: The following files were not included in this review because of the token budget:

  • lib/data/db.dart
  • lib/compute/derivation_engine.dart
  • lib/state/app_state.dart
  • lib/data/local_repository_impl.dart
  • lib/ui2/profile/gallery.dart
  • lib/ble/ble_engine.dart
  • lib/ui2/activity/live.dart
  • lib/ui2/grammar.dart
  • lib/ui2/activity/summary.dart
  • lib/ui2/screens/sleep_detail.dart
  • lib/ui2/screens/home_screen.dart
  • test/ui2_activity_test.dart
  • lib/compute/onehz_pipeline.dart
  • lib/ui2/screens/health_screen.dart
  • test/ui2_wiring_r2_test.dart
  • lib/ui2/screens/wellness_screen.dart
  • lib/ui2/profile/settings.dart
  • lib/ui2/charts.dart
  • lib/ui2/screens/metric_detail.dart
  • lib/ui2/screens/cycle_screen.dart
  • lib/ui2/screens/investigate.dart
  • lib/compute/crossday_pipeline.dart
  • test/ui2_home_health_golden_test.dart
  • lib/ui2/screens/calm_breathing.dart
  • lib/ui2/activity/poster.dart
  • test/ui2_charts_test.dart
  • lib/ui2/screens/day_timeline.dart
  • lib/app.dart
  • lib/ui2/screens/coach.dart
  • lib/ui2/screens/beats.dart
  • lib/ui2/screens/circadian_detail.dart
  • lib/compute/substrate.dart
  • lib/ui2/screens/nutrition_screen.dart
  • lib/ui2/screens/log_food.dart
  • lib/ui2/screens/journal_compose.dart
  • lib/ui2/profile/devices.dart
  • lib/ui2/onboarding/welcome.dart
  • lib/ui2/screens/driver_breakdown.dart
  • lib/ui2/screens/coach_figures.dart
  • lib/ui2/theme.dart
  • lib/data/nutrition_store.dart
  • lib/ui2/screens/rough_night.dart
  • test/ui2_router_test.dart
  • lib/ui2/activity/tiles.dart
  • lib/data/off_lookup.dart
  • test/startup_and_rebuild_recovery_test.dart
  • test/hr_ceiling_zones_test.dart
  • lib/ui2/screens/day_steps.dart
  • lib/ui2/activity/share.dart
  • lib/ui2/activity/zones.dart
    ... and 248 more

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Latest suggestions up to aa7f80e

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Rebuild event record missing when salvage throws

_db = fresh is assigned before _mergeFromDbFile completes. If _mergeFromDbFile
itself calls instance (which resolves _db), it correctly gets the fresh database.
However, if _mergeFromDbFile throws after partially writing rows, _db is already set
to fresh and lastRebuild is never assigned — the app opens with a partially-salvaged
database and no record of the rebuild event, so AppState never tells the user what
happened. lastRebuild should be assigned unconditionally (even with an empty salvage
map) before returning, which the current code does only in the success path of the
outer try. Move lastRebuild assignment into a finally block so it is always set when
a rebuild occurs.

lib/data/db.dart [130-196]

 static Future<Database> _openOrRebuild() async {
-try {
-  return await _open();
-} catch (e) {
-  ...
-  _db = fresh;
-  var salvaged = const <String, int>{};
   try {
-    salvaged = Map.of(
-      await _mergeFromDbFile(
-        quarantine,
-        only: _salvageTables,
-        tolerant: true,
-      ),
-    )..remove('_days');
-  } catch (_) {
-    // The quarantined file gave us nothing.
+    return await _open();
+  } catch (e) {
+    final dir = await getDatabasesPath();
+    final path = p.join(dir, dbName);
+    final quarantine = p.join(
+      dir,
+      '$dbName.unopenable-${DateTime.now().millisecondsSinceEpoch}',
+    );
+    final moved = <String>[];
+    for (final suffix in const ['', '-wal', '-shm']) {
+      final f = File('$path$suffix');
+      if (!f.existsSync()) continue;
+      await f.rename('$quarantine$suffix');
+      moved.add(suffix);
+    }
+    final Database fresh;
+    try {
+      fresh = await _open();
+    } catch (_) {
+      for (final suffix in moved) {
+        await File('$quarantine$suffix').rename('$path$suffix');
+      }
+      rethrow;
+    }
+    _db = fresh;
+    var salvaged = const <String, int>{};
+    try {
+      salvaged = Map.of(
+        await _mergeFromDbFile(
+          quarantine,
+          only: _salvageTables,
+          tolerant: true,
+        ),
+      )..remove('_days');
+    } catch (_) {
+      // The quarantined file gave us nothing.
+    } finally {
+      lastRebuild = (
+        cause: '$e',
+        quarantinePath: quarantine,
+        salvaged: salvaged,
+      );
+    }
+    return fresh;
   }
-  lastRebuild = (
-    cause: '$e',
-    quarantinePath: quarantine,
-    salvaged: salvaged,
-  );
-  return fresh;
-}
 }
Suggestion importance[1-10]: 7

__

Why: This is a valid and impactful bug: if _mergeFromDbFile throws an uncaught exception (beyond the inner catch (_)), lastRebuild is never set and AppState cannot inform the user about the rebuild. Moving lastRebuild assignment into a finally block ensures the rebuild event is always recorded, which is the stated design goal of the feature.

Medium
Burn plain dedupe slot unconditionally on medical finding

The comment explains that a presented medical exception should also burn the plain
'$date:exception' slot to prevent a de-escalation from firing a second notification.
However, recordFired is only called when fired is true, meaning if the medical
notification was suppressed by quiet hours (fired == false), the plain slot remains
open and a subsequent lower-priority finding will still go out — exactly the
scenario the comment says must not happen. The plain slot should be burned whenever
a medical finding is present, regardless of whether the OS actually delivered the
notification.

lib/compute/derivation_engine.dart [4248-4250]

-if (medical && fired) {
+if (medical) {
   await const FiredKeyStore().recordFired('$date:exception');
 }
Suggestion importance[1-10]: 6

__

Why: The suggestion raises a valid edge case: if the OS suppresses the medical notification (e.g., quiet hours), fired is false and the plain '$date:exception' slot stays open, allowing a lower-priority de-escalation to fire later. However, the comment in the PR explicitly says "a medical exception burns the day's plain slot" and "only on a real present, or a medical one lost to quiet hours would take the plain one down with it" — suggesting the current fired gate is intentional design, not a bug. The suggestion contradicts the stated design intent.

Low
Fix sticky latch in run() by using the shared lock helper

The run() method still sets _running = true directly at its entry point and clears
it manually at the end of the happy path, bypassing _withRunLock. This means the
finally reset in _withRunLock is not protecting run(), so any exception thrown
before the manual _running = false at the bottom of run() leaves the latch
permanently set — the exact sticky-boolean-latch bug pattern the repo documents.
run() should be converted to use _withRunLock so the finally block guarantees the
reset on every exit path.

lib/compute/derivation_engine.dart [1642-1656]

-static bool _running = false;
-bool get running => _running;
-
-/// Run [body] under the process-wide derivation lock, returning [busy]
-/// unchanged if a pass is already in flight. Only for entry points that do
-/// NOT call another locked entry point (which would deadlock-by-skip).
-static Future<T> _withRunLock<T>(T busy, Future<T> Function() body) async {
-  if (_running) return busy;
-  _running = true;
-  try {
-    return await body();
-  } finally {
-    _running = false;
-  }
+// In run(): replace the manual _running guard with _withRunLock
+Future<int> run(...) async {
+  return _withRunLock(0, () async {
+    // ... existing body of run(), removing the manual _running = true/false ...
+  });
 }
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that run() still manually sets _running = true/false rather than using _withRunLock, which could leave the latch stuck on exception. However, the improved_code is a sketch rather than a concrete patch, and the PR's existing code at line 1924 does _running = false in what appears to be a finally-equivalent position, making the actual risk unclear without seeing the full run() body.

Low
Avoid fabricating absence reason for readiness metric

When readinessScalar == null && showOvernight is true but _needNote returns null,
the code falls back to kUnknownAbsenceNote. However, kUnknownAbsenceNote is a
fabricated reason — the pipeline may have a real note on the envelope that was
simply not found at this path. This violates the "never fabricate a metric" rule: an
absent readiness with an unknown cause should emit null (abstain) rather than a
confident but invented note. Use null as the fallback instead of
kUnknownAbsenceNote.

lib/data/local_repository_impl.dart [351-354]

 final readinessNote = readinessScalar == null && showOvernight
-    ? _needNote(sleepBundle, 'clinical.readiness_composite') ??
-          kUnknownAbsenceNote
+    ? _needNote(sleepBundle, 'clinical.readiness_composite')
     : overnightNote;
Suggestion importance[1-10]: 4

__

Why: The suggestion argues that kUnknownAbsenceNote is a fabricated reason, but the PR's own comment explicitly states "the floor is 'we do not know', never a plausible guess" — meaning kUnknownAbsenceNote is intentionally used as an honest "unknown" sentinel rather than a fabricated specific reason. Removing it would leave the note as null, which may cause the UI to render a bare dash with no explanation at all, which the PR is specifically trying to fix.

Low
Avoid fabricating absence notes for activity metrics

activityNote always returns a non-null string, but _scalarMetric only attaches the
note when v == null. When a value IS present, the note is silently dropped
(correct), but when a value is absent and neither the bundle nor wake features carry
a named reason, kUnknownAbsenceNote is returned — a fabricated reason rather than an
honest abstention. Per the repo's invariant, absent input must yield null/"—", not
an invented explanation. Return null instead of kUnknownAbsenceNote so _absentMetric
produces a genuinely absent envelope with no note rather than a confident but false
one.

lib/data/local_repository_impl.dart [373-382]

-String activityNote(String key) {
+String? activityNote(String key) {
   if (activityBundle == null && wakeFeatures == null) {
     return needInputNote('today_activity');
   }
   if (activityBundle != null) {
-    return _absentNote(activityBundle, key) ?? kUnknownAbsenceNote;
+    return _absentNote(activityBundle, key);
   }
   final n = (wakeFeatures?['absent_notes'] as Map?)?[key];
-  return n is String && n.isNotEmpty ? n : kUnknownAbsenceNote;
+  return n is String && n.isNotEmpty ? n : null;
 }
Suggestion importance[1-10]: 4

__

Why: Similar to suggestion 1, the PR explicitly uses kUnknownAbsenceNote as an honest "we do not know" floor rather than a fabricated specific reason. The PR comment states "the floor is 'we do not know', never a plausible guess." Returning null instead would leave absent metrics with no note, which is the problem the PR is fixing. However, changing the return type to String? could have merit if the downstream _scalarMetric handles null notes correctly.

Low
Absent HR sentinel handling inconsistent with nullable Sample field

decoded.hr is typed as int (non-nullable) in Sample, so decoded.hr > 0 silently
treats a legitimately absent HR field (which the decoder maps to 0) as null —
correct for the sentinel case. However, if Sample.hr is ever nullable (as the schema
now allows), this comparison would throw a null-dereference. More critically, the
gen5 path above constructs Sample with hr: g.heartRate, where g.heartRate could be 0
for a record with no beat; the same > 0 guard handles it, but the pattern should use
decoded.hr null-awareness consistently. Use decoded.hr with a null-safe check
matching how the gen5 Sample is constructed (hr: g.heartRate == 0 ? null :
g.heartRate) so the write path and the construction path agree on the sentinel.

lib/data/db.dart [3899]

-'hr': decoded.hr > 0 ? decoded.hr : null,
+'hr': (decoded.hr != null && decoded.hr! > 0) ? decoded.hr : null,
Suggestion importance[1-10]: 3

__

Why: The suggestion raises a valid concern about null-safety consistency, but Sample.hr appears to be a non-nullable int in the existing codebase (the PR code uses decoded.hr > 0 without null checks elsewhere), making the decoded.hr! in the improved code unnecessary. The improved code adds ! which would be a compile error if hr is non-nullable, reducing the suggestion's validity.

Low
General
Fix DST-unsafe day subtraction for yesterday label

dayLabelOf from data/day_label.dart is the correct helper for local day labels, but
DateTime.now().subtract(const Duration(days: 1)) subtracts exactly 86 400 seconds,
which is wrong on DST transition days (23 h or 25 h). Per AGENTS.md §3 invariant 7,
day-length arithmetic must not assume 86 400 s. Use dayLabelOf with a DateTime
constructed from local midnight minus one day via the calendar, or use the existing
LocalDb.localDayLabelNow() approach adjusted by calendar days.

lib/compute/derivation_engine.dart [4136-4140]

 final today = LocalDb.localDayLabelNow();
-final yesterday = dayLabelOf(
-  DateTime.now().subtract(const Duration(days: 1)),
-);
+final now = DateTime.now();
+final yesterday = dayLabelOf(DateTime(now.year, now.month, now.day - 1));
 if (date == null || (date != today && date != yesterday)) return;
Suggestion importance[1-10]: 7

__

Why: Using DateTime.now().subtract(const Duration(days: 1)) is indeed DST-unsafe on 23h/25h transition days, and DateTime(now.year, now.month, now.day - 1) correctly uses calendar arithmetic. The fix is accurate, minimal, and directly addresses a real correctness issue on DST boundaries.

Medium
Use trailing window helper for baseline night count

LocalDb.metricSeries(key) is ORDER BY date ASC LIMIT n and returns the oldest
rows, not a trailing window. Using .length on its result to count how many baseline
nights exist is correct only if the table has fewer rows than the default limit —
once the user has more rows than the limit, the count is capped and the "have=N"
figure shown to the user is wrong. Use trailingSeriesValues('skin_temp_adc', n) or a
dedicated count query instead.

lib/data/local_repository_impl.dart [739-740]

-final have = (await LocalDb.metricSeries('skin_temp_adc')).length;
+final have = (await LocalDb.trailingSeriesValues('skin_temp_adc', 90)).length;
 return {'value': null, 'note': 'need_baseline:have=$have,need=3'};
Suggestion importance[1-10]: 3

__

Why: The suggestion raises a valid concern about metricSeries potentially being limited and returning fewer rows than actually exist. However, LocalDb.trailingSeriesValues is referenced without confirmation it exists in the codebase, making this suggestion potentially incorrect. The issue itself is real but the proposed fix references an unverified API.

Low

Previous suggestions

Suggestions up to commit 85d6490
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix DST-unsafe yesterday day-label computation

yesterday is computed by subtracting exactly 24 hours from DateTime.now(), which is
incorrect on DST transition days (23-hour or 25-hour days). Per AGENTS.md invariant
§3.7, day-length arithmetic must not assume 86400 s. Use dayLabelOf on a DateTime
that steps back by one calendar day via local midnight arithmetic, or use the
existing LocalDb.localDayLabelNow() approach consistently.

lib/compute/derivation_engine.dart [4130-4134]

 final today = LocalDb.localDayLabelNow();
+final now = DateTime.now();
 final yesterday = dayLabelOf(
-  DateTime.now().subtract(const Duration(days: 1)),
+  DateTime(now.year, now.month, now.day - 1),
 );
 if (date == null || (date != today && date != yesterday)) return;
Suggestion importance[1-10]: 7

__

Why: Subtracting Duration(days: 1) (exactly 86400 s) from DateTime.now() is incorrect on DST transition days, which can be 23 or 25 hours long. Using DateTime(now.year, now.month, now.day - 1) correctly steps back one calendar day in local time, and the improved_code accurately reflects this fix.

Medium
Clear stale rebuild record on successful database open

_db = fresh is assigned before _mergeFromDbFile runs. If _mergeFromDbFile itself
calls instance (which returns _db when open), it will correctly resolve to fresh.
However, if mergeFromDbFile throws partway through the salvage and the outer catch
(
) swallows it, lastRebuild is still set with whatever partial salvaged map was
built before the throw — but salvaged is const <String, int>{} in that case because
the assignment salvaged = Map.of(...) never completed. This is correct. The real
issue is that lastRebuild is a static field with no reset: if the app opens
successfully on the next cold start (no rebuild needed), lastRebuild from the
previous session persists in memory and AppState will show a stale rebuild card.
lastRebuild should be set to null at the start of a successful _open() path, or
cleared in _openOrRebuild before the try.

lib/data/db.dart [130-196]

 static Future<Database> _openOrRebuild() async {
   try {
-    return await _open();
+    final db = await _open();
+    lastRebuild = null; // Successful open — clear any stale rebuild record.
+    return db;
   } catch (e) {
-    ...
+    final dir = await getDatabasesPath();
+    final path = p.join(dir, dbName);
+    final quarantine = p.join(
+      dir,
+      '$dbName.unopenable-${DateTime.now().millisecondsSinceEpoch}',
+    );
+    final moved = <String>[];
+    for (final suffix in const ['', '-wal', '-shm']) {
+      final f = File('$path$suffix');
+      if (!f.existsSync()) continue;
+      await f.rename('$quarantine$suffix');
+      moved.add(suffix);
+    }
+    final Database fresh;
+    try {
+      fresh = await _open();
+    } catch (_) {
+      for (final suffix in moved) {
+        await File('$quarantine$suffix').rename('$path$suffix');
+      }
+      rethrow;
+    }
     _db = fresh;
     var salvaged = const <String, int>{};
     try {
       salvaged = Map.of(
         await _mergeFromDbFile(
           quarantine,
           only: _salvageTables,
           tolerant: true,
         ),
       )..remove('_days');
-    } catch (_) {
-      // The quarantined file gave us nothing.
-    }
+    } catch (_) {}
     lastRebuild = (
       cause: '$e',
       quarantinePath: quarantine,
       salvaged: salvaged,
     );
     return fresh;
   }
 }
Suggestion importance[1-10]: 6

__

Why: The lastRebuild static field persists across app restarts in memory within the same process lifecycle, and clearing it on a successful open prevents AppState from showing a stale rebuild card. This is a valid correctness concern, though its impact depends on whether the app process is restarted between sessions (on mobile, it typically is).

Low
Boolean latch never reset on exception in run()

run() still sets _running = true manually at its entry point (line _running = true;
after if (_running) return 0;) and clears it with _running = false; near the end of
the method body, outside of a try/finally. If an exception escapes between those two
points — before the finally block that calls _runStorageHousekeeping_running is
never cleared and the engine wedges permanently. run() should use _withRunLock (or
at minimum wrap its body in try/finally) the same way deriveImported now does.

lib/compute/derivation_engine.dart [1636-1650]

-static bool _running = false;
-bool get running => _running;
-...
-static Future<T> _withRunLock<T>(T busy, Future<T> Function() body) async {
-  if (_running) return busy;
-  _running = true;
-  try {
-    return await body();
-  } finally {
-    _running = false;
-  }
+Future<int> run(...) async {
+  return _withRunLock(0, () async {
+    // ... existing body of run(), removing the manual _running = true/false ...
+  });
 }
Suggestion importance[1-10]: 6

__

Why: The PR converts _running to a static field and introduces _withRunLock for deriveImported, but run() still manually sets _running = true/false without a try/finally guard, meaning an unhandled exception could permanently wedge the engine. This is a real correctness concern, though the improved_code is only a sketch.

Low
Prevent fabricated absence notes for ungated activity metrics

activityNote always returns a non-null string, but _scalarMetric only attaches the
note when v == null. When a value IS present the note is silently dropped (correct),
but when the value is absent the caller passes this note unconditionally — meaning
kUnknownAbsenceNote is fabricated for keys like steps and wear_min that have no
named gate, instead of returning null and letting the UI render an honest "—".
Return null instead of kUnknownAbsenceNote so absent metrics with no known reason
abstain rather than invent one.

lib/data/local_repository_impl.dart [373-382]

-String activityNote(String key) {
+String? activityNote(String key) {
   if (activityBundle == null && wakeFeatures == null) {
     return needInputNote('today_activity');
   }
   if (activityBundle != null) {
-    return _absentNote(activityBundle, key) ?? kUnknownAbsenceNote;
+    return _absentNote(activityBundle, key);
   }
   final n = (wakeFeatures?['absent_notes'] as Map?)?[key];
-  return n is String && n.isNotEmpty ? n : kUnknownAbsenceNote;
+  return n is String && n.isNotEmpty ? n : null;
 }
Suggestion importance[1-10]: 5

__

Why: The PR's own comment states "the floor is 'we do not know', never a plausible guess" and that steps and wear_min "say exactly that rather than borrowing the strain gate's reason" — meaning kUnknownAbsenceNote is intentionally used as the floor. However, the suggestion raises a valid concern about whether returning kUnknownAbsenceNote vs null is the right choice for ungated metrics, making it a moderate-impact design question.

Low
Avoid fabricating absence reason for readiness metric

When readinessScalar == null && showOvernight is true but overnightNote is non-null
(i.e. showOvernight is false), the ternary correctly falls to overnightNote. However
when showOvernight is true and readinessScalar == null, the fallback to
kUnknownAbsenceNote fabricates a reason for an absence that may simply be "not yet
derived today" — violating the never-fabricate rule. The note should remain null
rather than substituting kUnknownAbsenceNote when the pipeline itself returned
nothing, so the UI renders an honest "—" with no explanation rather than a wrong
one.

lib/data/local_repository_impl.dart [351-354]

 final readinessNote = readinessScalar == null && showOvernight
-    ? _needNote(sleepBundle, 'clinical.readiness_composite') ??
-          kUnknownAbsenceNote
+    ? _needNote(sleepBundle, 'clinical.readiness_composite')
     : overnightNote;
Suggestion importance[1-10]: 4

__

Why: The suggestion argues that falling back to kUnknownAbsenceNote fabricates a reason, but the PR's own comment says "the floor is 'we do not know', never a plausible guess" — so using kUnknownAbsenceNote as a floor is intentional design. The suggestion contradicts the stated intent of the PR, making it low-value.

Low
Use trailing window query for skin-temp baseline count

LocalDb.metricSeries is ORDER BY date ASC LIMIT n and returns the oldest rows, not a
trailing count. For a baseline gate that needs to know how many recent ADC nights
exist, trailingSeriesValues('skin_temp_adc', 3).length (or an equivalent trailing
query) should be used instead, otherwise a user with 100 old nights and a band swap
still shows have=100 and skips the "need 3 nights" gate entirely.

lib/data/local_repository_impl.dart [739-740]

-final have = (await LocalDb.metricSeries('skin_temp_adc')).length;
+final have = (await LocalDb.trailingSeriesValues('skin_temp_adc', 3)).length;
 return {'value': null, 'note': 'need_baseline:have=$have,need=3'};
Suggestion importance[1-10]: 3

__

Why: The suggestion references LocalDb.trailingSeriesValues which may not exist in the codebase, and the existing code (metricSeries('skin_temp_adc').length) was unchanged by this PR — it's pre-existing behavior. The concern about stale counts is valid but speculative without knowing the actual metricSeries implementation.

Low
Fix race in medical-finding dedupe slot burning

When a medical finding fires, the code burns the plain '$date:exception' slot via
FiredKeyStore to prevent a later de-escalation from re-notifying. However,
NotificationCenter.emit already uses its own dedupe guard internally, so the plain
slot also needs to be burned through NotificationCenter (or its underlying
FiredKeyStore) to be consistent — but more critically, the comment says "a presented
medical exception burns the day's plain slot", yet the plain key '$date:exception'
is only recorded in FiredKeyStore directly, bypassing whatever in-memory or lock
state NotificationCenter maintains. This means a concurrent emit with the plain key
could still race through NotificationCenter's own check-then-record before
FiredKeyStore.recordFired completes, re-firing the notification. The plain slot
should be burned by emitting a no-op through NotificationCenter or by recording it
before the medical emit returns.

lib/compute/derivation_engine.dart [4242-4244]

 if (medical && fired) {
   await const FiredKeyStore().recordFired('$date:exception');
+  // Also burn the plain slot through NotificationCenter so its internal
+  // dedupe state is consistent and a racing plain-key emit cannot slip through.
+  await const FiredKeyStore().recordFired('$date:exception:medical');
 }
Suggestion importance[1-10]: 2

__

Why: The improved_code doesn't actually fix the described race — it just records '$date:exception:medical' again (the key already used for the medical emit), which is redundant. The suggestion's logic is confused: the plain slot '$date:exception' is already being recorded, and the "fix" doesn't address the alleged race condition in a meaningful way.

Low
General
Document intentional v25 beat-drop to prevent future regression

When decoded == null because _decodeOneHzSample returned null for a gen4 v25 record
(explicitly refused with return null when r.histVersion == 25), the code falls into
the same branch as R10-lite and checks _isGen4R10LiteHistorical. A v25 record is not
R10-lite, so the check fails and 0 is returned — correct. However, if sample != null
and sample.rrIntervalsMs.isNotEmpty but the record is v25 (not R10-lite), the beats
are silently dropped even though the comment says "dropping it here is permanent".
The guard should also exclude v25 records explicitly, since the comment in
_decodeOneHzSample states v25 beats should not be banked (the record stays in
raw_archive). Add a v25 hex-prefix check or reuse the existing
_isGen4R10LiteHistorical guard as-is — but document that v25 beats are intentionally
dropped here too, to prevent a future caller from adding a v25 R-R path that would
ACK and trim the band's flash for records that were refused.

lib/data/db.dart [3868-3878]

 if (sample != null &&
     sample.rrIntervalsMs.isNotEmpty &&
     _isGen4R10LiteHistorical(raw.hex)) {
   return _queueRrBeats(
     batch,
     _recTsFrom(raw, sample),
     sample,
     deviceFamily: deviceFamily,
   );
 }
+// v25 records are refused by _decodeOneHzSample and must not have their
+// beats banked either: the record stays in raw_archive (unpruned) and the
+// band must NOT be ACKed to trim it. Returning 0 here is correct and
+// intentional — do not add a v25 R-R path.
 return 0;
Suggestion importance[1-10]: 2

__

Why: This suggestion only adds a comment to the existing_code without changing any logic. The improved_code is functionally identical to the existing_code, making this a documentation-only suggestion with minimal impact.

Low
Suggestions up to commit 1ab38b0
CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent absent-night note on present readiness value

When readinessScalar == null && showOvernight is true but overnightNote is non-null
(which cannot happen given the current logic, but the structure is fragile), the
fallback to kUnknownAbsenceNote fabricates a reason. More critically, when
showOvernight is false, overnightNote is needInputNote('scored_night') — but the
ternary condition readinessScalar == null && showOvernight is false, so the else
branch returns overnightNote regardless of whether readinessScalar is non-null. This
means a present readiness scalar will still carry the scored_night note, violating
the rule in _scalarMetric that a note on a present value is an explanation of an
absence that did not happen. The fix is to only attach overnightNote when
readinessScalar == null.

lib/data/local_repository_impl.dart [351-354]

-final readinessNote = readinessScalar == null && showOvernight
-    ? _needNote(sleepBundle, 'clinical.readiness_composite') ??
-          kUnknownAbsenceNote
-    : overnightNote;
+final readinessNote = readinessScalar == null
+    ? (showOvernight
+        ? _needNote(sleepBundle, 'clinical.readiness_composite') ??
+              kUnknownAbsenceNote
+        : overnightNote)
+    : null;
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that when showOvernight is false and readinessScalar is non-null, overnightNote (a non-null string) would be attached to a present value, violating the _scalarMetric contract. However, _scalarMetric already drops the note when v != null (per the PR change at line 188), so the practical impact is mitigated, but the logic is still semantically wrong and could cause issues if _scalarMetric behavior changes.

Medium
Fix null-safety crash when decoded HR field is null

The comment in _createDecodedStore explicitly states that hr == 0 is the off-skin
sentinel and that NULL should be written for a record that has no heart-rate field.
However, decoded.hr could be null itself (the Sample field is nullable after v43),
and calling .hr > 0 on a nullable int will throw a null check error at runtime when
decoded.hr is null. The null-safe comparison (decoded.hr ?? 0) > 0 should be used
instead.

lib/data/db.dart [3899]

-'hr': decoded.hr > 0 ? decoded.hr : null,
+'hr': (decoded.hr ?? 0) > 0 ? decoded.hr : null,
Suggestion importance[1-10]: 6

__

Why: If decoded.hr is nullable (as the PR's Sample model implies after v43), then decoded.hr > 0 would throw a null check error at runtime. Using (decoded.hr ?? 0) > 0 is the correct null-safe comparison and prevents a potential crash on the hottest write path.

Low
Ensure absent metrics always carry an explanation note

_absentMetric returns null when _absentNote returns null, and the null-aware spread
?_absentMetric(...) silently drops the entry. This means a figure that is absent but
has no named gate (e.g. max_hr_used on a day with no activity) produces no entry in
absent at all — the screen gets neither a value nor an absence envelope, which is
the same "bare dash with no explanation" the comment above says this block exists to
fix. When the note is null the entry should still be written with
kUnknownAbsenceNote so the screen always has something to render.

lib/data/local_repository_impl.dart [1536-1549]

 'absent': <String, Map<String, dynamic>>{
   for (final e in <String, (bool, String)>{
     'strain': (strain == null, 'strain'),
     'training_load': (trimp == null, 'trimp'),
     'calories': (calories == null, 'calories'),
     'calories_total': (caloriesTotal == null, 'calories_total'),
     'zones': (zoneMin.values.every((v) => v == null), 'zones'),
     'max_hr_used': (maxHrUsed == null, 'max_hr_used'),
   }.entries)
     if (e.value.$1)
-      e.key: ?_absentMetric(_absentNote(b, e.value.$2), 'ESTIMATE'),
+      e.key: _absentMetric(
+              _absentNote(b, e.value.$2) ?? kUnknownAbsenceNote,
+              'ESTIMATE')!,
 },
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that ?_absentMetric(...) silently drops entries when _absentNote returns null, leaving absent figures with no envelope in the absent map. Using kUnknownAbsenceNote as a fallback ensures the screen always has an absence explanation to render, which aligns with the stated goal of the block.

Low
Notification plain-slot burn bypasses NotificationCenter

When a medical finding fires, the code burns the plain '$date:exception' slot by
recording it in FiredKeyStore. However, the comment above says "a presented medical
exception burns the day's plain slot as well", but the emit() call uses
'$date:exception:medical' as the dedupeKey — the plain '$date:exception' key is
never passed to NotificationCenter.emit, so it is never checked by the deduplication
guard before this recordFired call. A subsequent plain (non-medical) finding on the
same day will still pass the emit deduplication check (since '$date:exception' was
never emitted, only recorded), meaning the plain notification fires anyway. The
plain slot should be burned by emitting (or at minimum checking) via
NotificationCenter.instance rather than writing directly to FiredKeyStore, which
bypasses the center's own lock and TOCTOU guard.

lib/compute/derivation_engine.dart [4242-4244]

     if (medical && fired) {
-      await const FiredKeyStore().recordFired('$date:exception');
+      // Burn the plain slot through the center so its lock and dedup guard
+      // are both exercised — writing directly to FiredKeyStore bypasses both.
+      await NotificationCenter.instance.emit(
+        NotificationEvent(
+          dedupeKey: '$date:exception',
+          category: NotifCategory.health,
+          priority: NotifPriority.critical,
+      title: findings.first.title,
+          body: findings.first.detail,
+          date: date,
+          route: '/heart',
+        ),
+      );
     }
Suggestion importance[1-10]: 5

__

Why: The concern about bypassing NotificationCenter's dedup guard is valid in principle, but the improved_code is incomplete and incorrect — it uses findings.first.title/detail unconditionally even in the multi-finding case, and the indentation/structure is broken. The logic intent is reasonable but the implementation provided is not directly applicable.

Low
Archive undecodable non-R10-lite records instead of silently dropping them

When decoded == null and the record is NOT a Gen4 R10-lite with beats, the function
silently returns 0 without persisting anything. However, the comment above states
the record "commits as decoded, so it is never archived, and the band is then acked
to trim it." This means non-R10-lite records that fail decoding (e.g. v25 records
explicitly returned as null) are silently dropped without being archived, violating
the invariant that undecodable records go to raw_archive. The early return should
only apply to the R10-lite beat-only path; other null-decoded records need to be
routed to the archive batch instead of silently discarded.

lib/data/db.dart [3862-3879]

 if (sample != null &&
     sample.rrIntervalsMs.isNotEmpty &&
     _isGen4R10LiteHistorical(raw.hex)) {
   return _queueRrBeats(
     batch,
     _recTsFrom(raw, sample),
     sample,
     deviceFamily: deviceFamily,
   );
 }
+// Non-R10-lite records that decode to null (e.g. v25) must go to
+// raw_archive so they are not silently lost before the band ACK trims them.
+if (!_isGen4R10LiteHistorical(raw.hex)) {
+  batch.insert(
+    'raw_archive',
+    {
+      'hex': raw.hex,
+      'counter': raw.counter,
+      'captured_at': DateTime.now().millisecondsSinceEpoch,
+      'reason': 'decode_null',
+    },
+    conflictAlgorithm: ConflictAlgorithm.ignore,
+  );
+  return 1;
+}
 return 0;
Suggestion importance[1-10]: 2

__

Why: The suggestion misreads the code flow. The _queueDecodedOneHz function is called from commitSyncBatch, which handles archiving separately — v25 records are explicitly returned as null from _decodeOneHzSample and the archiving decision is made upstream. The raw_archive schema and insert parameters in the improved code are also fabricated and don't match the actual table structure in the PR.

Low
General
Cross-day rollup races concurrent derive pass after import

finalizeImport calls _refreshBaselines() first, and _refreshBaselines internally
acquires _withRunLock. Then _runCrossDay and the notification pass run outside the
lock. Meanwhile run() also uses _running to guard the same day_result writes. If a
background run() starts between _refreshBaselines returning and _runCrossDay
executing, the cross-day artifact and notifications are written concurrently with an
active derive pass. The comment says "last write wins and the artifact is
version/day stamped" but the derive pass inside run() also calls _runCrossDay and
_runNotifications, so two concurrent cross-day encodes race to putBaseline. Wrapping
the whole finalizeImport body (or at least the rollup+notification steps) under
_withRunLock — with _refreshBaselines called before acquiring it, as it does today —
would close this window without deadlocking.

lib/compute/derivation_engine.dart [2972-2974]

-    static Future<T> _withRunLock<T>(T busy, Future<T> Function() body) async {
-      if (_running) return busy;
-      _running = true;
-      try {
-        return await body();
-      } finally {
-        _running = false;
-      }
+    Future<void> finalizeImport(Profile profile) async {
+      // Baselines take the lock themselves; call before acquiring it here.
+      await _refreshBaselines();
+      await _withRunLock(null, () async {
+        await _runCrossDay(profile);
+        await _runNotifications();
+      });
     }
-  ...
-  Future<void> finalizeImport(Profile profile) async {
-    await _refreshBaselines();
-    await _runCrossDay(profile);
Suggestion importance[1-10]: 4

__

Why: The race condition between finalizeImport and a concurrent run() is a real concern, but the PR's comment explicitly acknowledges this design choice ("last write wins, and the artifact is version/day stamped"). The improved_code also omits _runNotifications() from the original, making it incomplete.

Low
Guard against recursive rebuild loop before salvage

_db is set to fresh before the salvage runs so that _mergeFromDbFile's own instance
call resolves to the new database. However, if _mergeFromDbFile itself calls
instance and instance checks db.isOpen, a partially-initialized fresh handle that is
not yet open would cause instance to re-enter _openOrRebuild, creating a recursive
rebuild loop. The assignment _db = fresh should be guarded to only proceed after
confirming fresh.isOpen, or the salvage should use the fresh handle directly rather
than going through instance.

lib/data/db.dart [171-195]

-static Future<Database> _openOrRebuild() async {
-  try {
-    return await _open();
-  } catch (e) {
-    ...
-    _db = fresh;
-    var salvaged = const <String, int>{};
-    try {
-      salvaged = Map.of(
-        await _mergeFromDbFile(
-          quarantine,
-          only: _salvageTables,
-          tolerant: true,
-        ),
-      )..remove('_days');
-    } catch (_) {
-      // The quarantined file gave us nothing.
-    }
-    lastRebuild = (
-      cause: '$e',
-      quarantinePath: quarantine,
-      salvaged: salvaged,
-    );
-    return fresh;
-  }
+_db = fresh;
+assert(fresh.isOpen, '_openOrRebuild: fresh database is not open before salvage');
+var salvaged = const <String, int>{};
+try {
+  salvaged = Map.of(
+    await _mergeFromDbFile(
+      quarantine,
+      only: _salvageTables,
+      tolerant: true,
+    ),
+  )..remove('_days');
+} catch (_) {
+  // The quarantined file gave us nothing. The app still opens, and the
+  // file is still there — that is the whole point of not deleting it.
 }
+lastRebuild = (
+  cause: '$e',
+  quarantinePath: quarantine,
+  salvaged: salvaged,
+);
+return fresh;
Suggestion importance[1-10]: 3

__

Why: The concern about recursive re-entry is partially valid, but the code already guards against it: _db = fresh is set before _mergeFromDbFile is called, and instance checks db != null && db.isOpen — if fresh is open (which it must be since _open() succeeded), the guard works correctly. Adding an assert is a minor defensive improvement but doesn't fix a real bug.

Low
UTC day label used where local hour is required

DateTime.fromMillisecondsSinceEpoch returns a local-time DateTime only when the
isUtc parameter is omitted (it defaults to false), which is correct here. However, t
is an epoch-second value multiplied by 1000 to get milliseconds — if t is ever
stored as milliseconds already (e.g. a future schema change or a caller passing ms
directly), the hour computation silently wraps. More critically, the guard h < 0 ||
h > 23 can never be true for .hour (it is always 0–23), so it provides no real
protection. The real risk is v <= 0 passing a zero HR through: the comment says
"never an imputed value" but a stored 0 bpm (off-wrist second) would be excluded by
v <= 0, which is correct — confirm v == 0 is excluded, not just v < 0.

lib/compute/derivation_engine.dart [4350-4351]

-    static List<double?> hourlyHrProfile(Object? hrCurve, {int minMinutes = 5}) {
-    ...
-        final h = DateTime.fromMillisecondsSinceEpoch(t * 1000).hour;
-        if (h < 0 || h > 23) continue;
+    final h = DateTime.fromMillisecondsSinceEpoch(t * 1000).hour;
+    // h is always 0–23 from DateTime.hour; the bounds check is a no-op but
+    // kept as a compile-time reminder that the index is safe.
+    assert(h >= 0 && h <= 23);
+    if (v <= 0) continue; // already guarded above, but make intent explicit
+    sums[h] += v;
+    counts[h] += 1;
Suggestion importance[1-10]: 2

__

Why: The suggestion's title is misleading — the code already correctly uses local time via DateTime.fromMillisecondsSinceEpoch without isUtc: true. The h < 0 || h > 23 being a no-op is a minor style issue, and the improved_code doesn't meaningfully improve correctness.

Low
Use trailing-window helper for baseline depth count

LocalDb.metricSeries is ORDER BY date ASC LIMIT n and returns the oldest n rows,
not a trailing window. Per AGENTS.md §4.2, the correct helper for a trailing count
is trailingSeriesValues('skin_temp_adc', n). Using metricSeries here returns the
total count of all stored rows (no limit is passed), which is correct for a count
but the AGENTS.md footgun note flags metricSeries as returning oldest-n — if a limit
were ever added this would silently report the wrong baseline depth. Use
trailingSeriesValues to be consistent with the codebase's own convention and avoid a
future regression.

lib/data/local_repository_impl.dart [739-740]

-final have = (await LocalDb.metricSeries('skin_temp_adc')).length;
+final have = (await LocalDb.trailingSeriesValues('skin_temp_adc', 90)).length;
 return {'value': null, 'note': 'need_baseline:have=$have,need=3'};
Suggestion importance[1-10]: 2

__

Why: The suggestion references LocalDb.trailingSeriesValues which is not shown anywhere in the PR diff, making this speculative. The existing metricSeries call without a limit correctly returns all rows for counting purposes, so this is a low-impact stylistic concern at best.

Low
Suggestions up to commit 2125dc4
CategorySuggestion                                                                                                                                    Impact
Possible issue
Reset running flag in finally to prevent permanent latch

The _running = false reset at the end of run() is outside a finally block. If
_runStorageHousekeeping(), the strain rescale, or any other statement between the
last await and this reset throws, _running stays true permanently — the same
sticky-latch pattern the repo's own commit history calls out as a recurring bug. The
reset must be in a finally block (or use _withRunLock for the whole body of run()).

lib/compute/derivation_engine.dart [1918-1919]

-_running = false;
+} finally {
+  _running = false;
+}
 final finishedAt = DateTime.now().millisecondsSinceEpoch;
Suggestion importance[1-10]: 7

__

Why: This is a valid and important concern — if any statement between the last await and _running = false throws, the flag stays true permanently, matching the exact bug pattern the codebase's own history warns about. Wrapping in finally is the correct fix.

Medium
Null-safe HR sentinel comparison in decoded insert

If Sample.hr is nullable (int?), the expression decoded.hr > 0 will throw a
null-dereference at runtime for any record where hr is null. Use a null-safe
comparison to correctly map both null and 0 to a null DB value, matching the stated
intent that absence is NULL.

lib/data/db.dart [3899]

-'hr': decoded.hr > 0 ? decoded.hr : null,
+'hr': (decoded.hr ?? 0) > 0 ? decoded.hr : null,
Suggestion importance[1-10]: 6

__

Why: If Sample.hr is a nullable int?, then decoded.hr > 0 would throw a null pointer exception at runtime. The fix (decoded.hr ?? 0) > 0 ? decoded.hr : null correctly handles the null case and matches the stated intent of mapping both null and 0 to a null DB value. The score is moderate because the actual nullability of Sample.hr depends on the outer codebase definition.

Low
Absent metric fabricates a reason instead of abstaining

When showOvernight is true but readinessScalar == null, the code falls back to
kUnknownAbsenceNote — fabricating a confident "unknown" reason even when the
pipeline's own note is absent. This violates the never-fabricate rule: if neither
the pipeline note nor an overnight gate explains the absence, the note should be
null (rendered as "—") rather than a synthetic string. Use null as the final
fallback instead of kUnknownAbsenceNote.

lib/data/local_repository_impl.dart [351-354]

 final readinessNote = readinessScalar == null && showOvernight
-    ? _needNote(sleepBundle, 'clinical.readiness_composite') ??
-          kUnknownAbsenceNote
+    ? _needNote(sleepBundle, 'clinical.readiness_composite')
     : overnightNote;
Suggestion importance[1-10]: 4

__

Why: The suggestion argues that kUnknownAbsenceNote fabricates a reason, but the PR's own comments explicitly state "The floor is 'we do not know', never a plausible guess" and kUnknownAbsenceNote is designed as the honest "unknown" sentinel. The PR intentionally uses it as a fallback when no specific reason is available, so removing it would leave the absence unexplained rather than honestly labeled as unknown.

Low
Activity absence fabricates a reason instead of abstaining

activityNote always returns a non-null string, but _scalarMetric now only attaches a
note when the value is null (if (v == null) 'note': ?note). When a value IS present,
the note is silently dropped — which is correct — but when the value is absent,
kUnknownAbsenceNote is fabricated even when neither bundle nor wake-features has a
real reason. Per the never-fabricate rule, return null in those cases so the UI
renders "—" rather than a synthetic explanation.

lib/data/local_repository_impl.dart [373-382]

-String activityNote(String key) {
+String? activityNote(String key) {
   if (activityBundle == null && wakeFeatures == null) {
     return needInputNote('today_activity');
   }
   if (activityBundle != null) {
-    return _absentNote(activityBundle, key) ?? kUnknownAbsenceNote;
+    return _absentNote(activityBundle, key);
   }
   final n = (wakeFeatures?['absent_notes'] as Map?)?[key];
-  return n is String && n.isNotEmpty ? n : kUnknownAbsenceNote;
+  return n is String && n.isNotEmpty ? n : null;
 }
Suggestion importance[1-10]: 4

__

Why: Similar to suggestion 1, the PR deliberately uses kUnknownAbsenceNote as the "we do not know" floor rather than fabricating a plausible reason. The PR comments explicitly state this design intent. Returning null instead would leave absent metrics with no explanation at all, which the PR considers worse than an honest "unknown" note.

Low
Beats lost on non-R10-lite null-decoded records

When decoded is null for a non-R10-lite record (e.g. a v25 record that is explicitly
rejected by the histVersion == 25 guard), the function returns 0 and the record is
silently dropped with no beats persisted. However, the R10-lite path only fires when
_isGen4R10LiteHistorical is true AND a sample with RR intervals exists. Any other
null-decoded record with RR beats (e.g. a gen5 frame that _decodeOneHzSample could
not parse but whose sample carries beats) will lose those beats permanently — the
record commits as decoded, the band is ACKed, and the beats are gone. The guard
should also cover the case where decoded is null, a sample with beats exists, and
the record is NOT R10-lite, by at minimum not treating it as "decoded" for ACK
purposes, or by persisting the beats when safe.

lib/data/db.dart [3862-3879]

-if (sample != null &&
-    sample.rrIntervalsMs.isNotEmpty &&
-    _isGen4R10LiteHistorical(raw.hex)) {
+if (sample != null && sample.rrIntervalsMs.isNotEmpty) {
+  if (_isGen4R10LiteHistorical(raw.hex)) {
+    return _queueRrBeats(
+      batch,
+      _recTsFrom(raw, sample),
+      sample,
+      deviceFamily: deviceFamily,
+    );
+  }
+  // For other null-decoded records that carry beats (e.g. a rejected v25
+  // frame whose sample was pre-decoded by the live drain), persist the
+  // beats so the ACK does not permanently discard them.
   return _queueRrBeats(
     batch,
     _recTsFrom(raw, sample),
     sample,
     deviceFamily: deviceFamily,
   );
 }
 return 0;
Suggestion importance[1-10]: 3

__

Why: The suggestion raises a theoretical concern about non-R10-lite null-decoded records with RR beats being lost, but the PR code already handles the primary cases: v25 records are explicitly rejected before reaching _queueDecodedOneHz (returning null from _decodeOneHzSample), and the R10-lite path is the documented case where beats exist without a decodable 1 Hz record. The improved_code collapses the R10-lite check and a generic fallback into identical behavior, which doesn't actually address the concern differently and could persist beats from genuinely undecodable frames.

Low
Burn plain dedupe slot even when medical emit was suppressed

When a medical finding fires and burns the plain '$date:exception' slot, the code
correctly records the plain key. However, if fired is false (e.g. the medical key
was already claimed by quiet hours or a prior emit), the plain slot is never burned,
so a subsequent non-medical finding on the same day can still fire a second
notification — the exact de-escalation double-buzz the comment above describes
preventing. The plain key should be burned whenever the medical key was already
previously fired, not only when it fires now.

lib/compute/derivation_engine.dart [4242-4244]

-if (medical && fired) {
-  await const FiredKeyStore().recordFired('$date:exception');
+if (medical) {
+  if (fired) {
+    await const FiredKeyStore().recordFired('$date:exception');
+  } else {
+    // Medical key was already claimed (quiet hours or prior emit).
+    // Burn the plain slot anyway so a de-escalated re-derive cannot
+    // fire a second notification about findings the morning already covered.
+    await const FiredKeyStore().recordFired('$date:exception');
+  }
 }
Suggestion importance[1-10]: 3

__

Why: The 'improved_code' is functionally identical to the 'existing_code' — both branches execute the same recordFired call unconditionally when medical is true, so the sugge...

pub get with pubspec_overrides.yaml sitting there rewrote the tracked
lock to ../protocol and ../analytics, so ci died at the pin guard before
it ran a single test. same thing the guard's own comment says happened
twice in july. pins themselves were already right.

coderabbit refused the pr outright — 436 files against a 100 limit — so
.coderabbit.yaml drops test/, the platform dirs and assets/ and points it
at the ~230 files where a bug can actually be.

also the pin comments still said UNPUSHED and said the analytics audit
wave was uncommitted. both stopped being true last night.
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit d03e4f9

it's the one sub-tab that's irrelevant rather than empty for most
people, and an empty tab that can never fill reads like a feature you
failed to use. profile > preferences > cycle tracking. off hides the tab
and skips its query, keeps everything already logged.
ci went red on every golden after the pngs came out of the repo. skip
with a reason instead of an --exclude-tags flag, so the run says out
loud nobody checked the pixels. the 61 non-golden tests in the activity
file keep running, which a file-level tag would have killed.
zz_scratch_render_test.dart says 'deleted before commit' on line one and
then got committed. analyze runs --fatal-infos so the two infos were the
whole reason the test job was red — it never reached a test.
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 2125dc4

runHeadlessSync self-acquires its BandOwnership lease when the three
iOS gate entry points call it with no lease argument, so only that
call frame ever holds the token. If the run truly wedges rather than
just running long, HeadlessSyncGate's timeout hands the gate back to
the next wake but leaves the band lease itself stuck on an orphaned
future that never runs its own release. Every later headless wake
then silently no-ops, and a foreground connect spins forever in
acquireForeground's wait loop with nothing left to complete it.

Force-clear the headless owner from the gate's own timeout handler,
matching what the boot path already did explicitly for the lease it
holds directly.
The pin was still sitting on the PR branch head. PR #46 merged it to
main as bfea5e5, two commits ahead of the old pin — both test-only
deprecation-ignore annotations, lib/ untouched, so no kAlgoVersion
bump needed for this move.
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 1ab38b0

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 85d6490

- imported age was ~56 for everyone: healthkit sends
  timeIntervalSince1970 (seconds), we read it as ms. the test was
  feeding ms too, so it agreed with the bug.
- cancelling the passphrase prompt left the whole db decrypted in temp.
  the shred is in a finally on a try that throw never enters.
- our own .db.gz backup wasn't restorable. fixed in importEdgeBackup off
  the gzip magic bytes so settings restore gets it too, not just
  onboarding.
- add_medication replaced the whole row — other dose times, dose, kind,
  note all gone. merges now.
- water couldn't be stepped back to blank, the spread kept rewriting it.
- ollama/lm studio could never finish configuring: configured wanted an
  api key. local endpoints don't need one, public ones still do.
both merged. no kAlgoVersion bump — the analytics hop is two comment
lines in tests, the protocol one adds rr_ms to a decodeFrame branch
nothing here reads. no number moves.
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit aa7f80e

every face wrote a mood and none of them could write absence, so the
only way out of a mood you never meant to log was picking a different
wrong one. tap the selected face again and it's not-answered, same as
the stepper stepping down off zero. the writer already handled null.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant