Skip to content

diff/geomdiff silently drop flashes using an undefined aperture, reporting 0 changes at exit 0 #17

Description

@CameronBrooks11

Summary

A flash whose D-code selects an aperture that was never defined is dropped
from the geometry with no diagnostic of any severity. Both diff engines then
report 0 changes and exit 0 under --fail-on-diff, and the JSON report is
byte-identical to a comparison of two genuinely identical boards.

A fabrication change can therefore pass a --fail-on-diff CI gate invisibly.

Reproduction

before/ has one pad. after/ has that pad plus a second at (5,5)mm — but
D11 is never defined.

%FSLAX36Y36*%
%MOMM*%
%ADD10C,1.000000*%
D10*
X1000000Y1000000D03*
D11*
X5000000Y5000000D03*
M02*
$ gerberdiff geomdiff before after --fail-on-diff
geomdiff: 0/1 layers changed, 0 changes  (1 ms)
exit=0            # stderr: 0 bytes

$ gerberdiff diff before after --fail-on-diff
diff: 0/1 layers changed  (133 ms)
exit=0            # stderr: 0 bytes

Both reports compare equal (== on the parsed JSON) to the report produced by
diffing before against an exact copy of itself. There is no field a consumer
could branch on to tell the two situations apart.

Expected

Selecting an undefined aperture is invalid per RS-274X. It should be an Error
diagnostic, which both engines already promote to GerberParseError → exit 2.

The tool already holds the evidence

$ gerberdiff parse after/board-F_Cu.gbr
nets: 2
apertures: 1
bbox: x=[0.019685, 0.196850] y=[0.019685, 0.196850] inches
  • nets: 2, apertures: 1 — two draw ops, one aperture.
  • The bbox reaches 5.000 mm, the pad centre, not 5.500 mm, its edge —
    because _aperture_radius() returns 0.0 for a missing aperture
    (parse/gerber_state.py:207-217). A correctly rendered pad gives 5.500 mm
    (confirmed against a control file with %ADD11C,1.000000*%).

parse exits 0 here too.

Mechanism

  1. parse/gerber_state.py:406-407_handle_d_code assigns
    self._current_aperture = value for any value >= 10 without checking that
    the aperture exists. No diagnostic.
  2. parse/gerber_state.py:296 — the DrawOp is emitted carrying that
    aperture_index.
  3. geometry/layer_geometry.py:280_emit_flash does if ap is None: return.
    The flash vanishes; no diagnostic is appended.

Contrast — the adjacent case is handled correctly

An aperture definition naming an undefined macro takes the _error() path
(gerber_state.py:449), and both engines promote any Error diagnostic to a
raised GerberParseError (geometry/driver.py:105, diff/diff_engine.py:185),
giving exit 2 with a message. Verified:

$ gerberdiff geomdiff before after_undefined_macro --fail-on-diff
error: board-F_Cu.gbr: Aperture definition references undefined macro 'ROUNDPAD' (line 4)
exit=2

So the fix shape already exists in the codebase — this path just never reaches it.

Suggested fix

Emit an Error diagnostic where an aperture is selected or used without a
definition. Either at selection (_handle_d_code, cheapest and points at the
right line) or at use (_emit_flash / _emit_stroke, catches block-scope cases
too). Existing promotion machinery then produces exit 2 unchanged.

Worth considering alongside: _on_diagnostic in the diff and geomdiff CLI
paths (cli.py:302, cli.py:450) has no Error branch. Currently harmless,
since Errors are raised before reaching it — but it means any future
non-fatal Error would print nothing.

Environment

gerberdiff 0.29.1 (main, working tree clean), Python 3.13, Linux.
Test suite green at time of filing: 463 passed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    silence-defectSilence read as success. Add the case to https://heibench.com/silence.html

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions