Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a client telemetry system: new telemetry router and endpoints, expanded telemetry-related DB models (datetime-based dates, new survey tables, weight validation), new client fitness daily-survey workflows, helper logic for UTC-midnight dates, and test coverage for telemetry endpoints and CRUD. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant DB
Client->>API: GET /roles/client/fitness/{type}/today
API->>DB: get_or_create ClientTelemetry (today UTC-midnight)
DB-->>API: ClientTelemetry
API->>DB: get_or_create Daily{Type}Survey for client & telemetry
DB-->>API: DailySurvey (is_started/is_finished flags)
API-->>Client: Survey state + completion_id
sequenceDiagram
participant Client
participant API
participant DB
Client->>API: PUT /roles/client/telemetry/update_steps
API->>DB: fetch ClientTelemetry for client
DB-->>API: ClientTelemetry
API->>DB: create_or_update StepCount linked to telemetry
DB-->>API: StepCount (persisted)
API-->>Client: updated step count
Client->>API: GET /roles/client/telemetry/query/steps
API->>DB: query StepCount (paginated) for client
DB-->>API: list of StepCount rows
API-->>Client: steps list response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 4
🧹 Nitpick comments (1)
src/api/roles/client/telemetry.py (1)
1-9: Import framework symbols directly instead of throughfitness.py.Line 1 couples this router to another route module for
APIRouter,Session,select,PaginationParams, anddatetime, and line 9 declares an unusedtodayvariable. Direct imports make this module safer to import and reduce circular-import risk.♻️ Proposed import cleanup
-from src.api.roles.client.fitness import APIRouter, PaginationParams, Session, _get_or_create_telemetry, datetime, select +from datetime import datetime +from fastapi import APIRouter, Depends, HTTPException +from sqlmodel import Session, select + +from src.api.dependencies import get_client_account, PaginationParams +from src.api.roles.client.fitness import _get_or_create_telemetry from src.database.telemetry.models import ClientTelemetry, CompletedMealActivity, HealthMetrics, StepCount, CompletedSurvey, DailyMoodSurvey, CompletedWorkout from src.api.roles.client.domain import StepCountUpdateInput, StepCountUpdateOutput, WeightUpdateInput from src.database.session import get_session from src.database.account.models import Account -from fastapi import Depends, HTTPException -from src.api.dependencies import get_client_account - -today = datetime.utcnow().date()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/roles/client/telemetry.py` around lines 1 - 9, This module currently re-exports framework/runtime symbols from src.api.roles.client.fitness (APIRouter, Session, select, PaginationParams, datetime) and defines an unused today variable; replace those indirect imports by importing framework symbols directly (e.g., APIRouter from fastapi, Session from sqlalchemy.orm, select from sqlalchemy, datetime from the standard datetime module) and import PaginationParams and _get_or_create_telemetry from the modules where they are actually defined (locate their declarations and import them directly), then remove the unused today variable declaration; update import statements to reference the correct modules and keep only the names used in this file (APIRouter, PaginationParams, Session, _get_or_create_telemetry, datetime, select).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/roles/client/domain.py`:
- Around line 12-28: The module-level `@field_validator` and the function
step_count_must_be_non_negative are not attached to the model and the condition
0 > v > 100000 is impossible; move the validator into the StepCountUpdateInput
class as a `@field_validator`("step_count") class method (or define it as a model
method) and change the condition to reject values outside the allowed range
(e.g., if v < 0 or v > 100000: raise ValueError("Step count must be a
non-negative integer up to 100000")), and likewise add a corresponding
`@field_validator` inside WeightUpdateInput (e.g., ensure weight >= 0 and any
upper bound you need) to raise ValueError on invalid input so both
StepCountUpdateInput and WeightUpdateInput perform proper validation.
In `@src/api/roles/client/fitness.py`:
- Around line 108-119: The code uses a date() for "today" but
ClientTelemetry.date is a UTC-midnight datetime, so change the "today" value in
_get_or_create_telemetry to a UTC-midnight datetime (e.g.,
datetime.utcnow().replace(hour=0, minute=0, second=0, microsecond=0)) and use
that datetime when querying ClientTelemetry.date and when constructing the new
ClientTelemetry(client_id=..., date=...). This ensures the equality check
against ClientTelemetry.date and the created record use the same UTC-midnight
datetime value.
- Around line 232-248: The current submit handlers create a Completed* row
(e.g., CompletedSurvey) and commit before linking it to the parent survey, which
can leave orphan rows if the second commit fails; change the flow to call
db.flush() after creating the detail/completed object to get its generated id,
then set survey.is_finished and survey.completed_survey_id and finally
db.add(survey) and db.commit() once so both the detail and survey update are
persisted atomically; apply this pattern in the mood survey handler
(CompletedSurvey), the health metrics, step count, meal activity handlers, and
consolidate the workout submit handler so the detail flush and the survey update
are committed together (use db.flush(), db.add(), db.commit(), and db.refresh()
as needed).
- Around line 50-55: Add request-level validators on the payload classes: in
BodyMetricsSurveySubmitPayload add a `@field_validator` for weight that ensures
it's an integer > 0 and raises a ValueError with a clear message if not; in
StepsSurveySubmitPayload add a `@field_validator` for step_count that enforces 0
<= step_count <= 100000 and raises a ValueError otherwise. Use the
SQLModel/Pydantic field_validator decorator pattern already used in the codebase
(e.g., similar to StepCountUpdateInput) and ensure the error messages are
descriptive so FastAPI request validation returns useful feedback; apply the
same validation pattern to the other survey payload classes with the same fields
elsewhere in the file.
---
Nitpick comments:
In `@src/api/roles/client/telemetry.py`:
- Around line 1-9: This module currently re-exports framework/runtime symbols
from src.api.roles.client.fitness (APIRouter, Session, select, PaginationParams,
datetime) and defines an unused today variable; replace those indirect imports
by importing framework symbols directly (e.g., APIRouter from fastapi, Session
from sqlalchemy.orm, select from sqlalchemy, datetime from the standard datetime
module) and import PaginationParams and _get_or_create_telemetry from the
modules where they are actually defined (locate their declarations and import
them directly), then remove the unused today variable declaration; update import
statements to reference the correct modules and keep only the names used in this
file (APIRouter, PaginationParams, Session, _get_or_create_telemetry, datetime,
select).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61cf46c2-45e8-447f-b864-284e2ab517a4
📒 Files selected for processing (7)
src/api/app.pysrc/api/roles/client/client.pysrc/api/roles/client/domain.pysrc/api/roles/client/fitness.pysrc/api/roles/client/telemetry.pysrc/database/telemetry/models.pytests/test_client.py
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 `@src/api/roles/client/fitness.py`:
- Around line 71-79: The validator validate_meal_choice on class
MealSurveySubmitPayload currently only ensures at least one of
client_prescribed_meal_id or on_demand_meal_id is present; change it to enforce
exactly one by checking that (client_prescribed_meal_id is None) ==
(on_demand_meal_id is None) and raise a ValueError when both are None or both
are set, returning self only when exactly one is provided; apply the same
exact-one validation pattern to the other analogous validator found later in the
file.
- Around line 42-49: The WorkoutSurveySubmitPayload currently allows negative
numbers and empty payloads; update validation on the WorkoutSurveySubmitPayload
(fields completed_reps, completed_sets, completed_duration, estimated_calories)
to enforce non-negative (>=0) integers and reject negatives, and require that
either a workout reference (workout_plan_activity_id or workout_activity_id) is
provided OR an explicit "skipped/rest" flag/state is set before setting
is_finished; implement these checks where the payload is processed (e.g., in the
handler that sets is_finished) to return a validation error if neither reference
nor explicit skipped/rest is present or any numeric field is negative—apply the
same validation logic to the corresponding payload handling at the other
occurrence mentioned (lines ~327-346).
- Around line 122-153: Add DB-level unique constraints for ClientTelemetry on
(client_id, date) and for each survey model on client_telemetry_id, then make
_get_or_create_telemetry and _get_or_create_daily_survey concurrency-safe by
catching DB integrity/unique-constraint failures: attempt the insert as now, but
on IntegrityError (or equivalent) roll back the session and re-query the
existing row and return it; alternatively perform an upsert if your ORM/DB
supports it (use the unique constraint fields). Specifically, modify
_get_or_create_telemetry to create and commit inside a try/except that catches
IntegrityError, does db.rollback(), re-selects the telemetry by client_id/date
and returns it; do the same pattern for _get_or_create_daily_survey using
survey_model and client_telemetry_id so concurrent requests return the single
canonical row. Ensure you reference ClientTelemetry, _get_or_create_telemetry,
_get_or_create_daily_survey, and survey_model.client_telemetry_id in the
changes.
- Around line 336-340: The code persists client-supplied IDs (e.g.,
payload.workout_plan_activity_id into CompletedWorkout and
payload.client_prescribed_meal_id into CompletedMealActivity) without verifying
they belong to the authenticated account (acc.client_id); before creating
CompletedWorkout, query the DB to confirm payload.workout_plan_activity_id is
part of a workout plan owned by acc.client_id (join ClientWorkoutPlan by
workout_plan_id and ensure the activity exists in that plan) and, before
creating CompletedMealActivity, verify payload.client_prescribed_meal_id exists
in ClientPrescribedMeal with client_id == acc.client_id; if validation fails,
return an appropriate authorization/validation error instead of persisting.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 282fc19e-2081-4afc-9e58-9f4fa3f994c4
📒 Files selected for processing (2)
src/api/roles/client/domain.pysrc/api/roles/client/fitness.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/roles/client/domain.py
Comment on lines
+71
to
+79
| class MealSurveySubmitPayload(SQLModel): | ||
| client_prescribed_meal_id: Optional[int] = None | ||
| on_demand_meal_id: Optional[int] = None | ||
|
|
||
| @model_validator(mode="after") | ||
| def validate_meal_choice(self): | ||
| if self.client_prescribed_meal_id is None and self.on_demand_meal_id is None: | ||
| raise ValueError("Either client_prescribed_meal_id or on_demand_meal_id is required") | ||
| return self |
There was a problem hiding this comment.
Require exactly one meal source.
The current validator allows both client_prescribed_meal_id and on_demand_meal_id, so a single completed meal can be linked to two different sources. Enforce exactly one to keep meal telemetry unambiguous.
🛡️ Proposed validator fix
`@model_validator`(mode="after")
def validate_meal_choice(self):
- if self.client_prescribed_meal_id is None and self.on_demand_meal_id is None:
- raise ValueError("Either client_prescribed_meal_id or on_demand_meal_id is required")
+ has_prescribed_meal = self.client_prescribed_meal_id is not None
+ has_on_demand_meal = self.on_demand_meal_id is not None
+ if has_prescribed_meal == has_on_demand_meal:
+ raise ValueError("Exactly one of client_prescribed_meal_id or on_demand_meal_id is required")
return selfAlso applies to: 541-545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/roles/client/fitness.py` around lines 71 - 79, The validator
validate_meal_choice on class MealSurveySubmitPayload currently only ensures at
least one of client_prescribed_meal_id or on_demand_meal_id is present; change
it to enforce exactly one by checking that (client_prescribed_meal_id is None)
== (on_demand_meal_id is None) and raise a ValueError when both are None or both
are set, returning self only when exactly one is provided; apply the same
exact-one validation pattern to the other analogous validator found later in the
file.
Comment on lines
+122
to
+153
| def _get_or_create_telemetry(db: Session, client_id: int) -> ClientTelemetry: | ||
| today = datetime.utcnow().replace(hour=0, minute=0, second=0, microsecond=0) | ||
| telemetry = db.exec( | ||
| select(ClientTelemetry).where( | ||
| ClientTelemetry.client_id == client_id, | ||
| ClientTelemetry.date == today | ||
| ) | ||
| ).first() | ||
|
|
||
| if telemetry is None: | ||
| telemetry = ClientTelemetry(client_id=client_id, date=today) | ||
| db.add(telemetry) | ||
| db.commit() | ||
| db.refresh(telemetry) | ||
|
|
||
| return telemetry | ||
|
|
||
|
|
||
| def _get_or_create_daily_survey(db: Session, client_id: int, survey_model): | ||
| telemetry = _get_or_create_telemetry(db, client_id) | ||
| survey = db.exec( | ||
| select(survey_model).where( | ||
| survey_model.client_telemetry_id == telemetry.id | ||
| ) | ||
| ).first() | ||
|
|
||
| if survey is None: | ||
| survey = survey_model(is_seen=True, is_started=False, is_finished=False, client_telemetry_id=telemetry.id) | ||
| db.add(survey) | ||
| db.commit() | ||
| db.refresh(survey) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect telemetry models for unique constraints/indexes that protect daily get-or-create flows.
rg -n -C4 'class ClientTelemetry|class DailyMoodSurvey|class DailyWorkoutSurvey|class DailyBodyMetricsSurvey|class DailyStepsSurvey|class DailyMealSurvey|UniqueConstraint|unique=True|__table_args__' --type pyRepository: matt2jog/group_project_490_backend
Length of output: 6118
🏁 Script executed:
cat -n src/database/telemetry/models.pyRepository: matt2jog/group_project_490_backend
Length of output: 6334
Make daily get-or-create concurrency-safe.
Two simultaneous requests to /today or /start endpoints can both miss the existing telemetry/survey row in a race condition and create duplicate records. Add DB-level unique constraints on ClientTelemetry(client_id, date) and each survey model's client_telemetry_id, then update the get-or-create logic to handle insert collisions via retry or upsert.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/roles/client/fitness.py` around lines 122 - 153, Add DB-level unique
constraints for ClientTelemetry on (client_id, date) and for each survey model
on client_telemetry_id, then make _get_or_create_telemetry and
_get_or_create_daily_survey concurrency-safe by catching DB
integrity/unique-constraint failures: attempt the insert as now, but on
IntegrityError (or equivalent) roll back the session and re-query the existing
row and return it; alternatively perform an upsert if your ORM/DB supports it
(use the unique constraint fields). Specifically, modify
_get_or_create_telemetry to create and commit inside a try/except that catches
IntegrityError, does db.rollback(), re-selects the telemetry by client_id/date
and returns it; do the same pattern for _get_or_create_daily_survey using
survey_model and client_telemetry_id so concurrent requests return the single
canonical row. Ensure you reference ClientTelemetry, _get_or_create_telemetry,
_get_or_create_daily_survey, and survey_model.client_telemetry_id in the
changes.
This was referenced May 6, 2026
Merged
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.
Summary by CodeRabbit
New Features
Improvements
Tests