From 868caf059dd1fce9504ed1089cf2606d66b55b4c Mon Sep 17 00:00:00 2001 From: Cameron Brooks Date: Wed, 9 Sep 2026 22:30:47 -0400 Subject: [PATCH 1/5] ci: skip __pycache__ in the non-ASCII check grep -rP descends into compiled caches, whose .pyc bytes are not ASCII. A clean CI checkout has none, so the step passed; anyone running the same grep locally after a test run gets a failure on output nobody wrote. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 972c481..7143106 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,7 @@ jobs: - run: uv run ruff format --check gerberdiff/ tests/ - name: Check for non-ASCII characters run: | - if grep -rP '[^\x00-\x7F]' gerberdiff/ tests/ *.md *.toml; then + if grep -rP --exclude-dir=__pycache__ '[^\x00-\x7F]' gerberdiff/ tests/ *.md *.toml; then echo "Non-ASCII characters found" exit 1 fi From 1b28319df71261a49cfb079954a5ac11c92892e9 Mon Sep 17 00:00:00 2001 From: Cameron Brooks Date: Wed, 9 Sep 2026 22:30:47 -0400 Subject: [PATCH 2/5] chore: add a justfile and AGENTS.md Every recipe in check and test is a command ci.yml runs, with CI's arguments: ruff format --check and ruff check over gerberdiff/ and tests/, the non-ASCII grep, mypy, and pytest with --cov-fail-under=90. CONTRIBUTING.md's four hand-typed commands were missing the format --check, the coverage gate and the ASCII step, so passing them locally did not mean passing CI. AGENTS.md names the org contract as the floor and records what is specific here: the exit-code contract settled in 0685cde, that only geomdiff can return 2, that import gerberdiff stays Cairo-free, the ASCII rule, the coverage gate, and that planning/ is gitignored scratch rather than a plan. --- AGENTS.md | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ justfile | 74 ++++++++++++++++++++++ 2 files changed, 256 insertions(+) create mode 100644 AGENTS.md create mode 100644 justfile diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..aa09117 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,182 @@ +# AGENTS.md -- gerberdiff + +Instructions for humans and AI coding agents working in this repository. + +The org-wide contract at +is the floor. This file carries what is specific to gerberdiff; where the two +conflict, this file wins. + +## Project + +`gerberdiff` is a **differ** over fabrication output: Gerber (RS-274X) and +Excellon. It sits on the org's VERIFY layer, but it adjudicates a comparison +rather than a contract -- there is no declared intent to check against, so it +answers *identical / different / indeterminate* rather than *pass / fail*. + +Two engines over the same parsed IR, and they are not interchangeable: + +- **`diff`** rasterises both revisions through Cairo and reports changed + pixels and regions. Correct, and hard to read: a small uniform shift renders + as add/remove crescents indistinguishable from real material change. +- **`geomdiff`** works on the parsed vector geometry with shapely, and + attributes each change as `added` / `removed` / `moved` / `resized`, down to + micrometre displacements. Resolution-independent and Cairo-free. + +**Version 0.30.0. Four verbs: `parse`, `render`, `diff`, `geomdiff`.** +`parse` reports diagnostics for one file; `render` writes a PNG of one file; +`diff` and `geomdiff` compare two directories. Only `geomdiff` can return the +third outcome -- see the exit-code section, where that asymmetry is spelled out +as the thing it is. + +Treat this paragraph as code (org contract 2.5): the moment a verb is added, +removed, or gains an outcome it does not have, this section is false and the +change that made it false is not finished until it is corrected. + +## Start here + +**There is no `docs/DECISIONS.md` in this repository.** Every sibling has one; +this repo's numbered-decision equivalent is `CHANGELOG.md` plus the docstrings +that carry the reasoning at the point it binds -- `EXIT_*` in +`gerberdiff/cli.py`, `DiffOutcome` and `LayerGeometryDiff.unrepresented` in +`gerberdiff/geometry/types.py`. Cite a commit when you need to cite a decision. +If you are about to relitigate something, read the commit that settled it first. + +Then: + +1. `docs/schema.md` -- the JSON report schemas. This is a stable surface (org + contract 5); consumers branch on it and on the exit code, never on the + Python API. +2. `docs/geometry-diff.md` -- how the geometry engine works and what it does + not model. +3. `docs/architecture.md` -- module layout and the parse/render/diff/geometry + split. +4. `CONTRIBUTING.md` -- the ASCII rule, with the table of banned characters. + +`planning/`, `artifacts/` and `outputs/` are **gitignored local scratch**, not +part of the repository (`.gitignore` lines 221-223, and `git ls-files planning` +returns nothing). `planning/` in particular holds the 2026-06 phase plan for +the geometry engine; its own README records that work as finished and the docs +as historical. Do not plan against it and do not treat it as a tracker. + +## Stack + +- **Python >= 3.11**, packaged with `hatchling`, `uv` against a committed + `uv.lock`. +- **Runtime dependencies**: `click`, `numpy`, `cairocffi`, `scipy`, `shapely`. + Adding another is org contract section 10 -- escalate, do not decide. +- **Tooling**: `ruff` (format + lint), `mypy` (strict), `pytest` + + `pytest-cov`, `just`, `pre-commit`. +- The version is set in **both** `pyproject.toml` and `gerberdiff/__init__.py`. + Nothing checks that they agree; keep them in sync by hand. + +## Commands + +```sh +just setup # uv sync --dev +just fmt # format + autofix (mutates the tree) +just check # fmt-check + lint + ascii + typecheck -- the CI gate +just test # pytest with the 90% coverage gate +just test-nocov # pytest without it, as CI's Windows leg runs it +just hooks # every pre-commit hook over the whole tree +``` + +Run `just check && just test` before every commit. Never `--no-verify`. + +Every recipe inside `check` and `test` is a command `.github/workflows/ci.yml` +runs, with CI's arguments rather than tidier ones. Change a recipe and you must +change the workflow, or the name stops being true. Note that `check` includes +an **`ascii` recipe with no sibling equivalent**: CI greps `gerberdiff/ tests/ +*.md *.toml` for anything outside U+0000-U+007F and fails on a hit, so an em +dash in this file would turn CI red. + +`hooks` is not in `check` because nothing in `ci.yml` runs pre-commit. A green +`just check` says nothing about the hooks. + +## Outcomes and exit codes + +Settled in `0685cde` as org adjudication A3, and released in 0.30.0. These are +the org contract section 6.2 codes; gerberdiff conforms rather than chooses. + +| Outcome | Exit | Meaning | +|---|---|---| +| `identical` | 0 | No differences, and everything was modelled. | +| `different` | 1 | Differences found. A finding about the fabrication output. | +| `indeterminate` | 2 | Part of the comparison could not be made. **Not** a statement about the boards. | +| -- | 4 | Could not read or parse an input. Not a statement about the boards either. | +| -- | 64 | Usage: bad arguments, or an output file that exists without `--overwrite`. | + +Three things about this are load-bearing and each is enforced by code you can +read: + +1. **`2` is a third *diff outcome*, not a verdict.** A differ answers a + different question from a checker, so the word beside `identical` and + `different` is `indeterminate`, not `fail`. The code and its meaning are the + org's. `DiffOutcome` in `gerberdiff/geometry/types.py` is that enum. +2. **`different` outranks `indeterminate`.** A change that *was* found stays a + finding even when some other operation could not be modelled; the reverse + would let one unmodellable stroke mask a trace that actually moved. + `GeometryDiffResult.outcome` implements the precedence. +3. **`indeterminate` does not wait for `--fail-on-diff`.** That flag chooses + whether a *difference* is a failure. It has no bearing on whether the tool + could look, so exit `2` is returned regardless of it + (`gerberdiff/cli.py`, the `geomdiff` exit path). + +**The raster `diff` verb has no third outcome.** It exits `0`, `1`, `4` or +`64` and never `2`. That is the current state of the code, not a design claim: +`geomdiff` knows an operation was unmodelled because `layer_geometry.py` counts +the reason, and the raster path has no equivalent record. Do not describe +`diff` as carrying the third outcome, and do not add a `2` to it by inference +-- if the raster engine can be blind in a way it can detect, that is a change +with a finding behind it. + +## The rules that are not negotiable + +1. **What was not modelled must never be reported as unchanged.** This is the + whole of `0685cde` and the org's first rule in this repo's terms. When the + geometry engine cannot build a shape it records the reason in + `UNREPRESENTED_REASONS` (`gerberdiff/geometry/layer_geometry.py`), which + makes the layer `indeterminate`. Before that existed, a layer full of + unmodellable strokes reported `identical` at exit 0 with JSON + byte-identical to comparing a board against a copy of itself. +2. **`import gerberdiff` stays Cairo-free.** The render path imports + `cairocffi` lazily, so `geomdiff` and the Python API work on a machine with + no system Cairo -- which is why the Windows CI leg can run at all. Verify + with `python -c "import sys, gerberdiff; print('cairocffi' in sys.modules)"`; + it must print `False`. A top-level render import puts the whole library + behind a system library it does not need. +3. **Every tracked text file is pure ASCII (U+0000-U+007F).** Enforced by + `just ascii` and by CI. `CONTRIBUTING.md` carries the replacement table: + `--` for an em dash, `->` for an arrow, `>=` for the inequality. +4. **Coverage must not fall below 90%.** `just test` and CI both pass + `--cov-fail-under=90`. `gerberdiff/cli.py` is omitted from the measurement + (`[tool.coverage.run] omit`) because it is covered through integration + tests; that omission is why the gate can be met without CLI unit tests, and + it is not licence to move logic into `cli.py` to escape the gate. +5. **Do not reimplement geometry the library already does.** shapely owns the + boolean operations and the Minkowski sums. Where the engine cannot express + something, record it as unrepresented -- do not approximate it (org + contract 2.3: never substitute a plausible number for an answer). +6. **`ok` is the only required status check.** Adding a job to `ci.yml` means + adding it to the `ok` job's `needs:` and to its explicit + `result != 'success'` test. This workflow does no path filtering, so a + skipped job is always a defect and the tolerant `contains(needs.*.result, + 'failure')` form must not be used here (org contract 8.1). +7. Do not add AI attribution to commits or PR descriptions -- no co-author + trailers, session links, or generated-with footers. +8. Do not name, link, or describe any private repository in public output. + +## Related + +Siblings, deliberately non-overlapping: + +- `netspec` -- PCB connectivity against declared intent, with KiCad as the + oracle. Run its checks, and KiCad's own ERC and DRC, **before** reaching for + gerberdiff: they are cheaper and catch more. gerberdiff answers the last + question, about the bytes the fab actually receives. +- `partspec` -- verifies CAD-as-code mechanical parts. Its + `{identical: 0, different: 1, indeterminate: 2}` comparison verbs are where + this repo's outcome vocabulary comes from. +- `slicelab` and `prusaslicer-py` -- slicer drivers. `orlab` -- OpenRocket. + +Design review belongs to `kicad-happy`; authoring belongs to Konnect / SKiDL / +atopile. Neither is in scope here. diff --git a/justfile b/justfile new file mode 100644 index 0000000..dbb55ab --- /dev/null +++ b/justfile @@ -0,0 +1,74 @@ +# gerberdiff task runner +# +# Every recipe in `check` and `test` is a command .github/workflows/ci.yml runs. +# If you change one here, change it there; the point of this file is that +# `just check && just test` is the CI gate rather than a shorter list wearing +# its name. The path arguments are CI's, not `.`, for the same reason. + +set dotenv-load := false + +# Show available recipes +default: + @just --list + +# Install dependencies (ci.yml: `uv sync --dev` in all three jobs) +setup: + uv sync --dev + +# Update the lockfile after changing dependencies in pyproject.toml +lock: + uv lock + +# Format and apply lint fixes (mutates the working tree; CI never runs this) +fmt: + uv run ruff format gerberdiff/ tests/ + uv run ruff check --fix gerberdiff/ tests/ + +# Verify formatting without mutating (ci.yml lint job) +fmt-check: + uv run ruff format --check gerberdiff/ tests/ + +# Lint (ci.yml lint job) +lint: + uv run ruff check gerberdiff/ tests/ + +# --exclude-dir is a no-op on CI's clean checkout and load-bearing locally: +# `just test` leaves __pycache__ behind, .pyc files contain non-ASCII bytes, and +# without it `just check` after `just test` fails on compiled output nobody wrote. +# ci.yml carries the same flag so the two commands stay one command. + +# Every tracked text file is pure ASCII -- see CONTRIBUTING.md (ci.yml lint job) +ascii: + #!/usr/bin/env bash + set -euo pipefail + if grep -rP --exclude-dir=__pycache__ '[^\x00-\x7F]' gerberdiff/ tests/ *.md *.toml; then + echo "Non-ASCII characters found" + exit 1 + fi + +# Type-check (ci.yml typecheck job) +typecheck: + uv run mypy gerberdiff/ tests/ + +# The CI-equivalent gate: everything ci.yml's lint and typecheck jobs run +check: fmt-check lint ascii typecheck + +# Run tests with the coverage gate (ci.yml test job, Linux legs) +test: + uv run pytest tests/ --cov=gerberdiff --cov-fail-under=90 -q + +# ci.yml also passes --cov-report=xml on one leg, purely to upload the +# report as an artifact. Left out here: it writes a file nobody reads locally. + +# Tests without the coverage gate -- ci.yml's Windows leg, where no system Cairo makes the raster tests skip +test-nocov: + uv run pytest tests/ -q + +# Run every pre-commit hook. Nothing in ci.yml runs these, so `just check` says nothing about them +hooks: + pre-commit run --all-files + +# Remove build and tool caches +clean: + rm -rf dist .pytest_cache .ruff_cache .mypy_cache .coverage coverage.xml + find . -type d -name __pycache__ -exec rm -rf {} + 2>/dev/null || true From a1edb75d04569c7c4915be5ae78aad498fff4e24 Mon Sep 17 00:00:00 2001 From: Cameron Brooks Date: Wed, 9 Sep 2026 22:32:16 -0400 Subject: [PATCH 3/5] docs(agents): state the cairo-free claim to what was verified The lazy render import was credited with the Windows CI leg running. It is not the mechanism: the raster tests skip through HAS_CAIRO in tests/cairo_support.py, which exists because cairocffi raises OSError rather than ImportError when the shared library is missing. --- AGENTS.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index aa09117..6e067ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -139,11 +139,14 @@ with a finding behind it. unmodellable strokes reported `identical` at exit 0 with JSON byte-identical to comparing a board against a copy of itself. 2. **`import gerberdiff` stays Cairo-free.** The render path imports - `cairocffi` lazily, so `geomdiff` and the Python API work on a machine with - no system Cairo -- which is why the Windows CI leg can run at all. Verify - with `python -c "import sys, gerberdiff; print('cairocffi' in sys.modules)"`; - it must print `False`. A top-level render import puts the whole library - behind a system library it does not need. + `cairocffi` lazily, so the parse and geometry pipelines work on a machine + with no system Cairo. Verify with + `python -c "import sys, gerberdiff; print('cairocffi' in sys.modules)"`; it + must print `False`. A top-level render import puts the whole library behind + a system library it does not need. The tests skip the raster engine + separately, through `HAS_CAIRO` in `tests/cairo_support.py` -- + `pytest.importorskip` cannot do it, because `cairocffi` raises `OSError` + rather than `ImportError` when the shared library is missing. 3. **Every tracked text file is pure ASCII (U+0000-U+007F).** Enforced by `just ascii` and by CI. `CONTRIBUTING.md` carries the replacement table: `--` for an em dash, `->` for an arrow, `>=` for the inequality. From 45cbee659cd4aa0c6865fb1246f59ddf90cbab78 Mon Sep 17 00:00:00 2001 From: Cameron Brooks Date: Wed, 9 Sep 2026 22:33:38 -0400 Subject: [PATCH 4/5] docs(agents): drop an unchecked claim about the siblings "Every sibling has one" is false: orlab has no docs/DECISIONS.md either. The point stands without the comparison, so it is stated without one. --- AGENTS.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6e067ce..89a67e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,9 +34,10 @@ change that made it false is not finished until it is corrected. ## Start here -**There is no `docs/DECISIONS.md` in this repository.** Every sibling has one; -this repo's numbered-decision equivalent is `CHANGELOG.md` plus the docstrings -that carry the reasoning at the point it binds -- `EXIT_*` in +**There is no `docs/DECISIONS.md` in this repository**, though the org contract +(section 7) points at one and most siblings have it. This repo's +numbered-decision equivalent is `CHANGELOG.md` plus the docstrings that carry +the reasoning at the point it binds -- `EXIT_*` in `gerberdiff/cli.py`, `DiffOutcome` and `LayerGeometryDiff.unrepresented` in `gerberdiff/geometry/types.py`. Cite a commit when you need to cite a decision. If you are about to relitigate something, read the commit that settled it first. From 25b5adf4ce6179de7504f8a4bcec84941f8136b0 Mon Sep 17 00:00:00 2001 From: Cameron Brooks Date: Wed, 9 Sep 2026 23:06:19 -0400 Subject: [PATCH 5/5] docs(agents): qualify the exit codes and the ascii check's scope The exit table said `different` is exit 1. It is exit 1 only with --fail-on-diff; without the flag a run with 28928 changes across 14 of 15 layers exits 0. Both existing records of this -- the EXIT_DIFFERENT docstring and the 0.30.0 changelog entry -- carry the qualifier, and this file was the copy that dropped it, which is exactly the gate it tells a reader to write: `[ $? -eq 1 ]` never fires. Also note that 2 is unreachable once a change is found, since `different` outranks `indeterminate`, and point a caller at `summary.unrepresented` instead. The ASCII rule claimed every tracked text file is enforced. The argv is `gerberdiff/ tests/ *.md *.toml` with root-level globs, so 26 of 131 tracked files are outside it -- all of docs/, all of .github/, and the justfile carrying the rule. Scoped the sentence to what the check reads. Name the test matrix as the largest deliberate gap between `just test` and CI, and attribute kicad-happy to aklofas rather than implying it is a sibling. --- AGENTS.md | 56 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 89a67e4..942e76f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,8 +90,14 @@ an **`ascii` recipe with no sibling equivalent**: CI greps `gerberdiff/ tests/ *.md *.toml` for anything outside U+0000-U+007F and fails on a hit, so an em dash in this file would turn CI red. -`hooks` is not in `check` because nothing in `ci.yml` runs pre-commit. A green -`just check` says nothing about the hooks. +Some of what CI does, the recipes deliberately do not. `hooks` is outside +`check` because nothing in `ci.yml` runs pre-commit, so a green `check` says +nothing about the hooks. `setup` is `uv sync --dev` and not `--locked`, because +that is what CI runs. `--cov-report=xml` is dropped, because CI passes it on one +leg only to upload an artifact. **The largest omission is the matrix:** CI runs +the suite on Python 3.11, 3.12 and 3.13 on ubuntu plus 3.11 on windows, and +`just test` runs whatever interpreter `uv` resolves here. A green local `test` +is one leg of four, and the Windows leg is the one that skips the raster engine. ## Outcomes and exit codes @@ -101,11 +107,35 @@ the org contract section 6.2 codes; gerberdiff conforms rather than chooses. | Outcome | Exit | Meaning | |---|---|---| | `identical` | 0 | No differences, and everything was modelled. | -| `different` | 1 | Differences found. A finding about the fabrication output. | -| `indeterminate` | 2 | Part of the comparison could not be made. **Not** a statement about the boards. | +| `different` | 1 **only with `--fail-on-diff`**, otherwise 0 | Differences found. A finding about the fabrication output. | +| `indeterminate` | 2, regardless of the flag | Part of the comparison could not be made. **Not** a statement about the boards. | | -- | 4 | Could not read or parse an input. Not a statement about the boards either. | | -- | 64 | Usage: bad arguments, or an output file that exists without `--overwrite`. | +**Read the two qualifiers in that table before writing a gate.** They are the +whole difference between a gate that works and one that is always green: + +``` +$ gerberdiff geomdiff tests/fixtures/gerbers-before tests/fixtures/gerbers-after +geomdiff: different, 14/15 layers changed, 28928 changes (20356 ms) +$ echo $? +0 +``` + +Fourteen of fifteen layers changed and the exit code is `0`, because +`--fail-on-diff` was not passed. So `gerberdiff geomdiff a b; [ $? -eq 1 ]` +never fires. Pass the flag, or branch on `summary.outcome` in the JSON report. +`cli.py`'s `EXIT_DIFFERENT` docstring and the 0.30.0 `CHANGELOG.md` entry both +carry the qualifier; this table is not allowed to be the copy that drops it. + +**And `2` is reported only when nothing else was found.** Because `different` +outranks `indeterminate`, a run that both found a change *and* failed to model +something reports `different` -- so exit `2` is unreachable whenever a real +change exists, and a caller treating `2` as its only incomplete signal misses +the changed-and-incomplete case entirely. `summary.unrepresented` in the JSON, +and the stderr warning that prints even under `--quiet`, are emitted whenever +anything went unmodelled, independent of the outcome. Branch on those. + Three things about this are load-bearing and each is enforced by code you can read: @@ -148,9 +178,16 @@ with a finding behind it. separately, through `HAS_CAIRO` in `tests/cairo_support.py` -- `pytest.importorskip` cannot do it, because `cairocffi` raises `OSError` rather than `ImportError` when the shared library is missing. -3. **Every tracked text file is pure ASCII (U+0000-U+007F).** Enforced by - `just ascii` and by CI. `CONTRIBUTING.md` carries the replacement table: - `--` for an em dash, `->` for an arrow, `>=` for the inequality. +3. **Text must be pure ASCII (U+0000-U+007F).** `CONTRIBUTING.md` states the + rule for all tracked files and carries the replacement table: `--` for an em + dash, `->` for an arrow, `>=` for the inequality. **What is enforced is + narrower than what is stated.** `just ascii` and the CI step it mirrors take + the argv `gerberdiff/ tests/ *.md *.toml`, where the last two are + root-level globs -- so `docs/`, everything under `.github/`, `mkdocs.yml`, + `.pre-commit-config.yaml` and the `justfile` itself are outside the check. + 26 of 131 tracked files are text the check never reads. Write ASCII + everywhere; do not read a green `just ascii` as coverage of a file it did + not open. Widening the argv is tracked on #28. 4. **Coverage must not fall below 90%.** `just test` and CI both pass `--cov-fail-under=90`. `gerberdiff/cli.py` is omitted from the measurement (`[tool.coverage.run] omit`) because it is covered through integration @@ -182,5 +219,6 @@ Siblings, deliberately non-overlapping: this repo's outcome vocabulary comes from. - `slicelab` and `prusaslicer-py` -- slicer drivers. `orlab` -- OpenRocket. -Design review belongs to `kicad-happy`; authoring belongs to Konnect / SKiDL / -atopile. Neither is in scope here. +Outside the org and out of scope here: design review belongs to +[`aklofas/kicad-happy`](https://github.com/aklofas/kicad-happy), authoring to +Konnect / SKiDL / atopile.