Skip to content

feat(#646): label set types in workout UI #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
9thLevelSoftware merged 1 commit into from
Jul 10, 2026
Merged

feat(#646): label set types in workout UI #647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions shared/src/commonMain/composeResources/values-de/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,5 +700,9 @@
<string name="cd_equipment_rack">Equipment-Rack</string>
<string name="bodyweight_effective_load_includes">Inklusive: %1$s</string>
<string name="bodyweight_effective_load_more">+%1$d weitere</string>
<!-- ==================== Set type labels (Issue #646) ==================== -->
<string name="set_type_calibration">Calibration Reps</string>
<string name="set_type_warmup">Warm-up Set %1$d</string>
<string name="set_type_working">Working Set</string>
Comment on lines +703 to +706

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Android and Compose Multiplatform, string resources automatically fall back to the default values/strings.xml if they are missing from locale-specific files. Explicitly adding untranslated English strings to non-English locale files is redundant, pollutes the translation files, and makes it harder for translation tools/translators to identify untranslated keys. These should be removed and only added when actual German translations are available.


</resources>
4 changes: 4 additions & 0 deletions shared/src/commonMain/composeResources/values-es/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,5 +700,9 @@
<string name="cd_equipment_rack">Rack de equipamiento</string>
<string name="bodyweight_effective_load_includes">Incluye: %1$s</string>
<string name="bodyweight_effective_load_more">+%1$d más</string>
<!-- ==================== Set type labels (Issue #646) ==================== -->
<string name="set_type_calibration">Calibration Reps</string>
<string name="set_type_warmup">Warm-up Set %1$d</string>
<string name="set_type_working">Working Set</string>
Comment on lines +703 to +706

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Android and Compose Multiplatform, string resources automatically fall back to the default values/strings.xml if they are missing from locale-specific files. Explicitly adding untranslated English strings to non-English locale files is redundant, pollutes the translation files, and makes it harder for translation tools/translators to identify untranslated keys. These should be removed and only added when actual Spanish translations are available.


</resources>
4 changes: 4 additions & 0 deletions shared/src/commonMain/composeResources/values-fr/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,5 +700,9 @@
<string name="cd_equipment_rack">Rack d\'équipement</string>
<string name="bodyweight_effective_load_includes">Inclus : %1$s</string>
<string name="bodyweight_effective_load_more">+%1$d de plus</string>
<!-- ==================== Set type labels (Issue #646) ==================== -->
<string name="set_type_calibration">Calibration Reps</string>
<string name="set_type_warmup">Warm-up Set %1$d</string>
<string name="set_type_working">Working Set</string>
Comment on lines +703 to +706

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Android and Compose Multiplatform, string resources automatically fall back to the default values/strings.xml if they are missing from locale-specific files. Explicitly adding untranslated English strings to non-English locale files is redundant, pollutes the translation files, and makes it harder for translation tools/translators to identify untranslated keys. These should be removed and only added when actual French translations are available.


</resources>
4 changes: 4 additions & 0 deletions shared/src/commonMain/composeResources/values-it/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,9 @@
<string name="settings_ble_compat_on">Attivato</string>
<string name="settings_ble_compat_off">Disattivato</string>
<string name="settings_ble_compat_reconnect_hint">Ha effetto alla prossima connessione. Riconnettiti al trainer dopo la modifica.</string>
<!-- ==================== Set type labels (Issue #646) ==================== -->
<string name="set_type_calibration">Calibration Reps</string>
<string name="set_type_warmup">Warm-up Set %1$d</string>
<string name="set_type_working">Working Set</string>
Comment on lines +70 to +73

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Android and Compose Multiplatform, string resources automatically fall back to the default values/strings.xml if they are missing from locale-specific files. Explicitly adding untranslated English strings to non-English locale files is redundant, pollutes the translation files, and makes it harder for translation tools/translators to identify untranslated keys. These should be removed and only added when actual Italian translations are available.


</resources>
4 changes: 4 additions & 0 deletions shared/src/commonMain/composeResources/values-nl/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,5 +679,9 @@
<string name="cd_equipment_rack">Equipment rack</string>
<string name="bodyweight_effective_load_includes">Inclusief: %1$s</string>
<string name="bodyweight_effective_load_more">+%1$d meer</string>
<!-- ==================== Set type labels (Issue #646) ==================== -->
<string name="set_type_calibration">Calibration Reps</string>
<string name="set_type_warmup">Warm-up Set %1$d</string>
<string name="set_type_working">Working Set</string>
Comment on lines +682 to +685

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Android and Compose Multiplatform, string resources automatically fall back to the default values/strings.xml if they are missing from locale-specific files. Explicitly adding untranslated English strings to non-English locale files is redundant, pollutes the translation files, and makes it harder for translation tools/translators to identify untranslated keys. These should be removed and only added when actual Dutch translations are available.


</resources>
4 changes: 4 additions & 0 deletions shared/src/commonMain/composeResources/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,10 @@
<string name="equipment_rack_selected_count">%1$d selected</string>
<string name="equipment_rack_inline_context">Rack: %1$s</string>
<string name="detailed_metrics_not_captured">Detailed metrics were not captured for this set</string>
<!-- ==================== Set type labels (Issue #646) ==================== -->
<string name="set_type_calibration">Calibration Reps</string>
<string name="set_type_warmup">Warm-up Set %1$d</string>
<string name="set_type_working">Working Set</string>
<string name="equipment_rack_category_weighted_vest">Weighted vest</string>
<string name="equipment_rack_category_dip_belt">Dip belt</string>
<string name="equipment_rack_category_chains">Chains</string>
Expand Down
23 changes: 23 additions & 0 deletions shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/SetReadyScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import com.devil.phoenixproject.domain.model.RepCount
import com.devil.phoenixproject.presentation.util.SetTypeLabel
import com.devil.phoenixproject.presentation.util.setTypeLabel
import com.devil.phoenixproject.ui.theme.screenBackgroundBrush
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.semantics.Role
Expand Down Expand Up @@ -106,6 +109,8 @@ import vitruvianprojectphoenix.shared.generated.resources.equipment_rack_save_ov
import vitruvianprojectphoenix.shared.generated.resources.equipment_rack_save_override_title
import vitruvianprojectphoenix.shared.generated.resources.exit_routine_message
import vitruvianprojectphoenix.shared.generated.resources.exit_routine_title
import vitruvianprojectphoenix.shared.generated.resources.set_type_warmup
import vitruvianprojectphoenix.shared.generated.resources.set_type_working
import vitruvianprojectphoenix.shared.generated.resources.target_reps

/**
Expand All @@ -124,6 +129,9 @@ fun SetReadyScreen(navController: NavController, viewModel: MainViewModel, exerc
val weightRecommendation by viewModel.weightAdjustmentRecommendation.collectAsState()
val rackItems by viewModel.rackItems.collectAsState()
val activeRackItemIds by viewModel.activeRackItemIds.collectAsState()
// Issue #646: drive set-type badge in SetReady header
val currentWarmupSetIndex by viewModel.currentWarmupSetIndex.collectAsState()
val repCount by viewModel.repCount.collectAsState()

// Get current state
val setReadyState = routineFlowState as? RoutineFlowState.SetReady
Expand Down Expand Up @@ -435,6 +443,21 @@ fun SetReadyScreen(navController: NavController, viewModel: MainViewModel, exerc
style = MaterialTheme.typography.labelMedium,
color = MaterialTheme.colorScheme.onPrimaryContainer.copy(alpha = 0.72f),
)
// Issue #646: set-type badge — calibration label suppressed at SetReady
// (calibration reps run during the active set, not at this screen)
val setType = setTypeLabel(repCount, currentWarmupSetIndex, showCalibrationLabel = false)
val setTypeText = when (setType) {
is SetTypeLabel.Warmup -> stringResource(Res.string.set_type_warmup, setType.setNumber)
SetTypeLabel.Working -> stringResource(Res.string.set_type_working)
SetTypeLabel.Calibration -> "" // unreachable; showCalibrationLabel=false

@kilo-code-bot kilo-code-bot Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 The Roast: Calibration -> "" annotated // unreachable, wrapped in if (setTypeText.isNotEmpty()). The compiler can prove the branch is dead; the comment admits it; the runtime guard defends against it. Three layers of "I know this is awkward" stacked on a helper that knows too much.

🩹 The Fix: Split the helper. Keep setTypeLabel(repCount, currentWarmupSetIndex) returning Calibration | Warmup(n) | Working for the active set (ExecutionPage). Add a sibling readySetTypeLabel(repCount, currentWarmupSetIndex) returning just Warmup(n) | Working for SetReady. The unreachable branch and the isNotEmpty() guard both vanish; the call sites read more honestly.

📏 Severity: suggestion

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

}
if (setTypeText.isNotEmpty()) {
Text(
text = setTypeText,
style = MaterialTheme.typography.labelMedium,
color = MaterialTheme.colorScheme.onPrimaryContainer.copy(alpha = 0.72f),
)
}
}
}
}
Expand Down
19 changes: 19 additions & 0 deletions shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/WorkoutHud.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ import com.devil.phoenixproject.presentation.components.ForceCurveMiniGraph
import com.devil.phoenixproject.presentation.components.StableRepProgress
import com.devil.phoenixproject.presentation.util.LocalWindowSizeClass
import com.devil.phoenixproject.presentation.util.ResponsiveDimensions
import com.devil.phoenixproject.presentation.util.SetTypeLabel
import com.devil.phoenixproject.presentation.util.WindowWidthSizeClass
import com.devil.phoenixproject.presentation.util.isCompactAccessibilityLayout
import com.devil.phoenixproject.presentation.util.setTypeLabel
import com.devil.phoenixproject.ui.theme.AccessibilityTheme
import com.devil.phoenixproject.ui.theme.velocityZoneColor
import com.devil.phoenixproject.ui.theme.velocityZoneLabel
Expand Down Expand Up @@ -82,6 +84,7 @@ fun WorkoutHud(
onResetExerciseTimer: () -> Unit = {},
velocityLossThresholdPercent: Int = 20,
rackLoadAdjustment: RackLoadAdjustment = RackLoadAdjustment(),
currentWarmupSetIndex: Int = -1, // Issue #646: -1 = no warm-up phase
modifier: Modifier = Modifier,
) {
// Determine if we're in Echo mode
Expand Down Expand Up @@ -172,6 +175,7 @@ fun WorkoutHud(
onPauseExerciseTimer = onPauseExerciseTimer,
onResumeExerciseTimer = onResumeExerciseTimer,
onResetExerciseTimer = onResetExerciseTimer,
currentWarmupSetIndex = currentWarmupSetIndex,
)
}

Expand Down Expand Up @@ -471,6 +475,7 @@ private fun ExecutionPage(
onPauseExerciseTimer: () -> Unit = {},
onResumeExerciseTimer: () -> Unit = {},
onResetExerciseTimer: () -> Unit = {},
currentWarmupSetIndex: Int = -1, // Issue #646: -1 = no warm-up phase
) {
// Issue #192: Check if this is a timed exercise
val isTimedExercise = timedExerciseRemainingSeconds != null
Expand Down Expand Up @@ -506,6 +511,20 @@ private fun ExecutionPage(
)
}

// Issue #646: set-type badge (Calibration Reps / Warm-up Set N / Working Set)
val setType = setTypeLabel(repCount, currentWarmupSetIndex, showCalibrationLabel = true)

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Suppress calibration labels for bodyweight sets

When a routine reaches a bodyweight exercise while connected, ActiveSessionEngine.startWorkout configures warmupTarget = 0 and stores a fresh RepCount() with isWarmupComplete = false, so this unconditional showCalibrationLabel = true makes setTypeLabel return Calibration before the bodyweight timer branch renders. The HUD then labels a bodyweight set as “Calibration Reps” even though no firmware calibration reps run for bodyweight exercises; pass the exercise type through here or disable the calibration label for isCurrentExerciseBodyweight.

Useful? React with 👍 / 👎.

val setTypeText = when (setType) {
SetTypeLabel.Calibration -> stringResource(Res.string.set_type_calibration)
is SetTypeLabel.Warmup -> stringResource(Res.string.set_type_warmup, setType.setNumber)
SetTypeLabel.Working -> stringResource(Res.string.set_type_working)
}
Text(
text = setTypeText,
style = MaterialTheme.typography.labelLarge,
fontWeight = FontWeight.Medium,
color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.85f),
)

Spacer(modifier = Modifier.height(24.dp))
}

Expand Down
4 changes: 4 additions & 0 deletions shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/WorkoutTab.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ fun WorkoutTab(
velocityLossThresholdPercent = state.velocityLossThresholdPercent,
weightStepKg = state.weightStepKg,
rackLoadAdjustment = state.rackLoadAdjustment,
currentWarmupSetIndex = state.currentWarmupSetIndex,
)
}

Expand Down Expand Up @@ -305,6 +306,8 @@ fun WorkoutTab(
// Issue #266/#410: Configurable weight step from user preferences (kg)
weightStepKg: Float = 0.25f,
rackLoadAdjustment: RackLoadAdjustment = RackLoadAdjustment(),
// Issue #646: -1 means not currently in variable warm-up phase
currentWarmupSetIndex: Int = -1,
) {
// Note: HapticFeedbackEffect is now global in EnhancedMainScreen
// No need for local haptic effect here
Expand Down Expand Up @@ -342,6 +345,7 @@ fun WorkoutTab(
onResetExerciseTimer = onResetExerciseTimer,
velocityLossThresholdPercent = velocityLossThresholdPercent,
rackLoadAdjustment = rackLoadAdjustment,
currentWarmupSetIndex = currentWarmupSetIndex,
modifier = Modifier.fillMaxSize(),
)

Expand Down
39 changes: 39 additions & 0 deletions shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/util/SetTypeLabel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.devil.phoenixproject.presentation.util

import com.devil.phoenixproject.domain.model.RepCount

/**
* Issue #646: pure-Kotlin helper for the workout set-type badge.
*
* Decides whether the active workout header should read
* "Calibration Reps" / "Warm-up Set N" / "Working Set".
*
* Kept free of `@Composable` and string resources so it stays trivially
* testable as plain JVM code. Call sites resolve the sealed label to a
* locale string via [com.devil.phoenixproject.presentation.R] mappings.
*
* `showCalibrationLabel` is `false` at Set Ready because the firmware
* calibration buffer runs during the active set, not at Set Ready.
*/
sealed class SetTypeLabel {
data object Calibration : SetTypeLabel()
data class Warmup(val setNumber: Int) : SetTypeLabel()
data object Working : SetTypeLabel()
}

// ponytail: 3 mirrors Constants.DEFAULT_WARMUP_REPS; revisit if firmware
// calibration buffer size ever changes.
private const val CALIBRATION_BUFFER_REPS = 3

@kilo-code-bot kilo-code-bot Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 The Roast: You wrote CALIBRATION_BUFFER_REPS = 3 and then helpfully left a ponytail: comment telling future-you it mirrors Constants.DEFAULT_WARMUP_REPS. That's a Post-it note on a landmine. The engine uses Constants.DEFAULT_WARMUP_REPS in ~10 call sites; the day someone bumps that constant for a firmware change, this helper silently disagrees with the engine and the UI cheerfully labels live ROM reps as "Working Set."

🩹 The Fix: Delete CALIBRATION_BUFFER_REPS and import com.devil.phoenixproject.util.Constants, then reference Constants.DEFAULT_WARMUP_REPS directly in the when clause. One source of truth, one landmine defused. Add a regression test that exercises a non-3 buffer size if you want belt-and-suspenders.

📏 Severity: warning

Reply with @kilocode-bot fix it to have Kilo Code address this issue.


fun setTypeLabel(
repCount: RepCount,
currentWarmupSetIndex: Int,
showCalibrationLabel: Boolean,
): SetTypeLabel = when {
showCalibrationLabel && repCount.warmupReps < CALIBRATION_BUFFER_REPS && !repCount.isWarmupComplete ->
SetTypeLabel.Calibration
currentWarmupSetIndex >= 0 ->
SetTypeLabel.Warmup(currentWarmupSetIndex + 1)
else ->
SetTypeLabel.Working
}
Comment on lines +32 to +39

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

During the active set of a warm-up set, the first 3 reps are unweighted calibration reps. However, labeling them as Calibration Reps instead of Warm-up Set N defeats the core purpose of this feature (which is to remind the user that they are in a warm-up set). If a warm-up set is configured with 3 or fewer reps, the user will never see the warm-up label during the active set.

By reordering the conditions so that currentWarmupSetIndex >= 0 takes precedence, we ensure that warm-up sets are always clearly identified as such from the very first rep, while working sets still correctly display Calibration Reps for the first 3 reps.

Suggested change
): SetTypeLabel = when {
showCalibrationLabel && repCount.warmupReps < CALIBRATION_BUFFER_REPS && !repCount.isWarmupComplete ->
SetTypeLabel.Calibration
currentWarmupSetIndex >= 0 ->
SetTypeLabel.Warmup(currentWarmupSetIndex + 1)
else ->
SetTypeLabel.Working
}
): SetTypeLabel = when {
currentWarmupSetIndex >= 0 ->
SetTypeLabel.Warmup(currentWarmupSetIndex + 1)
showCalibrationLabel && repCount.warmupReps < CALIBRATION_BUFFER_REPS && !repCount.isWarmupComplete ->
SetTypeLabel.Calibration
else ->
SetTypeLabel.Working
}

42 changes: 42 additions & 0 deletions shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/SetTypeLabelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.devil.phoenixproject.presentation

import com.devil.phoenixproject.domain.model.RepCount
import com.devil.phoenixproject.presentation.util.SetTypeLabel
import com.devil.phoenixproject.presentation.util.setTypeLabel
import kotlin.test.Test
import kotlin.test.assertEquals

/**
* Issue #646: pin the helper that drives the active-workout set-type badge.
* Pure Kotlin, no Compose runtime.
*/
class SetTypeLabelTest {

@Test
fun calibration_when_buffer_incomplete() {
val rc = RepCount(warmupReps = 0, isWarmupComplete = false)
val label = setTypeLabel(rc, currentWarmupSetIndex = -1, showCalibrationLabel = true)
assertEquals(SetTypeLabel.Calibration, label)
}

@Test
fun working_when_buffer_complete() {
val rc = RepCount(warmupReps = 3, isWarmupComplete = true)
val label = setTypeLabel(rc, currentWarmupSetIndex = -1, showCalibrationLabel = true)
assertEquals(SetTypeLabel.Working, label)
}

@Test
fun warmup_during_variable_warmup_set_1() {
val rc = RepCount(warmupReps = 0, isWarmupComplete = false)
val label = setTypeLabel(rc, currentWarmupSetIndex = 0, showCalibrationLabel = false)
assertEquals(SetTypeLabel.Warmup(setNumber = 1), label)
}

@Test
fun working_when_no_warmup_index_at_setready() {
val rc = RepCount(warmupReps = 0, isWarmupComplete = false)
val label = setTypeLabel(rc, currentWarmupSetIndex = -1, showCalibrationLabel = false)
assertEquals(SetTypeLabel.Working, label)
}
Comment on lines +35 to +41

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a unit test to verify and pin the precedence behavior where warm-up sets take precedence over calibration reps during the active set.

Suggested change
@Test
fun working_when_no_warmup_index_at_setready() {
val rc = RepCount(warmupReps = 0, isWarmupComplete = false)
val label = setTypeLabel(rc, currentWarmupSetIndex = -1, showCalibrationLabel = false)
assertEquals(SetTypeLabel.Working, label)
}
@Test
fun working_when_no_warmup_index_at_setready() {
val rc = RepCount(warmupReps = 0, isWarmupComplete = false)
val label = setTypeLabel(rc, currentWarmupSetIndex = -1, showCalibrationLabel = false)
assertEquals(SetTypeLabel.Working, label)
}
@Test
fun warmup_takes_precedence_over_calibration_during_active_set() {
val rc = RepCount(warmupReps = 0, isWarmupComplete = false)
val label = setTypeLabel(rc, currentWarmupSetIndex = 0, showCalibrationLabel = true)
assertEquals(SetTypeLabel.Warmup(setNumber = 1), label)
}
}

}
Loading