From d9faec968ba612b1507303e33f6c42e539fcab77 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Thu, 20 Aug 2026 18:01:30 +0200 Subject: [PATCH 01/17] Add a failing test for X-column ordering inside a Y band The five frontmost nodes in the y-sorting fixture sit in two vertical columns that Y and Z cannot separate, so sorting a band by Z alone interleaves them. The fixture already had the positions, so it now serves both this and the y-sorting-threshold test. --- .../regression_jbeam/y-sorting-repro.jbeam | 12 +++++-- test-extra/transformation/Spec.hs | 1 + test-extra/transformation/Spec/Helpers.hs | 20 ++++++++++++ test-extra/transformation/Spec/Regression.hs | 32 +++++++++++++++++++ 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/examples/regression_jbeam/y-sorting-repro.jbeam b/examples/regression_jbeam/y-sorting-repro.jbeam index 2e3f2f13..87e9ec38 100644 --- a/examples/regression_jbeam/y-sorting-repro.jbeam +++ b/examples/regression_jbeam/y-sorting-repro.jbeam @@ -2,10 +2,16 @@ "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. + // Synthetic regression-test fixture, 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. + // the left side, kept because this spacing reproduces two real bugs. + // Issue #214: at y-sorting-threshold 0.1 the frontmost node sorts to + // the back of its group. + // X columns: the five frontmost nodes sit in two vertical columns + // that Y and Z cannot separate, so a band sorted by Z alone climbs + // one column, jumps to the other and comes back. + // Change these positions and both tests are measuring something else. ["nl0", 0.953, -1.967, 0.122], ["nl2", 0.92, -1.953, 0.439], ["nl4", 0.78, -1.815, 0.719], diff --git a/test-extra/transformation/Spec.hs b/test-extra/transformation/Spec.hs index ce7067c5..23b89f18 100644 --- a/test-extra/transformation/Spec.hs +++ b/test-extra/transformation/Spec.hs @@ -157,6 +157,7 @@ main = hspec $ do supportRenameIdempotencySpec letterEndingNodesSpec ySortingBandingSpec + xColumnSortingSpec metadataAcrossTreesSpec metadataPreservedSpec triangleMetadataSpec diff --git a/test-extra/transformation/Spec/Helpers.hs b/test-extra/transformation/Spec/Helpers.hs index 07ae87d4..2b8b944f 100644 --- a/test-extra/transformation/Spec/Helpers.hs +++ b/test-extra/transformation/Spec/Helpers.hs @@ -2,6 +2,7 @@ module Spec.Helpers ( parseJbeamFile, vertexPositionsInOrder, + vertexCoordinatesInOrder, vertexCoordinates, effectiveMetaByCoordinate, metaNumber, @@ -53,6 +54,25 @@ vertexPositionsInOrder topNode = , Just (Number yNum) <- [inner V.!? 2] ] +{- | Every vertex coordinate in a top node's "nodes" section, in the order +`transform` wrote them out. Use this where the defect is about which vertex +ended up where, rather than about which vertices survived. +-} +vertexCoordinatesInOrder :: Node -> [(Double, Double, Double)] +vertexCoordinatesInOrder topNode = + case NP.queryNodes nodesQuery topNode >>= NP.expectArray nodesQuery of + Left _ -> [] + Right rows -> + [ (realToFrac (nvValue x), realToFrac (nvValue y), realToFrac (nvValue z)) + | row <- V.toList rows + , Just inner <- [expectArray row] + , Just (String name) <- [inner V.!? 0] + , name /= "id" + , Just (Number x) <- [inner V.!? 1] + , Just (Number y) <- [inner V.!? 2] + , Just (Number z) <- [inner V.!? 3] + ] + {- | Every vertex coordinate in a top node's "nodes" section. Positions survive renaming, so they identify a vertex across a transform. -} diff --git a/test-extra/transformation/Spec/Regression.hs b/test-extra/transformation/Spec/Regression.hs index ff041164..01314f91 100644 --- a/test-extra/transformation/Spec/Regression.hs +++ b/test-extra/transformation/Spec/Regression.hs @@ -9,6 +9,7 @@ module Spec.Regression ( ySortingBandingSpec, metadataAcrossTreesSpec, metadataPreservedSpec, + xColumnSortingSpec, ) where import Data.Map qualified as M @@ -171,3 +172,34 @@ metadataPreservedSpec = ] M.keys metaAfter `shouldBe` M.keys metaBefore changed `shouldBe` [] + +{- | The same gen4-style left-side positions as the y-sorting fixture, read +for a different defect. Its five frontmost nodes sit in two vertical +columns: an inner one at X 0.780/0.920/0.953 and an outer one at X +0.998/1.036, the nose face and the fender beside it. Y and Z interleave +between the two columns, so no y-sorting-threshold can separate them; only +X can. Sorting a band by Z alone therefore climbs one column, jumps to the +other and comes back, which is what the jbeam maintainer marked up on his +render. + +The threshold here is 0.31 rather than the default because that is what puts +all five in one band, which is where the defect lives. It is not free choice: +between 0.153 and 0.16 the Y bands land on exactly the two columns, and this +assertion passes with nothing fixed at all. +-} +xColumnSortingSpec :: Spec +xColumnSortingSpec = + describe "vertices in one Y band but different X columns" + . it "keeps each column contiguous instead of interleaving them" + $ do + let cfg = newTransformationConfig {ySortingThreshold = 0.31} + inner = [(0.953, -1.967, 0.122), (0.92, -1.953, 0.439), (0.78, -1.815, 0.719)] + outer = [(1.036, -1.807, 0.125), (0.998, -1.791, 0.473)] + topNode <- parseJbeamFile ySortingReproFixture + case transform M.empty cfg topNode of + Left err -> expectationFailure ("transform failed: " ++ T.unpack err) + Right (_, _, _, resultNode) -> + take 5 (leftGroup (vertexCoordinatesInOrder resultNode)) + `shouldBe` inner ++ outer + where + leftGroup = filter (\(x, _, _) -> x >= 0.09) From db9744d8dbe5c2eb4960e40bdccfb2f54adf9682 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Thu, 20 Aug 2026 21:20:44 +0200 Subject: [PATCH 02/17] Added new configuration property --- .../JbeamEdit/Transformation/Config.hs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src-extra/transformation/JbeamEdit/Transformation/Config.hs b/src-extra/transformation/JbeamEdit/Transformation/Config.hs index 6579da3e..f58141eb 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/Config.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/Config.hs @@ -1,4 +1,5 @@ {-# LANGUAGE DeriveGeneric #-} +{-# LANGUAGE MultiWayIf #-} module JbeamEdit.Transformation.Config ( loadTransformationConfig, @@ -18,6 +19,7 @@ import Control.Monad (forM, when) import Data.Bifunctor (first) import Data.ByteString.Lazy qualified as LBS import Data.Functor (($>)) +import Data.Maybe (isJust, isNothing) import Data.Scientific (Scientific) import Data.Text (Text) import Data.Text qualified as T @@ -46,6 +48,9 @@ import Numeric.Natural (Natural) import System.OsPath import Text.Read +defaultXSortingThreshold :: Maybe Scientific +defaultXSortingThreshold = Nothing + defaultSortingThreshold :: Scientific defaultSortingThreshold = 0.05 @@ -65,6 +70,7 @@ defaultBreakpoints = data TransformationConfig = TransformationConfig { ySortingThreshold :: Scientific + , xSortingThreshold :: Maybe Scientific , xGroupBreakpoints :: XGroupBreakpoints , supportThreshold :: Scientific , maxSupportCoordinates :: Natural @@ -75,6 +81,7 @@ newTransformationConfig :: TransformationConfig newTransformationConfig = TransformationConfig defaultSortingThreshold + defaultXSortingThreshold defaultBreakpoints defaultSupportThreshold defaultMaxSupportCoordinates @@ -134,10 +141,25 @@ parseSupportThreshold o = do fail "'support-threshold' must be a percentage value of 1 or higher (e.g., 80 or 80.8). Values below 1 (e.g., 0.80) are not allowed." +parseXSortingThreshold :: Object -> Parser (Maybe Scientific) +parseXSortingThreshold o = do + thr <- o .:? "x-sorting-threshold" + let cleanThr = T.unpack . T.strip <$> thr + maybeValid = cleanThr >>= readMaybe + in if + | thr == Just "off" || isNothing thr -> pure defaultXSortingThreshold + | isJust maybeValid -> pure maybeValid + | True -> failWithMessage + where + failWithMessage = + fail + "TODO: proper error message" + instance FromJSON TransformationConfig where parseJSON = withObject "TransformationConfig" $ \o -> TransformationConfig <$> o .:? "y-sorting-threshold" .!= defaultSortingThreshold + <*> parseXSortingThreshold o <*> o .:? "x-group-breakpoints" .!= defaultBreakpoints <*> parseSupportThreshold o <*> o .:? "max-support-coordinates" .!= defaultMaxSupportCoordinates From 84b0b46230b1d1467223782598c4e0ad5e02b5e0 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:39:17 +0200 Subject: [PATCH 03/17] Set xSortingThreshold in the X-column test The key has no default, because no number means off: 0 gives every vertex its own band, which is the most X sorting rather than none. So the test has to ask for the pass explicitly, and the branch stops compiling until the field exists. --- test-extra/transformation/Spec/Regression.hs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test-extra/transformation/Spec/Regression.hs b/test-extra/transformation/Spec/Regression.hs index 01314f91..69425272 100644 --- a/test-extra/transformation/Spec/Regression.hs +++ b/test-extra/transformation/Spec/Regression.hs @@ -182,17 +182,26 @@ X can. Sorting a band by Z alone therefore climbs one column, jumps to the other and comes back, which is what the jbeam maintainer marked up on his render. -The threshold here is 0.31 rather than the default because that is what puts +The Y threshold here is 0.31 rather than the default because that is what puts all five in one band, which is where the defect lives. It is not free choice: between 0.153 and 0.16 the Y bands land on exactly the two columns, and this assertion passes with nothing fixed at all. + +`xSortingThreshold` has to be set explicitly because it has no default. There is +no number that means off (0 gives every vertex its own band, which is the most +X sorting rather than none), so the field is optional and absent means the pass +does not run at all. -} xColumnSortingSpec :: Spec xColumnSortingSpec = describe "vertices in one Y band but different X columns" . it "keeps each column contiguous instead of interleaving them" $ do - let cfg = newTransformationConfig {ySortingThreshold = 0.31} + let cfg = + newTransformationConfig + { ySortingThreshold = 0.31 + , xSortingThreshold = Just 0.2 + } inner = [(0.953, -1.967, 0.122), (0.92, -1.953, 0.439), (0.78, -1.815, 0.719)] outer = [(1.036, -1.807, 0.125), (0.998, -1.791, 0.473)] topNode <- parseJbeamFile ySortingReproFixture From 9e7481bf504190d5fb6951432f09c541a8443e30 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:22:58 +0200 Subject: [PATCH 04/17] Use sortVertices in moveSupportVertices --- .../JbeamEdit/Transformation.hs | 28 ++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index f3b8ff1d..64aee36e 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -186,34 +186,18 @@ moveSupportVertices protectedNames newNames tfCfg connMap vsPerType = , count >= thrCount ] - brks = xGroupBreakpoints tfCfg - - assignSupportNames = assignNames newNames brks SupportTree - vertexForest :: VertexForest vertexForest = case NE.nonEmpty supportVertices of Nothing -> M.empty Just vs -> - M.singleton - SupportTree - ( OMap1.singleton - ( SupportKey - , VertexTree - [sideComment SupportTree] - ( let prefix = vertexPrefix newNames brks SupportTree - sorted = NE.sortBy (on compare $ vY . aVertex) vs - (_, bandIndices) = mapAccumL (indexBand tfCfg) (0, firstY sorted) sorted - bandSortedPairs = - NE.sortBy - (compareAV prefix SupportTree) - bandIndices - bandSortedVertices = NE.map snd bandSortedPairs - (_, renamedVertices') = mapAccumL assignSupportNames M.empty bandSortedVertices - in renamedVertices' - ) + let supportTree = VertexTree [sideComment SupportTree] vs + sortedSupportTree = sortVertices SupportTree newNames tfCfg supportTree + in M.singleton + SupportTree + ( OMap1.singleton + (SupportKey, sortedSupportTree) ) - ) supportVertexNames = foldr (S.insert . anVertexName) S.empty supportVertices From 50981844b9387e2d2194eeb2686a95382ed4b882 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:50:58 +0200 Subject: [PATCH 05/17] Sort vertices into X columns inside each Y band Vertices that share a Y band but sit in different vertical columns were ordered by Z alone, so the output climbed one column, jumped to the other and came back. A second banding pass over X, run per Y band and enabled by the optional x-sorting-threshold, keeps each column contiguous. --- .../JbeamEdit/Transformation.hs | 49 +++++++++++++++---- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index 64aee36e..4033aa41 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -10,7 +10,7 @@ import Data.List.NonEmpty (NonEmpty) import Data.List.NonEmpty qualified as NE import Data.Map (Map) import Data.Map qualified as M -import Data.Maybe (fromMaybe, mapMaybe) +import Data.Maybe (fromMaybe, isJust, mapMaybe) import Data.Monoid.Extra (mwhen) import Data.Ord (Down (Down), comparing) import Data.Scientific (Scientific) @@ -420,21 +420,43 @@ assignNames newNames brks treeType prefixMap av = firstY :: NonEmpty AnnotatedVertex -> Scientific firstY = vY . aVertex . NE.head +firstX :: NonEmpty (Int, AnnotatedVertex) -> Scientific +firstX = vX . aVertex . snd . NE.head + indexBand - :: TransformationConfig + :: Scientific + -> (a -> Scientific) -> (Int, Scientific) - -> AnnotatedVertex - -> ((Int, Scientific), (Int, AnnotatedVertex)) -indexBand tfCfg (bandIndex, bandY) av = - let nodeY = vY (aVertex av) + -> a + -> ((Int, Scientific), (Int, a)) +indexBand thr f (bandIndex, bandY) av = + let nodeY = f av distance = abs (nodeY - bandY) - thr = ySortingThreshold tfCfg in if distance >= thr then ((bandIndex + 1, nodeY), (bandIndex + 1, av)) else ((bandIndex, bandY), (bandIndex, av)) +data Band = XBand Int Int | YBand Int deriving (Eq, Ord) + +columnBandVertices + :: TransformationConfig + -> NonEmpty (Int, AnnotatedVertex) + -> NonEmpty (Band, AnnotatedVertex) +columnBandVertices tfCfg vs = + let maybeThr = xSortingThreshold tfCfg + in case maybeThr of + Just thr -> + let bandGroups = NE.groupBy1 (on (==) fst) vs + assignColumnBand group = + let sorted = NE.sortBy (on compare $ vX . aVertex . snd) group + (_, bandIndices) = mapAccumL (indexBand thr (vX . aVertex . snd)) (0, firstX sorted) sorted + in NE.map (\(xBand, (yBand, vertex)) -> (XBand yBand xBand, vertex)) bandIndices + in sconcat $ NE.map assignColumnBand bandGroups + Nothing -> + NE.map (first YBand) vs + sortVertices :: VertexTreeType -> UpdateNamesMap @@ -443,13 +465,20 @@ sortVertices -> VertexTree sortVertices treeType newNames tfCfg (VertexTree comments vertices) = let brks = xGroupBreakpoints tfCfg + thr = ySortingThreshold tfCfg sorted = NE.sortBy (on compare $ vY . aVertex) vertices - (_, bandIndices) = mapAccumL (indexBand tfCfg) (0, firstY sorted) sorted + (_, bandIndices) = mapAccumL (indexBand thr (vY . aVertex)) (0, firstY sorted) sorted bandSortedAnnotated = + NE.sortBy (compareAV (vertexPrefix newNames brks treeType) treeType) bandIndices + columnBandIndices = columnBandVertices tfCfg bandSortedAnnotated + columnSortedAnnotated = NE.map snd $ - NE.sortBy (compareAV (vertexPrefix newNames brks treeType) treeType) bandIndices + NE.sortBy + (compareAV (vertexPrefix newNames brks treeType) treeType) + columnBandIndices renamedGroups = - snd $ mapAccumL (assignNames newNames brks treeType) M.empty bandSortedAnnotated + snd $ + mapAccumL (assignNames newNames brks treeType) M.empty columnSortedAnnotated in VertexTree comments renamedGroups updateVerticesInNode From f33cee0805fa35928de6a541279b26e58a602026 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 16:02:33 +0200 Subject: [PATCH 06/17] Cover the config parser, and fail on a numeric x-sorting-threshold A rejected config is not a loud failure: the loader prints the message and returns the defaults, so the run goes on to rewrite every neighbouring file with thresholds nobody asked for. That makes the parser worth pinning. The x-sorting-threshold spec is red. The key is read as text and then parsed, so it takes a quoted "0.2" and rejects a bare 0.2, which is the opposite of every key beside it. --- jbeam-edit.cabal | 1 + .../JbeamEdit/Transformation/Config.hs | 1 + test-extra/transformation/Spec.hs | 2 + test-extra/transformation/Spec/Config.hs | 70 +++++++++++++++++++ 4 files changed, 74 insertions(+) create mode 100644 test-extra/transformation/Spec/Config.hs diff --git a/jbeam-edit.cabal b/jbeam-edit.cabal index f3e0b62c..8c5b6dba 100644 --- a/jbeam-edit.cabal +++ b/jbeam-edit.cabal @@ -312,6 +312,7 @@ test-suite jbeam-edit-transformation-test main-is: Spec.hs hs-source-dirs: test-extra/transformation other-modules: + Spec.Config Spec.Helpers Spec.Regression Paths_jbeam_edit diff --git a/src-extra/transformation/JbeamEdit/Transformation/Config.hs b/src-extra/transformation/JbeamEdit/Transformation/Config.hs index f58141eb..042a26a5 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/Config.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/Config.hs @@ -3,6 +3,7 @@ module JbeamEdit.Transformation.Config ( loadTransformationConfig, + decodeConfig, transformationConfigFile, applyOperator, newTransformationConfig, diff --git a/test-extra/transformation/Spec.hs b/test-extra/transformation/Spec.hs index 23b89f18..201a0b7c 100644 --- a/test-extra/transformation/Spec.hs +++ b/test-extra/transformation/Spec.hs @@ -20,6 +20,7 @@ import JbeamEdit.Transformation.BeamExtraction (beamInKnownSet) import JbeamEdit.Transformation.BeamValidation import JbeamEdit.Transformation.Config import JbeamEdit.Transformation.Types (Beam) +import Spec.Config (configParsingSpec) import Spec.Helpers (parseJbeamFile) import Spec.Regression import System.Directory (getDirectoryContents) @@ -153,6 +154,7 @@ main = hspec $ do mapM_ (testInputFile "cfg-example" tfConfig) inputFiles mapM_ (testFixedPoint "cfg-default" newTransformationConfig) inputFiles mapM_ (testFixedPoint "cfg-example" tfConfig) inputFiles + configParsingSpec beamValidationSpec supportRenameIdempotencySpec letterEndingNodesSpec diff --git a/test-extra/transformation/Spec/Config.hs b/test-extra/transformation/Spec/Config.hs new file mode 100644 index 00000000..6abf9bf6 --- /dev/null +++ b/test-extra/transformation/Spec/Config.hs @@ -0,0 +1,70 @@ +{- | The config parser, read through `decodeConfig` because that is the entry +point `loadTransformationConfig` uses. A rejected config is not a loud +failure: the loader prints the message and returns the defaults, so the run +goes on to rewrite every neighbouring file with thresholds the user never +asked for. That is what makes these worth testing at all. +-} +module Spec.Config ( + configParsingSpec, +) where + +import Data.ByteString.Lazy qualified as LBS +import Data.Either (isLeft) +import Data.Text (Text) +import Data.Text qualified as T +import Data.Text.Encoding (encodeUtf8) +import JbeamEdit.Transformation.Config +import Test.Hspec + +parseField :: (TransformationConfig -> a) -> Text -> Either Text a +parseField field = + fmap field . decodeConfig . LBS.fromStrict . encodeUtf8 + +{- | Only the keys that predate the column pass. Leaving +`x-sorting-threshold` out keeps this source parseable, so the spec below +fails only for the reason it names. + +`support-threshold` is carried by every source here because it is the one +key the parser demands. +-} +olderThresholds :: Text +olderThresholds = + T.unlines + [ "y-sorting-threshold: 0.04" + , "support-threshold: 20" + , "max-support-coordinates: 3" + ] + +configParsingSpec :: Spec +configParsingSpec = describe "the transformation config parser" $ do + it "reads a bare YAML number for the keys that have always taken one" $ do + parseField ySortingThreshold olderThresholds `shouldBe` Right 0.04 + parseField supportThreshold olderThresholds `shouldBe` Right 20 + parseField maxSupportCoordinates olderThresholds `shouldBe` Right 3 + + it "reads a bare YAML number for x-sorting-threshold" $ + parseField xSortingThreshold (olderThresholds <> "x-sorting-threshold: 0.2\n") + `shouldBe` Right (Just 0.2) + + it "reads a config that leaves support-threshold out" $ do + pendingWith + "support-threshold is the only key read with .: rather than .:?, so a \ + \config that omits it is rejected outright and the loader falls back to \ + \every default. Whether that is meant to be a required key has not been \ + \decided." + parseField supportThreshold "y-sorting-threshold: 0.04\n" + `shouldBe` Right defaultSupportThreshold + + it "leaves the column pass off when x-sorting-threshold is absent" $ + parseField xSortingThreshold "support-threshold: 20\n" `shouldBe` Right Nothing + + it "accepts a support-threshold of exactly 1" $ + parseField supportThreshold "support-threshold: 1\n" `shouldBe` Right 1 + + it "rejects a support-threshold below 1" $ + parseField supportThreshold "support-threshold: 0.8\n" `shouldSatisfy` isLeft + + it "gives every default for an empty file" $ do + parseField ySortingThreshold "" `shouldBe` Right defaultSortingThreshold + parseField supportThreshold "" `shouldBe` Right defaultSupportThreshold + parseField xSortingThreshold "" `shouldBe` Right Nothing From db39cf441835082bc2207275e6929a60d99d6e69 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 16:31:27 +0200 Subject: [PATCH 07/17] Fixed the transformation config parser --- .../JbeamEdit/Transformation/Config.hs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation/Config.hs b/src-extra/transformation/JbeamEdit/Transformation/Config.hs index 042a26a5..5e23bbcd 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/Config.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/Config.hs @@ -24,10 +24,11 @@ import Data.Maybe (isJust, isNothing) import Data.Scientific (Scientific) import Data.Text (Text) import Data.Text qualified as T -import Data.Yaml ( +import Data.Yaml qualified as Y ( Object, ParseException (..), Parser, + Value (..), decodeEither', prettyPrintParseException, ) @@ -133,7 +134,7 @@ instance FromJSON XGroupBreakpoints where ) pure $ XGroupBreakpoints lst -parseSupportThreshold :: Object -> Parser Scientific +parseSupportThreshold :: Y.Object -> Y.Parser Scientific parseSupportThreshold o = do thr <- o .: "support-threshold" when (thr < 1) failWithMessage $> thr @@ -142,19 +143,18 @@ parseSupportThreshold o = do fail "'support-threshold' must be a percentage value of 1 or higher (e.g., 80 or 80.8). Values below 1 (e.g., 0.80) are not allowed." -parseXSortingThreshold :: Object -> Parser (Maybe Scientific) +parseXSortingThreshold :: Y.Object -> Y.Parser (Maybe Scientific) parseXSortingThreshold o = do thr <- o .:? "x-sorting-threshold" - let cleanThr = T.unpack . T.strip <$> thr - maybeValid = cleanThr >>= readMaybe - in if - | thr == Just "off" || isNothing thr -> pure defaultXSortingThreshold - | isJust maybeValid -> pure maybeValid - | True -> failWithMessage + case thr of + Nothing -> pure defaultXSortingThreshold + Just Y.Null -> pure defaultXSortingThreshold + Just (Y.Number number) -> pure (Just number) + Just (Y.String "off") -> pure defaultXSortingThreshold + val -> failWithMessage val where - failWithMessage = - fail - "TODO: proper error message" + failWithMessage val = + fail ("'x-sorting-threshold' set to unsupported value " ++ show val) instance FromJSON TransformationConfig where parseJSON = withObject "TransformationConfig" $ \o -> @@ -165,9 +165,9 @@ instance FromJSON TransformationConfig where <*> parseSupportThreshold o <*> o .:? "max-support-coordinates" .!= defaultMaxSupportCoordinates -formatParseError :: ParseException -> String -formatParseError (AesonException err) = err -formatParseError excp = prettyPrintParseException excp +formatParseError :: Y.ParseException -> String +formatParseError (Y.AesonException err) = err +formatParseError excp = Y.prettyPrintParseException excp transformationConfigFile :: OsPath transformationConfigFile = unsafeEncodeUtf ".jbeam-edit.yaml" @@ -177,7 +177,7 @@ decodeConfig "" = Right newTransformationConfig decodeConfig content = first (T.pack . formatParseError) - (decodeEither' $ LBS.toStrict content) + (Y.decodeEither' $ LBS.toStrict content) loadTransformationConfig :: OsPath -> IO TransformationConfig loadTransformationConfig filename = do From 01f5989df7b698c53791d92a36adfe7437875e50 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 16:48:05 +0200 Subject: [PATCH 08/17] Made Data.Yaml unqualified again --- .../JbeamEdit/Transformation/Config.hs | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation/Config.hs b/src-extra/transformation/JbeamEdit/Transformation/Config.hs index 5e23bbcd..9940ad57 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/Config.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/Config.hs @@ -1,5 +1,4 @@ {-# LANGUAGE DeriveGeneric #-} -{-# LANGUAGE MultiWayIf #-} module JbeamEdit.Transformation.Config ( loadTransformationConfig, @@ -20,11 +19,10 @@ import Control.Monad (forM, when) import Data.Bifunctor (first) import Data.ByteString.Lazy qualified as LBS import Data.Functor (($>)) -import Data.Maybe (isJust, isNothing) import Data.Scientific (Scientific) import Data.Text (Text) import Data.Text qualified as T -import Data.Yaml qualified as Y ( +import Data.Yaml ( Object, ParseException (..), Parser, @@ -48,7 +46,7 @@ import JbeamEdit.IOUtils import JbeamEdit.Transformation.Types (VertexTreeType (..)) import Numeric.Natural (Natural) import System.OsPath -import Text.Read +import Text.Read(readMaybe) defaultXSortingThreshold :: Maybe Scientific defaultXSortingThreshold = Nothing @@ -134,7 +132,7 @@ instance FromJSON XGroupBreakpoints where ) pure $ XGroupBreakpoints lst -parseSupportThreshold :: Y.Object -> Y.Parser Scientific +parseSupportThreshold :: Object -> Parser Scientific parseSupportThreshold o = do thr <- o .: "support-threshold" when (thr < 1) failWithMessage $> thr @@ -143,14 +141,14 @@ parseSupportThreshold o = do fail "'support-threshold' must be a percentage value of 1 or higher (e.g., 80 or 80.8). Values below 1 (e.g., 0.80) are not allowed." -parseXSortingThreshold :: Y.Object -> Y.Parser (Maybe Scientific) +parseXSortingThreshold :: Object -> Parser (Maybe Scientific) parseXSortingThreshold o = do thr <- o .:? "x-sorting-threshold" case thr of Nothing -> pure defaultXSortingThreshold - Just Y.Null -> pure defaultXSortingThreshold - Just (Y.Number number) -> pure (Just number) - Just (Y.String "off") -> pure defaultXSortingThreshold + Just Null -> pure defaultXSortingThreshold + Just (Number number) -> pure (Just number) + Just (String "off") -> pure defaultXSortingThreshold val -> failWithMessage val where failWithMessage val = @@ -165,9 +163,9 @@ instance FromJSON TransformationConfig where <*> parseSupportThreshold o <*> o .:? "max-support-coordinates" .!= defaultMaxSupportCoordinates -formatParseError :: Y.ParseException -> String -formatParseError (Y.AesonException err) = err -formatParseError excp = Y.prettyPrintParseException excp +formatParseError :: ParseException -> String +formatParseError (AesonException err) = err +formatParseError excp = prettyPrintParseException excp transformationConfigFile :: OsPath transformationConfigFile = unsafeEncodeUtf ".jbeam-edit.yaml" @@ -177,7 +175,7 @@ decodeConfig "" = Right newTransformationConfig decodeConfig content = first (T.pack . formatParseError) - (Y.decodeEither' $ LBS.toStrict content) + (decodeEither' $ LBS.toStrict content) loadTransformationConfig :: OsPath -> IO TransformationConfig loadTransformationConfig filename = do From cec548d737861bc6baee8de130cdd2f9f8f1a072 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:00:20 +0200 Subject: [PATCH 09/17] Accept an unquoted off for x-sorting-threshold YAML resolves a bare off to a boolean, so the string case alone caught only the quoted spelling and the spelling people actually write was rejected. The failure now names what the key takes instead of showing the parsed value, and narrowing the Text.Read import removes the name clash that forced Data.Yaml to be qualified. --- .../JbeamEdit/Transformation/Config.hs | 15 ++++++++++----- test-extra/transformation/Spec/Config.hs | 12 ++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation/Config.hs b/src-extra/transformation/JbeamEdit/Transformation/Config.hs index 9940ad57..354c61f5 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/Config.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/Config.hs @@ -46,7 +46,7 @@ import JbeamEdit.IOUtils import JbeamEdit.Transformation.Types (VertexTreeType (..)) import Numeric.Natural (Natural) import System.OsPath -import Text.Read(readMaybe) +import Text.Read (readMaybe) defaultXSortingThreshold :: Maybe Scientific defaultXSortingThreshold = Nothing @@ -141,18 +141,23 @@ parseSupportThreshold o = do fail "'support-threshold' must be a percentage value of 1 or higher (e.g., 80 or 80.8). Values below 1 (e.g., 0.80) are not allowed." +{- | Unquoted 'off' reaches this as a boolean, because that is how YAML +resolves it, so the string case alone would only catch the quoted spelling. +-} parseXSortingThreshold :: Object -> Parser (Maybe Scientific) parseXSortingThreshold o = do thr <- o .:? "x-sorting-threshold" case thr of Nothing -> pure defaultXSortingThreshold Just Null -> pure defaultXSortingThreshold - Just (Number number) -> pure (Just number) + Just (Bool False) -> pure defaultXSortingThreshold Just (String "off") -> pure defaultXSortingThreshold - val -> failWithMessage val + Just (Number number) -> pure (Just number) + Just _ -> failWithMessage where - failWithMessage val = - fail ("'x-sorting-threshold' set to unsupported value " ++ show val) + failWithMessage = + fail + "'x-sorting-threshold' must be a distance in meters (e.g., 0.2 for 20 cm), or 'off' to leave the column sorting out. Omitting the key does the same as 'off'." instance FromJSON TransformationConfig where parseJSON = withObject "TransformationConfig" $ \o -> diff --git a/test-extra/transformation/Spec/Config.hs b/test-extra/transformation/Spec/Config.hs index 6abf9bf6..af7519a3 100644 --- a/test-extra/transformation/Spec/Config.hs +++ b/test-extra/transformation/Spec/Config.hs @@ -58,6 +58,18 @@ configParsingSpec = describe "the transformation config parser" $ do it "leaves the column pass off when x-sorting-threshold is absent" $ parseField xSortingThreshold "support-threshold: 20\n" `shouldBe` Right Nothing + it "reads off as leaving the column pass out, quoted or not" $ do + parseField xSortingThreshold (olderThresholds <> "x-sorting-threshold: off\n") + `shouldBe` Right Nothing + parseField + xSortingThreshold + (olderThresholds <> "x-sorting-threshold: \"off\"\n") + `shouldBe` Right Nothing + + it "rejects an x-sorting-threshold that is neither a number nor off" $ + parseField xSortingThreshold (olderThresholds <> "x-sorting-threshold: soon\n") + `shouldSatisfy` isLeft + it "accepts a support-threshold of exactly 1" $ parseField supportThreshold "support-threshold: 1\n" `shouldBe` Right 1 From 09e3faafdf8c3ef4d5a7992b27d6b54c9c341205 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:00:20 +0200 Subject: [PATCH 10/17] Band each Y band once, and cover the second transform The column pass ran on a list already sorted by compareAV, which orders metadata ahead of the band index, so one Y band could reach it as two runs and be walked twice. It now runs straight off the Y pass, where equal band indices are always adjacent, and the sort that only existed to feed it is gone. A second spec transforms the output again, which nothing covered. --- .../JbeamEdit/Transformation.hs | 6 ++-- test-extra/transformation/Spec/Regression.hs | 36 +++++++++++-------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index 4033aa41..c8493d28 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -10,7 +10,7 @@ import Data.List.NonEmpty (NonEmpty) import Data.List.NonEmpty qualified as NE import Data.Map (Map) import Data.Map qualified as M -import Data.Maybe (fromMaybe, isJust, mapMaybe) +import Data.Maybe (fromMaybe, mapMaybe) import Data.Monoid.Extra (mwhen) import Data.Ord (Down (Down), comparing) import Data.Scientific (Scientific) @@ -468,9 +468,7 @@ sortVertices treeType newNames tfCfg (VertexTree comments vertices) = thr = ySortingThreshold tfCfg sorted = NE.sortBy (on compare $ vY . aVertex) vertices (_, bandIndices) = mapAccumL (indexBand thr (vY . aVertex)) (0, firstY sorted) sorted - bandSortedAnnotated = - NE.sortBy (compareAV (vertexPrefix newNames brks treeType) treeType) bandIndices - columnBandIndices = columnBandVertices tfCfg bandSortedAnnotated + columnBandIndices = columnBandVertices tfCfg bandIndices columnSortedAnnotated = NE.map snd $ NE.sortBy diff --git a/test-extra/transformation/Spec/Regression.hs b/test-extra/transformation/Spec/Regression.hs index 69425272..71aaf418 100644 --- a/test-extra/transformation/Spec/Regression.hs +++ b/test-extra/transformation/Spec/Regression.hs @@ -194,21 +194,29 @@ does not run at all. -} xColumnSortingSpec :: Spec xColumnSortingSpec = - describe "vertices in one Y band but different X columns" - . it "keeps each column contiguous instead of interleaving them" - $ do - let cfg = - newTransformationConfig - { ySortingThreshold = 0.31 - , xSortingThreshold = Just 0.2 - } - inner = [(0.953, -1.967, 0.122), (0.92, -1.953, 0.439), (0.78, -1.815, 0.719)] + describe "vertices in one Y band but different X columns" $ do + it "keeps each column contiguous instead of interleaving them" $ do + let inner = [(0.953, -1.967, 0.122), (0.92, -1.953, 0.439), (0.78, -1.815, 0.719)] outer = [(1.036, -1.807, 0.125), (0.998, -1.791, 0.473)] topNode <- parseJbeamFile ySortingReproFixture - case transform M.empty cfg topNode of - Left err -> expectationFailure ("transform failed: " ++ T.unpack err) - Right (_, _, _, resultNode) -> - take 5 (leftGroup (vertexCoordinatesInOrder resultNode)) - `shouldBe` inner ++ outer + withColumnSorting topNode $ \resultNode -> + take 5 (leftGroup (vertexCoordinatesInOrder resultNode)) + `shouldBe` inner ++ outer + + it "is a fixed point: a second transform moves nothing further" $ do + topNode <- parseJbeamFile ySortingReproFixture + withColumnSorting topNode $ \resultNode -> + withColumnSorting resultNode $ \againNode -> + vertexCoordinatesInOrder againNode + `shouldBe` vertexCoordinatesInOrder resultNode where leftGroup = filter (\(x, _, _) -> x >= 0.09) + columnSortingConfig = + newTransformationConfig + { ySortingThreshold = 0.31 + , xSortingThreshold = Just 0.2 + } + withColumnSorting node assert = + case transform M.empty columnSortingConfig node of + Left err -> expectationFailure ("transform failed: " ++ T.unpack err) + Right (_, _, _, resultNode) -> assert resultNode From a809d8069af904bbba57dc42a774cb3a41902ed4 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:00:20 +0200 Subject: [PATCH 11/17] Document x-sorting-threshold The key was missing from the parameter table and the sorting description still named three coordinates. --- TRANSFORMATION_DOCS.md | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/TRANSFORMATION_DOCS.md b/TRANSFORMATION_DOCS.md index 773f27fe..253506d5 100644 --- a/TRANSFORMATION_DOCS.md +++ b/TRANSFORMATION_DOCS.md @@ -38,6 +38,7 @@ Transformation reads `.jbeam-edit.yaml` in the working directory if present. Wit ```yaml y-sorting-threshold: 0.05 +x-sorting-threshold: off support-threshold: 96 max-support-coordinates: 3 @@ -52,12 +53,17 @@ x-group-breakpoints: Parameter reference: -| Key | Default | Description | -|--------------------------|---------|--------------------------------------------------------------------------------| -| `y-sorting-threshold` | 0.05 | Y distance (meters) below which two nodes are treated as the same depth band | -| `support-threshold` | 96 | Minimum beam count as a percentage of group size to classify a node as support | -| `max-support-coordinates`| 3 | Maximum number of support node candidates examined per spatial group | -| `x-group-breakpoints` | (above) | Rules that map X coordinate to Left, Middle, or Right | +| Key | Default | Description | +|---------------------------|---------|---------------------------------------------------------------------------------| +| `y-sorting-threshold` | 0.05 | Y distance (meters) below which two nodes are treated as the same depth band | +| `x-sorting-threshold` | off | X distance (meters) below which two nodes in one Y band count as the same column | +| `support-threshold` | 96 | Minimum beam count as a percentage of group size to classify a node as support | +| `max-support-coordinates` | 3 | Maximum number of support node candidates examined per spatial group | +| `x-group-breakpoints` | (above) | Rules that map X coordinate to Left, Middle, or Right | + +`x-sorting-threshold` is off unless you set it, and `off` is the only word it +accepts besides a distance. There is no number that turns it off: `0` gives +every node its own column, which is the most column sorting rather than none. Three ways this file can fail without saying much: @@ -160,6 +166,8 @@ Nodes within each group are sorted by three coordinates in order: A band starts at the node that opened it, not at the previous node, so a long run of small steps cannot chain into one band much wider than the threshold. +With `x-sorting-threshold` set, each Y band is banded a second time before step 2, by X and by the same opener rule. Nodes in the same Y band but different columns then come out one column at a time, lower node first within each. Without it a band that spans two columns is ordered by height alone, so the output climbs one column, jumps to the other and comes back. That is what the setting is for: on a body panel the nose face and the fender beside it can sit at heights that interleave, and no Y threshold separates them because they are at the same depth. + The threshold is there because nodes you placed at one depth are rarely at exactly the same Y. Without it, a millimetre of difference would order them by Y instead of by height, and the two sides of a symmetric vehicle would come out in different orders. --- From 7c3a253fc992e5ae49cac78951a70e5f6a15927b Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:17:08 +0200 Subject: [PATCH 12/17] Refactored Band type --- .../JbeamEdit/Transformation.hs | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index c8493d28..39117184 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -438,24 +438,17 @@ indexBand thr f (bandIndex, bandY) av = else ((bandIndex, bandY), (bandIndex, av)) -data Band = XBand Int Int | YBand Int deriving (Eq, Ord) - columnBandVertices - :: TransformationConfig + :: Scientific -> NonEmpty (Int, AnnotatedVertex) - -> NonEmpty (Band, AnnotatedVertex) -columnBandVertices tfCfg vs = - let maybeThr = xSortingThreshold tfCfg - in case maybeThr of - Just thr -> - let bandGroups = NE.groupBy1 (on (==) fst) vs - assignColumnBand group = - let sorted = NE.sortBy (on compare $ vX . aVertex . snd) group - (_, bandIndices) = mapAccumL (indexBand thr (vX . aVertex . snd)) (0, firstX sorted) sorted - in NE.map (\(xBand, (yBand, vertex)) -> (XBand yBand xBand, vertex)) bandIndices - in sconcat $ NE.map assignColumnBand bandGroups - Nothing -> - NE.map (first YBand) vs + -> NonEmpty ((Int, Int), AnnotatedVertex) +columnBandVertices thr vs = + let bandGroups = NE.groupBy1 (on (==) fst) vs + assignColumnBand group = + let sorted = NE.sortBy (on compare $ vX . aVertex . snd) group + (_, bandIndices) = mapAccumL (indexBand thr (vX . aVertex . snd)) (0, firstX sorted) sorted + in NE.map (\(xBand, (yBand, vertex)) -> ((yBand, xBand), vertex)) bandIndices + in sconcat $ NE.map assignColumnBand bandGroups sortVertices :: VertexTreeType @@ -468,15 +461,23 @@ sortVertices treeType newNames tfCfg (VertexTree comments vertices) = thr = ySortingThreshold tfCfg sorted = NE.sortBy (on compare $ vY . aVertex) vertices (_, bandIndices) = mapAccumL (indexBand thr (vY . aVertex)) (0, firstY sorted) sorted - columnBandIndices = columnBandVertices tfCfg bandIndices - columnSortedAnnotated = - NE.map snd $ - NE.sortBy - (compareAV (vertexPrefix newNames brks treeType) treeType) - columnBandIndices + + renameVertices + :: Ord a => NonEmpty (a, AnnotatedVertex) -> NonEmpty AnnotatedVertex + renameVertices banded = + let columnSortedAnnotated = + NE.map snd $ + NE.sortBy + (compareAV (vertexPrefix newNames brks treeType) treeType) + banded + in snd $ + mapAccumL (assignNames newNames brks treeType) M.empty columnSortedAnnotated + + renamedGroups = - snd $ - mapAccumL (assignNames newNames brks treeType) M.empty columnSortedAnnotated + case xSortingThreshold tfCfg of + Nothing -> renameVertices bandIndices + Just xThr -> renameVertices (columnBandVertices xThr bandIndices) in VertexTree comments renamedGroups updateVerticesInNode From 2acd5823b607d531c28da2d89cbb49f393d967bd Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:14:38 +0200 Subject: [PATCH 13/17] Link the example config instead of quoting one The block in the docs was a second copy of a file that already exists, and it had already drifted: it showed the default support-threshold while the example file is tuned. The linked file is the one the test suite runs. --- TRANSFORMATION_DOCS.md | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/TRANSFORMATION_DOCS.md b/TRANSFORMATION_DOCS.md index 253506d5..7aa0c60c 100644 --- a/TRANSFORMATION_DOCS.md +++ b/TRANSFORMATION_DOCS.md @@ -34,22 +34,9 @@ This tells the tool that when it would generate a prefix derived from `rl_f`, us ## Configuration -Transformation reads `.jbeam-edit.yaml` in the working directory if present. Without it, built-in defaults are used. Create this file to override any parameter: - -```yaml -y-sorting-threshold: 0.05 -x-sorting-threshold: off -support-threshold: 96 -max-support-coordinates: 3 - -x-group-breakpoints: - - breakpoint: ">= 0.09" - vertex-type: LeftTree - - breakpoint: "> -0.09" - vertex-type: MiddleTree - - breakpoint: "<= -0.09" - vertex-type: RightTree -``` +Transformation reads `.jbeam-edit.yaml` in the working directory if present. Without it, built-in defaults are used. Create this file to override any parameter. + +[`examples/jbeam-edit.yaml`](examples/jbeam-edit.yaml) is a working one to copy. It is the config the test suite transforms every example file with, so it cannot drift out of date the way a block quoted here would. Parameter reference: @@ -59,7 +46,7 @@ Parameter reference: | `x-sorting-threshold` | off | X distance (meters) below which two nodes in one Y band count as the same column | | `support-threshold` | 96 | Minimum beam count as a percentage of group size to classify a node as support | | `max-support-coordinates` | 3 | Maximum number of support node candidates examined per spatial group | -| `x-group-breakpoints` | (above) | Rules that map X coordinate to Left, Middle, or Right | +| `x-group-breakpoints` | ±0.09 | Rules that map X coordinate to Left, Middle, or Right (see Left, Middle, Right) | `x-sorting-threshold` is off unless you set it, and `off` is the only word it accepts besides a distance. There is no number that turns it off: `0` gives From abb34cda3cf27f3bb580d1dce43868752803792f Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:29:52 +0200 Subject: [PATCH 14/17] Explain how to pick the two sorting thresholds Both settings were a bare number in a table with nothing to derive it from. The new section says what each one means in the file, works an example through the regression fixture, and lists what to change when the result still looks wrong. --- TRANSFORMATION_DOCS.md | 102 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 96 insertions(+), 6 deletions(-) diff --git a/TRANSFORMATION_DOCS.md b/TRANSFORMATION_DOCS.md index 7aa0c60c..a2b4e71e 100644 --- a/TRANSFORMATION_DOCS.md +++ b/TRANSFORMATION_DOCS.md @@ -40,18 +40,108 @@ Transformation reads `.jbeam-edit.yaml` in the working directory if present. Wit Parameter reference: -| Key | Default | Description | -|---------------------------|---------|---------------------------------------------------------------------------------| -| `y-sorting-threshold` | 0.05 | Y distance (meters) below which two nodes are treated as the same depth band | +| Key | Default | Description | +|---------------------------|---------|----------------------------------------------------------------------------------| +| `y-sorting-threshold` | 0.05 | Y distance (meters) below which two nodes are treated as the same depth band | | `x-sorting-threshold` | off | X distance (meters) below which two nodes in one Y band count as the same column | -| `support-threshold` | 96 | Minimum beam count as a percentage of group size to classify a node as support | -| `max-support-coordinates` | 3 | Maximum number of support node candidates examined per spatial group | -| `x-group-breakpoints` | ±0.09 | Rules that map X coordinate to Left, Middle, or Right (see Left, Middle, Right) | +| `support-threshold` | 96 | Minimum beam count as a percentage of group size to classify a node as support | +| `max-support-coordinates` | 3 | Maximum number of support node candidates examined per spatial group | +| `x-group-breakpoints` | ±0.09 | Rules that map X coordinate to Left, Middle, or Right (see Left, Middle, Right) | `x-sorting-threshold` is off unless you set it, and `off` is the only word it accepts besides a distance. There is no number that turns it off: `0` gives every node its own column, which is the most column sorting rather than none. +### Picking the two sorting thresholds + +Both settings are distances in meters, so `0.05` means 5 cm. + +They also work the same way, and it is worth knowing how. The tool goes through +the nodes in order and starts a new group as soon as a node is at least the +threshold away from **the first node of the group it is currently filling**. It +does not compare each node to the one right before it. That is deliberate: a +long gentle slope would otherwise chain together into one enormous group. + +**Start with `y-sorting-threshold`.** It decides how much front to back +variation still counts as the same row of nodes. When you place a row across a +panel the nodes are never at exactly the same Y, and without a threshold a +millimetre of difference would decide the order, which would also make the two +sides of the car come out differently. + +The number you want is the depth of one row, not the space between rows. The +default 5 cm suits a row you placed carefully on a flat face. A curved panel +needs more, because the row follows the curve. + +[`examples/regression_jbeam/y-sorting-repro.jbeam`](examples/regression_jbeam/y-sorting-repro.jbeam) +shows what that looks like. Its five frontmost left side nodes are one row as +far as the modeller is concerned, but they cover a fair bit of depth: + +| Measurement | Value | +|-----------------------------------------------|-------| +| Depth of the front row, -1.967 back to -1.791 | 0.176 | +| Front row's first node back to the next row's | 0.323 | +| Space between the two rows, -1.791 to -1.644 | 0.147 | +| Biggest step inside the front row | 0.138 | + +Anything above 0.176 and up to 0.323 keeps that row together, so 0.31 is a +comfortable pick and it is what the regression test uses. The default 0.05 +splits the row in two. Note the trap again: the space between the rows is 0.147, +barely more than the 0.138 step inside the row, so a threshold picked from the +space between rows lands in the wrong place. + +| What you see | What to do | +|---------------------------------------------|----------------------------------------------| +| One row comes out ordered front to back | Raise it, the row is deeper than you thought | +| Nodes at clearly different depths are mixed | Lower it | + +**Then `x-sorting-threshold`.** It does the same thing sideways, inside each +row. Turn it on if the file zigzags: the heights climb, drop back down and climb +again. That happens when one row covers two vertical columns of nodes, say an +outer face and the one set back beside it, and the tool has nothing but height +to go on. + +Here is the part that trips people up. **The number you want is the width of a +column, not the space between the columns.** Because the tool measures from the +first node of a group, the threshold has to be a bit wider than your widest +column, and no wider than the step from one column's innermost node across to +the next column's innermost node. + +Those same five nodes show it. Once they are in one row, the inner column sits +at X 0.780, 0.920 and 0.953, and the outer one at 0.998 and 1.036: + +| Measurement | Value | +|------------------------------------------------|-------| +| Width of the inner column, 0.780 out to 0.953 | 0.173 | +| Inner column's innermost across to the outer's | 0.218 | +| Space between the two columns, 0.953 to 0.998 | 0.045 | +| Biggest step inside the inner column | 0.140 | + +So anything above 0.173 and up to 0.218 does the job, and 0.2 is the obvious +pick. Notice that the space between the columns is only 0.045, smaller than a +step inside the inner column. Set 0.045 and the tool splits the inner column in +two, leaves 0.780 on its own, and you get a third wrong order rather than the +right one. The space between the columns is the number that looks right, so it +is worth measuring the column itself instead. + +To find the number for your own vehicle, transform once with the setting off, +find a spot where the heights zigzag, and read the X values of those rows. The +two columns separate by eye. Take the first and last X of the wider column, +subtract, and pick something a little above that. Transform again and the zigzag +should be gone. + +If it still looks wrong: + +| What you see | What to do | +|--------------------------------------------|----------------------------------------------| +| Nothing changed at all | The threshold is too big, try a smaller one | +| Runs of one or two nodes, still zigzagging | The threshold is too small, try a bigger one | +| Still zigzagging whatever you set | Raise `y-sorting-threshold` first, see below | + +That last one is worth checking early, because no X value can fix it. Columns +only exist inside a row, so if your two columns sit further apart front to back +than `y-sorting-threshold` allows, they never end up in the same row and +`x-sorting-threshold` never gets to look at them together. + Three ways this file can fail without saying much: The file is read from the directory you run the command in, not from the directory holding the file you are transforming. Run the tool from somewhere else and you get the defaults. From a6e3e879dc6a61c1d33043d2075c24fadea64e02 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:33:03 +0200 Subject: [PATCH 15/17] Drop the claim that body files are unsupported The limitation is named nodes being renamed, not the kind of file. Saying body files outright contradicted the column sorting the document now explains, which exists because of a body panel. --- TRANSFORMATION_DOCS.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/TRANSFORMATION_DOCS.md b/TRANSFORMATION_DOCS.md index a2b4e71e..4d3ff508 100644 --- a/TRANSFORMATION_DOCS.md +++ b/TRANSFORMATION_DOCS.md @@ -10,7 +10,7 @@ This document explains what the tool does and how to configure it. Transformation targets structural files containing positional node data: frames, suspension arms, subframes, chassis rails. The file must have a `nodes` section with the `["id", "posX", "posY", "posZ"]` header. -It is not intended for body files, engine files, gauges, interior parts, or any file where nodes have semantic names like `int_strsl`, `dshsl`, `e1`, `cam`. Those names carry meaning that transformation does not understand and will overwrite. +It is not intended for any file where nodes have semantic names like `int_strsl`, `dshsl`, `e1`, `cam`, which is common in engine files, gauges and interior parts. Those names carry meaning that transformation does not understand and will overwrite. --- @@ -96,9 +96,9 @@ space between rows lands in the wrong place. **Then `x-sorting-threshold`.** It does the same thing sideways, inside each row. Turn it on if the file zigzags: the heights climb, drop back down and climb -again. That happens when one row covers two vertical columns of nodes, say an -outer face and the one set back beside it, and the tool has nothing but height -to go on. +again. That happens when one row covers two vertical columns of nodes, say the +nose face of a panel and the fender beside it, and the tool has nothing but +height to go on. Here is the part that trips people up. **The number you want is the width of a column, not the space between the columns.** Because the tool measures from the @@ -294,7 +294,7 @@ Without a filename argument, all `.jbeam` files in the directory are validated. ## Limitations -**Body files are not supported.** Body files mix structural nodes with semantically named nodes (`int_strsl`, `dshsl`, `rm_*`). Transformation renames all nodes without distinction. An `excludePrefixes` config option is planned to protect named nodes from being renamed. +**Named nodes are not protected.** Transformation renames every node in the file without distinction, so a file that mixes structural nodes with semantically named ones (`int_strsl`, `dshsl`, `rm_*`) loses those names. Check a file for named nodes before transforming it. An `excludePrefixes` config option is planned to protect them. **The transformation feature is experimental.** It is not included in the standard release binary. To use it, you need to build from source with the `transformation` flag enabled, or download a build that explicitly includes it. From 7c0e669f656259cc0050639651c2773e5dfc9305 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:36:42 +0200 Subject: [PATCH 16/17] Run fourmolu over the band refactor --- src-extra/transformation/JbeamEdit/Transformation.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index 39117184..93299460 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -473,7 +473,6 @@ sortVertices treeType newNames tfCfg (VertexTree comments vertices) = in snd $ mapAccumL (assignNames newNames brks treeType) M.empty columnSortedAnnotated - renamedGroups = case xSortingThreshold tfCfg of Nothing -> renameVertices bandIndices From b9a363c3bed070525458121f8394d230a9690887 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:46:31 +0200 Subject: [PATCH 17/17] Say the support-threshold default is decided, not open The pending spec read as if the question were still open. It is settled that the key should have a default like the others, and the spec goes green with the change that gives it one. --- test-extra/transformation/Spec/Config.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-extra/transformation/Spec/Config.hs b/test-extra/transformation/Spec/Config.hs index af7519a3..5394401f 100644 --- a/test-extra/transformation/Spec/Config.hs +++ b/test-extra/transformation/Spec/Config.hs @@ -50,8 +50,8 @@ configParsingSpec = describe "the transformation config parser" $ do pendingWith "support-threshold is the only key read with .: rather than .:?, so a \ \config that omits it is rejected outright and the loader falls back to \ - \every default. Whether that is meant to be a required key has not been \ - \decided." + \every default instead. The key is meant to have a default like the \ + \others, and this spec goes green with the change that gives it one." parseField supportThreshold "y-sorting-threshold: 0.04\n" `shouldBe` Right defaultSupportThreshold