From d5706c503b8bc6bf4c5e1555301c590997400697 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 11:43:30 -0400
Subject: [PATCH 01/20] --add DatasetDiagnosticTool class

---
 .../managers/DatasetDiagnosticsTool.cpp       |  76 ++++++++
 .../managers/DatasetDiagnosticsTool.h         | 184 ++++++++++++++++++
 2 files changed, 260 insertions(+)
 create mode 100644 src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
 create mode 100644 src/esp/metadata/managers/DatasetDiagnosticsTool.h

diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp b/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
new file mode 100644
index 0000000000..8d023fccc7
--- /dev/null
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
@@ -0,0 +1,76 @@
+
+// Copyright (c) Meta Platforms, Inc. and its affiliates.
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
+#include "DatasetDiagnosticsTool.h"
+#include "esp/core/Check.h"
+
+namespace esp {
+namespace metadata {
+namespace managers {
+
+const std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap = {
+    {"SaveCorrected", DSDiagnosticType::SaveCorrected},
+    {"TestForSceneInstanceDuplicates",
+     DSDiagnosticType::TestForDuplicateInstances},
+    {"TestForSemanticRegionDuplicates",
+     DSDiagnosticType::TestForDuplicateRegions},
+};
+
+bool DatasetDiagnosticsTool::setDiagnosticesFromJson(
+    const io::JsonGenericValue& _jsonObj,
+    const std::string& _msgStr) {
+  if (_jsonObj.IsString()) {
+    // Single diagnostic string specified.
+    return setNamedDiagnostic(_jsonObj.GetString(), true, true);
+  }
+  if (_jsonObj.IsArray()) {
+    bool success = false;
+    for (rapidjson::SizeType i = 0; i < _jsonObj.Size(); ++i) {
+      if (_jsonObj[i].IsString()) {
+        success = setNamedDiagnostic(_jsonObj[i].GetString(), true, true);
+      } else {
+        ESP_ERROR(Mn::Debug::Flag::NoSpace)
+            << _msgStr
+            << " configuration specifies `request_diagnostics` array with a "
+               "non-string element @idx "
+            << i << ". Skipping unknown element.";
+      }
+    }
+    return success;
+  }
+  // else json object support? tag->boolean map in json?
+  // Tag present but referneces neither a string nor an array
+  ESP_ERROR(Mn::Debug::Flag::NoSpace)
+      << _msgStr
+      << " configuration specifies `request_diagnostics` but specification "
+         "is unable to be parsed, so diagnostics request is ignored.";
+  return false;
+}  // DatasetDiagnosticsTool::setDiagnosticesFromJson
+
+bool DatasetDiagnosticsTool::setNamedDiagnostic(const std::string& _diagnostic,
+                                                bool _val,
+                                                bool _abortOnFail) {
+  auto mapIter = DSDiagnosticTypeMap.find(_diagnostic);
+  if (_abortOnFail) {
+    // If not found then abort.
+    ESP_CHECK(mapIter != DSDiagnosticTypeMap.end(),
+              "Unknown Diagnostic Test requested to be "
+                  << (_val ? "Enabled" : "Disabled") << " :" << _diagnostic
+                  << ". Aborting.");
+  } else {
+    if (mapIter == DSDiagnosticTypeMap.end()) {
+      ESP_ERROR() << "Unknown Diagnostic Test requested to be "
+                  << (_val ? "Enabled" : "Disabled") << " :" << _diagnostic
+                  << " so ignoring request.";
+      return false;
+    }
+  }
+  _setFlags(mapIter->second, _val);
+  return true;
+}  // DatasetDiagnosticsTool::setNamedDiagnostic
+
+}  // namespace managers
+}  // namespace metadata
+}  // namespace esp
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
new file mode 100644
index 0000000000..75e8c9532b
--- /dev/null
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -0,0 +1,184 @@
+
+// Copyright (c) Meta Platforms, Inc. and its affiliates.
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
+#ifndef ESP_METADATA_MANAGERS_DATASETDIAGNOSTICSTOOL_H_
+#define ESP_METADATA_MANAGERS_DATASETDIAGNOSTICSTOOL_H_
+
+#include "esp/core/Esp.h"
+#include "esp/io/Json.h"
+
+namespace esp {
+namespace metadata {
+namespace managers {
+
+/**
+ * @brief This enum class defines the various dataset diagnostics and remedies
+ * that Habitat-Sim can accept and process. These flags will be specified in
+ * config files and consumed by the config's manager.
+ */
+enum class DSDiagnosticType : uint32_t {
+  /**
+   * @brief Save any dataset configurations that fail a requested diagnostic
+   * and are consequently corrected. Ignored if no diagnostics are specified. If
+   * diagnostics are specified but this flag is not set then the corrections
+   * will only persist during current execution.
+   */
+  SaveCorrected = 1U,
+
+  /**
+   * @brief Test each @ref SceneInstanceAttributes file as it is loaded for
+   * duplicate rigid and articulated object instances. Duplicates will have all
+   * non-hidden fields equal. This would result in 2 identical objects being
+   * instantiated in the same location with the same initial state.
+   */
+  TestForDuplicateInstances = (1U << 1),
+
+  /**
+   * @brief Test each @ref SemanticAttributes file as it is loaded for
+   * duplicate region instances. Duplicates will have all non-hidden fields
+   * equal. This would result in multiple semantic regions being defined for the
+   * same area in the scene.
+   */
+  TestForDuplicateRegions = (1U << 2),
+
+  /**
+   * @brief End cap value - no Dataset Diagnostic Type enums should be
+   * defined at or past this enum.
+   */
+  EndDSDiagnosticTypesType = (1U << 31),
+};
+
+/**
+ * @brief Constant map to provide mappings from string tags to @ref
+ * DSDiagnosticType values. This will be used to match values set
+ * in json for requested dataset diagnostics to @ref DSDiagnosticType values.
+ */
+const extern std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap;
+
+/**
+ * @brief This class will track requested diagnostics for specific config
+ * files/attributes. It is intended that each @ref AbstractAttributesManager
+ * would instantiate one of these objects and use it to determine various
+ * diagnostic behavior.
+ */
+class DatasetDiagnosticsTool {
+ public:
+  DatasetDiagnosticsTool() {}
+
+  /**
+   * @brief Set diagnostic values based on specifications in passed @p _jsonObj.
+   * @param _jsonObj The json object referenced by the appropriate diagnostics
+   * tag.
+   * @param _msgStr A string containing the type of manager and the name of the
+   * attributes/config responsible for this call, for use in debug messages.
+   * @return Whether the diagnostic values were set successfully or not.
+   */
+  bool setDiagnosticesFromJson(const io::JsonGenericValue& _jsonObj,
+                               const std::string& _msgStr);
+
+  /**
+   * @brief Take a list of string tags that are defined as keys in
+   * @ref DSDiagnosticTypeMap and set their corresponding flag values to true.
+   * @param _keyMappings A list of diagnostics to enable.
+   * @param _abortOnFail Whether or not to abort the program if a diagnostic
+   * string in
+   * @p _keyMappings is unable to be mapped (not found in DSDiagnosticType).
+   * Otherwise the erroneous value will produce an error message but otherwise
+   * will be ignored.
+   */
+  void enableDiagnostics(const std::vector<std::string>& _keyMappings,
+                         bool _abortOnFail) {
+    for (const auto& key : _keyMappings) {
+      setNamedDiagnostic(key, true, _abortOnFail);
+    }
+  }  // enableDiagnostics
+
+  /**
+   * @brief Enable/disable the diagnostic given by @p _diagnostic based on
+   * @p _val.
+   *
+   * @param _diagnostic The string name of the diagnostic. This must be mappable
+   * to a @ref DSDiagnosticType via @p DSDiagnosticTypeMap .
+   * @param _val Whether to enable or disable the given diagnostic
+   * @param _abortOnFail Whether or not to abort the program if the
+   * @p _diagnostic requeseted is not found in  @ref DSDiagnosticType. If false,
+   * will print an error message and skip the unknown request.
+   * @return Whether the passed string successfully mapped to a known diagnostic
+   */
+  bool setNamedDiagnostic(const std::string& _diagnostic,
+                          bool _val,
+                          bool _abortOnFail = false);
+
+  /**
+   * @brief Specify whether or not to save any corrected dataset components if
+   * they received correction
+   */
+  void setSaveCorrected(bool _val) {
+    _setFlags(DSDiagnosticType::SaveCorrected, _val);
+  }
+  /**
+   * @brief Query whether or not to save any corrected dataset components if
+   * they received correction
+   */
+  bool saveCorrected() const {
+    return _getFlags(DSDiagnosticType::SaveCorrected);
+  }
+
+  /**
+   * @brief Specify whether or not to test for duplicate scene object instances
+   * in loaded @ref SceneInstanceAttributes and not process the duplicates if found.
+   */
+  void setTestDuplicateSceneInstances(bool _val) {
+    _setFlags(DSDiagnosticType::TestForDuplicateInstances, _val);
+  }
+  /**
+   * @brief Query whether or not to test for duplicate scene object instances
+   * in loaded @ref SceneInstanceAttributes and not process the duplicates if found.
+   */
+  bool testDuplicateSceneInstances() const {
+    return _getFlags(DSDiagnosticType::TestForDuplicateInstances);
+  }
+
+  /**
+   * @brief Specify whether or not to test for duplicate semantic region
+   * specifications
+   * in loaded @ref SemanticAttributes and not process the duplicates if found.
+   */
+  void setTestDuplicateSemanticRegions(bool _val) {
+    _setFlags(DSDiagnosticType::TestForDuplicateRegions, _val);
+  }
+  /**
+   * @brief Query whether or not to test for duplicate semantic region
+   * specifications in loaded @ref SemanticAttributes and not process the duplicates if found.
+   */
+  bool testDuplicateSemanticRegions() const {
+    return _getFlags(DSDiagnosticType::TestForDuplicateRegions);
+  }
+
+ private:
+  inline void _setFlags(DSDiagnosticType _flag, bool _val) {
+    if (_val) {
+      _diagnosticsFlags |= static_cast<uint32_t>(_flag);
+    } else {
+      _diagnosticsFlags &= ~static_cast<uint32_t>(_flag);
+    }
+  }
+
+  inline bool _getFlags(DSDiagnosticType _flag) const {
+    return (_diagnosticsFlags & static_cast<uint32_t>(_flag)) ==
+           static_cast<uint32_t>(_flag);
+  }
+  uint32_t _diagnosticsFlags = 0u;
+
+ public:
+  ESP_SMART_POINTERS(DatasetDiagnosticsTool)
+
+};  // class DatasetDiagnosticsTool
+
+}  // namespace managers
+}  // namespace metadata
+}  // namespace esp
+
+#endif  // ESP_METADATA_MANAGERS_DATASETDIAGNOSTICSTOOL_H_

From e1c6ef9d12401053224673cc1c8fce2a3f6702b3 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 13:20:50 -0400
Subject: [PATCH 02/20] --reference DatasetDiagnosticsTool in base
 AttributesManager

---
 .../managers/AbstractAttributesManager.h      | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
index 680e0d8241..26405e55db 100644
--- a/src/esp/metadata/managers/AbstractAttributesManager.h
+++ b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -9,6 +9,7 @@
  * @brief Class Template @ref esp::metadata::managers::AbstractAttributesManager
  */
 
+#include "DatasetDiagnosticsTool.h"
 #include "esp/metadata/attributes/AbstractAttributes.h"
 
 #include "esp/core/managedContainers/ManagedFileBasedContainer.h"
@@ -64,7 +65,8 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
                             const std::string& JSONTypeExt)
       : ManagedFileBasedContainer<T, Access>::ManagedFileBasedContainer(
             attrType,
-            JSONTypeExt) {}
+            JSONTypeExt),
+        _DSDiagnostics(DatasetDiagnosticsTool::create_unique()) {}
   ~AbstractAttributesManager() override = default;
 
   /**
@@ -187,6 +189,23 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
   virtual void setValsFromJSONDoc(AttribsPtr attribs,
                                   const io::JsonGenericValue& jsonConfig) = 0;
 
+  /**
+   * @brief Configure @ref _DSDiagnostics tool based on if passsed jsonConfig
+   * requests them.
+   */
+  bool setDSDiagnostics(AttribsPtr attribs,
+                        const io::JsonGenericValue& jsonConfig) {
+    io::JsonGenericValue::ConstMemberIterator jsonIter =
+        jsonConfig.FindMember("request_diagnostics");
+    if (jsonIter == jsonConfig.MemberEnd()) {
+      return false;
+    }
+    return this->_DSDiagnostics->setDiagnosticesFromJson(
+        jsonIter->value, Cr::Utility::formatString(
+                             "(setDSDiagnostics) <{}> : {}", this->objectType_,
+                             attribs->getSimplifiedHandle()));
+  }  // setDSDiagnostics
+
   /**
    * @brief This function takes the json block specifying user-defined values
    * and parses it into the passed existing attributes.
@@ -396,6 +415,12 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
       const std::string& srcAssetHandle,
       const std::function<void(const std::string&)>& handleSetter);
 
+  /**
+   * @brief Diagnostics tool that governs whether certain diagnostic operations
+   * should occur.
+   */
+  DatasetDiagnosticsTool::uptr _DSDiagnostics;
+
  public:
   ESP_SMART_POINTERS(AbstractAttributesManager<T, Access>)
 

From 9e1fc1a7be17fd116e81b7a5c652554c1db78abf Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 13:30:28 -0400
Subject: [PATCH 03/20] --query for DatasetDiagnostic requests from scene
 dataset config.

---
 src/esp/metadata/managers/SceneDatasetAttributesManager.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
index 602341e5fa..a4a264ecc8 100644
--- a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
@@ -69,6 +69,9 @@ SceneDatasetAttributesManager::initNewObjectInternal(
 void SceneDatasetAttributesManager::setValsFromJSONDoc(
     attributes::SceneDatasetAttributes::ptr dsAttribs,
     const io::JsonGenericValue& jsonConfig) {
+  // check for diagnostics requests
+  this->setDSDiagnostics(dsAttribs, jsonConfig);
+
   // dataset root directory to build paths from
   std::string dsDir = dsAttribs->getFileDirectory();
   // process stages

From ab1f2fc761def3d86b0c825fe97d8e185ba4cee7 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 13:57:17 -0400
Subject: [PATCH 04/20] --merge scene dataset diagnostics settings with each
 manager's tool

---
 src/esp/metadata/managers/AbstractAttributesManager.h    | 9 +++++++++
 src/esp/metadata/managers/DatasetDiagnosticsTool.h       | 7 +++++++
 .../metadata/managers/SceneDatasetAttributesManager.cpp  | 3 +++
 3 files changed, 19 insertions(+)

diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
index 26405e55db..d868e196f6 100644
--- a/src/esp/metadata/managers/AbstractAttributesManager.h
+++ b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -206,6 +206,15 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
                              attribs->getSimplifiedHandle()));
   }  // setDSDiagnostics
 
+  /**
+   * @brief Merge the passed DatasetDiagnosticsTool settings into
+   * @p _DSDiagnostics nondestructively (i.e. retaining this one's enabled
+   * diagnostics)
+   */
+  void mergeDSDiagnosticsTool(const DatasetDiagnosticsTool& _tool) {
+    this->_DSDiagnostics->mergeDiagnosticsTool(_tool);
+  }
+
   /**
    * @brief This function takes the json block specifying user-defined values
    * and parses it into the passed existing attributes.
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
index 75e8c9532b..c614a92b19 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -78,6 +78,13 @@ class DatasetDiagnosticsTool {
   bool setDiagnosticesFromJson(const io::JsonGenericValue& _jsonObj,
                                const std::string& _msgStr);
 
+  /**
+   * @brief Merge the passed @ref DatasetDiagnosticsTool's flag settings into this one's.
+   */
+  void mergeDiagnosticsTool(const DatasetDiagnosticsTool& _tool) {
+    _diagnosticsFlags |= _tool._diagnosticsFlags;
+  }
+
   /**
    * @brief Take a list of string tags that are defined as keys in
    * @ref DSDiagnosticTypeMap and set their corresponding flag values to true.
diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
index a4a264ecc8..8b4aa9cec1 100644
--- a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
@@ -141,6 +141,9 @@ void SceneDatasetAttributesManager::readDatasetJSONCell(
     const io::JsonGenericValue& jsonConfig,
     const U& attrMgr,
     std::map<std::string, std::string>* strKeyMap) {
+  // merge DatasetDiagnostics tool from this manager into passed manager
+  attrMgr->mergeDSDiagnosticsTool(*(this->_DSDiagnostics));
+
   io::JsonGenericValue::ConstMemberIterator jsonIter =
       jsonConfig.FindMember(tag);
   if (jsonIter != jsonConfig.MemberEnd()) {

From 74d7531f84a019ef39bf2d2be0d4acfda29e70ce Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 14:08:25 -0400
Subject: [PATCH 05/20] --tie in diagnostics with value setting from json

---
 .../SceneInstanceAttributesManager.cpp        | 48 ++++++++++++++-----
 .../managers/SemanticAttributesManager.cpp    | 27 +++++++++--
 2 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
index c7e5c759e1..7d976373d1 100644
--- a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
@@ -111,8 +111,19 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc(
                     << "` so no Stage can be created for this Scene.");
     }
   }
-  // TODO : drive by diagnostics when implemented
-  bool validateUnique = false;
+
+  // Set this via configuration value - should only be called when explicitly
+  // filtering dataset
+  // Whether uniqueness validation should be performed
+  bool validateUniqueness = this->_DSDiagnostics->testDuplicateSceneInstances();
+
+  // Only resave if instance attributes' attempt to be added reveals duplicate
+  // attributes
+  bool saveValidationResults =
+      validateUniqueness && this->_DSDiagnostics->saveCorrected();
+
+  // Whether this scene instance should be resaved or not.
+  bool resaveAttributes = false;
   // Check for object instances existence
   io::JsonGenericValue::ConstMemberIterator objJSONIter =
       jsonConfig.FindMember("object_instances");
@@ -123,8 +134,11 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc(
       for (rapidjson::SizeType i = 0; i < objectArray.Size(); ++i) {
         const auto& objCell = objectArray[i];
         if (objCell.IsObject()) {
-          attribs->addObjectInstanceAttrs(
-              createInstanceAttributesFromJSON(objCell), validateUnique);
+          // Resave if attributes not added due to being already found in the
+          // subconfiguration
+          bool objWasAdded = attribs->addObjectInstanceAttrs(
+              createInstanceAttributesFromJSON(objCell), validateUniqueness);
+          resaveAttributes = !objWasAdded || resaveAttributes;
         } else {
           ESP_WARNING(Mn::Debug::Flag::NoSpace)
               << "Object instance issue in Scene Instance `" << attribsDispName
@@ -134,9 +148,9 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc(
         }
       }
     } else {
-      // object_instances tag exists but is not an array. should warn (perhaps
-      // error?)
-      ESP_WARNING(Mn::Debug::Flag::NoSpace)
+      // object_instances tag exists but is not an array; gives error message.
+      // (Perhaps should fail?)
+      ESP_ERROR(Mn::Debug::Flag::NoSpace)
           << "Object instances issue in Scene Instance `" << attribsDispName
           << "`: JSON cell `object_instances` is not a valid JSON "
              "array, so no object instances loaded.";
@@ -161,8 +175,12 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc(
         const auto& artObjCell = articulatedObjArray[i];
 
         if (artObjCell.IsObject()) {
-          attribs->addArticulatedObjectInstanceAttrs(
-              createAOInstanceAttributesFromJSON(artObjCell), validateUnique);
+          // Resave if attributes not added due to being already found in the
+          // subconfiguration
+          bool artObjWasAdded = attribs->addArticulatedObjectInstanceAttrs(
+              createAOInstanceAttributesFromJSON(artObjCell),
+              validateUniqueness);
+          resaveAttributes = !artObjWasAdded || resaveAttributes;
         } else {
           ESP_WARNING(Mn::Debug::Flag::NoSpace)
               << "Articulated Object specification error in Scene Instance `"
@@ -172,9 +190,9 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc(
         }
       }
     } else {
-      // articulated_object_instances tag exists but is not an array. should
-      // warn (perhaps error?)
-      ESP_WARNING(Mn::Debug::Flag::NoSpace)
+      // articulated_object_instances tag exists but is not an array; gives
+      // error message. (Perhaps should fail?)
+      ESP_ERROR(Mn::Debug::Flag::NoSpace)
           << "Articulated Object instances issue in Scene "
              "InstanceScene Instance `"
           << attribsDispName
@@ -291,6 +309,12 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc(
   // check for user defined attributes
   this->parseUserDefinedJsonVals(attribs, jsonConfig);
 
+  // If we want to save corrected, and we need to due to corrections happening
+  if (saveValidationResults && resaveAttributes) {
+    // TODO Resave this attributes due to duplicate entries being filtered out
+    // via diagnostics
+  }
+
 }  // SceneInstanceAttributesManager::setValsFromJSONDoc
 
 SceneObjectInstanceAttributes::ptr
diff --git a/src/esp/metadata/managers/SemanticAttributesManager.cpp b/src/esp/metadata/managers/SemanticAttributesManager.cpp
index 94a9690e1f..6e67b3d526 100644
--- a/src/esp/metadata/managers/SemanticAttributesManager.cpp
+++ b/src/esp/metadata/managers/SemanticAttributesManager.cpp
@@ -199,6 +199,21 @@ void SemanticAttributesManager::setValsFromJSONDoc(
   // Check for region instances existence
   io::JsonGenericValue::ConstMemberIterator regionJSONIter =
       jsonConfig.FindMember("region_annotations");
+
+  // Set this via configuration value - should only be called when explicitly
+  // filtering dataset
+  // Whether uniqueness validation should be performed
+  bool validateUniqueness =
+      this->_DSDiagnostics->testDuplicateSemanticRegions();
+
+  // Only resave if instance attributes' attempt to be added reveals duplicate
+  // attributes
+  bool saveValidationResults =
+      validateUniqueness && this->_DSDiagnostics->saveCorrected();
+
+  // Whether this scene instance should be resaved or not.
+  bool resaveAttributes = false;
+
   if (regionJSONIter != jsonConfig.MemberEnd()) {
     // region_annotations tag exists
     if (regionJSONIter->value.IsArray()) {
@@ -206,8 +221,9 @@ void SemanticAttributesManager::setValsFromJSONDoc(
       for (rapidjson::SizeType i = 0; i < regionArray.Size(); ++i) {
         const auto& regionCell = regionArray[i];
         if (regionCell.IsObject()) {
-          semanticAttribs->addRegionInstanceAttrs(
-              createRegionAttributesFromJSON(regionCell), validateUnique);
+          bool regionWasAdded = semanticAttribs->addRegionInstanceAttrs(
+              createRegionAttributesFromJSON(regionCell), validateUniqueness);
+          resaveAttributes = !regionWasAdded || resaveAttributes;
         } else {
           ESP_WARNING(Mn::Debug::Flag::NoSpace)
               << "Region instance issue in Semantic Configuration `"
@@ -234,10 +250,15 @@ void SemanticAttributesManager::setValsFromJSONDoc(
         << "`: JSON cell with tag `region_annotations` does not exist in "
            "Semantic Configuration file.";
   }
-
   // check for user defined attributes
   this->parseUserDefinedJsonVals(semanticAttribs, jsonConfig);
 
+  // If we want to save corrected, and we need to due to corrections happening
+  if (saveValidationResults && resaveAttributes) {
+    // TODO Resave this attributes due to duplicate entries being filtered out
+    // via diagnostics
+  }
+
 }  // SemanticAttributesManager::setValsFromJSONDoc
 
 core::managedContainers::ManagedObjectPreregistration

From fe73c186d31800b1943013bf29540e521171d4b6 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 14:55:25 -0400
Subject: [PATCH 06/20] --clang-tidy

---
 .../managers/DatasetDiagnosticsTool.cpp       | 43 ++++++------
 .../managers/DatasetDiagnosticsTool.h         | 70 +++++++++----------
 2 files changed, 57 insertions(+), 56 deletions(-)

diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp b/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
index 8d023fccc7..65a4fda8a7 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
@@ -11,28 +11,28 @@ namespace metadata {
 namespace managers {
 
 const std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap = {
-    {"SaveCorrected", DSDiagnosticType::SaveCorrected},
-    {"TestForSceneInstanceDuplicates",
+    {"savecorrected", DSDiagnosticType::SaveCorrected},
+    {"testforsceneinstanceduplicates",
      DSDiagnosticType::TestForDuplicateInstances},
-    {"TestForSemanticRegionDuplicates",
+    {"testforsemanticregionduplicates",
      DSDiagnosticType::TestForDuplicateRegions},
 };
 
 bool DatasetDiagnosticsTool::setDiagnosticesFromJson(
-    const io::JsonGenericValue& _jsonObj,
-    const std::string& _msgStr) {
-  if (_jsonObj.IsString()) {
+    const io::JsonGenericValue& jsonObj,
+    const std::string& msgStr) {
+  if (jsonObj.IsString()) {
     // Single diagnostic string specified.
-    return setNamedDiagnostic(_jsonObj.GetString(), true, true);
+    return setNamedDiagnostic(jsonObj.GetString(), true, true);
   }
-  if (_jsonObj.IsArray()) {
+  if (jsonObj.IsArray()) {
     bool success = false;
-    for (rapidjson::SizeType i = 0; i < _jsonObj.Size(); ++i) {
-      if (_jsonObj[i].IsString()) {
-        success = setNamedDiagnostic(_jsonObj[i].GetString(), true, true);
+    for (rapidjson::SizeType i = 0; i < jsonObj.Size(); ++i) {
+      if (jsonObj[i].IsString()) {
+        success = setNamedDiagnostic(jsonObj[i].GetString(), true, true);
       } else {
         ESP_ERROR(Mn::Debug::Flag::NoSpace)
-            << _msgStr
+            << msgStr
             << " configuration specifies `request_diagnostics` array with a "
                "non-string element @idx "
             << i << ". Skipping unknown element.";
@@ -43,31 +43,32 @@ bool DatasetDiagnosticsTool::setDiagnosticesFromJson(
   // else json object support? tag->boolean map in json?
   // Tag present but referneces neither a string nor an array
   ESP_ERROR(Mn::Debug::Flag::NoSpace)
-      << _msgStr
+      << msgStr
       << " configuration specifies `request_diagnostics` but specification "
          "is unable to be parsed, so diagnostics request is ignored.";
   return false;
 }  // DatasetDiagnosticsTool::setDiagnosticesFromJson
 
-bool DatasetDiagnosticsTool::setNamedDiagnostic(const std::string& _diagnostic,
-                                                bool _val,
-                                                bool _abortOnFail) {
-  auto mapIter = DSDiagnosticTypeMap.find(_diagnostic);
-  if (_abortOnFail) {
+bool DatasetDiagnosticsTool::setNamedDiagnostic(const std::string& diagnostic,
+                                                bool val,
+                                                bool abortOnFail) {
+  const std::string diagnosticLC = Cr::Utility::String::lowercase(diagnostic);
+  auto mapIter = DSDiagnosticTypeMap.find(diagnosticLC);
+  if (abortOnFail) {
     // If not found then abort.
     ESP_CHECK(mapIter != DSDiagnosticTypeMap.end(),
               "Unknown Diagnostic Test requested to be "
-                  << (_val ? "Enabled" : "Disabled") << " :" << _diagnostic
+                  << (val ? "Enabled" : "Disabled") << " :" << diagnostic
                   << ". Aborting.");
   } else {
     if (mapIter == DSDiagnosticTypeMap.end()) {
       ESP_ERROR() << "Unknown Diagnostic Test requested to be "
-                  << (_val ? "Enabled" : "Disabled") << " :" << _diagnostic
+                  << (val ? "Enabled" : "Disabled") << " :" << diagnostic
                   << " so ignoring request.";
       return false;
     }
   }
-  _setFlags(mapIter->second, _val);
+  setFlags(mapIter->second, val);
   return true;
 }  // DatasetDiagnosticsTool::setNamedDiagnostic
 
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
index c614a92b19..a3ff98486b 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -65,87 +65,87 @@ const extern std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap;
  */
 class DatasetDiagnosticsTool {
  public:
-  DatasetDiagnosticsTool() {}
+  DatasetDiagnosticsTool() = default;
 
   /**
-   * @brief Set diagnostic values based on specifications in passed @p _jsonObj.
-   * @param _jsonObj The json object referenced by the appropriate diagnostics
+   * @brief Set diagnostic values based on specifications in passed @p jsonObj.
+   * @param jsonObj The json object referenced by the appropriate diagnostics
    * tag.
-   * @param _msgStr A string containing the type of manager and the name of the
+   * @param msgStr A string containing the type of manager and the name of the
    * attributes/config responsible for this call, for use in debug messages.
    * @return Whether the diagnostic values were set successfully or not.
    */
-  bool setDiagnosticesFromJson(const io::JsonGenericValue& _jsonObj,
-                               const std::string& _msgStr);
+  bool setDiagnosticesFromJson(const io::JsonGenericValue& jsonObj,
+                               const std::string& msgStr);
 
   /**
    * @brief Merge the passed @ref DatasetDiagnosticsTool's flag settings into this one's.
    */
-  void mergeDiagnosticsTool(const DatasetDiagnosticsTool& _tool) {
-    _diagnosticsFlags |= _tool._diagnosticsFlags;
+  void mergeDiagnosticsTool(const DatasetDiagnosticsTool& tool) {
+    _diagnosticsFlags |= tool._diagnosticsFlags;
   }
 
   /**
    * @brief Take a list of string tags that are defined as keys in
    * @ref DSDiagnosticTypeMap and set their corresponding flag values to true.
-   * @param _keyMappings A list of diagnostics to enable.
-   * @param _abortOnFail Whether or not to abort the program if a diagnostic
+   * @param keyMappings A list of diagnostics to enable.
+   * @param abortOnFail Whether or not to abort the program if a diagnostic
    * string in
-   * @p _keyMappings is unable to be mapped (not found in DSDiagnosticType).
+   * @p keyMappings is unable to be mapped (not found in DSDiagnosticType).
    * Otherwise the erroneous value will produce an error message but otherwise
    * will be ignored.
    */
-  void enableDiagnostics(const std::vector<std::string>& _keyMappings,
-                         bool _abortOnFail) {
-    for (const auto& key : _keyMappings) {
-      setNamedDiagnostic(key, true, _abortOnFail);
+  void enableDiagnostics(const std::vector<std::string>& keyMappings,
+                         bool abortOnFail) {
+    for (const auto& key : keyMappings) {
+      setNamedDiagnostic(key, true, abortOnFail);
     }
   }  // enableDiagnostics
 
   /**
-   * @brief Enable/disable the diagnostic given by @p _diagnostic based on
-   * @p _val.
+   * @brief Enable/disable the diagnostic given by @p diagnostic based on
+   * @p val.
    *
-   * @param _diagnostic The string name of the diagnostic. This must be mappable
+   * @param diagnostic The string name of the diagnostic. This must be mappable
    * to a @ref DSDiagnosticType via @p DSDiagnosticTypeMap .
-   * @param _val Whether to enable or disable the given diagnostic
-   * @param _abortOnFail Whether or not to abort the program if the
-   * @p _diagnostic requeseted is not found in  @ref DSDiagnosticType. If false,
+   * @param val Whether to enable or disable the given diagnostic
+   * @param abortOnFail Whether or not to abort the program if the
+   * @p diagnostic requeseted is not found in  @ref DSDiagnosticType. If false,
    * will print an error message and skip the unknown request.
    * @return Whether the passed string successfully mapped to a known diagnostic
    */
-  bool setNamedDiagnostic(const std::string& _diagnostic,
-                          bool _val,
-                          bool _abortOnFail = false);
+  bool setNamedDiagnostic(const std::string& diagnostic,
+                          bool val,
+                          bool abortOnFail = false);
 
   /**
    * @brief Specify whether or not to save any corrected dataset components if
    * they received correction
    */
-  void setSaveCorrected(bool _val) {
-    _setFlags(DSDiagnosticType::SaveCorrected, _val);
+  void setSaveCorrected(bool val) {
+    setFlags(DSDiagnosticType::SaveCorrected, val);
   }
   /**
    * @brief Query whether or not to save any corrected dataset components if
    * they received correction
    */
   bool saveCorrected() const {
-    return _getFlags(DSDiagnosticType::SaveCorrected);
+    return getFlags(DSDiagnosticType::SaveCorrected);
   }
 
   /**
    * @brief Specify whether or not to test for duplicate scene object instances
    * in loaded @ref SceneInstanceAttributes and not process the duplicates if found.
    */
-  void setTestDuplicateSceneInstances(bool _val) {
-    _setFlags(DSDiagnosticType::TestForDuplicateInstances, _val);
+  void setTestDuplicateSceneInstances(bool val) {
+    setFlags(DSDiagnosticType::TestForDuplicateInstances, val);
   }
   /**
    * @brief Query whether or not to test for duplicate scene object instances
    * in loaded @ref SceneInstanceAttributes and not process the duplicates if found.
    */
   bool testDuplicateSceneInstances() const {
-    return _getFlags(DSDiagnosticType::TestForDuplicateInstances);
+    return getFlags(DSDiagnosticType::TestForDuplicateInstances);
   }
 
   /**
@@ -153,19 +153,19 @@ class DatasetDiagnosticsTool {
    * specifications
    * in loaded @ref SemanticAttributes and not process the duplicates if found.
    */
-  void setTestDuplicateSemanticRegions(bool _val) {
-    _setFlags(DSDiagnosticType::TestForDuplicateRegions, _val);
+  void setTestDuplicateSemanticRegions(bool val) {
+    setFlags(DSDiagnosticType::TestForDuplicateRegions, val);
   }
   /**
    * @brief Query whether or not to test for duplicate semantic region
    * specifications in loaded @ref SemanticAttributes and not process the duplicates if found.
    */
   bool testDuplicateSemanticRegions() const {
-    return _getFlags(DSDiagnosticType::TestForDuplicateRegions);
+    return getFlags(DSDiagnosticType::TestForDuplicateRegions);
   }
 
  private:
-  inline void _setFlags(DSDiagnosticType _flag, bool _val) {
+  inline void setFlags(DSDiagnosticType _flag, bool _val) {
     if (_val) {
       _diagnosticsFlags |= static_cast<uint32_t>(_flag);
     } else {
@@ -173,7 +173,7 @@ class DatasetDiagnosticsTool {
     }
   }
 
-  inline bool _getFlags(DSDiagnosticType _flag) const {
+  inline bool getFlags(DSDiagnosticType _flag) const {
     return (_diagnosticsFlags & static_cast<uint32_t>(_flag)) ==
            static_cast<uint32_t>(_flag);
   }

From e96e000aedae55295733ec8301ec1d52a0afbff3 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 16:01:19 -0400
Subject: [PATCH 07/20] --add 'all' and 'allSaveCorrected' options.

---
 src/esp/metadata/managers/DatasetDiagnosticsTool.cpp | 4 ++++
 src/esp/metadata/managers/DatasetDiagnosticsTool.h   | 9 ++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp b/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
index 65a4fda8a7..2e5f610aab 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
@@ -16,6 +16,9 @@ const std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap = {
      DSDiagnosticType::TestForDuplicateInstances},
     {"testforsemanticregionduplicates",
      DSDiagnosticType::TestForDuplicateRegions},
+    // Future diagnostics should be listed here
+    {"all", DSDiagnosticType::AllDiagnostics},
+    {"allsavecorrected", DSDiagnosticType::AllDiagnosticsSaveCorrected},
 };
 
 bool DatasetDiagnosticsTool::setDiagnosticesFromJson(
@@ -53,6 +56,7 @@ bool DatasetDiagnosticsTool::setNamedDiagnostic(const std::string& diagnostic,
                                                 bool val,
                                                 bool abortOnFail) {
   const std::string diagnosticLC = Cr::Utility::String::lowercase(diagnostic);
+
   auto mapIter = DSDiagnosticTypeMap.find(diagnosticLC);
   if (abortOnFail) {
     // If not found then abort.
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
index a3ff98486b..724f3ba0fb 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -44,10 +44,13 @@ enum class DSDiagnosticType : uint32_t {
   TestForDuplicateRegions = (1U << 2),
 
   /**
-   * @brief End cap value - no Dataset Diagnostic Type enums should be
-   * defined at or past this enum.
+   * @brief Perform all diagnostics but do not save corrected results.
    */
-  EndDSDiagnosticTypesType = (1U << 31),
+  AllDiagnostics = ~SaveCorrected,
+  /**
+   * @brief Perform all diagnostics and save corrected results
+   */
+  AllDiagnosticsSaveCorrected = ~0U
 };
 
 /**

From d43a338219d06dd1dd15b5f276fcc877e1fa37d2 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 16:48:49 -0400
Subject: [PATCH 08/20] --handle saving attributes if required by diagnostic
 results

Also encapsulate the json setting process so that diagnostic settings can be cleared beforehand.
---
 .../metadata/managers/AOAttributesManager.cpp |  2 +-
 .../metadata/managers/AOAttributesManager.h   | 35 ++++++++-------
 .../managers/AbstractAttributesManager.h      | 19 +++++++-
 .../managers/AssetAttributesManager.cpp       |  2 +-
 .../managers/AssetAttributesManager.h         | 19 ++++----
 .../managers/DatasetDiagnosticsTool.h         | 43 ++++++++++++++++---
 .../managers/LightLayoutAttributesManager.cpp |  2 +-
 .../managers/LightLayoutAttributesManager.h   | 35 ++++++++-------
 .../managers/ObjectAttributesManager.cpp      |  5 ++-
 .../managers/ObjectAttributesManager.h        | 19 ++++----
 .../managers/PbrShaderAttributesManager.cpp   |  2 +-
 .../managers/PbrShaderAttributesManager.h     | 35 ++++++++-------
 .../managers/PhysicsAttributesManager.cpp     |  2 +-
 .../managers/PhysicsAttributesManager.h       | 35 ++++++++-------
 .../SceneDatasetAttributesManager.cpp         |  2 +-
 .../managers/SceneDatasetAttributesManager.h  | 19 ++++----
 .../SceneInstanceAttributesManager.cpp        |  8 ++--
 .../managers/SceneInstanceAttributesManager.h | 35 ++++++++-------
 .../managers/SemanticAttributesManager.cpp    |  8 ++--
 .../managers/SemanticAttributesManager.h      | 35 ++++++++-------
 .../managers/StageAttributesManager.cpp       |  2 +-
 .../managers/StageAttributesManager.h         | 35 ++++++++-------
 22 files changed, 238 insertions(+), 161 deletions(-)

diff --git a/src/esp/metadata/managers/AOAttributesManager.cpp b/src/esp/metadata/managers/AOAttributesManager.cpp
index 5b047ef636..e7dcb1c4e8 100644
--- a/src/esp/metadata/managers/AOAttributesManager.cpp
+++ b/src/esp/metadata/managers/AOAttributesManager.cpp
@@ -28,7 +28,7 @@ ArticulatedObjectAttributes::ptr AOAttributesManager::createObject(
   return attrs;
 }  // AOAttributesManager::createObject
 
-void AOAttributesManager::setValsFromJSONDoc(
+void AOAttributesManager::setValsFromJSONDocInternal(
     ArticulatedObjectAttributes::ptr aoAttr,
     const io::JsonGenericValue& jsonConfig) {
   std::string urdf_filepath = "";
diff --git a/src/esp/metadata/managers/AOAttributesManager.h b/src/esp/metadata/managers/AOAttributesManager.h
index b545718d75..8f17130437 100644
--- a/src/esp/metadata/managers/AOAttributesManager.h
+++ b/src/esp/metadata/managers/AOAttributesManager.h
@@ -55,15 +55,6 @@ class AOAttributesManager
       const std::string& aoConfigFilename,
       bool registerTemplate = true) override;
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::ArticulatedObjectAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief This is to be deprecated. Provide a map of the articulated object
    * model filenames (.urdf) that have been referenced in the Scene Dataset via
@@ -84,6 +75,16 @@ class AOAttributesManager
       const override;
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::ArticulatedObjectAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief Parse Marker_sets object in json, if present.
    * @param attribs (out) an existing attributes to be modified.
@@ -156,19 +157,21 @@ class AOAttributesManager
       CORRADE_UNUSED bool) override;
 
   /**
-   * @brief Not required for this manager.
-   *
-   * This method will perform any final manager-related handling after
-   * successfully registering an object.
+   * @brief This method will perform any final manager-related handling after
+   * successfully registering an object, specifically saving the attributes back
+   * to file if it was found to be necessary due to user-specified diagnostics.
    *
    * See @ref esp::attributes::managers::ObjectAttributesManager for an example.
    *
    * @param objectID the ID of the successfully registered managed object
    * @param objectHandle The name of the managed object
    */
-  void postRegisterObjectHandling(
-      CORRADE_UNUSED int objectID,
-      CORRADE_UNUSED const std::string& objectHandle) override {}
+  void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
+                                  const std::string& objectHandle) override {
+    if (this->_DSDiagnostics->saveRequired()) {
+      this->saveManagedObjectToFile(objectHandle, true);
+    }
+  }
 
   /**
    * @brief Any articulated-object-attributes-specific resetting that needs to
diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
index d868e196f6..8b2fc5ac8a 100644
--- a/src/esp/metadata/managers/AbstractAttributesManager.h
+++ b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -186,8 +186,13 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
    * @param attribs (out) an existing attributes to be modified.
    * @param jsonConfig json document to parse
    */
-  virtual void setValsFromJSONDoc(AttribsPtr attribs,
-                                  const io::JsonGenericValue& jsonConfig) = 0;
+
+  void setValsFromJSONDoc(AttribsPtr attribs,
+                          const io::JsonGenericValue& jsonConfig) {
+    // Clear diagnostic flags from previous run
+    this->_DSDiagnostics->clearDiagnostics();
+    this->setValsFromJSONDocInternal(attribs, jsonConfig);
+  }
 
   /**
    * @brief Configure @ref _DSDiagnostics tool based on if passsed jsonConfig
@@ -277,6 +282,16 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
       const AttribsPtr& attributes) const = 0;
 
  protected:
+  /**
+   * @brief Internally called only. Method to take an existing attributes and
+   * set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  virtual void setValsFromJSONDocInternal(
+      AttribsPtr attribs,
+      const io::JsonGenericValue& jsonConfig) = 0;
+
   /**
    * @brief Called internally right before attribute registration. Filepaths
    * in the json configs for Habitat SceneDatasets are specified relative to
diff --git a/src/esp/metadata/managers/AssetAttributesManager.cpp b/src/esp/metadata/managers/AssetAttributesManager.cpp
index 4ebfc7e3ed..1f78f55c38 100644
--- a/src/esp/metadata/managers/AssetAttributesManager.cpp
+++ b/src/esp/metadata/managers/AssetAttributesManager.cpp
@@ -249,7 +249,7 @@ AbstractPrimitiveAttributes::ptr AssetAttributesManager::buildObjectFromJSONDoc(
   return primAssetAttributes;
 }  // AssetAttributesManager::buildObjectFromJSONDoc
 
-void AssetAttributesManager::setValsFromJSONDoc(
+void AssetAttributesManager::setValsFromJSONDocInternal(
     CORRADE_UNUSED AttribsPtr attribs,
     CORRADE_UNUSED const io::JsonGenericValue& jsonConfig) {
   // TODO support loading values from JSON docs
diff --git a/src/esp/metadata/managers/AssetAttributesManager.h b/src/esp/metadata/managers/AssetAttributesManager.h
index 3274ab0f32..b8e0831f02 100644
--- a/src/esp/metadata/managers/AssetAttributesManager.h
+++ b/src/esp/metadata/managers/AssetAttributesManager.h
@@ -137,15 +137,6 @@ class AssetAttributesManager
       CORRADE_UNUSED const attributes::AbstractPrimitiveAttributes::ptr&
           attributes) const override {}
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(AttribsPtr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief Creates a template based on the provided template handle. Since the
    * primitive asset attributes templates encode their structure in their
@@ -455,6 +446,16 @@ class AssetAttributesManager
   }
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      AttribsPtr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief This method will perform any necessary updating that is
    * AbstractAttributesManager-specific upon template removal, such as removing
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
index 724f3ba0fb..c93c669fb0 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -122,20 +122,46 @@ class DatasetDiagnosticsTool {
                           bool abortOnFail = false);
 
   /**
-   * @brief Specify whether or not to save any corrected dataset components if
-   * they received correction
+   * @brief Specify whether or not we should save any corrected dataset
+   * components if they received correction
    */
-  void setSaveCorrected(bool val) {
+  void setShouldSaveCorrected(bool val) {
     setFlags(DSDiagnosticType::SaveCorrected, val);
   }
   /**
-   * @brief Query whether or not to save any corrected dataset components if
-   * they received correction
+   * @brief Query whether or not we should save any corrected dataset components
+   * if they received correction
    */
-  bool saveCorrected() const {
+  bool shouldSaveCorrected() const {
     return getFlags(DSDiagnosticType::SaveCorrected);
   }
 
+  /**
+   * @brief Set that a save is required. This value should be reset to false
+   * after it is accessed one time.  This is to bridge from reading the json
+   * file into the attributes and registering the attributes to the
+   * post-registration code.
+   */
+  void setSaveRequired(bool saveRequired) {
+    _requiresCorrectedSave = saveRequired;
+  }
+  /**
+   * @brief Get whether a save is required. This value should be reset to false
+   * after it is accessed one time. This is to bridge from reading the json
+   * file into the attributes and registering the attributes to the
+   * post-registration code.
+   */
+  bool saveRequired() {
+    bool retVal = _requiresCorrectedSave;
+    clearDiagnostics();
+    return retVal;
+  }
+
+  /**
+   * @brief Clear any flags set due to specific diagnostics
+   */
+  void clearDiagnostics() { _requiresCorrectedSave = false; }
+
   /**
    * @brief Specify whether or not to test for duplicate scene object instances
    * in loaded @ref SceneInstanceAttributes and not process the duplicates if found.
@@ -182,6 +208,11 @@ class DatasetDiagnosticsTool {
   }
   uint32_t _diagnosticsFlags = 0u;
 
+  /**
+   * @brief Save the attributes current being processed. This flag is only so
+   */
+  bool _requiresCorrectedSave = false;
+
  public:
   ESP_SMART_POINTERS(DatasetDiagnosticsTool)
 
diff --git a/src/esp/metadata/managers/LightLayoutAttributesManager.cpp b/src/esp/metadata/managers/LightLayoutAttributesManager.cpp
index cbf6d9d9fe..85a484d0cd 100644
--- a/src/esp/metadata/managers/LightLayoutAttributesManager.cpp
+++ b/src/esp/metadata/managers/LightLayoutAttributesManager.cpp
@@ -42,7 +42,7 @@ LightLayoutAttributes::ptr LightLayoutAttributesManager::createObject(
   return attrs;
 }  // PhysicsAttributesManager::createObject
 
-void LightLayoutAttributesManager::setValsFromJSONDoc(
+void LightLayoutAttributesManager::setValsFromJSONDocInternal(
     attributes::LightLayoutAttributes::ptr lightAttribs,
     const io::JsonGenericValue& jsonConfig) {
   const std::string layoutNameAndPath = lightAttribs->getHandle();
diff --git a/src/esp/metadata/managers/LightLayoutAttributesManager.h b/src/esp/metadata/managers/LightLayoutAttributesManager.h
index ef3ca652ff..7917825f3e 100644
--- a/src/esp/metadata/managers/LightLayoutAttributesManager.h
+++ b/src/esp/metadata/managers/LightLayoutAttributesManager.h
@@ -52,15 +52,6 @@ class LightLayoutAttributesManager
       const std::string& lightConfigName,
       bool registerTemplate = false) override;
 
-  /**
-   * @brief Function to take an existing LightLayoutAttributes and set its
-   * values from passed json config file.
-   * @param lightAttribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::LightLayoutAttributes::ptr lightAttribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief Function to take an existing LightInstanceAttributes and set its
    * values from passed json config file
@@ -95,6 +86,16 @@ class LightLayoutAttributesManager
       const override {}
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing LightLayoutAttributes and set its values from passed json
+   * config file.
+   * @param lightAttribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::LightLayoutAttributes::ptr lightAttribs,
+      const io::JsonGenericValue& jsonConfig) override;
   /**
    * @brief Used Internally.  Create and configure newly-created attributes
    * with any default values, before any specific values are set.
@@ -147,19 +148,21 @@ class LightLayoutAttributesManager
   }
 
   /**
-   * @brief Not required for this manager.
-   *
-   * This method will perform any final manager-related handling after
-   * successfully registering an object.
+   * @brief This method will perform any final manager-related handling after
+   * successfully registering an object, specifically saving the attributes back
+   * to file if it was found to be necessary due to user-specified diagnostics.
    *
    * See @ref esp::attributes::managers::ObjectAttributesManager for an example.
    *
    * @param objectID the ID of the successfully registered managed object
    * @param objectHandle The name of the managed object
    */
-  void postRegisterObjectHandling(
-      CORRADE_UNUSED int objectID,
-      CORRADE_UNUSED const std::string& objectHandle) override {}
+  void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
+                                  const std::string& objectHandle) override {
+    if (this->_DSDiagnostics->saveRequired()) {
+      this->saveManagedObjectToFile(objectHandle, true);
+    }
+  }
 
   /**
    * @brief Any lights-attributes-specific resetting that needs to happen on
diff --git a/src/esp/metadata/managers/ObjectAttributesManager.cpp b/src/esp/metadata/managers/ObjectAttributesManager.cpp
index fc374553bd..e925da3ddd 100644
--- a/src/esp/metadata/managers/ObjectAttributesManager.cpp
+++ b/src/esp/metadata/managers/ObjectAttributesManager.cpp
@@ -71,7 +71,7 @@ void ObjectAttributesManager::createDefaultPrimBasedAttributesTemplates() {
   }
 }  // ObjectAttributesManager::createDefaultPrimBasedAttributesTemplates
 
-void ObjectAttributesManager::setValsFromJSONDoc(
+void ObjectAttributesManager::setValsFromJSONDocInternal(
     attributes::ObjectAttributes::ptr objAttributes,
     const io::JsonGenericValue& jsonConfig) {
   this->setAbstractObjectAttributesFromJson(objAttributes, jsonConfig);
@@ -362,6 +362,9 @@ void ObjectAttributesManager::postRegisterObjectHandling(
   if (mapToAddTo_ != nullptr) {
     mapToAddTo_->emplace(objectTemplateID, objectTemplateHandle);
   }
+  if (this->_DSDiagnostics->saveRequired()) {
+    this->saveManagedObjectToFile(objectTemplateHandle, true);
+  }
 }  // ObjectAttributesManager::postRegisterObjectHandling
 
 }  // namespace managers
diff --git a/src/esp/metadata/managers/ObjectAttributesManager.h b/src/esp/metadata/managers/ObjectAttributesManager.h
index de49f52ee7..a857b63ce3 100644
--- a/src/esp/metadata/managers/ObjectAttributesManager.h
+++ b/src/esp/metadata/managers/ObjectAttributesManager.h
@@ -50,15 +50,6 @@ class ObjectAttributesManager
       const std::string& primAttrTemplateHandle,
       bool registerTemplate = true) override;
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::ObjectAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   // ======== File-based and primitive-based partition functions ========
 
   /**
@@ -158,6 +149,16 @@ class ObjectAttributesManager
   // ======== End File-based and primitive-based partition functions ========
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::ObjectAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief Create and save default primitive asset-based object templates,
    * saving their handles as non-deletable default handles.
diff --git a/src/esp/metadata/managers/PbrShaderAttributesManager.cpp b/src/esp/metadata/managers/PbrShaderAttributesManager.cpp
index 6da1e01a19..ee0b9c70c7 100644
--- a/src/esp/metadata/managers/PbrShaderAttributesManager.cpp
+++ b/src/esp/metadata/managers/PbrShaderAttributesManager.cpp
@@ -29,7 +29,7 @@ PbrShaderAttributes::ptr PbrShaderAttributesManager::createObject(
   return attrs;
 }  // PbrShaderAttributesManager::createObject
 
-void PbrShaderAttributesManager::setValsFromJSONDoc(
+void PbrShaderAttributesManager::setValsFromJSONDocInternal(
     PbrShaderAttributes::ptr pbrShaderAttribs,
     const io::JsonGenericValue& jsonConfig) {
   ////////////////////////////
diff --git a/src/esp/metadata/managers/PbrShaderAttributesManager.h b/src/esp/metadata/managers/PbrShaderAttributesManager.h
index 2512bc8e2b..0c6f710cf1 100644
--- a/src/esp/metadata/managers/PbrShaderAttributesManager.h
+++ b/src/esp/metadata/managers/PbrShaderAttributesManager.h
@@ -56,15 +56,6 @@ class PbrShaderAttributesManager
           ESP_DEFAULT_PBRSHADER_CONFIG_REL_PATH,
       bool registerTemplate = true) override;
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::PbrShaderAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief This will set all the @ref metadata::attributes::PbrShaderAttributes
    * to have IBL either on or off.
@@ -115,6 +106,16 @@ class PbrShaderAttributesManager
           pbrShaderAttribs) const override;
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::PbrShaderAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief Used Internally.  Create and configure newly-created attributes with
    * any default values, before any specific values are set.
@@ -163,19 +164,21 @@ class PbrShaderAttributesManager
       CORRADE_UNUSED bool forceRegistration) override;
 
   /**
-   * @brief Not required for this manager.
-   *
-   * This method will perform any final manager-related handling after
-   * successfully registering an object.
+   * @brief This method will perform any final manager-related handling after
+   * successfully registering an object, specifically saving the attributes back
+   * to file if it was found to be necessary due to user-specified diagnostics.
    *
    * See @ref esp::attributes::managers::ObjectAttributesManager for an example.
    *
    * @param objectID the ID of the successfully registered managed object
    * @param objectHandle The name of the managed object
    */
-  void postRegisterObjectHandling(
-      CORRADE_UNUSED int objectID,
-      CORRADE_UNUSED const std::string& objectHandle) override {}
+  void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
+                                  const std::string& objectHandle) override {
+    if (this->_DSDiagnostics->saveRequired()) {
+      this->saveManagedObjectToFile(objectHandle, true);
+    }
+  }
 
   /**
    * @brief Any physics-attributes-specific resetting that needs to happen on
diff --git a/src/esp/metadata/managers/PhysicsAttributesManager.cpp b/src/esp/metadata/managers/PhysicsAttributesManager.cpp
index 306c1ad979..3eef6de906 100644
--- a/src/esp/metadata/managers/PhysicsAttributesManager.cpp
+++ b/src/esp/metadata/managers/PhysicsAttributesManager.cpp
@@ -30,7 +30,7 @@ PhysicsManagerAttributes::ptr PhysicsAttributesManager::createObject(
   return attrs;
 }  // PhysicsAttributesManager::createObject
 
-void PhysicsAttributesManager::setValsFromJSONDoc(
+void PhysicsAttributesManager::setValsFromJSONDocInternal(
     PhysicsManagerAttributes::ptr physicsManagerAttributes,
     const io::JsonGenericValue& jsonConfig) {
   // load the simulator preference - default is "none"
diff --git a/src/esp/metadata/managers/PhysicsAttributesManager.h b/src/esp/metadata/managers/PhysicsAttributesManager.h
index e418fcee84..a4c1377aad 100644
--- a/src/esp/metadata/managers/PhysicsAttributesManager.h
+++ b/src/esp/metadata/managers/PhysicsAttributesManager.h
@@ -55,15 +55,6 @@ class PhysicsAttributesManager
       const std::string& physicsFilename = ESP_DEFAULT_PHYSICS_CONFIG_REL_PATH,
       bool registerTemplate = true) override;
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::PhysicsManagerAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief This function will be called to finalize attributes' paths before
    * registration, moving fully qualified paths to the appropriate hidden
@@ -76,6 +67,16 @@ class PhysicsAttributesManager
           attributes) const override {}
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::PhysicsManagerAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief Used Internally.  Create and configure newly-created attributes with
    * any default values, before any specific values are set.
@@ -137,19 +138,21 @@ class PhysicsAttributesManager
   }
 
   /**
-   * @brief Not required for this manager.
-   *
-   * This method will perform any final manager-related handling after
-   * successfully registering an object.
+   * @brief This method will perform any final manager-related handling after
+   * successfully registering an object, specifically saving the attributes back
+   * to file if it was found to be necessary due to user-specified diagnostics.
    *
    * See @ref esp::attributes::managers::ObjectAttributesManager for an example.
    *
    * @param objectID the ID of the successfully registered managed object
    * @param objectHandle The name of the managed object
    */
-  void postRegisterObjectHandling(
-      CORRADE_UNUSED int objectID,
-      CORRADE_UNUSED const std::string& objectHandle) override {}
+  void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
+                                  const std::string& objectHandle) override {
+    if (this->_DSDiagnostics->saveRequired()) {
+      this->saveManagedObjectToFile(objectHandle, true);
+    }
+  }
 
   /**
    * @brief Any physics-attributes-specific resetting that needs to happen on
diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
index 8b4aa9cec1..69b7f1f645 100644
--- a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
@@ -66,7 +66,7 @@ SceneDatasetAttributesManager::initNewObjectInternal(
   return newAttributes;
 }  // SceneDatasetAttributesManager::initNewObjectInternal
 
-void SceneDatasetAttributesManager::setValsFromJSONDoc(
+void SceneDatasetAttributesManager::setValsFromJSONDocInternal(
     attributes::SceneDatasetAttributes::ptr dsAttribs,
     const io::JsonGenericValue& jsonConfig) {
   // check for diagnostics requests
diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.h b/src/esp/metadata/managers/SceneDatasetAttributesManager.h
index a2a558bc0f..84e6e14c06 100644
--- a/src/esp/metadata/managers/SceneDatasetAttributesManager.h
+++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.h
@@ -43,15 +43,6 @@ class SceneDatasetAttributesManager
       const std::string& attributesTemplateHandle,
       bool registerTemplate = true) override;
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::SceneDatasetAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief This will set the current physics manager attributes that is
    * governing the world that this SceneDatasetAttributesManager's datasets will
@@ -109,6 +100,16 @@ class SceneDatasetAttributesManager
       const override {}
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::SceneDatasetAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief This will validate a loaded dataset map with file location values
    * from the dataset config, by attempting to either verify those
diff --git a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
index 7d976373d1..a6fa83825a 100644
--- a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
@@ -67,7 +67,7 @@ SceneInstanceAttributesManager::initNewObjectInternal(
   return newAttributes;
 }  // SceneInstanceAttributesManager::initNewObjectInternal
 
-void SceneInstanceAttributesManager::setValsFromJSONDoc(
+void SceneInstanceAttributesManager::setValsFromJSONDocInternal(
     SceneInstanceAttributes::ptr attribs,
     const io::JsonGenericValue& jsonConfig) {
   const std::string attribsDispName = attribs->getSimplifiedHandle();
@@ -120,7 +120,7 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc(
   // Only resave if instance attributes' attempt to be added reveals duplicate
   // attributes
   bool saveValidationResults =
-      validateUniqueness && this->_DSDiagnostics->saveCorrected();
+      validateUniqueness && this->_DSDiagnostics->shouldSaveCorrected();
 
   // Whether this scene instance should be resaved or not.
   bool resaveAttributes = false;
@@ -310,7 +310,9 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc(
   this->parseUserDefinedJsonVals(attribs, jsonConfig);
 
   // If we want to save corrected, and we need to due to corrections happening
-  if (saveValidationResults && resaveAttributes) {
+  bool saveRequired = (saveValidationResults && resaveAttributes);
+  this->_DSDiagnostics->setSaveRequired(saveRequired);
+  if (saveRequired) {
     // TODO Resave this attributes due to duplicate entries being filtered out
     // via diagnostics
   }
diff --git a/src/esp/metadata/managers/SceneInstanceAttributesManager.h b/src/esp/metadata/managers/SceneInstanceAttributesManager.h
index 1cf0776b1d..590ff913ce 100644
--- a/src/esp/metadata/managers/SceneInstanceAttributesManager.h
+++ b/src/esp/metadata/managers/SceneInstanceAttributesManager.h
@@ -56,15 +56,6 @@ class SceneInstanceAttributesManager
     defaultPbrShaderAttributesHandle_ = pbrHandle;
   }
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::SceneInstanceAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief This will return a @ref
    * attributes::SceneObjectInstanceAttributes object with passed handle.
@@ -112,6 +103,16 @@ class SceneInstanceAttributesManager
       const override {}
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::SceneInstanceAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief Gets the name/key value of the appropriate enum corresponding to the
    * desired Translation Origin used to determine the location of the asset in
@@ -219,19 +220,21 @@ class SceneInstanceAttributesManager
   }
 
   /**
-   * @brief Not required for this manager.
-   *
-   * This method will perform any final manager-related handling after
-   * successfully registering an object.
+   * @brief This method will perform any final manager-related handling after
+   * successfully registering an object, specifically saving the attributes back
+   * to file if it was found to be necessary due to user-specified diagnostics.
    *
    * See @ref esp::attributes::managers::ObjectAttributesManager for an example.
    *
    * @param objectID the ID of the successfully registered managed object
    * @param objectHandle The name of the managed object
    */
-  void postRegisterObjectHandling(
-      CORRADE_UNUSED int objectID,
-      CORRADE_UNUSED const std::string& objectHandle) override {}
+  void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
+                                  const std::string& objectHandle) override {
+    if (this->_DSDiagnostics->saveRequired()) {
+      this->saveManagedObjectToFile(objectHandle, true);
+    }
+  }
 
   /**
    * @brief Name of the attributes used for the default Pbr/Ibl shader
diff --git a/src/esp/metadata/managers/SemanticAttributesManager.cpp b/src/esp/metadata/managers/SemanticAttributesManager.cpp
index 6e67b3d526..a11a617906 100644
--- a/src/esp/metadata/managers/SemanticAttributesManager.cpp
+++ b/src/esp/metadata/managers/SemanticAttributesManager.cpp
@@ -136,7 +136,7 @@ SemanticAttributesManager::createRegionAttributesFromJSON(
   return regionAttrs;
 }  // SemanticAttributesManager::createRegionAttributesFromJSON
 
-void SemanticAttributesManager::setValsFromJSONDoc(
+void SemanticAttributesManager::setValsFromJSONDocInternal(
     SemanticAttributes::ptr semanticAttribs,
     const io::JsonGenericValue& jsonConfig) {
   const std::string attribsDispName = semanticAttribs->getSimplifiedHandle();
@@ -209,7 +209,7 @@ void SemanticAttributesManager::setValsFromJSONDoc(
   // Only resave if instance attributes' attempt to be added reveals duplicate
   // attributes
   bool saveValidationResults =
-      validateUniqueness && this->_DSDiagnostics->saveCorrected();
+      validateUniqueness && this->_DSDiagnostics->shouldSaveCorrected();
 
   // Whether this scene instance should be resaved or not.
   bool resaveAttributes = false;
@@ -254,7 +254,9 @@ void SemanticAttributesManager::setValsFromJSONDoc(
   this->parseUserDefinedJsonVals(semanticAttribs, jsonConfig);
 
   // If we want to save corrected, and we need to due to corrections happening
-  if (saveValidationResults && resaveAttributes) {
+  bool saveRequired = (saveValidationResults && resaveAttributes);
+  this->_DSDiagnostics->setSaveRequired(saveRequired);
+  if (saveRequired) {
     // TODO Resave this attributes due to duplicate entries being filtered out
     // via diagnostics
   }
diff --git a/src/esp/metadata/managers/SemanticAttributesManager.h b/src/esp/metadata/managers/SemanticAttributesManager.h
index de649f69be..dc431157c5 100644
--- a/src/esp/metadata/managers/SemanticAttributesManager.h
+++ b/src/esp/metadata/managers/SemanticAttributesManager.h
@@ -53,15 +53,6 @@ class SemanticAttributesManager
       const std::string& semanticConfigFilename,
       bool registerTemplate = true) override;
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::SemanticAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief This will return a @ref
    * attributes::SemanticVolumeAttributes object with passed handle.
@@ -82,6 +73,16 @@ class SemanticAttributesManager
       const attributes::SemanticAttributes::ptr& attributes) const override;
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::SemanticAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief Used Internally.  Create a @ref
    * esp::metadata::attributes::SemanticVolumeAttributes object from the
@@ -152,19 +153,21 @@ class SemanticAttributesManager
                             CORRADE_UNUSED bool forceRegistration) override;
 
   /**
-   * @brief Not required for this manager.
-   *
-   * This method will perform any final manager-related handling after
-   * successfully registering an object.
+   * @brief This method will perform any final manager-related handling after
+   * successfully registering an object, specifically saving the attributes back
+   * to file if it was found to be necessary due to user-specified diagnostics.
    *
    * See @ref esp::attributes::managers::ObjectAttributesManager for an example.
    *
    * @param objectID the ID of the successfully registered managed object
    * @param objectHandle The name of the managed object
    */
-  void postRegisterObjectHandling(
-      CORRADE_UNUSED int objectID,
-      CORRADE_UNUSED const std::string& objectHandle) override {}
+  void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
+                                  const std::string& objectHandle) override {
+    if (this->_DSDiagnostics->saveRequired()) {
+      this->saveManagedObjectToFile(objectHandle, true);
+    }
+  }
 
   /**
    * @brief Any physics-attributes-specific resetting that needs to happen on
diff --git a/src/esp/metadata/managers/StageAttributesManager.cpp b/src/esp/metadata/managers/StageAttributesManager.cpp
index 5b611ffa2a..3139aceb4b 100644
--- a/src/esp/metadata/managers/StageAttributesManager.cpp
+++ b/src/esp/metadata/managers/StageAttributesManager.cpp
@@ -276,7 +276,7 @@ void StageAttributesManager::setDefaultAssetNameBasedAttributes(
   }
 }  // StageAttributesManager::setDefaultAssetNameBasedAttributes
 
-void StageAttributesManager::setValsFromJSONDoc(
+void StageAttributesManager::setValsFromJSONDocInternal(
     attributes::StageAttributes::ptr stageAttributes,
     const io::JsonGenericValue& jsonConfig) {
   this->setAbstractObjectAttributesFromJson(stageAttributes, jsonConfig);
diff --git a/src/esp/metadata/managers/StageAttributesManager.h b/src/esp/metadata/managers/StageAttributesManager.h
index 032961e124..ee67cab05c 100644
--- a/src/esp/metadata/managers/StageAttributesManager.h
+++ b/src/esp/metadata/managers/StageAttributesManager.h
@@ -70,15 +70,6 @@ class StageAttributesManager
       const std::string& primAttrTemplateHandle,
       bool registerTemplate = true) override;
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::StageAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief This function will be called to finalize attributes' paths before
    * registration, moving fully qualified paths to the appropriate hidden
@@ -90,6 +81,16 @@ class StageAttributesManager
       const attributes::StageAttributes::ptr& attributes) const override;
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::StageAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief Create and save default primitive asset-based object templates,
    * saving their handles as non-deletable default handles.
@@ -164,19 +165,21 @@ class StageAttributesManager
       bool forceRegistration) override;
 
   /**
-   * @brief Not required for this manager.
-   *
-   * This method will perform any final manager-related handling after
-   * successfully registering an object.
+   * @brief This method will perform any final manager-related handling after
+   * successfully registering an object, specifically saving the attributes back
+   * to file if it was found to be necessary due to user-specified diagnostics.
    *
    * See @ref esp::attributes::managers::ObjectAttributesManager for an example.
    *
    * @param objectID the ID of the successfully registered managed object
    * @param objectHandle The name of the managed object
    */
-  void postRegisterObjectHandling(
-      CORRADE_UNUSED int objectID,
-      CORRADE_UNUSED const std::string& objectHandle) override {}
+  void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
+                                  const std::string& objectHandle) override {
+    if (this->_DSDiagnostics->saveRequired()) {
+      this->saveManagedObjectToFile(objectHandle, true);
+    }
+  }
 
   /**
    * @brief Any scene-attributes-specific resetting that needs to happen on

From e249028dcee1ece4fd9b365cb8549ae1a150973a Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 16:59:19 -0400
Subject: [PATCH 09/20] --message showing that modified attributes should be
 saved.

More extensive reporting can eventually be implemented.
---
 .../managers/AbstractAttributesManager.h         | 15 ++++++++++++---
 .../metadata/managers/DatasetDiagnosticsTool.cpp |  4 ++--
 .../metadata/managers/DatasetDiagnosticsTool.h   | 16 +++++-----------
 .../managers/SceneInstanceAttributesManager.cpp  |  9 ++-------
 .../managers/SemanticAttributesManager.cpp       |  8 ++------
 5 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
index 8b2fc5ac8a..86e6d9fc95 100644
--- a/src/esp/metadata/managers/AbstractAttributesManager.h
+++ b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -182,7 +182,8 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
 
   /**
    * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
+   * json config file. The JSON functionality is encapsulated with diagnostic
+   * preparation and reporting.
    * @param attribs (out) an existing attributes to be modified.
    * @param jsonConfig json document to parse
    */
@@ -192,7 +193,15 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
     // Clear diagnostic flags from previous run
     this->_DSDiagnostics->clearDiagnostics();
     this->setValsFromJSONDocInternal(attribs, jsonConfig);
-  }
+    if (this->_DSDiagnostics->saveRequired()) {
+      ESP_WARNING(Mn::Debug::Flag::NoSpace)
+          << "<" << this->objectType_
+          << "> : Diagnostics exposed issues in loaded attributes : `"
+          << attribs->getSimplifiedHandle()
+          << "` and the user has requested that the corrected attributes will "
+             "be saved.";
+    }
+  }  // setValsFromJSONDoc
 
   /**
    * @brief Configure @ref _DSDiagnostics tool based on if passsed jsonConfig
@@ -205,7 +214,7 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
     if (jsonIter == jsonConfig.MemberEnd()) {
       return false;
     }
-    return this->_DSDiagnostics->setDiagnosticesFromJson(
+    return this->_DSDiagnostics->setDiagnosticesFromJSON(
         jsonIter->value, Cr::Utility::formatString(
                              "(setDSDiagnostics) <{}> : {}", this->objectType_,
                              attribs->getSimplifiedHandle()));
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp b/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
index 2e5f610aab..e606fa3088 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
@@ -21,7 +21,7 @@ const std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap = {
     {"allsavecorrected", DSDiagnosticType::AllDiagnosticsSaveCorrected},
 };
 
-bool DatasetDiagnosticsTool::setDiagnosticesFromJson(
+bool DatasetDiagnosticsTool::setDiagnosticesFromJSON(
     const io::JsonGenericValue& jsonObj,
     const std::string& msgStr) {
   if (jsonObj.IsString()) {
@@ -50,7 +50,7 @@ bool DatasetDiagnosticsTool::setDiagnosticesFromJson(
       << " configuration specifies `request_diagnostics` but specification "
          "is unable to be parsed, so diagnostics request is ignored.";
   return false;
-}  // DatasetDiagnosticsTool::setDiagnosticesFromJson
+}  // DatasetDiagnosticsTool::setDiagnosticesFromJSON
 
 bool DatasetDiagnosticsTool::setNamedDiagnostic(const std::string& diagnostic,
                                                 bool val,
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
index c93c669fb0..ef577fc23b 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -78,7 +78,7 @@ class DatasetDiagnosticsTool {
    * attributes/config responsible for this call, for use in debug messages.
    * @return Whether the diagnostic values were set successfully or not.
    */
-  bool setDiagnosticesFromJson(const io::JsonGenericValue& jsonObj,
+  bool setDiagnosticesFromJSON(const io::JsonGenericValue& jsonObj,
                                const std::string& msgStr);
 
   /**
@@ -137,8 +137,7 @@ class DatasetDiagnosticsTool {
   }
 
   /**
-   * @brief Set that a save is required. This value should be reset to false
-   * after it is accessed one time.  This is to bridge from reading the json
+   * @brief Set that a save is required. This is to bridge from reading the json
    * file into the attributes and registering the attributes to the
    * post-registration code.
    */
@@ -146,16 +145,11 @@ class DatasetDiagnosticsTool {
     _requiresCorrectedSave = saveRequired;
   }
   /**
-   * @brief Get whether a save is required. This value should be reset to false
-   * after it is accessed one time. This is to bridge from reading the json
-   * file into the attributes and registering the attributes to the
+   * @brief Get whether a save is required. This is to bridge from reading the
+   * json file into the attributes and registering the attributes to the
    * post-registration code.
    */
-  bool saveRequired() {
-    bool retVal = _requiresCorrectedSave;
-    clearDiagnostics();
-    return retVal;
-  }
+  bool saveRequired() { return _requiresCorrectedSave; }
 
   /**
    * @brief Clear any flags set due to specific diagnostics
diff --git a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
index a6fa83825a..0778369227 100644
--- a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
@@ -310,13 +310,8 @@ void SceneInstanceAttributesManager::setValsFromJSONDocInternal(
   this->parseUserDefinedJsonVals(attribs, jsonConfig);
 
   // If we want to save corrected, and we need to due to corrections happening
-  bool saveRequired = (saveValidationResults && resaveAttributes);
-  this->_DSDiagnostics->setSaveRequired(saveRequired);
-  if (saveRequired) {
-    // TODO Resave this attributes due to duplicate entries being filtered out
-    // via diagnostics
-  }
-
+  this->_DSDiagnostics->setSaveRequired(saveValidationResults &&
+                                        resaveAttributes);
 }  // SceneInstanceAttributesManager::setValsFromJSONDoc
 
 SceneObjectInstanceAttributes::ptr
diff --git a/src/esp/metadata/managers/SemanticAttributesManager.cpp b/src/esp/metadata/managers/SemanticAttributesManager.cpp
index a11a617906..a528edc722 100644
--- a/src/esp/metadata/managers/SemanticAttributesManager.cpp
+++ b/src/esp/metadata/managers/SemanticAttributesManager.cpp
@@ -254,12 +254,8 @@ void SemanticAttributesManager::setValsFromJSONDocInternal(
   this->parseUserDefinedJsonVals(semanticAttribs, jsonConfig);
 
   // If we want to save corrected, and we need to due to corrections happening
-  bool saveRequired = (saveValidationResults && resaveAttributes);
-  this->_DSDiagnostics->setSaveRequired(saveRequired);
-  if (saveRequired) {
-    // TODO Resave this attributes due to duplicate entries being filtered out
-    // via diagnostics
-  }
+  this->_DSDiagnostics->setSaveRequired(saveValidationResults &&
+                                        resaveAttributes);
 
 }  // SemanticAttributesManager::setValsFromJSONDoc
 

From 190c0a29b64635b70eb378b1176ffeaaa0f8bd9c Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 23 Aug 2024 17:20:24 -0400
Subject: [PATCH 10/20] --oops. Missing const

---
 src/esp/metadata/managers/DatasetDiagnosticsTool.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
index ef577fc23b..9491c37913 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -44,11 +44,13 @@ enum class DSDiagnosticType : uint32_t {
   TestForDuplicateRegions = (1U << 2),
 
   /**
-   * @brief Perform all diagnostics but do not save corrected results.
+   * @brief Shortcut to perform all diagnostics but do not save corrected
+   * results.
    */
   AllDiagnostics = ~SaveCorrected,
+
   /**
-   * @brief Perform all diagnostics and save corrected results
+   * @brief Shortcut to perform all diagnostics and save corrected results.
    */
   AllDiagnosticsSaveCorrected = ~0U
 };
@@ -82,7 +84,8 @@ class DatasetDiagnosticsTool {
                                const std::string& msgStr);
 
   /**
-   * @brief Merge the passed @ref DatasetDiagnosticsTool's flag settings into this one's.
+   * @brief Merge the passed @ref DatasetDiagnosticsTool's @p _diagnosticsFlag
+   * settings into this one's, preserving this one's state.
    */
   void mergeDiagnosticsTool(const DatasetDiagnosticsTool& tool) {
     _diagnosticsFlags |= tool._diagnosticsFlags;
@@ -149,7 +152,7 @@ class DatasetDiagnosticsTool {
    * json file into the attributes and registering the attributes to the
    * post-registration code.
    */
-  bool saveRequired() { return _requiresCorrectedSave; }
+  bool saveRequired() const { return _requiresCorrectedSave; }
 
   /**
    * @brief Clear any flags set due to specific diagnostics

From 6f61584c12025bb555c94dde3b1293d84b58ef3e Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Mon, 26 Aug 2024 09:38:44 -0400
Subject: [PATCH 11/20] --add diagnostic record, w/weak ref to target
 attributes.

---
 .../managers/AbstractAttributesManager.h      |  2 +-
 .../managers/DatasetDiagnosticsTool.h         | 78 +++++++++++++++++--
 2 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
index 86e6d9fc95..b7ae327832 100644
--- a/src/esp/metadata/managers/AbstractAttributesManager.h
+++ b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -191,7 +191,7 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
   void setValsFromJSONDoc(AttribsPtr attribs,
                           const io::JsonGenericValue& jsonConfig) {
     // Clear diagnostic flags from previous run
-    this->_DSDiagnostics->clearDiagnostics();
+    this->_DSDiagnostics->clearSaveRequired();
     this->setValsFromJSONDocInternal(attribs, jsonConfig);
     if (this->_DSDiagnostics->saveRequired()) {
       ESP_WARNING(Mn::Debug::Flag::NoSpace)
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
index 9491c37913..ad87452f45 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -8,6 +8,7 @@
 
 #include "esp/core/Esp.h"
 #include "esp/io/Json.h"
+#include "esp/metadata/attributes/AbstractAttributes.h"
 
 namespace esp {
 namespace metadata {
@@ -55,6 +56,69 @@ enum class DSDiagnosticType : uint32_t {
   AllDiagnosticsSaveCorrected = ~0U
 };
 
+/**
+ * @brief Construct to record the results of a series of diagnostics against a
+ * single attributes.
+ */
+template <class T>
+class DSDiagnosticRecord {
+ public:
+  static_assert(
+      std::is_base_of<esp::metadata::attributes::AbstractAttributes, T>::value,
+      "AbstractManagedPhysicsObject :: Managed physics object type must be "
+      "derived from esp::physics::PhysicsObjectBase");
+
+  typedef std::weak_ptr<T> WeakObjRef;
+
+  DSDiagnosticRecord(uint32_t diagnosticsFlags)
+      : _diagnosticsFlags(diagnosticsFlags) {}
+
+  /**
+   * @brief set the reference to this diagnostic record's subject
+   */
+  void setObjectRef(const std::shared_ptr<T>& objRef) { weakObjRef_ = objRef; }
+
+  inline void setFlags(DSDiagnosticType _flag, bool _val) {
+    if (_val) {
+      _diagnosticsFlags |= static_cast<uint32_t>(_flag);
+    } else {
+      _diagnosticsFlags &= ~static_cast<uint32_t>(_flag);
+    }
+  }
+
+  inline bool getFlags(DSDiagnosticType _flag) const {
+    return (_diagnosticsFlags & static_cast<uint32_t>(_flag)) ==
+           static_cast<uint32_t>(_flag);
+  }
+
+ protected:
+  /**
+   * @brief This function accesses the underlying shared pointer of this
+   * object's @p weakObjRef_ if it exists; if not, it provides a message.
+   * @return Either a shared pointer of this record's object, or nullptr if
+   * dne.
+   */
+  std::shared_ptr<T> inline getObjectReference() const {
+    std::shared_ptr<T> sp = weakObjRef_.lock();
+    if (!sp) {
+      ESP_ERROR()
+          << "This attributes no longer exists. Please delete any variable "
+             "references.";
+    }
+    return sp;
+  }  // getObjectReference
+
+  // Non-owning reference to attributes this record pertains to.
+  WeakObjRef weakObjRef_;
+
+ private:
+  uint32_t _diagnosticsFlags = 0u;
+
+ public:
+  ESP_SMART_POINTERS(DSDiagnosticRecord)
+
+};  // struct DSDiagnosticRecord
+
 /**
  * @brief Constant map to provide mappings from string tags to @ref
  * DSDiagnosticType values. This will be used to match values set
@@ -85,7 +149,8 @@ class DatasetDiagnosticsTool {
 
   /**
    * @brief Merge the passed @ref DatasetDiagnosticsTool's @p _diagnosticsFlag
-   * settings into this one's, preserving this one's state.
+   * settings into this one's, preserving this one's diagnostic requests as
+   * well.
    */
   void mergeDiagnosticsTool(const DatasetDiagnosticsTool& tool) {
     _diagnosticsFlags |= tool._diagnosticsFlags;
@@ -147,6 +212,12 @@ class DatasetDiagnosticsTool {
   void setSaveRequired(bool saveRequired) {
     _requiresCorrectedSave = saveRequired;
   }
+
+  /**
+   * @brief Clear any flags set due to specific diagnostics
+   */
+  void clearSaveRequired() { _requiresCorrectedSave = false; }
+
   /**
    * @brief Get whether a save is required. This is to bridge from reading the
    * json file into the attributes and registering the attributes to the
@@ -154,11 +225,6 @@ class DatasetDiagnosticsTool {
    */
   bool saveRequired() const { return _requiresCorrectedSave; }
 
-  /**
-   * @brief Clear any flags set due to specific diagnostics
-   */
-  void clearDiagnostics() { _requiresCorrectedSave = false; }
-
   /**
    * @brief Specify whether or not to test for duplicate scene object instances
    * in loaded @ref SceneInstanceAttributes and not process the duplicates if found.

From 37b14d65d8f74b04b2b8197b4d152e0daf169898 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Tue, 10 Sep 2024 09:37:43 -0400
Subject: [PATCH 12/20] --clang-tidy naming convention

---
 src/esp/metadata/managers/AOAttributesManager.h  |  2 +-
 .../managers/AbstractAttributesManager.h         | 16 ++++++++--------
 .../managers/LightLayoutAttributesManager.h      |  2 +-
 .../managers/ObjectAttributesManager.cpp         |  2 +-
 .../managers/PbrShaderAttributesManager.h        |  2 +-
 .../metadata/managers/PhysicsAttributesManager.h |  2 +-
 .../managers/SceneDatasetAttributesManager.cpp   |  2 +-
 .../managers/SceneInstanceAttributesManager.cpp  |  9 +++++----
 .../managers/SceneInstanceAttributesManager.h    |  2 +-
 .../managers/SemanticAttributesManager.cpp       |  8 ++++----
 .../managers/SemanticAttributesManager.h         |  2 +-
 .../metadata/managers/StageAttributesManager.h   |  2 +-
 12 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/src/esp/metadata/managers/AOAttributesManager.h b/src/esp/metadata/managers/AOAttributesManager.h
index 8f17130437..2326924010 100644
--- a/src/esp/metadata/managers/AOAttributesManager.h
+++ b/src/esp/metadata/managers/AOAttributesManager.h
@@ -168,7 +168,7 @@ class AOAttributesManager
    */
   void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
                                   const std::string& objectHandle) override {
-    if (this->_DSDiagnostics->saveRequired()) {
+    if (this->datasetDiagnostics_->saveRequired()) {
       this->saveManagedObjectToFile(objectHandle, true);
     }
   }
diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
index b7ae327832..2b518a2356 100644
--- a/src/esp/metadata/managers/AbstractAttributesManager.h
+++ b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -66,7 +66,7 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
       : ManagedFileBasedContainer<T, Access>::ManagedFileBasedContainer(
             attrType,
             JSONTypeExt),
-        _DSDiagnostics(DatasetDiagnosticsTool::create_unique()) {}
+        datasetDiagnostics_(DatasetDiagnosticsTool::create_unique()) {}
   ~AbstractAttributesManager() override = default;
 
   /**
@@ -191,9 +191,9 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
   void setValsFromJSONDoc(AttribsPtr attribs,
                           const io::JsonGenericValue& jsonConfig) {
     // Clear diagnostic flags from previous run
-    this->_DSDiagnostics->clearSaveRequired();
+    this->datasetDiagnostics_->clearSaveRequired();
     this->setValsFromJSONDocInternal(attribs, jsonConfig);
-    if (this->_DSDiagnostics->saveRequired()) {
+    if (this->datasetDiagnostics_->saveRequired()) {
       ESP_WARNING(Mn::Debug::Flag::NoSpace)
           << "<" << this->objectType_
           << "> : Diagnostics exposed issues in loaded attributes : `"
@@ -204,7 +204,7 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
   }  // setValsFromJSONDoc
 
   /**
-   * @brief Configure @ref _DSDiagnostics tool based on if passsed jsonConfig
+   * @brief Configure @ref datasetDiagnostics_ tool based on if passsed jsonConfig
    * requests them.
    */
   bool setDSDiagnostics(AttribsPtr attribs,
@@ -214,7 +214,7 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
     if (jsonIter == jsonConfig.MemberEnd()) {
       return false;
     }
-    return this->_DSDiagnostics->setDiagnosticesFromJSON(
+    return this->datasetDiagnostics_->setDiagnosticesFromJSON(
         jsonIter->value, Cr::Utility::formatString(
                              "(setDSDiagnostics) <{}> : {}", this->objectType_,
                              attribs->getSimplifiedHandle()));
@@ -222,11 +222,11 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
 
   /**
    * @brief Merge the passed DatasetDiagnosticsTool settings into
-   * @p _DSDiagnostics nondestructively (i.e. retaining this one's enabled
+   * @p datasetDiagnostics_ nondestructively (i.e. retaining this one's enabled
    * diagnostics)
    */
   void mergeDSDiagnosticsTool(const DatasetDiagnosticsTool& _tool) {
-    this->_DSDiagnostics->mergeDiagnosticsTool(_tool);
+    this->datasetDiagnostics_->mergeDiagnosticsTool(_tool);
   }
 
   /**
@@ -452,7 +452,7 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
    * @brief Diagnostics tool that governs whether certain diagnostic operations
    * should occur.
    */
-  DatasetDiagnosticsTool::uptr _DSDiagnostics;
+  DatasetDiagnosticsTool::uptr datasetDiagnostics_;
 
  public:
   ESP_SMART_POINTERS(AbstractAttributesManager<T, Access>)
diff --git a/src/esp/metadata/managers/LightLayoutAttributesManager.h b/src/esp/metadata/managers/LightLayoutAttributesManager.h
index 7917825f3e..2f070483fa 100644
--- a/src/esp/metadata/managers/LightLayoutAttributesManager.h
+++ b/src/esp/metadata/managers/LightLayoutAttributesManager.h
@@ -159,7 +159,7 @@ class LightLayoutAttributesManager
    */
   void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
                                   const std::string& objectHandle) override {
-    if (this->_DSDiagnostics->saveRequired()) {
+    if (this->datasetDiagnostics_->saveRequired()) {
       this->saveManagedObjectToFile(objectHandle, true);
     }
   }
diff --git a/src/esp/metadata/managers/ObjectAttributesManager.cpp b/src/esp/metadata/managers/ObjectAttributesManager.cpp
index e925da3ddd..700592e5d2 100644
--- a/src/esp/metadata/managers/ObjectAttributesManager.cpp
+++ b/src/esp/metadata/managers/ObjectAttributesManager.cpp
@@ -362,7 +362,7 @@ void ObjectAttributesManager::postRegisterObjectHandling(
   if (mapToAddTo_ != nullptr) {
     mapToAddTo_->emplace(objectTemplateID, objectTemplateHandle);
   }
-  if (this->_DSDiagnostics->saveRequired()) {
+  if (this->datasetDiagnostics_->saveRequired()) {
     this->saveManagedObjectToFile(objectTemplateHandle, true);
   }
 }  // ObjectAttributesManager::postRegisterObjectHandling
diff --git a/src/esp/metadata/managers/PbrShaderAttributesManager.h b/src/esp/metadata/managers/PbrShaderAttributesManager.h
index 0c6f710cf1..58403b30db 100644
--- a/src/esp/metadata/managers/PbrShaderAttributesManager.h
+++ b/src/esp/metadata/managers/PbrShaderAttributesManager.h
@@ -175,7 +175,7 @@ class PbrShaderAttributesManager
    */
   void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
                                   const std::string& objectHandle) override {
-    if (this->_DSDiagnostics->saveRequired()) {
+    if (this->datasetDiagnostics_->saveRequired()) {
       this->saveManagedObjectToFile(objectHandle, true);
     }
   }
diff --git a/src/esp/metadata/managers/PhysicsAttributesManager.h b/src/esp/metadata/managers/PhysicsAttributesManager.h
index a4c1377aad..336aa99d49 100644
--- a/src/esp/metadata/managers/PhysicsAttributesManager.h
+++ b/src/esp/metadata/managers/PhysicsAttributesManager.h
@@ -149,7 +149,7 @@ class PhysicsAttributesManager
    */
   void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
                                   const std::string& objectHandle) override {
-    if (this->_DSDiagnostics->saveRequired()) {
+    if (this->datasetDiagnostics_->saveRequired()) {
       this->saveManagedObjectToFile(objectHandle, true);
     }
   }
diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
index 69b7f1f645..af4ea43c05 100644
--- a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
@@ -142,7 +142,7 @@ void SceneDatasetAttributesManager::readDatasetJSONCell(
     const U& attrMgr,
     std::map<std::string, std::string>* strKeyMap) {
   // merge DatasetDiagnostics tool from this manager into passed manager
-  attrMgr->mergeDSDiagnosticsTool(*(this->_DSDiagnostics));
+  attrMgr->mergeDSDiagnosticsTool(*(this->datasetDiagnostics_));
 
   io::JsonGenericValue::ConstMemberIterator jsonIter =
       jsonConfig.FindMember(tag);
diff --git a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
index 0778369227..97bdb1c5cf 100644
--- a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp
@@ -115,12 +115,13 @@ void SceneInstanceAttributesManager::setValsFromJSONDocInternal(
   // Set this via configuration value - should only be called when explicitly
   // filtering dataset
   // Whether uniqueness validation should be performed
-  bool validateUniqueness = this->_DSDiagnostics->testDuplicateSceneInstances();
+  bool validateUniqueness =
+      this->datasetDiagnostics_->testDuplicateSceneInstances();
 
   // Only resave if instance attributes' attempt to be added reveals duplicate
   // attributes
   bool saveValidationResults =
-      validateUniqueness && this->_DSDiagnostics->shouldSaveCorrected();
+      validateUniqueness && this->datasetDiagnostics_->shouldSaveCorrected();
 
   // Whether this scene instance should be resaved or not.
   bool resaveAttributes = false;
@@ -310,8 +311,8 @@ void SceneInstanceAttributesManager::setValsFromJSONDocInternal(
   this->parseUserDefinedJsonVals(attribs, jsonConfig);
 
   // If we want to save corrected, and we need to due to corrections happening
-  this->_DSDiagnostics->setSaveRequired(saveValidationResults &&
-                                        resaveAttributes);
+  this->datasetDiagnostics_->setSaveRequired(saveValidationResults &&
+                                             resaveAttributes);
 }  // SceneInstanceAttributesManager::setValsFromJSONDoc
 
 SceneObjectInstanceAttributes::ptr
diff --git a/src/esp/metadata/managers/SceneInstanceAttributesManager.h b/src/esp/metadata/managers/SceneInstanceAttributesManager.h
index 590ff913ce..06a5f3535b 100644
--- a/src/esp/metadata/managers/SceneInstanceAttributesManager.h
+++ b/src/esp/metadata/managers/SceneInstanceAttributesManager.h
@@ -231,7 +231,7 @@ class SceneInstanceAttributesManager
    */
   void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
                                   const std::string& objectHandle) override {
-    if (this->_DSDiagnostics->saveRequired()) {
+    if (this->datasetDiagnostics_->saveRequired()) {
       this->saveManagedObjectToFile(objectHandle, true);
     }
   }
diff --git a/src/esp/metadata/managers/SemanticAttributesManager.cpp b/src/esp/metadata/managers/SemanticAttributesManager.cpp
index a528edc722..fab936eee0 100644
--- a/src/esp/metadata/managers/SemanticAttributesManager.cpp
+++ b/src/esp/metadata/managers/SemanticAttributesManager.cpp
@@ -204,12 +204,12 @@ void SemanticAttributesManager::setValsFromJSONDocInternal(
   // filtering dataset
   // Whether uniqueness validation should be performed
   bool validateUniqueness =
-      this->_DSDiagnostics->testDuplicateSemanticRegions();
+      this->datasetDiagnostics_->testDuplicateSemanticRegions();
 
   // Only resave if instance attributes' attempt to be added reveals duplicate
   // attributes
   bool saveValidationResults =
-      validateUniqueness && this->_DSDiagnostics->shouldSaveCorrected();
+      validateUniqueness && this->datasetDiagnostics_->shouldSaveCorrected();
 
   // Whether this scene instance should be resaved or not.
   bool resaveAttributes = false;
@@ -254,8 +254,8 @@ void SemanticAttributesManager::setValsFromJSONDocInternal(
   this->parseUserDefinedJsonVals(semanticAttribs, jsonConfig);
 
   // If we want to save corrected, and we need to due to corrections happening
-  this->_DSDiagnostics->setSaveRequired(saveValidationResults &&
-                                        resaveAttributes);
+  this->datasetDiagnostics_->setSaveRequired(saveValidationResults &&
+                                             resaveAttributes);
 
 }  // SemanticAttributesManager::setValsFromJSONDoc
 
diff --git a/src/esp/metadata/managers/SemanticAttributesManager.h b/src/esp/metadata/managers/SemanticAttributesManager.h
index dc431157c5..39d87e204a 100644
--- a/src/esp/metadata/managers/SemanticAttributesManager.h
+++ b/src/esp/metadata/managers/SemanticAttributesManager.h
@@ -164,7 +164,7 @@ class SemanticAttributesManager
    */
   void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
                                   const std::string& objectHandle) override {
-    if (this->_DSDiagnostics->saveRequired()) {
+    if (this->datasetDiagnostics_->saveRequired()) {
       this->saveManagedObjectToFile(objectHandle, true);
     }
   }
diff --git a/src/esp/metadata/managers/StageAttributesManager.h b/src/esp/metadata/managers/StageAttributesManager.h
index ee67cab05c..96fb4dbbd5 100644
--- a/src/esp/metadata/managers/StageAttributesManager.h
+++ b/src/esp/metadata/managers/StageAttributesManager.h
@@ -176,7 +176,7 @@ class StageAttributesManager
    */
   void postRegisterObjectHandling(CORRADE_UNUSED int objectID,
                                   const std::string& objectHandle) override {
-    if (this->_DSDiagnostics->saveRequired()) {
+    if (this->datasetDiagnostics_->saveRequired()) {
       this->saveManagedObjectToFile(objectHandle, true);
     }
   }

From d2d133527e58bee16c594d73347d92c13bf47a4e Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 13 Sep 2024 09:01:01 -0400
Subject: [PATCH 13/20] --single arg ctor

---
 src/esp/metadata/managers/DatasetDiagnosticsTool.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
index ad87452f45..5a6198e215 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/managers/DatasetDiagnosticsTool.h
@@ -70,7 +70,7 @@ class DSDiagnosticRecord {
 
   typedef std::weak_ptr<T> WeakObjRef;
 
-  DSDiagnosticRecord(uint32_t diagnosticsFlags)
+  explicit DSDiagnosticRecord(uint32_t diagnosticsFlags)
       : _diagnosticsFlags(diagnosticsFlags) {}
 
   /**

From 1ac1a0f1fa9d12598b7d6401013d9b8f01d9e65c Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 13 Sep 2024 11:06:08 -0400
Subject: [PATCH 14/20] --move to diagnostics namespace/directory

---
 .../DatasetDiagnosticsTool.cpp                       | 12 +++++-------
 .../DatasetDiagnosticsTool.h                         | 10 +++++-----
 .../metadata/managers/AbstractAttributesManager.h    |  3 ++-
 3 files changed, 12 insertions(+), 13 deletions(-)
 rename src/esp/metadata/{managers => diagnostics}/DatasetDiagnosticsTool.cpp (90%)
 rename src/esp/metadata/{managers => diagnostics}/DatasetDiagnosticsTool.h (97%)

diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
similarity index 90%
rename from src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
rename to src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
index e606fa3088..2df59221c7 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.cpp
+++ b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
@@ -8,15 +8,13 @@
 
 namespace esp {
 namespace metadata {
-namespace managers {
+namespace diagnostics {
 
 const std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap = {
     {"savecorrected", DSDiagnosticType::SaveCorrected},
-    {"testforsceneinstanceduplicates",
-     DSDiagnosticType::TestForDuplicateInstances},
-    {"testforsemanticregionduplicates",
-     DSDiagnosticType::TestForDuplicateRegions},
-    // Future diagnostics should be listed here
+    {"sceneinstanceduplicates", DSDiagnosticType::TestForDuplicateInstances},
+    {"semanticregionduplicates", DSDiagnosticType::TestForDuplicateRegions},
+    // Future diagnostics should be listed here, before "all"
     {"all", DSDiagnosticType::AllDiagnostics},
     {"allsavecorrected", DSDiagnosticType::AllDiagnosticsSaveCorrected},
 };
@@ -76,6 +74,6 @@ bool DatasetDiagnosticsTool::setNamedDiagnostic(const std::string& diagnostic,
   return true;
 }  // DatasetDiagnosticsTool::setNamedDiagnostic
 
-}  // namespace managers
+}  // namespace diagnostics
 }  // namespace metadata
 }  // namespace esp
diff --git a/src/esp/metadata/managers/DatasetDiagnosticsTool.h b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
similarity index 97%
rename from src/esp/metadata/managers/DatasetDiagnosticsTool.h
rename to src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
index 5a6198e215..4d17f7c64a 100644
--- a/src/esp/metadata/managers/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
@@ -3,8 +3,8 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the root directory of this source tree.
 
-#ifndef ESP_METADATA_MANAGERS_DATASETDIAGNOSTICSTOOL_H_
-#define ESP_METADATA_MANAGERS_DATASETDIAGNOSTICSTOOL_H_
+#ifndef ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICSTOOL_H_
+#define ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICSTOOL_H_
 
 #include "esp/core/Esp.h"
 #include "esp/io/Json.h"
@@ -12,7 +12,7 @@
 
 namespace esp {
 namespace metadata {
-namespace managers {
+namespace diagnostics {
 
 /**
  * @brief This enum class defines the various dataset diagnostics and remedies
@@ -281,8 +281,8 @@ class DatasetDiagnosticsTool {
 
 };  // class DatasetDiagnosticsTool
 
-}  // namespace managers
+}  // namespace diagnostics
 }  // namespace metadata
 }  // namespace esp
 
-#endif  // ESP_METADATA_MANAGERS_DATASETDIAGNOSTICSTOOL_H_
+#endif  // ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICSTOOL_H_
diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
index 2b518a2356..a29251b425 100644
--- a/src/esp/metadata/managers/AbstractAttributesManager.h
+++ b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -9,8 +9,8 @@
  * @brief Class Template @ref esp::metadata::managers::AbstractAttributesManager
  */
 
-#include "DatasetDiagnosticsTool.h"
 #include "esp/metadata/attributes/AbstractAttributes.h"
+#include "esp/metadata/diagnostics/DatasetDiagnosticsTool.h"
 
 #include "esp/core/managedContainers/ManagedFileBasedContainer.h"
 #include "esp/io/Io.h"
@@ -31,6 +31,7 @@ namespace managers {
 using core::config::Configuration;
 using core::managedContainers::ManagedFileBasedContainer;
 using core::managedContainers::ManagedObjectAccess;
+using diagnostics::DatasetDiagnosticsTool;
 
 /**
  * @brief Class template defining responsibilities and functionality for

From c10b73dbc5a1f522301dda545b8beaf214e84779 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Wed, 4 Dec 2024 08:58:02 -0500
Subject: [PATCH 15/20] --update cmakelist

---
 src/esp/CMakeLists.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/esp/CMakeLists.txt b/src/esp/CMakeLists.txt
index b878364cfd..83ee8c2e80 100644
--- a/src/esp/CMakeLists.txt
+++ b/src/esp/CMakeLists.txt
@@ -311,6 +311,8 @@ set(
   metadata/attributes/SemanticAttributes.cpp
   metadata/attributes/StageAttributes.h
   metadata/attributes/StageAttributes.cpp
+  metadata/diagnostics/DatasetDiagnosticsTool.h
+  metadata/diagnostics/DatasetDiagnosticsTool.cpp
   metadata/managers/AbstractAttributesManager.h
   metadata/managers/AbstractObjectAttributesManager.h
   metadata/managers/AOAttributesManager.h

From f218d98e3227fb65a180a990438149a93ffb1ccb Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Wed, 11 Dec 2024 09:32:09 -0500
Subject: [PATCH 16/20] --update method name for SensorAttributesManager

---
 .../managers/SensorAttributesManager.cpp      |  2 +-
 .../managers/SensorAttributesManager.h        | 19 ++++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/esp/metadata/managers/SensorAttributesManager.cpp b/src/esp/metadata/managers/SensorAttributesManager.cpp
index 6df662eac3..a2ce85abc1 100644
--- a/src/esp/metadata/managers/SensorAttributesManager.cpp
+++ b/src/esp/metadata/managers/SensorAttributesManager.cpp
@@ -158,7 +158,7 @@ AbstractSensorAttributes::ptr SensorAttributesManager::buildObjectFromJSONDoc(
   return sensorAttributes;
 }  // SensorAttributesManager::buildObjectFromJSONDoc
 
-void SensorAttributesManager::setValsFromJSONDoc(
+void SensorAttributesManager::setValsFromJSONDocInternal(
     AbstractSensorAttributes::ptr attribs,
     const io::JsonGenericValue& jsonConfig) {
   // TODO support loading values from JSON docs for each type of
diff --git a/src/esp/metadata/managers/SensorAttributesManager.h b/src/esp/metadata/managers/SensorAttributesManager.h
index 3c40bb58cc..296d3c58e4 100644
--- a/src/esp/metadata/managers/SensorAttributesManager.h
+++ b/src/esp/metadata/managers/SensorAttributesManager.h
@@ -98,15 +98,6 @@ class SensorAttributesManager
       const std::string& filename,
       const io::JsonGenericValue& jsonConfig) override;
 
-  /**
-   * @brief Method to take an existing attributes and set its values from passed
-   * json config file.
-   * @param attribs (out) an existing attributes to be modified.
-   * @param jsonConfig json document to parse
-   */
-  void setValsFromJSONDoc(attributes::AbstractSensorAttributes::ptr attribs,
-                          const io::JsonGenericValue& jsonConfig) override;
-
   /**
    * @brief This function will be called to finalize attributes' paths before
    * registration, moving fully qualified paths to the appropriate hidden
@@ -119,6 +110,16 @@ class SensorAttributesManager
           attributes) const override {}
 
  protected:
+  /**
+   * @brief Internally accessed from AbstractAttributesManager. Method to take
+   * an existing attributes and set its values from passed json config file.
+   * @param attribs (out) an existing attributes to be modified.
+   * @param jsonConfig json document to parse
+   */
+  void setValsFromJSONDocInternal(
+      attributes::AbstractSensorAttributes::ptr attribs,
+      const io::JsonGenericValue& jsonConfig) override;
+
   /**
    * @brief Internal only. Create an attributes from a SensorSpec.
    *

From 6f509e9dba50a2a6b76e168e9f171a1067a06741 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Tue, 28 Jan 2025 12:55:31 -0500
Subject: [PATCH 17/20] --minor

---
 src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
index 4d17f7c64a..44e22642ab 100644
--- a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
@@ -58,15 +58,16 @@ enum class DSDiagnosticType : uint32_t {
 
 /**
  * @brief Construct to record the results of a series of diagnostics against a
- * single attributes.
+ * single attributes/configuration. TODO: build this record for each diagnostic
+ * process to facilitate reporting.
  */
 template <class T>
 class DSDiagnosticRecord {
  public:
   static_assert(
       std::is_base_of<esp::metadata::attributes::AbstractAttributes, T>::value,
-      "AbstractManagedPhysicsObject :: Managed physics object type must be "
-      "derived from esp::physics::PhysicsObjectBase");
+      "DSDiagnosticRecord :: Diagnostic record type must be derived "
+      "from esp::metadata::attributes::AbstractAttributes");
 
   typedef std::weak_ptr<T> WeakObjRef;
 

From 1f070c0f0b43e6d2b84c898276ae7b3563790248 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Wed, 29 Jan 2025 13:22:11 -0500
Subject: [PATCH 18/20] --message for diagnostics creation

---
 src/esp/metadata/managers/SceneDatasetAttributesManager.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
index af4ea43c05..84f85da48c 100644
--- a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
@@ -70,8 +70,10 @@ void SceneDatasetAttributesManager::setValsFromJSONDocInternal(
     attributes::SceneDatasetAttributes::ptr dsAttribs,
     const io::JsonGenericValue& jsonConfig) {
   // check for diagnostics requests
-  this->setDSDiagnostics(dsAttribs, jsonConfig);
-
+  bool dsSet = this->setDSDiagnostics(dsAttribs, jsonConfig);
+  ESP_VERY_VERBOSE(Mn::Debug::Flag::NoSpace)
+      << "Attempt to set dataset diagnostics was a "
+      << (dsSet ? "Success" : "Failure");
   // dataset root directory to build paths from
   std::string dsDir = dsAttribs->getFileDirectory();
   // process stages

From 8340c1e78a9e3714b0c67918b34fae56ab01e76f Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Mon, 3 Feb 2025 10:32:32 -0500
Subject: [PATCH 19/20] --minor

---
 .../diagnostics/DatasetDiagnosticsTool.cpp    |  3 ++-
 .../diagnostics/DatasetDiagnosticsTool.h      | 21 ++++++++++++++++---
 .../managers/AbstractAttributesManager.h      | 15 ++++++-------
 .../SceneDatasetAttributesManager.cpp         |  2 +-
 4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
index 2df59221c7..470c4e31ff 100644
--- a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
+++ b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
@@ -46,7 +46,8 @@ bool DatasetDiagnosticsTool::setDiagnosticesFromJSON(
   ESP_ERROR(Mn::Debug::Flag::NoSpace)
       << msgStr
       << " configuration specifies `request_diagnostics` but specification "
-         "is unable to be parsed, so diagnostics request is ignored.";
+         "is unable to be parsed as either a string or an array, so "
+         "diagnostics request is ignored.";
   return false;
 }  // DatasetDiagnosticsTool::setDiagnosticesFromJSON
 
diff --git a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
index 44e22642ab..d72763343c 100644
--- a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
@@ -208,21 +208,36 @@ class DatasetDiagnosticsTool {
   /**
    * @brief Set that a save is required. This is to bridge from reading the json
    * file into the attributes and registering the attributes to the
-   * post-registration code.
+   * post-registration code. True means we corrected dataset components due to
+   * diagnostic activities and we wish to save those corrected components.
    */
   void setSaveRequired(bool saveRequired) {
     _requiresCorrectedSave = saveRequired;
   }
 
   /**
-   * @brief Clear any flags set due to specific diagnostics
+   * @brief Reset the current state of the Diagnostics tool.
+   */
+  void reset() {
+    clearSaveRequired();
+    clearDiagnosticFlags();
+  }
+
+  /**
+   * @brief Clear save flag set due to specific diagnostic conditions
    */
   void clearSaveRequired() { _requiresCorrectedSave = false; }
 
+  /**
+   * @brief Clear all existing diagnostic flag settings.
+   */
+  void clearDiagnosticFlags() { _diagnosticsFlags = 0u; }
+
   /**
    * @brief Get whether a save is required. This is to bridge from reading the
    * json file into the attributes and registering the attributes to the
-   * post-registration code.
+   * post-registration code. True means we corrected dataset components due to
+   * diagnostic activities and we wish to save those corrected components.
    */
   bool saveRequired() const { return _requiresCorrectedSave; }
 
diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
index a29251b425..e5a008f36a 100644
--- a/src/esp/metadata/managers/AbstractAttributesManager.h
+++ b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -192,7 +192,7 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
   void setValsFromJSONDoc(AttribsPtr attribs,
                           const io::JsonGenericValue& jsonConfig) {
     // Clear diagnostic flags from previous run
-    this->datasetDiagnostics_->clearSaveRequired();
+    this->datasetDiagnostics_->reset();
     this->setValsFromJSONDocInternal(attribs, jsonConfig);
     if (this->datasetDiagnostics_->saveRequired()) {
       ESP_WARNING(Mn::Debug::Flag::NoSpace)
@@ -208,18 +208,19 @@ class AbstractAttributesManager : public ManagedFileBasedContainer<T, Access> {
    * @brief Configure @ref datasetDiagnostics_ tool based on if passsed jsonConfig
    * requests them.
    */
-  bool setDSDiagnostics(AttribsPtr attribs,
-                        const io::JsonGenericValue& jsonConfig) {
+  bool setDSDiagnosticsFromJSON(AttribsPtr attribs,
+                                const io::JsonGenericValue& jsonConfig) {
     io::JsonGenericValue::ConstMemberIterator jsonIter =
         jsonConfig.FindMember("request_diagnostics");
     if (jsonIter == jsonConfig.MemberEnd()) {
       return false;
     }
     return this->datasetDiagnostics_->setDiagnosticesFromJSON(
-        jsonIter->value, Cr::Utility::formatString(
-                             "(setDSDiagnostics) <{}> : {}", this->objectType_,
-                             attribs->getSimplifiedHandle()));
-  }  // setDSDiagnostics
+        jsonIter->value,
+        Cr::Utility::formatString("(setDSDiagnosticsFromJSON) <{}> : {}",
+                                  this->objectType_,
+                                  attribs->getSimplifiedHandle()));
+  }  // setDSDiagnosticsFromJSON
 
   /**
    * @brief Merge the passed DatasetDiagnosticsTool settings into
diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
index 84f85da48c..d6eaa04509 100644
--- a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
+++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp
@@ -70,7 +70,7 @@ void SceneDatasetAttributesManager::setValsFromJSONDocInternal(
     attributes::SceneDatasetAttributes::ptr dsAttribs,
     const io::JsonGenericValue& jsonConfig) {
   // check for diagnostics requests
-  bool dsSet = this->setDSDiagnostics(dsAttribs, jsonConfig);
+  bool dsSet = this->setDSDiagnosticsFromJSON(dsAttribs, jsonConfig);
   ESP_VERY_VERBOSE(Mn::Debug::Flag::NoSpace)
       << "Attempt to set dataset diagnostics was a "
       << (dsSet ? "Success" : "Failure");

From 78b8552357d0ba9d442641fed439c9caadd7cf88 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Thu, 6 Feb 2025 11:34:02 -0500
Subject: [PATCH 20/20] --reorganize; build diagnostics parent classes for
 other subsystems

---
 src/esp/CMakeLists.txt                        |   5 +
 src/esp/core/diagnostics/DiagnosticsRecord.h  | 121 +++++++++++++
 src/esp/core/diagnostics/DiagnosticsTool.h    | 152 ++++++++++++++++
 .../diagnostics/DatasetDiagnostics.cpp        |  40 +++++
 .../metadata/diagnostics/DatasetDiagnostics.h |  78 ++++++++
 .../diagnostics/DatasetDiagnosticsRecord.h    |  69 ++++++++
 .../diagnostics/DatasetDiagnosticsTool.cpp    |  34 +---
 .../diagnostics/DatasetDiagnosticsTool.h      | 167 ++----------------
 8 files changed, 483 insertions(+), 183 deletions(-)
 create mode 100644 src/esp/core/diagnostics/DiagnosticsRecord.h
 create mode 100644 src/esp/core/diagnostics/DiagnosticsTool.h
 create mode 100644 src/esp/metadata/diagnostics/DatasetDiagnostics.cpp
 create mode 100644 src/esp/metadata/diagnostics/DatasetDiagnostics.h
 create mode 100644 src/esp/metadata/diagnostics/DatasetDiagnosticsRecord.h

diff --git a/src/esp/CMakeLists.txt b/src/esp/CMakeLists.txt
index 83ee8c2e80..692aade153 100644
--- a/src/esp/CMakeLists.txt
+++ b/src/esp/CMakeLists.txt
@@ -145,6 +145,8 @@ set(
   core/Check.h
   core/Configuration.cpp
   core/Configuration.h
+  core/diagnostics/DiagnosticsRecord.h
+  core/diagnostics/DiagnosticsTool.h
   core/Esp.cpp
   core/Esp.h
   core/Logging.cpp
@@ -313,6 +315,9 @@ set(
   metadata/attributes/StageAttributes.cpp
   metadata/diagnostics/DatasetDiagnosticsTool.h
   metadata/diagnostics/DatasetDiagnosticsTool.cpp
+  metadata/diagnostics/DatasetDiagnosticsRecord.h
+  metadata/diagnostics/DatasetDiagnostics.h
+  metadata/diagnostics/DatasetDiagnostics.cpp
   metadata/managers/AbstractAttributesManager.h
   metadata/managers/AbstractObjectAttributesManager.h
   metadata/managers/AOAttributesManager.h
diff --git a/src/esp/core/diagnostics/DiagnosticsRecord.h b/src/esp/core/diagnostics/DiagnosticsRecord.h
new file mode 100644
index 0000000000..a27e6590e7
--- /dev/null
+++ b/src/esp/core/diagnostics/DiagnosticsRecord.h
@@ -0,0 +1,121 @@
+// Copyright (c) Meta Platforms, Inc. and its affiliates.
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
+#ifndef ESP_CORE_DIAGNOSTICS_DIAGNOSTICSRECORD_H_
+#define ESP_CORE_DIAGNOSTICS_DIAGNOSTICSRECORD_H_
+
+#include <type_traits>
+#include <unordered_map>
+#include "esp/core/Esp.h"
+
+namespace esp {
+namespace core {
+namespace diagnostics {
+
+/**
+ * @brief Construct to provide a foundation for building a record of the results
+ * of a series of diagnostics.
+ */
+class DiagnosticsRecord {
+ public:
+  explicit DiagnosticsRecord(uint32_t diagnosticsFlags)
+      : diagnosticsFlags_(diagnosticsFlags) {}
+
+ protected:
+  /**
+   * @brief Set the diagnostic flag representing the type of diagnostics
+   * recorded by this record
+   */
+  template <class T>
+  inline void setFlag(T flag, bool val) {
+    //_flag must be an enum value
+    static_assert(std::is_enum<T>::value,
+                  "The Diagnostics Record flag must be an enum");
+    //_flag must be backed by a uint32_t
+    static_assert(
+        std::is_same<typename std::underlying_type<T>::type, uint32_t>::value,
+        "The Diagnostics Record flag enum must be backed by a uint32_t");
+    if (val) {
+      diagnosticsFlags_ |= static_cast<uint32_t>(flag);
+    } else {
+      diagnosticsFlags_ &= ~static_cast<uint32_t>(flag);
+    }
+  }
+
+  /**
+   * @brief Get the diagnostic flag representing the type of diagnostics
+   * recorded by this record
+   */
+  template <class T>
+  inline bool getFlag(T flag) const {
+    //_flag must be an enum value
+    static_assert(std::is_enum<T>::value,
+                  "The Diagnostics Record flag must be an enum");
+    //_flag must be backed by a uint32_t
+    static_assert(
+        std::is_same<typename std::underlying_type<T>::type, uint32_t>::value,
+        "The Diagnostics Record flag enum must be backed by a uint32_t");
+    return (diagnosticsFlags_ & static_cast<uint32_t>(flag)) ==
+           static_cast<uint32_t>(flag);
+  }
+
+  /**
+   * @brief Record diagnostic information for specified test.
+   * @param diagFlag The diagnostic test enum representing the type of the test.
+   * @param report String report to be appended to the @p _resultsLog
+   */
+  template <class T>
+  void recordDiagResult(T diagFlag, const std::string& report) {
+    //_flag must be an enum value
+    static_assert(std::is_enum<T>::value,
+                  "The Diagnostics Record flag must be an enum");
+    //_flag must be backed by a uint32_t
+    static_assert(
+        std::is_same<typename std::underlying_type<T>::type, uint32_t>::value,
+        "The Diagnostics Record flag enum must be backed by a uint32_t");
+    // either add a new or retrieve existing
+    auto emplaceRes = resultsLog_.emplace(static_cast<uint32_t>(diagFlag),
+                                          std::vector<std::string>());
+    emplaceRes.first->second.push_back(report);
+  }
+
+  /**
+   * @brief Retrieve results log for specific diagnostic test type
+   */
+  template <class T>
+  const std::vector<std::string> getDiagResult(T diagFlag) const {
+    //_flag must be an enum value
+    static_assert(std::is_enum<T>::value,
+                  "The Diagnostics Record flag must be an enum");
+    //_flag must be backed by a uint32_t
+    static_assert(
+        std::is_same<typename std::underlying_type<T>::type, uint32_t>::value,
+        "The Diagnostics Record flag enum must be backed by a uint32_t");
+    auto reportIter = resultsLog_.find(static_cast<uint32_t>(diagFlag));
+    if (reportIter == resultsLog_.end()) {
+      // Caller should check for empty and respond accordingly
+      return {};
+    }
+    return reportIter->second;
+  }  // getDiagResult
+
+ private:
+  /**
+   * @brief Holds collections of strings recording the results of each enabled
+   * diagnostic.
+   */
+  std::unordered_map<uint32_t, std::vector<std::string>> resultsLog_;
+  /**
+   * @brief Flags to record the diagnostics performed
+   */
+  uint32_t diagnosticsFlags_ = 0u;
+
+ public:
+  ESP_SMART_POINTERS(DiagnosticsRecord)
+};  // class DiagnosticsRecord
+
+}  // namespace diagnostics
+}  // namespace core
+}  // namespace esp
+#endif  // ESP_CORE_DIAGNOSTICS_DIAGNOSTICSRECORD_H_
diff --git a/src/esp/core/diagnostics/DiagnosticsTool.h b/src/esp/core/diagnostics/DiagnosticsTool.h
new file mode 100644
index 0000000000..1b7ae2f205
--- /dev/null
+++ b/src/esp/core/diagnostics/DiagnosticsTool.h
@@ -0,0 +1,152 @@
+// Copyright (c) Meta Platforms, Inc. and its affiliates.
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
+#ifndef ESP_CORE_DIAGNOSTICS_DIAGNOSTICSTOOL_H_
+#define ESP_CORE_DIAGNOSTICS_DIAGNOSTICSTOOL_H_
+
+#include <type_traits>
+#include "esp/core/Check.h"
+#include "esp/core/Esp.h"
+
+namespace esp {
+namespace core {
+namespace diagnostics {
+
+/**
+ * @brief This class will track requested diagnostics for specific config
+ * files/attributes. It is intended that each @ref AbstractAttributesManager
+ * would instantiate one of these objects and use it to determine various
+ * diagnostic behavior.
+ */
+class DiagnosticsTool {
+ public:
+  DiagnosticsTool() = default;
+
+  /**
+   * @brief Reset the current state of the Diagnostics tool.
+   */
+  void reset() {
+    clearDiagnosticFlags();
+    resetIndiv();
+  }
+
+  /**
+   * @brief Implementation class-specific reset code
+   */
+  virtual void resetIndiv() = 0;
+
+  /**
+   * @brief Clear all existing diagnostic flag settings.
+   */
+  void clearDiagnosticFlags() { diagnosticsFlags_ = 0u; }
+
+ protected:
+  /**
+   * @brief Merge the passed @ref DiagnosticsTool's @p _diagnosticsFlag
+   * settings into this one's, preserving this one's diagnostic requests as
+   * well. Only referenced internally to ensure tool type consistency.
+   */
+  void mergeDiagnosticsToolInternal(const DiagnosticsTool& tool) {
+    diagnosticsFlags_ |= tool.diagnosticsFlags_;
+  }
+
+  /**
+   * @brief Enable/disable the diagnostic given by @p diagnostic string based on
+   * @p val.
+   *
+   * @param diagnostic The string name of the diagnostic. This must be mappable
+   * to an appropriate enum type via @p diagMap .
+   * @param diagMap Map holding the string to enum mapping for the implementing
+   * diagnostic tool.
+   * @param val Whether to enable or disable the given diagnostic
+   * @param abortOnFail Whether or not to abort the program if the
+   * @p diagnostic requeseted is not found in  @p diagMap. If false,
+   * will print an error message and skip the unknown request.
+   * @return Whether the passed string successfully mapped to a known diagnostic
+   */
+  template <class T>
+  bool setNamedDiagnosticInternal(const std::string& diagnostic,
+                                  const std::map<std::string, T>& diagMap,
+                                  bool val,
+                                  bool abortOnFail = false);
+
+  template <class T>
+  inline void setFlag(T _flag, bool _val) {
+    //_flag must be an enum value
+    static_assert(std::is_enum<T>::value,
+                  "The Diagnostics Tool's flag must be an enum");
+    //_flag must be backed by a uint32_t
+    static_assert(
+        std::is_same<typename std::underlying_type<T>::type, uint32_t>::value,
+        "The Diagnostics Tool's flag enum must be backed by a uint32_t");
+    if (_val) {
+      diagnosticsFlags_ |= static_cast<uint32_t>(_flag);
+    } else {
+      diagnosticsFlags_ &= ~static_cast<uint32_t>(_flag);
+    }
+  }
+
+  template <class T>
+  inline bool getFlag(T _flag) const {
+    //_flag must be an enum value
+    static_assert(std::is_enum<T>::value,
+                  "The Diagnostics Tool's flag must be an enum");
+    //_flag must be backed by a uint32_t
+    static_assert(
+        std::is_same<typename std::underlying_type<T>::type, uint32_t>::value,
+        "The Diagnostics Tool's flag enum must be backed by a uint32_t");
+    return (diagnosticsFlags_ & static_cast<uint32_t>(_flag)) ==
+           static_cast<uint32_t>(_flag);
+  }
+
+ private:
+  uint32_t diagnosticsFlags_ = 0u;
+
+ public:
+  ESP_SMART_POINTERS(DiagnosticsTool)
+
+};  // class DiagnosticsTool
+
+/////////////////////////////
+// Class Template Method Definitions
+template <class T>
+bool DiagnosticsTool::setNamedDiagnosticInternal(
+    const std::string& diagnostic,
+    const std::map<std::string, T>& diagMap,
+    bool val,
+    bool abortOnFail) {
+  //_flag must be an enum value
+  static_assert(std::is_enum<T>::value,
+                "The Diagnostics Tool's flag must be an enum");
+  //_flag must be backed by a uint32_t
+  static_assert(
+      std::is_same<typename std::underlying_type<T>::type, uint32_t>::value,
+      "The Diagnostics Tool's flag enum must be backed by a uint32_t");
+  const std::string diagnosticLC =
+      Corrade::Utility::String::lowercase(diagnostic);
+
+  auto mapIter = diagMap.find(diagnosticLC);
+  if (abortOnFail) {
+    // If not found then abort.
+    ESP_CHECK(mapIter != diagMap.end(),
+              "Unknown Diagnostic Test requested to be "
+                  << (val ? "Enabled :" : "Disabled :") << diagnostic
+                  << ". Aborting.");
+  } else {
+    if (mapIter == diagMap.end()) {
+      ESP_ERROR() << "Unknown Diagnostic Test requested to be "
+                  << (val ? "Enabled" : "Disabled") << " :" << diagnostic
+                  << " so ignoring request.";
+      return false;
+    }
+  }
+  setFlag(mapIter->second, val);
+  return true;
+}  // DiagnosticsTool::setNamedDiagnostic
+
+}  // namespace diagnostics
+}  // namespace core
+}  // namespace esp
+
+#endif  // ESP_CORE_DIAGNOSTICS_DIAGNOSTICSTOOL_H_
diff --git a/src/esp/metadata/diagnostics/DatasetDiagnostics.cpp b/src/esp/metadata/diagnostics/DatasetDiagnostics.cpp
new file mode 100644
index 0000000000..5491c7b2ea
--- /dev/null
+++ b/src/esp/metadata/diagnostics/DatasetDiagnostics.cpp
@@ -0,0 +1,40 @@
+
+// Copyright (c) Meta Platforms, Inc. and its affiliates.
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
+#include "DatasetDiagnostics.h"
+
+namespace esp {
+namespace metadata {
+namespace diagnostics {
+
+const std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap = {
+    {"savecorrected", DSDiagnosticType::SaveCorrected},
+    {"sceneinstanceduplicates", DSDiagnosticType::TestForDuplicateInstances},
+    {"semanticregionduplicates", DSDiagnosticType::TestForDuplicateRegions},
+    // Future diagnostics should be listed here, before "all"
+    {"all", DSDiagnosticType::AllDiagnostics},
+    {"allsavecorrected", DSDiagnosticType::AllDiagnosticsSaveCorrected},
+};
+
+std::string getDSDiagnosticsTypeName(DSDiagnosticType dsDiagTypeName) {
+  // this verifies that enum value being checked is supported by string-keyed
+  // map. The values below should be the minimum and maximum enums supported by
+  // DSDiagnosticTypeMap
+  if (dsDiagTypeName <= DSDiagnosticType::Unknown) {
+    return "unknown";
+  }
+  // Must always be valid value
+  for (const auto& it : DSDiagnosticTypeMap) {
+    if (it.second == dsDiagTypeName) {
+      return it.first;
+    }
+  }
+  return "unknown";
+}  // getDSDiagnosticsTypeName
+
+}  // namespace diagnostics
+
+}  // namespace metadata
+}  // namespace esp
diff --git a/src/esp/metadata/diagnostics/DatasetDiagnostics.h b/src/esp/metadata/diagnostics/DatasetDiagnostics.h
new file mode 100644
index 0000000000..483ea58bcc
--- /dev/null
+++ b/src/esp/metadata/diagnostics/DatasetDiagnostics.h
@@ -0,0 +1,78 @@
+
+// Copyright (c) Meta Platforms, Inc. and its affiliates.
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
+#ifndef ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICS_H_
+#define ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICS_H_
+
+#include "esp/core/Esp.h"
+
+namespace esp {
+namespace metadata {
+namespace diagnostics {
+
+/**
+ * @brief This enum class defines the various dataset diagnostics and remedies
+ * that Habitat-Sim can accept and process. These flags will be specified in
+ * config files and consumed by the config's manager.
+ */
+enum class DSDiagnosticType : uint32_t {
+  /**
+   * @brief Unknown/unspecified diagnostic
+   */
+  Unknown = 0,
+  /**
+   * @brief Save any dataset configurations that fail a requested diagnostic
+   * and are consequently corrected. Ignored if no diagnostics are specified. If
+   * diagnostics are specified but this flag is not set then the corrections
+   * will only persist during current execution.
+   */
+  SaveCorrected = 1U,
+
+  /**
+   * @brief Test each @ref SceneInstanceAttributes file as it is loaded for
+   * duplicate rigid and articulated object instances. Duplicates will have all
+   * non-hidden fields equal. This would result in 2 identical objects being
+   * instantiated in the same location with the same initial state.
+   */
+  TestForDuplicateInstances = (1U << 1),
+
+  /**
+   * @brief Test each @ref SemanticAttributes file as it is loaded for
+   * duplicate region instances. Duplicates will have all non-hidden fields
+   * equal. This would result in multiple semantic regions being defined for the
+   * same area in the scene.
+   */
+  TestForDuplicateRegions = (1U << 2),
+
+  /**
+   * @brief Shortcut to perform all diagnostics but do not save corrected
+   * results.
+   */
+  AllDiagnostics = ~SaveCorrected,
+
+  /**
+   * @brief Shortcut to perform all diagnostics and save corrected results.
+   */
+  AllDiagnosticsSaveCorrected = ~0U
+};
+
+/**
+ * @brief Constant map to provide mappings from string tags to @ref
+ * DSDiagnosticType values. This will be used to match values set
+ * in json for requested dataset diagnostics to @ref DSDiagnosticType values.
+ */
+const extern std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap;
+
+/**
+ * @brief This method will convert a @ref DSDiagnosticType value to the
+ * string key that maps to it in the DSDiagnosticTypeMap
+ */
+std::string getDSDiagnosticsTypeName(DSDiagnosticType dsDiagTypeName);
+
+}  // namespace diagnostics
+}  // namespace metadata
+}  // namespace esp
+
+#endif
diff --git a/src/esp/metadata/diagnostics/DatasetDiagnosticsRecord.h b/src/esp/metadata/diagnostics/DatasetDiagnosticsRecord.h
new file mode 100644
index 0000000000..a0e3195914
--- /dev/null
+++ b/src/esp/metadata/diagnostics/DatasetDiagnosticsRecord.h
@@ -0,0 +1,69 @@
+
+// Copyright (c) Meta Platforms, Inc. and its affiliates.
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
+#ifndef ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICSRECORD_H_
+#define ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICSRECORD_H_
+
+#include "esp/core/Esp.h"
+#include "esp/core/diagnostics/DiagnosticsRecord.h"
+#include "esp/metadata/attributes/AbstractAttributes.h"
+
+namespace esp {
+namespace metadata {
+namespace diagnostics {
+
+/**
+ * @brief Construct to record the results of a series of diagnostics against a
+ * single attributes/configuration. TODO: build this record for each diagnostic
+ * process to facilitate reporting.
+ */
+template <class T>
+class DSDiagnosticRecord : public core::diagnostics::DiagnosticsRecord {
+ public:
+  static_assert(
+      std::is_base_of<esp::metadata::attributes::AbstractAttributes, T>::value,
+      "DSDiagnosticRecord :: Diagnostic record type must be derived "
+      "from esp::metadata::attributes::AbstractAttributes");
+
+  typedef std::weak_ptr<T> WeakObjRef;
+
+  explicit DSDiagnosticRecord(uint32_t diagnosticsFlags)
+      : DiagnosticsRecord(diagnosticsFlags) {}
+
+  /**
+   * @brief set the reference to this diagnostic record's subject
+   */
+  void setObjectRef(const std::shared_ptr<T>& objRef) { weakObjRef_ = objRef; }
+
+ protected:
+  /**
+   * @brief This function accesses the underlying shared pointer of this
+   * object's @p weakObjRef_ if it exists; if not, it provides a message.
+   * @return Either a shared pointer of this record's object, or nullptr if
+   * dne.
+   */
+  std::shared_ptr<T> inline getObjectReference() const {
+    std::shared_ptr<T> sp = weakObjRef_.lock();
+    if (!sp) {
+      ESP_ERROR()
+          << "This attributes no longer exists. Please delete any variable "
+             "references.";
+    }
+    return sp;
+  }  // getObjectReference
+
+  // Non-owning reference to attributes this record pertains to.
+  WeakObjRef weakObjRef_;
+
+ public:
+  ESP_SMART_POINTERS(DSDiagnosticRecord)
+
+};  // class DSDiagnosticRecord
+
+}  // namespace diagnostics
+}  // namespace metadata
+}  // namespace esp
+
+#endif  // ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICSRECORD_H_
diff --git a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
index 470c4e31ff..18fc56b779 100644
--- a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
+++ b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.cpp
@@ -4,21 +4,13 @@
 // LICENSE file in the root directory of this source tree.
 
 #include "DatasetDiagnosticsTool.h"
+#include "DatasetDiagnostics.h"
 #include "esp/core/Check.h"
 
 namespace esp {
 namespace metadata {
 namespace diagnostics {
 
-const std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap = {
-    {"savecorrected", DSDiagnosticType::SaveCorrected},
-    {"sceneinstanceduplicates", DSDiagnosticType::TestForDuplicateInstances},
-    {"semanticregionduplicates", DSDiagnosticType::TestForDuplicateRegions},
-    // Future diagnostics should be listed here, before "all"
-    {"all", DSDiagnosticType::AllDiagnostics},
-    {"allsavecorrected", DSDiagnosticType::AllDiagnosticsSaveCorrected},
-};
-
 bool DatasetDiagnosticsTool::setDiagnosticesFromJSON(
     const io::JsonGenericValue& jsonObj,
     const std::string& msgStr) {
@@ -51,30 +43,6 @@ bool DatasetDiagnosticsTool::setDiagnosticesFromJSON(
   return false;
 }  // DatasetDiagnosticsTool::setDiagnosticesFromJSON
 
-bool DatasetDiagnosticsTool::setNamedDiagnostic(const std::string& diagnostic,
-                                                bool val,
-                                                bool abortOnFail) {
-  const std::string diagnosticLC = Cr::Utility::String::lowercase(diagnostic);
-
-  auto mapIter = DSDiagnosticTypeMap.find(diagnosticLC);
-  if (abortOnFail) {
-    // If not found then abort.
-    ESP_CHECK(mapIter != DSDiagnosticTypeMap.end(),
-              "Unknown Diagnostic Test requested to be "
-                  << (val ? "Enabled" : "Disabled") << " :" << diagnostic
-                  << ". Aborting.");
-  } else {
-    if (mapIter == DSDiagnosticTypeMap.end()) {
-      ESP_ERROR() << "Unknown Diagnostic Test requested to be "
-                  << (val ? "Enabled" : "Disabled") << " :" << diagnostic
-                  << " so ignoring request.";
-      return false;
-    }
-  }
-  setFlags(mapIter->second, val);
-  return true;
-}  // DatasetDiagnosticsTool::setNamedDiagnostic
-
 }  // namespace diagnostics
 }  // namespace metadata
 }  // namespace esp
diff --git a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
index d72763343c..f5748ce869 100644
--- a/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
+++ b/src/esp/metadata/diagnostics/DatasetDiagnosticsTool.h
@@ -6,137 +6,23 @@
 #ifndef ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICSTOOL_H_
 #define ESP_METADATA_DIAGNOSTICS_DATASETDIAGNOSTICSTOOL_H_
 
-#include "esp/core/Esp.h"
+#include "DatasetDiagnostics.h"
+#include "esp/core/diagnostics/DiagnosticsTool.h"
 #include "esp/io/Json.h"
-#include "esp/metadata/attributes/AbstractAttributes.h"
 
 namespace esp {
 namespace metadata {
 namespace diagnostics {
 
-/**
- * @brief This enum class defines the various dataset diagnostics and remedies
- * that Habitat-Sim can accept and process. These flags will be specified in
- * config files and consumed by the config's manager.
- */
-enum class DSDiagnosticType : uint32_t {
-  /**
-   * @brief Save any dataset configurations that fail a requested diagnostic
-   * and are consequently corrected. Ignored if no diagnostics are specified. If
-   * diagnostics are specified but this flag is not set then the corrections
-   * will only persist during current execution.
-   */
-  SaveCorrected = 1U,
-
-  /**
-   * @brief Test each @ref SceneInstanceAttributes file as it is loaded for
-   * duplicate rigid and articulated object instances. Duplicates will have all
-   * non-hidden fields equal. This would result in 2 identical objects being
-   * instantiated in the same location with the same initial state.
-   */
-  TestForDuplicateInstances = (1U << 1),
-
-  /**
-   * @brief Test each @ref SemanticAttributes file as it is loaded for
-   * duplicate region instances. Duplicates will have all non-hidden fields
-   * equal. This would result in multiple semantic regions being defined for the
-   * same area in the scene.
-   */
-  TestForDuplicateRegions = (1U << 2),
-
-  /**
-   * @brief Shortcut to perform all diagnostics but do not save corrected
-   * results.
-   */
-  AllDiagnostics = ~SaveCorrected,
-
-  /**
-   * @brief Shortcut to perform all diagnostics and save corrected results.
-   */
-  AllDiagnosticsSaveCorrected = ~0U
-};
-
-/**
- * @brief Construct to record the results of a series of diagnostics against a
- * single attributes/configuration. TODO: build this record for each diagnostic
- * process to facilitate reporting.
- */
-template <class T>
-class DSDiagnosticRecord {
- public:
-  static_assert(
-      std::is_base_of<esp::metadata::attributes::AbstractAttributes, T>::value,
-      "DSDiagnosticRecord :: Diagnostic record type must be derived "
-      "from esp::metadata::attributes::AbstractAttributes");
-
-  typedef std::weak_ptr<T> WeakObjRef;
-
-  explicit DSDiagnosticRecord(uint32_t diagnosticsFlags)
-      : _diagnosticsFlags(diagnosticsFlags) {}
-
-  /**
-   * @brief set the reference to this diagnostic record's subject
-   */
-  void setObjectRef(const std::shared_ptr<T>& objRef) { weakObjRef_ = objRef; }
-
-  inline void setFlags(DSDiagnosticType _flag, bool _val) {
-    if (_val) {
-      _diagnosticsFlags |= static_cast<uint32_t>(_flag);
-    } else {
-      _diagnosticsFlags &= ~static_cast<uint32_t>(_flag);
-    }
-  }
-
-  inline bool getFlags(DSDiagnosticType _flag) const {
-    return (_diagnosticsFlags & static_cast<uint32_t>(_flag)) ==
-           static_cast<uint32_t>(_flag);
-  }
-
- protected:
-  /**
-   * @brief This function accesses the underlying shared pointer of this
-   * object's @p weakObjRef_ if it exists; if not, it provides a message.
-   * @return Either a shared pointer of this record's object, or nullptr if
-   * dne.
-   */
-  std::shared_ptr<T> inline getObjectReference() const {
-    std::shared_ptr<T> sp = weakObjRef_.lock();
-    if (!sp) {
-      ESP_ERROR()
-          << "This attributes no longer exists. Please delete any variable "
-             "references.";
-    }
-    return sp;
-  }  // getObjectReference
-
-  // Non-owning reference to attributes this record pertains to.
-  WeakObjRef weakObjRef_;
-
- private:
-  uint32_t _diagnosticsFlags = 0u;
-
- public:
-  ESP_SMART_POINTERS(DSDiagnosticRecord)
-
-};  // struct DSDiagnosticRecord
-
-/**
- * @brief Constant map to provide mappings from string tags to @ref
- * DSDiagnosticType values. This will be used to match values set
- * in json for requested dataset diagnostics to @ref DSDiagnosticType values.
- */
-const extern std::map<std::string, DSDiagnosticType> DSDiagnosticTypeMap;
-
 /**
  * @brief This class will track requested diagnostics for specific config
  * files/attributes. It is intended that each @ref AbstractAttributesManager
  * would instantiate one of these objects and use it to determine various
  * diagnostic behavior.
  */
-class DatasetDiagnosticsTool {
+class DatasetDiagnosticsTool : public core::diagnostics::DiagnosticsTool {
  public:
-  DatasetDiagnosticsTool() = default;
-
+  DatasetDiagnosticsTool() : DiagnosticsTool() {}
   /**
    * @brief Set diagnostic values based on specifications in passed @p jsonObj.
    * @param jsonObj The json object referenced by the appropriate diagnostics
@@ -154,7 +40,7 @@ class DatasetDiagnosticsTool {
    * well.
    */
   void mergeDiagnosticsTool(const DatasetDiagnosticsTool& tool) {
-    _diagnosticsFlags |= tool._diagnosticsFlags;
+    mergeDiagnosticsToolInternal(tool);
   }
 
   /**
@@ -188,21 +74,24 @@ class DatasetDiagnosticsTool {
    */
   bool setNamedDiagnostic(const std::string& diagnostic,
                           bool val,
-                          bool abortOnFail = false);
+                          bool abortOnFail = false) {
+    return setNamedDiagnosticInternal(diagnostic, DSDiagnosticTypeMap, val,
+                                      abortOnFail);
+  }
 
   /**
    * @brief Specify whether or not we should save any corrected dataset
    * components if they received correction
    */
   void setShouldSaveCorrected(bool val) {
-    setFlags(DSDiagnosticType::SaveCorrected, val);
+    setFlag(DSDiagnosticType::SaveCorrected, val);
   }
   /**
    * @brief Query whether or not we should save any corrected dataset components
    * if they received correction
    */
   bool shouldSaveCorrected() const {
-    return getFlags(DSDiagnosticType::SaveCorrected);
+    return getFlag(DSDiagnosticType::SaveCorrected);
   }
 
   /**
@@ -216,23 +105,15 @@ class DatasetDiagnosticsTool {
   }
 
   /**
-   * @brief Reset the current state of the Diagnostics tool.
+   * @brief Reset the DatasetDiagnostics-specific state of the tool
    */
-  void reset() {
-    clearSaveRequired();
-    clearDiagnosticFlags();
-  }
+  virtual void resetIndiv() override { clearSaveRequired(); }
 
   /**
    * @brief Clear save flag set due to specific diagnostic conditions
    */
   void clearSaveRequired() { _requiresCorrectedSave = false; }
 
-  /**
-   * @brief Clear all existing diagnostic flag settings.
-   */
-  void clearDiagnosticFlags() { _diagnosticsFlags = 0u; }
-
   /**
    * @brief Get whether a save is required. This is to bridge from reading the
    * json file into the attributes and registering the attributes to the
@@ -246,14 +127,14 @@ class DatasetDiagnosticsTool {
    * in loaded @ref SceneInstanceAttributes and not process the duplicates if found.
    */
   void setTestDuplicateSceneInstances(bool val) {
-    setFlags(DSDiagnosticType::TestForDuplicateInstances, val);
+    setFlag(DSDiagnosticType::TestForDuplicateInstances, val);
   }
   /**
    * @brief Query whether or not to test for duplicate scene object instances
    * in loaded @ref SceneInstanceAttributes and not process the duplicates if found.
    */
   bool testDuplicateSceneInstances() const {
-    return getFlags(DSDiagnosticType::TestForDuplicateInstances);
+    return getFlag(DSDiagnosticType::TestForDuplicateInstances);
   }
 
   /**
@@ -262,31 +143,17 @@ class DatasetDiagnosticsTool {
    * in loaded @ref SemanticAttributes and not process the duplicates if found.
    */
   void setTestDuplicateSemanticRegions(bool val) {
-    setFlags(DSDiagnosticType::TestForDuplicateRegions, val);
+    setFlag(DSDiagnosticType::TestForDuplicateRegions, val);
   }
   /**
    * @brief Query whether or not to test for duplicate semantic region
    * specifications in loaded @ref SemanticAttributes and not process the duplicates if found.
    */
   bool testDuplicateSemanticRegions() const {
-    return getFlags(DSDiagnosticType::TestForDuplicateRegions);
+    return getFlag(DSDiagnosticType::TestForDuplicateRegions);
   }
 
  private:
-  inline void setFlags(DSDiagnosticType _flag, bool _val) {
-    if (_val) {
-      _diagnosticsFlags |= static_cast<uint32_t>(_flag);
-    } else {
-      _diagnosticsFlags &= ~static_cast<uint32_t>(_flag);
-    }
-  }
-
-  inline bool getFlags(DSDiagnosticType _flag) const {
-    return (_diagnosticsFlags & static_cast<uint32_t>(_flag)) ==
-           static_cast<uint32_t>(_flag);
-  }
-  uint32_t _diagnosticsFlags = 0u;
-
   /**
    * @brief Save the attributes current being processed. This flag is only so
    */