Repository navigation
Publish the command result code on Apple, as Android already does (#894) - #899
Merged
Merged
Conversation
ryanbr
force-pushed
the
fix/swift-command-response-result
branch
from
July 27, 2026 19:59
2ea1e6b to
ba6a488
Compare
A COMMAND_RESPONSE carries three things: which command it answers, the origin-seq echo, and the result code. Kotlin publishes all three — on 5/MG since the port, on 4.0 since #791. Swift published only the first, on either family. An Apple strap log named the command and then said nothing about whether it succeeded. That is not only a display gap. The result code is the accept/reject signal: UNSUPPORTED(3) is how the 5/MG haptics rejection was identified (#48), FAILURE(0) on an empty extended-battery reply is the whole finding in #791, and SUCCESS(1) is the entire evidence column in #891. Because the interpreter did not expose it, every Swift probe that needed it re-derived the byte privately — FeatureFlagProbe.resultLabel, DeviceConfigReadProbe via that, and BodyLocationProbe's own "Result code @12" — three copies of one two-line decode. Decoded in the two post-hooks rather than as schema fields. Adding `resp_seq`/`result` to the COMMAND_RESPONSE field list would have been a smaller diff and would have picked up the +4 shift for 5/MG automatically, but the schema-field loop reads at a raw frame offset with no payload bound, so a response whose declared payload ends before offset 8 would have reported its CRC32 trailer as a result code. The hooks already compute the bounded payload slice with exactly the guard Kotlin uses, so both fields are read from there and a short reply decodes neither. `CommandResult` does go in the shared schema, so `enumName` renders "SUCCESS(1)" and falls back to "0x7F(127)" — byte-identical to the Kotlin twin's commandResultLabel/hexLabel, which is what makes logs from the two platforms comparable rather than merely both present. Tests: `Whoop4ResponseResultTests` is the Swift twin of the Kotlin file of the same name and reuses its real #791 captures byte for byte, so the two suites now assert the same decode of the same frames — including the duplicate-write case where one send produced three responses sharing one origin-seq echo. Adds a fail-closed case (declared payload ends before the result byte -> neither field) and an unknown-code case (0x7F stays hex, no invented name). The #891 MG toggle fixtures now assert resp_seq and result too, which is what #893 had to leave out because this decoder did not produce them; the battery fixture pins that the new fields did not shift the offsets its 47% value is read from. The Python-golden parity corpus gains the two fields on its one COMMAND_RESPONSE entry. Updating it surfaced a counterexample to the #791 mapping itself: that frame is a real GET_BATTERY_LEVEL reply carrying a valid 42.5%, and its payload[1] is 0x00 — so it now reads FAILURE(0) on a reply that plainly succeeded. Kotlin's own battery fixture (91.2%) has the same shape, and Android has rendered it that way since #791, so this is upstream of this change rather than introduced by it. Filed as #900 and asserted meanwhile in testSuccessfulBatteryReadStillReportsResultZero, so the behaviour is deliberate and a future correction has to come past a test instead of silently changing a log. Verified: schema and golden JSON parse, offsets land on Kotlin's (4.0 @7/@8, 5/MG @11/@12), CRCs of both reused 4.0 fixtures re-verified independently, and the golden reserializes byte-identically apart from the intended entry. `fb.add` populates `parsed` on both decode paths, so FastPathParityTests' fast==full contract still holds. No BLE write path changes. Swift cannot be built on this host (no libc6-dev), so the suite runs in CI. doc_comment_lint and i18n_audit pass.
ryanbr
force-pushed
the
fix/swift-command-response-result
branch
from
July 27, 2026 20:32
ba6a488 to
ba70583
Compare
ryanbr
added a commit
that referenced
this pull request
Jul 27, 2026
) Until now no real WHOOP 5/MG frame in the tree carried `result = FAILURE(0)`. The only non-SUCCESS fixtures on that family are the two synthetic GET_HELLO frames, so the FAILURE branch of the result decode added in #899 had never been exercised by bytes a strap actually sent. #891's latest hardware runs supply one. The same MG accepted `SELECT_WRIST` with arg=1 and refused it with arg=0 minutes apart in one session, which gives a success/failure pair on a single opcode — the shape that made the 4.0 result mapping credible when GET_EXTENDED_BATTERY_INFO was seen answering both ways, and which 5/MG had no equivalent of. It is also the first fixture for opcode 123 on either family. Both frames re-verified before committing: CRC16-Modbus header, CRC32 tail, declared length, and every field against the decoder rather than by hand. Provenance stated at the fixture per #903 — WHOOP 5 MG WS50_r03 build 215, posted with the raw bytes in #891 — and the payloads are inert (`01 01 00 00` and `00 01 00 00`): no name, serial or token. Swift and Kotlin assert the identical bytes. The refusal is deliberately not interpreted. The strap ran the same opcode successfully minutes earlier, so it is not "unknown command", but whether it refuses a mutation or whether 0 is simply not a valid argument is open — the right=0/left=1 mapping is inferred from the vendor client's enum order and has never been attested. The test pins the decode of both outcomes and leaves the meaning to #891. Also sharpens why byte 13 stays undecoded. The new run sent arg=0 and got byte 13 = 1 back, which refutes the echo reading outright. It does not establish the read-back reading: across all eight MG command responses now on record, over four opcodes and both result codes, byte 13 has taken exactly one value, so a constant fits every observation as well as stored state does and is simpler. Naming the slot would encode the unproven half of a result that only refuted the alternative. No production code changes — fixtures and rationale only.
ryanbr
added a commit
that referenced
this pull request
Jul 28, 2026
PROTOCOL.md documented the envelope down to `payload [11..]` and stopped, so the two bytes every reply actually opens with were described nowhere — even though #899 made both decoders publish them and added CommandResult to the schema. Three places had been describing the same layout ad hoc instead: DeviceConfigReadProbe and FeatureFlagProbe each say "the 2-byte response header (pay[1] is the 5/MG result code)" in their own comments, and the ECG paragraph mentions a result code in passing. New §2.4 states it once, for both families: resp_cmd/resp_seq/result at [6][7][8] on 4.0 and the same +4 on 5/MG. It also records that resp_seq is the strap's counter rather than the echoed host-assigned envelope seq — a real capture shows envelope seq 147 beside resp_seq 2, and the repeated-resp_seq signature is what identified a duplicated write in #791. The part worth writing down is the first BODY byte. GET_BATTERY_LEVEL puts the charge percentage there (47 in the hardware-confirmed fixture), so the slot carries real data, but on every other captured command it has only ever been 1. For the wrist and ECG toggles what that means is open: sending SELECT_WRIST with argument 0 got 1 back, which refutes an echo of the request, but every frame captured so far had a stored value of 1, so "reads back stored state" and "this handler writes a literal 1" predict identically on all of them. Documented as open, with what would settle it, rather than resolved — the decoder deliberately leaves the byte unnamed and this explains why to the next reader. Old §2.4 Checksums and §2.5 Reassembly shift to §2.5/§2.6. No cross-reference in the tree points at either; the only internal PROTOCOL.md section reference is §2.2, unchanged. Every claim checked against source before writing: both platforms' offsets from the decoders, the PENDING-then-SUCCESS and UNSUPPORTED(3)-on-haptics results from the captures that produced them, and the body-byte table recomputed from the committed fixtures.
ryanbr
added a commit
that referenced
this pull request
Aug 7, 2026
…1120) On reconnect after a gap, a strap offload commits chunks and fires onBackfillChunkCommitted → IntelligenceEngine.analyzeRecent(maxDays=21), a ~54h × up-to-21-day re-score (~70s of CPU on a large DB). That pass is a real update path and should run — but it never advanced the #836 HR-fingerprint watermark, because on Android the watermark logic lived ONLY in AppViewModel's 15-min loop. So immediately afterwards the next idle tick saw fp != watermark and re-ran the IDENTICAL multi-day re-score: every reconnect paid for the heavy pass twice (worse across sliced offloads). Fix: capture repository.hrFingerprint() before the post-backfill pass and, on success, NoopPrefs.setAnalyzeWatermark(fp) — so the next 15-min tick sees no change and skips. Captured-before / written-only-on-success matches the UI loop, so an interrupted pass can't advance the watermark past unscored data. Parity: this is exactly what Swift already does — analyzeRecent sets the watermark at the end of EVERY successful run (IntelligenceEngine.swift:1549) and refreshAfterCompletedBackfill calls it force:true, so the Swift 15-min tick already skips. No Swift change; this brings Android into lockstep. Scope (maxDays / window) is unchanged — no per-day skip (the baseline-fold + #899 window-wide dedup make that a data-loss trap), so scores stay byte-identical.
ryanbr
added a commit
that referenced
this pull request
Aug 7, 2026
…tery, both platforms) (#1124) * analyze: halve the idle backstop re-score cadence to 30 min (#836 battery) A phone-battery capture showed ~6-7 full 21-day analyzeRecent passes in a single hour — each reading ~21×54h of HR and running analyzeDay + the sleep stager over a 16.6M-row DB. Root: the 15-min backstop loop's watermark gate (#1120) can't skip while the strap is connected and streaming live HR, because the HR fingerprint (count:maxTs) advances every second — so it re-scored the WHOLE 21-day window every 15 min even though only TODAY's daytime HR changed (the other 20 days, and the night-computed baseline, are unaffected by daytime HR). That's the #836 CPU cost. Fix: bump ANALYZE_INTERVAL_MS 15 -> 30 min. This loop is a pure BACKSTOP — every real update (sync-with-rows, import, sleep/workout edit, recalibrate, the #547 heal) rescores via its OWN forced path, and the #386 app-resume kick still wakes it EARLY the moment the user opens NOOP. So halving the cadence only delays the idle refresh of Today's live Effort/steps by up to 15 min while connected; recovery/sleep are night-computed and untouched, and no real data update is delayed. Lowest-risk of the #836 options: a pure time throttle, NOT day-scoping (which would hit the #899 window-wide-dedup / baseline-fold data-loss trap). One constant, all-or-nothing (never a partial pass). Android-only tuning; the Swift AppModel loop is a parity follow-up. compileFullDebugKotlin passes. * analyze: mirror the 30-min backstop cadence on iOS/macOS (#836 parity) Swift AppModel's analyze loop had the same 15-min backstop (Task.sleep 900s) with the same force:false gate that can't skip while live HR streams — so iOS paid the same #836 re-score CPU. Bump to 30 min to match the Android ANALYZE_INTERVAL_MS change. App-target Swift — validated by app-build, not a strap.
ryanbr
added a commit
that referenced
this pull request
Aug 11, 2026
… computedId (#1248) The #899 overlap heal deduped only under computedId, so an Oura ring's own hypnogram (banked under the ring id) was never collapsed and re-detected every pass — one night ballooned to 14 rows / 9 phantom naps. Sweep the heal over computedId + every registered device id, deduping each device's rows among themselves and deleting under that same id. Reuses SleepSessionDedup; pure healDeviceIds helper tested byte-identical Swift+Kotlin.
ryanbr
added a commit
that referenced
this pull request
Aug 12, 2026
…#899 sweep (#1284) (#1293) Two #1284 residuals from pipiche38's measured hardware report. 1. #1246/#1251 flag a hypnogram page unwritten only when EVERY code byte is 0xFF. A PARTLY-written page fills front-to-back and leaves the TAIL as 0xFF pad, which still unpacks as run*4 epochs of fake awake (measured ~86-88 min/night, pushing efficiency far below WHOOP's). Flag a TRAILING run of >= MIN_TRAILING_UNWRITTEN (6 code bytes = 24 min) consecutive 0xFF as unwritten too. Trailing-only + a run floor, on purpose: a leading/interior 0xFF run stays real wake (a pre-onset run is dropped by the assembler's onset clip), and a short trailing run is spared. Byte-identical Swift OuraDecoders + Android OuraDecoders (same 6-byte floor), tests both sides (the reporter's hdr=01 record, the floor boundary, trailing-only). The floor is tunable: the measured pads ran 9-10 bytes; pipiche can pin the exact value against the full capture. 2. The #899 dedup heal only logged on a non-empty drop, so a heal that collapsed rows was indistinguishable in the strap log from never having run. Always log the sweep ('Dedup(#899): swept N id(s), removed M ...'), both platforms. Existing #1246 tests preserved (1-2 byte cases stay below the 6-byte floor).
ryanbr
added a commit
that referenced
this pull request
Aug 12, 2026
…ual 3) (#1297) * sleep(#1210): shadow-instrument the dormant band wake-veto (output-neutral) Follow-up to #1210 / #738. The band sleep_state wake-veto is landed but default-off, so we have no measurement of what it WOULD do on real banded nights - the exact recovered-vs-reverse ratio #1210 says is still too thin (12 nights). This adds an OUTPUT-NEUTRAL shadow: when a band stream is present, the flag is off, and a diagnostic traceSink is collecting, scoring computes what the veto would recover and emits one namespaced trace line per banded night: bandVeto(shadow): startTs=<unix> recoveredMin=<n> eff <raw>%-> <shadow>% Nothing changes in the persisted hypnogram or efficiency - the shadow stages are computed for the log only; the flag-gated (unchanged) stages are still what is stored and displayed. Guarded on traceSink, so zero cost in normal operation; it fires only while a diagnostic is being generated, which re-scores the recent window and thus reports the delta for every recent banded night. Honest scope: the line reports MAGNITUDE and FREQUENCY (how often the band disputes our interior wake, and by how much), which is what #1210 item 1 asks to widen. It does NOT settle the SIGN question - whether that move is toward truth. The PSG harness currently measures the recipe UNDER-calling wake overall (bias -4.92 pp), so the flip decision still needs the truth comparison, not just the band. This gathers the band side of that evidence across wearers without any visible change. Retire on the flip. Byte-identical Swift (StrandAnalytics) + Kotlin analytics; the shadow line carries no sleep day= / gate run= / day= token so CaptureAccumulator never counts it as a captured day (pinned by a test both platforms). Plus an end-to-end test that a banded night with interior false-wake emits the line while the returned hypnogram stays byte-identical to the untraced call. No app-target Swift; swift-packages + android unit tests cover it. * sleep(#1210 item 2): add the new-nights-only cutoff to the band wake-veto Complements the shadow instrumentation with the SAFE half of option B: a dormant bandStateWakeVetoCutoffTs (0 = every night). When the veto is eventually flipped on, a non-zero cutoff (a unix second) restricts the correction to sessions whose start is at or after it, so nights already in history keep their raw efficiency - flipping the default can never silently re-score months of banked nights upward. Inclusive boundary. This resolves #1210 item 2 (the retroactive-rescore decision) by building the gate-to-new-nights path, without flipping anything: the flag stays false, so the cutoff is inert. The eventual flip is then a dated one-liner (set enabled = true AND cutoffTs = the flip date) with history provably untouched. The shadow (previous commit) deliberately passes cutoffTs = 0 so it still measures the FULL veto potential across every banded night, history included, for the validation distribution - independent of whatever cutoff the flip later uses. Byte-identical Swift + Kotlin; pinned by a test both platforms (a night before the cutoff is unchanged, one at/after it gets the veto). No app-target Swift. * sleep: fix stale bandStateWakeVetoEnabled doc (Kotlin) - it is default-OFF The Kotlin flag doc read 'Default-ON gate ... Flip to false', contradicting both the actual value (false) and the Swift twin it claims to mirror (default-off, flip to true only with PSG evidence, -4.92 pp under-call). A maintainer reading the Kotlin side would misjudge the shipped state. Align it to reality + the Swift source of truth; note the new-nights cutoff to set alongside the flip. Doc-only, no behaviour change. * oura(sleep): clamp the trailing-0xFF floor to >= 2 so a lone byte can't be eaten (#1284) Hardening surfaced by @pipiche38's six-night hardware validation of #1293. The trailing-run unwritten rule keys off minTrailingUnwritten (shipped 6). Nothing stopped that constant going below 2, and at 1 a LONE trailing 0xFF byte - four genuine awake epochs, exactly the case #1246 was careful to protect - would be flagged unwritten and dropped. The existing FloorIsExclusive test asserts minTrailingUnwritten*4, so it scaled with the constant and would have stayed green. Clamp the effective floor to max(2, minTrailingUnwritten) in both decoders. The whole-record rule already fires only at codeCount >= 2, so this just holds the trailing rule to the same minimum: the constant stays freely tunable upward (the validation confirms 6 is right), but can never drop low enough to eat a single real-wake byte. Shipped behaviour is unchanged (max(2,6) = 6). Byte-identical Swift (OuraProtocol) + Kotlin (com.noop.oura), each with a new test pinning 'a written record ending in a single trailing 0xFF stays awake' plus a floor >= 2 tripwire. Swift 37/37 + Android 39/39 green locally. No app-target Swift. Addresses the hardening item in #1284; residuals 2 (banked-night heal) and 3 (0x49 session keying) remain open. * oura(sleep): log the duplicate-session GENERATION moment (#1284 residual 3) @pipiche38's #1284 shows a night still mints a second row - a short session banked by an early partial drain, nested inside the real one banked by the later full burst - which the #899 heal only cleans up afterward. From the strap log alone the GENERATION is invisible; #1249 fixed the heal, not the cause. Instrument-first (the cause is a session-identity change in hardware-gated BLE persist code that needs a strap to validate): track each sleep session's [start,end] banked this connection, and when a new persist lands wholly inside - or overlapping - an already-banked one, log it with the 0x49 anchor state: Oura: duplicate-gen(#1284) new session [S -> E] NESTED IN already-banked [s -> e] this connection - NO 0x49 anchor (write-envelope end); PK=(deviceId,startTs) mints a 2nd row A persist with NO 0x49 anchor is the prime suspect for the short row, so the line names it. Diagnostic ONLY - nothing is skipped, no BLE/connection/write path changes; the fix is the 0x49-window session keying pipiche recommends, which this pins on the next capture. Byte-parity Swift (Strand/BLE, app-target) + Kotlin (com.noop.ble); bounded in-memory list, reset per connection with the 0x49 windows. Android compiles; Swift app-target verified via the app-build gate. Residual 1 done+hardened (#1293/#1296); residual 2 (banked heal) open. * oura(sleep): stamp the 0x49 anchor state on every persist line (#1284 residual 3) The duplicate-gen line fires on the SECOND (real) persist, but the key evidence is that the FIRST (short) row was banked WITHOUT a 0x49 anchor - and that first persist has nothing prior to overlap, so it never trips the duplicate-gen line. Stamp [0x49-anchored] / [NO-0x49-anchor] on every 'sleep session persisted' line so the unanchored early-drain row is self-evident on its own, making the whole generation sequence legible from the strap log alone. Byte-parity Swift + Kotlin; diagnostic only. * oura(sleep): use sleepStart (not w) for the Kotlin duplicate-gen anchor label (#1284) Re-review parity fix. The Kotlin duplicate-gen line labelled the anchor off the 0x49 window match (w != null) while the Swift twin and both persist-tag lines use the 0x49 ONSET (sleepStart). Three problems: the two platforms emitted different text for the same scenario; the window-match is the wrong signal for a startTs-PK duplicate (a persist can match the window yet leave the START unanchored when the onset fails its plausibility clamp, exactly the duplicate-prone case); and the same persist could read one anchor label on the duplicate-gen line and the opposite on the persist tag. Use sleepStart on both, byte-identical to Swift and consistent with the persist tag. Kotlin-only (Swift was already correct); Android compiles. * oura(sleep): label the persist anchor by 0x49 ONSET, not generic anchor (#1284) Re-review clarity fix. The tag said 0x49-anchored / NO-0x49-anchor, keyed on sleepStart (the 0x49 ONSET). But a burst can have its END 0x49-refined while the onset fails its plausibility clamp, so the old NO-0x49-anchor tag could sit right under the existing 'hypnogram burst end refined by 0x49' line and read as a contradiction. The tag measures the ONSET (the start anchor that governs the startTs PK), so name it that: 0x49-onset / no-0x49-onset, and the duplicate-gen line likewise. Removes the ambiguity for the reader. Byte-parity Swift + Kotlin; diagnostic only; Android compiles.
5 tasks done
ryanbr
added a commit
that referenced
this pull request
Aug 15, 2026
…h platforms) At persist time (SourceCoordinator persist closure, Swift + Kotlin) read the day's already-stored sessions via candidateReadWindow and run the reconciler: insert a distinct session, skip when a fuller same-night row is stored, or delete the earlier/shorter duplicates and upsert the fuller one. The DB read also sees cross-connection duplicates the per-connection diagnostic can't. - Delete uses the tombstone-free row delete on both sides (Swift store.deleteSleepSession; Kotlin repo.deleteSleepSessionRowOnly), matching the #899 heal so a superseded duplicate can't suppress the survivor's re-detection. - A true re-persist (same startTs) is excluded from candidates so it upserts idempotently rather than delete+insert (no churn, keeps userEdited). - Kotlin reconciler timestamps moved Int -> Long (SleepSession.startTs is Long; avoids the 2038 32-bit overflow and matches the Swift 64-bit Int). Kotlin reconciler tests still 6/6 green. Runtime is hardware-gated on @pipiche38's ring (Q3: one row across two connections). Swift app-target validated via the app-build gate; reconciler via swift-packages; Android via local testFullDebugUnitTest.
This was referenced Aug 15, 2026
ryanbr
added a commit
that referenced
this pull request
Aug 15, 2026
…heal (#1284) (#1338) * fix(sleep): collapse a near-adjacent pre-onset fragment in the dedup heal (#1284) The #899 SleepSessionDedup overlap rule requires overlap > 0, so it misses the Oura SleepNet pre-onset fragment: a short session that ends seconds BEFORE the 0x49-anchored night begins with no overlap (measured 69 s on 08-10/11; the pre-onset run is ~7 min / 14 codes). Extend isDuplicate so two DISJOINT sessions within a 15-min edge gap are also one night — a gap that small can only be one interrupted night, never two real sleeps (a nap is hours out). The existing overlap branch and survivor sort (userEdited > fresh > longest) are unchanged, so the fuller/edited row still wins and WHOOP behaviour is untouched. Swift + Kotlin twins; 3 new tests each (fragment collapses, real nap stays separate, gap > 15 min stays separate). Full Android suite green (3834). Heal-side (post-hoc): this completes the sweep so no duplicate survives to the next analyze pass. It does NOT stop duplicate GENERATION or the transient phantom-nap window before the heal runs — that needs the separate generation-side 0x49/sleep-day keying (#1284 residual 3, hardware-gated). * test(sleep): use only public dedup API in the #1284 near-adjacent tests overlapSeconds is internal and WhoopStoreTests imports WhoopStore without @testable, so the disjointness sanity assertions didn't compile in swift-packages CI (Kotlin's internal is same-module, so it passed locally). isDuplicate/dedupe (public) already cover the behaviour; drop the redundant overlapSeconds calls.
ryanbr
added a commit
that referenced
this pull request
Aug 16, 2026
Follow-ups on the same heal, both serving residual 3. Tests: two survivor-selection cases the whole design leans on — - mode-2 partial drain: a 234 min partial nested in a 494 min decode of one night collapses to the FULLER row (longest-wins), so a partial re-drain never clobbers a complete night; - mode-1 identical re-anchors: several equal-length rows whose end chases wall-clock resolve by the latest-endTs tie-break — the row that matched WHOOP's wake. Pins that load-bearing order so it is not retuned away from ground truth. Swift + Kotlin. Heal diagnostic: the #899/#1284 store heal logged only a removal COUNT. It now logs, per dropped row, the dropped and surviving shapes (`[start->end] min= segs= json=`, the same vocabulary as the dup-gen diagnostic), so the strap log alone confirms the heal keeps the fuller / end-correct copy — the corpus the generation-side 0x49-onset keying will be designed against. Log-only, counts not content, both platforms. Deletion behaviour unchanged.
ryanbr
added a commit
that referenced
this pull request
Aug 16, 2026
Follow-ups on the same heal, both serving residual 3. Tests: two survivor-selection cases the whole design leans on — - mode-2 partial drain: a 234 min partial nested in a 494 min decode of one night collapses to the FULLER row (longest-wins), so a partial re-drain never clobbers a complete night; - mode-1 identical re-anchors: several equal-length rows whose end chases wall-clock resolve by the latest-endTs tie-break — the row that matched WHOOP's wake. Pins that load-bearing order so it is not retuned away from ground truth. Swift + Kotlin. Heal diagnostic: the #899/#1284 store heal logged only a removal COUNT. It now logs, per dropped row, the dropped and surviving shapes (`[start->end] min= segs= json=`, the same vocabulary as the dup-gen diagnostic), so the strap log alone confirms the heal keeps the fuller / end-correct copy — the corpus the generation-side 0x49-onset keying will be designed against. Log-only, counts not content, both platforms. Deletion behaviour unchanged.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…ryanbr#899 sweep (ryanbr#1284) (ryanbr#1293) Two ryanbr#1284 residuals from pipiche38's measured hardware report. 1. ryanbr#1246/ryanbr#1251 flag a hypnogram page unwritten only when EVERY code byte is 0xFF. A PARTLY-written page fills front-to-back and leaves the TAIL as 0xFF pad, which still unpacks as run*4 epochs of fake awake (measured ~86-88 min/night, pushing efficiency far below WHOOP's). Flag a TRAILING run of >= MIN_TRAILING_UNWRITTEN (6 code bytes = 24 min) consecutive 0xFF as unwritten too. Trailing-only + a run floor, on purpose: a leading/interior 0xFF run stays real wake (a pre-onset run is dropped by the assembler's onset clip), and a short trailing run is spared. Byte-identical Swift OuraDecoders + Android OuraDecoders (same 6-byte floor), tests both sides (the reporter's hdr=01 record, the floor boundary, trailing-only). The floor is tunable: the measured pads ran 9-10 bytes; pipiche can pin the exact value against the full capture. 2. The ryanbr#899 dedup heal only logged on a non-empty drop, so a heal that collapsed rows was indistinguishable in the strap log from never having run. Always log the sweep ('Dedup(ryanbr#899): swept N id(s), removed M ...'), both platforms. Existing ryanbr#1246 tests preserved (1-2 byte cases stay below the 6-byte floor).
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…residual 3) (ryanbr#1297) * sleep(ryanbr#1210): shadow-instrument the dormant band wake-veto (output-neutral) Follow-up to ryanbr#1210 / ryanbr#738. The band sleep_state wake-veto is landed but default-off, so we have no measurement of what it WOULD do on real banded nights - the exact recovered-vs-reverse ratio ryanbr#1210 says is still too thin (12 nights). This adds an OUTPUT-NEUTRAL shadow: when a band stream is present, the flag is off, and a diagnostic traceSink is collecting, scoring computes what the veto would recover and emits one namespaced trace line per banded night: bandVeto(shadow): startTs=<unix> recoveredMin=<n> eff <raw>%-> <shadow>% Nothing changes in the persisted hypnogram or efficiency - the shadow stages are computed for the log only; the flag-gated (unchanged) stages are still what is stored and displayed. Guarded on traceSink, so zero cost in normal operation; it fires only while a diagnostic is being generated, which re-scores the recent window and thus reports the delta for every recent banded night. Honest scope: the line reports MAGNITUDE and FREQUENCY (how often the band disputes our interior wake, and by how much), which is what ryanbr#1210 item 1 asks to widen. It does NOT settle the SIGN question - whether that move is toward truth. The PSG harness currently measures the recipe UNDER-calling wake overall (bias -4.92 pp), so the flip decision still needs the truth comparison, not just the band. This gathers the band side of that evidence across wearers without any visible change. Retire on the flip. Byte-identical Swift (StrandAnalytics) + Kotlin analytics; the shadow line carries no sleep day= / gate run= / day= token so CaptureAccumulator never counts it as a captured day (pinned by a test both platforms). Plus an end-to-end test that a banded night with interior false-wake emits the line while the returned hypnogram stays byte-identical to the untraced call. No app-target Swift; swift-packages + android unit tests cover it. * sleep(ryanbr#1210 item 2): add the new-nights-only cutoff to the band wake-veto Complements the shadow instrumentation with the SAFE half of option B: a dormant bandStateWakeVetoCutoffTs (0 = every night). When the veto is eventually flipped on, a non-zero cutoff (a unix second) restricts the correction to sessions whose start is at or after it, so nights already in history keep their raw efficiency - flipping the default can never silently re-score months of banked nights upward. Inclusive boundary. This resolves ryanbr#1210 item 2 (the retroactive-rescore decision) by building the gate-to-new-nights path, without flipping anything: the flag stays false, so the cutoff is inert. The eventual flip is then a dated one-liner (set enabled = true AND cutoffTs = the flip date) with history provably untouched. The shadow (previous commit) deliberately passes cutoffTs = 0 so it still measures the FULL veto potential across every banded night, history included, for the validation distribution - independent of whatever cutoff the flip later uses. Byte-identical Swift + Kotlin; pinned by a test both platforms (a night before the cutoff is unchanged, one at/after it gets the veto). No app-target Swift. * sleep: fix stale bandStateWakeVetoEnabled doc (Kotlin) - it is default-OFF The Kotlin flag doc read 'Default-ON gate ... Flip to false', contradicting both the actual value (false) and the Swift twin it claims to mirror (default-off, flip to true only with PSG evidence, -4.92 pp under-call). A maintainer reading the Kotlin side would misjudge the shipped state. Align it to reality + the Swift source of truth; note the new-nights cutoff to set alongside the flip. Doc-only, no behaviour change. * oura(sleep): clamp the trailing-0xFF floor to >= 2 so a lone byte can't be eaten (ryanbr#1284) Hardening surfaced by @pipiche38's six-night hardware validation of ryanbr#1293. The trailing-run unwritten rule keys off minTrailingUnwritten (shipped 6). Nothing stopped that constant going below 2, and at 1 a LONE trailing 0xFF byte - four genuine awake epochs, exactly the case ryanbr#1246 was careful to protect - would be flagged unwritten and dropped. The existing FloorIsExclusive test asserts minTrailingUnwritten*4, so it scaled with the constant and would have stayed green. Clamp the effective floor to max(2, minTrailingUnwritten) in both decoders. The whole-record rule already fires only at codeCount >= 2, so this just holds the trailing rule to the same minimum: the constant stays freely tunable upward (the validation confirms 6 is right), but can never drop low enough to eat a single real-wake byte. Shipped behaviour is unchanged (max(2,6) = 6). Byte-identical Swift (OuraProtocol) + Kotlin (com.noop.oura), each with a new test pinning 'a written record ending in a single trailing 0xFF stays awake' plus a floor >= 2 tripwire. Swift 37/37 + Android 39/39 green locally. No app-target Swift. Addresses the hardening item in ryanbr#1284; residuals 2 (banked-night heal) and 3 (0x49 session keying) remain open. * oura(sleep): log the duplicate-session GENERATION moment (ryanbr#1284 residual 3) @pipiche38's ryanbr#1284 shows a night still mints a second row - a short session banked by an early partial drain, nested inside the real one banked by the later full burst - which the ryanbr#899 heal only cleans up afterward. From the strap log alone the GENERATION is invisible; ryanbr#1249 fixed the heal, not the cause. Instrument-first (the cause is a session-identity change in hardware-gated BLE persist code that needs a strap to validate): track each sleep session's [start,end] banked this connection, and when a new persist lands wholly inside - or overlapping - an already-banked one, log it with the 0x49 anchor state: Oura: duplicate-gen(ryanbr#1284) new session [S -> E] NESTED IN already-banked [s -> e] this connection - NO 0x49 anchor (write-envelope end); PK=(deviceId,startTs) mints a 2nd row A persist with NO 0x49 anchor is the prime suspect for the short row, so the line names it. Diagnostic ONLY - nothing is skipped, no BLE/connection/write path changes; the fix is the 0x49-window session keying pipiche recommends, which this pins on the next capture. Byte-parity Swift (Strand/BLE, app-target) + Kotlin (com.noop.ble); bounded in-memory list, reset per connection with the 0x49 windows. Android compiles; Swift app-target verified via the app-build gate. Residual 1 done+hardened (ryanbr#1293/ryanbr#1296); residual 2 (banked heal) open. * oura(sleep): stamp the 0x49 anchor state on every persist line (ryanbr#1284 residual 3) The duplicate-gen line fires on the SECOND (real) persist, but the key evidence is that the FIRST (short) row was banked WITHOUT a 0x49 anchor - and that first persist has nothing prior to overlap, so it never trips the duplicate-gen line. Stamp [0x49-anchored] / [NO-0x49-anchor] on every 'sleep session persisted' line so the unanchored early-drain row is self-evident on its own, making the whole generation sequence legible from the strap log alone. Byte-parity Swift + Kotlin; diagnostic only. * oura(sleep): use sleepStart (not w) for the Kotlin duplicate-gen anchor label (ryanbr#1284) Re-review parity fix. The Kotlin duplicate-gen line labelled the anchor off the 0x49 window match (w != null) while the Swift twin and both persist-tag lines use the 0x49 ONSET (sleepStart). Three problems: the two platforms emitted different text for the same scenario; the window-match is the wrong signal for a startTs-PK duplicate (a persist can match the window yet leave the START unanchored when the onset fails its plausibility clamp, exactly the duplicate-prone case); and the same persist could read one anchor label on the duplicate-gen line and the opposite on the persist tag. Use sleepStart on both, byte-identical to Swift and consistent with the persist tag. Kotlin-only (Swift was already correct); Android compiles. * oura(sleep): label the persist anchor by 0x49 ONSET, not generic anchor (ryanbr#1284) Re-review clarity fix. The tag said 0x49-anchored / NO-0x49-anchor, keyed on sleepStart (the 0x49 ONSET). But a burst can have its END 0x49-refined while the onset fails its plausibility clamp, so the old NO-0x49-anchor tag could sit right under the existing 'hypnogram burst end refined by 0x49' line and read as a contradiction. The tag measures the ONSET (the start anchor that governs the startTs PK), so name it that: 0x49-onset / no-0x49-onset, and the duplicate-gen line likewise. Removes the ambiguity for the reader. Byte-parity Swift + Kotlin; diagnostic only; Android compiles.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…heal (ryanbr#1284) (ryanbr#1338) * fix(sleep): collapse a near-adjacent pre-onset fragment in the dedup heal (ryanbr#1284) The ryanbr#899 SleepSessionDedup overlap rule requires overlap > 0, so it misses the Oura SleepNet pre-onset fragment: a short session that ends seconds BEFORE the 0x49-anchored night begins with no overlap (measured 69 s on 08-10/11; the pre-onset run is ~7 min / 14 codes). Extend isDuplicate so two DISJOINT sessions within a 15-min edge gap are also one night — a gap that small can only be one interrupted night, never two real sleeps (a nap is hours out). The existing overlap branch and survivor sort (userEdited > fresh > longest) are unchanged, so the fuller/edited row still wins and WHOOP behaviour is untouched. Swift + Kotlin twins; 3 new tests each (fragment collapses, real nap stays separate, gap > 15 min stays separate). Full Android suite green (3834). Heal-side (post-hoc): this completes the sweep so no duplicate survives to the next analyze pass. It does NOT stop duplicate GENERATION or the transient phantom-nap window before the heal runs — that needs the separate generation-side 0x49/sleep-day keying (ryanbr#1284 residual 3, hardware-gated). * test(sleep): use only public dedup API in the ryanbr#1284 near-adjacent tests overlapSeconds is internal and WhoopStoreTests imports WhoopStore without @testable, so the disjointness sanity assertions didn't compile in swift-packages CI (Kotlin's internal is same-module, so it passed locally). isDuplicate/dedupe (public) already cover the behaviour; drop the redundant overlapSeconds calls.
ryanbr
added a commit
that referenced
this pull request
Aug 17, 2026
…re battery drain On a heavy WHOOP 4.0 user (21 nights, ~178k HR rows/night, a 1.26 GB store) every re-score re-read *every* night's raw streams and re-ran analyzeDay, even though a post-offload only adds rows to the 1-2 most-recent days — measured median ~4.6 min/pass, all CPU, fired back-to-back through an offload storm. Pass 1 already keeps only each night's small result (not the raw streams), and every field except recovery is baseline-independent (pass 2 re-scores the cheap recovery composite on every day regardless). So a night whose scored inputs are unchanged since it was last scored re-produces a byte-identical result: keep an in-memory day -> (key, scan) cache on the long-lived engine and reuse the scan when the key matches, skipping the 7 stream reads + analyzeDay. Fail-safe: a miss, a non-4.0 owner, an active Test-Centre trace, or a config change all fall through to the identical full path; the cache only ever skips the analyzeDay STAGE — every night still flows into pass 2, so baselines, recovery, the stale-day eviction and the #899 heal are byte-unaffected (no banking / data-loss surface, avoiding the #836/#841 trap). Per-day key = night HR fingerprint (count + newest ts, the same witness the whole-pass gate trusts) + the 4.0 window-wide skin anchor. Pass-global inputs (profile, baselines1, sleep need/consistency, habitual midsleep, tz, toggles) drop the whole cache via a per-pass config signature. Gated to registered WHOOP 4.0 owners (the reported drain; the one family classified identically on both platforms) — 5/MG is a deliberate follow-up. Reused nights re-emit their per-day diagnostics so the strap log is unchanged. Both platforms (parity contract): Swift carries it on the DayScan the loop already builds; Kotlin adds a CachedDayScan + a windowed hrFingerprint DAO query. Pure key helper (AnalyzeRecentDayCache) unit-tested on both.
ryanbr
added a commit
that referenced
this pull request
Aug 17, 2026
…re battery drain (#1395) On a heavy WHOOP 4.0 user (21 nights, ~178k HR rows/night, a 1.26 GB store) every re-score re-read *every* night's raw streams and re-ran analyzeDay, even though a post-offload only adds rows to the 1-2 most-recent days — measured median ~4.6 min/pass, all CPU, fired back-to-back through an offload storm. Pass 1 already keeps only each night's small result (not the raw streams), and every field except recovery is baseline-independent (pass 2 re-scores the cheap recovery composite on every day regardless). So a night whose scored inputs are unchanged since it was last scored re-produces a byte-identical result: keep an in-memory day -> (key, scan) cache on the long-lived engine and reuse the scan when the key matches, skipping the 7 stream reads + analyzeDay. Fail-safe: a miss, a non-4.0 owner, an active Test-Centre trace, or a config change all fall through to the identical full path; the cache only ever skips the analyzeDay STAGE — every night still flows into pass 2, so baselines, recovery, the stale-day eviction and the #899 heal are byte-unaffected (no banking / data-loss surface, avoiding the #836/#841 trap). Per-day key = night HR fingerprint (count + newest ts, the same witness the whole-pass gate trusts) + the 4.0 window-wide skin anchor. Pass-global inputs (profile, baselines1, sleep need/consistency, habitual midsleep, tz, toggles) drop the whole cache via a per-pass config signature. Gated to registered WHOOP 4.0 owners (the reported drain; the one family classified identically on both platforms) — 5/MG is a deliberate follow-up. Reused nights re-emit their per-day diagnostics so the strap log is unchanged. Both platforms (parity contract): Swift carries it on the DayScan the loop already builds; Kotlin adds a CachedDayScan + a windowed hrFingerprint DAO query. Pure key helper (AnalyzeRecentDayCache) unit-tested on both.
ryanbr
added a commit
that referenced
this pull request
Aug 25, 2026
Re-review of the previous commit: on both platforms the summary emit was inserted between the #1538 comment and the dayCache line that comment describes, so it read as documentation for the new call instead. Same shape as the detached-doc-comment trap, but with `//` comments, which the doc lint does not check — it only binds `/** */` and `///` blocks. Moved above the comment block on both sides. No behaviour change; the summary still emits immediately before the dayCache line, in the same order on both platforms. Also confirmed while re-reviewing: both analyzeRecentOnCpu call sites are inside analyzeGate.withLock (opened 420, closed 437), so the collector field genuinely has no concurrent access; the #899 heal re-pass calls the method a second time under that same lock, where reset() correctly gives that pass its own summary, matching the old two-sets-of-lines behaviour; and there is no early return between reset() and emit() on either platform, so a pass can never collect days and then drop them. compileFullDebugKotlin clean; budget test 61495/61535; 5 JVM tests green; doc_comment_lint clean.
8 of 12 tasks
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 1, 2026
…otlin change Comments in AppModel.swift, BLEManager.swift, LiveState.swift, and WhoopBleClient.kt stated "GRDB writer / main-actor contention" as the confirmed MECHANISM behind the 48s->573s overlap inflation. Only the CORRELATION (overlap present => pass much slower) is actually confirmed from the device log — the mechanism was never isolated. Corrected all four to say so plainly rather than stating an unverified explanation as fact. Reverted POST_BACKFILL_ANALYZE_DELAY_MS's 1.5s->30s change from the previous commit. It was raised to "match" the Swift debounce raise, but Android's mechanism is leading-edge with a lockout, not trailing-edge- with-reset like Swift's Combine .debounce — raising the constant would not reproduce the Swift fix's actual coalescing effect, only delay every pass with no benefit, in a module that cannot be compiled or tested in this environment. The change that actually matters (IntelligenceEngine.isAnalyzing + the requestSync guard) is unaffected and still stands. Also records, in docs/superpowers/plans/2026-08-23-sync-rescore-storm.md, a pre-existing race NOT fixed by this branch: analyzeRecent's `computing` lock is set only after two await points (repo.storeHandle(), store.hrFingerprint()), so two close-together callers could both pass the `guard !computing` check before either sets it and both run a full pass concurrently. This predates this branch; the ryanbr#899-A re-arm mechanism already handles a DETECTED collision safely (serializes a re-run rather than corrupting state), so the failure mode is wasted CPU, not a correctness bug. Left for a future session — fixing it properly needs either a non-suspending check-and-set or an actor, more surgery than warranted here. Verified: full iOS build succeeds (xcodegen generate && xcodebuild -scheme NOOPiOS -destination 'generic/platform=iOS' CODE_SIGNING_ALLOWED=NO build). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqUdM3EM8iwNoxmbcKxLLU
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 1, 2026
Two code-review findings, both about the lifecycle of
refreshAfterCompletedBackfill:
- No reentrancy guard. The debounce sink launches an unstructured
Task per lastSyncedAt emission with no cancellation or dedup of a
prior in-flight call. Two overlapping invocations could interleave:
the second hits analyzeRecent's `guard !computing` and returns
immediately, but its `defer { live.analyzing = false }` still
fired while the first invocation was still genuinely computing --
hiding the bar and reopening the exact overlap window
live.analyzing exists to close.
- The 120s backfillWaited poll cap proceeded unconditionally. If a
still-genuinely-running backfill outlived the cap, the function
claimed live.analyzing, called syncProgress.beginAnalyze()
(flipping the bar out of .offload -- further real offload ticks
silently no-op), then hid the bar entirely on return --
misrepresenting a still-in-progress sync as complete. This also
diverged from this plan's own stated design ("hard gate: re-arm
and return"); what shipped was a bounded poll-then-proceed instead.
Fixed both by mirroring the ryanbr#899-A re-arm shape already used in
IntelligenceEngine.analyzeRecent: a `refreshInFlight` guard plus
`pendingPostOffloadRefresh` re-arm flag. A re-entrant call now sets
the flag and returns without touching live.analyzing/the bar at all;
the in-flight call's own defer re-invokes once after it fully
finishes. And: if the 120s cap is hit with live.backfilling still
true, bail out cleanly without claiming the bar -- BLE's own
offload-side finish/timeout paths (previous commit) keep owning its
teardown, and a future lastSyncedAt emission drives a correctly
gated retry through the normal debounce path.
No Kotlin twin: reentrancy/lifecycle plumbing around a Swift-only
progress-bar type.
Verified via build only (no unit-testable path for this reentrancy
timing): `xcodegen generate && xcodebuild -scheme NOOPiOS
-destination 'generic/platform=iOS' CODE_SIGNING_ALLOWED=NO build`
succeeds.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XeGMt5M6bknba8Zm8WFe6L
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 1, 2026
…claims Adds docs/superpowers/plans/2026-08-25-rescore-floor.md, planning the four mechanisms behind the residual measured on 2026-08-25: the never-implemented forced-pass floor, the ryanbr#899-A re-arm chain, the unguarded 30-min cadence tick, and (deliberately left alone, with reasons) the whole-store hrFingerprint() gate. Corrects two claims made in yesterday's entry while recording the log pull, both verified against the commits: - The floor was not silently dropped. 59771a0's commit message defers it openly, together with the skipIfUnchanged fingerprint gate, and names ryanbr#1392 - not ryanbr#1196 - as that gate's blocker. Only the 2026-08-23 plan file's corrections section is silent on it. - 066d562 did not revert a narrowed fingerprint gate. Its diff never touches IntelligenceEngine.swift; it reverted the Kotlin POST_BACKFILL_ANALYZE_DELAY_MS constant. The narrowing was never written. The second correction changes the shape of the remaining work: the windowed hrFingerprint(deviceId:from:to:) the 2026-08-23 plan pointed at is per-device, which is the exact ryanbr#1392 failure, so narrowing that gate needs a new cross-device windowed read in Packages/WhoopStore with a plausibly binding Kotlin twin - not the small reuse the old plan assumed. The plan leaves it alone and says what would have to be verified on-device first. Also records a fifth defect not previously noted: the ryanbr#899-A re-arm at IntelligenceEngine.swift:560 re-invokes without skipIfUnchanged, discarding the post-offload caller's own no-op gate. Verified: python3 Tools/pending-validation.py --list parses the file. No code changed; the plan is not implemented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeGMt5M6bknba8Zm8WFe6L
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 1, 2026
… dropping The re-score storm reduction commits on this branch never got the 15-minute minimum interval between automatic passes the original plan specified for Commit 1 (deferred openly in 59771a0's message, never followed up). A device log pulled 2026-08-25 showed the cost: 5 passes / ~7.9 min re-score CPU in a 44-minute window, versus a <=2 passes / <2 min target, because nothing capped how often an automatic tick or post-offload pass could re-run the same window. New Strand/Data/AnalyzePolicy.swift, shaped like the existing Strand/BLE/BackfillPolicy.swift: a pure enum, an AnalyzeTrigger naming what asked, and a decide() function returning .run or .deferUntil(instant) - deferred, not dropped, since an automatic re-score guards a freshly-synced night's data reaching the dashboard. 900s floor, matching BackfillPolicy.periodicFloorSeconds. .dataChange (every user- or data-driven caller: manual re-score, import, edit, recalibrate, heals) and .background (the BGProcessingTask wake, whose own fingerprint gate is trustworthy while the process is suspended) always bypass. A local-midnight rollover, using the exact AnalyticsEngine.dayString helper analyzeRecent's own scoring loop uses, also always runs, so the first pass after midnight is never floored. IntelligenceEngine.analyzeRecent gains a trigger: AnalyzeTrigger = .dataChange parameter (the default bypasses the floor for every existing caller) and checks the floor FIRST - before the ryanbr#899-A lock guard and before any store read, so a floored trigger costs nothing and never sets pendingForcedRescore. deferredRescoreDueAt publishes the rejected instant. The floor's own watermark (noop.analyze.lastPassEndedAt) is written at completion gated on !Task.isCancelled ONLY, never combined with the stricter !wmKey.isEmpty check the watermark write beside it uses - the same class of bug review finding ryanbr#6 already fixed once for completedPassCount, now avoided here by construction. AppModel wires the post-offload caller (refreshAfterCompletedBackfill) and the 30-minute cadence tick to the floor, and adds a coalesced single-retry Task via a new $deferredRescoreDueAt sink in init() so a floored trigger is never silently lost - it retries once, through refreshAfterCompletedBackfill (inheriting its reentrancy guard and bar teardown), at the deferred instant. The post-offload floor check sits AFTER repo.refresh(days: 120), not before, so a floored pass still surfaces newly-offloaded raw to Trends/streak reads - skipping that would reproduce the ryanbr#1196 flicker shape; only the analyze pass and progress bar are skipped. New StrandTests/AnalyzePolicyTests.swift pins the floor boundary (elapsed >= 900 vs <), the .dataChange/.background bypass, backwards-clock self-healing, and the local-midnight rollover at a non-zero tzOffsetSec. Second of five commits implementing docs/superpowers/plans/2026-08-25-rescore-floor.md. No Kotlin twin: pure iOS scheduling, and Android's analyzeGate is a Mutex with different semantics (blocks-then-runs, not drop-and-rearm) - 066d562 already reverted one attempt to mechanically mirror a Swift scheduling constant into that model. Kotlin cannot be compiled on this machine; nothing was run against it. Verified: full iOS build (xcodegen generate && xcodebuild -scheme NOOPiOS -destination 'generic/platform=iOS' CODE_SIGNING_ALLOWED=NO build) and the macOS test suite (xcodebuild -scheme Strand -destination 'platform=macOS' test, 1221 tests / 0 failures, including all 6 new AnalyzePolicyTests) both pass against the full branch state (this commit plus the two that follow it). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeGMt5M6bknba8Zm8WFe6L
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 1, 2026
…br#899-A re-arm The re-arm added in the previous commit's floor still hardcoded `analyzeRecent(maxDays: maxDays, force: true)` on its re-invoke - no trigger, no skipIfUnchanged. Two costs: the re-invoke always defaulted to `.dataChange`, which always bypasses the floor, so a `.postOffload` re-arm ran IMMEDIATELY regardless of how recently a pass had already scored the same window; and the post-offload caller's own `skipIfUnchanged` no-op gate was silently discarded on every re-arm. Two new fields, pendingForcedRescoreTrigger/pendingForcedRescoreSkipIfUnchanged, record the dropped call's identity at the drop site with a most-privileged- wins merge (a .dataChange drop - heal/import/edit/recalibrate - always overrides an already-pending .postOffload, and forces skipIfUnchanged back to false with it, so a real data event can never be silently downgraded into something the floor is later allowed to defer). The defer's re-invoke now carries these through instead of hardcoding force: true alone. Because the re-invoke runs after the just-completed pass's own lastPassEndedAt write, a carried .postOffload re-arm is now floored by AnalyzePolicy on its own entry and becomes a single deferred retry instead of an immediate second pass. This is what actually collapses the measured 2026-08-25 log's chain - pass 1 (1021s) followed immediately by two more full passes re-scoring the same 7 nights in the next 3 minutes - into one deferred retry. Extends StrandTests/IntelligenceForcedRescoreRearmTests.swift's RearmModel with floor-awareness and the carry, adding 5 new cases: a floored .postOffload trigger still eventually runs exactly one pass (the core no-data-loss claim); a replay of the measured 2026-08-25 timeline that collapses to one deferred pass instead of two immediate ones (fails on the code from the previous commit alone, passes with this one); .dataChange is never floored on re-arm; the most-privileged-wins merge in both orders; and a 5-cycle retry-chain check with strictly increasing deferral instants. The 5 pre-existing test methods are unchanged and still pass unmodified. Third of five commits implementing docs/superpowers/plans/2026-08-25-rescore-floor.md. No Kotlin twin: Android has no shared `computing` lock at all (the forced post-backfill rescore runs on its own ioScope coroutine and is never dropped), so there is nothing to carry - already documented in this test file's header comment. Verified: full iOS build and the macOS test suite (xcodebuild -scheme Strand -destination 'platform=macOS' test, 1221 tests / 0 failures, including all 10 IntelligenceForcedRescoreRearmTests - 5 pre-existing + 5 new) both pass against the full branch state (this commit plus the two that precede and the one that follows it). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeGMt5M6bknba8Zm8WFe6L
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 1, 2026
The commit that targets the owner's actual complaint. "When I open the app in the morning" is a cold-launch pass by construction: dayScanCache is in-memory only and dayScanCacheConfigSig starts "", which never equals a real signature, so the FIRST pass of every process drops the whole cache and re-reads plus re-scores every night. Measured 2026-08-27, backgrounded, 9 nights: 2403s. Commits 1, 1b, 2 and 5 do nothing for that pass. SHAPE: the plan recommended rebuilding each night from banked state and persisting only the cache key. Audited first, and it is not possible losslessly: nightlySkinTempC is read by the fold (:1503, :1532) and nothing banks the raw degC mean - only the baseline-relative 2dp skinTempDevC, nil outright when the baseline is unusable. workouts lose zoneTimePct/avgHRRPct/hrmax/caloriesKJ into WorkoutRow, and bouts overlapping a real workout are never banked at all. Shipped instead: persist the PROJECTION pass 2 consumes. A cached scan has one consumer - on a hit, pass 1 does out.append(cached.scan); continue and performs no read, no score, no side effect. Pass 2 reads all 13 DayScan fields but only 7 of DayResult's 15; the other 8 are read only inside pass 1 before caching, and pass 2 recomputes its own from baselines2 anyway. Restoring them at DayResult's own defaults reproduces what a cache hit hands pass 2 today, byte for byte. Also smaller than serializing DayScan wholesale: two new Codable types, not nine. INVALIDATION - the plan was wrong in both directions, corrected here: - The ".dataChange drops the cache, one line" seam DOES NOT EXIST. removeAll() appears once (:854), gated on the signature alone; AnalyzePolicy is a pure rate-limiter that never touches the cache. - Manual sleep edits and dismissed sleep spans need NO invalidation. Both apply in pass 2 (:1701-1703, :2186), fresh every pass, after the cache. A registry owner change is already in the per-day key. - The one real hole, which the plan missed: the ryanbr#899 banked-sleep heal deletes banked rows whose only route into a later pass is bandSleepStateSamples - a pass-1 read NOT in the per-day key - and its own re-arm would then re-run a pass that reuses the very days it invalidated. Until now that healed by accident on relaunch. Dropped at the delete site, since the heal raises no analyzeRecent call of its own. - Second hole, also not in the plan: the three trace arrays are replayed on a cache HIT (ryanbr#1575), so a persisted scan captured with the Sleep trace on would replay trace lines after it was switched off. The three trace-active booleans now sign the pass: one drop per toggle, which is what ryanbr#1575 intended. STORAGE: Application Support/noop-dayscan-cache.json, version-tagged, isExcludedFromBackup. Not UserDefaults - the two session bands are on a 30s grid (~960 entries/night), so this is hundreds of KB and UserDefaults loads wholesale at launch. Deliberately NOT added to BackupSettings.whitelist, which is opt-in, so a restored .noopbak cannot import another device's cache (precedent: noop.analyzeWatermark, :2372). Any decode failure or version mismatch reads as "no cache" - i.e. the cold pass we already have. No Kotlin twin, as a judgement call rather than a rule: a device-local derived cache with no schema, migration, formula or stored analytics result. It changes WHICH days recompute, never WHAT they compute to, so both platforms still produce identical scores; Android keeps the in-memory-only cache and repeats more work after a relaunch. KOTLIN NOT COMPILED HERE either way - no JDK, no Android SDK, no CI. Verification: - xcodebuild -scheme Strand -destination platform=macOS test -> 1228 tests, 0 failures (1 pre-existing skip: XiaomiImporterIntegrationTests). Includes 7 new DayScanCacheStoreTests: field-by-field round trip of everything the fold reads, an edited session's adjusted onset, the 8 omitted fields pinned at their defaults, version mismatch rejected, corrupt file rejected, save/load including the config signature, and clear. - Packages/StrandAnalytics: swift build clean after the two Codable additions. - App targets compiled (CI does not build them): -scheme Strand macOS and -scheme NOOPiOS -destination id=819D37A3... both BUILD SUCCEEDED. - python3 Tools/doc_comment_lint.py -> OK. - NOT observed on hardware. Expected: pass 1 reuses 8 of 9 nights, so ~300s instead of 2403s. Recorded as claim (c) in PENDING_VALIDATION, along with the two sub-claims no test can reach: the ryanbr#899 heal drop and the trace-toggle drop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013p6RwiHQZaea2aNPh5jySa
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 4, 2026
…e to the scored span A pass cut short mid-scan (a BGProcessingTask cancelled by its expirationHandler) returns a partial `dailies` covering only the newest days the loop reached. persistComputedScores was already contracted for exactly this (ryanbr#1005-STORM, ComputedScoreReconcilePolicy.reconcileFromDay). Two other window-wide reconciles in the same function were not, and both destroy data the pass never re-scored: - The ryanbr#899 sleep heal filtered `healable` on the full oldestDay...newestDay while `keptStarts` — the bank-recency witness it dedupes against — is built only from the days actually scored. So on a truncated pass every older day was deduped with no witness at all, resolving on longest-wins alone. Worse, any drop there clears the whole persisted day-scan cache, so an interrupted pass could delete the checkpoint the next fire needs to make progress. - The detected-workout delete spanned `windowStart...now` while `workoutRows` is built only from `scoredNights`, removing detected workouts across every older day the pass could not re-insert. Both now use `reconcileFromDay`, which is `oldestDay` on a complete pass — so the normal path is byte-identical and only the truncated path changes. The workout delete takes timestamps rather than day keys, so it maps `reconcileFromDay` back onto the scan loop's own geometry (newest-first from nowLocalMidnight, per-day `from` of dayStart - 30h) to get the earliest instant the pass could have touched. Prerequisite for the incremental day-scan checkpointing that follows: without the heal contraction, a truncated pass can wipe the cache it just checkpointed. Verification: macOS Strand builds clean. The truncated path is only reachable from a real BGProcessingTask cancellation on device, so behaviour there is tracked in docs/PENDING_VALIDATION.md rather than asserted here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 4, 2026
…fires accumulate The end-of-pass save persists "once per pass, never per day" -- correct for a pass that finishes, worthless for one that doesn't. A BGProcessingTask fire that iOS terminates WITHOUT cancelling unwinds nothing and persists nothing, so every fire re-scanned the same days from scratch and discarded the result. That is the 2026-09-04 pattern on device 819D37A3: 11 granted background windows, no outcome recorded, no expiry, no crash, and analyze.lastPassEndedAt frozen for 12 hours. Now a freshly-scored day can checkpoint the cache from inside the detached scan loop, so fragmented fires ACCUMULATE -- the next one finds those days cached and reaches further back, instead of restarting. Safe to write a partial map: DayScanCacheStore.load() validates every entry independently against its own per-day key, and dayCacheConfigSig is fixed for the whole loop, so any prefix is mutually consistent -- a partial file can only hold FEWER days than a complete one, never claim more. It is also un-pruned (the window filter runs after the loop), which the next pass fixes on load. Writes are already .atomic, so no torn file. Throttled at 5 s AND gated on a day having been scored fresh: a cache-hit day continues above and its entry is already on disk byte-identical, so checkpointing it would re-encode the whole envelope for nothing. That matters because the write is O(window), not O(1) -- it re-projects every entry and encodes ~407 KB. Timed into the existing cost line (checkpoints=N totalling Nms) so the interval gets tuned against a device number rather than by feel. Depends on the preceding commit: without the ryanbr#899 heal contraction, a truncated pass could clear the very cache this banks. Verification: macOS Strand builds clean. The interrupted path only occurs on a real device termination, so the accumulate-across-fires claim is tracked in docs/PENDING_VALIDATION.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 4, 2026
The ryanbr#899-A re-arm spawns an unstructured Task {} that inherits no cancellation, and its carried trigger falls back to .dataChange -- which AnalyzePolicy.decide never floors. So a background fire whose ryanbr#899 heal set pendingForcedRescore could spawn a second, full-width, completely unbudgeted pass inside the same short window that had already failed to finish the first one, on the exact path this branch exists to keep cheap. The existing !Task.isCancelled guard only covers the deliberate-cancellation race, not this: the heal sets the flag from inside a background pass that is still running normally. Dropping the re-arm for trigger == .background loses nothing, because the next BGProcessingTask fire already IS the successor -- SyncAnalyzeBackgroundScheduler arms one before the work starts, precisely so an interruption can't leave the path unscheduled. Foreground triggers are untouched. Verification: macOS Strand builds clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
xxblyxx
added a commit
to xxblyxx/noop
that referenced
this pull request
Sep 7, 2026
First overnight on this branch found the hole. Three cpu_resource_fatal kills (23:42, 00:12, 04:12) while background fires were ALSO persisting — lastScorePersistAt 04:12:48, nine seconds before the 04:12:57 kill. The watchdog predicate only asked "did anything persist within 6h", so a fire that persists and then dies read as healthy, the bite never shrank past 1, and the persist-only bite 0 (~29s CPU, which does fit inside the ~48s allowance) was never reached. Adds `killsSinceBiteChange` to FireHealth and checks it FIRST: a dead fire is evidence about the bite regardless of whether it managed to persist on the way down. The app cannot read its own crash reports, so a kill is inferred the same way debugLines() already documents — lastStageAt newer than lastOutcomeAt means the fire reached a stage and never recorded an outcome. Counted once per dead fire via lastCountedDeadFireAt, because BLE state restoration relaunches the app constantly and every launch runs the watchdog. Reset when the bite changes, so each bite is judged on its own kills. Still rate-limited and still behind the cold-start guard — both pinned by tests. Also records the full overnight result in PENDING_VALIDATION, including the measurement this was all for: per-night duty=1.00-1.02 (the pass is CPU-bound, not I/O-bound) and an e-core multiplier of 2.6-3.6x, so one night (28-40s CPU) plus pass 2 (28.9s CPU) is ~57s against ~48s allowed. The "may still not fit" risk that entry named landed on the failing side. Sleep is still truncated ~4h. And the bug that probably matters more than any of this, recorded but NOT fixed here because its root cause is not yet identified: the day-scan cache collapsed to a single entry and stays there (on-disk noop-dayscan-cache.json is 18KB holding 1 entry where it should be ~407KB holding ~20), so every fire re-scores today COLD. That is why a night costs 28-40s instead of near-zero. The ryanbr#899 banked-sleep heal is ruled out by evidence — all three overnight Dedup(ryanbr#899) lines report `removed 0`, and its clear is gated on removals being non-empty. Verification: macOS + NOOPiOS build; BackgroundAnalyzeSchedulePolicyTests 40/40, including that a kill shrinks the bite while persists look healthy (the exact 09-06→07 shape), that the kill path stays rate-limited, and that a fresh install still does nothing. Not verifiable here: whether bite 0 actually survives a night — needs the device. No Kotlin twin: iOS BGTask scheduling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sCyGB9d3tpHyEYfSrq4wt
5 tasks done
pipiche38
added a commit
to pipiche38/noop
that referenced
this pull request
Sep 22, 2026
…mputed id only, never to a ring's own rows The post-upsert heal ran `dedupe(healable, freshStarts: keptStarts)` for every id in `healDeviceIds` (ryanbr#1248), including an Oura ring's own. `keptStarts` is the computed pass's bank witness, and the comment claimed it "only matches the computedId rows" - false on an Oura day, where the pass's sessions ARE the ring's `providedSleep` rows with `startTs` copied verbatim. The ring row the pass had READ was therefore ranked "fresh" in the ring's own sweep and outranked every fuller re-serve the ring banked while the pass was in flight (hours on a phone, where the OS suspends the app between the read and the heal and wakes it precisely for the drain that persists the fuller row). On 2026-09-19/20 the heal deleted the 598-min full night (22:23 -> 08:21) one second after it landed and kept the 337-min row read at 04:14; the day scored 315 min ending 04:48. `SleepSessionDedup.healWitness` is now the one shared rule: the witness goes to the computedId sweep only; every other id gets none and falls back to longest-wins, the read-side default. The heal's existing re-arm re-pass then scores the kept full night. Kotlin twin case-for-case; no schema change and no behaviour change for a WHOOP strap, whose rows live only under computedId. Tests: SleepSessionDedupTests (Swift 33/33) and SleepSessionDedupTest (Kotlin 33/33) gain the witness rule and the 09-19/20 scenario, with the leaked-witness outcome pinned so it cannot creep back. macOS `Strand` target compiled locally. Refs ryanbr#899, ryanbr#1248, ryanbr#1284. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcXLwCpyu7skJQcfYAoDzi
ryanbr
pushed a commit
to pipiche38/noop
that referenced
this pull request
Sep 23, 2026
…mputed id only, never to a ring's own rows The post-upsert heal ran `dedupe(healable, freshStarts: keptStarts)` for every id in `healDeviceIds` (ryanbr#1248), including an Oura ring's own. `keptStarts` is the computed pass's bank witness, and the comment claimed it "only matches the computedId rows" - false on an Oura day, where the pass's sessions ARE the ring's `providedSleep` rows with `startTs` copied verbatim. The ring row the pass had READ was therefore ranked "fresh" in the ring's own sweep and outranked every fuller re-serve the ring banked while the pass was in flight (hours on a phone, where the OS suspends the app between the read and the heal and wakes it precisely for the drain that persists the fuller row). On 2026-09-19/20 the heal deleted the 598-min full night (22:23 -> 08:21) one second after it landed and kept the 337-min row read at 04:14; the day scored 315 min ending 04:48. `SleepSessionDedup.healWitness` is now the one shared rule: the witness goes to the computedId sweep only; every other id gets none and falls back to longest-wins, the read-side default. The heal's existing re-arm re-pass then scores the kept full night. Kotlin twin case-for-case; no schema change and no behaviour change for a WHOOP strap, whose rows live only under computedId. Tests: SleepSessionDedupTests (Swift 33/33) and SleepSessionDedupTest (Kotlin 33/33) gain the witness rule and the 09-19/20 scenario, with the leaked-witness outcome pinned so it cannot creep back. macOS `Strand` target compiled locally. Refs ryanbr#899, ryanbr#1248, ryanbr#1284. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcXLwCpyu7skJQcfYAoDzi
ryanbr
pushed a commit
that referenced
this pull request
Sep 23, 2026
…nly (#2372) An Oura night displayed as ending 04:48 when the wearer woke at 08:18. The ring had served the full night, the strap log shows it persisted [22:23 -> 08:21], and the #899 heal deleted it one second later, keeping a 337-minute row read hours earlier. The post-upsert heal ran dedupe(healable, freshStarts: keptStarts) for every id in healDeviceIds, on a comment claiming the witness "only matches the computedId rows". That is false on a day scored from a device-provided hypnogram: the pass's sessions ARE the ring's own rows with startTs copied verbatim, so the same keys name the row the pass READ in the ring's own table. In the ring's sweep that row ranks fresh and beats longest, so the heal preferred whatever the pass had consumed at read time over anything the ring banked afterwards. On a phone the window is hours wide, because iOS suspends the app between a pass's read and its heal and wakes it for the very drain that banks the fuller re-serve. It is self-perpetuating: the next pass reads the survivor, re-witnesses it, and deletes the next fuller row. SleepSessionDedup.healWitness is now the one shared rule on both platforms: the witness goes to the computedId sweep alone, every other id falls back to longest-wins, which the #1284 corpus already showed is the WHOOP-matching row on every night. No schema change, and no behaviour change for a strap, whose rows live only under computedId. Reviewed. Both call sites take the rule, no raw keptStarts survives, and the false comment was corrected in place rather than left to mislead the next reader. The regression outcome is pinned beside the fixed one in the same test, so a leaked witness cannot creep back unnoticed. The change moves a DELETION path strictly toward caution: healing too little leaves duplicate rows that the read-side dedup already collapses, healing too much deletes a night. Rebased onto current main and re-verified there: SleepSessionDedupTest 33/33 on Android, doc-comment lint clean, 16/16 in CI including test (WhoopStore), which is the only run of the Swift half (this host cannot link GRDB against system sqlite3, though all 366 targets compile). Hardware confirmation is still owed: one night on a ring with the pass suspended across a drain, where the strap log should read dropped min=<shorter> kept min=<longer> after a wake. Author: Pipiche (@pipiche38). Refs #899, #1248, #1284.
1 task done
5 tasks done
4 tasks done
laurence-cw
added a commit
to laurence-cw/loop
that referenced
this pull request
Oct 6, 2026
…history repair re-arms an ordinary window Two field failures, one each on the two phones this fork runs on. WHOOP 5/MG (strap offloads every few minutes): MetricKit reported 356 background exits for the CPU limit in one day and 12 h 55 min of CPU in 13 h backgrounded. Every background process was killed about 3.5 min after its bluetooth relaunch, mid-pass, so no pass finished and last night never appeared. The pass was paced to ~50% CPU; the offload's own decode and inserts ran beside it, and together they crossed iOS's 80%-over-60 s limit. Rest three seconds per second of work (~25%). WHOOP 4.0: the one-time sleep-wear history repair (4000 days) took 4.7 h. An offload landing mid-pass re-armed it at the same width with its completion callback, and the callback was withheld while any re-arm was pending, so the repair ran back to back and never marked itself done, holding the lock while last night waited. Only the ryanbr#899 heal re-arm needs the same width now (pendingRearmSameWidth); any other trigger during a repair re-arms an ordinary 21-day pass, and the repair delivers its completion. The recent-first pass also runs ahead of a history repair, so the newest nights land in minutes rather than after the repair. Tests: pacing constants and rest arithmetic updated; a repair with a trigger injected mid-pass completes once and is followed by one ordinary pass; recent-first decision table. Full StrandTests: 2282 run, 0 failures apart from UnescalatedWorkTests, which passes 3/3 alone (task-priority timing; touched nothing here). Swift only, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Oct 30, 2026
…lanes updateSleepStages (hand-edited rows only), deleteSleepSessionRowOnly (the ryanbr#899/ryanbr#1248 duplicate heal) and healImplausibleTimestamps changed lane rows in Room without mirroring them, so the core kept stale stages and purged rows. They now mirror like the other lane writes; the prune reads the affected lane keys with the DAO's own predicates before deleting. Analysis migration required: no
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Oct 30, 2026
The daily step checked `pendingAnalysis != nil` after `analyzeRecent` returned, to run the ryanbr#899 heal re-pass before the later steps. That check was always false: `runAnalysisQueue` resumes a pass's waiters and dequeues the follow-up in the same synchronous stretch, so by the time the rebuild task resumed the re-pass was already running with nothing queued, and the weekly scores, stress proxy and the rest ran beside it. The step now waits for the queue to drain (`waitForAnalysisQueueToDrain`). The step also stays owed when a pass gave up without scoring (no store, no metric config, a failed day-cycle read or sleep write) instead of being recorded done. A rebuild stopped between its last step's cursor write and the recipe cursor's restarted from the beginning: the position after `.publish` read as no position at all. `firstOwedStep` now answers nil there, and the run only writes the recipe cursor. Analysis migration required: no
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #894, the divergence the #891 fixtures surfaced in #893.
A
COMMAND_RESPONSEcarries three things: which command it answers, the origin-seq echo, and the result code. Kotlin publishes all three — on 5/MG since the port, on 4.0 since #791. Swift published only the first, on either family. An Apple strap log named the command and then said nothing about whether it succeeded.Why it is more than a display fix
The result code is the accept/reject signal.
UNSUPPORTED(3)is how the 5/MG haptics rejection was identified (#48),FAILURE(0)on an empty extended-battery reply is the entire finding in #791, andSUCCESS(1)is the evidence column in #891.Because the interpreter never exposed it, every Swift probe that needed the answer re-derived the byte privately:
FeatureFlagProbe.resultLabel,DeviceConfigReadProbevia that, andBodyLocationProbe's own "Result code @12". Three copies of one two-line decode, and any new probe needed a fourth.Post-hooks, not schema fields
Worth recording, because the smaller diff was wrong. Swift already gets
resp_cmdfrom the schema field spec atoff: 6, shifted by+4for 5/MG — so addingresp_seq/resultatoff: 7/8would have been three lines of JSON and would have picked up both families' offsets for free.It fails open badly, though. The schema-field loop calls
readDTypeat a raw frame offset with no payload bound, so a response whose declared payload ends before offset 8 would report its CRC32 trailer bytes as a result code. Both post-hooks already compute the bounded payload slice using exactly the guard Kotlin uses (pay.count >= 2), so the fields are read from there instead and a short reply decodes neither.CommandResultdoes go in the shared schema, soenumNamerendersSUCCESS(1)and falls back to0x7F(127)— byte-identical to Kotlin'scommandResultLabel/hexLabel. That is what makes logs from the two platforms comparable rather than merely both non-empty.Tests
Whoop4ResponseResultTestsis the Swift twin of the Kotlin file of the same name and reuses its real #791 captures byte for byte, so the two suites assert the same decode of the same frames — including the duplicate-write case where oneGET_DATA_RANGEsend produced three CRC-valid responses sharing one origin-seq echo while the strap-side seq advanced.Two new edge cases: a declared payload ending before the result byte decodes neither field, and an undocumented code
0x7Fstays hex rather than becoming an invented name.The #891 MG toggle fixtures now assert
resp_seqandresulttoo — what #893 had to leave out because this decoder did not produce them. The battery fixture additionally pins that the new fields did not shift the payload offsets its independently-known 47% is read from.What updating the golden surfaced
The Python-golden parity corpus has one
COMMAND_RESPONSEentry, which gains the two fields. Doing that turned up a counterexample to the #791 mapping itself.That frame is a real 4.0
GET_BATTERY_LEVELreply carrying a valid 42.5% — it plainly succeeded — and itspayload[1]is0x00, so it now readsFAILURE(0). Kotlin's ownFramingTestbattery fixture (91.2%) has the same00 00prefix, and Android has rendered it that way since #791. So this is upstream of this change rather than introduced by it, and matching the twin is the point here.Filed as #900 with the candidate explanations, and asserted meanwhile in
testSuccessfulBatteryReadStillReportsResultZero— the behaviour is deliberate, and a future correction has to come past a test instead of silently changing a log.Why
resp_cmdis not bounded the same wayThe obvious question, since it sits two lines above.
resp_cmdcomes from the generic schema-field loop, which reads at a raw frame offset — so a CRC-valid frame whose declared length stops before offset 6 decodes a CRC32 byte as the command. Reachable, and confirmed with a crafted frame.Fixing it is one line, but it is in the loop every packet type shares (
EVENT'sevent@6,METADATA'smeta_type@6), so it changes decode behaviour beyond command responses. Filed as #901 rather than folded in here.Verification
@7/@8, 5/MG@11/@12).json.dumps(indent=0)round-trips the file exactly), so the diff is 21 lines on one entry rather than a reflow.fb.addpopulatesparsedon both the fast and full paths, soFastPathParityTests'fast.parsed == full.parsedcontract still holds; the per-fieldrawcontract holds by construction.libc6-dev), so the suite runs in CI.Follow-on left out deliberately: collapsing the three probe-local
resultLabelhelpers onto the new field is now possible but is a separate concern.