Fix hrv

[felipao-mx]

• make some HRV/Body Battery data fields optional. It addresses the case when there is no HRV data for that specific period of time. Maybe the user wasn't wearing the watch

Summary by CodeRabbit

• Bug Fixes

  • Stricter handling of body battery readings to skip entries with missing status/level and to return readings in chronological order.

• Improvements

  • Made several HRV fields optional to better handle incomplete or partial data.

• Tests

  • Added a test for incomplete body battery data and adjusted an existing test to use deterministic concurrency.

• Chores

  • Bumped package version.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Unpacks and stricter-validates body battery reading tuples and explicitly returns readings sorted by timestamp; makes several HRV model fields optional with defaults; adds/adjusts tests for body battery list/get and bumps package version to 0.5.20.

Changes

Cohort / File(s)	Summary
Body battery readings parsing src/garth/data/body_battery/readings.py	Unpacks four values per entry into timestamp, status, level, version; skips entries when level or status is None; constructs BodyBatteryReading from unpacked vars and returns readings sorted by timestamp.
HRV data model src/garth/data/hrv.py	Makes HRVSummary.last_night_avg and last_night_5_min_high optional with default None; makes HRVData sleep timestamp fields (sleep_start_timestamp_gmt, sleep_end_timestamp_gmt, sleep_start_timestamp_local, sleep_end_timestamp_local) optional with default None.
Version bump src/garth/version.py	Updates __version__ from "0.5.19" to "0.5.20".
Tests — body battery tests/data/test_body_battery_data.py	Adds test_daily_body_battery_stress_get_incomplete_data to assert readings' level and status are non-None; updates test_daily_body_battery_stress_list to call DailyBodyBatteryStress.list(..., max_workers=1) and a minor formatting tweak.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Confirm the None-only guard doesn't drop valid falsy values (e.g., 0) that should be kept.
• Verify timestamps are always present and comparable before sorting to avoid exceptions.
• Check callers of HRV models handle newly-optional timestamp and summary fields.
• Ensure tests' use of max_workers=1 matches the DailyBodyBatteryStress.list signature and concurrency semantics.

Possibly related PRs

• Add Body Battery Support #116 — Adjusts body battery reading parsing; modifies the same parsing logic in src/garth/data/body_battery/readings.py.

Suggested reviewers

• matin

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'Fix hrv' is vague and doesn't clearly convey the main change. The PR also modifies body battery readings and version, but the title only hints at HRV without explaining what was actually fixed.	Use a more descriptive title like 'Make HRV and Body Battery fields optional' or 'Handle missing HRV and Body Battery data' to better reflect all changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-hrv

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (81f712a) to head (5cafd95).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #152   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           45        45           
  Lines         1896      1905    +9     
=========================================
+ Hits          1896      1905    +9     

Flag	Coverage Δ
unittests	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 81f712a and 02ea59a.

📒 Files selected for processing (3)

• src/garth/data/body_battery/readings.py (1 hunks)
• src/garth/data/hrv.py (2 hunks)
• src/garth/version.py (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: test 3.12

🔇 Additional comments (4)

src/garth/version.py (1)

1-1: LGTM! Version bump aligns with the PR changes.

The version increment from 0.5.19 to 0.5.20 is appropriate for this patch-level change that makes data fields optional.

src/garth/data/body_battery/readings.py (1)

37-40: LGTM! Using unpacked variables improves readability.

Replacing direct indexing with named variables makes the code clearer and more maintainable.

src/garth/data/hrv.py (2)

47-50: Verify downstream code handles optional sleep timestamp fields.

Making sleep timestamp fields optional with default None appropriately handles cases where sleep data is unavailable (e.g., user not wearing the watch). Ensure that any code accessing sleep_start_timestamp_gmt, sleep_end_timestamp_gmt, sleep_start_timestamp_local, or sleep_end_timestamp_local fields includes proper None checks before performing date/time operations.

---

27-28: Good change to handle missing HRV data gracefully.

Making last_night_avg and last_night_5_min_high optional with default None in HRVSummary aligns with the PR objective to prevent failures when HRV measurements are unavailable. The optional type hints (int | None) ensure type checkers will enforce proper None handling in any downstream code that accesses these fields.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02ea59a and 70154b2.

⛔ Files ignored due to path filters (1)

• tests/data/cassettes/test_daily_body_battery_stress_get_incomplete_data.yaml is excluded by !tests/**/cassettes/**

📒 Files selected for processing (2)

• src/garth/data/body_battery/readings.py (1 hunks)
• tests/data/test_body_battery_data.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

tests/**

⚙️ CodeRabbit configuration file

tests/**: - test functions shouldn't have a return type hint

• it's ok to use assert instead of pytest.assume()

Files:

• tests/data/test_body_battery_data.py

🧬 Code graph analysis (1)

tests/data/test_body_battery_data.py (1)

src/garth/data/body_battery/daily_stress.py (3)

• DailyBodyBatteryStress (20-90)
• get (73-90)
• body_battery_readings (37-39)

🪛 GitHub Actions: CI

tests/data/test_body_battery_data.py

[error] 121-122: Item 'None' of 'DailyBodyBatteryStress | None' has no attribute 'body_battery_readings'.

🔇 Additional comments (4)

src/garth/data/body_battery/readings.py (3)

32-34: LGTM! Past review feedback addressed correctly.

The unpacking with *_ now handles arrays with more than 4 elements, and the explicit None checks prevent incorrectly skipping readings with level=0 or empty-string status.

---

37-40: LGTM! Cleaner usage of unpacked variables.

Using the unpacked variables instead of array indices improves readability and maintainability.

---

44-44: LGTM! Explicit sorting ensures chronological order.

The explicit return statement makes the sorting behavior clear and ensures readings are always in chronological order.

tests/data/test_body_battery_data.py (1)

130-131: LGTM! Sequential execution for VCR compatibility.

Setting max_workers=1 appropriately prevents VCR issues with concurrent requests during testing.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/data/test_body_battery_data.py (1)

118-124: Consider using more explicit None check for clarity.

The test correctly addresses the previous review comment by checking that daily_data is not None before accessing its attributes. However, assert daily_data could be made more explicit as assert daily_data is not None to improve readability and make the intent clearer to both developers and type checkers.

🔎 Suggested refinement

 @pytest.mark.vcr
 def test_daily_body_battery_stress_get_incomplete_data(authed_client: Client):
     daily_data = DailyBodyBatteryStress.get("2025-12-18", client=authed_client)
-    assert daily_data
+    assert daily_data is not None, "Expected data for 2025-12-18"
     assert all(r.level is not None for r in daily_data.body_battery_readings)
     assert all(r.status is not None for r in daily_data.body_battery_readings)

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70154b2 and 5cafd95.

📒 Files selected for processing (1)

• tests/data/test_body_battery_data.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

tests/**

⚙️ CodeRabbit configuration file

tests/**: - test functions shouldn't have a return type hint

• it's ok to use assert instead of pytest.assume()

Files:

• tests/data/test_body_battery_data.py

🧬 Code graph analysis (1)

tests/data/test_body_battery_data.py (1)

src/garth/data/body_battery/daily_stress.py (2)

• get (73-90)
• body_battery_readings (37-39)

[matin]

looks good. feel free to merge and release