Stop "Duplicate as manual" deleting the session it copied (both platforms)

Strand/Screens/WorkoutsView.swift

@@ -132,6 +132,12 @@ struct WorkoutsView: View {
     /// Wraps the optional edited row so `.sheet(item:)` can present add (editing == nil) or edit.
     private struct WorkoutSheetTarget: Identifiable {
         let editing: WorkoutRow?
+        /// True for "Duplicate as manual": the form pre-fills FROM a read-only row, but the save is a pure
+        /// ADD. The pre-fill row is not a stored manual row, so it must never travel on as `replacing:` —
+        /// it carries the ORIGINAL's natural key while claiming source "manual", and the repository would
+        /// take that as an edit and retire the row it was copied from. `Repository.saveManualWorkout`
+        /// documents that an imported row is never passed as `replacing`; this is what makes that true.
+        var isCopy = false
         let id = UUID()
     }
 
@@ -235,7 +241,8 @@ struct WorkoutsView: View {
         .sheet(item: $sheet) { target in
             ManualWorkoutSheet(editing: target.editing) { row, replacing in
                 Task {
-                    await repo.saveManualWorkout(row, replacing: replacing)
+                    // A copy pre-fills the form but replaces nothing — see `WorkoutSheetTarget.isCopy`.
+                    await repo.saveManualWorkout(row, replacing: target.isCopy ? nil : replacing)
                     // #598: rescore the just-added workout from the strap's HR for its window NOW, so its
                     // average / peak HR, strain and calories appear immediately (from your own strap data)
                     // instead of waiting up to 15 minutes for the next analyze tick. No-ops when the strap
@@ -429,7 +436,9 @@ struct WorkoutsView: View {
 
     // MARK: - Row actions (edit · relabel · dismiss · delete)
 
-    private func editWorkout(_ row: WorkoutRow) { sheet = WorkoutSheetTarget(editing: row) }
+    private func editWorkout(_ row: WorkoutRow, isCopy: Bool = false) {
+        sheet = WorkoutSheetTarget(editing: row, isCopy: isCopy)
+    }
 
     private func relabel(_ row: WorkoutRow, to sport: String) {
         Task {
@@ -1574,7 +1583,7 @@ struct WorkoutsView: View {
             Button("Delete", role: .destructive) { delete(row) }
         case .whoop, .apple, .lifting, .activityFile:
             // Imported history is read-only; offer a copy-to-manual edit path that doesn't touch it.
-            Button("Duplicate as manual…") { editWorkout(asManualCopy(row)) }
+            Button("Duplicate as manual…") { editWorkout(asManualCopy(row), isCopy: true) }
         }
     }
 

android/app/src/main/java/com/noop/ui/WorkoutEditing.kt

@@ -82,6 +82,21 @@ object WorkoutEditing {
         classify(row.source) == WorkoutSource.DETECTED &&
             markers.any { row.startTs < it.endTs && it.startTs < row.endTs }
 
+    /**
+     * What the edit dialog should hand [WhoopRepository.saveManualWorkout] as `replacing`.
+     *
+     * Only a stored MANUAL or DETECTED row is genuinely being replaced. [isCopy] marks "Duplicate as
+     * manual", where the form pre-fills FROM a read-only session but the save is a pure ADD — and it has to
+     * be passed in, because the copy is built with source "manual" so the form treats it as editable, and
+     * so classifies as MANUAL. Testing the source alone silently let every duplicate through carrying the
+     * ORIGINAL's startTs. (#1488)
+     */
+    fun replacingRowFor(editing: WorkoutRow?, isCopy: Boolean): WorkoutRow? =
+        if (isCopy) null else editing?.takeIf {
+            val c = classify(it.source)
+            c == WorkoutSource.MANUAL || c == WorkoutSource.DETECTED
+        }
+
     /** The durable marker for a detected [row] (caller inserts it into `dismissedWorkout`). */
     fun dismissedMarker(row: WorkoutRow): DismissedWorkout =
         DismissedWorkout(deviceId = row.deviceId, startTs = row.startTs, endTs = row.endTs)

android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt

@@ -345,7 +345,7 @@ fun WorkoutsScreen(vm: AppViewModel) {
                     selectionMode = false; selectedKeys = emptySet()
                 },
                 onCancelSelect = { selectionMode = false; selectedKeys = emptySet() },
-                onEdit = { dialog = DialogTarget(it) },
+                onEdit = { editRow, isCopy -> dialog = DialogTarget(editRow, isCopy) },
                 onRelabel = { row, sport ->
                     vm.relabelDetected(row, sport)
                     pendingNoteSport = WorkoutEditing.displaySport(sport)
@@ -372,6 +372,7 @@ fun WorkoutsScreen(vm: AppViewModel) {
     dialog?.let { target ->
         ManualWorkoutDialog(
             editing = target.editing,
+            isCopy = target.isCopy,
             onDismiss = { dialog = null },
             onSave = { row, replacing ->
                 vm.saveManualWorkout(row, replacing)
@@ -382,8 +383,11 @@ fun WorkoutsScreen(vm: AppViewModel) {
     }
 }
 
-/** Drives the manual add/edit dialog. [editing] null = add a new workout, non-null = edit it. */
-private data class DialogTarget(val editing: WorkoutRow?)
+/** Drives the manual add/edit dialog. [editing] null = add a new workout, non-null = edit it.
+ *  [isCopy] marks "Duplicate as manual": the dialog pre-fills FROM a read-only row, but the save is a
+ *  pure ADD and must not travel on as `replacing`. Source alone cannot express this — the copy claims
+ *  "manual" precisely so the form treats it as editable. (#1488) */
+private data class DialogTarget(val editing: WorkoutRow?, val isCopy: Boolean = false)
 
 private data class WorkoutRecoveryTrendPoint(
     val startTs: Long,
@@ -1149,7 +1153,7 @@ private fun SessionsSection(
     onMerge: (List<WorkoutRow>) -> Unit,
     onBulkDelete: (List<WorkoutRow>) -> Unit,
     onCancelSelect: () -> Unit,
-    onEdit: (WorkoutRow) -> Unit,
+    onEdit: (WorkoutRow, Boolean) -> Unit,
     onRelabel: (WorkoutRow, String) -> Unit,
     onDismiss: (WorkoutRow) -> Unit,
     onDelete: (WorkoutRow) -> Unit,
@@ -1349,7 +1353,7 @@ private fun SessionRow(
     selectionMode: Boolean,
     selected: Boolean,
     onToggleRow: (WorkoutRow) -> Unit,
-    onEdit: (WorkoutRow) -> Unit,
+    onEdit: (WorkoutRow, Boolean) -> Unit,
     onRelabel: (WorkoutRow, String) -> Unit,
     onDismiss: (WorkoutRow) -> Unit,
     onDelete: (WorkoutRow) -> Unit,
@@ -1905,7 +1909,7 @@ private fun DetailRow(label: String, value: String) {
 @Composable
 private fun RowActionsMenu(
     row: WorkoutRow,
-    onEdit: (WorkoutRow) -> Unit,
+    onEdit: (WorkoutRow, Boolean) -> Unit,
     onRelabel: (WorkoutRow, String) -> Unit,
     onDismiss: (WorkoutRow) -> Unit,
     onDelete: (WorkoutRow) -> Unit,
@@ -1926,7 +1930,7 @@ private fun RowActionsMenu(
                     )
                     DropdownMenuItem(
                         text = { Text(uiString(R.string.l10n_workouts_screen_edit_details_9e62bb59), style = NoopType.body, color = Palette.textPrimary) },
-                        onClick = { open = false; onEdit(row) },
+                        onClick = { open = false; onEdit(row, false) },
                     )
                     DropdownMenuItem(
                         text = { Text(uiString(R.string.l10n_workouts_screen_dismiss_not_a_workout_560c7bb5), style = NoopType.body, color = Palette.statusCritical) },
@@ -1936,7 +1940,7 @@ private fun RowActionsMenu(
                 WorkoutSource.MANUAL -> {
                     DropdownMenuItem(
                         text = { Text(uiString(R.string.l10n_workouts_screen_edit_b454359e), style = NoopType.body, color = Palette.textPrimary) },
-                        onClick = { open = false; onEdit(row) },
+                        onClick = { open = false; onEdit(row, false) },
                     )
                     DropdownMenuItem(
                         text = { Text(uiString(R.string.l10n_workouts_screen_delete_f6fdbe48), style = NoopType.body, color = Palette.statusCritical) },
@@ -1946,7 +1950,7 @@ private fun RowActionsMenu(
                 WorkoutSource.WHOOP, WorkoutSource.APPLE, WorkoutSource.LIFTING, WorkoutSource.ACTIVITY_FILE -> {
                     DropdownMenuItem(
                         text = { Text(uiString(R.string.l10n_workouts_screen_duplicate_as_manual_2d580d46), style = NoopType.body, color = Palette.textPrimary) },
-                        onClick = { open = false; onEdit(WorkoutEditing.asManualCopy(row)) },
+                        onClick = { open = false; onEdit(WorkoutEditing.asManualCopy(row), true) },
                     )
                 }
             }
@@ -1987,6 +1991,7 @@ private fun Cell(text: String, modifier: Modifier, color: Color? = null) {
 @Composable
 private fun ManualWorkoutDialog(
     editing: WorkoutRow?,
+    isCopy: Boolean = false,
     onDismiss: () -> Unit,
     onSave: (row: WorkoutRow, replacing: WorkoutRow?) -> Unit,
 ) {
@@ -2108,10 +2113,14 @@ private fun ManualWorkoutDialog(
             // it: a manual key change deletes the stale row; a detected original is durably dismissed).
             // Duplicating an imported WHOOP/Apple row is a pure ADD — never pass it, or a changed key
             // would delete the imported original.
-            val replacing = editing?.takeIf {
-                val c = WorkoutEditing.classify(it.source)
-                c == WorkoutSource.MANUAL || c == WorkoutSource.DETECTED
-            }
+            //
+            // The source test alone never enforced that. A duplicate is built with source "manual" so the
+            // form treats it as editable, so it classified as MANUAL and passed straight through, carrying
+            // the ORIGINAL's startTs. That reaches the Health Connect write-back, which deletes by startTs
+            // ALONE (`noop-workout-<startTs>`, no deviceId in the key) — so duplicating a strap session
+            // removed the original's records, and a duplicate saved at a new start left them deleted with
+            // nothing to restore them. [DialogTarget.isCopy] carries what the source cannot. (#1488)
+            val replacing = WorkoutEditing.replacingRowFor(editing, isCopy)
             val context = LocalContext.current
             TextButton(onClick = {
                 built?.let {

android/app/src/test/java/com/noop/ui/DuplicateAsManualTest.kt

@@ -0,0 +1,66 @@
+package com.noop.ui
+
+import com.noop.data.WorkoutRow
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+
+/**
+ * "Duplicate as manual" must not touch the session it copied.
+ *
+ * The action is offered only on READ-ONLY rows — strap, Apple, lifting, activity file — and the menu
+ * describes it as a copy path that leaves the original alone. The copy is built with source "manual" so the
+ * form treats it as editable, which meant the dialog's own guard ("pass `replacing` only for a MANUAL or
+ * DETECTED row") classified it as MANUAL and passed it straight through, carrying the ORIGINAL's startTs.
+ *
+ * The database survived that on Android because its delete is keyed by deviceId. The Health Connect
+ * write-back did not: `deleteExercise` deletes `noop-workout-<startTs>` with no deviceId in the key at all,
+ * so duplicating a strap session removed the original's records — and a duplicate saved at a NEW start left
+ * them deleted with nothing to restore them.
+ */
+class DuplicateAsManualTest {
+
+    private val start = 1_780_000_000L
+
+    private fun row(source: String, deviceId: String = "my-whoop") = WorkoutRow(
+        deviceId = deviceId, startTs = start, endTs = start + 3_600, sport = "Run",
+        source = source, durationS = 3_600.0,
+    )
+
+    /**
+     * The regression: a duplicate replaces nothing, even though its source says "manual". Passing it on is
+     * what let the write-back delete by the original's start.
+     */
+    @Test fun aDuplicateReplacesNothing() {
+        val copyOfStrapSession = row("manual", deviceId = "whoop-ABC123")
+        assertNull(WorkoutEditing.replacingRowFor(copyOfStrapSession, isCopy = true))
+    }
+
+    /** A real edit of a stored manual row still replaces it — the delete-before-write must keep working. */
+    @Test fun editingAManualRowStillReplacesIt() {
+        val stored = row("manual")
+        assertEquals(stored, WorkoutEditing.replacingRowFor(stored, isCopy = false))
+    }
+
+    /** A detected bout also replaces: the repository dismisses the original durably so it can't re-detect. */
+    @Test fun editingADetectedBoutStillReplacesIt() {
+        val detected = row("my-whoop-noop")
+        assertEquals(detected, WorkoutEditing.replacingRowFor(detected, isCopy = false))
+    }
+
+    /**
+     * And the case the source test DID catch stays caught: an imported row that somehow arrives without the
+     * copy flag is still never replaced, so this is a second lock rather than a swap.
+     */
+    @Test fun anImportedRowIsNeverReplacedEvenWithoutTheFlag() {
+        assertNull(WorkoutEditing.replacingRowFor(row("apple-health"), isCopy = false))
+        assertNull(WorkoutEditing.replacingRowFor(row("health-connect"), isCopy = false))
+        assertNull(WorkoutEditing.replacingRowFor(row("whoop"), isCopy = false))
+    }
+
+    /** A fresh add has nothing to replace either way. */
+    @Test fun aFreshAddReplacesNothing() {
+        assertNull(WorkoutEditing.replacingRowFor(null, isCopy = false))
+        assertNull(WorkoutEditing.replacingRowFor(null, isCopy = true))
+    }
+}