Codex/profile readiness fixtures#651
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 implements a comprehensive profile-based training settings system, including a new Profile tab, migration of training preferences to a profile-scoped SQLDelight aggregate, and a robust QA seeding mechanism for local testing. My feedback highlights that the rackSnapshotJson function currently uses brittle hardcoded JSON strings, which should be replaced with the project's JSON serializer to ensure consistency and maintainability.
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.
| private fun rackSnapshotJson(profileKey: String, isProfileA: Boolean): String { | ||
| val id = rackItemId(profileKey) | ||
| return if (isProfileA) { | ||
| "[{\"id\":\"$id\",\"name\":\"QA Weighted Vest A\",\"category\":\"WEIGHTED_VEST\",\"weightKg\":10.0,\"behavior\":\"ADDED_RESISTANCE\",\"enabled\":true,\"sortOrder\":1,\"createdAt\":1700000001000,\"updatedAt\":1700000001100}]" | ||
| } else { | ||
| "[{\"id\":\"$id\",\"name\":\"QA Assistance B\",\"category\":\"ASSISTANCE\",\"weightKg\":7.5,\"behavior\":\"COUNTERWEIGHT\",\"enabled\":true,\"sortOrder\":2,\"createdAt\":1700000002000,\"updatedAt\":1700000002100}]" | ||
| } |
There was a problem hiding this comment.
The rackSnapshotJson function uses hardcoded JSON strings to create the rack snapshot. This is brittle and can easily break if the RackItem serialization changes. To improve maintainability and ensure consistency with the rest of the app, consider using the project's JSON serializer to generate this string from RackItem data classes.
This would involve creating the RackItem instances and then using a Json instance to encode them into a string, which ensures that the generated JSON is always in sync with the RackItem data class definition and its serialization rules.
private fun rackSnapshotJson(profileKey: String, isProfileA: Boolean): String {
val json = kotlinx.serialization.json.Json { encodeDefaults = true }
val rackItem = if (isProfileA) {
RackItem(
id = rackItemId(profileKey),
name = "QA Weighted Vest A",
category = RackItemCategory.WEIGHTED_VEST,
weightKg = 10.0f,
behavior = RackItemBehavior.ADDED_RESISTANCE,
enabled = true,
sortOrder = 1,
createdAt = 1_700_000_001_000L,
updatedAt = 1_700_000_001_100L,
)
} else {
RackItem(
id = rackItemId(profileKey),
name = "QA Assistance B",
category = RackItemCategory.ASSISTANCE,
weightKg = 7.5f,
behavior = RackItemBehavior.COUNTERWEIGHT,
enabled = true,
sortOrder = 2,
createdAt = 1_700_000_002_000L,
updatedAt = 1_700_000_002_100L,
)
}
return json.encodeToString(listOf(rackItem))
}![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: eff8470a88
ℹ️ 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".
| ), | ||
| selected = core.weightUnit, | ||
| enabled = enabled, | ||
| onSelected = { onCoreChange(core.copy(weightUnit = it)) }, |
There was a problem hiding this comment.
When a profile switches between KG and LB here, the existing weightIncrement is carried over even though UserPreferences.effectiveWeightIncrement interprets the stored value in the newly selected display unit. For example, a user with a 5 kg step who switches to LB now silently gets a 5 lb step (and a 1 lb step becomes 1 kg when switching back), which changes workout weight adjustments/recommendations; reset to the unit default (-1f) or convert the increment when changing weightUnit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efac83b241
ℹ️ 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".
| if (verbalEvent != null) { | ||
| coordinator._hapticEvents.emit(verbalEvent) | ||
| Logger.i { "VBT: VERBAL_ENCOURAGEMENT emitted (tier=${verbalEvent.vulgarTier}, dominatrix=${verbalEvent.dominatrixMode}, vulgar=${verbalEvent.vulgarMode})" } |
There was a problem hiding this comment.
When a velocity-loss alert fires with verbal encouragement enabled, this new if (verbalEvent != null) block is not closed before the legacy val prefs = ... emission below, so the code emits a second VERBAL_ENCOURAGEMENT using raw vulgarModeEnabled/dominatrixModeActive values. In the inspected workout path, a profile with persisted vulgar/dominatrix intent but adultsOnlyConfirmed == false first emits the neutralized event and then immediately emits an adult-routed event, bypassing the local adult confirmation gate; close this block before the legacy code or remove the legacy emission.
Useful? React with 👍 / 👎.
Code Review Roast 🔥Verdict: Comment Only | Recommendation: Address existing review feedback; can merge after. Overview
No new inline comments this pass. I verified the three outstanding review comments against the current HEAD (
Files Reviewed (high-risk subset, 14 files)
(Full PR is 197 files / 55k insertions — only the changed-code paths relevant to the profile preferences feature and the three flagged lines were read in full.) Ponytail Review
Ponytail net: ~65 lines removable (Ponytail suggestions are optional cleanup on top of the open review feedback). Suggested Minimal PatchNo patch from this review. Existing comments cover the correctness items; the Ponytail shrink pass is optional and should land as a separate refactor if at all. Final Merge GuidanceCan merge after the existing 🏆 Best part: The verbal encouragement fix at 💀 Worst part: The two pre-existing comments ( 📊 Overall: A feature branch that ate the whole codebase and somehow came out the other side with the critical adult-confirmation gate intact. Ship after addressing the open comments — this isn't a code-quality problem, it's a follow-up hygiene problem. Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 71.2K · Output: 14.9K · Cached: 2.4M |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e15c7953a4
ℹ️ 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".
| @@ -466,7 +559,7 @@ abstract class BaseDataBackupManager( | |||
| dominantSide = session.dominantSide, | |||
| strengthProfile = session.strengthProfile, | |||
| formScore = session.formScore, | |||
| profile_id = session.profileId ?: "default", | |||
| profile_id = session.profileId ?: activeProfileId, | |||
There was a problem hiding this comment.
When importing a per-session/per-routine backup from another install or from a deleted profile, the backup has no userProfiles entry (see exportSession/exportRoutine) but mapSessionToBackup still serializes the source profileId. This assignment preserves that non-existent profile id instead of falling back to the active profile, so the imported workout history is written under an orphan profile and disappears from the user's active profile; resolve session.profileId against the imported/existing profile set, and mirror the fix in the streaming import path.
Useful? React with 👍 / 👎.
1 participant
No description provided.