Conversation
Unlike MorningTrainingReadinessData which filters for morning snapshots only, TrainingReadinessData returns all entries including post-exercise updates and realtime variable updates. Closes #168 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 24 minutes and 43 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 (2)
WalkthroughIntroduces TrainingReadinessData, a new data model for training readiness metrics accessible via the Changes
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #169 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 60 62 +2
Lines 2729 2807 +78
=========================================
+ Hits 2729 2807 +78
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:
|
- Make input_context nullable to handle API responses with null values - Use future date (2030-01-01) for empty response test - Update test assertions to be more flexible Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/data/test_training_readiness.py (1)
9-32: Test coverage looks good.The test properly validates:
- Non-empty response for a valid date
- All entries match the requested calendar_date
- Presence of morning context ("AFTER_WAKEUP_RESET")
- Field values on the morning entry
- Empty response handling for far-future dates
Optional: More defensive retrieval of morning entry
While line 18 already asserts "AFTER_WAKEUP_RESET" exists, using
next()with a default would make the code slightly more robust:- morning_entry = next( - e for e in entries if e.input_context == "AFTER_WAKEUP_RESET" - ) + morning_entry = next( + (e for e in entries if e.input_context == "AFTER_WAKEUP_RESET"), + None + ) + assert morning_entry is not NoneThis prevents potential
StopIterationexceptions if the assertion logic changes later.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
tests/data/cassettes/test_morning_training_readiness_data_get.yamlis excluded by!tests/**/cassettes/**tests/data/cassettes/test_morning_training_readiness_data_list.yamlis excluded by!tests/**/cassettes/**tests/data/cassettes/test_training_readiness_data_get.yamlis excluded by!tests/**/cassettes/**tests/data/cassettes/test_training_readiness_data_list.yamlis excluded by!tests/**/cassettes/**
📒 Files selected for processing (5)
docs/api/scores.mdsrc/garth/__init__.pysrc/garth/data/__init__.pysrc/garth/data/training_readiness.pytests/data/test_training_readiness.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Auto-format Python source files using ruff viamake format
Lint Python source files using ruff format check, ruff check, and mypy viamake lint
Methods that make API calls should return Pydantic dataclasses of the response for consistency
Files:
tests/data/test_training_readiness.pysrc/garth/data/__init__.pysrc/garth/data/training_readiness.pysrc/garth/__init__.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Run all tests with coverage viamake testandmake testcov
Use pytest with VCR cassettes for HTTP recording/playback in tests
Files:
tests/data/test_training_readiness.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_training_readiness.py
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Keep code examples in documentation concise and focused on usage, omitting import statements like
import garth
Files:
docs/api/scores.md
🧠 Learnings (1)
📚 Learning: 2026-01-09T21:51:43.789Z
Learnt from: CR
Repo: matin/garth PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T21:51:43.789Z
Learning: Applies to {garth/stats/**/*.py,garth/data/**/*.py} : Use Pydantic dataclasses for data validation in stats and data modules
Applied to files:
src/garth/data/training_readiness.py
🧬 Code graph analysis (4)
tests/data/test_training_readiness.py (2)
src/garth/data/training_readiness.py (2)
TrainingReadinessData(13-65)get(45-60)src/garth/http.py (1)
Client(19-245)
src/garth/data/__init__.py (1)
src/garth/data/training_readiness.py (1)
TrainingReadinessData(13-65)
src/garth/data/training_readiness.py (4)
src/garth/utils.py (2)
camel_to_snake_dict(17-33)format_end_date(66-71)src/garth/data/_base.py (1)
Data(15-50)tests/conftest.py (1)
client(21-22)src/garth/http.py (2)
Client(19-245)connectapi(186-192)
src/garth/__init__.py (1)
src/garth/data/training_readiness.py (1)
TrainingReadinessData(13-65)
🔇 Additional comments (8)
src/garth/__init__.py (1)
11-11: LGTM! TrainingReadinessData properly exposed in public API.The import and all export correctly make
TrainingReadinessDataaccessible asgarth.TrainingReadinessData, maintaining alphabetical order and following the existing pattern for other data classes.Also applies to: 52-52
src/garth/data/__init__.py (1)
14-14: LGTM! Data module export is correct.The import and all export properly expose
TrainingReadinessDatathrough the data module, maintaining alphabetical order and consistency with other data classes.Also applies to: 32-32
docs/api/scores.md (1)
173-290: LGTM! Comprehensive documentation for TrainingReadinessData.The documentation clearly distinguishes
TrainingReadinessDatafromMorningTrainingReadinessData, provides complete examples showing multiple entries with differentinput_contextvalues, and includes a practical filtering example. The examples follow coding guidelines by omitting import statements.tests/data/test_training_readiness.py (1)
35-49: LGTM! List method test validates sorting correctly.The test properly verifies:
- Multiple entries are returned across the date range
- Entries are correctly sorted by
(calendar_date, timestamp)tuplesrc/garth/data/training_readiness.py (4)
1-10: LGTM! Imports are clean and necessary.All imports are used and properly organized.
12-42: LGTM! Dataclass structure is comprehensive and well-typed.The field definitions properly capture all training readiness metrics. Key design choices are sound:
sleep_score: int | Noneandinput_context: str | Nonecorrectly handle nullable values from the APIrecovery_time_change_phrase: str | None = Noneprovides a sensible default- Uses Pydantic dataclass for validation, as per coding guidelines
44-60: LGTM! get() method correctly handles multi-entry responses.The implementation properly:
- Returns
list[Self] | Noneto accommodate multiple entries per day (unlike single-entry data models)- Handles empty responses by returning None
- Converts camelCase API responses to snake_case
- Uses type casting for clarity
The cast to
list[dict[str, Any]]on line 59 assumes the API always returns a list. Based on the API endpoint pattern and PR description stating it returns "all training readiness entries", this assumption is valid.
62-65: LGTM! list() method provides consistent sorting.Sorting by
(calendar_date, timestamp)ensures chronological ordering of entries, which is essential for multi-entry data where multiple updates can occur on the same day.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
TrainingReadinessDataclass that returns all training readiness entries for a dayMorningTrainingReadinessData(which filters for morning snapshots only), this returns all entries including post-exercise updates and realtime variable updatesinput_contextfield to distinguish between entry types (AFTER_WAKEUP_RESET,AFTER_POST_EXERCISE_RESET,UPDATE_REALTIME_VARIABLES)Closes #168
Test plan
get()andlist()methods🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
TrainingReadinessDatain the public API for fetching and listing all daily training readiness entries, including post-exercise and real-time updates.Documentation
✏️ Tip: You can customize this high-level summary in your review settings.