-
-
Notifications
You must be signed in to change notification settings - Fork 26
fix(health-connect): 1-based setIndex and named exercise mapping (issue #639) #641
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.
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 2 commits into
main
from
fix/issue-639-health-connect-setindex-and-exercise-mapping
Jul 9, 2026
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
228 changes: 228 additions & 0 deletions
228
...Test/kotlin/com/devil/phoenixproject/data/integration/HealthIntegrationSegmentTypeTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,228 @@ | ||
| package com.devil.phoenixproject.data.integration | ||
|
|
||
| import androidx.health.connect.client.records.ExerciseSegment | ||
| import androidx.health.connect.client.records.ExerciseSessionRecord | ||
| import kotlin.test.Test | ||
| import kotlin.test.assertEquals | ||
| import kotlin.test.assertNotEquals | ||
| import kotlin.test.assertTrue | ||
|
|
||
| /** | ||
| * Regression guard for issue #639. | ||
| * | ||
| * Two product failures observed on Health Connect detail pages for the | ||
| * "Tuesday Upper (Old School)" routine on Android 16 / Pixel 10 Pro XL: | ||
| * | ||
| * 1. `ExerciseSegment.setIndex` was being written as 0, 1, 2 (Phoenix's | ||
| * internal 0-based convention) so the user saw "Set 0" instead of "Set 1". | ||
| * Fix: convert at the writer boundary to the 1-based user-visible number | ||
| * while keeping the rest of the internal pipeline 0-based. | ||
| * 2. `segmentTypeForExercise` mapped "Neutral Wide Grip Pulldown" and | ||
| * "Front Raise" to the generic `EXERCISE_SEGMENT_TYPE_WEIGHTLIFTING` | ||
| * (displayed as the literal "Weightlifting"). Fix: extend the mapping so | ||
| * pulldown / lat-pull variants map to `LAT_PULL_DOWN` and front-raise / | ||
| * lateral-raise / leg-* / hip-thrust / back-extension variants map to | ||
| * their specific Health Connect segment types. | ||
| * | ||
| * These tests lock the post-fix invariants so a future refactor cannot | ||
| * silently regress to "Set 0" labels or generic "Weightlifting" segments. | ||
| */ | ||
| class HealthIntegrationSegmentTypeTest { | ||
|
|
||
| @Test | ||
| fun reporterRoutineMapsEachExerciseToItsSpecificSegmentType() { | ||
| // Exercises from "Tuesday Upper (Old School)" (issue #639 reporter routine). | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_BENCH_PRESS, | ||
| segmentTypeForExerciseInternal("Incline Bench Press"), | ||
| "Incline Bench Press must map to BENCH_PRESS so Health Connect shows 'Bench press'.", | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LAT_PULL_DOWN, | ||
| segmentTypeForExerciseInternal("Neutral Wide Grip Pulldown"), | ||
| "Neutral Wide Grip Pulldown must map to LAT_PULL_DOWN, not the generic WEIGHTLIFTING label.", | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_DOUBLE_ARM_TRICEPS_EXTENSION, | ||
| segmentTypeForExerciseInternal("Tricep Pushdown"), | ||
| "Tricep Pushdown must map to DOUBLE_ARM_TRICEPS_EXTENSION.", | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_ARM_CURL, | ||
| segmentTypeForExerciseInternal("Bayesian Cable Curl"), | ||
| "Bayesian Cable Curl must map to ARM_CURL.", | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_FRONT_RAISE, | ||
| segmentTypeForExerciseInternal("Front Raise"), | ||
| "Front Raise must map to FRONT_RAISE, not the generic WEIGHTLIFTING label.", | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun pulldownVariantsMapToLatPullDownNotGenericWeightlifting() { | ||
| // The pre-fix code mapped all of these to EXERCISE_SEGMENT_TYPE_WEIGHTLIFTING, | ||
| // which Health Connect displays as the literal "Weightlifting". | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LAT_PULL_DOWN, | ||
| segmentTypeForExerciseInternal("Lat Pulldown"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LAT_PULL_DOWN, | ||
| segmentTypeForExerciseInternal("Wide-grip lat pull-down"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LAT_PULL_DOWN, | ||
| segmentTypeForExerciseInternal("Cable Pulldown"), | ||
| ) | ||
| // Distinct from pulldown — pull-ups stay on PULL_UP. | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_PULL_UP, | ||
| segmentTypeForExerciseInternal("Pull-Up"), | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun raiseVariantsMapToSpecificTypesNotGenericWeightlifting() { | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_DUMBBELL_FRONT_RAISE, | ||
| segmentTypeForExerciseInternal("Dumbbell Front Raise"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LATERAL_RAISE, | ||
| segmentTypeForExerciseInternal("Lateral Raise"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_DUMBBELL_LATERAL_RAISE, | ||
| segmentTypeForExerciseInternal("Dumbbell Lateral Raise"), | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun legAndHipVariantsMapToSpecificTypes() { | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LEG_CURL, | ||
| segmentTypeForExerciseInternal("Lying Leg Curl"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LEG_CURL, | ||
| segmentTypeForExerciseInternal("Hamstring Curl"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LEG_EXTENSION, | ||
| segmentTypeForExerciseInternal("Leg Extension"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LEG_PRESS, | ||
| segmentTypeForExerciseInternal("Leg Press"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LEG_RAISE, | ||
| segmentTypeForExerciseInternal("Hanging Leg Raise"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_HIP_THRUST, | ||
| segmentTypeForExerciseInternal("Barbell Hip Thrust"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_BACK_EXTENSION, | ||
| segmentTypeForExerciseInternal("Back Extension"), | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun unknownExerciseNameFallsBackSafelyToWeightlifting() { | ||
| // Ad-hoc weightlifting outside a routine is the only legitimate use of | ||
| // the generic WEIGHTLIFTING segment type. Unknown exercise names must | ||
| // still resolve (no exception) so Health Connect insertion does not | ||
| // crash on typos or future exercise names. | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_WEIGHTLIFTING, | ||
| segmentTypeForExerciseInternal(""), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_WEIGHTLIFTING, | ||
| segmentTypeForExerciseInternal("Some Future Exercise XYZ-2000"), | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun existingMappingsArePreserved() { | ||
| // The new mappings must not regress the existing reporter-relevant | ||
| // branches (and a few more stable ones). | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_BENCH_PRESS, | ||
| segmentTypeForExerciseInternal("Flat Bench Press"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_DEADLIFT, | ||
| segmentTypeForExerciseInternal("Conventional Deadlift"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_SQUAT, | ||
| segmentTypeForExerciseInternal("Back Squat"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_SHOULDER_PRESS, | ||
| segmentTypeForExerciseInternal("Overhead Press"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LUNGE, | ||
| segmentTypeForExerciseInternal("Walking Lunge"), | ||
| ) | ||
| assertEquals( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_DUMBBELL_ROW, | ||
| segmentTypeForExerciseInternal("Dumbbell Row"), | ||
| ) | ||
| } | ||
|
9thLevelSoftware marked this conversation as resolved.
|
||
|
|
||
| @Test | ||
| fun pulldownMappingIsCompatibleWithStrengthTrainingSession() { | ||
| // The writer falls back to WEIGHTLIFTING -> UNKNOWN if a segment type | ||
| // is not compatible with the session type (EXERCISE_TYPE_STRENGTH_TRAINING). | ||
| // Lock in that LAT_PULL_DOWN is a valid segment under strength training. | ||
| assertTrue( | ||
| ExerciseSegment.isSegmentTypeCompatibleWithSessionType( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_LAT_PULL_DOWN, | ||
| ExerciseSessionRecord.EXERCISE_TYPE_STRENGTH_TRAINING, | ||
| ), | ||
| "LAT_PULL_DOWN must be compatible with STRENGTH_TRAINING sessions, otherwise the writer would silently downgrade to WEIGHTLIFTING (re-introducing the bug).", | ||
| ) | ||
| assertTrue( | ||
| ExerciseSegment.isSegmentTypeCompatibleWithSessionType( | ||
| ExerciseSegment.EXERCISE_SEGMENT_TYPE_FRONT_RAISE, | ||
| ExerciseSessionRecord.EXERCISE_TYPE_STRENGTH_TRAINING, | ||
| ), | ||
| "FRONT_RAISE must be compatible with STRENGTH_TRAINING sessions.", | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun setIndexWriterConversion_isOneBased() { | ||
| // Call the same helper used by the Android Health Connect writer so this | ||
| // trips if production accidentally reverts to passing 0-based setIndex | ||
| // values through unchanged. | ||
| assertEquals(1, toHealthConnectSetIndex(0), "setNumber 0 must become 1.") | ||
| assertEquals(2, toHealthConnectSetIndex(1), "setNumber 1 must become 2.") | ||
| assertEquals(3, toHealthConnectSetIndex(2), "setNumber 2 must become 3.") | ||
| } | ||
|
|
||
| @Test | ||
| fun setIndexWriterConversion_clampsNegativeToOne() { | ||
| // Defensive: if a future change ever yields a negative setIndex, the | ||
| // writer must still emit >=1 (Health Connect expects 0-based | ||
| // *non-negative* setIndex; we now use 1-based so the floor is 1). | ||
| assertEquals(1, toHealthConnectSetIndex(-1)) | ||
| assertEquals(1, toHealthConnectSetIndex(Int.MIN_VALUE)) | ||
| } | ||
|
|
||
| @Test | ||
| fun setIndexConversionIsNotIdentity() { | ||
| // Tripwire: if a future refactor accidentally removes the +1, this | ||
| // asserts the conversion still happens. (Catches the most plausible | ||
| // regression shape — copying setIndex through unchanged.) | ||
| assertNotEquals(0, toHealthConnectSetIndex(0), "setIndex 0 must NOT be written as 0 (was the pre-fix bug).") | ||
| assertNotEquals(1, toHealthConnectSetIndex(1), "setIndex 1 must NOT be written as 1 (was the pre-fix bug).") | ||
| assertNotEquals(2, toHealthConnectSetIndex(2), "setIndex 2 must NOT be written as 2 (was the pre-fix bug).") | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.