Fix distance-step read path and validate duration_meters (#52 follow-up)

README.md

@@ -345,9 +345,11 @@ Save a reusable cycling/intervals workout **template** to the Coros library. The
 }
 ```
 
+**Distance-based steps:** use `duration_meters` instead of `duration_minutes` for a step that should end at a real distance regardless of pace, e.g. `{"name": "1km @ 4:00/km", "duration_meters": 1000, "intensity_low": 235, "intensity_high": 245}` (pace in sec/km with `intensity_type: 3`). Exactly one of the two keys per step.
+
 `sport_type`: `2` = Indoor Cycling (default), `200` = Road Bike
 
-Returns: `workout_id`, `name`, `total_minutes`, `steps_count`, `message`
+Returns: `workout_id`, `name`, `total_minutes`, `distance_meters_total`, `steps_count`, `message`
 
 ### `save_strength_workout_template`
 

coros_mcp/coros_api.py

@@ -669,7 +669,9 @@ async def fetch_activity_detail(auth: StoredAuth, activity_id: str, sport_type:
 # ---------------------------------------------------------------------------
 
 # sportType=2 = Indoor Cycling (indoor trainer); intensityType=6 = power in watts
-# targetType=2 = time-based (seconds); exerciseType=2 = cycling block
+# targetType=2 = time-based (seconds); targetType=5 = distance-based (meters
+# x100, intensity values x1000 with intensityMultiplier=1000); exerciseType=2
+# = cycling block
 # IntensityType values: 1=weight, 2=HR, 3=pace, 4=speed, 5=none, 6=power, 7=cadence
 
 # Note: the workout API uses sportType=1 for Running; the activity API uses
@@ -706,16 +708,33 @@ async def fetch_activity_detail(auth: StoredAuth, activity_id: str, sport_type:
 _KNOWN_SPORT_TYPES = _RUNNING_ACTIVITY_SPORT_TYPES | _CYCLING_SPORT_TYPES
 
 
+def _unscale(value: float | int | None, divisor: int) -> float | int | None:
+    """Divide a wire-scaled value back down, returning an int when exact."""
+    if value is None:
+        return None
+    result = value / divisor
+    return int(result) if result.is_integer() else result
+
+
 def _parse_workout(item: dict) -> dict:
     exercises = []
     for ex in item.get("exercises", []):
-        exercises.append({
+        parsed = {
             "name": ex.get("name"),
-            "duration_seconds": ex.get("targetValue"),
             "intensity_low": ex.get("intensityValue"),
             "intensity_high": ex.get("intensityValueExtend"),
             "sets": ex.get("sets", 1),
-        })
+        }
+        if ex.get("targetType") == 5:
+            # Distance step (see _target_fields): targetValue is meters x100;
+            # intensityMultiplier=1000 means intensity values are scaled x1000.
+            parsed["distance_meters"] = _unscale(ex.get("targetValue"), 100)
+            if ex.get("intensityMultiplier") == 1000:
+                parsed["intensity_low"] = _unscale(parsed["intensity_low"], 1000)
+                parsed["intensity_high"] = _unscale(parsed["intensity_high"], 1000)
+        else:
+            parsed["duration_seconds"] = ex.get("targetValue")
+        exercises.append(parsed)
     # sportType from the workout API is always a wire ID (runs come back as 1,
     # never 100/102/103), so the wire-keyed lookup below is correct here.
     sport = item.get("sportType")
@@ -862,9 +881,21 @@ def _target_fields(s: dict) -> dict:
                 "duration_minutes or duration_meters, not both"
             )
         if "duration_meters" in s:
+            try:
+                meters = float(s["duration_meters"])
+            except (TypeError, ValueError):
+                raise ValueError(
+                    f"step {s.get('name', '<unnamed>')!r}: duration_meters "
+                    f"must be a number, got {s['duration_meters']!r}"
+                ) from None
+            if meters <= 0:
+                raise ValueError(
+                    f"step {s.get('name', '<unnamed>')!r}: duration_meters "
+                    f"must be positive, got {meters}"
+                )
             return {
                 "targetType": 5,
-                "targetValue": int(s["duration_meters"] * 100),
+                "targetValue": int(meters * 100),
                 "intensityValue": int(low * 1000),
                 "intensityValueExtend": int(high * 1000),
                 "intensityMultiplier": 1000,
@@ -1088,7 +1119,8 @@ async def save_workout_template(
 
     Plain step:
       - name: str — step label (e.g. "10:00 Warm-up")
-      - duration_minutes: float — step duration in minutes
+      - duration_minutes: float — step duration in minutes, OR
+        duration_meters: float — step distance in meters (exactly one of the two)
       - intensity_low: int — lower intensity target (watts, BPM, etc. per intensity_type)
       - intensity_high: int — upper intensity target (0 = open-ended)
 

coros_mcp/server.py

@@ -755,7 +755,8 @@ async def list_workout_templates() -> dict:
     dict with keys: workouts (list), count
     Each entry contains: id, name, sport_type, sport_name,
     estimated_time_seconds, exercise_count, exercises (list of steps with
-    name, duration_seconds, intensity_low, intensity_high, sets)
+    name, intensity_low, intensity_high, sets, and either duration_seconds
+    for time-based steps or distance_meters for distance-based steps)
     """
     auth = await _get_auth()
     if auth is None:

tests/test_workout_payloads.py

@@ -899,3 +899,91 @@ async def gated_fetch(_auth, _sport_type):
     assert call_count == 1
     for r in results:
         assert set(r.keys()) == {"T1010", "T1052", "T1120"}
+
+
+# ---------------------------------------------------------------------------
+# duration_meters validation (follow-up to PR #52 review)
+# ---------------------------------------------------------------------------
+
+
+def _distance_step(value):
+    return {"name": "step", "duration_meters": value, "intensity_low": 235, "intensity_high": 245}
+
+
+def test_negative_duration_meters_raises():
+    with pytest.raises(ValueError, match="must be positive"):
+        _build_workout_program_payload(name="bad", steps=[_distance_step(-100)])
+
+
+def test_zero_duration_meters_raises():
+    with pytest.raises(ValueError, match="must be positive"):
+        _build_workout_program_payload(name="bad", steps=[_distance_step(0)])
+
+
+def test_non_numeric_duration_meters_raises():
+    """A non-numeric string must not hit Python string-repeat ("1000" * 100)."""
+    with pytest.raises(ValueError, match="must be a number"):
+        _build_workout_program_payload(name="bad", steps=[_distance_step("fast")])
+
+
+def test_numeric_string_duration_meters_coerced():
+    """float() coercion means "1000" builds the same step as 1000."""
+    payload = _build_workout_program_payload(name="ok", steps=[_distance_step("1000")])
+    assert payload["exercises"][0]["targetValue"] == 100000
+
+
+# ---------------------------------------------------------------------------
+# _parse_workout round-trip for distance steps (follow-up to PR #52 review)
+# ---------------------------------------------------------------------------
+
+
+def test_parse_workout_distance_step_round_trips():
+    """A distance step read back from the API must not be misreported as a
+    ~28h duration_seconds with x1000 intensity values."""
+    from coros_mcp.coros_api import _parse_workout
+
+    item = {
+        "id": 42,
+        "name": "1km rep",
+        "sportType": 1,
+        "estimatedTime": 0,
+        "exerciseNum": 1,
+        "exercises": [{
+            "name": "1km @ 4:00/km",
+            "targetType": 5,
+            "targetValue": 100000,
+            "intensityValue": 235000,
+            "intensityValueExtend": 245000,
+            "intensityMultiplier": 1000,
+            "sets": 1,
+        }],
+    }
+    ex = _parse_workout(item)["exercises"][0]
+    assert "duration_seconds" not in ex
+    assert ex["distance_meters"] == 1000
+    assert ex["intensity_low"] == 235
+    assert ex["intensity_high"] == 245
+
+
+def test_parse_workout_time_step_unchanged():
+    from coros_mcp.coros_api import _parse_workout
+
+    item = {
+        "id": 43,
+        "name": "tempo",
+        "sportType": 1,
+        "exercises": [{
+            "name": "Tempo",
+            "targetType": 2,
+            "targetValue": 1200,
+            "intensityValue": 240,
+            "intensityValueExtend": 250,
+            "intensityMultiplier": 0,
+            "sets": 1,
+        }],
+    }
+    ex = _parse_workout(item)["exercises"][0]
+    assert "distance_meters" not in ex
+    assert ex["duration_seconds"] == 1200
+    assert ex["intensity_low"] == 240
+    assert ex["intensity_high"] == 250