X-column ordering inside a Y band - #235
Merged
Merged
Conversation
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.
webdevred
force-pushed
the
x-column-sorting
branch
from
August 20, 2026 19:32
c6be83d to
db9744d
Compare
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.
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.
webdevred
force-pushed
the
x-column-sorting
branch
from
August 23, 2026 13:51
ab2c33f to
5098184
Compare
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.
webdevred
marked this pull request as ready for review
August 23, 2026 14:34
webdevred
changed the title
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.
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.
The key was missing from the parameter table and the sorting description still named three coordinates.
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.
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The five frontmost left-side vertices in the y-sorting fixture 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. This adds the test that pins the wanted order. It is a draft because nothing implements it yet: the test fails, and the branch deliberately carries no production change. #234 has the geometry and why no y-sorting-threshold can fix it.
The threshold in the test is not free choice, and that is the part worth a second look. At 0.31 all five vertices land in one band, which is where the defect lives. Between 0.153 and 0.16 the Y bands happen to land on exactly the two columns and the assertion passes with nothing fixed at all, so the test would claim coverage it does not have. Both of those were run, not reasoned about.
No new fixture. The positions were already in
examples/regression_jbeam/y-sorting-repro.jbeam, so that file now serves two tests and its comment says so, which is the one edit here outside test code.