fix: report a rosbag's served size, retire the removed snapshot routes

docs/api/rest.rst

@@ -1772,6 +1772,45 @@ recording therefore reports its size once. One fault code can appear on several
 descriptors, one per occurrence it kept, told apart by ``creation_date``, which
 is the time that recording was made.
 
+``size`` is the number of bytes the download route below puts on the wire for
+that descriptor, so a client can size a buffer or a progress bar from the
+listing. For a rosbag held in a single storage file, which is the normal case,
+that is the file (``.mcap`` or ``.db3``) and it is the only file the
+download serves. The bag directory also holds ``metadata.yaml``, and those bytes
+are not part of the transfer.
+
+.. _rest-recording-size-rule:
+
+**One recording, one size.** The descriptor ``size`` here, the
+``environment_data.snapshots[].size_bytes`` a fault reports for the same
+recording, and the ``Content-Length`` of its download are the same number when
+the bag's ``metadata.yaml`` names exactly one storage file, that name is a direct
+child of the bag directory ending in ``.db3`` or ``.mcap``, and the file is
+present - the normal case - and that number is the storage file. A recording also
+has a footprint on the gateway host, which is larger because the directory holds
+``metadata.yaml`` as well. That figure is what the recording spends against its
+storage quota and is not reported by the API.
+
+Four shapes fall outside that condition, and there the three numbers part
+deliberately:
+
+- a recording split across several storage files, past the configured maximum
+  bag size;
+- a bag with no ``metadata.yaml``, or one that cannot be parsed;
+- a bag whose named storage file is absent from the directory;
+- a name that points outside the bag directory, or at a file that is neither
+  ``.db3`` nor ``.mcap``. The gateway and the fault manager both decline such a
+  name, because a name decides which file on the host is served and measured.
+
+In each of them the descriptor ``size`` and the nested ``size_bytes`` carry the
+recording's stored total, while the download hands over whichever single storage
+file the bag directory holds. ``size`` can therefore exceed ``Content-Length``,
+and that gap is the signal in each of these cases: a client comparing the two can
+tell the transfer it just made is a part of the recording and not the whole of
+it, which no single reported number could express. When the directory holds no
+storage file at all, the download has nothing to send and answers ``500``, so
+there is no ``Content-Length`` to compare.
+
 Download Bulk Data
 ~~~~~~~~~~~~~~~~~~
 
@@ -1787,6 +1826,9 @@ Download a specific bulk-data file.
   a pre-#620 fault-code URL is not the segment the client sent, and the format
   is the one persisted at capture time (``mcap`` or ``sqlite3``). For every
   other category it is the stored item's own name, e.g. ``report.zip``.
+- ``Content-Length``: the served file's length. For how it relates to the
+  descriptor ``size`` of the same recording, see
+  :ref:`One recording, one size <rest-recording-size-rule>`
 - ``Accept-Ranges``: ``bytes`` - the download is served by a range-aware
   provider, so a client may fetch part of the file
 - ``Access-Control-Expose-Headers``: ``Content-Disposition``

docs/tutorials/snapshots.rst

@@ -52,11 +52,13 @@ Quick Start
 
       ros2 launch ros2_medkit_gateway gateway.launch.py
 
-3. **When a fault is confirmed, query its snapshots:**
+3. **When a fault is confirmed, read its snapshots from the fault itself:**
+
+   They are returned inline, under ``environment_data.snapshots``.
 
    .. code-block:: bash
 
-      curl http://localhost:8080/api/v1/faults/MOTOR_OVERHEAT/snapshots
+      curl http://localhost:8080/api/v1/apps/motor_controller/faults/MOTOR_OVERHEAT
 
 Configuration Options
 ---------------------

postman/collections/ros2-medkit-gateway.postman_collection.json

@@ -1355,74 +1355,6 @@
             "description": "List faults with cluster details. Clusters are groups of similar faults that occurred within a time window.\n\nEach cluster includes:\n- `cluster_id`: Unique cluster identifier\n- `rule_id`, `rule_name`: The auto-cluster rule that created this cluster\n- `representative_code`: The fault shown as the cluster representative (based on rule config: first, most_recent, or highest_severity)\n- `representative_severity`: Severity of the representative fault\n- `fault_codes[]`: All fault codes in the cluster\n- `first_at`, `last_at`: Timestamps of first and last faults in cluster"
           },
           "response": []
-        },
-        {
-          "name": "GET Fault Snapshots (System-wide)",
-          "request": {
-            "method": "GET",
-            "header": [],
-            "url": {
-              "raw": "{{base_url}}/faults/SENSOR_OVERTEMP/snapshots",
-              "host": [
-                "{{base_url}}"
-              ],
-              "path": [
-                "faults",
-                "SENSOR_OVERTEMP",
-                "snapshots"
-              ]
-            },
-            "description": "Get topic snapshots captured when a fault transitioned to CONFIRMED status. Snapshots provide system state at the moment of fault confirmation for post-mortem debugging. Returns object with fault_code, captured_at timestamp, and topics object keyed by topic name containing message_type and parsed data."
-          },
-          "response": []
-        },
-        {
-          "name": "GET Fault Snapshots (Filtered by Topic)",
-          "request": {
-            "method": "GET",
-            "header": [],
-            "url": {
-              "raw": "{{base_url}}/faults/SENSOR_OVERTEMP/snapshots?topic=/joint_states",
-              "host": [
-                "{{base_url}}"
-              ],
-              "path": [
-                "faults",
-                "SENSOR_OVERTEMP",
-                "snapshots"
-              ],
-              "query": [
-                {
-                  "key": "topic",
-                  "value": "/joint_states"
-                }
-              ]
-            },
-            "description": "Get snapshots filtered by specific topic name. Use this when you only need data from a particular topic."
-          },
-          "response": []
-        },
-        {
-          "name": "GET Component Fault Snapshots",
-          "request": {
-            "method": "GET",
-            "header": [],
-            "url": {
-              "raw": "{{base_url}}/components/temp_sensor/faults/SENSOR_OVERTEMP/snapshots",
-              "host": [
-                "{{base_url}}"
-              ],
-              "path": [
-                "components",
-                "temp_sensor",
-                "faults",
-                "SENSOR_OVERTEMP",
-                "snapshots"
-              ]
-            },
-            "description": "Get topic snapshots for a specific component's fault. Same as the system-wide endpoint but scoped to a component context. Useful when working within a component-centric workflow."
-          },
-          "response": []
         }
       ]
     },

src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/rosbag_capture.hpp

@@ -487,4 +487,40 @@ class RosbagCapture {
   bool dynamic_discovery_{false};
 };
 
+/// Bytes a client receives when it downloads the recording at @p bag_path.
+///
+/// A recording occupies a directory and is served as a single file. Those are two
+/// different quantities and the fault manager needs both. ``RosbagFileInfo::size_bytes``
+/// is the directory total, because that is what the recording costs against
+/// ``max_total_storage_mb`` and what eviction frees. This is the other one: the storage
+/// file the download hands over, which is what a caller sizing a buffer or a progress
+/// bar needs. Reporting the total in its place overstated every download by
+/// ``metadata.yaml``, and on a short recording that is around a tenth of the transfer.
+///
+/// The file is the one ``metadata.yaml`` names in ``relative_file_paths``, read through
+/// the same library that wrote it, so this answers with the bag's own record of its
+/// contents.
+///
+/// Falls back to @p stored_total_bytes, never to zero, when no single served file can
+/// be named:
+/// - no ``metadata.yaml``, or one that cannot be read or parsed.
+/// - ``relative_file_paths`` naming other than exactly one file. Past
+///   ``max_bag_size_mb`` rosbag2 splits a recording across several storage files, and
+///   then no single number describes the download at all.
+/// - a name that points outside the bag directory, or at something other than a
+///   ``.db3`` or ``.mcap`` file. A name decides which file on the host is measured,
+///   so only a storage file that is a direct child of the bag is followed. The
+///   gateway applies the same rule to the same field.
+/// - a named file that cannot be stat'd.
+///
+/// None of those is an error worth logging. A pre-metadata bag and a split bag are
+/// both normal, this runs once per reported row on every request, and the fallback is
+/// a real measurement of the recording. A zero would describe the recording as
+/// empty.
+///
+/// @param bag_path Bag directory as stored in ``RosbagFileInfo::file_path``
+/// @param stored_total_bytes The stored directory total, used as the fallback
+/// @return Size of the served storage file, or @p stored_total_bytes
+size_t rosbag_served_bytes(const std::string & bag_path, size_t stored_total_bytes);
+
 }  // namespace ros2_medkit_fault_manager

src/ros2_medkit_fault_manager/src/fault_manager_node.cpp

@@ -1158,7 +1158,9 @@ void FaultManagerNode::handle_get_fault(const std::shared_ptr<ros2_medkit_msgs::
     // The RECORDING, not the fault code: several recordings of one fault would
     // otherwise all carry the same id and collapse into one download.
     rosbag_snapshot.bulk_data_id = rosbag_info.recording_id;
-    rosbag_snapshot.size_bytes = rosbag_info.size_bytes;
+    // What a download of this recording transfers, not what it occupies on disk.
+    // The stored figure is the directory total and stays the quota's business.
+    rosbag_snapshot.size_bytes = rosbag_served_bytes(rosbag_info.file_path, rosbag_info.size_bytes);
     rosbag_snapshot.duration_sec = rosbag_info.duration_sec;
     rosbag_snapshot.format = rosbag_info.format;
     rosbag_snapshot.captured_at_ns = rosbag_info.created_at_ns;
@@ -1491,15 +1493,24 @@ void FaultManagerNode::handle_get_snapshots(
   }
   result["topics"] = topics_json;
 
-  // Include rosbag info if available
+  // Include rosbag info if available.
+  //
+  // No download URL. This payload used to carry one built as
+  // /api/v1/faults/{code}/snapshots/bag, a route that no longer exists and answers
+  // 404, so the field described a download nobody could perform. It is not replaced
+  // here either: a recording is addressed under its entity
+  // (/api/v1/{entity-type}/{id}/bulk-data/rosbags/{recording_id}), and the entity
+  // type is a fact of the gateway's discovery model that the fault manager does not
+  // hold. Any URL built here would be a guess at one of four prefixes. The gateway
+  // resolves the entity itself and builds that URI from the recording id it gets on
+  // the GetFault snapshot entries, which is the one place the mapping is known.
   auto rosbag_info = storage_->get_rosbag_file(request->fault_code);
   if (rosbag_info) {
     nlohmann::json rosbag_json;
     rosbag_json["available"] = true;
     rosbag_json["duration_sec"] = rosbag_info->duration_sec;
-    rosbag_json["size_bytes"] = rosbag_info->size_bytes;
+    rosbag_json["size_bytes"] = rosbag_served_bytes(rosbag_info->file_path, rosbag_info->size_bytes);
     rosbag_json["format"] = rosbag_info->format;
-    rosbag_json["download_url"] = "/api/v1/faults/" + request->fault_code + "/snapshots/bag";
     result["rosbag"] = rosbag_json;
   } else {
     result["rosbag"] = {{"available", false}};
@@ -1598,7 +1609,7 @@ void FaultManagerNode::handle_get_rosbag(const std::shared_ptr<ros2_medkit_msgs:
   response->fault_codes = attached_codes;
   response->format = rosbag_info->format;
   response->duration_sec = rosbag_info->duration_sec;
-  response->size_bytes = rosbag_info->size_bytes;
+  response->size_bytes = rosbag_served_bytes(rosbag_info->file_path, rosbag_info->size_bytes);
 
   RCLCPP_DEBUG(get_logger(), "GetRosbag returned file '%s' for %s", rosbag_info->file_path.c_str(), subject.c_str());
 }
@@ -1635,7 +1646,7 @@ void FaultManagerNode::handle_list_rosbags(
     response->file_paths.push_back(info.file_path);
     response->formats.push_back(info.format);
     response->durations_sec.push_back(info.duration_sec);
-    response->sizes_bytes.push_back(info.size_bytes);
+    response->sizes_bytes.push_back(rosbag_served_bytes(info.file_path, info.size_bytes));
     response->created_at_ns.push_back(info.created_at_ns);
   }
 

src/ros2_medkit_fault_manager/src/rosbag_capture.cpp

@@ -30,6 +30,8 @@
 #include <mutex>
 #include <optional>
 #include <rosbag2_cpp/writer.hpp>
+#include <rosbag2_storage/bag_metadata.hpp>
+#include <rosbag2_storage/metadata_io.hpp>
 #include <rosbag2_storage/storage_options.hpp>
 #include <set>
 #include <sstream>
@@ -1263,6 +1265,37 @@ std::string fnv1a_hex(const std::string & text) {
   return oss.str();
 }
 
+/// The file @p relative_name names inside the bag at @p bag_path, when that is a
+/// name this side follows: a direct child of the bag directory carrying a storage
+/// extension, which is what rosbag2 writes.
+///
+/// Path concatenation carries no such guarantee. An absolute name replaces the bag
+/// directory outright and a `..` name climbs out of it, so without this test a
+/// metadata.yaml chooses which file on the host gets measured and reported as the
+/// recording's size. The gateway applies the same rule in its own
+/// rosbag_named_storage_file (bulkdata_handlers.cpp); the two packages share no
+/// code, so the rule is written out on both sides and a change to it belongs on
+/// both.
+///
+/// nullopt when the name is not one of those. The caller treats that as it treats
+/// a named file that is absent: the stored total.
+std::optional<std::filesystem::path> named_storage_file(const std::string & bag_path,
+                                                        const std::string & relative_name) {
+  std::filesystem::path bag = std::filesystem::path(bag_path).lexically_normal();
+  if (!bag.has_filename()) {
+    bag = bag.parent_path();  // tolerate a trailing slash
+  }
+  const std::filesystem::path named = (bag / relative_name).lexically_normal();
+  if (named.parent_path() != bag) {
+    return std::nullopt;
+  }
+  const std::string extension = named.extension().string();
+  if (extension != ".db3" && extension != ".mcap") {
+    return std::nullopt;
+  }
+  return named;
+}
+
 }  // namespace
 
 std::string RosbagCapture::bag_directory_name(const std::string & fault_code, int64_t timestamp_ms) {
@@ -1325,6 +1358,12 @@ std::string RosbagCapture::generate_bag_path(const std::string & fault_code) con
   return base_path + "/" + bag_directory_name(fault_code, timestamp);
 }
 
+// The recording's footprint, and the figure stored on its rows. It is what the
+// recording costs against max_total_storage_mb and what evicting it frees, so it
+// counts everything in the directory including metadata.yaml and every file of a
+// split. rosbag_served_bytes() below is the other measurement of the same
+// recording, the one the API reports, and the two are deliberately different
+// numbers - see its comment for which question each answers.
 size_t RosbagCapture::calculate_bag_size(const std::string & bag_path) const {
   size_t total_size = 0;
 
@@ -1345,6 +1384,48 @@ size_t RosbagCapture::calculate_bag_size(const std::string & bag_path) const {
   return total_size;
 }
 
+// The bytes a download of this recording actually transfers, which is the one
+// storage file the bulk-data route hands over. calculate_bag_size() above answers
+// the storage question (what the recording costs on disk) and this one answers the
+// client's question (what is about to arrive). The two are separate figures, which
+// is what lets the quota and the API each stay honest: a footprint reported here
+// would overstate every download by metadata.yaml.
+//
+// The served file is the one the bag's own metadata.yaml names, and only when that
+// name points at a storage file inside the bag directory. See the header for every
+// fallback and why none of them logs.
+size_t rosbag_served_bytes(const std::string & bag_path, size_t stored_total_bytes) {
+  try {
+    rosbag2_storage::MetadataIo metadata_io;
+    if (!metadata_io.metadata_file_exists(bag_path)) {
+      return stored_total_bytes;
+    }
+
+    const rosbag2_storage::BagMetadata metadata = metadata_io.read_metadata(bag_path);
+    // Exactly one, or there is no single served file to measure. Zero means a bag
+    // that recorded nothing addressable. More than one means a split, where the
+    // download hands over one segment and the rest are unreachable through it - a
+    // defect of the download route, not something a size can paper over.
+    if (metadata.relative_file_paths.size() != 1) {
+      return stored_total_bytes;
+    }
+
+    const auto storage_file = named_storage_file(bag_path, metadata.relative_file_paths.front());
+    if (!storage_file) {
+      return stored_total_bytes;
+    }
+    std::error_code ec;
+    const auto served = std::filesystem::file_size(*storage_file, ec);
+    if (ec) {
+      return stored_total_bytes;
+    }
+    return static_cast<size_t>(served);
+  } catch (const std::exception &) {
+    // read_metadata throws on a metadata.yaml that cannot be read or parsed.
+    return stored_total_bytes;
+  }
+}
+
 std::vector<std::string> RosbagCapture::evict_bags_over_quota(FaultStorage * storage, size_t max_bytes) {
   size_t current_bytes = storage->get_total_rosbag_storage_bytes();
   if (current_bytes <= max_bytes) {