fix(import): split collapsed unconnected stacked-pad nets using PCB layout names - #1204
detail-app[bot] wants to merge 1 commit into
Conversation
| let layout_name = matching_layout_names | ||
| .into_iter() | ||
| .next() | ||
| .unwrap_or_else(|| collapsed_name.clone()); |
There was a problem hiding this comment.
🔴 Missing layout names preserve shorts
When a collapsed endpoint lacks a layout assignment, unwrap_or_else places it on the shared base net. Any endpoint already using that base joins it, so the import recreates the unintended short.
Prompt for agents
In crates/pcbc/src/import/extract.rs, restore_unconnected_net_suffixes_from_layout groups every port without a matching layout net name under collapsed_name. This collides with a port whose layout name is the exact base and also groups multiple unnamed ports together, preserving the short the function is intended to remove. Assign each unresolved endpoint a deterministic unique net identity while preserving an available layout base name for at most one endpoint. Avoid collisions with existing net names and add tests for base-plus-missing and multiple-missing cases.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Investigation for akhilles: the fallback does retain a shared partition for base-plus-missing or multiple-missing endpoints. However, assigning unique nets to those endpoints is not justified by missing PCB evidence. KiCad master exports expanded stacked pad numbers as nodes in one schematic net (eeschema/netlist_exporters/netlist_exporter_xml.cpp, makeListOfNets), then PCBNew independently suffixes valid no-connect pad names (pcbnew/netlist_reader/board_netlist_updater.cpp, updateFootprintNets). Missing PCB assignments also occur when no matching netlist endpoint exists; they do not establish isolation. The stacked-notation support was introduced after KiCad 9.0, so the PR’s KiCad 9+ premise is also too broad. This comment remains unresolved pending the source-of-truth decision: whether import may replace schematic connectivity with PCB-derived no-connect partitions. No synthetic endpoint identities or defensive reconstruction added; green CI will not clear the existing behavior hold.
There was a problem hiding this comment.
Stale comment
Left a non-blocking comment and did not approve; no reviewers were assigned. Bugbot and Security Agent completed with no findings, but this is a functional
pcb importconnectivity change rather than a small fixup, so it still needs human review.Sent by Cursor Approval Agent: Pull Request Router and Approver
ced5fd0 to
5014766
Compare
There was a problem hiding this comment.
Stale comment
Left a non-blocking comment and did not approve; no reviewers were assigned. Bugbot completed with no findings and Security Agent was not running, but this is a functional
pcb importconnectivity change rather than a small fixup, so it still needs human review.Sent by Cursor Approval Agent: Pull Request Router and Approver
5014766 to
0fb8284
Compare
There was a problem hiding this comment.
Left a non-blocking comment and did not approve; no reviewers were assigned. Bugbot completed with no findings and Security Agent was not running, but this is a functional pcb import connectivity change rather than a small fixup, so it still needs human review.
Sent by Cursor Approval Agent: Pull Request Router and Approver


Detail bug report: View on Detail
Bug
pcb importof a KiCad 9+ project could silently short two unconnected pads into one net. KiCad's schematic netlist collapses an unconnected stacked pin (e.g.[5,6]) into a singleunconnected-(REF-Pad5)net whose block carries two distinct<node>entries (pin "5"andpin "6"), givingImportNetData { ports.len() == 2 }. The PCB layout, by contrast, keeps the two pads disambiguated as<base>and<base>_1— namesextract_kicad_layout_dataalready harvests ontoImportLayoutPad.net_names.extract_irnever reconciled the collapsed net against those per-pad names, so the 2-portunconnected-(...)net flowed intoImportIr.netsunchanged. Downstreambuild_not_connected_netsonly classifiesunconnected-(nets withports.len() == 1, so the collapsed net was treated as a real net and both pins were bound to one generatedNet(...), creating an unintended short. The in-repoverify_physical_partitionscouldn't catch it because both the source and built partitions carried the same collapsed{{REF.5},{REF.6}}set. This violates the README invariant: "A source pin with no connection may remain isolated on its own generated net. It must not share that net with another endpoint."Fix
After
extract_kicad_layout_dataon the project-import branch, call a newrestore_unconnected_net_suffixes_from_layout(&mut netlist.nets, &netlist.components)?(crates/pcbc/src/import/extract.rs). It splits anyunconnected-(net withports.len() > 1into one net per distinct per-pad layout name already collected onImportLayoutPad.net_names:is_disambiguated_unconnected_namepredicate accepts only the exact<base>form and<base>_<all-digits>(e.g.unconnected-(U1-Pad5)andunconnected-(U1-Pad5)_1), so unrelated nets that merely share a string prefix are never stolen.The standalone-schematic branch is untouched (the restore call is gated on
ImportSourceKind::Project). Added oneCHANGELOG.mdbullet underUnreleased > Fixed, perAGENTS.md.Testing
cargo check --workspace --locked,cargo fmt --all -- --check,cargo clippy --workspace --all-targets --all-features -- -D warnings, andcargo test --doc -p pcbcall pass clean (zero warnings).extract.rscovering the split of the collapsed 2-port net, the no-op pass-through of single-port unconnected nets (KiCad 7/8 case), real named multi-port nets being left intact, the multi-component split with the no-layout-name fallback, the hard-error path for an ambiguous pad, and the predicate's accept/reject edge cases.cargo nextest run -p pcbc import::extract→ 17/17 pass.cargo nextest run -p pcbc import→ 97/98 pass after installingkicad-cli9.0.9. The one failure (missing_footprint_assignment_preserves_unset_marker_and_still_imports) is a pre-existingpcb-component-genfootprint-rendering fixture mismatch, confirmed identical on the clean baseline (verified bygit stash+ rerun), and unrelated to net extraction.[5,6]pin and ranpcbc importagainst it (release binary) — exit 0, generatedU(name="U1", P5_6=NotConnected()), a single isolated no-connect with no short.Pad5net name fromGetSmallestStackedPadNumber, expanded into two<node>entries) does not reproduce on the installedkicad-cli9.0.9 release — itsstring_utils.cpplacksExpandStackedPinNotationentirely, so the pin stays as[5,6]single-node. I corroborated the fix against the bug report's quoted master netlist via a parser-level E2E: feeding the 2-portunconnected-(U1-Pad5)net block (nodespin "5"/pin "6") through the realparse_kicad_sexpr_netlistyieldsports.len() == 2, andrestore_unconnected_net_suffixes_from_layoutthen splits it into two 1-port nets. (That throwaway test was reverted before commit.)project_import_preserves_sources_and_existing_archive_behavior) passes both at baseline and with the fix.cargo nextest run --workspace --profile ci --locked→ 2390/2408 pass with the fix vs 2384/2402 on the clean baseline. The same 18 pre-existing environmental failures appear in both (14 inpcb-layout, which this PR doesn't touch; 4 inpcbc::integrationrequiring board tooling/network/fixture-format) — zero new regressions, 6 new passing tests.Automatic Fixes PRs can be configured here.
Note
Medium Risk
Changes net identity during KiCad project import and could mis-split nets if layout names are ambiguous or malformed, though scope is limited to multi-port
unconnected-(nets with strict suffix rules.Overview
Fixes a KiCad 9+ project import bug where schematic netlists can merge stacked unconnected pins (e.g.
[5,6]) into one multi-portunconnected-(...)net while the PCB keeps separate names likeunconnected-(U1-Pad5)andunconnected-(U1-Pad5)_1. That collapse could makepcb importtreat the net as a real connection and short two pads that should stay isolated.After layout data is extracted for project imports, the importer now runs
restore_unconnected_net_suffixes_from_layout, which re-splits any multi-portunconnected-(net using per-pad layout net names fromImportLayoutPad.net_names. Matching is limited to the base name and numeric_Nsuffixes viais_disambiguated_unconnected_name; ambiguous pads error, and pads without layout names keep the collapsed base so nothing is dropped. Standalone schematic import is unchanged.CHANGELOG documents the fix under 0.4.50 Fixed. Six unit tests cover split behavior, pass-through, real nets, cross-component cases, errors, and suffix matching.
Reviewed by Cursor Bugbot for commit 0fb8284. Bugbot is set up for automated code reviews on this repo. Configure here.