Closes#126.
A placed schematic symbol carries its reference designator in two places:
(symbol
(property "Reference" "R5" …) ← what eeschema renders
(instances
(project "MyProject"
(path "/sheet-uuid/symbol-uuid"
(reference "R5") ← what netlist + PCB sync read
(unit 1) )))
…)
Before this change, `edit_schematic_component` with `newReference` updated
only the (property "Reference" …) field. The (reference "…") leaves inside
(instances) → (project) → (path) kept the old value. eeschema rendered the
new reference correctly and ERC passed, but `kicad-cli sch export netlist`
and "Update PCB from Schematic" both read from the (instances) block and
silently used the OLD reference — producing destructive PCB-sync diffs on
what users thought was a clean rename. Severity was high for anyone running
batch renames because the symptom only surfaces at PCB-sync time, by which
point many renames may be queued.
Walk the (instances) subtree within the matched symbol block after the
property update and replace every `(reference "OLD")` leaf with the new
value. The regex matches `(reference "X")` specifically (not
`(property "Reference" "X"`), and the walk is constrained to the
(instances …) range via the existing _find_matching_paren helper so other
(reference …) tokens elsewhere in the file can't be affected.
Adds tests/test_edit_schematic_component_instances.py covering:
- Single-instance rename updates both property and instances leaf
- Hierarchical case with multiple (path …) entries all updated atomically
- No-instances-block schematics don't crash (older KiCad / partial files)
- The regex doesn't clobber (property "Reference" …) on the instances pass
- Other field values (Value, Footprint) are left intact
- The response payload's updated.reference reflects the new ref
All 6 tests fail on main without the fix (3 fully, 3 on the instances
assertions only) and pass on this branch.
The pre-existing TestAddComponentMirrorParam failures in
test_add_schematic_component.py are unrelated and present on main —
documented in inktomi's PR #169.
Co-authored-by: mixelpixx <11727006+mixelpixx@users.noreply.github.com>
JLCPCBPartsManager defaulted db_path to a "data/" directory computed
relative to __file__, which fails with read-only filesystems when the
package is installed to a system-managed prefix (e.g. /nix/store, an
immutable container image, or /usr/lib). The same pattern in
download_jlcpcb.py would silently scatter the ~1.5 GB JLCPCB cache
inside the install tree even when it is writable.
The original integration plan (docs/archive/JLCPCB_INTEGRATION_PLAN.md)
called for a per-user database under ~/.kicad-mcp/. This change moves
the default to the platform-appropriate user data directory by adding
a new PlatformHelper.get_data_dir() helper that mirrors the existing
get_config_dir() / get_cache_dir() conventions:
- Linux: XDG_DATA_HOME/kicad-mcp or ~/.local/share/kicad-mcp
- macOS: ~/Library/Application Support/kicad-mcp
- Windows: %USERPROFILE%\.kicad-mcp\data
Both JLCPCBPartsManager and download_jlcpcb.py now resolve their
database paths through this helper. ensure_directories() and
detect_platform() include the new directory. Unit tests parallel to
the existing config_dir/cache_dir cases cover platform-appropriate
paths and the relative-XDG_DATA_HOME edge case.
* Feat: save 2D board view to file instead of returning base64
The 2D view was returning base64-encoded image data in JSON, which
often exceeded token/message size limits. Now saves the rendered
image (PNG/JPG/SVG) next to the PCB file and returns the file path.
This makes the output usable by tools that can read image files
directly.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(board): add opt-in responseMode param to get_board_2d_view
Add a responseMode string parameter (enum: inline | file, default inline)
so callers can choose how the rendered image is delivered.
- inline (default, pre-PR behavior): image bytes are base64-encoded
and returned in the imageData response field -- backward-compatible.
- file: image is written next to the .kicad_pcb as
<board>_2d_view.<ext> and filePath is returned -- resolves the
MCP message-size limit problem on large boards.
Rendering logic is shared between both modes; only response packaging
differs. Updated tool schema (Python + TypeScript) and replaced the
existing test file with 5 focused unit tests covering inline/file modes
for PNG and SVG formats plus the default-is-inline contract.
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: guard SWIG auto-save against external file changes
After every board-mutating SWIG command, kicad_interface._auto_save_board()
unconditionally calls pcbnew.SaveBoard() with the in-memory board. When the
on-disk .kicad_pcb has been modified externally between our LoadBoard and
SaveBoard (KiCad GUI's own save, git checkout, another process), the
in-memory state silently overwrites those external changes - losing data
the user can't see was at risk.
This change records the file's mtime_ns + sha256 at LoadBoard and verifies
the signature matches before each auto-save:
* If the signature has diverged, refuse the save and attach a structured
warning to the command result so callers know their mutation is
in-memory only and they need to reload before retrying.
* If it matches, copy the existing file to .mcp-backups/<name>.<ts>
(rotating, keeps last 20) before overwriting.
* Update the recorded signature after our own writes so subsequent
saves are not falsely flagged.
Backwards compatible:
* No tool schemas changed.
* Successful saves return as before, with an extra `autoSave` field
when the wrapper observed something noteworthy.
* Refused saves return success: true (the in-memory mutation did
succeed) plus warnings: [...] and autoSave.diskChangedExternally,
so callers can detect the situation programmatically.
Adds tests/test_auto_save_guard.py (10 tests, all passing) covering:
signature math, refusal on external change, backup creation + content,
backup rotation, first-save semantics (no recorded signature proceeds
normally), and skip cases (no board / no path).
Motivation: the aircam-pdb fork-user lost ~480 traces and the full
footprint layout to a silent overwrite incident on 2026-05-03; recovery
was only possible because VS Code's local-history extension happened to
have a snapshot from a few minutes earlier. This guard makes that class
of incident loud and locally recoverable.
* fix(auto-save-guard): refuse only on content divergence, not mtime
The guard added in 9ba0010 records `(mtime_ns, sha256)` as the file's
disk signature and refuses auto-save when the recorded tuple no longer
matches the current one. Comparing the full tuple meant any mtime delta
fired the refusal — including a bare `touch` of the file, an atime-style
backup tool, or any MCP read path that opened the .kicad_pcb between
load and save. Users were trapped: every write needed an explicit
save_project call to bypass the false positive (documented as a
workaround in fork users' notes).
Compare on sha256 only; mtime is incidental. The actual data-loss
scenario the guard is meant to catch — an external write that genuinely
changed the file — produces a different content hash, which is what the
guard now keys off. After a touch-only mtime advance with content
unchanged, refresh the recorded signature so we don't re-hash on every
subsequent call.
Drops the mtime-equality fast path on _disk_signature: a filesystem with
coarse mtime resolution (FAT32, some network mounts) could accept two
writes inside one mtime tick; trusting mtime as a hash cache key would
re-introduce a class of silent overwrites the guard exists to prevent.
The hash itself is cheap (sha256 over a typical .kicad_pcb completes in
tens of milliseconds).
Adds 4 regression tests in test_auto_save_guard.py:
- touch-only mtime advance proceeds and refreshes the signature
- content change at the same mtime is still refused (hash divergence
must drive the decision, not tuple equality)
- the user-facing warning calls out "contents", not "mtime"
- _disk_signature returns the same hash when content is unchanged
even after the file's mtime advances
The handler iterated `board.GetFootprints()` and assigned nets to existing
pads, but had no path to *add* footprints for schematic symbols whose
Reference was not yet on the board. New parts placed on the schematic
landed in the net list with no PCB representation — the rats nest had
nowhere to terminate, and place_component / route_pad_to_pad would fail
because the target footprint did not exist.
KiCad's "Update PCB from Schematic" (F8) implicitly adds the missing
footprints; bring the MCP's behaviour in line.
Implementation:
* `_extract_components_from_schematic` runs `kicad-cli sch export netlist
--format kicadxml` (the same path `_handle_generate_netlist` already
uses) and returns a flat `[{reference, value, footprint}]` list. Walks
hierarchical sub-sheets transparently because kicad-cli does.
* `_add_missing_footprints_from_schematic` resolves each missing component
against the project's fp-lib-table via `LibraryManager`, calls
`pcbnew.FootprintLoad`, sets reference / value / FPID, and places the
footprint at the board origin (the user / autoplacer can position it).
Power and flag references (`#PWR…`, `#FLG…`) are excluded — they have
no PCB footprint.
* The pad-net assignment loop now runs *after* the add path, so newly
placed footprints get their nets assigned in the same call.
* Response payload gains `footprints_added` and `footprints_skipped`
diagnostic lists. The textual `message` field reports both the new
footprint count and the existing net / pad counts.
Adds tests/test_sync_schematic_to_board_footprints.py — 9 unit tests
covering the add path (missing ref, already-present ref, power refs,
empty footprint, unknown library) and the kicad-cli helper (XML parse,
missing kicad-cli, non-zero exit).
WireManager.add_wire and add_polyline_wire bailed out with
"No sheet_instances section found in schematic" when called on a
hierarchical sub-sheet — that block only exists in the root .kicad_sch.
The handler in kicad_interface.py converted the False return into a
flat "Failed to add wire" message, leaving callers with no diagnostic.
Apply the same fallback that WireManager.add_label has used since the
hierarchical-design support: when (sheet_instances ...) is absent,
append the new (wire ...) item at the end of the outer (kicad_sch ...)
form.
Adds tests/test_add_wire_sub_sheet.py with 6 regression tests covering
both add_wire and add_polyline_wire on a sub-sheet, including paren
balance, sexpdata round-trip, and segment count for an N-point polyline.
DynamicSymbolLoader.create_component_instance only handled root
schematics: it located its insertion point via `(sheet_instances`,
which exists only in the root .kicad_sch. Adding a component to any
hierarchical sub-sheet raised "Could not find insertion point in
schematic" and aborted the call.
Fall back to inserting just before the closing paren of the outer
(kicad_sch ...) form when the marker is absent. WireManager.add_label
already uses the same fallback for hierarchical sub-sheets.
Adds three regression tests in TestCreateComponentInstanceSubSheet
covering successful insertion, paren balance, and sexpdata round-trip
on a sub-sheet without (sheet_instances).
`BoardOutlineCommands.add_mounting_hole` produced footprints with an empty
library:name id (`(footprint "" ...)` in the .kicad_pcb), which KiCad's GUI
Move tool refuses to select — users couldn't drag the resulting MHs in the
editor. It also emitted the pad with the default `*.Cu` + `*.Mask` LSET on
NPTHs; with `padDiameter > diameter` that creates phantom copper annular
rings on every Cu layer and trips clearance DRC against neighbouring nets.
Repro: call `add_mounting_hole` with `position={x:117,y:84.5,unit:"mm"},
diameter:3.2, padDiameter:3.5`. The resulting MH is unselectable and DRC
reports phantom F.Cu pad shorts to neighbouring component pads.
Changes:
- Set a real FPID via `module.SetFPID(pcbnew.LIB_ID(lib, name))`. Default
is `MountingHole:MountingHole_<diameter:g>mm` (e.g. 3.2 → 3.2mm); a new
optional `footprintLibId` parameter overrides.
- For NPTH (the default `plated:false`), restrict the pad's LSET to
`F.Mask` + `B.Mask` only. PTH path is unchanged — the default Cu+Mask
LSET is correct there.
- Update the schema in `tool_schemas.py`: previously advertised
`x`/`y`/`diameter` at the top level, but the impl reads
`position={x,y,unit}`, `padDiameter`, `plated`. Schema now matches the
implementation and exposes the new `footprintLibId` param.
- New `tests/test_add_mounting_hole.py` regression suite (7 tests) asserting
SetFPID is invoked with non-empty lib:name (default + override forms),
NPTH SetLayerSet excludes any Cu layer, and PTH does not call
SetLayerSet (preserves default Cu+Mask).
Some community-generated KiCad symbol libraries (e.g.
PCM_Diode_Schottky_AKL:MBRS130) define each pin number twice — once as a
visible "real" pin with non-zero length whose ``at`` coordinate is the
wire-connection endpoint, and once as an inner zero-length "ghost" pin
used as an internal graphic-anchoring join. Both definitions live inside
the same ``lib_symbols`` block.
``parse_symbol_definition`` stored pins via ``pins[number] = pin_data``
— a plain assignment. Each duplicate-numbered pin encountered during the
recursive walk overwrote the previous one. The recursion order put the
ghost pins last for MBRS130, so the ghost won and ``get_pin_location``
returned a coordinate that did not match any wire/label.
Downstream this caused ``get_connections_for_net`` to silently miss diode
pins on the rails they were wired to — on a real schematic, querying
``+BATT`` returned 8 of 9 expected nodes (D1/1 absent) and ``+3V3``
returned 44 of 46 (D1/2 and D2/2 absent), because the BFS could not find
the diode's pin endpoint at the labelled position.
Fix: when the same pin number is defined more than once, keep the entry
with the greater ``length``. The outer real pin has length > 0; the
inner ghost has length == 0. Strict-greater comparison resolves ties to
first-encountered, so legitimate same-length duplicates (e.g., per-unit
repetitions in multi-unit symbols) keep stable existing behaviour.
Tests: four unit tests in ``tests/test_pin_locator_duplicate_pin_defs.py``
cover (a) outer-then-ghost order (the real bug), (b) ghost-then-outer
order (length-not-order heuristic), (c) no-duplicate baseline regression,
and (d) equal-length tie keeps first-encountered. Two of the four fail
on main, all four pass on this branch.
Full suite: 671 passed, 11 skipped, 0 regressions (modulo the pre-existing
tests/test_get_pin_angle.py collection error which is unrelated to
pin_locator).
End-to-end on the real schematic that triggered the report: after the
fix ``get_connections_for_net('+BATT')`` returns all 9 expected nodes
matching ``kicad-cli sch export netlist`` exactly. The companion fix in
PR #177 (wire_connectivity pwr-flag bridge) closes the orthogonal
over-merge bug; together they bring net membership to full parity with
kicad-cli on schematics that use both ``PWR_FLAG`` markers and
diodes from community libraries.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every power:PWR_FLAG symbol carries Value="PWR_FLAG" — the ERC marker
inherits its rail's name from the wire/label it sits on. Commit 7f3a379
added #FLG symbols to the same handling loop as #PWR power ports inside
_parse_virtual_connections, so every pwr-flag was getting appended to
label_to_points["PWR_FLAG"]. The BFS in _find_connected_wires uses
label_to_points for virtual jumps; reaching any pwr-flag pin caused it
to teleport to every other pwr-flag pin, walking across each one's stub
wire into a different power rail. The result: get_net_connections(rail)
returned the union of pins on every rail that had a pwr-flag, for any
rail.
Fix: pwr-flag pin positions still register as anchors in point_to_label
(preserving the original intent of 7f3a379 so find_orphaned_wires keeps
accepting them), but they no longer enter label_to_points. The pwr-flag
remains electrically connected to its rail via the wire-graph BFS through
the wire it sits on; the label-jump mechanism is unnecessary for that
path and actively harmful when the "label" is the same for unrelated
rails.
Tests: three unit tests on _parse_virtual_connections cover the bug
(over-merge gone), regression check (power ports still work in both maps),
and edge case (pwr-flag and port at same point — port name wins). All
three fail on main, pass on this branch.
Full suite: 667 passed, 11 skipped, 0 regressions (modulo the pre-existing
tests/test_get_pin_angle.py collection error which is unrelated to
wire_connectivity).
End-to-end verification on a single-sheet schematic with 7 distinct
power rails each carrying a pwr-flag: every queried net now matches
the official kicad-cli netlist output (modulo a separate library-symbol
bug on PCM_Diode_Schottky_AKL:MBRS130 with duplicate pin definitions,
out of scope here).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dynamic_symbol_loader only consulted the project-local sym-lib-table
and a hardcoded list of bundled symbol directories, so libraries
registered via the user-global sym-lib-table (Preferences > Manage
Symbol Libraries > Global) were invisible to add_schematic_component.
Common case: company libraries that live under OneDrive / a network
share / any other custom path the user added through the GUI.
Also widened the sym-lib-table parser regex to accept quoted URIs (and
quoted names) that contain spaces — required for paths like
"C:/Users/.../OneDrive - Company/Documents/KiCad/...". The old bare-
word capture stopped at the first space.
Search order is now:
1. Project sym-lib-table
2. User-global sym-lib-table (~/AppData/Roaming/kicad/<ver>/sym-lib-table
on Windows, ~/.config/kicad/<ver>/sym-lib-table on Linux,
~/Library/Preferences/kicad/<ver>/sym-lib-table on macOS)
3. Bundled / well-known symbol directories
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cairocffi uses cffi's ffi.dlopen('cairo-2') which requires the DLL to
be on the system PATH. On Windows, prepend KiCad's bin directory to
PATH early in kicad_interface.py (before any cairocffi import) so
cairo-2.dll can be found. Checks PYTHONPATH, Python executable dir,
and default KiCad 9/8 install paths.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The _ipc_move_component and _ipc_place_component handlers were ignoring
the unit field from position parameters, always treating values as mm.
When inches were specified, components would be placed at 1/25.4th of
the intended position. Now reads the unit field and converts to mm
before passing to the IPC backend.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
KiCad 9's IPC API returns Footprint objects where fp.definition lacks
the library_link attribute, causing all component queries to fail with
"'Footprint' object has no attribute 'library_link'" warnings and
returning 0 components. This adds a hasattr check with fallback to
fp.definition.id.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The eeschema-ground-truth fixture (_build_mirror_case) passed
'mirror': 'x' to ComponentManager.add_component, which silently drops
the kwarg — so the resulting .kicad_sch had no (mirror x|y) token,
eeschema rendered an unmirrored symbol, and our pin coords (also
unmirrored) tautologically matched. The mirror tests were GREEN both
before and after the rotation/mirror fix in 7e67cb9, providing zero
regression coverage for the mirror semantics.
Fix:
- _build_mirror_case now applies the mirror via the same low-level
helper (WireDragger.update_symbol_rotation_mirror) that
rotate_schematic_component uses, with a guard assertion that the
written file actually contains (mirror x|y).
- Two new pin-down unit tests in test_add_schematic_component.py
document and lock down ComponentManager.add_component's silent-drop
behavior for mirror, so the next person to touch that path knows to
update the eeschema-truth fixture if they grow real mirror support.
Verified: with the production fix at 7e67cb9 reverted, the kicad-cli
mirror tests now go RED (previously they stayed GREEN regardless).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two bugs in WireDragger.pin_world_xy (and corresponding bugs in
PinLocator.get_pin_angle) caused pin coordinates and angles to land on
the wrong pin in 4 of 8 polarized cases (rot=90, rot=270, mirror x on a
vertical part, mirror y on a vertical part). Verified end-to-end against
`kicad-cli sch export netlist`.
(1) Rotation direction. After PR #145's `-ly` Y-flip, calling the
standard math (Y-up CCW) `_rotate` is effectively CW in screen Y-down.
eeschema's TRANSFORM(0,1,-1,0) for rot=90 is screen-CCW. They agreed at
0° and 180° (where the rotation matrices coincide) but disagreed at 90°
and 270°.
(2) Mirror axis semantics swapped. Per eeschema symbol.h:43-44,
SYM_MIRROR_X = TRANSFORM(1,0,0,-1) negates Y, and SYM_MIRROR_Y =
TRANSFORM(-1,0,0,1) negates X. Our code did the inverse: `mirror_x`
negated the X component and `mirror_y` negated the Y component.
Fix shape for `_rotate`: chose option (b) — leave `_rotate` as standard
math and negate the angle at the call site (`_rotate(lx, ly, -rotation)`).
This converts math-CCW to screen-CCW without disturbing
`TestRotatePoint`'s direct expectations of `_rotate`.
Final composition order in `pin_world_xy` matches eeschema's parser
(rotation set first into m_transform, then mirror composed via
`new = old * temp` so the mirror is applied first to the coordinate):
1. Y-flip: ly = -ly (lib Y-up → screen Y-down)
2. Mirror: if mirror_x: ly = -ly (negate screen-Y)
if mirror_y: lx = -lx (negate screen-X)
3. Rotate: _rotate(lx, ly, -rotation) (screen-CCW)
4. Translate: add (sym_x, sym_y)
Verified by hand for {rot=90, rot=270} × {none, mirror_x, mirror_y}
against the TRANSFORM matrices in transform.cpp:44 and symbol.h:43-44.
`PinLocator.get_pin_angle` mirrors the same composition in angle space.
For an angle, Y-flip and mirror_x both negate the angle; mirror_y maps
to (180 - angle). The screen-CCW rotation in `pin_world_xy` corresponds
to subtracting (not adding) the symbol rotation in standard atan2
convention — fixed accordingly. Geometry test
(`test_get_pin_angle.py::test_get_pin_angle_matches_geometric_expectation`)
derives expected angles from `pin_world_xy` itself, so it pins the two
together.
`tests/test_rotate_schematic_mirror.py::test_pin_positions_mirror_x_flips_x`
encoded the OLD inverted semantics and is updated/renamed to
`test_pin_positions_mirror_x_flips_y` with a pin that has non-zero Y so
the assertion is meaningful under the corrected semantics.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`test_rotate_handler_no_crash` permanently replaced
`sys.modules["schemas.tool_schemas"].TOOL_SCHEMAS` with `[]`, leaking into
later tests. When test_wire_connectivity (or any test) ran after this one
and did `from schemas.tool_schemas import TOOL_SCHEMAS`, it got the empty
list and `TOOL_SCHEMAS["get_wire_connections"]` raised `TypeError: list
indices must be integers or slices, not str`.
Save the original sys.modules entries and restore them in a `finally`
block so the stubs are scoped to the test body. Whole suite now passes
(678 tests, previously 4 failed in TestSchema when run in suite order).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Use None default + 'instances or []' to keep mutable-default lint happy
and satisfy the Iterable annotation.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
All 10 failing tests in test_hierarchical_pad_net_map.py share the same root cause:
broken test fixture, not a production bug. PR #88 (commit 7cafbda, "fix:
get_schematic_pin_locations now accounts for mirror flags") introduced
PinLocator._get_symbol_transform, which reads symbol position/rotation/lib_id
directly from the .kicad_sch file via sexpdata + WireDragger.find_symbol — it
deliberately bypasses the kicad-skip cache so mirror/rotation mutations are
authoritative. The hierarchical-pad-net-map tests, however, only mocked
skip.Schematic and wrote a lib_symbols-only stub to disk with no (symbol ...)
instance, so _get_symbol_transform returned None and every pin lookup failed
with "Could not read transform for R1/R2". The tests last passed at 8a42812
(introduction) and broke at 7cafbda; PR #145 did not touch this code path.
Per-test classification (all 10): broken fixture.
- TestLabelAtPin::test_global_label_pin1 — fixture
- TestLabelAtPin::test_global_label_pin2 — fixture
- TestLabelAtPin::test_both_pins_mapped — fixture
- TestLabelAtPin::test_local_label_also_works — fixture
- TestLabelAtPin::test_hierarchical_label_also_works — fixture
- TestLabelViaWire::test_label_one_hop_away — fixture
- TestLabelViaWire::test_label_two_hops_away — fixture
- TestMultipleSubsheets::test_components_in_subsheet_collected — fixture
- TestMultipleSubsheets::test_top_and_sub_components_merged — fixture
- TestRotatedSymbol::test_90_degree_rotation — fixture (rotation expected
values already match the post-PR-145 convention; only fixture needed)
Fix: add _build_sch_with_instances() helper that emits a real (symbol ...)
block alongside the existing TestLib:R lib_symbols, so sexpdata can resolve
the transform. The skip.Schematic mock is still used for labels and wires.
All 15 tests in this file now pass; the broader related set
(test_pin_locator_y_flip, test_get_pin_angle, test_move_with_wire_preservation,
test_pin_locator_and_component) also pass — 84 total.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Resolves the four failing tests in tests/test_pin_locator_and_component.py
left behind by the PR #145 / commit 3c22580 Y-flip work.
Per-test rationale:
- TestPinLocatorYAxisNegation::{test_pin1_y_above_center_for_rotation_0,
test_pin2_y_below_center_for_rotation_0, test_pin1_rotated_90}: stale.
Their assertions encoded the *correct* post-PR-145 convention (96.19,
103.81, etc.), but their setup MagicMock'd self._schematic_cache while
bypassing _get_symbol_transform, which reads the .kicad_sch file
directly via sexpdata. The end-to-end Y-flip behaviour is already
covered against eeschema in tests/test_pin_locator_y_flip.py — keeping
three mock-based duplicates added no value, so they were removed.
- TestPinLocatorReferenceRstrip::test_get_pin_location_finds_symbol_with_trailing_underscore:
revealed a real production bug. PinLocator.get_pin_location strips a
trailing "_" on the kicad-skip lookup path, but the sexpdata-based
_get_symbol_transform delegates to WireDragger.find_symbol which used an
exact-equality comparison. With kicad-skip's "R1_" artifact the function
returned None, so the whole pin-location call failed even when the symbol
was clearly present. Fixed find_symbol to apply the same rstrip("_") on
the stored reference before comparing, mirroring the existing behaviour
in PinLocator. The test was also rewritten to use a real temp .kicad_sch
(with the on-disk reference mangled to "R1_") so it actually exercises
both lookup paths instead of bypassing one with mocks.
Files changed:
- python/commands/wire_dragger.py:78-89 — rstrip("_") on the reference
read out of the symbol property before comparing to the caller-supplied
reference.
- tests/test_pin_locator_and_component.py — removed three stale mock-based
Y-axis tests (covered by tests/test_pin_locator_y_flip.py end-to-end);
rewrote rstrip tests to use a real schematic file so _get_symbol_transform
is actually exercised.
Verified: tests/test_pin_locator_and_component.py + test_pin_locator_y_flip.py
+ test_get_pin_angle.py + test_move_with_wire_preservation.py — 69 passed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
After every board-mutating SWIG command, kicad_interface._auto_save_board()
unconditionally calls pcbnew.SaveBoard() with the in-memory board. When the
on-disk .kicad_pcb has been modified externally between our LoadBoard and
SaveBoard (KiCad GUI's own save, git checkout, another process), the
in-memory state silently overwrites those external changes - losing data
the user can't see was at risk.
This change records the file's mtime_ns + sha256 at LoadBoard and verifies
the signature matches before each auto-save:
* If the signature has diverged, refuse the save and attach a structured
warning to the command result so callers know their mutation is
in-memory only and they need to reload before retrying.
* If it matches, copy the existing file to .mcp-backups/<name>.<ts>
(rotating, keeps last 20) before overwriting.
* Update the recorded signature after our own writes so subsequent
saves are not falsely flagged.
Backwards compatible:
* No tool schemas changed.
* Successful saves return as before, with an extra `autoSave` field
when the wrapper observed something noteworthy.
* Refused saves return success: true (the in-memory mutation did
succeed) plus warnings: [...] and autoSave.diskChangedExternally,
so callers can detect the situation programmatically.
Adds tests/test_auto_save_guard.py (10 tests, all passing) covering:
signature math, refusal on external change, backup creation + content,
backup rotation, first-save semantics (no recorded signature proceeds
normally), and skip cases (no board / no path).
Motivation: the aircam-pdb fork-user lost ~480 traces and the full
footprint layout to a silent overwrite incident on 2026-05-03; recovery
was only possible because VS Code's local-history extension happened to
have a snapshot from a few minutes earlier. This guard makes that class
of incident loud and locally recoverable.
The KiCad s-expression parser rejects raw newline and carriage-return
characters inside quoted string literals — a multi-line text annotation
written through `add_text` produced a `.kicad_sch` file that eeschema
silently tolerated but `kicad-cli sch ...` refused with "Failed to load
schematic." The escape pass only handled backslashes and double quotes.
Add `\\n` → `\\\\n` and `\\r` → `\\\\r` to the same escape chain. Order
matters: backslashes are escaped first so we don't double-escape our
own escapes.
A new regression test (`test_escapes_newlines_in_multiline_text`)
checks both that the resulting quoted string literal contains no raw
newline characters and that the file round-trips cleanly through the
sexpdata parser.
End-to-end smoke: a 4-line annotation written through the patched
add_text now passes `kicad-cli sch erc` (exit 0) where the previous
behaviour failed parse.
Note: the same escape gap exists in `_make_hierarchical_label_text`
and `_make_sheet_pin_text` for unescaped quotes/newlines in the user-
supplied text. Not fixed here to keep this PR scoped to the documented
add_text bug; happy to fold it in if a reviewer prefers.
Library symbol pins are stored Y-up (positive Y is upward in the symbol
editor's coordinate system) but `.kicad_sch` is Y-down (positive Y is
downward in the schematic). `pin_world_xy` was returning `sym_y + ry`
without negating the rotated lib Y, so for any non-symmetric symbol
pin 1 and pin 2 ended up at swapped world positions.
For symmetric two-pin passives (R, C non-polarized) this was invisible
because pin 1 and pin 2 are electrically equivalent. For polarized
parts — electrolytic and polymer caps, diodes, MOSFETs, BJTs — it
silently swapped polarity. A label snapped to a polarized cap's pin 1
ended up on pin 2, which is catastrophic at first power-up.
The order matches eeschema's actual transformation:
mirror in lib space → Y-flip to screen → rotate → translate.
The existing regression test in test_pin_locator_y_flip.py was already
written with the correct expected coordinates but the matching code fix
was never landed; that test now passes.
Three tests in test_move_with_wire_preservation.py had baked the buggy
expected coordinates into their assertions; updated those to the correct
y-flipped values. The touching-pin fixture had to flip R2's Y from
-7.62 to +7.62 so the two pins still meet under the corrected formula.
Verified end-to-end on a 46-component aerospace PDB schematic: all
8 polarized-part pins (4 polymer caps + 4 TVS diodes) now produce
world coordinates that match the labels actually placed in the file.
1. add_schematic_net_label failed on schematics with no existing labels.
The clone-based path required a pre-existing label to copy from;
the documented "fallback to sexpdata" was a misleading log line —
the RuntimeError was caught and the call silently returned False.
Restore hand-built sexpdata construction (without the buggy
fields_autoplaced token, with orientation-aware justify).
2. get_pin_angle returned the wrong angle for every mirrored symbol
(off by exactly 180°, all rotations, both mirror axes). The
mirror_x and mirror_y formulas were swapped relative to the
pin_world_xy convention — pin_world_xy mirrors a position by
flipping its local axis component, so the matching angle
transform is (180 - θ) for mirror_x and -θ for mirror_y.
Add regression tests:
- test_add_label_empty_schematic.py — first label on empty schematic,
orientation-aware justify.
- test_get_pin_angle.py — full 24-case matrix
(4 rotations × 3 mirror states × 2 pins).
Upstream added `from annotations import AnnotationLoader` and moved
`from commands.wire_manager import WireManager` to module-level in
kicad_interface.py. The smoke test now stubs annotations and ensures
python/ is on sys.path so commands.* imports resolve without installing.
Previously the handler used kicad-skip to apply rotation and mirror.
kicad-skip has no API for (mirror x/y) on placed symbols, causing:
'NoneType' object has no attribute 'value'
Fix:
- Rewrote _handle_rotate_schematic_component to use sexpdata (same
approach as move_schematic_component) for both rotation and mirror
- Added WireDragger.compute_pin_positions_for_rotation: computes old
and new pin world positions when rotation/mirror changes at fixed (x,y)
- Added WireDragger.update_symbol_rotation_mirror: updates (at) rotation
and adds/removes/replaces the (mirror x/y) sexpdata token cleanly
- Connected wires now follow pin positions after rotate/mirror via the
existing WireDragger.drag_wires infrastructure
Tests: 10 unit tests in tests/test_rotate_schematic_mirror.py covering
update_symbol_rotation_mirror, compute_pin_positions_for_rotation, and
a handler smoke test.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the manual add_schematic_junction tool with automatic junction
management. WireManager.sync_junctions inserts/removes junction dots
based on wire endpoints plus component pin positions and is invoked
after add_wire, add_polyline_wire, delete_wire, move, and rotate.
- Pin-aware: parses lib_symbols and applies KiCad's mirror/rotate/
translate transform to compute world pin coordinates
- Multi-unit safe: filters lib_symbols sub-units by the placed
symbol's (unit N) field plus the unit-0 common body
- Removes the now-unused WireManager.add_junction static method
- Updates CHANGELOG [Unreleased] with the tool removal notice
- Adds .mcp.json to .gitignore (machine-local paths)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The symbol-to-schematic y-flip was applied twice in sequence (two identical
negation blocks with matching comments), cancelling out and leaving pin
Y-coordinates mirrored about the placement Y. For symmetric passives the
bug is invisible (pin 1 and pin 2 are electrically interchangeable); for
ICs with non-equivalent pins (power pins, opamp inputs, etc.) this causes
tools that go through PinLocator — connect_to_net, add_schematic_connection,
add_schematic_net_label with componentRef+pinNumber — to place connections
at the mirror-flipped pin. Verified against kicad-cli generate_netlist
ground truth: on a Device:R placed at (111.76, 83.82), pin 1 resolves to
y=80.01 (actual) vs y=87.63 (pre-fix).
This is a regression of PR #103, which originally fixed the y-negation;
the redundant second block was added subsequently.
Includes a regression test with both a straight Device:R and a rotated
Device:C to exercise the y-flip + rotation pipeline.
KiCad serialises rescued or locally-customised library entries with an
extra (lib_name "...") child before (lib_id "..."):
(symbol
(lib_name "RESISTOR_0603_4")
(lib_id "MF_Passives:RESISTOR_0603")
(at 132.08 44.45 90)
...)
The block-matching regex in _handle_get_schematic_component,
_handle_edit_schematic_component, and _handle_delete_schematic_component
required (lib_id IMMEDIATELY after (symbol, so any placed component
using this form was silently invisible to lookup. The user-visible
symptom is "Component '<ref>' not found in schematic" even though the
component is plainly present (and reachable through list / IPC paths).
This bug also affected set/remove_schematic_component_property and the
existing footprint/value/reference rewriting paths in edit, since they
all share the same lookup code.
The parent-position lookup used a similarly-strict regex
((symbol (lib_id "...") (at ...))), which silently fell back to (0,0)
on (lib_name)-first symbols and caused new properties added through
the custom-properties path to anchor at the schematic origin instead
of the parent symbol.
Fix: relax the symbol-block opening pattern to (symbol\s+\( — matching
any opening paren after (symbol — and read the symbol's origin from
the first (at ...) inside the block. Library-definition entries inside
(lib_symbols ...) are still excluded by the existing range check
(they use the (symbol "name" ...) form with a quoted string, not a
paren).
Adds 7 regression tests in TestLibNameBeforeLibIdOrdering using a
real-world (lib_name)-first resistor block, covering get / edit /
set-property / remove-property / delete and verifying that newly
added properties anchor to the symbol origin instead of (0, 0).
Promotes BOM / sourcing fields (MPN, Manufacturer, DigiKey_PN, LCSC,
JLCPCB_PN, Voltage, Tolerance, Dielectric, ...) to first-class citizens
on placed schematic symbols.
New MCP tools:
- set_schematic_component_property: add or update one custom property
on a component (convenience wrapper around edit_schematic_component).
- remove_schematic_component_property: delete one custom property.
The four built-in fields (Reference, Value, Footprint, Datasheet) are
protected and rejected.
edit_schematic_component enhancements:
- New `properties` parameter: map of property name to either a string
value or a full spec object { value, x?, y?, angle?, hide?, fontSize? }.
Adds the property when missing, otherwise updates the existing field
(and optionally its label position / visibility). Lets a single tool
call attach an entire BOM payload to a component.
- New `removeProperties` parameter: list of custom property names to
delete in the same call.
- Property values are now backslash-escaped so descriptions containing
a double-quote or a backslash no longer corrupt the .kicad_sch file.
- New properties default to (hide yes) so they appear in BOM exports
without cluttering the schematic canvas.
get_schematic_component description clarified to highlight that it
already returns every field on the symbol, including custom ones.
New MCP prompt component_sourcing_properties guides agents through the
conventional property names recognised by downstream BOM tooling and
the recommended call sequence.
Implementation (python/kicad_interface.py):
- _PROTECTED_PROPERTY_FIELDS frozenset
- _escape_sexpr_string / _find_matching_paren static helpers
- _set_property_in_block / _set_hide_on_property /
_remove_property_from_block surgical text-level edits that preserve
formatting and the property's UUID
- _handle_edit_schematic_component rewritten to orchestrate
add/update/remove and return a per-property summary
- New handlers _handle_set_schematic_component_property and
_handle_remove_schematic_component_property registered in the
command dispatch table
Tests (tests/test_schematic_component_properties.py):
32 tests covering escape helper, paren matcher, add/update/remove
(single + batched), full spec dicts, default position, default
(hide yes), special-character escaping, UUID preservation, protected
built-in field rejection, no-op removal, both new convenience tools,
and input validation. All 590 tests in the project still pass.
Docs: README, SCHEMATIC_TOOLS_REFERENCE, TOOL_INVENTORY, CHANGELOG.
Adds two new MCP tools for working with free-form text annotations
(SCH_TEXT elements) in KiCad schematics:
- add_schematic_text: place a text note with optional angle, font size,
bold/italic, and justification
- list_schematic_texts: list all text annotations with optional
case-insensitive substring filter
Includes WireManager.add_text / list_texts using _text_insert + sexpdata,
handler dispatch in KiCADInterface, TypeScript tool definitions, registry
entry, reference doc updates, and 30 unit tests.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
15 tests covering:
- Empty schematic → empty maps
- Global / local / hierarchical labels at pin endpoints
- Net propagation through one and two wire hops
- Unconnected pin stays absent from map
- Power (#PWR) and flag (#FLG) symbols excluded from component map
- Components in sub-sheets collected alongside top-level components
- 90° rotated symbol produces correct absolute pin positions
Uses MagicMock skip.Schematic objects patched in both the walker
import and commands.pin_locator module namespace; lib_symbols are
provided in real on-disk .kicad_sch files so sexpdata parsing runs
without mocking.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers create_component_instance and _handle_add_schematic_component:
- default unit=1 behaviour
- explicit unit 2 and 4 written to (unit N) in schematic S-expression
- (instances ...) block also records the correct unit
- multiple units of the same reference placed independently
- handler param validation (missing schematicPath, missing component)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the three bugs fixed in the previous commit:
- TestAddComponentNoTrailingUnderscore (integration): verifies clone() + no extra append()
- TestPinLocatorYAxisNegation (unit): rotation=0 and 90° cases with y-negation
- TestPinLocatorReferenceRstrip (unit): lookup tolerates 'R1_' artifact from kicad-skip
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Moves a net label (local, global, or hierarchical) to a new position in
place, avoiding the error-prone delete-then-re-add workflow. Supports an
optional currentPosition disambiguator and labelType filter.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mechanical application of the `.gitattributes` rules from the prior commit.
All 50 files differ only in line endings — verified by
`git diff --cached --ignore-all-space` being empty.
Before: main had 42 CRLF + 27 LF Python files plus mixed-ending in YAML,
templates, and shell scripts. After: every text file is LF (except the
Windows-native *.ps1, *.bat scripts which remain CRLF per gitattributes).
This eliminates the noisy-diff failure mode seen in PR #102, where a
small logic change produced a 918-line diff due to whole-file CRLF→LF
conversion.
Add optional netName (exact case-sensitive match) and labelType
(net/global/power enum) parameters. Both are optional and AND
together when combined. Omitting both preserves current behaviour.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Parser used a 5000-char heuristic slice per symbol; any symbol block
shorter than 5000 chars bled into the next one, and last-write-wins in
the properties dict ensured the neighbor's data clobbered the target.
Reported as Simulation_SPICE:OPAMP returning PJFET data.
Switch to parenthesis-depth tracking to find the true end of each
(symbol ...) block. Also surface Sim.Pins so agents can read opamp pin
numbering without inferring it from schematic placement.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two new tools for managing hierarchical schematic connections:
- add_schematic_hierarchical_label: create sheet interface ports on
sub-sheet schematics. These are the sub-sheet side of hierarchical
connections, linking to sheet pins on the parent.
- add_sheet_pin: add pins to sheet symbol blocks on the parent
schematic. Targets the correct sheet by matching the Sheetname
property. The pinName must match a hierarchical_label in the
sub-sheet.
Both tools use text-based S-expression insertion (not sexpdata
round-trip) to preserve KiCad's native file formatting. Labels
include proper justification based on orientation: left-justify for
rightward labels (0°), right-justify for leftward labels (180°).
Wire manager additions:
- _find_insertion_point(): locates sheet_instances block or final paren
- _text_insert(): inserts formatted S-expression text at the right position
- _make_hierarchical_label_text(): generates hierarchical_label S-expression
- _make_sheet_pin_text(): generates sheet pin S-expression
- WireManager.add_hierarchical_label(): static method for label insertion
- WireManager.add_sheet_pin(): static method for pin insertion into
named sheet blocks
12 unit tests covering insertion, orientation/justification mapping,
parameter validation, multi-sheet targeting, and error handling.
