newton/_src/utils/import_mjcf.py (1)

1335-1338: Minor: Missing blank line before the next section comment.

For consistency with the rest of the file (e.g., line 1254-1255 has a blank line before # parse contact pairs), add a blank line after the tendon parsing block.

📝 Suggested fix

                 if verbose:
                     joint_names_str = ", ".join(f"{builder.joint_key[j]}*{c}" for j, c in joint_entries)
                     print(f"Parsed fixed tendon: {tendon_name} ({joint_names_str})")
+
     # -----------------
     # parse actuators

newton/_src/solvers/flags.py (1)

51-52: Consider expanding the docstring for consistency with other flags.

Other flag docstrings enumerate the specific properties (e.g., line 34 lists "joint_target_ke, joint_target_kd, joint_effort_limit..."). The current docstring "eg stiffness" is less precise.

📝 Suggested improvement

     FIXED_TENDON_PROPERTIES = 1 << 7
-    """Indicates fixed tendon properties: eg stiffness."""
+    """Indicates fixed tendon property updates: tendon_stiffness, tendon_damping, tendon_frictionloss, tendon_range, tendon_margin, tendon_solref, tendon_solimp, tendon_armature, tendon_actfrcrange."""

newton/tests/test_import_mjcf.py (1)

994-1007: Potential brittleness: hard‑coded tendon_invweight0 values.
Line 994‑1007 compares exact MuJoCo‑computed values; these can vary across MuJoCo versions or platforms. Please verify stability against the supported MuJoCo build, and consider loosening or guarding this check if it proves version‑sensitive.

newton/tests/test_fixed_tendon.py (1)

177-177: Remove unused variable.

start_tendon_length is computed but never used. As per static analysis hint, this should be removed or used for additional assertions if intended.

♻️ Proposed fix

-        start_tendon_length = joint_start_positions[0] * coeff0 + joint_start_positions[1] * coeff1
-
         builder = newton.ModelBuilder()

newton/tests/test_fixed_tendon.py (1)

177-177: Remove unused variable.

start_tendon_length is computed but never used. Either remove it or use it in an assertion/logging for clarity.

♻️ Proposed fix

-        start_tendon_length = joint_start_positions[0] * coeff0 + joint_start_positions[1] * coeff1
-
         builder = newton.ModelBuilder()

newton/tests/test_fixed_tendon.py (1)

95-96: Fix the misleading tendon-length comment.
The comment uses exponent notation and incorrect arithmetic; it should reflect the linear combination and the correct value.

📝 Proposed fix

-        # Length of tendon at start is: pos**coef0 + pos1*coef1 = 2*0.5 + 0*0.0 = 1.0
+        # Length of tendon at start is: pos0*coef0 + pos1*coef1 = 0.5*1 + 0.0*1 = 0.5

newton/tests/test_fixed_tendon.py (2)

79-80: Rename unused loop variables to satisfy linting.
Ruff flags i as unused in both loops; prefer _i.

♻️ Proposed fix

-        for i in range(0, 2):
+        for _i in range(0, 2):
             builder.add_world(individual_builder)
@@
-        for i in range(0, 2):
+        for _i in range(0, 2):
             builder.add_world(individual_builder)

Also applies to: 209-210

---

203-203: Remove unused start_tendon_length.
It’s assigned but never used (Ruff F841).

♻️ Proposed fix

-        start_tendon_length = joint_start_positions[0] * coeff0 + joint_start_positions[1] * coeff1

newton/tests/test_import_mjcf.py (2)

27-27: Tests need conditional import or skip decorators for optional MuJoCo dependency.

The module-level import from newton.solvers import SolverMuJoCo will cause the entire test file to fail importing in environments where the optional mujoco or mujoco_warp packages are not installed.

🔧 Suggested fix using pytest.importorskip

-from newton.solvers import SolverMuJoCo
+# Guard against missing optional mujoco dependencies
+try:
+    from newton.solvers import SolverMuJoCo
+    HAS_MUJOCO = True
+except ImportError:
+    HAS_MUJOCO = False
+    SolverMuJoCo = None

Then decorate the test methods using SolverMuJoCo:

`@unittest.skipUnless`(HAS_MUJOCO, "mujoco not available")
def test_single_mujoco_fixed_tendon_parsing(self):
    ...

---

1027-1047: Comments don't match the code being tested.

Lines 1027 and 1038 both say "Check solver solimplimit" but the code is actually checking solimpfriction values (tendon_solimp_fri and tendon_solimp_friction).

newton/_src/solvers/mujoco/solver_mujoco.py (3)

991-998: Missing bounds validation for tendon joint ranges.

The code doesn't validate that joint_start + joint_num <= joint_entry_count before iterating. If the tendon attributes specify an invalid range, this could cause an out-of-bounds array access on tendon_joint or tendon_coef.

Additionally, the None check for tendon_joint/tendon_coef at lines 996-997 is inside the loop rather than before entering it when joint_num > 0.

🐛 Proposed fix

             # Add joints for this fixed tendon's linear combination
             joint_start = int(tendon_joint_adr[i])
             joint_num = int(tendon_joint_num[i])
+
+            # Validate range before iterating
+            if joint_num > 0:
+                if tendon_joint is None or tendon_coef is None:
+                    warnings.warn(
+                        f"Skipping tendon {i}: tendon_joint/tendon_coef must be provided when tendon_joint_num > 0.",
+                        stacklevel=2,
+                    )
+                    continue
+                if joint_start < 0 or joint_start + joint_num > joint_entry_count:
+                    warnings.warn(
+                        f"Skipping tendon {i}: invalid joint range (start={joint_start}, num={joint_num}, max={joint_entry_count}).",
+                        stacklevel=2,
+                    )
+                    continue

             for j in range(joint_start, joint_start + joint_num):
-                if tendon_joint is None or tendon_coef is None:
-                    break
-
                 newton_joint = int(tendon_joint[j])

---

2746-2763: Bug: Tendon mapping breaks when global tendons (world < 0) are present.

The calculation tendons_per_world = total_tendons // model.num_worlds assumes all tendons are world-specific. If any tendon has world < 0 (global), this division will be incorrect, causing the modulo-based template mapping to alias global tendons to wrong Newton indices.

For example, with 1 global tendon and 2 tendons per world across 2 worlds (5 total), tendons_per_world becomes 2, causing incorrect remapping.

🐛 Suggested fix

                 mujoco_attrs = getattr(model, "mujoco", None)
                 tendon_world = getattr(mujoco_attrs, "tendon_world", None) if mujoco_attrs else None
-                if tendon_world is not None:
-                    total_tendons = len(tendon_world)
-                    tendons_per_world = total_tendons // model.num_worlds if model.num_worlds > 0 else total_tendons
-                else:
-                    tendons_per_world = ntendon
+                tendon_world_np = tendon_world.numpy() if tendon_world is not None else None
+                if tendon_world_np is not None and model.num_worlds > 0:
+                    # Count tendons from the first non-global world only
+                    tendons_per_world = int(np.sum(tendon_world_np == first_world))
+                else:
+                    tendons_per_world = ntendon
                 mjc_tendon_to_newton_tendon_np = np.full((nworld, ntendon), -1, dtype=np.int32)
                 for mjc_tendon, newton_tendon in enumerate(selected_tendons):
+                    # Handle global tendons specially
+                    if tendon_world_np is not None and tendon_world_np[newton_tendon] < 0:
+                        for w in range(nworld):
+                            mjc_tendon_to_newton_tendon_np[w, mjc_tendon] = newton_tendon
+                        continue
                     template_tendon = newton_tendon % tendons_per_world if tendons_per_world > 0 else newton_tendon
                     for w in range(nworld):
                         mjc_tendon_to_newton_tendon_np[w, mjc_tendon] = w * tendons_per_world + template_tendon

---

3244-3249: Docstring incorrectly mentions springlength.

The docstring lists "springlength" among the updated properties, but the implementation explicitly skips it (as noted in the comment at lines 3265-3266). The docstring should be updated to match the actual behavior.

✏️ Suggested fix

     def update_tendon_properties(self):
         """Update fixed tendon properties in the MuJoCo model.

-        Updates tendon stiffness, damping, frictionloss, range, margin, solref, solimp,
-        armature, and actfrcrange from Newton custom attributes.
+        Updates tendon stiffness, damping, frictionloss, range, margin, solref_limit, solimp_limit,
+        solref_friction, solimp_friction, armature, and actfrcrange from Newton custom attributes.
+
+        Note: springlength is intentionally not updated at runtime (MuJoCo auto-computes when set to -1).
         """

newton/_src/solvers/mujoco/solver_mujoco.py (1)

2874-2888: Consider removing commented-out tendon fields.

Per a previous discussion, commented-out fields in this list were intended to be removed once implementation is complete, rather than left as comments. The commented fields tendon_length0 and tendon_invweight0 (lines 2887-2888) could be removed, with the explanation moved to a separate note if needed.

This is a minor style consideration and not blocking.

newton/tests/test_import_mjcf.py (2)

1027-1047: Comments don't match the code being tested.

Lines 1027 and 1038 say "Check solver solimplimit" / "Check model solimplimit" but the code is checking solimpfriction values (tendon_solimp_fri and tendon_solimp_friction).

🔧 Suggested fix

-                # Check solver solimplimit
+                # Check solver solimpfriction
                 for k in range(0, 5):
                     expected = expected_solimpfriction[0][j][k]
                     measured = solver.mjw_model.tendon_solimp_fri.numpy()[i][j][k]
                     ...

-                # Check model solimpfriction
+                # Check model solimpfriction
                 for k in range(0, 5):

---

27-27: Module-level import will break tests when MuJoCo is not installed.

SolverMuJoCo requires optional dependencies (mujoco, mujoco_warp). This import at module level causes all tests in this file to fail when these dependencies are unavailable, even tests that don't use SolverMuJoCo.

🛠️ Suggested fix: Use conditional import with skip decorator

 import newton
 import newton.examples
 from newton._src.geometry.types import GeoType
 from newton._src.sim.builder import ShapeFlags
-from newton.solvers import SolverMuJoCo
+
+try:
+    from newton.solvers import SolverMuJoCo
+    HAS_MUJOCO = True
+except ImportError:
+    HAS_MUJOCO = False

Then decorate MuJoCo-dependent tests:

`@unittest.skipUnless`(HAS_MUJOCO, "MuJoCo dependencies not available")
def test_single_mujoco_fixed_tendon_parsing(self):
    ...

newton/_src/solvers/mujoco/solver_mujoco.py (2)

2746-2763: Fix tendon mapping when global tendons are present.

The current mapping logic uses tendons_per_world = total_tendons // model.num_worlds which doesn't correctly handle global tendons (tendon_world < 0). Global tendons are counted in total_tendons but should not be divided across worlds. This will cause incorrect Newton tendon indices to be looked up for global tendons and may alias mappings incorrectly.

🐛 Suggested fix from past review

-                if tendon_world is not None:
-                    total_tendons = len(tendon_world)
-                    tendons_per_world = total_tendons // model.num_worlds if model.num_worlds > 0 else total_tendons
-                else:
-                    tendons_per_world = ntendon
+                tendon_world_np = tendon_world.numpy() if tendon_world is not None else None
+                if tendon_world_np is not None and model.num_worlds > 0:
+                    tendons_per_world = int(np.sum(tendon_world_np == first_world))
+                else:
+                    tendons_per_world = ntendon
                 mjc_tendon_to_newton_tendon_np = np.full((nworld, ntendon), -1, dtype=np.int32)
                 for mjc_tendon, newton_tendon in enumerate(selected_tendons):
+                    if tendon_world_np is not None and tendon_world_np[newton_tendon] < 0:
+                        # Global tendon: map to the same Newton index for all worlds
+                        for w in range(nworld):
+                            mjc_tendon_to_newton_tendon_np[w, mjc_tendon] = newton_tendon
+                        continue
                     template_tendon = newton_tendon % tendons_per_world if tendons_per_world > 0 else newton_tendon
                     for w in range(nworld):
                         mjc_tendon_to_newton_tendon_np[w, mjc_tendon] = w * tendons_per_world + template_tendon

---

991-998: Add bounds validation for tendon joint ranges.

The loop iterates from joint_start to joint_start + joint_num without validating that this range is within bounds of tendon_joint and tendon_coef arrays. If tendon_joint_adr or tendon_joint_num contains invalid values, this will cause an IndexError.

🐛 Proposed fix

             # Add joints for this fixed tendon's linear combination
             joint_start = int(tendon_joint_adr[i])
             joint_num = int(tendon_joint_num[i])
 
+            # Validate joint range bounds
+            if joint_start < 0 or joint_num < 0:
+                warnings.warn(
+                    f"Skipping tendon {i}: invalid joint range (start={joint_start}, num={joint_num}).",
+                    stacklevel=2,
+                )
+                continue
+            if tendon_joint is not None and joint_start + joint_num > len(tendon_joint):
+                warnings.warn(
+                    f"Skipping tendon {i}: joint range exceeds array bounds "
+                    f"(start={joint_start}, num={joint_num}, max={len(tendon_joint)}).",
+                    stacklevel=2,
+                )
+                continue
+
             for j in range(joint_start, joint_start + joint_num):