Skip to content

Support-vertex renaming is not idempotent, corrupts names on repeated --transform #215

Description

@webdevred

Problem

Support-vertex naming is computed in two steps that make different, incompatible assumptions about the shape of the input name:

  1. 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.

  2. 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:

  1. 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.
  2. 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions