Skip to content

Import KiCad designs as persistent schematics - #1282

Open
LK wants to merge 4 commits into
mainfrom
feat/import-persistent-schematics
Open

LK wants to merge 4 commits into
mainfrom
feat/import-persistent-schematics

Conversation

@LK

@LK LK commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Make pcb import retain the original KiCad document for the same persistent schematic workflow used by Quiche and pcb apply schematic, rather than reconstructing a schematic from pcb:sch placement comments. There is no browser frontend embedded in the CLI.

Behavior

  • Copy the original hierarchy into layout/ and enable persistent schematics. Bind managed symbols to evaluated component Paths/per-unit UUIDs and bind native labels/power pins to generated net identities without changing displayed text or native electrical merging.
  • Generate components from their actual page-local cached symbols, including alias overrides, rather than substituting generic passives. Preserve displayed Value/Footprint/Description, including empty fields, separately from geometry and BOM data.
  • Reuse the shared no-connect analysis from main. Only geometry-proven intentional-open stacks normalize to separate generated NotConnected() terminals; raw extraction and native documents remain truthful. Physical-pin partition validation and final schematic agreement remain blocking.
  • Report source schematic/PCB parity mismatches without blocking import. Connectivity follows the schematic; import does not repair a stale PCB. Retain diagnostics and mismatch counts, removing the old tolerated/blocking heuristics.
  • Preserve the full-project/standalone distinction: projects retain PCB/rules/project configuration and the source archive; standalone imports create the persistent schematic project without inventing a PCB/archive. Existing PCB sync identity/net-name prepatch behavior remains.
  • Remove legacy positioning/comment modules, passive-promotion classification/substitution, and unused report data.

Based on current main after #1280 and #1281. Their shared compatibility fixes, including both later #1280 review corrections and regressions, are preserved and not duplicated in this diff. Related compatibility context: ENG-1396/ENG-1736 (cache aliases), ENG-1412/ENG-1682 (stacked/hidden NC), ENG-1728 (page isolation only, not cross-sheet multi-unit support).

Diff after review fixes: 28 files, +2,619/−3,545, net −926 lines. This includes an unchanged 1,105-line public KiCad LM358 fixture and provenance README; no Antmicro designs are added as fixtures.

Review corrections preserve hidden-power binding provenance, separate parts with different displayed fields, reject incompatible retained project filenames before forced cleanup, and resolve schematic components absent from the PCB without replacing embedded geometry. Off-board pinless unbound symbols remain native documentation during apply; electrical or Path-bound symbols still require agreement. Differing display fields across units of one component are explicitly rejected rather than overwritten.

Executed verification

All checks ran on this importer branch using CARGO_TARGET_DIR=/home/user/workspace/repo/target:

Command Result
cargo build -p pcbc Passed
cargo nextest run -p pcbc -E 'test(import) | test(schematic)' --no-fail-fast 108 passed, 274 unrelated tests filtered
cargo nextest run -p pcb-kicad-sch -p pcb-component-gen --run-ignored all --no-fail-fast 231 passed, none skipped; includes native alias export and review regressions
cargo clippy -p pcbc -p pcb-kicad-sch -p pcb-component-gen --all-targets -- -D warnings Passed
cargo test --doc -p pcbc -p pcb-kicad-sch -p pcb-component-gen Passed; zero doctests
cargo fmt --all -- --check; git diff --check Passed; no snapshots changed/accepted

Repository regressions cover original schematic/hierarchy preservation, same-sheet LM358 units 1/2/3, duplicate pin names, hidden/stacked NCs, native-vs-generated partitions, parity diagnostics, source/PCB preservation and unchanged second apply. Electrical fixture variants omit unsourced simulation parts from the BOM and build/apply without suppression; the stacked-NC/legacy BOM fixtures use BOM-only suppression. No electrical checks are suppressed.

Public replay: limited compatibility, not a general Antmicro success claim

Replayed all ten previously tested public repositories on the initial rebased importer commit with KiCad CLI 10.0.6, upgrading only disposable working copies. 19 attempts: four successes, fifteen documented failures, unchanged from the prior integration. After the review corrections, reran FTDI and Jetson in both modes: all four still pass with the partition/second-apply/PCB checks below. The other eight repositories were not rerun after review corrections.

Public input revision Project and root-standalone result
FTDI toolkit Both pass; 110 native physical partitions preserved
Jetson AGX CSI adapter Both pass; 177 native partitions preserved, 179 generated partitions after verified NC normalization
CM4 LVDS adapter, DC-SCM breakout, ICT baseboard, USB-C power adapter Both modes reject unsupported buses
D1600E PSU breakout Both verify 170 generated partitions, then reject a reused managed sheet file
Thunderbolt GPU, Thunderbolt PCIe Both modes reject unbound multi-unit identities split across sheets; PCIe also reports a marker conflict
Raspberry Pi CM4 CSI adapter Project references a missing modern schematic; no standalone attempt

For all four successes, executed:

pcbc import <source.kicad_pro-or-kicad_sch> <out> --force
pcbc build <board.zen> --offline
pcbc build <board.zen> --offline -S bom
pcbc apply schematic <board.zen> --offline --no-open -f json -S bom # twice
kicad-cli sch export netlist --format kicadxml -o <xml> <source-or-applied.kicad_sch>

Unsuppressed public builds fail on missing BOM information; -S bom builds and all eight applies pass, with no electrical suppression. Complete native (reference, physical pin) partitions match source exports in both modes. All second applies report changed: false and preserve schematic/project bytes. Both full-project PCBs are unchanged by apply. A disposable Rust probe using pcb-sexpr compares their entire source/applied PCB trees: identical except existing net-name/footprint Path bindings, retaining net codes, placement, routing and geometry.

Jetson Auxiliary was exported to SVG before import and after two applies, rendered in Chromium at 2×, pixel-compared (magick compare -metric AE: 0 (0)) and visually inspected. Symbols, wiring, graphics and NC crosses remain intact with no visible binding metadata.

Review regressions additionally rendered differing descriptions and an off-board pinless documentation symbol through native SVG/Chromium at 2×. Both before/after comparisons returned zero differing pixels and were visually inspected.

Limitations

Buses, managed components in reused sheet files, cross-sheet multi-unit import, and differing per-unit display fields remain unsupported; failures are not bypassed. Detached child-only import was previously observed to fail and was not retested here. FTDI/Jetson standalone imports still lack custom footprint geometry and are not layout-ready. Source ERC/DRC/parity findings remain visible; success does not mean those source designs pass ERC/DRC. First apply may normalize KiCad document structure; the second must be unchanged.

The separate Quiche frontend and real native GUI-save behavior were not exercised. Verification covers shared schematic APIs, CLI workflows, native KiCad exports, and rendering. Only repository/public inputs were used; no private/customer designs or transcripts were accessed.


Note

High Risk
Major refactor of the KiCad import and schematic-reconciliation pipeline, including electrical connectivity semantics and apply-schematic binding on copied native documents.

Overview
KiCad import now keeps the original schematic hierarchy under layout/ and turns on schematic = True so Quiche and pcb apply schematic work on the copied document instead of rebuilding placement from legacy # pcb:sch comments. The large schematic placement / passive-promotion / comment generation path is removed; components are built from page-local embedded symbol definitions, with Value / Footprint / Description (including empty strings) emitted into generated .zen via properties.

After generation, bind_imported_schematic patches copied sheets with managed Path / per-unit UUIDs and hidden pcb:net bindings that map native labels and power pins to Zener nets without changing displayed names or KiCad merge behavior. pcb-kicad-sch extends connectivity for those bindings, treats off-board pinless graphics as native documentation during apply, and exposes marked_no_connect_targets so stacked native no-connects drive NotConnected() validation. Source schematic/PCB parity is reported but no longer blocks import; structural pin/partition checks and final schematic inspection still fail bad conversions.

Reviewed by Cursor Bugbot for commit cfa6d04. Bugbot is set up for automated code reviews on this repo. Configure here.

devin-ai-integration[bot]

This comment was marked as resolved.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Left a non-blocking comment; not approved. Cursor Bugbot did not reach a terminal state within the wait window, and this is a functional user-facing import change rather than a small fixup, so human review is needed. No reviewers were assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

cursor[bot]

This comment was marked as resolved.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Left a non-blocking comment; not approved. Cursor Bugbot skipped on the current head, and this is a functional user-facing import change rather than a small fixup, so human review is needed. No reviewers were assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

devin-ai-integration[bot]

This comment was marked as resolved.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a non-blocking comment; not approved. Cursor Bugbot passed with no unresolved findings that require human review, and Cursor Security Agent was not running. This is a functional user-facing import change rather than a small fixup, so human review is needed. No reviewers were assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

LK commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Review follow-through for @LK is pushed through cfa6d040; no unresolved review threads remain.

Latest-head CI: Linux/macOS/Windows tests, lint, Python tests, runtime checks, publish-bundle evaluation and performance checks pass. The earlier macOS KiCad-discovery test failure is fixed. Both E2E jobs still fail at Test pcb import (E2E), which consumes the private fixture repository. Under the requested repository/public-only data restriction, private fixture contents and their failing-step logs were not inspected. No coverage or electrical validation was disabled. A public reproduction is needed to diagnose those failures; CI is not green.

Local verification: 108 importer/schematic tests; 231 shared tests including ignored/native cases; CLI build, strict scoped Clippy, format/diff checks and doctests pass. FTDI and Jetson project/root-schematic replays pass with BOM-only suppression, exact native physical partitions (110/177), byte-unchanged second applies and preserved physical PCB trees. Description and pinless-documentation native SVG renders match before/after pixel-for-pixel. Per-unit display overrides are explicitly rejected rather than silently collapsed. Quiche frontend and native GUI-save behavior remain untested.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

Comment on lines +43 to +52
let layout_kicad_pro = match layout_kicad_pro {
Some(path) => path,
None => {
let path = layout_dir.join(selection.selected.kicad_sch.with_extension("kicad_pro"));
if !path.exists() {
fs::write(&path, "{}\n")?;
}
path
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Retained PCB bypasses import synchronization

A standalone reimport can retain a matching PCB while layout_kicad_pcb stays absent. Generation then defaults to four layers and skips PCB identity and net synchronization.

Learn more

The standalone force path preserves the existing layout/ directory and accepts its project when the project filename matches the imported schematic. The new project-selection block returns only the .kicad_pro path; layout_kicad_pcb remains None. write_imported_board_zen treats that as a schematic-only layout, uses four copper layers, and never calls the PCB prepatcher. The retained PCB remains discoverable through the board's layout_path, but its layer count, net names, and footprint Paths no longer participate in regeneration.

Example: Import a two-layer board.kicad_pro, then force-import board.kicad_sch into the same output. The matching layout/board.kicad_pro and layout/board.kicad_pcb survive, but the regenerated board declares four layers and the PCB keeps its old generated Paths.

Recommended fix: When standalone reimport accepts a retained matching project, discover its matching .kicad_pcb and populate MaterializedBoard.layout_kicad_pcb. Then run the same stackup extraction and PCB prepatch path as project import. If retaining an existing PCB is not supported, reject this case before cleanup instead of silently treating it as absent.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

This branch has not been deployed

No deployments
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.

1 participant