Conversation
ryanbr
left a comment
There was a problem hiding this comment.
Thanks @pipiche38. The schema work is exactly right, and that is the part of a change like this that usually goes wrong. Every pin agrees: roomVersion 41 in @Database and SCHEMA_VERSION, MIGRATION_40_41 both defined and registered in ALL_MIGRATIONS, roomSchemas/41.json added, GRDB v47-oura-met-sample, the new table described "platform": "both", and the two schema_oracle.json copies byte-identical at 15bb95c6c200708e59ab2e08e85a61d4b0320660e433cde17cd606f19f36ffe0, which the oracle's own readme requires.
Withholding the estimate below the coverage floor instead of falling back to the HR path, and judging today's coverage against elapsed hours rather than 24 h, are both the right calls.
One blocker.
The parity ledger fails
parity-governance only runs when Tools/ is touched, so CI stays green on this. main is clean, so these are the branch's:
add-unpaired-function: android/app/src/main/java/com/noop/data/WhoopDao.kt::ouraMetSamples/4#1
add-unpaired-function: android/app/src/main/java/com/noop/data/WhoopDao.kt::countOuraMetFor/1#1
add-unpaired-function: android/app/src/main/java/com/noop/data/WhoopRepository.kt::insertOuraMetSamples/1#1
add-unpaired-function: android/app/src/main/java/com/noop/data/WhoopRepository.kt::ouraMetSamples/4#1
com/noop/data is inside the inventory globs and Packages/WhoopStore/Sources is too, so the Swift side in OuraMetStore.swift should pair with these rather than sit beside them. If the names and arities already correspond, a Swift twin: reference on each Kotlin declaration resolves it; where they genuinely differ in shape, a disposition is the honest answer.
This is the third PR this week to hit that gate with CI green, so the gate is the problem as much as any one branch. I will look at making it visible on PRs that do not touch Tools/.
I have not finished reading the MET arithmetic itself. The schema, the toggle default and the withholding rule are reviewed; the energy maths and its oracle are not yet, and I will come back to those.
e042637 to
eaa4c7c
Compare
ryanbr
left a comment
There was a problem hiding this comment.
Coming back with the MET arithmetic I said I had not read. It holds up, and the two places it could have gone badly wrong are both handled deliberately.
The one that would have been serious
activeKcalEst = met.totalKcal reads like a category error: the estimate carries restingKcal, activeKcal and totalKcal separately, and the field is named activeKcalEst. Assigning a total to it would inflate every ring day by a whole resting day the moment the toggle went on, which is a change nobody would attribute to a calories toggle.
It is correct. estimateDayCalories is documented as a "Backward-compatible total-kcal facade for the stored daily metric" and returns estimateDayEnergy(...).totalKcal, so the HR path already puts a TOTAL in that field. activeKcalEst is a pre-existing misnomer and your MetEnergyEstimate doc says it matches the split "so the persisted totalKcal keeps its meaning". Both paths also build their active share the same way, as the excess over resting rather than the gross rate, so the two agree in structure and not only in units. Worth the paragraph because the name invites the wrong conclusion and the next reader deserves to find the answer written down.
The constant
0.0175 is arithmetic, not a fit, and it checks: 3.5 ml O₂·kg⁻¹·min⁻¹ at 5 kcal per litre is 0.0035 L × 5 = 0.0175 kcal·kg⁻¹·min⁻¹ exactly. 1.5 MET is Oura's own documented floor rather than a tuned knob. r = 1.000 at 0.6 kcal/day RMSE over 75 days is what you would expect if the rule IS their rule, which is a much stronger claim than a good fit, and your wording keeps the distinction.
The twin-skip is the other thing I checked: (s.ts - lastStart) * 2 < min(s.secPerSample, lastPeriod), integer, with ties broken to the LOWER reading, so a minute served twice by a re-drain cannot count twice in either coverage or energy. Withholding below 50% coverage instead of substituting the HR path is right, and scoping today's coverage to elapsed hours rather than 24 h is the detail that stops it withholding every morning.
What is left
The branch is dirty against main now, and that one is mine: 44ef71ebe repaired the parity authority, which touches parity_twin_map.json and parity_ledger_baseline.json, so your fifth commit collides with it. The governance suite is 49 of 49 on main again, and the drift was the ledger's own "debt decreased; baseline cleanup is optional" line that the suite asserts is empty.
Rebase onto 44ef71ebe and re-run --refresh-derived. Do not hand-merge parity_twin_map.json: it is generated, and a hand merge produces a file that matches neither tree. Resolve it to main's copy and regenerate. I did exactly that on #2293 half an hour ago and its parity-governance check is green, so the path is known to work. Say the word if you would rather I pushed the rebase for you.
…e, no fitted constant
Adds Calories.estimateDayEnergyFromMET (Swift) / estimateDayEnergyFromMet (Kotlin):
active = Σ_{met ≥ 1.5} (met − 1.5) × 0.0175 kcal·kg⁻¹·min⁻¹ × weight over the day's samples
— Oura's support wording ("the portion that exceeds 1.5 MET") at the standard MET→kcal
definition (1 MET = 3.5 ml O₂·kg⁻¹·min⁻¹). Resting is the same revised Harris–Benedict the
HR path uses, scaled by covered seconds; coverage is reported over the window the caller
passes so today's partial day is judged against elapsed hours. Missing minutes are unknown —
never extrapolated to activity, never banked as rest.
The rule was identified, not fitted: on the Oura export's own per-minute MET series it
reproduces active_calories with r = 1.0000 and 0.6 kcal/day RMSE over 75 current-era days
(r 0.9999 / 3 kcal over 396 pre-2025 days), zero intercept, the only input being the
wearer's weight.
A re-served minute counts once. The ring's timestamps go through a per-session 0x13
anchor, so a record served again under a second session lands 2–5 s off its first copy,
while the ring's own minute grid steps back a second between records (a real successor
starts 59–61 s on). Half a period tells them apart: two samples are the same minute when
|Δ| × 2 < min(epochS), and the walk compares against the last counted start.
Kotlin is pinned to Swift by oracle: the real Calories enum compiled standalone over a
case spread (84 output lines) (four profiles × empty / rest floor / bout / 1.5 threshold edge / decoder
boundary / partial coverage / 120 s cadence / duplicate ts / 3-s twins / 59-s phase steps /
window edges / partial today / DST 23 h / clamp / bad epochs / realistic day), stdout pasted
verbatim into MetCaloriesOracleTest. MetCaloriesTests pins the Swift side with hand-derived
literals.
Refs ryanbr#2242
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RgqTwKfXLXzKo81ECb4aDz
…ample, both platforms Until now the 0x50 MET stream existed only as the diagnostic JSONL sidecar, so nothing scored could read it. GRDB v47-oura-met-sample / Room MIGRATION_40_41 (schema 41) add ouraMetSample(deviceId, ts, met REAL, state, epochS) keyed (deviceId, ts), column order identical on both sides and pinned in schema_oracle.json (both copies). epochS is carried per row so a cadence other than 60 s scales the estimate rather than skewing it. Insert drops a re-served twin: the (deviceId, ts) key alone does not catch a record served again under another session anchor (it lands a few seconds off), so the insert reads the stored rows around the batch and drops any incoming sample within half a period of a stored one or of one accepted earlier in the batch (earlier start wins, lower MET on an exact tie). The pure rule is OuraMetSample.droppingTwins / OuraMetSampleEntity.droppingTwins, same integer test as the estimator, testable without a database. A 59-s phase-step successor is kept. Swift: OuraMetStore (insert, bounded range read, count), the table on DeviceRegistryStore.deviceScopedTables so forgetting a ring clears it. Android: entity, DAO insert/read/delete, DeviceRegistry.deleteDeviceData wiring, repository wrappers, the committed v41 Room schema for the upgrade test. Tests: OuraMetStoreTests (Swift), OuraMetSampleMigrationTest (Kotlin, incl. the twin rule); SchemaOracleTest / WhoopDatabaseUpgradeTest / the device-scoped-table guards on both sides. Refs ryanbr#2242 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgqTwKfXLXzKo81ECb4aDz
…es when supplied analyzeDay gains dayMet (the day owner's own per-minute MET samples), dayMetNow (so today's coverage is judged against elapsed hours, not 24 h) and a caloriesDiag sink. Rule: MET samples present ⇒ activeKcalEst = estimateDayEnergyFromMET(...).totalKcal when the stream covers ≥ 50 % of the window; below that the estimate is withheld (nil) rather than substituted by the HR path, which on a ring day runs over sparse banked HR and does not track Oura's number (r ≈ −0.1). nil/empty dayMet keeps the HR path byte-identical — every WHOOP and pure-function caller is unchanged. One always-on log line per MET day. Both platforms; AnalyticsEngineMetCaloriesTests / AnalyticsEngineMetCaloriesTest pin the selection, the withhold, the elapsed-hours window and the byte-identical no-MET path. Refs ryanbr#2242 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgqTwKfXLXzKo81ECb4aDz
… an Experimental toggle
The toggle (iOS AppModel.ouraMetCaloriesKey / Android NoopPrefs.KEY_OURA_MET_CALORIES,
default OFF) gates:
• the writer — OuraLiveSource fans each anchored 0x50 record out to ouraMetSample, one
row per minute, next to the existing JSONL sidecar call. The record's timestamp is the
END of its last sample: fitted against Oura's own per-minute export, every record length
n matched best at exactly −n minutes (85 % exact minute matches, r 0.90, vs 23 % / 0.57
read forward), so sample i starts at utc − (n − i)·60 and a record straddling local
midnight lands its minutes on the right days.
• the read — IntelligenceEngine reads the day owner's rows (registry active id, calendar-
day scoped like dayHr) and hands them to analyzeDay as dayMet, with dayMetNow = now; the
calories line goes through the same per-day diag recorder as the Effort funnel.
• the day-cycle fold — DayCycleIntelligenceIntegration (and Android's
PhysiologicalStepCycleEngine) recomputed Keytel for every cycle and wrote it over
activeKcalEst, which would have hidden the MET decision on any phone with a day-cycle
history. The fold now takes the same decision analyzeDay takes, over its own window
[onset, min(endExclusive, now)): covered → the MET total, thin → withheld with no HR
substitute, no rows or toggle off → Keytel byte for byte. A "stepsCycle calories …
path=met" trace line names the decision.
The toggle joins the day-cache config signature on both platforms so a flip re-scores every
cached day. Android threads the flag Context-free like spo2CandidateDisplay, carries it into
the pass on a field and reads the MET rows inside readDaySkinAndWristOff, and hands the fold
a Boolean rather than a lambda — all to stay inside analyzeRecentOnCpu's JaCoCo budget (a
new suspend call there spills ~0.7 K instructions of continuation state).
Settings gains "Experimental · Oura Calories" (iOS card / Android row), shown only with an
Oura ring, caption stating the method, the < 50 % coverage withhold, that Oura re-scores a
logged workout's minutes by activity type (NOOP does not, so such a day reads lower), and that
turning it on starts storing MET samples. Strings in de/es/fr/pt-PT plus it/pl/ru/zh-Hans/
zh-Hant (iOS) and pl/ru/zh (Android). Docs: OURA_PROTOCOL.md §6.13 (day-sum validation of
the 0x50 decode, end-of-record timestamps, the formula); README Oura table "Active calories".
OFF keeps both the DB and every score byte-identical. Tests: DayCacheConfigFieldTests /
DayCacheConfigFieldTest; DayCycleRecoveryTests (Swift, xcodebuild test) and
DayCycleMetCaloriesTest (Kotlin, Robolectric + Room) on a seeded 8-h main sleep: covered
cycle carries the MET total, 10 %-covered is withheld, no reader / empty table = Keytel.
Refs ryanbr#2242
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RgqTwKfXLXzKo81ECb4aDz
Review follow-up. The parity ledger flagged the MET store's Kotlin reads and writes as one-sided; run in full it also flagged the MET toggle's arity changes to existing functions, which read as new identities. Paired, with twin claims where the ledger reads them: - `WhoopRepository.insertOuraMetSamples` <-> `WhoopStore.insertOuraMetSamples` - `WhoopRepository.ouraMetSamples` <-> `WhoopStore.ouraMetSamples` (on the Repository, since the DAO's `@Query` puts its declaration outside the claim's attach window) - `AnalyticsEngine.analyzeDay`, `IntelligenceEngine.analyzeRecent` and `DayCycleIntelligenceIntegration.compute`, whose parameter lists grew here Removed: `WhoopStore.ouraMetSampleCount` and `WhoopDao.countOuraMetFor`. Neither had a production caller (the Kotlin one had no caller at all); the store tests count through `ouraMetSamples` instead. `platform_specific` dispositions for what is Kotlin-only by construction: the Room `@Insert` and `@Query` primitives under the paired Repository functions, the per-table forget-device delete (Swift clears the table through the `DeviceRegistryStore` name list), the Room migration override (its GRDB twin is the `v47-oura-met-sample` closure, pinned by the shared schema oracle), and the two JVM 64 KiB extractions `readDaySkinAndWristOff` and `PhysiologicalStepCycleEngine.compute`. The authority is re-derived with `--migrate-authority` because `main` at 5783c49 no longer reproduces its stored one. Verified: `parity_ledger.py` OK; `parity_ratchet.py --migrate-authority` 0 errors; `test_parity_*` 180 OK; StrandAnalytics 2068/0; WhoopStore 617/0; macOS `StrandTests` (DayCacheConfigField, DayCycleRecovery) 17/0; `NOOPiOS` builds; Android 6402 tests, the 8 failures reproduce identically on `upstream/main`; doc lint and `i18n_audit --ci` clean. Refs ryanbr#2242 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01484Ef871BnAuniyDJYYHoV
eaa4c7c to
9d30eb4
Compare
…otal Review follow-up. `activeKcalEst = met.totalKcal` reads like a category error: the estimate carries the active share separately. It is correct, because the field is a pre-existing misnomer and the HR path's `estimateDayCalories` already stores `estimateDayEnergy(...).totalKcal` in it. That is now written at both `analyzeDay` sites, so the next reader does not have to work it out again. Comment only. Refs ryanbr#2242 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01484Ef871BnAuniyDJYYHoV
|
Thanks @ryanbr, for the arithmetic read and for pushing the rebase. I had done the same thing locally (resolved to
The ledger, from your first review. The full run on this branch listed more than the four you pasted: the toggle adds parameters to existing unpaired functions, and each new arity counts as a new identity. Here is how each one was handled. Paired with twin claims:
Removed:
One leftover: the message of Verified on the tree as pushed: |
ryanbr
left a comment
There was a problem hiding this comment.
Thanks @pipiche38. I checked the three things you said you did rather than taking them on trust, and all three hold: 58ded8940 is comment-only (4 insertions, every one a comment line), ouraMetSampleCount and countOuraMetFor are genuinely gone, and the twin claims are where you describe, including ouraMetSamples sitting on the Repository function because the DAO's @Query pushes the declaration past the claim window. The ledger account is the most complete one anybody has written on a PR here.
You are also right about 9d30eb47d's message. --migrate-authority was what the tree needed before 44ef71ebe; on top of it the plain refresh is what ran, and the message is stale. That is mine, not yours, and leaving it rather than force-pushing over my push was the right call. The squash message is what lands on main, so I will make that one accurate and the branch text can stay as it is.
The rebase no longer conflicts on generated files, it conflicts on meaning
main has moved. The rescore work from PR 2293 landed a few hours ago, and it caches the Effort and calories of a sleep-to-sleep cycle that has already ended, rather than re-reading a day of heart rate every pass. Its key is
loadKey = "\(window.onset)-\(window.endExclusive)|\(hrWitness.joined(separator: ","))"
and hrWitness is a per-owner count and newest timestamp of the HEART RATE in the window. Nothing else.
Your change makes that cycle's calories come from the MET stream instead. Put together, an ended cycle's calories are cached under a key that does not witness the MET series at all, and two things follow:
- A later drain that banks more MET minutes inside an ended window leaves the HR witness unchanged, so the cache keeps serving calories computed from the thinner earlier stream.
- On the first pass after the toggle goes on, cycles already cached under the HR-only key keep serving the HR figure and never recompute from MET.
That second one is the failure your own hunk says you already fixed once: "the log said 1220 kcal, the export held 743". that cache is a second route to the same outcome, and neither PR has the bug alone.
What it needs
A MET witness in the key, mirroring the HR one: a (count, maxTs) aggregate over ouraMetSample for the owner and window, appended to loadKey next to hrWitness. WhoopStore.hrFingerprint(deviceId:from:to:) in Reads.swift is the exact shape, index-only, two aggregates.
There is an irony in that: ouraMetSampleCount, which you removed as dead in answer to my first review, is close to the primitive this now wants. It WAS dead when you removed it. The need only appeared when that work merged ahead of you, so this is the merge order costing you a function rather than a wrong call at the time.
I have aborted my rebase rather than resolve this one, because picking a side here is a design decision about your feature and not a mechanical merge. Tell me if you would rather I pushed the witness myself and you reviewed it, or you take it.
Replaces #2273, rebased onto current
main. The eight commits there (four layers plus three fixes from the hardware days) are folded into four commits, one per layer, each fix inside the layer it corrects; the end tree is the same as #2273's head. #2273's thread keeps the two hardware read-outs in full. Refs #2242.What
On an Oura day
DailyMetric.activeKcalEstis today computed by the HR-only Keytel path over the ring's sparse banked HR; against Oura's own numbers it correlates at r ≈ −0.1. The ring already streams its own minute-by-minute activity intensity (0x50, MET), which NOOP has only ever kept as a diagnostic JSONL sidecar. This PR persists that series and, behind a default-off Experimental toggle, scores the day's calories from it with Oura's documented rule.Calories.estimateDayEnergyFromMET/estimateDayEnergyFromMet:active = Σ_{MET ≥ 1.5}(MET − 1.5) × 0.0175 kcal·kg⁻¹·min⁻¹ × weight, resting = the same revised Harris–Benedict the HR path uses over the covered minutes only, coverage reported over the window the caller passes. Missing minutes are unknown — never extrapolated to activity, never banked as rest. A re-served minute counts once: two samples are the same minute when|Δ| × 2 < min(epochS)(a re-served twin lands 2–5 s off its first copy under another session anchor; the ring's own grid steps back a second between records, so a real successor is 59–61 s on). Kotlin pinned to Swift by oracle (the realCaloriesenum compiled standalone, stdout pasted verbatim intoMetCaloriesOracleTest, 84 lines).ouraMetSample(deviceId, ts, met REAL, state, epochS)keyed(deviceId, ts): GRDBv47-oura-met-sample, RoomMIGRATION_40_41(schema 41), same column order, pinned inschema_oracle.json, on the device-scoped delete lists on both sides. Insert drops a twin by the same integer rule (OuraMetSample.droppingTwins/OuraMetSampleEntity.droppingTwins).epochSis carried per row so a cadence other than 60 s scales rather than skews.analyzeDaygainsdayMet(+dayMetNow,caloriesDiag). MET present ⇒activeKcalEst = MET totalwhen the stream covers ≥ 50 % of the window (today: of the hours elapsed); below that the number is withheld rather than substituted by the HR path.nil/empty keeps the HR path byte-identical for every WHOOP and pure-function caller. One always-on log line per MET day.OuraLiveSourcefans each anchored0x50record out to the table, only while the toggle is on (an OFF install's DB and scores are unchanged). The day-cycle fold (DayCycleIntelligenceIntegration, AndroidPhysiologicalStepCycleEngine) takes the same decisionanalyzeDaytakes over its own window instead of writing Keytel over it. Settings gains "Experimental · Oura Calories" (Oura-paired only), strings in all locales, README Oura table row,OURA_PROTOCOL.md§6.13 update. The toggle joins the day-cache config signature so a flip re-scores every cached day.Why this formula, and why no fitted constant
Oura's support page says active calories are "the portion that exceeds 1.5 MET". Taken literally, at the textbook MET→kcal definition (1 MET = 3.5 ml O₂·kg⁻¹·min⁻¹ ≈ 0.0175 kcal·kg⁻¹·min⁻¹), it reproduces the Oura app's own
active_calorieson the export's per-minute MET series with r = 1.0000 and 0.6 kcal/day RMSE over 75 current-era days (r 0.9999 / 3 kcal over 396 pre-2025 days), zero intercept, the wearer's weight being the only input. The constant was identified by that comparison, not fitted to it — nothing here is calibrated to a vendor number.Validation (the "tracks a varying input" bar)
0x50decode reproduces the export's Σ(MET) on 10/10 clean days within 1–8 % (16 days, 16,839 overlapping minutes).0x50record's timestamp is the END of its last sample — every record length n aligns to the export best at exactly −n minutes (85 % exact minute matches, r 0.90; read forward: 23 % / 0.57). The writer stamps sample i atutc − (n − i)·60, so a record straddling midnight lands on the right days. Documented in §6.13.Still an estimate of true expenditure (Kristiansson et al. 2023: free-living AEE MAPE 46–90 %), labelled as one.
Hardware (Gen 3, iOS, toggle ON)
MET path - coverage 72% (1035 samples), active 203 kcal— checked by hand against the stored rows (Σ(MET−1.5) = 184.1 MET-min × 0.0175 × 63 kg = 202.9). Two defects found that day, both fixed inside layers 1/2 and 4 here: a minute re-served under a second session anchor counted twice (+8 %), and the day-cycle fold wrote Keytel over the MET decision. With the twin rule: active 183.6 kcal vs the Oura app's 187 (−1.8 %). Details: feat(oura): active calories from the ring's own MET stream, behind a default-off Experimental toggle (#2242) #2273, comment 5713756097.activeKcalEst1397.2, the fold's wake-to-wake window, 0.3 % from the calendar line); store dedupe 1,197 rows = 1,197 distinct minutes. It also exposed the ring's one-second grid step, which the first twin rule read as a twin; with the half-period rule the same rows give 245.7 kcal vs the Oura app's 247 (−0.5 %), no workout that day. Details: feat(oura): active calories from the ring's own MET stream, behind a default-off Experimental toggle (#2242) #2273, comment 5727135808.Verification (on the rebased head)
swift test: StrandAnalytics 2068/0, WhoopStore 617/0 (schema oracle + device-scoped-table guard included).Strand(macOS) andNOOPiOSboth BUILD SUCCEEDED;DayCycleRecoveryTests10/0 underxcodebuild test.compileFullDebugKotlin,testFullDebugUnitTest6393 run, 8 failures — the dev Mac's French-JVM-locale formatting tests plus one FP artefact, which fail identically on a branch without this change; every test this PR adds or touches is green (MetCaloriesOracleTest,AnalyticsEngineMetCaloriesTest4/4,DayCycleMetCaloriesTest3/3,OuraMetSampleMigrationTest4/4,OuraMetSampleTwinTest3/3,DayCacheConfigFieldTest7/7,SchemaOracleTest,WhoopDatabaseUpgradeTest);WhoopDatabaseUpgradeTestruns v40 → v41 through the real chain against the committed schema;IntelligenceEngineJacocoBudgetTestgreen — the Android read lives inside the existingreadDaySkinAndWristOffsuspension point because a new suspend call inanalyzeRecentOnCpuspills ~700 instructions of continuation state.i18n_audit --ciOK,doc_comment_lintOK.