Sort vertices by a precomputed Y band instead of a fuzzy comparison

examples/regression_jbeam/README.md

@@ -0,0 +1,9 @@
+# Regression jbeam fixtures
+
+Small `.jbeam` files that exist purely to reproduce a specific bug for a
+regression test. Unlike `examples/jbeam/`, these are **not** written or
+vetted by the jbeam maintainer, not curated demo material, and
+not picked up by `jbeam-edit-dump-ast` (which only scans `examples/jbeam/`).
+Don't treat them as examples of good jbeam, and don't add to this
+directory unless a test genuinely needs a fixture that can't be built
+from what's already in the project.

examples/regression_jbeam/y-sorting-repro.jbeam

@@ -0,0 +1,37 @@
+{
+"testpart":{
+    "nodes":[
+        ["id", "posX", "posY", "posZ"],
+        // Synthetic regression-test fixture for issue #214, not vetted by
+        // the jbeam maintainer and not intended as a demo/example.
+        // Real node positions from a gen4-style body file, reduced to just
+        // the left side, kept because this spacing reproduces a real bug.
+        ["nl0", 0.953, -1.967, 0.122],
+        ["nl2", 0.92, -1.953, 0.439],
+        ["nl4", 0.78, -1.815, 0.719],
+        ["nl6", 1.036, -1.807, 0.125],
+        ["nl8", 0.998, -1.791, 0.473],
+        ["nl10", 0.944, -1.644, 0.72],
+        ["nl12", 0.823, -1.362, 0.841],
+        ["nl14", 0.952, -1.354, 0.772],
+        ["nl18", 0.971, -1.107, 0.628],
+        ["nl20", 0.841, -1.079, 0.885],
+        ["nl22", 0.964, -1.058, 0.106],
+        ["nl24", 0.975, -1.054, 0.424],
+        ["nl26", 0.038, -0.997, 0.943],
+        ["nl27", 0.616, -0.958, 0.933],
+        ["nl29", 0.808, -0.898, 0.923],
+        ["nl31", 0.038, -0.838, 0.949],
+        ["nl32", 0.593, -0.78, 0.937],
+        ["nl34", 0.835, -0.688, 0.91],
+        ["nl36", 0.975, -0.645, 0.631],
+        ["nl38", 0.972, -0.644, 0.106],
+    ],
+    "beams":[
+        ["id1:", "id2:"],
+    ],
+    "triangles":[
+        ["id1:", "id2:", "id3:"],
+    ],
+},
+}

src-extra/transformation/JbeamEdit/Transformation.hs

@@ -5,7 +5,7 @@
 import Data.Bool (bool)
 import Data.Foldable.Extra (notNull)
 import Data.Function (on)
 import Data.List (foldl', partition)
 import Data.List.NonEmpty (NonEmpty)
 import Data.List.NonEmpty qualified as NE
 import Data.Map (Map)
@@ -179,7 +179,6 @@
         ]
 
       brks = xGroupBreakpoints tfCfg
-      thr = ySortingThreshold tfCfg
 
       assignSupportNames = assignNames newNames brks SupportTree
 
@@ -194,12 +193,12 @@
                   ( SupportKey
                   , VertexTree
                       [sideComment SupportTree]
-                      ( snd
-                          . mapAccumL
-                            assignSupportNames
-                            M.empty
-                          . NE.sortBy (compareAV thr SupportTree)
-                          $ NE.map (uncurry updateSupportVertexName) vs
+                      ( let renamedVertices = NE.map (uncurry updateSupportVertexName) vs
+                            sorted = NE.sortBy (on compare $ vY . aVertex) renamedVertices
+                            (_, bandIndices) = mapAccumL (indexBand tfCfg) (0, firstY sorted) sorted
+                            bandSortedVertices = NE.map snd $ NE.sortBy (compareAV SupportTree) bandIndices
+                            (_, renamedVertices') = mapAccumL assignSupportNames M.empty bandSortedVertices
+                         in renamedVertices'
                       )
                   )
               )
@@ -331,19 +330,15 @@
 treesOrder = [LeftTree, MiddleTree, RightTree, SupportTree]
 
 compareAV
-  :: Scientific -> VertexTreeType -> AnnotatedVertex -> AnnotatedVertex -> Ordering
-compareAV thr treeType vertex1 vertex2 =
+  :: VertexTreeType -> (Int, AnnotatedVertex) -> (Int, AnnotatedVertex) -> Ordering
+compareAV treeType (band1, vertex1) (band2, vertex2) =
   let supportNameCompare =
         bool
           EQ
           (on compare (dropIndex . vName . aVertex) vertex1 vertex2)
           (treeType == SupportTree)
-      y1 = vY . aVertex $ vertex1
-      y2 = vY . aVertex $ vertex2
       compareZ = comparing (vZ . aVertex) vertex1 vertex2
-      compareY =
-        let yDiff = abs $ y1 - y2
-         in bool EQ (compare y1 y2) (yDiff > thr)
+      compareY = compare band1 band2
       compareX = on compare (vX . aVertex) vertex1 vertex2
    in mconcat
         [ supportNameCompare
@@ -397,18 +392,38 @@
       prefixMap' = M.insert cleanPrefix (lastIdx + 1) prefixMap
    in (prefixMap', av {aVertex = newVertex})
 
+-- | Y of the first vertex, to seed the first band on a vertex rather than on 0.
+firstY :: NonEmpty AnnotatedVertex -> Scientific
+firstY = vY . aVertex . NE.head
+
+indexBand
+  :: TransformationConfig
+  -> (Int, Scientific)
+  -> AnnotatedVertex
+  -> ((Int, Scientific), (Int, AnnotatedVertex))
+indexBand tfCfg (bandIndex, bandY) av =
+  let nodeY = vY (aVertex av)
+      distance = abs (nodeY - bandY)
+      thr = ySortingThreshold tfCfg
+   in if distance >= thr
+        then
+          ((bandIndex + 1, nodeY), (bandIndex + 1, av))
+        else
+          ((bandIndex, bandY), (bandIndex, av))
+
 sortVertices
   :: VertexTreeType
   -> UpdateNamesMap
   -> TransformationConfig
   -> VertexTree
   -> VertexTree
 sortVertices treeType newNames tfCfg (VertexTree comments vertices) =
-  let thr = ySortingThreshold tfCfg
-      brks = xGroupBreakpoints tfCfg
-      sortedGroups = NE.sortBy (compareAV thr treeType) vertices
-
-      renamedGroups = snd $ mapAccumL (assignNames newNames brks treeType) M.empty sortedGroups
+  let brks = xGroupBreakpoints tfCfg
+      sorted = NE.sortBy (on compare $ vY . aVertex) vertices
+      (_, bandIndices) = mapAccumL (indexBand tfCfg) (0, firstY sorted) sorted
+      bandSortedAnnotated = NE.map snd $ NE.sortBy (compareAV treeType) bandIndices
+      renamedGroups =
+        snd $ mapAccumL (assignNames newNames brks treeType) M.empty bandSortedAnnotated
    in VertexTree comments renamedGroups
 
 updateVerticesInNode

src-extra/transformation/JbeamEdit/Transformation/Config.hs

@@ -49,7 +49,7 @@ import Text.Read
 defaultSortingThreshold :: Scientific
 defaultSortingThreshold = 0.05
 
-defaultSupportThreshold :: Double
+defaultSupportThreshold :: Scientific
 defaultSupportThreshold = 96
 
 defaultMaxSupportCoordinates :: Natural
@@ -66,7 +66,7 @@ defaultBreakpoints =
 data TransformationConfig = TransformationConfig
   { ySortingThreshold :: Scientific
   , xGroupBreakpoints :: XGroupBreakpoints
-  , supportThreshold :: Double
+  , supportThreshold :: Scientific
   , maxSupportCoordinates :: Natural
   }
   deriving (Generic)
@@ -125,7 +125,7 @@ instance FromJSON XGroupBreakpoints where
           )
     pure $ XGroupBreakpoints lst
 
-parseSupportThreshold :: Object -> Parser Double
+parseSupportThreshold :: Object -> Parser Scientific
 parseSupportThreshold o = do
   thr <- o .: "support-threshold"
   when (thr < 1) failWithMessage $> thr

test-extra/transformation/Spec.hs

@@ -2,11 +2,16 @@ module Spec (
   main,
 ) where
 
+import Data.Char (isDigit)
 import Data.List (isPrefixOf, isSuffixOf)
 import Data.Map qualified as M
 import Data.Set qualified as S
+import Data.Text (Text)
 import Data.Text qualified as T
-import JbeamEdit.Core.Node (Node)
+import Data.Vector qualified as V
+import GHC.IsList (fromList)
+import JbeamEdit.Core.Node (Node (..), NumberValue (..), expectArray)
+import JbeamEdit.Core.NodePath qualified as NP
 import JbeamEdit.Formatting
 import JbeamEdit.IOUtils (tryReadFile)
 import JbeamEdit.Parsing.Jbeam (parseNodes)
@@ -85,6 +90,63 @@ beamValidationSpec = do
     it "has no duplicate beams" $
       findDuplicateBeams internalBeams `shouldBe` []
 
+nodesQuery :: NP.NodePath
+nodesQuery = fromList [NP.ObjectIndex 0, NP.ObjectKey "nodes"]
+
+{- | (name, Y position) for every vertex in a transformed top node's
+"nodes" section, in file order (i.e. the order `transform` actually
+wrote them out in). Read straight back out of the output rather than
+correlated against the input names, since `transform` renames vertices.
+-}
+vertexPositionsInOrder :: Node -> [(Text, Double)]
+vertexPositionsInOrder topNode =
+  case NP.queryNodes nodesQuery topNode >>= NP.expectArray nodesQuery of
+    Left _ -> []
+    Right rows ->
+      [ (name, realToFrac (nvValue yNum))
+      | row <- V.toList rows
+      , Just inner <- [expectArray row]
+      , Just (String name) <- [inner V.!? 0]
+      , name /= "id"
+      , Just (Number yNum) <- [inner V.!? 2]
+      ]
+
+{- | Real left-side structural node positions from a NASCAR gen4-style body
+file (see issue #214). This specific spacing reproduces a real transform
+run: with y-sorting-threshold 0.1, the frontmost node (nl0, Y=-1.967) ends
+up sorted to the back of its group instead of the front.
+-}
+ySortingReproFixture :: FilePath
+ySortingReproFixture = "examples/regression_jbeam/y-sorting-repro.jbeam"
+
+ySortingBandingSpec :: Spec
+ySortingBandingSpec =
+  describe "y-sorting-threshold"
+    . it
+      "never places a node behind another node that is more than the threshold further forward"
+    $ do
+      let thr = 0.1 :: Double
+          cfg = newTransformationConfig {ySortingThreshold = 0.1}
+      topNode <- parseJbeamFile ySortingReproFixture
+      case transform M.empty cfg topNode of
+        Left err -> expectationFailure ("transform failed: " ++ T.unpack err)
+        Right (_, _, _, resultNode) -> do
+          let positions = zip (vertexPositionsInOrder resultNode) [0 :: Int ..]
+              groupPrefix = T.dropWhileEnd isDigit
+              -- Only compare nodes within the same output group (e.g.
+              -- "nll", "nlm"): a Left-tree node and a Middle-tree node
+              -- are unrelated blocks in the file, not a single ordered
+              -- sequence, so their relative position isn't meaningful.
+              outOfOrder =
+                [ (y1, y2)
+                | ((n1, y1), i1) <- positions
+                , ((n2, y2), i2) <- positions
+                , i1 < i2
+                , groupPrefix n1 == groupPrefix n2
+                , y1 - y2 > thr
+                ]
+          outOfOrder `shouldBe` []
+
 main :: IO ()
 main = hspec $ do
   let exampleConfigPath = unsafeEncodeUtf "examples/jbeam-edit.yaml"
@@ -102,3 +164,4 @@ main = hspec $ do
   mapM_ (testInputFile "cfg-default" newTransformationConfig) inputFiles
   mapM_ (testInputFile "cfg-example" tfConfig) inputFiles
   beamValidationSpec
+  ySortingBandingSpec