Repository navigation
feat: Sleep score recalculation after sleep session merge - #884
Merged
Merged
Conversation
czajkub
added
backend
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a bulk-delete method to HealthScoreRepository to remove internal health scores for a user/date/category/provider and extends EventRecordService to compute a session's local date, delete stale internal SLEEP scores for affected local dates, and repopulate per-date sleep scores via the sleep scoring service during sleep create/merge/update flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant ERS as EventRecordService
participant HSR as HealthScoreRepository
participant SSS as SleepScoreService
participant DB as Database
ERS->>ERS: detect sleep create/merge/update\ncompute local_date = _local_sleep_date(start_datetime, zone_offset)
ERS->>HSR: delete_for_user_date(user_id, local_date, category=SLEEP, provider="internal")
HSR->>DB: DELETE FROM health_scores WHERE user_id/recorded_at/category/provider
DB-->>HSR: deleted_count
HSR-->>ERS: deleted_count
ERS->>SSS: get_sleep_scores_for_date(user_id, local_date)
SSS->>DB: query sleep events for user/local_date
DB-->>SSS: sleep event data
SSS-->>ERS: score components (overall, duration, stages, consistency, interruptions)
ERS->>HSR: bulk_create HealthScore rows (internal, components) recorded_at=UTC-midnight
HSR->>DB: INSERT INTO health_scores (...)
DB-->>HSR: created_count
HSR-->>ERS: created_count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 docstrings
🧪 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 |
|
🚅 Deployed to the open-wearables-pr-884 environment in Open Wearables
|
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-884
April 16, 2026 10:22
Destroyed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/services/event_record_service.py`:
- Around line 133-167: _recompute_sleep_score only recomputes the sleep score
for the post-update local date, leaving a stale score on the old local date when
a session is shifted across midnight; change the API and callers to recompute
both affected local dates: update _recompute_sleep_score to accept an iterable
(or rename to _recompute_sleep_scores) and for each date call
health_score_repo.delete_for_user_date and then get_sleep_scores_for_date_range
for that date set (using sleep_score_service.get_sleep_scores_for_date_range),
and update callers that currently compute only the new date (use
self._local_sleep_date(old.start_datetime, old.zone_offset) and
self._local_sleep_date(new.start_datetime, new.zone_offset) to build a set of
dates and pass that set into the revised method so both the old and new local
dates are deleted and recomputed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ec50a00-a91e-47a5-957b-b409e9a7a0ba
📒 Files selected for processing (2)
backend/app/repositories/health_score_repository.pybackend/app/services/event_record_service.py
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-884
April 16, 2026 11:43
Destroyed
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/app/services/event_record_service.py (1)
123-131: Consider defensive check for empty zone_offset strings.The check
if zone_offset is not Nonepasses for empty strings, which would cause anIndexErroronzone_offset[0]. While input validation should prevent empty strings, a truthy check would be more defensive.💡 Optional defensive improvement
`@staticmethod` def _local_sleep_date(start_datetime: datetime, zone_offset: str | None) -> date: """Return the local calendar date of a sleep session start (mirrors SQL logic in fill task).""" dt = start_datetime if start_datetime.tzinfo is not None else start_datetime.replace(tzinfo=timezone.utc) - if zone_offset is not None: + if zone_offset: sign = 1 if zone_offset[0] == "+" else -1 hours, minutes = int(zone_offset[1:3]), int(zone_offset[4:6]) dt = dt.astimezone(timezone(timedelta(hours=sign * hours, minutes=sign * minutes))) return dt.date()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/event_record_service.py` around lines 123 - 131, In _local_sleep_date, the current check "if zone_offset is not None" allows empty strings and can cause IndexError when indexing zone_offset; change the guard to verify zone_offset is truthy and properly formatted (e.g., ensure len(zone_offset) >= 6 and zone_offset[0] in "+-") before parsing, and if invalid either treat as no offset (keep UTC) or raise a clear ValueError; update the parsing branch in _local_sleep_date to only run when the validated zone_offset is present and well-formed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/app/services/event_record_service.py`:
- Around line 123-131: In _local_sleep_date, the current check "if zone_offset
is not None" allows empty strings and can cause IndexError when indexing
zone_offset; change the guard to verify zone_offset is truthy and properly
formatted (e.g., ensure len(zone_offset) >= 6 and zone_offset[0] in "+-") before
parsing, and if invalid either treat as no offset (keep UTC) or raise a clear
ValueError; update the parsing branch in _local_sleep_date to only run when the
validated zone_offset is present and well-formed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abb9cbd8-dad4-460c-88c9-ce7fed33fda5
📒 Files selected for processing (1)
backend/app/services/event_record_service.py
czajkub
force-pushed
the
sleep-merge-score-recalculation
branch
from
April 23, 2026 11:13
4c1f202 to
dd9082b
Compare
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-884
April 23, 2026 11:13
Destroyed
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/app/services/event_record_service.py (1)
131-177: LGTM — correct transactional handling with one optional polish.The helpers are well-structured:
_local_sleep_datecorrectly snapshots the pre-mutation timezone, and_recompute_sleep_scorescleanly deletes-then-repopulates internal SLEEP scores within the same (uncommitted) transaction sosleep_score_servicesees flushed session data. Early-returning on emptyscoreswhile leaving deletes in place is also the right call — a date with no sleep records should have no score. The delete path (provider="internal"string) and insert path (provider=ProviderName.INTERNALenum) are consistent:ProviderName.INTERNALserializes to"internal"in both directions.Optional nit (not blocking): the manual zone_offset parsing at lines 134-138 could be simplified using
datetime.strptime:♻️ Optional refactor using strptime
- dt = start_datetime if start_datetime.tzinfo is not None else start_datetime.replace(tzinfo=timezone.utc) - if zone_offset is not None: - sign = 1 if zone_offset[0] == "+" else -1 - hours, minutes = int(zone_offset[1:3]), int(zone_offset[4:6]) - dt = dt.astimezone(timezone(timedelta(hours=sign * hours, minutes=sign * minutes))) - return dt.date() + dt = start_datetime if start_datetime.tzinfo is not None else start_datetime.replace(tzinfo=timezone.utc) + if zone_offset is not None: + tz = datetime.strptime(zone_offset, "%z").tzinfo + dt = dt.astimezone(tz) + return dt.date()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/event_record_service.py` around lines 131 - 177, The manual zone_offset parsing in _local_sleep_date can be simplified: instead of slicing zone_offset and building a timedelta, parse the offset string with datetime.strptime using the "%z" format to obtain a tzinfo and then call dt.astimezone(that_tzinfo); update _local_sleep_date to use that parsed tzinfo when zone_offset is not None and keep the existing dt.tzinfo handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/app/services/event_record_service.py`:
- Around line 131-177: The manual zone_offset parsing in _local_sleep_date can
be simplified: instead of slicing zone_offset and building a timedelta, parse
the offset string with datetime.strptime using the "%z" format to obtain a
tzinfo and then call dt.astimezone(that_tzinfo); update _local_sleep_date to use
that parsed tzinfo when zone_offset is not None and keep the existing dt.tzinfo
handling unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 875d0948-3b5a-43ac-b8b6-ce07394156d6
📒 Files selected for processing (2)
backend/app/repositories/health_score_repository.pybackend/app/services/event_record_service.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/repositories/health_score_repository.py
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-884
April 23, 2026 11:21
Destroyed
KaliszS
approved these changes
May 7, 2026
0xMrMasry
pushed a commit
to 0xMrMasry/op
that referenced
this pull request
May 10, 2026
This was referenced May 11, 2026
This was referenced May 29, 2026
6 tasks done
6 tasks done
This branch was successfully deployed
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
2 participants
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.
Description
When another sleep session is merged (or an incoming one), the sleep score stays the same (even if e.g. the sleep stages changed), and now it will be correctly recalculating afterwards
Checklist
General
docs/(or no docs update needed)Backend Changes
You have to be in
backenddirectory to make it work:uv run pre-commit run --all-filespassesSummary by CodeRabbit