From 835dfc730ceaa38afe2e2e6c683a226cd4ffaa54 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:01:12 +0200 Subject: [PATCH 1/4] Exclude triangle-referenced vertices from support-vertex eligibility --- examples/jbeam-edit.yaml | 2 +- .../JbeamEdit/Transformation.hs | 49 ++++++++++++++++--- .../Transformation/BeamExtraction.hs | 2 - src/JbeamEdit/Core/Node.hs | 5 ++ 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/examples/jbeam-edit.yaml b/examples/jbeam-edit.yaml index 906ae8ea..4e5be912 100644 --- a/examples/jbeam-edit.yaml +++ b/examples/jbeam-edit.yaml @@ -1,5 +1,5 @@ y-sorting-threshold: 0.05 -support-threshold: 96 +support-threshold: 20 max-support-coordinates: 3 x-group-breakpoints: diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index 34681f0e..07db848d 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -38,6 +38,7 @@ import JbeamEdit.Transformation.OMap1 qualified as OMap1 import JbeamEdit.Transformation.Types import JbeamEdit.Transformation.VertexExtraction import System.OsPath +import Data.Maybe (fromJust) prefixForType :: VertexTreeType -> Text prefixForType LeftTree = "l" @@ -158,19 +159,24 @@ updateSupportVertexName vType (AnnotatedVertex c v m) = AnnotatedVertex c (v {vN name = vName v newName = dropIndex name <> prefixForType vType +-- | Vertices whose name appears in the given set (e.g. names referenced by +-- "triangles") are never eligible to become support vertices, regardless of +-- their connection count / threshold. moveSupportVertices - :: UpdateNamesMap + :: Set Text + -> UpdateNamesMap -> TransformationConfig -> VertexConnMap -> M.Map VertexTreeType [AnnotatedVertex] -> (VertexForest, M.Map VertexTreeType [AnnotatedVertex]) -moveSupportVertices newNames tfCfg connMap vsPerType = +moveSupportVertices protectedNames newNames tfCfg connMap vsPerType = let supportVertices :: [(VertexTreeType, AnnotatedVertex)] supportVertices = [ (vType, av) | (vType, vs) <- M.toList vsPerType , av <- vs , let name = vName (aVertex av) + , name `S.notMember` protectedNames , let vertexCount = length vs thrCount = max 1 (round $ supportThreshold tfCfg / 100 * fromIntegral vertexCount) @@ -216,12 +222,13 @@ notElemByVertexName notElemByVertexName vertex = S.notMember (anVertexName vertex) moveVerticesInVertexForest - :: Node + :: Set Text + -> Node -> UpdateNamesMap -> TransformationConfig -> VertexForest -> Either Text ([Node], VertexForest) -moveVerticesInVertexForest topNode newNames tfCfg vertexTrees = +moveVerticesInVertexForest triangleVertexNames topNode newNames tfCfg vertexTrees = let allVertices = concatMap (concatMap (NE.toList . tAnnotatedVertices . snd) . toList) @@ -234,7 +241,7 @@ moveVerticesInVertexForest topNode newNames tfCfg vertexTrees = (badBeamNodes, conns) <- vertexConns (maxSupportCoordinates tfCfg) topNode groupedVertices let (supportForest, nonSupportVertices) = - moveSupportVertices newNames tfCfg conns groupedVertices + moveSupportVertices triangleVertexNames newNames tfCfg conns groupedVertices newForest <- foldM (addVertexTreeToForest newNames tfCfg nonSupportVertices vertexTrees) @@ -500,6 +507,31 @@ updateOtherFiles formattingConfig updatedNames filepath = do (formatNodeAndWrite formattingConfig filepath node') Left err -> putErrorLine err +trianglesQuery :: NP.NodePath +trianglesQuery = fromList [NP.ObjectIndex 0, NP.ObjectKey "triangles"] + +extractTexts :: Node -> Maybe (Set Text) +extractTexts node = do + outer <- expectArray node + texts <- mapM extractTriple (V.toList outer) + pure . S.fromList $ concatMap (\(a, b, c) -> [a, b, c]) texts + where + extractTriple n = do + inner <- expectArray n + case V.toList inner of + [a, b, c] -> (,,) <$> maybeString a <*> maybeString b <*> maybeString c + _ -> Nothing + +-- | Names of all vertices referenced by any triangle in the "triangles" +-- section. Returns an empty set (not an error) if the section is absent; +-- fails only if the section exists but is malformed. +getTriangleVertexNames :: Node -> Either Text (Set Text) +getTriangleVertexNames topNode = + case NP.queryNodes trianglesQuery topNode of + Left _ -> Right S.empty + Right node -> + maybe (Left "triangles node malformed: expected array of [String,String,String] triples") Right (extractTexts node) + transform :: UpdateNamesMap -> TransformationConfig @@ -509,10 +541,11 @@ transform newNames tfCfg topNode = getVertexForest (xGroupBreakpoints tfCfg) verticesQuery topNode >>= getNamesAndUpdateTree where - getNamesAndUpdateTree (badNodes, globals, vertexForest) = + getNamesAndUpdateTree (badNodes, globals, vertexForest) = do + triangleVertexNames <- getTriangleVertexNames topNode let vertexNames = getVertexNamesInForest vertexForest - in moveVerticesInVertexForest topNode newNames tfCfg vertexForest - >>= getUpdatedNamesAndUpdateGlobally badNodes globals vertexNames + moveVerticesInVertexForest triangleVertexNames topNode newNames tfCfg vertexForest + >>= getUpdatedNamesAndUpdateGlobally badNodes globals vertexNames getUpdatedNamesAndUpdateGlobally badVertexNodes globals oldVertexNames (badBeamNodes, updatedVertexForest) = let updatedVertexNames = getVertexNamesInForest updatedVertexForest updateMap = M.fromList $ on zip M.elems oldVertexNames updatedVertexNames diff --git a/src-extra/transformation/JbeamEdit/Transformation/BeamExtraction.hs b/src-extra/transformation/JbeamEdit/Transformation/BeamExtraction.hs index a5e9141c..f3276abc 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/BeamExtraction.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/BeamExtraction.hs @@ -43,8 +43,6 @@ extractBeamFromArray sectionMeta vec effectiveMeta = M.union inlineMeta sectionMeta in Just (Beam (mkBeamPair n1 n2) effectiveMeta) where - maybeString (String t) = Just t - maybeString _ = Nothing maybeObject n@(Object _) = Just n maybeObject _ = Nothing diff --git a/src/JbeamEdit/Core/Node.hs b/src/JbeamEdit/Core/Node.hs index 176cdb76..e713ebf8 100644 --- a/src/JbeamEdit/Core/Node.hs +++ b/src/JbeamEdit/Core/Node.hs @@ -6,6 +6,7 @@ module JbeamEdit.Core.Node ( isNumberNode, isStringNode, maybeObjectKey, + maybeString, isSinglelineComment, commentIsAttachedToPreviousNode, isComplexNode, @@ -179,6 +180,10 @@ expectObject :: Node -> Maybe (Vector Node) expectObject (Object ov) = Just (ovNodes ov) expectObject _ = Nothing +maybeString :: Node -> Maybe Text +maybeString (String t) = Just t +maybeString _ = Nothing + possiblyChildren :: Node -> Maybe (Vector Node) possiblyChildren n = expectArray n <|> expectObject n From 994f7fed7be826a121b74660ff50db374b80c952 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:46:42 +0200 Subject: [PATCH 2/4] Fixed hlint issues --- .hlint.yaml | 1 + src-extra/transformation/JbeamEdit/Transformation.hs | 1 - src/JbeamEdit/Core/Node.hs | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.hlint.yaml b/.hlint.yaml index 7f49e554..6d4bcb46 100644 --- a/.hlint.yaml +++ b/.hlint.yaml @@ -26,6 +26,7 @@ - name: [Prelude.head] within: [JbeamEdit.Transformation.OMap1] - warn: {name: Use explicit module export list} +- warn: {name: Use DerivingStrategies} - group: {name: dollar, enabled: true} - group: {name: extra, enabled: true} - group: {name: teaching, enabled: true} diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index 07db848d..760f57ca 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -38,7 +38,6 @@ import JbeamEdit.Transformation.OMap1 qualified as OMap1 import JbeamEdit.Transformation.Types import JbeamEdit.Transformation.VertexExtraction import System.OsPath -import Data.Maybe (fromJust) prefixForType :: VertexTreeType -> Text prefixForType LeftTree = "l" diff --git a/src/JbeamEdit/Core/Node.hs b/src/JbeamEdit/Core/Node.hs index e713ebf8..891b5acc 100644 --- a/src/JbeamEdit/Core/Node.hs +++ b/src/JbeamEdit/Core/Node.hs @@ -43,12 +43,12 @@ import Data.Vector qualified as V newtype ArrayValue = ArrayValue { avElements :: Vector (Node, Bool) } - deriving (Eq, Ord, Read, Show) + deriving stock (Eq, Ord, Read, Show) newtype ObjectValue = ObjectValue { ovElements :: Vector (Node, Bool) } - deriving (Eq, Ord, Read, Show) + deriving stock (Eq, Ord, Read, Show) type ObjectKey = (Node, Node) From abdc201ea3ef8f67869a1507e1f37c95378287b3 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:52:58 +0200 Subject: [PATCH 3/4] Ran fourmolu --- .../JbeamEdit/Transformation.hs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index 760f57ca..8481f3c0 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -158,9 +158,10 @@ updateSupportVertexName vType (AnnotatedVertex c v m) = AnnotatedVertex c (v {vN name = vName v newName = dropIndex name <> prefixForType vType --- | Vertices whose name appears in the given set (e.g. names referenced by --- "triangles") are never eligible to become support vertices, regardless of --- their connection count / threshold. +{- | Vertices whose name appears in the given set (e.g. names referenced by +"triangles") are never eligible to become support vertices, regardless of +their connection count / threshold. +-} moveSupportVertices :: Set Text -> UpdateNamesMap @@ -521,15 +522,21 @@ extractTexts node = do [a, b, c] -> (,,) <$> maybeString a <*> maybeString b <*> maybeString c _ -> Nothing --- | Names of all vertices referenced by any triangle in the "triangles" --- section. Returns an empty set (not an error) if the section is absent; --- fails only if the section exists but is malformed. +{- | Names of all vertices referenced by any triangle in the "triangles" +section. Returns an empty set (not an error) if the section is absent; +fails only if the section exists but is malformed. +-} getTriangleVertexNames :: Node -> Either Text (Set Text) getTriangleVertexNames topNode = case NP.queryNodes trianglesQuery topNode of Left _ -> Right S.empty Right node -> - maybe (Left "triangles node malformed: expected array of [String,String,String] triples") Right (extractTexts node) + maybe + ( Left + "triangles node malformed: expected array of [String,String,String] triples" + ) + Right + (extractTexts node) transform :: UpdateNamesMap @@ -543,7 +550,12 @@ transform newNames tfCfg topNode = getNamesAndUpdateTree (badNodes, globals, vertexForest) = do triangleVertexNames <- getTriangleVertexNames topNode let vertexNames = getVertexNamesInForest vertexForest - moveVerticesInVertexForest triangleVertexNames topNode newNames tfCfg vertexForest + moveVerticesInVertexForest + triangleVertexNames + topNode + newNames + tfCfg + vertexForest >>= getUpdatedNamesAndUpdateGlobally badNodes globals vertexNames getUpdatedNamesAndUpdateGlobally badVertexNodes globals oldVertexNames (badBeamNodes, updatedVertexForest) = let updatedVertexNames = getVertexNamesInForest updatedVertexForest From 85da82be03bd76df98166d25567b1a35750f7845 Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Sat, 8 Aug 2026 18:43:47 +0200 Subject: [PATCH 4/4] Tolerate metadata rows when scanning triangles for protected vertices getTriangleVertexNames failed the whole transform as soon as any row in "triangles" wasn't a bare [String, String, String] triple. Per-triangle metadata objects (groundModel, dragCoef, etc.) are normal in real jbeam files, 11 of 11 sampled example files have them, so this broke --transform on almost anything with a triangles section (e.g. van_frame.jbeam). Skip comment and object rows the same way BeamExtraction.possiblyBeam already does for "beams", instead of failing the section outright. Fixture lives in examples/regression_jbeam/, not examples/jbeam/, so it stays out of jbeam-edit-dump-ast's scan (which only reads examples/jbeam/ and would exitFailure on any file that fails to transform) and out of the curated example set the jbeam maintainer keeps. --- examples/regression_jbeam/README.md | 9 ++++++ .../triangles-with-metadata-repro.jbeam | 23 +++++++++++++ .../JbeamEdit/Transformation.hs | 32 +++++++++++-------- test-extra/transformation/Spec.hs | 19 +++++++++++ 4 files changed, 69 insertions(+), 14 deletions(-) create mode 100644 examples/regression_jbeam/README.md create mode 100644 examples/regression_jbeam/triangles-with-metadata-repro.jbeam diff --git a/examples/regression_jbeam/README.md b/examples/regression_jbeam/README.md new file mode 100644 index 00000000..ed93e21f --- /dev/null +++ b/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. diff --git a/examples/regression_jbeam/triangles-with-metadata-repro.jbeam b/examples/regression_jbeam/triangles-with-metadata-repro.jbeam new file mode 100644 index 00000000..8e5c6283 --- /dev/null +++ b/examples/regression_jbeam/triangles-with-metadata-repro.jbeam @@ -0,0 +1,23 @@ +{ +"testpart":{ + "nodes":[ + ["id", "posX", "posY", "posZ"], + // Synthetic regression-test fixture, not vetted by the jbeam + // maintainer and not intended as a demo/example. + ["n0", 1.0, -1.0, 0.0], + ["n1", 1.0, 0.0, 0.0], + ["n2", 1.0, 1.0, 0.0], + ], + "beams":[ + ["id1:", "id2:"], + ["n0", "n1"], + ["n1", "n2"], + ], + "triangles":[ + ["id1:", "id2:", "id3:"], + {"groundModel": "metal"}, + {"dragCoef": 30}, + ["n0", "n1", "n2"], + ], +}, +} diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index 8481f3c0..7a0079ce 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) +import Data.Maybe (fromMaybe, mapMaybe) import Data.Monoid.Extra (mwhen) import Data.Ord (Down (Down), comparing) import Data.Scientific (Scientific) @@ -510,11 +510,21 @@ updateOtherFiles formattingConfig updatedNames filepath = do trianglesQuery :: NP.NodePath trianglesQuery = fromList [NP.ObjectIndex 0, NP.ObjectKey "triangles"] -extractTexts :: Node -> Maybe (Set Text) -extractTexts node = do - outer <- expectArray node - texts <- mapM extractTriple (V.toList outer) - pure . S.fromList $ concatMap (\(a, b, c) -> [a, b, c]) texts +{- | Vertex names referenced by the triangle rows in a "triangles" array. +Comment and metadata (object) rows are skipped rather than treated as +errors, because per-triangle metadata objects (e.g. `{"groundModel": "metal"}`) +are normal in real jbeam files, the same tolerance BeamExtraction.possiblyBeam +has for "beams". Any other row that isn't a [String, String, String] +triple (the header row included, harmlessly) is likewise skipped rather +than failing the whole section. +-} +extractTriangleVertexNames :: Vector Node -> Set Text +extractTriangleVertexNames = + S.fromList + . concatMap (\(a, b, c) -> [a, b, c]) + . mapMaybe extractTriple + . V.toList + . V.filter (\n -> not (isCommentNode n) && not (isObjectNode n)) where extractTriple n = do inner <- expectArray n @@ -524,19 +534,13 @@ extractTexts node = do {- | Names of all vertices referenced by any triangle in the "triangles" section. Returns an empty set (not an error) if the section is absent; -fails only if the section exists but is malformed. +fails only if the section exists but its value isn't an array at all. -} getTriangleVertexNames :: Node -> Either Text (Set Text) getTriangleVertexNames topNode = case NP.queryNodes trianglesQuery topNode of Left _ -> Right S.empty - Right node -> - maybe - ( Left - "triangles node malformed: expected array of [String,String,String] triples" - ) - Right - (extractTexts node) + Right node -> extractTriangleVertexNames <$> NP.expectArray trianglesQuery node transform :: UpdateNamesMap diff --git a/test-extra/transformation/Spec.hs b/test-extra/transformation/Spec.hs index c3463f55..202ce867 100644 --- a/test-extra/transformation/Spec.hs +++ b/test-extra/transformation/Spec.hs @@ -85,6 +85,24 @@ beamValidationSpec = do it "has no duplicate beams" $ findDuplicateBeams internalBeams `shouldBe` [] +{- | Real jbeam files commonly interleave per-triangle metadata objects +(e.g. `{"groundModel": "metal"}`) among triangle rows. That is normal, +not malformed input. `getTriangleVertexNames` used to fail the whole +`transform` call on the first such row instead of skipping it. +-} +trianglesWithMetadataFixture :: FilePath +trianglesWithMetadataFixture = "examples/regression_jbeam/triangles-with-metadata-repro.jbeam" + +triangleMetadataSpec :: Spec +triangleMetadataSpec = + describe "triangles with inline metadata rows" + . it "does not fail transform" + $ do + topNode <- parseJbeamFile trianglesWithMetadataFixture + case transform M.empty newTransformationConfig topNode of + Left err -> expectationFailure ("transform failed: " ++ T.unpack err) + Right _ -> pure () + main :: IO () main = hspec $ do let exampleConfigPath = unsafeEncodeUtf "examples/jbeam-edit.yaml" @@ -102,3 +120,4 @@ main = hspec $ do mapM_ (testInputFile "cfg-default" newTransformationConfig) inputFiles mapM_ (testInputFile "cfg-example" tfConfig) inputFiles beamValidationSpec + triangleMetadataSpec