FIX: PR review fixes for Group B face-neighbor filters

- Fix missing return before MakeErrorResult in ComputeSurfaceFeaturesDirect
  for 1D geometry error path (pre-existing bug). Add same error path to
  ComputeSurfaceFeaturesScanline.
- Replace std::minmax_element full-scan validation with deferred max tracking
  during the main loop in ComputeFeatureNeighborsDirect and Scanline,
  eliminating a redundant OOC full-scan (1.4x OOC speedup).
- Add Doxygen @brief comments to operator()() in all 8 new algorithm classes.

Author: joeykleingers

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/BadDataNeighborOrientationCheckScanline.cpp

@@ -27,6 +27,13 @@ BadDataNeighborOrientationCheckScanline::BadDataNeighborOrientationCheckScanline
 BadDataNeighborOrientationCheckScanline::~BadDataNeighborOrientationCheckScanline() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Flips bad voxels to good using chunk-sequential multi-pass scanning.
+ * OOC path: Phase 1 counts matching good neighbors using chunk-sequential iteration.
+ * Phase 2 uses repeated chunk-sequential scans (instead of a worklist) to flip
+ * eligible voxels, with a chunk-skip optimization that avoids loading chunks
+ * with no eligible voxels.
+ */
 Result<> BadDataNeighborOrientationCheckScanline::operator()()
 {
   const float misorientationTolerance = m_InputValues->MisorientationTolerance * numbers::pi_v<float> / 180.0f;

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/BadDataNeighborOrientationCheckWorklist.cpp

@@ -29,6 +29,12 @@ BadDataNeighborOrientationCheckWorklist::BadDataNeighborOrientationCheckWorklist
 BadDataNeighborOrientationCheckWorklist::~BadDataNeighborOrientationCheckWorklist() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Flips bad voxels to good using a worklist-based propagation algorithm.
+ * In-core path: Phase 1 counts matching good neighbors per bad voxel.
+ * Phase 2 iteratively flips eligible voxels using a deque worklist,
+ * propagating new eligibility to neighbors immediately.
+ */
 Result<> BadDataNeighborOrientationCheckWorklist::operator()()
 {
   const float misorientationTolerance = m_InputValues->MisorientationTolerance * numbers::pi_v<float> / 180.0f;

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeBoundaryCellsDirect.cpp

@@ -22,6 +22,10 @@ ComputeBoundaryCellsDirect::ComputeBoundaryCellsDirect(DataStructure& dataStruct
 ComputeBoundaryCellsDirect::~ComputeBoundaryCellsDirect() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Counts boundary faces per voxel using direct Z-Y-X iteration.
+ * In-core path: iterates all voxels sequentially, checking 6 face neighbors.
+ */
 Result<> ComputeBoundaryCellsDirect::operator()()
 {
   const auto& imageGeometry = m_DataStructure.getDataRefAs<ImageGeom>(m_InputValues->ImageGeometryPath);

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeBoundaryCellsScanline.cpp

@@ -22,6 +22,11 @@ ComputeBoundaryCellsScanline::ComputeBoundaryCellsScanline(DataStructure& dataSt
 ComputeBoundaryCellsScanline::~ComputeBoundaryCellsScanline() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Counts boundary faces per voxel using chunk-sequential iteration.
+ * OOC path: iterates chunks in order via loadChunk/getChunkLowerBounds/getChunkUpperBounds,
+ * then Z-Y-X within each chunk. Same logic as ComputeBoundaryCellsDirect.
+ */
 Result<> ComputeBoundaryCellsScanline::operator()()
 {
   const auto& imageGeometry = m_DataStructure.getDataRefAs<ImageGeom>(m_InputValues->ImageGeometryPath);

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureNeighborsDirect.cpp

@@ -9,8 +9,6 @@
 #include "simplnx/Utilities/MessageHelper.hpp"
 #include "simplnx/Utilities/NeighborUtilities.hpp"
 
-#include <sstream>
-
 using namespace nx::core;
 
 // -----------------------------------------------------------------------------
@@ -27,6 +25,11 @@ ComputeFeatureNeighborsDirect::ComputeFeatureNeighborsDirect(DataStructure& data
 ComputeFeatureNeighborsDirect::~ComputeFeatureNeighborsDirect() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Computes feature neighbor lists using direct sequential iteration.
+ * In-core path: Phase 1 iterates all voxels to build per-feature neighbor
+ * counts. Phase 2 deduplicates and computes shared surface areas.
+ */
 Result<> ComputeFeatureNeighborsDirect::operator()()
 {
   auto storeBoundaryCells = m_InputValues->StoreBoundaryCells;
@@ -58,16 +61,10 @@ Result<> ComputeFeatureNeighborsDirect::operator()()
   usize totalPoints = featureIds.getNumberOfTuples();
   usize totalFeatures = numNeighbors.getNumberOfTuples();
 
-  /* Ensure that we will be able to work with the user selected featureId Array */
-  const auto [minFeatureId, maxFeatureId] = std::minmax_element(featureIds.begin(), featureIds.end());
-  if(static_cast<usize>(*maxFeatureId) >= totalFeatures)
-  {
-    std::stringstream out;
-    out << "Data Array " << featureIdsPath.getTargetName() << " has a maximum value of " << *maxFeatureId << " which is greater than the "
-        << " number of features from array " << numNeighborsPath.getTargetName() << " which has " << totalFeatures << ". Did you select the "
-        << " incorrect array for the 'FeatureIds' array?";
-    return MakeErrorResult(-24500, out.str());
-  }
+  // Max feature ID validation is deferred to after the main loop to avoid
+  // a separate full scan through the data store. The loop's
+  // `feature < neighborlist.size()` guard prevents out-of-bounds access.
+  int32 observedMaxFeatureId = 0;
 
   auto& imageGeom = m_DataStructure.getDataRefAs<ImageGeom>(imageGeomPath);
   SizeVec3 uDims = imageGeom.getDimensions();
@@ -123,6 +120,10 @@ Result<> ComputeFeatureNeighborsDirect::operator()()
 
     onsurf = 0;
     feature = featureIds[voxelIndex];
+    if(feature > observedMaxFeatureId)
+    {
+      observedMaxFeatureId = feature;
+    }
     if(feature > 0 && feature < neighborlist.size())
     {
       int64 xIdx = voxelIndex % dims[0];
@@ -168,6 +169,15 @@ Result<> ComputeFeatureNeighborsDirect::operator()()
     }
   }
 
+  // Validate max feature ID (deferred from before the loop to avoid a separate full scan)
+  if(static_cast<usize>(observedMaxFeatureId) >= totalFeatures)
+  {
+    return MakeErrorResult(-24500,
+                           fmt::format("Data Array {} has a maximum value of {} which is greater than the number of features from array {} which has {}. "
+                                       "Did you select the incorrect array for the 'FeatureIds' array?",
+                                       featureIdsPath.getTargetName(), observedMaxFeatureId, numNeighborsPath.getTargetName(), totalFeatures));
+  }
+
   FloatVec3 spacing = imageGeom.getSpacing();
 
   // We do this to create new set of NeighborList objects

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureNeighborsScanline.cpp

@@ -9,8 +9,6 @@
 #include "simplnx/Utilities/MessageHelper.hpp"
 #include "simplnx/Utilities/NeighborUtilities.hpp"
 
-#include <sstream>
-
 using namespace nx::core;
 
 // -----------------------------------------------------------------------------
@@ -27,6 +25,12 @@ ComputeFeatureNeighborsScanline::ComputeFeatureNeighborsScanline(DataStructure&
 ComputeFeatureNeighborsScanline::~ComputeFeatureNeighborsScanline() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Computes feature neighbor lists using chunk-sequential iteration.
+ * OOC path: Phase 1 iterates chunks in order to build per-feature neighbor
+ * counts. Phase 2 deduplicates and computes shared surface areas.
+ * Same logic as ComputeFeatureNeighborsDirect.
+ */
 Result<> ComputeFeatureNeighborsScanline::operator()()
 {
   auto storeBoundaryCells = m_InputValues->StoreBoundaryCells;
@@ -58,16 +62,10 @@ Result<> ComputeFeatureNeighborsScanline::operator()()
   usize totalPoints = featureIds.getNumberOfTuples();
   usize totalFeatures = numNeighbors.getNumberOfTuples();
 
-  /* Ensure that we will be able to work with the user selected featureId Array */
-  const auto [minFeatureId, maxFeatureId] = std::minmax_element(featureIds.begin(), featureIds.end());
-  if(static_cast<usize>(*maxFeatureId) >= totalFeatures)
-  {
-    std::stringstream out;
-    out << "Data Array " << featureIdsPath.getTargetName() << " has a maximum value of " << *maxFeatureId << " which is greater than the "
-        << " number of features from array " << numNeighborsPath.getTargetName() << " which has " << totalFeatures << ". Did you select the "
-        << " incorrect array for the 'FeatureIds' array?";
-    return MakeErrorResult(-24500, out.str());
-  }
+  // Max feature ID validation is deferred to after the chunk-sequential loop
+  // to avoid a separate full scan through OOC data. The loop's
+  // `feature < neighborlist.size()` guard prevents out-of-bounds access.
+  int32 observedMaxFeatureId = 0;
 
   auto& imageGeom = m_DataStructure.getDataRefAs<ImageGeom>(imageGeomPath);
   SizeVec3 uDims = imageGeom.getDimensions();
@@ -140,6 +138,10 @@ Result<> ComputeFeatureNeighborsScanline::operator()()
           const int64 voxelIndex = kStride + jStride + static_cast<int64>(xIdx);
           onsurf = 0;
           feature = featureIds[voxelIndex];
+          if(feature > observedMaxFeatureId)
+          {
+            observedMaxFeatureId = feature;
+          }
           if(feature > 0 && feature < neighborlist.size())
           {
             const int64 ix = static_cast<int64>(xIdx);
@@ -188,6 +190,15 @@ Result<> ComputeFeatureNeighborsScanline::operator()()
     }
   }
 
+  // Validate max feature ID (deferred from before the loop to avoid a separate OOC scan)
+  if(static_cast<usize>(observedMaxFeatureId) >= totalFeatures)
+  {
+    return MakeErrorResult(-24500,
+                           fmt::format("Data Array {} has a maximum value of {} which is greater than the number of features from array {} which has {}. "
+                                       "Did you select the incorrect array for the 'FeatureIds' array?",
+                                       featureIdsPath.getTargetName(), observedMaxFeatureId, numNeighborsPath.getTargetName(), totalFeatures));
+  }
+
   // Phase 2: Build NeighborList objects (operates on small feature-level data, no chunk iteration needed)
   FloatVec3 spacing = imageGeom.getSpacing();
 

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeSurfaceAreaToVolumeDirect.cpp

@@ -24,6 +24,11 @@ ComputeSurfaceAreaToVolumeDirect::ComputeSurfaceAreaToVolumeDirect(DataStructure
 ComputeSurfaceAreaToVolumeDirect::~ComputeSurfaceAreaToVolumeDirect() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Computes surface-area-to-volume ratio using direct Z-Y-X iteration.
+ * In-core path: accumulates per-feature surface area from face-neighbor
+ * comparisons, then divides by voxel volume. Optionally computes sphericity.
+ */
 Result<> ComputeSurfaceAreaToVolumeDirect::operator()()
 {
   // Input Cell Data

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeSurfaceAreaToVolumeScanline.cpp

@@ -24,6 +24,11 @@ ComputeSurfaceAreaToVolumeScanline::ComputeSurfaceAreaToVolumeScanline(DataStruc
 ComputeSurfaceAreaToVolumeScanline::~ComputeSurfaceAreaToVolumeScanline() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Computes surface-area-to-volume ratio using chunk-sequential iteration.
+ * OOC path: iterates chunks in order via loadChunk/getChunkLowerBounds/getChunkUpperBounds.
+ * Same logic as ComputeSurfaceAreaToVolumeDirect.
+ */
 Result<> ComputeSurfaceAreaToVolumeScanline::operator()()
 {
   // Input Cell Data

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeSurfaceFeaturesDirect.cpp

@@ -196,6 +196,11 @@ ComputeSurfaceFeaturesDirect::ComputeSurfaceFeaturesDirect(DataStructure& dataSt
 ComputeSurfaceFeaturesDirect::~ComputeSurfaceFeaturesDirect() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Identifies surface features using direct Z-Y-X iteration.
+ * In-core path: delegates to findSurfaceFeatures3D or findSurfaceFeatures2D
+ * depending on geometry dimensionality.
+ */
 Result<> ComputeSurfaceFeaturesDirect::operator()()
 {
   const auto pMarkFeature0NeighborsValue = m_InputValues->MarkFeature0Neighbors;
@@ -223,7 +228,7 @@ Result<> ComputeSurfaceFeaturesDirect::operator()()
   }
   else
   {
-    MakeErrorResult(-1000, fmt::format("Image Geometry at path '{}' must be either 3D or 2D", pFeatureGeometryPathValue.toString()));
+    return MakeErrorResult(-1000, fmt::format("Image Geometry at path '{}' must be either 3D or 2D", pFeatureGeometryPathValue.toString()));
   }
 
   return {};

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeSurfaceFeaturesScanline.cpp

@@ -110,6 +110,11 @@ ComputeSurfaceFeaturesScanline::ComputeSurfaceFeaturesScanline(DataStructure& da
 ComputeSurfaceFeaturesScanline::~ComputeSurfaceFeaturesScanline() noexcept = default;
 
 // -----------------------------------------------------------------------------
+/**
+ * @brief Identifies surface features using chunk-sequential iteration.
+ * OOC path: iterates chunks in order, handling both 3D and 2D geometries
+ * with coordinate remapping. Same logic as ComputeSurfaceFeaturesDirect.
+ */
 Result<> ComputeSurfaceFeaturesScanline::operator()()
 {
   const auto pMarkFeature0NeighborsValue = m_InputValues->MarkFeature0Neighbors;
@@ -216,6 +221,10 @@ Result<> ComputeSurfaceFeaturesScanline::operator()()
                 surfaceFeatures[gNum] = 1;
               }
             }
+            else
+            {
+              return MakeErrorResult(-1000, fmt::format("Image Geometry at path '{}' must be either 3D or 2D", pFeatureGeometryPathValue.toString()));
+            }
           }
         }
       }