Problem
Support-vertex naming is computed in two steps that make different, incompatible assumptions about the shape of the input name:
-
updateSupportVertexName (src-extra/transformation/JbeamEdit/Transformation.hs:152-159) unconditionally appends a side letter:
newName = dropIndex name <> prefixForType vType
It assumes its input is always a "raw" prefix with no existing side annotation (e.g. rl19 -> rl -> rlm, per the docs example). It never checks whether the name already ends in an existing side letter.
-
assignNames (Transformation.hs:368-405) does have re-transform detection (branches checking T.last prefix' == 's', e.g. lines 386, 395), but it looks for that marker in the name after updateSupportVertexName has already mutated it, one character further back than where the detection logic expects it.
The combination means: a node that already went through support-naming once (e.g. nlsl1, structure <base><sideletter>s<groupletter><index>) does not get recognized as already-processed on a second --transform run.
Repro
Starting from a support node named nlsl1 (X=0.457, classified Left):
updateSupportVertexName: dropIndex("nlsl1") = "nlsl", + "l" (Left suffix) = "nlsll"
assignNames on "nlsll": unsnoc gives prefix' = "nlsl", last char 'l', not 's', so the "already a support name" branch does not fire, falls through to the default branch, producing updatedPrefix("nlsl") <> "sl" = "nlslsl"
Confirmed on both master (d6bc439, built and run directly) and triangles-excluded-from-support-vertices, identical corruption on both. nlsl1 -> nlslsl, nlsl2 -> nlslsl1, nrsr1 -> nrsrsr, nrsr2 -> nrsrsr1.
All beam/triangle references were still rewritten consistently to match (checked by diffing node positions before/after), so this does not create a broken mesh, but it does mean re-running --transform is not a stable fixed point, and the resulting names are wrong.
Suggested direction
Two options, haven't picked one yet:
- Minimal patch: make
updateSupportVertexName check whether dropIndex name already ends in an existing side letter (l/m/r) and replace instead of append, mirroring the rule assignNames branches 1/2 already use for ordinary nodes.
- Consolidate: stop having
updateSupportVertexName pre-mutate the name at all, let assignNames alone own "derive clean support prefix from raw vertex name", checking for both an existing side letter and an existing s marker in one place instead of splitting the detection across two functions with mismatched assumptions.
Test gap
The test harness (test-extra/transformation/Spec.hs, topNodeSpec) only ever runs transform once per fixture and diffs against an expected-output fixture. There is no "transform the output again" test shape at all currently, this needs a new kind of spec, not just a new fixture pair.
Fix branch: fix-support-vertex-rename-idempotency
Problem
Support-vertex naming is computed in two steps that make different, incompatible assumptions about the shape of the input name:
updateSupportVertexName(src-extra/transformation/JbeamEdit/Transformation.hs:152-159) unconditionally appends a side letter:It assumes its input is always a "raw" prefix with no existing side annotation (e.g.
rl19->rl->rlm, per the docs example). It never checks whether the name already ends in an existing side letter.assignNames(Transformation.hs:368-405) does have re-transform detection (branches checkingT.last prefix' == 's', e.g. lines 386, 395), but it looks for that marker in the name afterupdateSupportVertexNamehas already mutated it, one character further back than where the detection logic expects it.The combination means: a node that already went through support-naming once (e.g.
nlsl1, structure<base><sideletter>s<groupletter><index>) does not get recognized as already-processed on a second--transformrun.Repro
Starting from a support node named
nlsl1(X=0.457, classified Left):updateSupportVertexName:dropIndex("nlsl1")="nlsl",+ "l"(Left suffix) ="nlsll"assignNameson"nlsll":unsnocgivesprefix' = "nlsl", last char'l', not's', so the "already a support name" branch does not fire, falls through to the default branch, producingupdatedPrefix("nlsl") <> "sl"="nlslsl"Confirmed on both
master(d6bc439, built and run directly) andtriangles-excluded-from-support-vertices, identical corruption on both.nlsl1->nlslsl,nlsl2->nlslsl1,nrsr1->nrsrsr,nrsr2->nrsrsr1.All beam/triangle references were still rewritten consistently to match (checked by diffing node positions before/after), so this does not create a broken mesh, but it does mean re-running
--transformis not a stable fixed point, and the resulting names are wrong.Suggested direction
Two options, haven't picked one yet:
updateSupportVertexNamecheck whetherdropIndex namealready ends in an existing side letter (l/m/r) and replace instead of append, mirroring the ruleassignNamesbranches 1/2 already use for ordinary nodes.updateSupportVertexNamepre-mutate the name at all, letassignNamesalone own "derive clean support prefix from raw vertex name", checking for both an existing side letter and an existingsmarker in one place instead of splitting the detection across two functions with mismatched assumptions.Test gap
The test harness (
test-extra/transformation/Spec.hs,topNodeSpec) only ever runstransformonce per fixture and diffs against an expected-output fixture. There is no "transform the output again" test shape at all currently, this needs a new kind of spec, not just a new fixture pair.Fix branch:
fix-support-vertex-rename-idempotency