fix(xodr): preserve the five warned-and-dropped spec areas (#539)

CHANGELOG.md

@@ -1345,6 +1345,27 @@ Current version on `main`: **0.0.1**.
   about silently never matched.
 
 ### Fixed
+- **Five spec areas that were warned about and then thrown away now round-trip**
+  ([#539](https://github.com/Robomous/RoadMaker/issues/539), fmt-f2):
+  `<lateralProfile><shape>` and the legacy `<crossfall>` (§10.5.1), road
+  `<surface>`/`<CRG>` (§10.6), `<junctionGroup>` (§12.16), `<railroad>` and root
+  `<station>` (chapter 15), and an `<include>` outside `<header>` (§7.1).
+
+  These were **loud and lossy** — one diagnostic, then permanent data loss,
+  which is worse than a silent drop in one respect: the file looked like it had
+  been understood. A round trip flattened a non-planar carriageway, dropped a
+  CRG-referenced surface, made the standard's own roundabout grouping
+  unrepresentable, and lost a tram file's rail layer.
+
+  All five now ride the preserved tier built for
+  [#453](https://github.com/Robomous/RoadMaker/issues/453) — reusing
+  `capture_unmodeled()` rather than adding five more walks — and the diagnostic
+  is downgraded from *"is not supported yet and was ignored"* (true before, a
+  lie now) to *"is preserved verbatim but not modeled; it round-trips unchanged
+  and has no effect in this build"*. Nothing here is modeled, and none of it
+  needs to be for v0.1.0; modeling stays with whoever owns the feature later
+  (`<junctionGroup>` with [#495](https://github.com/Robomous/RoadMaker/issues/495),
+  for instance).
 - **The last parse scopes that dropped unmodeled input in silence now preserve
   it** ([#453](https://github.com/Robomous/RoadMaker/issues/453), fmt-f1). The
   Preserved tier was established for signals, objects and controllers, but six

core/include/roadmaker/road/network.hpp

@@ -373,6 +373,22 @@ class RoadNetwork {
     preserved_user_data_ = std::move(fragments);
   }
 
+  /// Root-level `<OpenDRIVE>` children RoadMaker does not model —
+  /// `<junctionGroup>` (§12.16), `<station>` (chapter 15), and an `<include>`
+  /// outside `<header>` (§7.1) — preserved as verbatim XML fragments in
+  /// document order (fmt-f2, #539). All three were warned about and then
+  /// DROPPED, so a legal file lost a roundabout grouping or a rail layer on its
+  /// first save. Sibling of `preserved_user_data_`, separate because the writer
+  /// emits the two in different places.
+  [[nodiscard]] const std::vector<std::string>& preserved_root_children() const {
+    return preserved_root_children_;
+  }
+
+  /// Replaces the preserved root children wholesale. Parser-only, as above.
+  void set_preserved_root_children(std::vector<std::string> fragments) {
+    preserved_root_children_ = std::move(fragments);
+  }
+
 private:
   Arena<Road, RoadId> roads_;
   Arena<LaneSection, LaneSectionId> sections_;
@@ -386,6 +402,7 @@ class RoadNetwork {
   GeoReference georeference_;
   RawXml preserved_header_;
   std::vector<std::string> preserved_user_data_;
+  std::vector<std::string> preserved_root_children_;
 };
 
 /// Plan-view bounding box of the whole network as {lo_x, lo_y, hi_x, hi_y},

core/include/roadmaker/road/road.hpp

@@ -104,6 +104,17 @@ struct Road {
   /// #453). Held on the Road because `<lanes>` has no struct of its own.
   RawXml lanes_extras;
 
+  /// Unmodeled children of `<lateralProfile>` — `<shape>` (§10.5.1) and the
+  /// legacy `<crossfall>` — preserved verbatim (fmt-f2, #539). Both were warned
+  /// about and dropped, so a round trip FLATTENED a non-planar carriageway.
+  RawXml lateral_profile_extras;
+
+  /// Unmodeled children of `<road>` itself — `<surface>`/`<CRG>` (§10.6) and
+  /// `<railroad>` (chapter 15) — preserved verbatim (fmt-f2, #539). A
+  /// CRG-referencing file used to lose its surface detail, and a tram file its
+  /// rail layer, on the first save.
+  RawXml road_extras;
+
   /// Unmodeled children of `<elevationProfile>` — anything besides
   /// `<elevation>` — preserved verbatim (fmt-f1, #453). Same reason: the
   /// profile container has no struct of its own.

core/src/xodr/reader.cpp

@@ -206,7 +206,7 @@ class Parser {
     parse_surfaces(root);
     parse_terrain_reference(root);
     parse_root_user_data(root);
-    warn_unsupported_root_children(root);
+    preserve_unmodeled_root_children(root);
 
     return std::move(result_);
   }
@@ -251,6 +251,22 @@ class Parser {
   }
 
   /// Unsupported-element warnings are emitted once per element name.
+  /// The "preserved, not modeled" note (fmt-f2, #539) — the downgrade from
+  /// `warn_unsupported`, which said "ignored" because the element really was
+  /// dropped. These elements now survive a round trip byte-for-byte; what the
+  /// user still needs to know is that this build assigns them no meaning, so a
+  /// `<shape>` will not bend the carriageway and a `<junctionGroup>` will not
+  /// group anything. Once per element name, like warn_unsupported.
+  void note_preserved(const std::string& element, const std::string& location) {
+    if (warned_elements_.insert("preserved:" + element).second) {
+      diag(Severity::Warning,
+           location,
+           fmt::format("element <{}> is preserved verbatim but not modeled; it round-trips "
+                       "unchanged and has no effect in this build",
+                       element));
+    }
+  }
+
   void warn_unsupported(const std::string& element, const std::string& location) {
     if (warned_elements_.insert(element).second) {
       diag(Severity::Warning,
@@ -542,12 +558,23 @@ class Parser {
     }
     pending_refs_.push_back(std::move(pending));
 
+    // <surface>/<CRG> (§10.6) and <railroad> (chapter 15) used to be warned
+    // about and DROPPED (fmt-f2, #539). Preserved verbatim now — a
+    // CRG-referencing file kept its surface detail, a tram file its rail layer.
+    static constexpr std::string_view kRoadChildren[] = {"planView",
+                                                         "elevationProfile",
+                                                         "lateralProfile",
+                                                         "lanes",
+                                                         "link",
+                                                         "type",
+                                                         "userData",
+                                                         "objects",
+                                                         "signals"};
+    capture_unmodeled(road_node, {}, kRoadChildren, network().road(road_id)->road_extras);
     for (const pugi::xml_node child : road_node.children()) {
-      const std::string name = child.name();
-      if (name != "planView" && name != "elevationProfile" && name != "lateralProfile" &&
-          name != "lanes" && name != "link" && name != "type" && name != "userData" &&
-          name != "objects" && name != "signals") {
-        warn_unsupported(name, location);
+      const std::string_view name = child.name();
+      if (std::ranges::find(kRoadChildren, name) == std::end(kRoadChildren)) {
+        note_preserved(std::string(name), location);
       }
     }
     current_road_ = {};
@@ -792,9 +819,15 @@ class Parser {
           superelevation, fmt::format("{}/lateralProfile/superelevation[{}]", location, index)));
       ++index;
     }
+    // <shape> (§10.5.1) and the legacy <crossfall> used to be warned about and
+    // DROPPED, which flattened a non-planar carriageway on the first save
+    // (fmt-f2, #539). They are preserved verbatim instead; modeling stays with
+    // whoever owns road shape later.
+    static constexpr std::string_view kLateralChildren[] = {"superelevation"};
+    capture_unmodeled(profile, {}, kLateralChildren, road.lateral_profile_extras);
     for (const pugi::xml_node child : profile.children()) {
       if (std::string_view(child.name()) != "superelevation") {
-        warn_unsupported(child.name(), location + "/lateralProfile");
+        note_preserved(child.name(), location + "/lateralProfile");
       }
     }
   }
@@ -3534,18 +3567,26 @@ class Parser {
     return link;
   }
 
-  void warn_unsupported_root_children(const pugi::xml_node& root) {
+  /// Root children RoadMaker does not model — `<junctionGroup>` (§12.16),
+  /// `<station>` (chapter 15) and an `<include>` outside `<header>` (§7.1) —
+  /// are PRESERVED rather than warned-and-dropped (fmt-f2, #539). A legal file
+  /// used to lose its roundabout grouping or rail layer on the first save.
+  ///
+  /// Root `<userData>` carries RoadMaker extensions (rm:surface, rm:terrain);
+  /// they are parsed by parse_surfaces/parse_terrain_reference, and every other
+  /// code is preserved by parse_root_user_data (fmt-s2, #326) — so it is named
+  /// modeled here and never lands in this list.
+  void preserve_unmodeled_root_children(const pugi::xml_node& root) {
+    std::vector<std::string> fragments;
     for (const pugi::xml_node child : root.children()) {
       const std::string_view name = child.name();
-      // Root <userData> carries RoadMaker extensions (rm:surface, rm:terrain);
-      // they are parsed by parse_surfaces/parse_terrain_reference, and every
-      // other code is preserved verbatim by parse_root_user_data (fmt-s2,
-      // #326) — none of it is unsupported.
       if (name != "header" && name != "road" && name != "junction" && name != "controller" &&
           name != "userData") {
-        warn_unsupported(std::string(name), "OpenDRIVE");
+        fragments.push_back(node_to_string(child));
+        note_preserved(std::string(name), "OpenDRIVE");
       }
     }
+    network().set_preserved_root_children(std::move(fragments));
   }
 
   RoadNetwork& network() { return result_.network; }

core/src/xodr/writer.cpp

@@ -1363,9 +1363,13 @@ void write_road(pugi::xml_node root,
       append_fragment(profile, fragment);
     }
   }
-  if (!road.superelevation.empty()) {
-    write_poly3_list(
-        road_node.append_child("lateralProfile"), "superelevation", "s", road.superelevation);
+  if (!road.superelevation.empty() || !road.lateral_profile_extras.children.empty()) {
+    pugi::xml_node lateral = road_node.append_child("lateralProfile");
+    write_poly3_list(lateral, "superelevation", "s", road.superelevation);
+    // <shape>/<crossfall>, preserved rather than dropped (fmt-f2, #539).
+    for (const std::string& fragment : road.lateral_profile_extras.children) {
+      append_fragment(lateral, fragment);
+    }
   }
 
   pugi::xml_node lanes = road_node.append_child("lanes");
@@ -1416,6 +1420,12 @@ void write_road(pugi::xml_node root,
   // <signals> follows <objects> in the road element sequence (1.9.0 §10.1).
   write_signals(road_node, network, road_id, road, options);
 
+  // Road children RoadMaker does not model — <surface>/<CRG> (§10.6),
+  // <railroad> (chapter 15) — preserved rather than dropped (fmt-f2, #539).
+  for (const std::string& fragment : road.road_extras.children) {
+    append_fragment(road_node, fragment);
+  }
+
   // Authoring waypoints round-trip through the spec-sanctioned <userData>
   // extension (OpenDRIVE 1.9.0 §7.2: code required, value optional free
   // text). Emitted last so the normative children keep their order.
@@ -3028,6 +3038,13 @@ Expected<std::string> write_xodr(const RoadNetwork& network,
     append_fragment(root, fragment);
   }
 
+  // Root children RoadMaker does not model — <junctionGroup> (§12.16),
+  // <station> (chapter 15), an <include> outside <header> (§7.1) — preserved
+  // rather than dropped (fmt-f2, #539).
+  for (const std::string& fragment : network.preserved_root_children()) {
+    append_fragment(root, fragment);
+  }
+
   std::ostringstream out;
   doc.save(out, "  ", pugi::format_default, pugi::encoding_utf8);
   return std::move(out).str();

core/tests/fuzz/corpus/preserved_sweep.xodr

@@ -0,0 +1,93 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- VALID seed: the five spec areas that were warned-and-DROPPED (fmt-f2,
+     #539). One diagnostic, then permanent data loss — worse than a silent drop
+     in one respect, because the file looked like it had been understood.
+
+       1. <lateralProfile><shape> (§10.5.1) and the legacy <crossfall>:
+          parse_lateral_profile modeled <superelevation> only, so a round trip
+          FLATTENED a non-planar carriageway.
+       2. Road <surface>/<CRG> (§10.6): not in parse_road's child whitelist, so
+          a CRG-referencing file lost its surface detail on save.
+       3. <junctionGroup> (§12.16): the root sweep whitelisted
+          header/road/junction/controller/userData, so the standard's own
+          roundabout grouping was unrepresentable — directly relevant to #495.
+       4. <railroad> road child and root <station> (chapter 15): a tram file
+          lost its rail layer.
+       5. <include> outside <header> (§7.1): preserved INSIDE the header since
+          #326, warned-dropped anywhere else.
+
+     None of these is modeled by this build, and none needs to be for v0.1.0.
+     The bar is that they round-trip byte-for-byte and that the diagnostic says
+     "preserved, not modeled" rather than "ignored". Modeling stays with
+     whoever owns the feature later.
+
+     Apache-2.0. -->
+<OpenDRIVE>
+  <header revMajor="1" revMinor="8" name="preserved_sweep" vendor="RoadMaker" />
+  <road name="sweeping" length="100" id="1" junction="-1">
+    <planView>
+      <geometry s="0" x="0" y="0" hdg="0" length="100">
+        <line />
+      </geometry>
+    </planView>
+    <lateralProfile>
+      <superelevation s="0" a="0" b="0" c="0" d="0" />
+      <shape s="0" t="-3.5" a="0.02" b="0" c="0" d="0" />
+      <shape s="0" t="3.5" a="0.02" b="0" c="0" d="0" />
+      <crossfall side="both" s="0" a="0.015" b="0" c="0" d="0" />
+    </lateralProfile>
+    <lanes>
+      <laneSection s="0">
+        <center>
+          <lane id="0" type="none" level="false" />
+        </center>
+        <right>
+          <lane id="-1" type="driving" level="false">
+            <width sOffset="0" a="3.5" b="0" c="0" d="0" />
+          </lane>
+        </right>
+      </laneSection>
+    </lanes>
+    <surface>
+      <CRG file="pavement.crg" sStart="0" sEnd="100" orientation="same" mode="attached" />
+    </surface>
+    <railroad>
+      <switch name="north" id="7" position="dynamic">
+        <mainTrack id="1" s="40" dir="+" />
+        <sideTrack id="2" s="0" dir="+" />
+        <partner name="south" id="8" />
+      </switch>
+    </railroad>
+  </road>
+  <road name="ring" length="60" id="2" junction="-1">
+    <planView>
+      <geometry s="0" x="0" y="-40" hdg="0" length="60">
+        <line />
+      </geometry>
+    </planView>
+    <lanes>
+      <laneSection s="0">
+        <center>
+          <lane id="0" type="none" level="false" />
+        </center>
+        <right>
+          <lane id="-1" type="driving" level="false">
+            <width sOffset="0" a="3.5" b="0" c="0" d="0" />
+          </lane>
+        </right>
+      </laneSection>
+    </lanes>
+  </road>
+  <junction id="100" name="ring_a" />
+  <junction id="101" name="ring_b" />
+  <junctionGroup id="900" name="roundabout" type="roundabout">
+    <junctionReference junction="100" />
+    <junctionReference junction="101" />
+  </junctionGroup>
+  <station name="tram_stop" id="500" type="onLine">
+    <platform name="north" id="1">
+      <segment roadId="2" sStart="10" sEnd="40" side="right" />
+    </platform>
+  </station>
+  <include file="extra_geometry.xodr" />
+</OpenDRIVE>

core/tests/test_round_trip.cpp

@@ -282,6 +282,70 @@ TEST(RoundTrip, LaneDirectionSurvivesWriteParseWrite) {
   EXPECT_EQ(*xml, *again);
 }
 
+// --- the warned-and-dropped spec areas (fmt-f2, #539) ------------------------
+//
+// These were LOUD and lossy: one diagnostic, then permanent data loss. Worse
+// than a silent drop in one respect — the file looked like it had been
+// understood. None of them is modeled by this build; the bar is that they
+// round-trip byte-for-byte and that the diagnostic says so.
+
+TEST(RoundTrip, TheWarnedAndDroppedScopesNowSurviveWriteParseWrite) {
+  const std::filesystem::path sample =
+      std::filesystem::path(RM_FUZZ_CORPUS_DIR) / "preserved_sweep.xodr";
+  const auto loaded = roadmaker::load_xodr(sample);
+  ASSERT_TRUE(loaded.has_value()) << (loaded ? "" : loaded.error().message);
+
+  const auto written = roadmaker::write_xodr(loaded->network, "preserved_sweep");
+  ASSERT_TRUE(written.has_value());
+
+  for (const std::string_view marker : {
+           "<shape",         // 1. lateralProfile <shape> (§10.5.1)
+           "<crossfall",     // 1. the legacy <crossfall>
+           "<surface",       // 2. road <surface> (§10.6)
+           "<CRG",           // 2. its <CRG> child
+           "<junctionGroup", // 3. §12.16 — the roundabout grouping (#495)
+           "<railroad",      // 4. chapter 15, road child
+           "<station",       // 4. chapter 15, root child
+           "<include",       // 5. §7.1, outside <header>
+       }) {
+    EXPECT_NE(written->find(marker), std::string::npos) << "lost: " << marker;
+  }
+  // Content, not just the tag: a writer emitting an empty <shape/> would pass
+  // a tag-only check while still having flattened the carriageway.
+  EXPECT_NE(written->find(R"(file="pavement.crg")"), std::string::npos);
+  EXPECT_NE(written->find(R"(junction="101")"), std::string::npos);
+  EXPECT_NE(written->find(R"(file="extra_geometry.xodr")"), std::string::npos);
+
+  // Fixed point.
+  const auto reparsed = roadmaker::parse_xodr(*written, "preserved_sweep");
+  ASSERT_TRUE(reparsed.has_value());
+  const auto again = roadmaker::write_xodr(reparsed->network, "preserved_sweep");
+  ASSERT_TRUE(again.has_value());
+  EXPECT_EQ(*written, *again);
+  EXPECT_EQ(roadmaker::count_errors(loaded->diagnostics), 0U);
+}
+
+TEST(RoundTrip, TheSweptScopesSayPreservedRatherThanIgnored) {
+  // The diagnostic is half the fix. "not supported yet and was ignored" was
+  // TRUE before and is a lie now; what the user still needs to know is that the
+  // element has no effect, not that it was thrown away.
+  const auto loaded =
+      roadmaker::load_xodr(std::filesystem::path(RM_FUZZ_CORPUS_DIR) / "preserved_sweep.xodr");
+  ASSERT_TRUE(loaded.has_value());
+
+  const auto says = [&](std::string_view needle) {
+    return std::ranges::any_of(loaded->diagnostics, [&](const roadmaker::Diagnostic& d) {
+      return d.message.find(needle) != std::string::npos;
+    });
+  };
+  EXPECT_TRUE(says("<shape> is preserved verbatim but not modeled"));
+  EXPECT_TRUE(says("<junctionGroup> is preserved verbatim but not modeled"));
+  EXPECT_TRUE(says("<railroad> is preserved verbatim but not modeled"));
+  // And nothing claims to have ignored them any more.
+  EXPECT_FALSE(says("<shape> is not supported yet and was ignored"));
+  EXPECT_FALSE(says("<junctionGroup> is not supported yet and was ignored"));
+}
+
 // --- the remaining non-preserving parse scopes (fmt-f1, #453) ----------------
 //
 // Six scopes that each dropped unmodeled input in silence. They are asserted