Skip to content

Make support-vertex renaming a fixed point and fix the #189 output regressions - #218

Draft
webdevred wants to merge 7 commits into
masterfrom
fix-support-vertex-rename-idempotency
Draft

Make support-vertex renaming a fixed point and fix the #189 output regressions#218
webdevred wants to merge 7 commits into
masterfrom
fix-support-vertex-rename-idempotency

Conversation

@webdevred

@webdevred webdevred commented Aug 9, 2026

Copy link
Copy Markdown
Owner

--transform was not a fixed point. updateSupportVertexName appended a side letter to a support name before assignNames ran, and assignNames was written to recognise its own output one character further along, so running the tool twice grew the name and nlsl1 became nlslsl. The pre-mutation is gone and assignNames now derives the whole name from the source name and the x coordinate, which makes a support prefix a fixed point of its own rule.

Two more things stopped it from settling once the names did. compareAV ordered support vertices by the name they currently have, which is the thing being replaced, so the first and second pass sorted on different keys. It now sorts on the prefix the vertex is about to be given. And breakVertices handed the comments and metadata in front of the next group back in reverse, because it reverses the finished tree but not the leftovers, which is why frame's // Middle side and // prefix group rl_m kept swapping places on every run.

Separately, #189 introduced three regressions in the example output that nobody caught because the fixtures were regenerated against the new behaviour. Support vertices reached the forest by two routes and keyed their entries differently, so they never collided and came out as two "Support nodes" sections with metadata stranded between them. Two OMap1 merges also dropped the incoming side's comments unconditionally, even when the existing side had none.

Frame's rear support node changes from rl_rsm to rl_sm. The side letter comes from the coordinate rather than from the name, and _r is a side letter as far as the transformation can tell. The ordinary nodes in that group already lost it, rl_r35 and rl_r38 have been coming out as rl_m0 and rl_m1 all along, so the two now agree rather than contradicting each other.

Outside the obvious scope: bfl_f and bfr_f in examples/jbeam/fender.jbeam are renamed back to bfl0 and bfr0. They were renamed to prove that letter-ending names survive a transform, but only the two node rows were changed, so every beam and triangle has been pointing at names that no longer existed. The property they were there to prove now has its own fixture under examples/regression_jbeam/.

Deliberately left alone: support classification still falls back to the node name, so a name ending in a letter becomes a support vertex regardless of how many beams reach it. That is wrong in principle, but it is not damage this branch caused, and it cannot be replaced until the connectivity threshold works. Measured across 3073 vehicle files, round(0.96 * n) exceeds the maximum possible degree n - 1 for every group under 25 nodes, which covers 97% of files, so dropping the name shortcut today would swap a wrong classification for almost no classification at all. Both halves have their own issue. #219 #220

Still draft, for two reasons. The example output wants a read from someone who knows jbeam, particularly the fender, which regains its side comments and its right-side group metadata. And suspension is not a fixed point yet, which is pending in the new spec: its three support nodes collapse to the same prefix, so the tie falls through to the metadata, and metadata is not carried across vertex tree boundaries. That is an older bug with its own issue and is not something this branch caused.

Support vertices can be classified two ways: moveSupportVertices pulls
out highly-connected nodes via the threshold, and addVertexTreeToForest
separately classifies letter-ending groups as SupportTree directly.
Both used to key their OMap1 entry differently (SupportKey vs a
prefix-derived key), so they never collided and ended up as two
separate "Support nodes" sections with duplicated/misplaced metadata
instead of one merged section.

Also fixes two spots where a collision in an OMap1 merge (in
mergeOMap1Trees and the near-identical insertTreeInMap) discarded the
incoming side's comments unconditionally, even when the existing side
had none to fall back on.

Regenerated examples/transformed_jbeam/fender-cfg-{default,example}.jbeam
via jbeam-edit-dump-ast; frame and suspension are unaffected.
--transform was not a fixed point. updateSupportVertexName appended a
side letter to the name before assignNames ran, and assignNames was
written to recognise its own output one character further along. Running
the tool twice therefore grew the name: nlsl1 became nlslsl.

Drop the pre-mutation and let assignNames derive the whole name from the
source name and the x coordinate. A support prefix is now the name with
its trailing digits, side letter and s removed, followed by s and the
side letter for the group the coordinate falls in. That is a fixed
point, so a second pass lands on the same name.

Frame's rear support node changes from rl_rsm to rl_sm. The side letter
comes from the coordinate rather than from the name, and _r is a side
letter as far as the transformation can tell, so it is dropped. The
ordinary nodes in that group already lost it: rl_r35 and rl_r38 have
been coming out as rl_m0 and rl_m1 all along. The two now agree.

The test transforms a fixture with three support hubs sharing one prefix
group twice and asserts the second pass renames nothing further. The
fixture lives in examples/regression_jbeam/ so it stays out of
jbeam-edit-dump-ast's scan and out of the curated example set.
They were renamed to prove that letter-ending names survive a transform,
but only the two node rows were changed. Every beam and triangle kept
referring to bfl0 and bfr0, so the example has been carrying two
dangling references ever since. The missing digit suffix also broke
prefix-key lookup, which is what silently dropped the Left side and
Right side comments from the output.

The property they were there to prove now has its own fixture in
examples/regression_jbeam/, with a test that asserts every vertex
coordinate survives a transform. That states it directly instead of
leaving it implicit in a node count, and it keeps scaffolding out of an
example the jbeam maintainer edits.

Fender's output goes back to the pre-regression structure: side comment,
side metadata, side coordinates, then one support section.
compareAV ordered support vertices by their current name prefix, and the
current name is what the transformation is about to replace. The first
pass sorted on the source prefixes (rl, rl_f, rl_r), the second on the
names the first pass produced, so the section came out in a different
order each time.

Sort on the prefix the vertex is about to be given instead. It is a pure
function of the name and the x coordinate, so it can be computed before
the rename, and support prefixes are a fixed point of it, so both passes
agree.

Frame's support section settles after one pass. Suspension still moves,
for an unrelated reason: all three of its support nodes collapse to the
same prefix, so the tie falls through to the metadata, and metadata is
not carried across vertex tree boundaries.
breakVertices builds its accumulator back to front and reverses it
before handing the finished tree back. The nodes it hands to the next
tree, the metadata and comments sitting directly in front of the vertex
that ended the current one, were never reversed, so they arrived
backwards.

With two comments in front of a group that means they swap places on
every run, which is why frame's // Middle side and // prefix group rl_m
kept trading positions and never settled. The first tree in a section is
unaffected, it gets its comments from newVertexTree rather than from
this hand-off, which is why only the second and later groups moved.
The existing idempotency spec runs one small regression fixture and
compares names and Y positions only, so comments, metadata and beam
references were never checked. That is how the comment reordering and
the metadata shift stayed invisible.

Parse each committed file in examples/transformed_jbeam/, transform it
again and compare the whole formatted text. The committed file is
already the output of one transform, so it is the input the second pass
would see.

Suspension is pending, because metadata is not carried across vertex
tree boundaries. That is tracked separately.
@webdevred
webdevred force-pushed the fix-support-vertex-rename-idempotency branch from fbf9621 to 2e5ea11 Compare August 9, 2026 16:23
@webdevred webdevred changed the title Fix support-vertex rename idempotency and #189 output regressions Make support-vertex renaming a fixed point and fix the #189 output regressions Aug 9, 2026
A Windows checkout hands the parser CRLF, and commentStripSpace keeps
the newline after a block comment opener by matching on "\n", so the
comment collapsed and the texts differed. The spec is not asserting
anything about line endings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant