Add personal record sync and PR feedback UI#493
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces phase-specific personal records (PRs) across the application, updating repositories, view models, UI components, and the sync manager to support concentric, eccentric, and combined PR phases. Key additions include a backfill mechanism for historical sessions and authoritative top-level PR syncing to the portal. Feedback highlights a major performance bottleneck in the backfill process due to scanning the entire workout history on every sync, potential null-pointer exceptions in ExerciseEditBottomSheet when accessing nullable PR properties, and a sync issue where sessionId fails to resolve for PRs associated with previously synced sessions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| override suspend fun backfillPhaseSpecificPRs(profileId: String): Int = withContext(Dispatchers.IO) { | ||
| val prRepository = SqlDelightPersonalRecordRepository(db) | ||
| val sessions = getWorkoutSessionsModifiedSince(0L, profileId) | ||
| var changedRows = 0 |
There was a problem hiding this comment.
Scanning the entire workout history from 0L on every sync operation is a major performance bottleneck. For users with a large history, this will execute thousands of database queries and transactions synchronously, blocking the sync process and draining the device's battery. Consider implementing a checkpoint mechanism (e.g., storing the last backfilled session timestamp in a settings/metadata table) or running this backfill only once when the app detects it hasn't been completed.
| ) { | ||
| Text( | ||
| text = "$weightPercentOfPR% of PR", | ||
| text = "$weightPercentOfPR% of ${currentExercisePR.phase.displayLabel()} PR", |
There was a problem hiding this comment.
Since currentExercisePR is nullable, accessing currentExercisePR.phase directly without safe navigation will cause a compilation error or a NullPointerException at runtime when no PR is set. Use safe navigation and provide a fallback label.
| text = "$weightPercentOfPR% of ${currentExercisePR.phase.displayLabel()} PR", | |
| text = "$weightPercentOfPR% of ${currentExercisePR?.phase?.displayLabel() ?: "Combined"} PR", |
| Column { | ||
| Text( | ||
| "Personal Record", | ||
| "${pr.phase.displayLabel()} Personal Record", |
There was a problem hiding this comment.
Since pr is nullable, accessing pr.phase directly without safe navigation will cause a compilation error or a NullPointerException at runtime. Use safe navigation and provide a fallback label.
| "${pr.phase.displayLabel()} Personal Record", | |
| "${pr?.phase?.displayLabel() ?: "Combined"} Personal Record", |
| val sessionIdByPrKey = sessions.associate { session -> | ||
| "${session.exerciseId}:${session.timestamp}" to session.id | ||
| } |
There was a problem hiding this comment.
Since sessions only contains workout sessions modified since lastSync (delta-loaded), any personal record whose corresponding session was synced in a previous sync operation will fail to resolve its sessionId (it will be null). This breaks the session-to-PR relationship on the portal. Consider querying the database for the session IDs of recentPRs if they are not found in the delta sessions list.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7336caf24f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sessionId = sessionId, | ||
| achievedAt = epochToIso8601(record.timestamp), | ||
| updatedAt = epochToIso8601(currentTimeMillis()), | ||
| localProfileId = record.profileId, |
There was a problem hiding this comment.
For non-default profiles, the PersonalRecord instances passed into this new portal DTO path come from SqlDelightSyncRepository.getFullPRsModifiedSince, which currently constructs them without copying row.profile_id, so record.profileId falls back to the domain default of "default". That makes every dedicated PR row pushed through localProfileId look like it belongs to the default profile even though the query was scoped to the active profile, causing PRs to be deduped/attached under the wrong portal profile for multi-profile users. Please populate the profile id (and keep using it here) before sending these rows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Use the workout session timestamp for phase PRs
For phase PRs recorded during a normal completed workout, this passes the local timestamp value from line 116 (currentTimeMillis() at PR calculation time), while the saved WorkoutSession.timestamp is the workout start time (coordinator.workoutStartTime). The new sync linker later looks up sessions with exerciseId + achievedAt exact equality, and the backfill path will not rewrite equal-weight PRs, so these live phase PRs are pushed with sessionId = null and the matching set is not marked as a PR. Use the actual session timestamp/id when creating these rows, or make the sync/linker able to resolve the owning session without exact timestamp equality.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Testing