Repository navigation
feat(backend): add service and endpoint for health scores - #804
Merged
Merged
Conversation
czajkub
added
backend
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds HealthScore domain: repository with JSONB component queries and bulk insert, query params schema and validation, service layer and singleton, API listing endpoint with filters/pagination, factories, and end-to-end tests for repository, service, and API. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/API
participant Router as API Router
participant Service as HealthScoreService
participant Repo as HealthScoreRepository
participant DB as PostgreSQL (HealthScore, DataSource)
Client->>Router: GET /users/{user_id}/health-scores (query params)
Router->>Service: get_scores_with_filters(db_session, user_id, params)
Service->>Repo: get_with_filters(db_session, user_id, params)
Repo->>DB: SELECT ... JOIN DataSource WHERE DataSource.user_id = user_id AND <filters> ORDER BY recorded_at DESC LIMIT/OFFSET
DB-->>Repo: rows + total_count
Repo-->>Service: (models list, total_count)
Service-->>Router: (models list, total_count)
Router-->>Client: PaginatedResponse with data, pagination, metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
🚅 Deployed to the open-wearables-pr-804 environment in Open Wearables
|
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-804
April 8, 2026 11:40
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: 4
🤖 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/repositories/health_score_repository.py`:
- Line 50: The docstring stating "Bulk insert health scores, updating on
conflict with the unique constraint." conflicts with the actual implementation
which calls on_conflict_do_nothing; update either the docstring or the behavior:
either change the docstring to say "do nothing on conflict" to match the
on_conflict_do_nothing call, or replace on_conflict_do_nothing with an
on_conflict_do_update clause that sets the desired columns (e.g.,
timestamp/score) so the DB updates on unique constraint conflicts; locate the
method whose docstring is "Bulk insert health scores, updating on conflict with
the unique constraint." and the database call using on_conflict_do_nothing to
make the aligned change.
- Line 59: The current on_conflict_do_nothing call with
index_elements=["data_source_id","category","recorded_at"] will not dedupe rows
where data_source_id IS NULL; change the upsert to either (A) target a coalesced
expression so NULLs are treated the same (e.g. use a COALESCE(data_source_id,
'INTERNAL') expression in the conflict key via sa.text or equivalent) or (B)
switch to on_conflict_do_update so duplicates (including NULL data_source_id)
will update instead of inserting; also update the function/docstring in
health_score_repository.py (the upsert code path and the docstring near the
current call) to reflect the actual behavior (use COALESCE in the conflict key
or perform an on_conflict_do_update with appropriate set_ values) and ensure the
conflict key expression matches any database index you create for COALESCE-based
uniqueness.
In `@backend/app/schemas/model_crud/activities/health_score.py`:
- Around line 44-51: Add a Pydantic schema-level validator to
HealthScoreQueryParams that rejects windows where start_datetime is not strictly
before end_datetime: implement a `@root_validator` (or `@model_validator` in newer
Pydantic) inside the HealthScoreQueryParams class to check if both
start_datetime and end_datetime are provided and raise a ValueError (with a
clear message like "start_datetime must be before end_datetime") when
start_datetime >= end_datetime; update any tests accordingly so invalid query
params are rejected at the schema level rather than passed to the DB layer.
In `@backend/app/services/health_score_service.py`:
- Around line 20-50: Add the missing `@handle_exceptions` decorator to all public
service methods in this file: get_by_all_components, get_by_any_component,
bulk_create, get_latest_by_category, get_latest_per_category, and
get_scores_with_filters; import handle_exceptions from app.utils.exceptions at
the top of the module and place the decorator immediately above each method
definition so the service layer follows the exception-handling convention.
🪄 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: 04c6e261-5c0e-4b1f-9ba3-6d1f47e81a21
📒 Files selected for processing (5)
backend/app/repositories/__init__.pybackend/app/repositories/health_score_repository.pybackend/app/schemas/model_crud/activities/__init__.pybackend/app/schemas/model_crud/activities/health_score.pybackend/app/services/health_score_service.py
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-804
April 8, 2026 11:56
Destroyed
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/tests/repositories/test_health_score_repository.py (1)
194-221:⚠️ Potential issue | 🟠 MajorAdd a test for duplicate health scores with
data_source_id=None(internally computed scores).The existing test at lines 194–221 only validates duplicate prevention for non-null
data_source_id(external provider scores). Due to PostgreSQL NULL semantics, the UNIQUE constraint on(data_source_id, category, recorded_at)does NOT prevent duplicates whendata_source_idis NULL. This creates a gap in test coverage for internally computed scores.Suggested test addition
class TestHealthScoreRepositoryBulkCreate: @@ def test_bulk_create_ignores_duplicates(self, db: Session, repo: HealthScoreRepository) -> None: ... assert len(results) == 1 assert results[0].value == Decimal("80.00") + + def test_bulk_create_allows_duplicates_when_data_source_id_is_null(self, db: Session, repo: HealthScoreRepository) -> None: + recorded_at = datetime.now(timezone.utc) + first = HealthScoreCreate( + id=uuid4(), + data_source_id=None, + provider=ProviderName.INTERNAL, + category=HealthScoreCategory.SLEEP, + value=Decimal("80.00"), + recorded_at=recorded_at, + ) + second = HealthScoreCreate( + id=uuid4(), + data_source_id=None, + provider=ProviderName.INTERNAL, + category=HealthScoreCategory.SLEEP, + value=Decimal("99.00"), + recorded_at=recorded_at, + ) + repo.bulk_create(db, [first, second]) + db.commit() + + results = ( + db.query(HealthScore) + .filter( + HealthScore.data_source_id.is_(None), + HealthScore.category == HealthScoreCategory.SLEEP, + HealthScore.recorded_at == recorded_at, + ) + .all() + ) + assert len(results) == 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/repositories/test_health_score_repository.py` around lines 194 - 221, The test test_bulk_create_ignores_duplicates only covers duplicates when data_source_id is set, but NULL data_source_id bypasses the DB UNIQUE constraint; add a new test variant that creates two HealthScoreCreate objects with data_source_id=None (same category and recorded_at) and calls HealthScoreRepository.bulk_create twice, then query HealthScore to assert only one row exists and its value matches the first insert; ensure you use the same recorded_at and category and reference HealthScoreCreate, HealthScoreRepository.bulk_create, and the data_source_id=None case so the repository logic for handling internal (computed) scores is exercised.
🤖 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/tests/factories.py`:
- Around line 501-507: The _create method in HealthScoreFactory currently
discards any explicit data_source_id and always creates a DataSource, which
prevents tests from setting data_source_id=None; update
HealthScoreFactory._create so it first checks whether "data_source_id" exists in
kwargs (even if its value is None) and if present, use it as-is without creating
a DataSource; otherwise preserve the existing behavior of accepting a
data_source object via the data_source kwarg or creating a DataSourceFactory()
and setting kwargs["data_source_id"] to its id before calling super()._create.
In `@backend/tests/services/test_health_score_service.py`:
- Around line 93-95: The test has function-local imports inside test_bulk_create
(uuid4 and HealthScoreCreate); move these imports to module scope by adding
"from uuid import uuid4" and "from app.schemas.model_crud.activities import
HealthScoreCreate" at the top of the test file, then remove the in-function
import statements so test_bulk_create only uses those symbols without
re-importing them.
---
Duplicate comments:
In `@backend/tests/repositories/test_health_score_repository.py`:
- Around line 194-221: The test test_bulk_create_ignores_duplicates only covers
duplicates when data_source_id is set, but NULL data_source_id bypasses the DB
UNIQUE constraint; add a new test variant that creates two HealthScoreCreate
objects with data_source_id=None (same category and recorded_at) and calls
HealthScoreRepository.bulk_create twice, then query HealthScore to assert only
one row exists and its value matches the first insert; ensure you use the same
recorded_at and category and reference HealthScoreCreate,
HealthScoreRepository.bulk_create, and the data_source_id=None case so the
repository logic for handling internal (computed) scores is exercised.
🪄 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: 14e2a2a5-9486-4acc-a17e-73136f0f042b
📒 Files selected for processing (7)
backend/app/repositories/health_score_repository.pybackend/app/schemas/model_crud/activities/health_score.pybackend/app/services/health_score_service.pybackend/tests/api/v1/test_health_scores.pybackend/tests/factories.pybackend/tests/repositories/test_health_score_repository.pybackend/tests/services/test_health_score_service.py
✅ Files skipped from review due to trivial changes (1)
- backend/app/services/health_score_service.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/schemas/model_crud/activities/health_score.py
- backend/app/repositories/health_score_repository.py
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-804
April 8, 2026 12:22
Destroyed
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-804
April 8, 2026 12:26
Destroyed
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/app/api/routes/v1/__init__.py (2)
64-64: Tag naming deviates from kebab-case guideline.Per coding guidelines, tags should use "singular + kebab-case" (e.g.,
health-score). However, the current value"Health Scores"follows the pattern of adjacent tags ("Summaries","Timeseries","Events"). Consider aligning with the guideline for consistency if the team standardizes on kebab-case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/v1/__init__.py` at line 64, The tag on the router include uses a non-kebab-case plural string; update the call to v1_router.include_router(health_scores_router, tags=[...]) to use the singular kebab-case tag "health-score" instead of "Health Scores" so it matches the project's "singular + kebab-case" convention and the other route tags.
11-11: Import is out of alphabetical order.The
health_scoresimport should come aftergarmin_webhooks(alphabetically:events<garmin_webhooks<health_scores). Runningisortor pre-commit should fix this automatically.Proposed fix
from .events import router as events_router -from .health_scores import router as health_scores_router from .garmin_webhooks import router as garmin_webhooks_router +from .health_scores import router as health_scores_router from .import_xml import router as import_xml_router🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/v1/__init__.py` at line 11, The import order is not alphabetical: move the "from .health_scores import router as health_scores_router" line so it appears after "from .garmin_webhooks import router as garmin_webhooks_router" (ensuring the sequence is events_router, garmin_webhooks_router, health_scores_router); alternatively run isort/pre-commit to reorder imports automatically and verify the module-level routers remain correctly named (events_router, garmin_webhooks_router, health_scores_router).
🤖 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/api/routes/v1/__init__.py`:
- Line 64: The tag on the router include uses a non-kebab-case plural string;
update the call to v1_router.include_router(health_scores_router, tags=[...]) to
use the singular kebab-case tag "health-score" instead of "Health Scores" so it
matches the project's "singular + kebab-case" convention and the other route
tags.
- Line 11: The import order is not alphabetical: move the "from .health_scores
import router as health_scores_router" line so it appears after "from
.garmin_webhooks import router as garmin_webhooks_router" (ensuring the sequence
is events_router, garmin_webhooks_router, health_scores_router); alternatively
run isort/pre-commit to reorder imports automatically and verify the
module-level routers remain correctly named (events_router,
garmin_webhooks_router, health_scores_router).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 26b313c0-8efa-404b-82fd-6e653fadbbf0
📒 Files selected for processing (6)
backend/app/api/routes/v1/__init__.pybackend/app/api/routes/v1/health_scores.pybackend/app/repositories/health_score_repository.pybackend/tests/api/v1/test_health_scores.pybackend/tests/repositories/test_health_score_repository.pybackend/tests/services/test_health_score_service.py
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/tests/repositories/test_health_score_repository.py
- backend/tests/api/v1/test_health_scores.py
- backend/app/repositories/health_score_repository.py
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-804
April 8, 2026 12:35
Destroyed
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/tests/factories.py (1)
492-508: Align defaultproviderwith generateddata_sourceto avoid inconsistent fixtures.Right now defaults can produce
provider=GARMINwith a generated Appledata_source_id. That can make tests ambiguous.Proposed adjustment
class HealthScoreFactory(BaseFactory): @@ - provider = ProviderName.GARMIN + provider = ProviderName.APPLE @@ def _create(cls, model_class: type[HealthScore], *args: Any, **kwargs: Any) -> HealthScore: if "data_source_id" in kwargs: kwargs.pop("data_source", None) else: data_source = kwargs.pop("data_source", None) if data_source is None: data_source = DataSourceFactory() kwargs["data_source_id"] = data_source.id + if "provider" not in kwargs: + kwargs["provider"] = data_source.provider return super()._create(model_class, *args, **kwargs)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/factories.py` around lines 492 - 508, The HealthScore factory can produce mismatched defaults (provider=ProviderName.GARMIN) while creating a DataSource with a different provider; update the _create method in the HealthScore factory so that when you generate a DataSource (the data_source variable created from DataSourceFactory()), you also set kwargs["provider"] = data_source.provider unless a provider was explicitly passed in; keep existing behavior when data_source_id is passed in and ensure you still set kwargs["data_source_id"] = data_source.id after creating the DataSource.
🤖 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/tests/factories.py`:
- Line 485: The new HealthScoreFactory class was added but not exported in the
module's __all__; update the module-level __all__ list to include
"HealthScoreFactory" so it is reachable by `from tests.factories import *`.
Locate the __all__ variable in this file and add the string "HealthScoreFactory"
alongside the other factory names (ensure exact spelling and quotes match
existing entries).
---
Nitpick comments:
In `@backend/tests/factories.py`:
- Around line 492-508: The HealthScore factory can produce mismatched defaults
(provider=ProviderName.GARMIN) while creating a DataSource with a different
provider; update the _create method in the HealthScore factory so that when you
generate a DataSource (the data_source variable created from
DataSourceFactory()), you also set kwargs["provider"] = data_source.provider
unless a provider was explicitly passed in; keep existing behavior when
data_source_id is passed in and ensure you still set kwargs["data_source_id"] =
data_source.id after creating the DataSource.
🪄 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: fa35e400-d5cf-49c0-a8f6-7b23b25b292e
📒 Files selected for processing (2)
backend/tests/factories.pybackend/tests/services/test_health_score_service.py
✅ Files skipped from review due to trivial changes (1)
- backend/tests/services/test_health_score_service.py
| return super()._create(model_class, *args, **kwargs) | ||
|
|
||
|
|
||
| class HealthScoreFactory(BaseFactory): |
Contributor
There was a problem hiding this comment.
Export the new factory in __all__ for consistency with other factories.
HealthScoreFactory is added but not listed in the module export list, which can break from tests.factories import * patterns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/tests/factories.py` at line 485, The new HealthScoreFactory class was
added but not exported in the module's __all__; update the module-level __all__
list to include "HealthScoreFactory" so it is reachable by `from tests.factories
import *`. Locate the __all__ variable in this file and add the string
"HealthScoreFactory" alongside the other factory names (ensure exact spelling
and quotes match existing entries).
czajkub
force-pushed
the
health-score-services
branch
from
April 8, 2026 12:43
23ce1b9 to
d5c8524
Compare
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-804
April 8, 2026 12:44
Destroyed
KaliszS
approved these changes
Apr 8, 2026
railway-app
Bot
temporarily deployed
to
Open Wearables / open-wearables-pr-804
April 8, 2026 13:16
Destroyed
czajkub
added a commit
that referenced
this pull request
Apr 8, 2026
…)" This reverts commit 5ddfde6.
This was referenced Apr 9, 2026
9 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
Add service, repository and endpoint for health scores with some helper methods
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
New Features
Tests