Conversation
Closes #134 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@matin has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds HeartRateReading and DailyHeartRate data models, exposes DailyHeartRate in package exports, implements API fetch/listing and readings conversion, and adds tests covering get, readings, and list behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant API as Garmin API
participant Converter as camel_to_snake_dict
participant Model as DailyHeartRate
Client->>API: GET /wellness-service/wellness/dailyHeartRate/?date={day}
API-->>Client: JSON response (camelCase keys)
Client->>Converter: Convert response keys to snake_case
Converter-->>Client: snake_case dict
Client->>Model: Instantiate DailyHeartRate(**dict)
Model-->>Client: DailyHeartRate instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @tests/data/test_heart_rate.py:
- Around line 29-37: The VCR cassette only contains a single date but the test
calls DailyHeartRate.list(end="2026-01-07", days=3) which requests three dates;
either change the test to request only the recorded date (e.g., set days=1 on
the DailyHeartRate.list call) or re-record the cassette to include the three
dates by running the test with VCR in record mode (e.g., use
--vcr-record=new_episodes) so the recorded interactions match the requests.
🧹 Nitpick comments (2)
src/garth/data/heart_rate.py (2)
22-50: Consider more precise typing forheart_rate_values.The type
list[list[int | None]]suggests a list of arbitrary-length integer lists, but the actual structure is[[timestamp_ms, heart_rate], ...](pairs). A more precise type likelist[tuple[int | None, int | None]]would better document the expected structure and could help catch misuse.That said, this matches the raw API response shape so keeping it flexible is reasonable.
67-80: Consider replacingassertwith an explicit exception for production safety.
assertstatements are removed when Python runs with optimization flags (-Oor-OO). If the API returns an unexpected type in production with optimizations enabled, the code would fail with a confusingTypeErrorfromcamel_to_snake_dictinstead of a clear message.♻️ Suggested fix
if not hr_data: return None - assert isinstance(hr_data, dict), ( - f"Expected dict from {path}, got {type(hr_data).__name__}" - ) + if not isinstance(hr_data, dict): + raise TypeError( + f"Expected dict from {path}, got {type(hr_data).__name__}" + ) hr_data = camel_to_snake_dict(hr_data) return cls(**hr_data)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/data/cassettes/test_daily_heart_rate_get.yamlis excluded by!tests/**/cassettes/**tests/data/cassettes/test_daily_heart_rate_list.yamlis excluded by!tests/**/cassettes/**tests/data/cassettes/test_daily_heart_rate_readings.yamlis excluded by!tests/**/cassettes/**
📒 Files selected for processing (4)
src/garth/__init__.pysrc/garth/data/__init__.pysrc/garth/data/heart_rate.pytests/data/test_heart_rate.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Usemake formatto auto-format Python source files using ruff
Usemake lintto lint Python source files (ruff format check, ruff check, mypy)
Files:
src/garth/__init__.pysrc/garth/data/__init__.pysrc/garth/data/heart_rate.pytests/data/test_heart_rate.py
**/__init__.py
📄 CodeRabbit inference engine (CLAUDE.md)
Provide the main client instance as
garth.clientfor direct API access
Files:
src/garth/__init__.pysrc/garth/data/__init__.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest with VCR cassettes for HTTP recording/playback in tests
Maintain comprehensive test coverage across all modules with separate test directories mirroring source structure
Files:
tests/data/test_heart_rate.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: - test functions shouldn't have a return type hint
- it's ok to use
assertinstead ofpytest.assume()
Files:
tests/data/test_heart_rate.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: matin/garth PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-08T00:25:14.763Z
Learning: Applies to data/**/*.py : Raw data retrieval from Garmin Connect API should be organized in the `data/` module with submodules for specific data types (body_battery/, hrv.py, sleep.py, weight.py)
📚 Learning: 2026-01-08T00:25:14.763Z
Learnt from: CR
Repo: matin/garth PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-08T00:25:14.763Z
Learning: Applies to data/**/*.py : Raw data retrieval from Garmin Connect API should be organized in the `data/` module with submodules for specific data types (body_battery/, hrv.py, sleep.py, weight.py)
Applied to files:
src/garth/__init__.pysrc/garth/data/__init__.pysrc/garth/data/heart_rate.py
🧬 Code graph analysis (3)
src/garth/__init__.py (1)
src/garth/data/heart_rate.py (1)
DailyHeartRate(23-85)
src/garth/data/__init__.py (1)
src/garth/data/heart_rate.py (1)
DailyHeartRate(23-85)
src/garth/data/heart_rate.py (3)
src/garth/utils.py (1)
camel_to_snake_dict(17-33)src/garth/data/_base.py (1)
Data(15-47)src/garth/http.py (2)
Client(19-245)connectapi(186-192)
🪛 GitHub Actions: CI
tests/data/test_heart_rate.py
[error] 1-1: VCR cassette mismatch: CannotOverwriteExistingCassetteException: Can't overwrite existing cassette ('/home/runner/work/garth/garth/tests/data/cassettes/test_daily_heart_rate_list.yaml') in your current record mode ('none'). No match for the request (GET https://connectapi.garmin.com/wellness-service/wellness/dailyHeartRate/?date=2026-01-06). Found 1 similar request with 1 different matcher(s): date mismatch (expected 2026-01-07, got 2026-01-06).
🔇 Additional comments (7)
src/garth/data/heart_rate.py (3)
14-19: LGTM!Clean dataclass definition for individual heart rate readings with appropriate fields.
52-65: LGTM!The
readingsproperty correctly filters out None values and converts millisecond timestamps to UTC datetimes. The list comprehension is clean and follows Pythonic patterns.
82-85: LGTM!The
listoverride correctly delegates to the parent class and adds sorting bycalendar_date, which is a sensible default for time-series data.src/garth/__init__.py (1)
5-5: LGTM!
DailyHeartRateis correctly exported and added to__all__in alphabetical order, consistent with the existing pattern for other data classes.Also applies to: 37-37
tests/data/test_heart_rate.py (2)
7-14: LGTM!Test validates the key fields returned by
DailyHeartRate.get()with sensible assertions. The VCR marker ensures reproducible HTTP interactions.
17-26: LGTM!Good test coverage for the
readingsproperty, validating both the presence of readings and the structure of individualHeartRateReadingobjects.src/garth/data/__init__.py (1)
7-7: LGTM!
DailyHeartRateis correctly exported from thedatasubpackage, following the established pattern for other data types. This aligns with the project structure where raw data retrieval from Garmin Connect API is organized in thedata/module. Based on learnings.Also applies to: 27-27
Use max_workers=1 to ensure sequential requests are properly recorded by VCR for the list test. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #156 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 58 60 +2
Lines 2603 2672 +69
=========================================
+ Hits 2603 2672 +69
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
DailyHeartRatedata class for fetching daily heart rate metrics from Garmin ConnectHeartRateReadingdataclass for individual heart rate samples with timestampsget()for single day andlist()for date rangesreadingsproperty to convert raw heart rate values to typed objectsCloses #134
Test plan
get(),list(), andreadingsproperty🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.