SHA-21: FILE_TYPE validation for Workout files - #55
Conversation
Add Workout required messages (workout + workout_step) and fields under ConformanceLevel.FILE_TYPE. Activity rules unchanged; Course and other unimplemented file_id.type values still fail closed. SDK Workout fixtures and strict builder paths are covered; README, design doc status, and Towncrier fragment updated. Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac99c8b97e
ℹ️ 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".
| ``num_valid_steps`` is required on the workout message. Step count is not | ||
| cross-checked against that value: official SDK Workout fixtures sometimes | ||
| disagree (repeat meta-steps), and acceptance requires those fixtures to | ||
| validate clean. ``workout_session`` is optional (multi-sport only). |
There was a problem hiding this comment.
Cross-check the number of valid workout steps
A workout whose num_valid_steps disagrees with its actual steps now passes FILE_TYPE validation—for example, num_valid_steps = 0 with one ordinary step produces no finding. Repeat records explain why this cannot be compared with the raw workout_step count, but they are meta-steps and should be excluded when calculating the valid-step count; otherwise strict builds can emit incorrect metadata that workout consumers use to determine the executable steps.
Useful? React with 👍 / 👎.
| MesgNum.WORKOUT_STEP.value: ( | ||
| 'message_index', | ||
| 'duration_type', | ||
| 'target_type', |
There was a problem hiding this comment.
Require values for duration types that consume them
When a step selects a value-based duration such as WorkoutStepDuration.TIME, this check accepts the step even if field 2 (duration_value/duration_time) is absent, so strict=True can produce a timed workout step with no duration. The requirement must be conditional because OPEN may omit the value, but non-open duration types represented by the duration-value subfields need their corresponding value validated.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #55 +/- ##
==========================================
- Coverage 93.35% 93.25% -0.10%
==========================================
Files 35 35
Lines 2528 2581 +53
Branches 404 421 +17
==========================================
+ Hits 2360 2407 +47
- Misses 100 105 +5
- Partials 68 69 +1 ☔ View full report in Codecov by Harness. |
Cross-check num_valid_steps against non-repeat workout_step count. Require duration_value (or duration_* subfield) for non-OPEN duration types. Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39be4071fa
ℹ️ 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".
| value = _workout_step_duration_value(duration_type) | ||
| if value is None: | ||
| return False | ||
| return value >= int(WorkoutStepDuration.REPEAT_UNTIL_STEPS_CMPLT.value) |
There was a problem hiding this comment.
Match only actual repeat duration values
For workout steps using non-repeat duration types whose enum values are above 6, such as POWER_LESS_THAN (14), this threshold classifies the step as repeat metadata and excludes it from valid_step_count. A valid workout containing one such step with num_valid_steps = 1 therefore fails FILE_TYPE validation with “found 0 non-repeat” (the same affects values 14–16, 19–26, 28–29, and 31); check membership in the actual REPEAT_UNTIL_* values instead of treating every value from 6 onward as a repeat.
Useful? React with 👍 / 👎.
Rebased onto main after Workout (#55). Course structure + fields, course.sport required, timer start/stop ordering around track records. Co-authored-by: multica-agent <github@multica.ai>
Course FILE_TYPE validation. Codex: require course.sport; timer start before records and before stop. Rebased onto main after #55.
Summary
Implements FILE_TYPE rules for Workout FIT files (Multica SHA-21 / design doc Phase 4 §7 I).
IMPLEMENTED_FILE_TYPESwithFileType.WORKOUTworkoutmessage and at least oneworkout_stepworkout.num_valid_steps; each stepmessage_index,duration_type,target_typenews/SHA-21.featureNotes
num_valid_stepsagainst step count (official SDK Workout fixtures with repeat meta-steps disagree)workout_sessionis optional (multi-sport)Test plan
uv run pytest fit_tool/tests/test_validation.py fit_tool/tests/test_workout_files.pyuv run pytest(417 passed, 1 skipped)uv run ruff checkon touched modulesWorkout*.fit+ trainerroad fixture passConformanceLevel.FILE_TYPE