Fix crash when deleting all workouts from a cycle#501
Conversation
Add bounds checks to all index-based operations in CycleEditorViewModel. The crash occurred because SwipeToDismissBox's confirmValueChange callback can fire with a stale Compose-captured index after MutableStateFlow.update retries its lambda with a newer (shorter) item list. https://claude.ai/code/session_011C2KidTSW3skiLyPgRTNqx
![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 bounds checks across several functions in CycleEditorViewModel.kt (such as deleteItem, duplicateItem, reorderItems, convertToWorkout, convertToRest, and changeRoutine) to prevent out-of-bounds errors. The review feedback suggests a minor optimization in reorderItems to return early when the source and destination indices are identical, avoiding unnecessary list operations.
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.
|
|
||
| fun reorderItems(from: Int, to: Int) { | ||
| _uiState.update { state -> | ||
| if (from !in state.items.indices || to !in state.items.indices) return@update state |
There was a problem hiding this comment.
If from and to are equal, the reordering operation is a no-op. We can optimize this by returning early when from == to, which avoids unnecessary list copying, item removal, insertion, and renumbering.
| if (from !in state.items.indices || to !in state.items.indices) return@update state | |
| if (from == to || from !in state.items.indices || to !in state.items.indices) return@update state |
![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: 244356bc73
ℹ️ 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".
|
|
||
| fun deleteItem(index: Int) { | ||
| _uiState.update { state -> | ||
| if (index !in state.items.indices) return@update state |
There was a problem hiding this comment.
When a stale onDelete arrives after a prior delete (for example, swipe-delete day 1 and then day 2 before recomposition, so the second captured index is no longer in state.items.indices), this guard returns without changing lastDeletedItem. CycleEditorScreen still shows the “Day removed” snackbar and may call undoDelete(), which then restores the previous deletion rather than the item represented by the second swipe, or reports a removal that never happened. In the rapid-delete path this fix targets, no-op deletes need to be reported/handled so snackbar and undo state stay aligned.
Useful? React with 👍 / 👎.
Summary
CycleEditorViewModel.deleteItem()threwIndexOutOfBoundsExceptionwhen swipe-deleting cycle items rapidly (reported via TestFlight, iPhone 14 Pro Max / iOS 26.5)SwipeToDismissBox.confirmValueChangefires with the Compose-capturedindex, butMutableStateFlow.updateretries its lambda with the latest state — which may have fewer items after a concurrent deleteindex !in state.items.indices) to all six index-based mutation methods:deleteItem,duplicateItem,reorderItems,convertToWorkout,convertToRest,changeRoutineTest plan
https://claude.ai/code/session_011C2KidTSW3skiLyPgRTNqx
Generated by Claude Code