Skip to content

fix(schematic): resolve analysis pins by placed unit - #349

Merged
mixelpixx merged 1 commit into
mixelpixx:mainfrom
neusse:neusse/fix-multi-unit-analysis
Aug 28, 2026
Merged

fix(schematic): resolve analysis pins by placed unit#349
mixelpixx merged 1 commit into
mixelpixx:mainfrom
neusse:neusse/fix-multi-unit-analysis

Conversation

@neusse

@neusse neusse commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Multi-unit symbols are now resolved through the unit actually placed on the sheet. This prevents analysis and connectivity repair from reporting or snapping to phantom pins, while allowing component queries to see nets on later units.

The PR also completes the requested sch_batch audit with handler-level tests against the real KiCad ECC83 fixture: bulk move, shared-field edit, and both reference-delete tools operate on all three units without changing a neighboring component or the embedded library definition.

Part of #182.

Approach

The affected analysis and export handlers now reuse the existing shared placed_pins_by_reference / placed_pins path, which selects common pins plus the placed unit before applying that unit's transform. This removes the duplicated extract_lib_pins logic that superimposed every library unit on every placement.

export_netlist_summary still emits one entry per placed symbol instance and preserves unresolved instances with an empty pin list. fix_connectivity changes only its candidate pin endpoints; labels and wire endpoints are unchanged.

design_review.rs is intentionally excluded because PR #336 owns that file. Its author has been asked to make the same unit-aware correction and add ECC83 coverage.

Compatibility and safety

No tool names, schemas, response fields, configuration, IPC, or file formats change. Existing single-unit behavior is preserved; multi-unit coordinates, connectivity, and per-placement pin counts are corrected.

The only mutating production path touched is fix_connectivity; its existing dry-run behavior, conditional atomic write, and stale-file protection are unchanged. The narrower target set prevents a wire from being moved to an unplaced unit's phantom pin. Rollback is one commit.

Validation

  • cargo fmt --all -- --check
  • cargo test --workspace --locked --lib --tests
  • cargo test --workspace --locked --doc
  • cargo clippy --workspace --locked --all-targets -- -D warnings
  • cargo test -p konnect-core --locked multi_unit_ -- --nocapture (34 passed)
  • Real KiCad ECC83 schematic fixture exercised through registered MCP tool handlers
  • Live KiCad GUI/e2e checks were not run; these S-expression paths do not require a running editor, and the relevant environment-dependent tests remain ignored as designed

Review checklist

  • The diff is focused and contains no generated output, personal data, or unrelated cleanup.
  • No new public names or renames.
  • New behavior and failure paths have regression coverage.
  • File mutations retain the existing atomic-write and stale-source protections.
  • No IPC mutation changes.
  • No tools were added or removed, so tool counts and directory documentation are unchanged.

@neusse
neusse force-pushed the neusse/fix-multi-unit-analysis branch from 57ad38e to 731835b Compare August 27, 2026 20:17
@neusse

neusse commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Release-integration note: Konnect v0.10.0 landed after this branchs earlier green CI, so I rebased the multi-unit analysis patch onto current main and re-ran the multi-unit suite. All 34 focused tests pass across analysis, export, sch_batch, and existing component mutations. This PR intentionally remains Part of #182: design_review still has unit-blind walks owned by the remaining work, so the issue must not close on this merge. Fresh CI is now authoritative.

@neusse

neusse commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Release 10 blind-side revalidation note: v0.10.0 changed the surrounding schematic guidance and repository test surface after this PR was opened. This draft was rebased onto current upstream, the real ECC83 multi-unit slice was rerun, and all platform CI is green. Scope remains deliberately partial: design_review in #336 must become unit-aware before #182 closes.

@neusse
neusse marked this pull request as ready for review August 28, 2026 02:29
@mixelpixx
mixelpixx merged commit c43bcb4 into mixelpixx:main Aug 28, 2026
10 checks passed
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.

2 participants