feat!: a third outcome for the differ, and the org exit-code contract - #24
Merged
Merged
Conversation
Resolves adjudication A3. For a differ too, silence must never read as no difference. has_changes is a boolean, so it was false both for two identical boards and for a board carrying geometry the engine does not model. A stroke drawn with a macro or block aperture is not modelled -- documented as a known limitation, but invisible at runtime. A board with an added trace therefore reported 0 changes at exit 0, with the JSON byte-identical to comparing a board against a copy of itself, while the raster engine reported the change. Two engines, one question, opposite answers, and the geometry engine gave the dangerous one. The engine now records what it could not model, per reason. A layer carrying any of it is indeterminate, and the comparison reports identical | different | indeterminate. different outranks indeterminate, so one unmodellable stroke cannot mask a trace that moved. The reasons are named on stderr even under --quiet: suppressing them is the failure being reported on. Indeterminate exits 2 without waiting for --fail-on-diff. That flag chooses whether a difference is a failure; it has no bearing on whether the tool could look. BREAKING CHANGE: the exit-code contract now matches partspec and netspec. 0 no differences, 1 differences (with --fail-on-diff), 2 could not complete the comparison, 4 an input could not be read or parsed, 64 usage (EX_USAGE). Previously 2 meant a parse error, and 1 doubled as both "differences found" and "could not write the output file". The geometry report schema goes 2 -> 3, additively.
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
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.
Resolves adjudication A3. For a differ too, silence must never read as no
difference.
The defect, reproduced
has_changesis aboolean, so it wasfalseboth for two identical boards and for aboard carrying geometry the engine does not model.
A stroke drawn with a macro or block aperture is not modelled by the geometry engine.
docs/geometry-diff.mdhas said so under "Known limitations" for a long time — but theoutput never did:
Two engines, one question, opposite answers — and the geometry engine gave the
dangerous one. The JSON was byte-identical to comparing
beforeagainst itself, sothere was no field a consumer could branch on.
The fix
The engine records what it could not model, by reason
(
UNREPRESENTED_REASONS), covering every silentreturnin the emit path: strokes withmacro or block apertures, apertures whose extents cannot be computed, macro flashes that
evaluate to empty, and block nesting past the replay depth.
A layer carrying any of it is
indeterminate, and the comparison reportsidentical | different | indeterminate:differentoutranksindeterminate, so one unmodellable stroke cannot mask a tracethat actually moved. The reasons go to stderr even under
--quiet— suppressing themis the failure being reported on.
Indeterminate does not wait for
--fail-on-diff. That flag chooses whether adifference is a failure; it has no bearing on whether the tool could look. Same
reasoning as netspec's
incomplete.BREAKING: exit codes now match partspec and netspec
A1 settled this vocabulary across the org; gerberdiff was the last member out of step.
01--fail-on-diff)2--align-offset464EX_USAGE)1and2were each doing two jobs. A gate written[ $? -eq 0 ]is unaffected; onethat treated
2as a parse failure must now read4.Geometry report schema
version2 -> 3, addingsummary.outcome,summary.unrepresentedand a per-layerunrepresented. Additive — every v2 field keepsits meaning.
Tests
New
tests/test_indeterminate.py, 14 tests.Proved red by mutation rather than by reverting the source: neutering
cannot_representto a no-op — restoring exactly the old silent-drop behaviour whilekeeping the API — turns 6 of them red and leaves 8 green. The 8 are the pure-outcome
logic and the controls (identical boards still exit 0, a fully-modelled board records
nothing). Reverting the whole module instead would have failed at import and proved
nothing per-test, which is the shape §2.4 of the org contract warns about.
Ten existing tests changed, every one a direct consequence of the renumbering
(overwrite guards
1 -> 64, parse errors2 -> 4, schema2 -> 3) and each inspectedindividually.
ruff,ruff format,mypy, the ASCII scan and the coverage gate all pass: 487passed, 95.77% (baseline 473).