fix(sim/mujoco): metric depth passthrough + capture fd-2 depth warning + silence export_xml attach noise (#891)

* fix(sim/mujoco): metric depth passthrough + capture fd-2 depth warning + silence export_xml attach noise

render_depth treated MuJoCo's depth output as a normalized [0,1] OpenGL
buffer and re-linearized it with znear*zfar/(zfar - d*(zfar-znear)). But
MuJoCo >= 3.0's Renderer.enable_depth_rendering() returns METRIC depth in
meters directly, so that re-projection collapsed near-field depth onto the
near-clip plane: a scene whose nearest surface sits at 0.41 m reported a
minimum of 0.01 m (znear) regardless of content. Consume the metric array
as-is; only sanitize NaN/+inf -> far-clip, -inf/negative -> 0, clamp to
[0, zfar].

The ARB_clip_control depth warning is a C-level write to fd 2, which
contextlib.redirect_stderr cannot see (it only swaps sys.stderr), so the
warning both leaked to the console and never reached the response text it
was meant to populate. Add a reusable capture_stderr_fd() context manager
(os.dup2-based fd-2 capture, mirroring filter_mujoco_attach_noise, with the
same no-op fallback when fd 2 can't be duped) and use it in render_depth:
surface the notice in the response text, drop the duplicate console line
(log at DEBUG), and pass any other stderr lines through unchanged.

export_xml called spec.to_xml() raw while every other call site wraps it in
filter_mujoco_attach_noise(), so it spammed benign 'Attach conflict ...
keeping parent value' chatter to the console for every attached-robot scene.
Wrap it too.

Tests: rewrite test_render_depth_linearizes_normalized_buffer_to_meters
(which enshrined the bug) as test_render_depth_passes_through_metric_depth,
add test_render_depth_sanitizes_nan_and_inf. Both fail on pre-fix code.

* fix(sim/mujoco): document best-effort stderr-forward except per review feedback

* review(sim/mujoco): R3 -- add explanatory comments to bare except clauses (addresses CodeQL threads backend.py:387,400)

---------

Co-authored-by: strands-agent <217235299+strands-agent@users.noreply.github.com>
Co-authored-by: strands-robots <agent@strands.local>
Co-authored-by: cagataycali <cagatay@cali.so>

Author: 4 people

strands_robots/simulation/mujoco/backend.py

@@ -347,3 +347,55 @@ def filter_mujoco_attach_noise():
                 )
         # Tear down the UserWarning filter last.
         _wctx.__exit__(None, None, None)
+
+
+@contextlib.contextmanager
+def capture_stderr_fd():
+    """Capture C-level (fd 2) stderr writes into a list-wrapped string.
+
+    Unlike ``contextlib.redirect_stderr`` -- which only swaps Python's
+    ``sys.stderr`` object and is blind to writes coming from C extensions
+    such as MuJoCo's OpenGL backend -- this redirects the underlying file
+    descriptor 2, so warnings like the one-time ``ARB_clip_control`` notice
+    emitted by ``renderer.render()`` are actually captured.
+
+    Yields a single-element list; after the block exits, ``box[0]`` holds the
+    captured text (possibly empty).
+
+    No-op-ish fallback: if fd 2 can't be duped (captured stderr, pytest
+    capsys, Jupyter), it yields an empty box and lets writes pass through
+    rather than breaking the wrapped call.
+    """
+    box = [""]
+    try:
+        orig_fd = sys.stderr.fileno()
+        saved_fd = os.dup(orig_fd)
+    except (AttributeError, OSError, ValueError):
+        # No real fd to capture; yield empty and pass through.
+        yield box
+        return
+
+    tmp = tempfile.TemporaryFile(mode="w+b")
+    try:
+        with _noise_lock:
+            os.dup2(tmp.fileno(), orig_fd)
+            try:
+                yield box
+            finally:
+                try:
+                    sys.stderr.flush()
+                except (ValueError, OSError):
+                    pass  # stderr may be closed/detached (pytest capsys, Jupyter)
+                os.dup2(saved_fd, orig_fd)
+    finally:
+        try:
+            tmp.seek(0)
+            box[0] = tmp.read().decode(errors="replace")
+        except OSError:
+            box[0] = ""
+        finally:
+            tmp.close()
+            try:
+                os.close(saved_fd)
+            except OSError:
+                pass  # fd may already be closed during interpreter shutdown

strands_robots/simulation/mujoco/physics.py

@@ -20,7 +20,11 @@
 
 import numpy as np
 
-from strands_robots.simulation.mujoco.backend import _NO_WORLD_MSG, _ensure_mujoco
+from strands_robots.simulation.mujoco.backend import (
+    _NO_WORLD_MSG,
+    _ensure_mujoco,
+    filter_mujoco_attach_noise,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -1205,7 +1209,12 @@ def export_xml(self, output_path: str | None = None) -> dict[str, Any]:
             }
 
         try:
-            xml = spec.to_xml()
+            # spec.to_xml() emits benign "Attach conflict ... keeping parent
+            # value" chatter (Python UserWarning + raw fd-2 writes) for every
+            # attached robot scene. Every other call site wraps it; export_xml
+            # must too, or the noise leaks straight to the user's console.
+            with filter_mujoco_attach_noise():
+                xml = spec.to_xml()
         except Exception as e:
             return {"status": "error", "content": [{"text": f"spec.to_xml() failed: {e}"}]}
 

strands_robots/simulation/mujoco/rendering.py

@@ -4,7 +4,12 @@
 import logging
 from typing import TYPE_CHECKING, Any
 
-from strands_robots.simulation.mujoco.backend import _NO_WORLD_MSG, _can_render, _ensure_mujoco
+from strands_robots.simulation.mujoco.backend import (
+    _NO_WORLD_MSG,
+    _can_render,
+    _ensure_mujoco,
+    capture_stderr_fd,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -734,22 +739,38 @@ def render_depth(
             # text (the LLM otherwise never hears about it).
             clip_warn = getattr(self, "_depth_warn_text", None)
             if clip_warn is None:
-                import contextlib as _ctx
-                import io as _io
                 import sys as _sys
 
-                buf = _io.StringIO()
-                with _ctx.redirect_stderr(buf):
+                # MuJoCo's ARB_clip_control notice is a C-level write to fd 2,
+                # so Python's contextlib.redirect_stderr (which only swaps the
+                # sys.stderr object) cannot see it. Capture the real fd.
+                with capture_stderr_fd() as _cap:
                     renderer.enable_depth_rendering()
                     depth = renderer.render()
                     renderer.disable_depth_rendering()
-                captured = buf.getvalue()
-                # Also forward to the real stderr so logs don't vanish.
-                if captured and _sys.__stderr__ is not None:
-                    try:
-                        _sys.__stderr__.write(captured)
-                    except Exception:
-                        pass
+                captured = _cap[0]
+                # Forward captured stderr, but drop the ARB_clip_control line
+                # -- it's now surfaced in the response text below, so echoing
+                # it to the console too would be duplicate noise. Anything
+                # *other* than that benign notice is passed through unchanged
+                # so genuine errors never vanish.
+                if captured:
+                    kept_lines = [ln for ln in captured.splitlines(keepends=True) if "ARB_clip_control" not in ln]
+                    leftover = "".join(kept_lines)
+                    if leftover.strip() and _sys.__stderr__ is not None:
+                        try:
+                            _sys.__stderr__.write(leftover)
+                        except (ValueError, OSError):
+                            # Best-effort forward of non-benign stderr; the
+                            # original __stderr__ may be closed or detached
+                            # (pytest capsys, teardown). Nothing to recover.
+                            pass
+                    if "ARB_clip_control" in captured:
+                        logger.debug(
+                            "Suppressed benign MuJoCo depth warning "
+                            "(surfaced in response text): ARB_clip_control "
+                            "unavailable, depth precision degraded."
+                        )
                 if "ARB_clip_control" in captured:
                     # ARB_clip_control missing -> OpenGL depth buffer uses
                     # default [0,1] range with compressed far-plane precision.
@@ -759,7 +780,7 @@ def render_depth(
                     # consumers should treat these values as approximate.
                     self._depth_warn_text = (
                         "Warning: Depth accuracy limited on this GPU (missing ARB_clip_control). "
-                        "Linearized Min/Max are in meters but precision is degraded "
+                        "Metric Min/Max are in meters but precision is degraded "
                         "(especially for far-plane pixels) - treat as approximate."
                     )
                 else:
@@ -770,25 +791,27 @@ def render_depth(
                 depth = renderer.render()
                 renderer.disable_depth_rendering()
 
-            # Linearize OpenGL depth buffer to metric depth (meters).
-            # MuJoCo renderer returns normalized values in [0, 1] where 0 = near,
-            # 1 = far plane. Convert: z = znear*zfar / (zfar - d*(zfar - znear))
+            # MuJoCo >= 3.0's ``Renderer.enable_depth_rendering()`` returns
+            # METRIC depth in meters directly (distance from the camera to the
+            # first surface along each ray), NOT a normalized [0, 1] OpenGL
+            # depth buffer. Re-linearizing it with the znear/zfar formula (as
+            # older OpenGL pipelines required) is wrong and collapses the whole
+            # frame to znear -- so we consume the array as-is.
             #
-            # On MuJoCo >= 3.0, `model.vis.map.{znear,zfar}` are fractions of
-            # `model.stat.extent` (the model's bounding scale), NOT absolute
-            # meters - multiply by extent to get real clip-plane distances.
-            # pyproject.toml pins mujoco>=3.2, so this convention is safe here.
+            # pyproject.toml pins mujoco>=3.2, so the metric-depth convention is
+            # guaranteed. We only sanitize: pixels with no geometry come back as
+            # the far-clip distance (large finite value); NaN/inf can appear on
+            # some GL backends and would poison min/max and the PNG, so replace
+            # them with the far-clip distance before computing bounds.
             import numpy as _np
 
             extent = float(self._world._model.stat.extent)
-            znear = float(self._world._model.vis.map.znear) * extent
             zfar = float(self._world._model.vis.map.zfar) * extent
-            # Avoid division by zero for pixels at exactly the far plane
-            denom = zfar - depth * (zfar - znear)
-            denom = _np.where(denom == 0, 1e-10, denom)
-            depth_m = znear * zfar / denom
-            # Clamp: pixels at far plane (depth==1) -> zfar
-            depth_m = _np.clip(depth_m, znear, zfar)
+            depth_m = _np.asarray(depth, dtype=_np.float32)
+            depth_m = _np.nan_to_num(depth_m, nan=zfar, posinf=zfar, neginf=0.0)
+            # Negative depth is non-physical (a surface behind the camera);
+            # clamp the lower bound at 0 and cap runaway values at the far clip.
+            depth_m = _np.clip(depth_m, 0.0, zfar)
 
             dmin = float(depth_m.min())
             dmax = float(depth_m.max())

tests/simulation/mujoco/test_rendering.py

@@ -881,16 +881,16 @@ def test_render_all_errors_on_unresolved_requested_cameras(monkeypatch) -> None:
     assert "ghost" in r["content"][0]["text"]
 
 
-# render_depth() metric-depth linearization - GL-free coverage.
+# render_depth() metric-depth handling - GL-free coverage.
 #
 # render_depth() resolves a camera, asks _get_renderer() for an offscreen
-# renderer, grabs a normalized [0, 1] OpenGL depth buffer, and linearizes it
-# to metric depth (meters) using the model's znear/zfar clip planes scaled by
-# stat.extent. The linearization math, the one-time ARB_clip_control warning
-# capture, the cached-warning fast path, and the failure handler are all pure
-# Python that only needs a compiled model (no live GL), so we drive them with
-# a real world plus a scripted fake renderer. This pins the metric conversion
-# and the warning contract on every platform, including GL-less CI.
+# renderer, and reads MuJoCo's metric depth buffer (meters) directly, only
+# sanitizing NaN/inf and clamping to [0, zfar]. The passthrough + sanitation,
+# the one-time ARB_clip_control warning capture, the cached-warning fast path,
+# and the failure handler are all pure Python that only needs a compiled model
+# (no live GL), so we drive them with a real world plus a scripted fake
+# renderer. This pins the metric-depth contract and the warning contract on
+# every platform, including GL-less CI.
 
 
 def _depth_world():
@@ -906,10 +906,14 @@ def _depth_world():
 
 
 class _FakeDepthRenderer:
-    """Scripted offscreen renderer: returns a fixed normalized depth buffer.
+    """Scripted offscreen renderer: returns a fixed METRIC depth buffer.
+
+    MuJoCo >= 3.0 returns metric depth (meters) directly from
+    ``enable_depth_rendering()`` + ``render()`` -- not a normalized [0, 1]
+    OpenGL buffer -- so the fake mirrors that contract.
 
     Args:
-        depth: the [0, 1] normalized depth array render() should return.
+        depth: the metric depth array (meters) render() should return.
         stderr_text: text to emit on enable_depth_rendering(), used to drive
             the ARB_clip_control warning-capture branch.
         raise_on_render: when set, render() raises it (failure-path coverage).
@@ -937,25 +941,52 @@ def render(self):
         return self._depth
 
 
-def test_render_depth_linearizes_normalized_buffer_to_meters(monkeypatch) -> None:
-    """A normalized [0,1] depth buffer is converted to metric depth bounded by
-    the model's znear/zfar clip planes; near pixel -> znear, far pixel -> zfar."""
+def test_render_depth_passes_through_metric_depth(monkeypatch) -> None:
+    """MuJoCo >= 3.0 returns METRIC depth (meters) directly, so render_depth
+    must surface those meters as-is -- NOT re-linearize them with a znear/zfar
+    formula (which wrongly collapses the whole frame to znear).
+
+    Regression for the depth bug: the engine treated the renderer output as a
+    normalized [0, 1] OpenGL buffer and re-projected it through
+    ``znear*zfar / (zfar - d*(zfar-znear))``, so a scene with surfaces at
+    1.1 m / 2.5 m reported Min==Max==znear (0.01 m). The renderer already hands
+    back meters; the engine must report them unchanged (modulo NaN/inf
+    sanitation and a [0, zfar] clamp)."""
+    np = pytest.importorskip("numpy")
+    sim = _depth_world()
+    try:
+        # Metric depths in meters: nearest surface 0.75 m, farthest 2.40 m.
+        buf = np.array([[0.75, 2.40], [1.50, 1.50]], dtype=np.float32)
+        monkeypatch.setattr(sim, "_get_renderer", lambda w, h: _FakeDepthRenderer(buf))
+
+        r = sim.render_depth(camera_name="cam_a", width=2, height=2)
+        assert r["status"] == "success", r
+        payload = next(b["json"] for b in r["content"] if isinstance(b, dict) and "json" in b)
+        assert payload["depth_min"] == pytest.approx(0.75, rel=1e-4)
+        assert payload["depth_max"] == pytest.approx(2.40, rel=1e-4)
+        assert "Depth 2x2 from 'cam_a'" in r["content"][0]["text"]
+    finally:
+        sim.destroy()
+
+
+def test_render_depth_sanitizes_nan_and_inf(monkeypatch) -> None:
+    """NaN/inf pixels (some GL backends emit them for empty rays) must not
+    poison the metric min/max or the PNG -- they're replaced with the far-clip
+    distance, and negative depth is clamped to 0."""
     np = pytest.importorskip("numpy")
     sim = _depth_world()
     try:
         extent = float(sim._world._model.stat.extent)
-        znear = float(sim._world._model.vis.map.znear) * extent
         zfar = float(sim._world._model.vis.map.zfar) * extent
-        # 0.0 -> near plane, 1.0 -> far plane, plus a mid value.
-        buf = np.array([[0.0, 1.0], [0.5, 0.5]], dtype=np.float32)
+        buf = np.array([[0.5, np.nan], [np.inf, -1.0]], dtype=np.float32)
         monkeypatch.setattr(sim, "_get_renderer", lambda w, h: _FakeDepthRenderer(buf))
 
         r = sim.render_depth(camera_name="cam_a", width=2, height=2)
         assert r["status"] == "success", r
         payload = next(b["json"] for b in r["content"] if isinstance(b, dict) and "json" in b)
-        assert payload["depth_min"] == pytest.approx(znear, rel=1e-4)
+        # min comes from the clamped -1.0 -> 0.0; max from nan/inf -> zfar.
+        assert payload["depth_min"] == pytest.approx(0.0, abs=1e-6)
         assert payload["depth_max"] == pytest.approx(zfar, rel=1e-4)
-        assert "Depth 2x2 from 'cam_a'" in r["content"][0]["text"]
     finally:
         sim.destroy()