Read snapshots from where the gateway puts them

run_tests.py

@@ -8,8 +8,9 @@
 import sys
 
 # Remove ROS 2 paths before importing pytest
-sys.path = [p for p in sys.path if '/opt/ros' not in p]
+sys.path = [p for p in sys.path if "/opt/ros" not in p]
 
 if __name__ == "__main__":
     import pytest
+
     sys.exit(pytest.main(sys.argv[1:] or ["-v", "tests/"]))

src/ros2_medkit_mcp/mcp_app.py

@@ -57,6 +57,7 @@
     FaultSnapshotsArgs,
     FreezeFrameSnapshot,
     FunctionIdArgs,
+    GatewaySnapshot,
     GetConfigurationArgs,
     GetCyclicSubArgs,
     GetLockArgs,
@@ -250,6 +251,35 @@ def format_snapshot(snapshot: FreezeFrameSnapshot | RosbagSnapshot) -> str:
     return "\n".join(lines)
 
 
+def format_gateway_snapshot(snapshot: GatewaySnapshot) -> str:
+    """Format one environment_data.snapshots entry for display.
+
+    Args:
+        snapshot: A freeze frame or rosbag entry as the gateway sends it.
+
+    Returns:
+        Formatted string describing the snapshot.
+    """
+    lines = [f"  Snapshot: {snapshot.name or snapshot.type}"]
+    if snapshot.captured_at:
+        lines.append(f"    Captured At: {snapshot.captured_at}")
+
+    if snapshot.type == "rosbag":
+        # The URI carries the recording id, which is what distinguishes one
+        # occurrence's black box from another's.
+        lines.append(f"    Download URI: {snapshot.bulk_data_uri}")
+        if snapshot.size_bytes:
+            lines.append(f"    File Size: {snapshot.size_bytes / (1024 * 1024):.2f} MB")
+        if snapshot.duration_sec is not None:
+            lines.append(f"    Duration: {snapshot.duration_sec:.2f}s")
+        if snapshot.format:
+            lines.append(f"    Format: {snapshot.format}")
+    elif snapshot.data is not None:
+        lines.append(f"    Data: {snapshot.data}")
+
+    return "\n".join(lines)
+
+
 def format_environment_data(env_data: EnvironmentData) -> str:
     """Format environment data for LLM readability.
 
@@ -261,18 +291,24 @@ def format_environment_data(env_data: EnvironmentData) -> str:
     """
     lines = ["\nEnvironment Data:"]
 
-    if env_data.extended_data_records:
-        records = env_data.extended_data_records
-
-        if records.freeze_frame_snapshots:
-            lines.append(f"  Freeze Frame Snapshots ({len(records.freeze_frame_snapshots)}):")
-            for snap in records.freeze_frame_snapshots:
-                lines.append(format_snapshot(snap))
-
-        if records.rosbag_snapshots:
-            lines.append(f"  Rosbag Snapshots ({len(records.rosbag_snapshots)}):")
-            for snap in records.rosbag_snapshots:
-                lines.append(format_snapshot(snap))
+    # Read from environment_data.snapshots, which is where the gateway actually
+    # puts them, discriminated by `type`. The freezeFrameSnapshots /
+    # rosbagSnapshots containers under extended_data_records are never populated,
+    # so everything below used to render nothing at all.
+    freeze_frames = [s for s in env_data.snapshots if s.type == "freeze_frame"]
+    recordings = [s for s in env_data.snapshots if s.type == "rosbag"]
+
+    if freeze_frames:
+        lines.append(f"  Freeze Frame Snapshots ({len(freeze_frames)}):")
+        for snap in freeze_frames:
+            lines.append(format_gateway_snapshot(snap))
+
+    if recordings:
+        # Plural on purpose: since ros2_medkit#620 a fault keeps one black box
+        # per occurrence, and each is downloaded by its own URI.
+        lines.append(f"  Rosbag Recordings ({len(recordings)}):")
+        for snap in recordings:
+            lines.append(format_gateway_snapshot(snap))
 
     return "\n".join(lines)
 
@@ -521,22 +557,14 @@ async def download_rosbags_for_fault(
             )
         ]
 
-    # Get extended data records
-    records = env_data.get("extendedDataRecords") or env_data.get("extended_data_records")
-    if not records:
-        return [
-            TextContent(
-                type="text",
-                text=f"No snapshot data found for fault {fault_code}",
-            )
-        ]
-
-    # Get rosbag snapshots
-    rosbag_snapshots = records.get("rosbagSnapshots") or records.get("rosbag_snapshots", [])
+    # Snapshots live in environment_data.snapshots, discriminated by `type`.
+    # The rosbagSnapshots / freezeFrameSnapshots containers under
+    # extended_data_records are never populated by the gateway, so reading them
+    # made this tool answer "no rosbag snapshots" for every fault that had them.
+    snapshots = env_data.get("snapshots") or []
+    rosbag_snapshots = [s for s in snapshots if s.get("type") == "rosbag"]
     if not rosbag_snapshots:
-        freeze_frames = records.get("freezeFrameSnapshots") or records.get(
-            "freeze_frame_snapshots", []
-        )
+        freeze_frames = [s for s in snapshots if s.get("type") == "freeze_frame"]
         if freeze_frames:
             return [
                 TextContent(
@@ -559,7 +587,10 @@ async def download_rosbags_for_fault(
     errors: list[str] = []
 
     for snap in rosbag_snapshots:
-        snap_id = snap.get("snapshotId") or snap.get("snapshot_id", "unknown")
+        # The recording's own name. There is no snapshotId on the wire, and a
+        # fault can hold several recordings, so falling back to the fault code
+        # would give every one of them the same identity in the report below.
+        snap_id = snap.get("name") or "unknown"
         bulk_uri = snap.get("bulkDataUri") or snap.get("bulk_data_uri")
 
         if not bulk_uri:

src/ros2_medkit_mcp/models.py

@@ -582,6 +582,46 @@ class RosbagSnapshot(BaseModel):
     model_config = {"populate_by_name": True, "extra": "allow"}
 
 
+class GatewaySnapshot(BaseModel):
+    """One entry of ``environment_data.snapshots``, as the gateway sends it.
+
+    The gateway puts freeze frames and rosbags in a single list discriminated by
+    ``type``; it has never sent the ``freezeFrameSnapshots`` / ``rosbagSnapshots``
+    containers ExtendedDataRecords declares, so nothing read through those was
+    ever populated. A fault can carry several rosbag entries since
+    ros2_medkit#620 - one per occurrence it kept a black box for - each addressed
+    by its own ``bulk_data_uri``.
+    """
+
+    type: str = Field(..., description='"freeze_frame" or "rosbag"')
+    name: str = Field(default="", description="Snapshot name; for a rosbag, its recording id")
+    bulk_data_uri: str | None = Field(
+        default=None,
+        alias="bulkDataUri",
+        description="Path to download this recording; rosbag entries only",
+    )
+    size_bytes: int | None = Field(
+        default=None, alias="sizeBytes", description="Recording size in bytes"
+    )
+    duration_sec: float | None = Field(
+        default=None, alias="durationSec", description="Recorded span in seconds"
+    )
+    format: str | None = Field(default=None, description='Storage format: "mcap" or "sqlite3"')
+    data: Any | None = Field(default=None, description="Captured value; freeze-frame entries only")
+    x_medkit: dict[str, Any] | None = Field(
+        default=None,
+        alias="x-medkit",
+        description="Vendor extension; carries captured_at, topic and message_type",
+    )
+
+    @property
+    def captured_at(self) -> str | None:
+        """ISO 8601 capture time, which the gateway nests under ``x-medkit``."""
+        return (self.x_medkit or {}).get("captured_at")
+
+    model_config = {"populate_by_name": True, "extra": "allow"}
+
+
 class ExtendedDataRecords(BaseModel):
     """Extended data records containing diagnostic snapshots."""
 
@@ -605,7 +645,11 @@ class EnvironmentData(BaseModel):
     extended_data_records: ExtendedDataRecords | None = Field(
         default=None,
         alias="extendedDataRecords",
-        description="Snapshot data including freeze frames and rosbags",
+        description="First and last occurrence timestamps",
+    )
+    snapshots: list[GatewaySnapshot] = Field(
+        default_factory=list,
+        description="Freeze frames and rosbag recordings, discriminated by type",
     )
 
     model_config = {"populate_by_name": True, "extra": "allow"}

tests/fixtures/fault_detail_two_recordings.json

@@ -0,0 +1,58 @@
+{
+    "environment_data": {
+        "extended_data_records": {
+            "first_occurrence": "2026-08-15T16:05:49.981Z",
+            "last_occurrence": "2026-08-15T16:05:49.981Z"
+        },
+        "snapshots": [
+            {
+                "data": 1,
+                "name": "probe",
+                "type": "freeze_frame",
+                "x-medkit": {
+                    "captured_at": "2026-08-15T16:05:50.034Z",
+                    "full_data": {
+                        "data": 1
+                    },
+                    "message_type": "std_msgs/msg/Float32",
+                    "topic": "/e2e/probe"
+                }
+            },
+            {
+                "bulk_data_uri": "/apps/e2e_rosbag_seeder/bulk-data/rosbags/fault_E2E_FLAPPING_SENSOR_1786809950036",
+                "duration_sec": 2.481570097,
+                "format": "mcap",
+                "name": "rosbag_fault_E2E_FLAPPING_SENSOR_1786809950036",
+                "size_bytes": 6225,
+                "type": "rosbag"
+            },
+            {
+                "bulk_data_uri": "/apps/e2e_rosbag_seeder/bulk-data/rosbags/fault_E2E_FLAPPING_SENSOR_1786809943972",
+                "duration_sec": 2.4892582,
+                "format": "mcap",
+                "name": "rosbag_fault_E2E_FLAPPING_SENSOR_1786809943972",
+                "size_bytes": 6225,
+                "type": "rosbag"
+            }
+        ]
+    },
+    "item": {
+        "code": "E2E_FLAPPING_SENSOR",
+        "fault_name": "Intermittent sensor dropout seen twice",
+        "severity": 2,
+        "status": {
+            "aggregatedStatus": "active",
+            "confirmedDTC": "1",
+            "pendingDTC": "0",
+            "testFailed": "1"
+        }
+    },
+    "x-medkit": {
+        "occurrence_count": 2,
+        "reporting_sources": [
+            "/e2e_rosbag_seeder"
+        ],
+        "severity_label": "ERROR",
+        "status_raw": "CONFIRMED"
+    }
+}

tests/test_bulkdata_tools.py

@@ -420,22 +420,24 @@ async def test_download_rosbags_success(self, client: SovdClient) -> None:
                 "status": {"aggregatedStatus": "active"},
                 "fault_name": "Motor Overheating",
             },
+            # Snapshots come in environment_data.snapshots, discriminated by
+            # type - the shape the gateway actually sends. Two rosbag entries:
+            # one fault that came back, keeping a black box for each occurrence.
             "environment_data": {
-                "extended_data_records": {
-                    "freeze_frame_snapshots": [],
-                    "rosbag_snapshots": [
-                        {
-                            "snapshot_id": "rb-1",
-                            "timestamp": "2026-02-04T10:00:00Z",
-                            "bulk_data_uri": "/apps/motor/bulk-data/rosbags/rb-1",
-                        },
-                        {
-                            "snapshot_id": "rb-2",
-                            "timestamp": "2026-02-04T10:01:00Z",
-                            "bulk_data_uri": "/apps/motor/bulk-data/rosbags/rb-2",
-                        },
-                    ],
-                }
+                "snapshots": [
+                    {
+                        "type": "rosbag",
+                        "name": "rb-1",
+                        "bulk_data_uri": "/apps/motor/bulk-data/rosbags/rb-1",
+                        "format": "mcap",
+                    },
+                    {
+                        "type": "rosbag",
+                        "name": "rb-2",
+                        "bulk_data_uri": "/apps/motor/bulk-data/rosbags/rb-2",
+                        "format": "mcap",
+                    },
+                ]
             },
         }
 
@@ -483,12 +485,7 @@ async def test_download_only_freeze_frames(self, client: SovdClient) -> None:
                 "status": {"aggregatedStatus": "active"},
             },
             "environment_data": {
-                "extended_data_records": {
-                    "freeze_frame_snapshots": [
-                        {"snapshot_id": "ff-1", "timestamp": "2026-02-04T10:00:00Z", "data": {}}
-                    ],
-                    "rosbag_snapshots": [],
-                }
+                "snapshots": [{"type": "freeze_frame", "name": "ff-1", "data": 1}]
             },
         }
 
@@ -536,21 +533,18 @@ async def test_download_with_errors(self, client: SovdClient) -> None:
                 "status": {"aggregatedStatus": "active"},
             },
             "environment_data": {
-                "extended_data_records": {
-                    "freeze_frame_snapshots": [],
-                    "rosbag_snapshots": [
-                        {
-                            "snapshot_id": "rb-ok",
-                            "timestamp": "2026-02-04T10:00:00Z",
-                            "bulk_data_uri": "/apps/motor/bulk-data/rosbags/rb-ok",
-                        },
-                        {
-                            "snapshot_id": "rb-fail",
-                            "timestamp": "2026-02-04T10:01:00Z",
-                            "bulk_data_uri": "/apps/motor/bulk-data/rosbags/rb-fail",
-                        },
-                    ],
-                }
+                "snapshots": [
+                    {
+                        "type": "rosbag",
+                        "name": "rb-ok",
+                        "bulk_data_uri": "/apps/motor/bulk-data/rosbags/rb-ok",
+                    },
+                    {
+                        "type": "rosbag",
+                        "name": "rb-fail",
+                        "bulk_data_uri": "/apps/motor/bulk-data/rosbags/rb-fail",
+                    },
+                ]
             },
         }