Fix/live decode regressions#14
Closed
abdulsaheel wants to merge 2 commits into
Closed
Conversation
Three uncommitted tweaks had crept in without justification or test coverage; reverted all three back to known-good behavior and left a comment at each site so they don't get silently re-applied: - wristOn: skinContact>50 broke the parity suite (2906/2934, all 28 misses on wrist_on) — 28 real v24 records show hr=87-97 (clearly worn) with skinContact<=50, proving skinContact is contact quality, not wear, exactly as already documented. Back to hr>0, which matches the oracle on every case. - parseRealtimeHr's RR sanity clamp (200-2500ms = 24-300bpm) had been relaxed to `v>0`, letting corrupted/out-of-range timing values straight into live RR with no guard. - _r10Motion's detrend window had been widened 9->25 with nothing backing it; ±25 approaches the 7-40 sample autocorrelation lag range it feeds, risking attenuation of genuine step periodicity. No test (parity or otherwise) exercises steps_inc/activity from R10, so there was no way to validate the change either way. dart test: 71/71 passing, including the full 2934-case parity suite.
Was the original reference the Dart decoders were ported from and kept in parity with (7 commits, right up through the latest feature work), not a one-off leftover — but decode_parity_cases.json is now the frozen, self-contained oracle dart test actually runs against, with no runtime dependency on ts/ (verified: no test/config spawns node/tsc against it). Drops the unported v25-handling WIP that was sitting in ts/live.ts uncommitted; that work is not carried over to live.dart.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (3)
📝 WalkthroughWalkthroughTypeScript live decoding utilities, the R24 record decoder, and their test harness were removed. Dart decoder comments were expanded to document RR bounds, R10 parameter history, and contact-quality interpretation without changing runtime behavior. ChangesDecoder cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
1 task
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
1 participant
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.
This pull request focuses on restoring and clarifying several physiological and signal-processing gates in both the Dart and TypeScript codebases for WHOOP record decoding. The main goal is to revert prior relaxations that allowed out-of-range or corrupted data to pass through, and to improve in-code documentation about these physiological checks. Additionally, the TypeScript test file is removed, likely to streamline the codebase or transition to a different testing approach.
The most important changes are:
Restored physiological/data quality gates:
lib/src/control.dart(parseRealtimeHr): Restored the RR interval gate to only accept values between 200–2500ms, preventing corrupted or out-of-range timing data from being included in live RR streams. Previous relaxations tov>0are reverted, with clear comments explaining the rationale.lib/src/live.dart(_Motion _r10Motion): The detrending window width for step detection is reverted from 25 back to 9, with a detailed comment explaining the risk of attenuating genuine step periodicity at wider windows and the lack of test coverage for the previous change.Documentation and signal meaning clarifications:
lib/src/live.dart(decodeRecord): Added a clarifying comment thatskinContactis a quality metric, not a wear detection signal, and thathr>0is the only reliable indicator for "wrist on" in the current fixture.Test and code cleanup:
ts/test_decoder.tsfile, which contained a test suite for the TypeScript R24 decoder. This likely reflects a shift in testing strategy or a move to a different framework.No changes to the public TypeScript decoder API or core parsing logic were made.
These changes improve the physiological reliability of the decoded data and clarify the codebase for future maintainers.
Summary by CodeRabbit
Refactor
Documentation
Bug Fixes