Skip to content

fix(exports): verify every reported artifact - #270

Open
nordic-style wants to merge 2 commits into
mixelpixx:mainfrom
nordic-style:codex/fix/verify-export-artifacts
Open

fix(exports): verify every reported artifact#270
nordic-style wants to merge 2 commits into
mixelpixx:mainfrom
nordic-style:codex/fix/verify-export-artifacts

Conversation

@nordic-style

Copy link
Copy Markdown
Contributor

Summary

  • verify every schematic, PCB, Gerber, drill, BOM, and position artifact before reporting success
  • reject missing, empty, or non-regular output files even when kicad-cli exits successfully
  • build manufacturing responses exclusively from artifacts that passed verification
  • fix PCB PDF layer names and argument formatting so the requested document is actually produced

Closes #252

Approach

The export boundary now enforces a shared postcondition: every advertised artifact must exist, be a regular file, and contain data. Gerber and drill exports additionally require the expected output set, while project snapshots propagate either schematic or PCB export failure instead of returning phantom paths.

Manufacturing packages now expose complete; incomplete packages return an MCP error and omit upload instructions. The file list is assembled from verified paths rather than a raw directory scan, so stale or empty files cannot masquerade as current output.

This PR is intentionally stacked on #269. Its first commit is the export-option implementation from that PR; after #269 merges, the remaining diff is the focused artifact-verification commit cda33a3.

Compatibility and rollback

  • snapshot_project now fails when either requested PDF is missing or empty. This is an intentional correction of false-success behavior.
  • Requested drill output is mandatory for a complete manufacturing package.
  • Manufacturing responses add complete; partial packages use isError: true and no longer include upload instructions.
  • KiCad project sources remain read-only throughout export and validation.
  • After fix(manufacturing): apply Gerber and position options #269 merges, the behavior can be rolled back by reverting the single artifact-verification commit.

Validation

  • cargo test --workspace --locked --lib --tests: all new tests pass; the workspace reaches 555/558 konnect-core tests, with only the same three pre-existing local update_symbols_from_library_* failures caused by the installed KiCad Device:R library shadowing the test fixture
  • cargo test --workspace --locked --doc: pass
  • cargo clippy --workspace --locked --all-targets -- -D warnings: pass
  • cargo fmt --all -- --check: pass
  • real MCP manufacturing export against a temporary copy of the Voice Satellite six-layer board: complete: true, 14 verified non-empty artifacts, including all 11 requested Gerber plots plus PTH and NPTH drill files
  • real MCP project snapshot: verified non-empty schematic and PCB PDFs
  • fake-CLI regression tests confirm that a zero exit status without output fails closed

@mixelpixx

Copy link
Copy Markdown
Owner

#266 is merged and covers this branch's export_pdf rewrite (plus the SVG half this one lacked). The artifact-verification core here — issue #252, checking every reported export exists and is non-empty — is still wanted and still yours to land: please rebase over current main so this PR keeps only that. Note snapshot_project's let _ = swallow (#252) is the highest-value target.

@anyn99

anyn99 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

The premise here — a reported artifact must exist and be non-empty — holds up
well. One case it does not reach yet, found while measuring something else:
exporting a hierarchical schematic to SVG writes one file per sheet.

KiCad 10.0.4, its own bundled pic_programmer demo, through
kicad-cli sch export svg:

input pic_programmer.kicad_sch  ->  pic_programmer.svg
                                    pic_programmer-pic_sockets.svg
input pic_sockets.kicad_sch     ->  pic_sockets.svg

A leaf sheet yields one file; a root with children yields one per sheet.

export_schematic_svg returns output_dir.join("<stem>.svg"), so the
verify_nonempty_file this PR adds covers the root sheet and the child sheets
are neither verified nor reported. On a hierarchical schematic the caller never
learns they were written at all.

This is kicad-cli's own asymmetry rather than anything on this side:

sch export svg  --output OUTPUT_DIR    "Output directory"
sch export pdf  --output OUTPUT_FILE   "Output file"

PDF puts every sheet in one document; SVG cannot, so it takes a directory and
names the files itself.

Worth folding into this PR? Verifying the child files means enumerating them,
and once enumerated, returning them is nearly free. It does change
export_schematic_svg's return type, which #277 is also touching. If you would
rather not, #291 takes it: apply the caller's stem to every file written and
report them all.

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.

[BUG] snapshot_project returns artifact paths it never verifies

3 participants