fix(pcb): write mounting holes under the footprint names KiCad ships

[mixelpixx]

Summary

add_mounting_hole formatted its lib_id as MountingHole:MountingHole_{drill:.1}mm. Checked against the installed KiCad 10 MountingHole.pretty (167 footprints): there is no plain MountingHole_3.2mm — 3.2 mm exists only as the M3 family — and KiCad spells round sizes without a decimal (MountingHole_3mm, never 3.0mm). So the default call and every integer size wrote a name no stock KiCad resolves.

Closes #462

Approach

• Name table, spelled as the library spells it. MOUNTING_HOLE_NAMES lists the 19 unplated mounting holes KiCad 10 ships (plain sizes 2 … 6.5 mm, metric-clearance sizes 2.2mm_M2, 3.2mm_M3, 4.3mm_M4, 5.3mm_M5, 6.4mm_M6, 8.4mm_M8). mounting_hole_lib_id returns None for anything else, and the handler refuses before writing with invalid_argument on drill_diameter listing the shipped sizes. A conformance test checks every table entry against the installed library when one is present (and that the old formatter's names do not exist there), skipping silently otherwise — the same pattern as the guidance library-ID check.
• KiCad's own footprint when it resolves. When MountingHole.pretty resolves from the machine (project or global fp-lib-table, or a discovered install), the hole placed is that library footprint, through the same prepare_footprint_source / extract_* / board_footprint_sexp pipeline place_component uses (three of those made pub(crate)). Name and geometry are then KiCad's, so lib_footprint_mismatch has nothing to say.
• Inline geometry otherwise, honestly labelled. Without a resolvable library, Konnect's unplated-hole geometry is written under the shipped name, with the pad now equal to the drill as KiCad's plain MountingHole_* footprints have it (the old drill + 0.5 annulus matched no library footprint). The response carries geometry: "library" | "inline" and, for inline, geometry_note.
• Derived, not echoed. On the file path the footprint field is read back from the saved board (written_footprint_lib_id), not repeated from the request.

Branch and dependencies

Base branch: main at 2fbc5f5.
Depends on: nothing. Independent of #511 (different files).
Series order: none — single PR, one commit.
Overlap: none with any open PR.
Next PR to promote after this one: none in this chain.

Compatibility and safety

• Behaviour change (fix): the lib_id written for every drill changes to the shipped name; an unshipped drill is now refused instead of written. drill_diameter keeps its default of 3.2 and its type; its description now lists the shipped sizes.
• Additive response fields: geometry, geometry_note. footprint keeps its name and meaning but is now derived from the file on the file path.
• File mutations: the library path goes through insert_into_board (consistent read, duplicate-reference refusal, parse-before-write, atomic write); the inline path is unchanged apart from the name and pad size. Refusals write nothing (asserted byte-identical).
• IPC mutations: same attempt_ipc_write gate as before; a reachable KiCad that rejects never touches the file (existing test, kept).
• docs/API_MIGRATIONS.md and the tool-directory.md row carry the change. No tool added or removed.
• Rollback: revert the one commit.

Validation

Run on commit 449002d (Windows, KiCad 10.0 installed), exit codes captured directly:

cargo fmt --all -- --check                                        exit 0
cargo clippy --workspace --locked --all-targets -- -D warnings    exit 0
cargo test --workspace --locked --lib --tests                     exit 0 (no failures)
cargo test --workspace --locked --doc                             exit 0
cargo xtask fix-doc-counts --check                                21 / 226 / 233; unchanged
cargo test -p konnect-core --lib mounting_hole                    8 passed (3 existing, 5 new)

• cargo fmt --all -- --check
• cargo test --workspace --locked --lib --tests
• cargo test --workspace --locked --doc
• cargo clippy --workspace --locked --all-targets -- -D warnings
• Real-KiCad check: every_shipped_name_exists_in_the_installed_library ran against the installed KiCad 10 here (19/19 present; MountingHole_3.2mm / _3.0mm / _4.0mm confirmed absent), and the E2E below. Linux/macOS not run here; hosted CI covers the matrix, and the conformance test skips where no KiCad is installed.

Reproduced first, on main with a release binary: placing 3.2 / 3.0 / 4.0 mm holes wrote MountingHole_3.2mm, MountingHole_3.0mm, MountingHole_4.0mm — none of which exist in the installed MountingHole.pretty. Headless kicad-cli pcb upgrade/drc still load such a board (the footprint is embedded), so the "fails to load" symptom in the issue is the GUI's library-resolution path; the wrong names are the defect either way and are reproduced exactly.

Negative controls (each neutered, then restored):

guard neutered	tests that fail
old {drill:.1}mm formatter restored in mounting_hole_lib_id	shipped_drills_get_the_library_spelling, the_default_drill_writes_a_name_stock_kicad_resolves
refusal removed (every drill gets a name)	an_unshipped_drill_has_no_name, an_unshipped_drill_refuses_before_writing, plus the two above

Restored tree: 8/8 green.

E2E on this branch's release binary over MCP (fresh project, KiCad IPC unreachable so the guarded file path runs):

call	result
add_mounting_hole 3.2	MountingHole:MountingHole_3.2mm_M3, geometry: "library"
add_mounting_hole 3.0	MountingHole:MountingHole_3mm, geometry: "library"
add_mounting_hole 4.0	MountingHole:MountingHole_4mm, geometry: "library"
add_mounting_hole 3.3	refused: invalid_argument on drill_diameter, shipped sizes listed, nothing written

All three names verified present as .kicad_mod files in the installed MountingHole.pretty; kicad-cli pcb drc on the result reports no lib_footprint_mismatch. It does report lib_footprint_issues: library not enabled in the current configuration for each hole — a headless-CLI limitation, not the board: this machine's global fp-lib-table is KiCad's standard nested (type "Table") entry for the stock libraries, and the same headless run flags every footprint of KiCad's own ecc83 demo identically. The GUI resolves them. So the one check I cannot make headlessly is the GUI open with no library warning; the names are now the ones the GUI's own library browser lists.

Review checklist

• The diff is focused and contains no generated output, personal data, or unrelated cleanup.
• The branch includes current upstream/main, has no merge conflicts, and CI passed on this exact head.
• The branch was based on latest upstream/main, not a release tag.
• The PR shows only its unique commits and diff; dependencies and series position are explicit.
• Every review conversation is resolved.
• New names follow docs/NAMING_CONVENTIONS.md (geometry, geometry_note); no public renames.
• New behavior and failure paths have regression coverage.
• File mutations are atomic and preserve unrelated content.
• IPC mutations verify the requested board and do not leave partial batches (unchanged gate).
• No tools added/removed; cargo xtask fix-doc-counts --check unchanged.

Maintainer merge state

• The PR has exactly one current status:* workflow label.
• status:ready-to-merge applies to this exact head SHA.
• All required checks and review conversations satisfy the main ruleset.
• Merge with gh pr merge N --merge after final verification.
• Terminal issue closure: add_mounting_hole writes footprint names KiCad 10 does not ship #462. Next PR to promote: none.

Maintainer refresh on current main

Reconstructed onto upstream/main at fbebd991e668aa8396bb4ec2893ae4b86db337cf; current head is 817f5e75687fec53f1522060100c60bac8181a44.

The requested corrections are now included:

• Both library and inline file paths use one PreparedFootprintPlacement operation backed by the existing atomic, duplicate-reference-safe insertion path.
• MountingHoleSpec carries drill_diameter_mm, reference, and resolved lib_id together; the low-level preparation/serialization helpers are private again.
• File readback requires exactly one footprint with the requested reference. Duplicate or ambiguous identity is an error, never a first-match success.
• New regression coverage proves duplicate H1 refusal leaves the board byte-identical.
• New ignored live test crosses JSON-RPC MCP, routing, library discovery, and KiCad IPC, then verifies live readback and the saved board.

Observed live on Windows / KiCad 10.0.6:

cargo test -p konnect --test live_kicad_tools --locked add_mounting_hole_round_trips_through_live_kicad -- --ignored --nocapture --test-threads=1
1 passed; 0 failed

The call returned source: "ipc", live KiCad read back MountingHole:MountingHole_3.2mm_M3, and KiCad save persisted that exact id with a 3.2 mm np_thru_hole drill.

Duplicate-safety negative control: temporarily disabling reference rejection made inline_file_placement_refuses_a_duplicate_reference_without_writing fail because the duplicate was committed and readback became ambiguous. The restored guard passes.

Exact-head local gate:

cargo fmt --all -- --check                                      exit 0
cargo clippy --workspace --locked --all-targets -- -D warnings  exit 0
cargo test --workspace --locked --lib --tests                   exit 0
cargo test --workspace --locked --doc                           exit 0
cargo xtask fix-doc-counts --check                              exit 0; 21 / 226 / 233 unchanged
cargo test -p konnect-core --locked mounting_hole --lib          exit 0; 9 passed

Issue #462's title/evidence has been corrected: the invalid library names are reproduced; a board-load failure is not. Headless KiCad loads the embedded geometry. A GUI warning across every library-table configuration remains validation debt, not a claimed pass.

[neusse]

Please correct the inline fallback/readback path and supply the missing live-KiCad evidence before this can close #462.

1. The inline branch appends H1 directly and then written_footprint_lib_id returns the first matching Reference. If H1 already exists, the write can create a duplicate and the response can report the old footprint's lib_id rather than the one just written. Route inline geometry through the same duplicate-reference-checked insertion path as library placement, or bind/read back the inserted footprint by unique identity. Add a duplicate-reference test proving byte-identical refusal and correct result-derived readback.
2. Exercise the changed IPC success path with KiCad live, then read the saved board and show the placed lib_id/geometry agrees. The current E2E explicitly used an unreachable IPC endpoint, so the changed live path is core behavior without observation. Keep an unavailable GUI/library-warning check as named validation debt if necessary; the original "board fails to load" symptom was not reproduced by kicad-cli and should be corrected in #462 rather than asserted.
3. Tighten the new internal contract: use unit-bearing names (drill_diameter_mm) and preferably a small named MountingHoleSpec instead of (f64, &str). Also hide the deep pcb_components pipeline behind one cohesive shared operation rather than making several placement internals pub(crate) to be orchestrated from pcb_board.

The exact KiCad 10 name table, unsupported-size refusal, migration text, fixture/conformance checks, and negative controls are otherwise strong. After these changes, push one refreshed head, rerun all ten checks, and update the evidence/closure text; #511 remains ahead of this independent PR only because it is already exact-head approved.

[neusse]

Queue refresh — the existing three-item correction request remains unchanged on head 449002d81326fdd90245c63345e0065d5d89ce1d.

Please: (1) route inline mounting-hole placement through duplicate-reference-safe insertion with identity-bound readback and a byte-identical refusal regression; (2) supply live-KiCad IPC success/readback evidence and correct #462's unreproduced board-load claim; and (3) tighten the internal contract behind one cohesive shared operation using unit-bearing/named inputs such as drill_diameter_mm and MountingHoleSpec.

Make the code corrections without repeatedly rebasing. When this PR reaches its queue turn after #485, reconstruct once onto current main, preserve all migration/tool-directory entries, run all ten checks, and return the exact head SHA. The current branch is conflicting, so status:waiting-on-author remains correct.

[neusse]

The refreshed exact head satisfies the requested duplicate-reference safety, identity-bound readback, cohesive internal contract, and live KiCad IPC evidence. Hosted checks are still required before merge.