feat(schematic): place sheet pins on the top and bottom edges

[triglav-modular]

Summary

Sheet pins can be placed on all four edges of a sheet block. add_sheet_pin and edit_sheet_pin take an optional side of right, left, top or bottom, and import_sheet_pins' existing side enum is extended from two values to the same four.

Closes #514

The reproduction, and why it is not cosmetic. KiCad does not derive a sheet pin's edge from its position — it reads the edge from the rotation in the pin's at, and relocates a pin whose position disagrees with it. add_sheet_pin and edit_sheet_pin had no rotation input at all and always wrote 0, so every pin they wrote was a right-edge pin, and import_sheet_pins was the only tool writing any other angle with an enum of ["right", "left"]. Rotations 90 and 270 were unreachable from any tool, so a sheet with pins along its top or bottom edge could not be authored through Konnect.

The failure that makes this a bug rather than a missing feature is that it is silent and three-way. On a sheet box spanning x 50..130, y 50..100, asking for a pin at the middle of the top edge — this is a live run against this branch with side omitted, which is exactly the pre-change behaviour and still the behaviour of any call that omits it:

stage	what it says
the add_sheet_pin response	{"added_pin": "NO_SIDE", "pin_type": "passive", "sheet": "card", "side": "right", "x": 80.0, "y": 50.0}
the .kicad_sch on disk	(at 80 50 0)
kicad-cli sch erc, on that same unmodified file	the pin is at (130, 50)

A 50 mm jump to the right edge, reported as success at every stage a caller can observe. Note that the response is not lying about the file — the file really does say rotation 0, which is the right edge. It is the position that KiCad discards. The rotation 0 is what moved it: KiCad bound the pin to the right edge because that is the edge rotation 0 names, and slid the position to match.

Review round 1 — the three corrections, on 5d9569813be39e33b90c39b063960d0b50ec554c

1. import_sheet_pins had the same defect this PR exists to fix, on its own code path — and the reviewer was right to call it a defect rather than a nitpick. It generated positions from the total pin count on the sheet and never checked one against the edge it was placing on. Two ways to reach the corner: enough labels for one edge, or enough pins already sitting on the other three, since every one of them pushed the next import's first slot further down. Past the corner KiCad clamps the pin back onto the box — the silent relocation, re-entered through the one tool most likely to write many pins at once.

Three changes, matching what was asked:

• The stack now continues below only the pins already on the selected edge, identified by the rotation they carry, rather than below every pin on the sheet.
• The whole import is planned before any of it is written, and every generated point goes through ensure_pin_is_on_sheet_edge — the same guard add_sheet_pin uses, so the two cannot drift apart.
• An overflow refuses the import entire with nothing written, naming the edge, how many pins it holds, how many are already on it, and which pin was the first that did not fit.

2. changed_fields now comes from the saved delta. edit_sheet_pin pushed side whenever side was an argument. Restating the edge a pin already sits on is now the honest no-op it is: side is absent from changed_fields, nothing is committed, and the call is still a legal one-field request rather than the "no fields to change" refusal. changed and requested_fields join the response, spelled as edit_sheet already spells them, so a no-op is legible rather than silent. The other three fields keep the behaviour they had; only the argument this PR introduces is in scope here.

3. tool-directory.md, the migration note, and the spelling. All three sheet-pin rows now carry the public side contract, including the refusal and the changed_fields rule. docs/API_MIGRATIONS.md gains the atomic-refusal and response-derivation paragraphs. New prose and error strings use KiCad: the 15 lines this branch added with KiCAD are corrected, and the ~400 pre-existing occurrences elsewhere in the file and the codebase are untouched, which is what docs/NAMING_CONVENTIONS.md line 15 asks for. The real-KiCad round-trip evidence below is preserved, and re-run on the new head rather than inherited.

Review round 2 — the import's next slot, on 10fbcb3

You were right, and the regression written in round 1 could not have caught it. import_sheet_pins chose its next slot from the count of pins attributed to the selected edge. A count does not say where those pins are: an edge holding pins in slots 1 and 3 holds two, so the import took slot 3 and wrote on top of the second one. The round 1 regression imports onto an empty opposite edge, so it never exercised a sparse same-edge case. Reproduced by measurement before anything was changed — the imported pin landed at (50.0, 57.62), byte-identical coordinates to the pin already there.

The slot each pin occupies is now derived from its coordinate and the largest taken, so the stack continues after the pin that reaches furthest along the edge. The atomic refusal is untouched.

Three things came out of doing this that are worth your attention.

1. Deriving a slot is a division, and it needs a tolerance. (62.7 - 50.0) / 2.54 is 5.000000000000001, so rounding outward without one reads slot 5 as slot 6 and the import skips a slot. It goes wrong at slots 5, 10, 12, 13, 17, 24, 26 and 31 within a single sheet. Removing the tolerance left all 60 tests passing — every fixture in the file sits on slots 1, 3 or 19, the three where the error happens to fall the harmless way. import_sheet_pins_reads_a_slot_the_division_cannot_represent_exactly lands on slot 5, and the neuter fails once it exists. The tolerance is a thousand times finer than the nanometre KiCad stores a coordinate in.

2. "Valid" is load-bearing in your wording, and measuring without it broke a working call. edit_sheet resizes a sheet without moving its pins, so shrinking one strands them past the new corner. Those pins still carry the edge's rotation, so measuring them reported an edge "filled through slot 19" whose capacity was now 7 — refusing every import onto an edge holding no valid pin at all. Caught by building the pre-change and post-change binaries and running the same scenario through both: the old one placed the pin at y = 55.08, the new one refused. A pin past the end of the edge now reserves a slot instead, exactly as a pin whose rotation names no edge already did, and the two binaries agree again on that case. import_sheet_pins_ignores_a_pin_the_sheet_shrank_away_from covers it.

3. One consequence of "after the furthest" is intended, but I would rather you chose it than inherit it. A single pin on an edge's last slot fills the edge while every slot before it stays free, so the import refuses where it used to place pins into the gap. That is the literal reading of your request, and it is what is implemented. It seemed wrong to let that caller be told to enlarge a sheet that has 18 free slots, so the refusal says so instead:

import_sheet_pins(side="left")  → The 'left' edge of sheet 'card' holds 19 sheet pins at
                                  2.54 mm apart and is already filled through slot 19, so
                                  'NET0' and everything after it would be placed past a
                                  corner. […] 18 slot(s) before it are empty, but an import
                                  continues after the furthest pin rather than filling gaps,
                                  so moving or deleting the outlying pin is the smaller fix
                                  here. Enlarge the sheet with edit_sheet, or import onto
                                  more than one side.

If you meant the import to fill gaps rather than stack past them, say so and I will change it — it is a small change and it would make that refusal unreachable. I did not assume it, because "the first slot after the furthest valid pin" says the opposite.

A live sparse import on this head, sheet box x 50..130 / y 50..100, with pins already in slots 1 and 3 of the left edge:

A1  (at 50 52.54 180)     ← already there, slot 1
A3  (at 50 57.62 180)     ← already there, slot 3
VIN (at 50 60.16 180)     ← imported: slot 4, and the slot 2 gap is left alone

kicad-cli sch upgrade returns all three byte-identical, so KiCad does not compact the gap either.

The capacity arithmetic is now in one sheet_pin_edge_capacity, used by both the refusal that reports the number and the import that decides a pin is off the edge by it, so the two cannot drift into describing the same edge differently.

Review round 3 — a count is not a position, again, on a4cfe12

(The live captures below were taken on an a4cfe12 build. The implementation has not changed since: a4cfe12 → 90087bc → the six commits now on dbc01de, each step a rebase only. git range-diff 39e0061..90087bc dbc01de..8586359~2 reports five of six =, and the sixth differs only by a use line that merged with opt_positive_f64. Nothing is relabelled — the runs are exactly as they happened, on code identical to what is here now. The two test-only commits on top add no production code.)

All three were real, all three were mine, and each was reproduced before it was fixed.

1 and 3 are one defect wearing two faces. sheet_pin_edge_capacity used an untoleranced floor(). Your 31-slot example is exact, but it is not one span: the capacity is computed from span_end - span_start, and 132 geometries in a scan of ordinary sheet sizes undercount — a sheet at y = 50 of height 5.08 mm is two slots and reads as one. The undercount is not cosmetic, because capacity decides which pins count as being on the edge: a valid last-slot pin was judged stranded, reserved as a bare count, and the import wrote on top of it. Live, before the fix: is_error=false, two pins both at (50, 55.08).

2 is the same lesson I was given in round 2 and applied too narrowly. The reserve was a count. A count says how many slots to step past, not which coordinates are occupied, so a single-label import cleared the pin and a multi-label import walked onto it. Live, before the fix: an unattributable pin at slot 5 with five labels imported, and N4 landed exactly on it. I fixed "a count is not a position" for the pins that could be measured and left it in place for the pins that could not.

The fix. Every pin that could occupy a slot on this edge contributes its actual slot, and the generator steps over any slot already taken. Your rule is unchanged — the stack still continues after the furthest pin genuinely on the edge, and gaps below it are still not filled; this only refuses to write on top of something. A stranded pin also contributes the slot KiCad would clamp it onto, which is the "or a corner" half of your first item: a pin past the end does not stay there, and on an edge whose length is an exact number of slots that corner is a slot an import would otherwise take. Capacity now carries the same slack the slot arithmetic already used.

Live on an a4cfe12 build, the three cases:

unattributable pin at slot 5, five labels
  N1 52.54  N2 55.08  N3 57.62  N4 60.16  [ODD 62.7 rot 45]  N5 65.24
  -> six pins, six distinct coordinates: the stack steps OVER the pin

exact-multiple edge (5.08 mm = 2 slots), last slot occupied
  -> "holds 2 sheet pins ... is already filled through slot 2" and refuses;
     file sha256 unchanged across the call

sheet shrunk away from a pin, three labels
  A 52.54  B 55.08  C 57.62, and D still at 98.26

One behaviour change to declare. A stranded or unattributable pin used to cost a slot whether or not it was in the way; now it costs only the slots it actually occupies. The shrunk-edge import above therefore starts at 52.54 where it previously started at 55.08. That is the intended consequence of keeping positions rather than a count, and it is the only difference a before/after run of eleven scenarios through both binaries showed, besides the two overlaps disappearing.

An audit after the fix found a panic I had introduced, and it is worth naming because it is not in the review. Ordering the "block the clamp point" logic through f64::clamp asserts min <= max; the span is built as origin and origin + size, and edit_sheet writes any size it is given, so a sheet with a negative size inverts the span and takes the process down instead of returning an error. Reachable from this crate's own API in three calls. It is ordered bounds and min/max now, with a regression that fails — by panicking — when the clamp is put back. I have not touched edit_sheet's lack of a positivity check, which is a separate question from this PR.

Approach

side is written as the rotation KiCad reads the edge from — right 0, top 90, left 180, bottom 270 — and the mapping is confirmed by KiCad's own writer rather than inferred (round trip below). One SHEET_PIN_SIDE_DESC and one rotation_for_sheet_pin_side serve all three tools so the vocabulary cannot drift, and sheet_pin_side_for_rotation is its inverse so a response can name the side the saved pin actually carries.

Two design decisions worth stating

side, not a raw rotation. import_sheet_pins already spelled this argument side with edge names, so a rotation argument would have given the same concept two vocabularies in one toolset. It is also the safer type: an enum of four edge names cannot be handed 45 degrees, whereas a number can, and KiCad has no edge for 45 degrees. Both points were put to the maintainer with the design and accepted.

An off-edge position is refused, not written. This is the part that does real work. KiCad accepts such a write and then moves the pin, so writing it would mean reporting success for a position the editor does not use — the exact failure this argument exists to end. Konnect's boundary is the last point at which the caller can still be told the request cannot be carried out, so that is where it is refused, with a structured invalid_argument naming the offending axis and nothing written.

The check covers both axes, because KiCad constrains a pin along its edge as well as across it: a pin at (at 130 140 0) on that box comes back from KiCad as (at 130 100 0), clamped to the corner. Corners are on the edge and are accepted — they belong to two edges at once — and a millimetre past one is refused.

import_sheet_pins is held to the same rule even though its caller supplies no position, because the caller cannot see an overflow coming in its own arguments; the tool refuses the whole import rather than writing what fitted.

The response is read back off the committed file

All three tools load the saved schematic after commit_edited_sheet_item and report the pin that is in it. edit_sheet_pin looks the pin up under the name it saved under, since new_name may have just changed it, and an import that wrote nothing reports no side rather than asserting one.

This is not decoration. A response that restates its arguments cannot tell a caller that KiCad moved their pin — which is the whole failure side exists to surface — so an echoing response would have left the new argument unable to do the job it was added for. It is also the same rule as #329's add_bus_entry fix: when a response field is a claim about the file, the file has to be consulted. The observable difference is the writer's six decimal places: add_sheet_pin(x=130.0000004, y=55.0000004, side="right") is inside the 1e-6 edge tolerance, so it is accepted, writes (at 130 55 0), and now answers x: 130.0, y: 55.0.

changed_fields on edit_sheet_pin is the same rule one step further in, and is what the second correction applies: a field is named there because the delta reached the file, not because the caller asked for it.

Branch and dependencies

Base branch: main, reconstructed onto current main (dbc01de). git range-diff reports five of the six original commits identical to the reviewed a4cfe12; the sixth differs only by the use line, which now also imports opt_positive_f64 from #551. Two test-only commits follow, described below.

Reconstruction onto dbc01de — two test-only commits

#551 enforces advertised input schemas at dispatch, which changed what two of
this branch's tests are worth. Both were re-verified by neutering, not by
passing.

import_sheet_pins negative-size regression. It asked edit_sheet to
invert the sheet's span and discarded the result. opt_positive_f64 now refuses
that call, so the sheet stayed rectangular and the test passed while exercising
an ordinary box. A backwards span is still reachable — a file written by an
older version, by hand, or by another tool can carry one — so the fixture now
writes the inverted span into the schematic and asserts both that the
substitution landed and that the sheet reads back inverted. Restoring
coord.clamp(span_start, span_end) in place of the ordered bounds makes it
panic inside f64::clamp; restoring the guard makes it pass.

A dispatch-level test for the side enum. Every other test of this feature
calls the handlers directly and never reaches the validator, so a schema
narrowed back to right|left would leave top and bottom unreachable with
the suite still green. The new test dispatches all three sheet-pin tools with
each of the four edges and asserts none is refused for side; middle is its
own positive control, so an inert validator fails the test rather than passing
it. Narrowing one enum fails it with "top" is not one of "right" or "left".

main has since moved to 30e0abd (#522). It does not conflict, so no rebase was taken — git merge-tree --write-tree origin/main HEAD returns a single tree oid with no conflict section, even though #522 touches both docs/API_MIGRATIONS.md and tool-directory.md. Say the word if you would rather have the branch moved forward anyway.

Depends on: nothing. No open pull request touches sch_hierarchy.rs except this one — checked across every open PR, not only mine — so it can be sequenced independently of the sch_components.rs set.

Unique commits: six, touching crates/konnect-core/src/tools/sch_hierarchy.rs, docs/API_MIGRATIONS.md and tool-directory.md and nothing else.

No tool was added or removed, so there are no registry tool_count, DEV.md or README counts to update; cargo xtask fix-doc-counts confirms the documentation is already current. The three tool-directory.md rows changed are prose, not counts.

Compatibility and safety

• Purely additive at the schema. One new optional side property on two tool schemas, and two new values on an enum that already existed on the third. side defaults to right, which is rotation 0 — exactly what all three tools have always written.
• add_sheet_pin and edit_sheet_pin: no existing call changes meaning, and nothing that used to be accepted is now refused. The edge validation rides on the argument, not on the position: a call that omits side gets the old behaviour, including the old silent relocation. add_sheet_pin_without_a_side_still_accepts_any_position pins that, and it is the one test that fails if the check is made unconditional.
• import_sheet_pins is the one place where an existing call can newly fail, and it is deliberate. A sheet whose child labels no longer fit on one edge now returns an error instead of a success over pins KiCad would move. The migration note says so in those terms. Every import that fits is unaffected, and the two old sides land exactly where they always did — import_sheet_pins_keeps_placing_a_left_or_right_stack_down_the_edge is the scope test for that.
• A second import_sheet_pins case can newly fail, added in round 2 and deliberate. The stack continues after the pin that reaches furthest along the edge, so a single pin sitting on the edge's last slot fills it while every slot before stays free: the import now refuses where it used to place pins into that gap. The refusal names the free slots and points at the outlying pin rather than suggesting a resize. This is the literal reading of "the first slot after the furthest valid pin" — see the round 2 section for the alternative, if gap-filling is what you meant.
• A caller that starts passing side can see a refusal where it used to see a success. That is the point of the argument: the success it replaces had written a position the editor did not use.
• Response additions only. side is new on all three responses; changed and requested_fields are new on edit_sheet_pin; x and y keep their names and meanings and are now read from the saved file rather than echoed. changed_fields keeps its name and reports the same thing it always claimed to report. No tool, argument, or existing response field was renamed or removed.
• Durable migration record. docs/API_MIGRATIONS.md gains ## Unreleased: side on the sheet-pin tools (minor release), inserted above the existing entries and preserving every one now on main — the file is byte-identical to main once the new entry is cut out.
• File mutations go through the existing schematic write path unchanged. The only new write is the rotation in a sheet pin's at, a field KiCad already writes and Konnect already parsed. A refused import and a no-op edit both skip the commit outright, so neither reserialises the sheet.
• Rollback is reverting the two commits. No format migration is involved, and a file written by this code is one eeschema already writes.

Validation

cargo fmt --all -- --check                                        clean
cargo test --workspace --locked --lib --tests                     1827 passed, 0 failed, 29 ignored
cargo test --workspace --locked --doc                             8 passed, 0 failed, 2 ignored
cargo clippy --workspace --locked --all-targets -- -D warnings    clean

The exact head is 8586359 on base dbc01de. konnect-core's own lib suite is 1295 of those tests, and it was also re-run under env -i with an empty HOME and no KICAD10_* set — a bare runner in one command — where it passes 1295 as well, with the native dependency rebuilt from source inside that run rather than reused, so nothing here resolves anything through an installed KiCad and passes locally while failing on the Linux and Windows runners.

The flaky tools::pcb_board::board_session_safety_tests::a_kicad_holding_another_project_edits_this_board_file reported in the first round did not recur in this round's runs. It is unrelated to this diff, and the mechanism named there — that module's test mock binding a TcpListener to 127.0.0.1:0, reading the port and dropping the listener before nng listens on it — is unchanged and untouched here.

The KiCad 10.0.6 round trip, re-run on this exact head

Preserved from the earlier rounds and re-run against an a4cfe12 build, not inherited — the round 2 change moves where an imported pin lands, so this was re-measured rather than relabelled. A fresh create_project fixture, one sheet, box 80 x 50 at (50, 50). Written by Konnect, then re-saved by kicad-cli sch upgrade — KiCad's own parser and writer:

pin	requested	Konnect wrote	KiCad wrote back
R_PIN	side: "right"	(at 130 55 0)	(at 130 55 0)	identical
T_PIN	side: "top"	(at 60 50 90)	(at 60 50 90)	identical
L_PIN	side: "left"	(at 50 60 180)	(at 50 60 180)	identical
B_PIN	side: "bottom"	(at 70 100 270)	(at 70 100 270)	identical
C_TOP	side: "right", on the corner	(at 130 50 0)	(at 130 50 0)	identical
C_BOT	side: "right", on the corner	(at 130 100 0)	(at 130 100 0)	identical
NO_SIDE	side omitted	(at 80 50 0)	(at 130 50 0)	moved 50 mm

kicad-cli sch upgrade exit 0; kicad-cli sch erc exit 0.

The last row is the positive control, and it is the reason to believe the other six. "Nothing moved" is worth nothing from a comparison that could not see a move. NO_SIDE was written at the same top-edge y as T_PIN, with side left out, and it is the only pin in the run that moved — 50 mm, to the right edge, silently, exactly as before this change. The same run leaves the six sided pins byte-identical.

ERC says it independently, and on the unmodified file rather than the upgraded copy: it places NO_SIDE at (130, 50) while the file on disk says (at 80 50 0), and it places each of the six sided pins at the position the file gives. Seven pins produce only six distinct ERC positions, because after the relocation NO_SIDE and C_TOP occupy the same point — the relocation is visible in KiCad's connectivity, not only in its geometry.

The import overflow, on the same live binary

Run against an a4cfe12 build over stdio. Sheet box x 50..130, y 50..100, so the left edge is 50 mm long and holds 19 pins at 2.54 mm. The child sheet carries 20 hierarchical labels:

import_sheet_pins(side="left")  → The 'left' edge of sheet 'card' holds 19 sheet pins at
                                  2.54 mm apart and is empty, so 'NET19' and
                                  everything after it would be placed past a corner. KiCad
                                  clamps such a pin back onto the box, piling the overflow
                                  on the corner instead of leaving it where it was written,
                                  so the import is refused entire and nothing was written.
                                  Enlarge the sheet with edit_sheet, or import onto more
                                  than one side.

sha256 of the parent schematic is unchanged across the call, and the file contains zero sheet pins afterwards — not nineteen and a refusal.

The boundary is then checked from the other side. With one label removed, the same import writes all 19, first at y = 52.54 and last at y = 98.26. Adding the twentieth label back and re-importing produces the same refusal, now reading is already filled through slot 19, and again leaves the file hash unchanged at 19 pins. So the capacity is neither over- nor under-stated by one.

And KiCad agrees about the boundary. That 19-pin file — plus one top-edge pin — round-trips through kicad-cli sch upgrade with every one of the 20 (at …) lines byte-identical, and kicad-cli sch erc on the unmodified file places all 19 left pins at x = 50 from y = 52.54 to y = 98.26, and reports no pin at the corner (50, 100). The last pin that this change lets in is one KiCad does not move.

changed_fields from the delta, on the same live binary

add_sheet_pin (pin_name=CLK, x=70, y=100, side="bottom")
  → {"added_pin": "CLK", "side": "bottom", "x": 70.0, "y": 100.0, ...}

edit_sheet_pin(pin_name=CLK, side="bottom")            ← restating the side it already has
  → {"changed": false, "changed_fields": [], "requested_fields": ["side"],
     "pin": {"name": "CLK", "side": "bottom", "x": 70.0, "y": 100.0}, ...}

edit_sheet_pin(pin_name=CLK, side="top", x=70, y=50)   ← a real move
  → {"changed": true, "changed_fields": ["position", "side"],
     "requested_fields": ["position", "side"],
     "pin": {"name": "CLK", "side": "top", "x": 70.0, "y": 50.0}, ...}

The no-op leaves the file's sha256 unchanged: it is not written and then reported as unwritten, it is genuinely not written.

Refusals, on the same live binary

Nothing is written; each returns a structured invalid_argument naming the axis, and none of the three pin names appears in the file afterwards. Re-captured on an a4cfe12 build, so the KiCad spelling below is the string the binary now emits:

x=60  y=75  side=top        → field "y": y = 75 is not on the 'top' edge of sheet 'card',
                              which is y = 50. KiCad relocates a sheet pin whose position
                              and side disagree, so the pin would not stay where this call
                              puts it
x=130 y=140 side=right      → field "y": y = 140 is past the end of the 'right' edge of
                              sheet 'card', which runs from y = 50 to 100. KiCad clamps a
                              sheet pin to the nearest corner rather than leaving it off
                              the box
x=130 y=55  side=diagonal   → field "side": 'diagonal' is not a sheet edge — must be one
                              of: right, left, top, bottom

The first is the across-the-edge axis, the second the along-the-edge span. Corner inclusivity is the positive half of that second check: C_TOP (130, 50) and C_BOT (130, 100) are the two ends of the right edge, both accepted, both round-tripped unchanged.

The negative control: the rotation comes from side, not from the position

This one is worth spelling out because the obvious test does not test it. Every single-side case sits on exactly one edge, so an implementation that ignored the argument and inferred the side from the position passes all four of them — demonstrated, not argued: a position-inferring neuter leaves add_sheet_pin_writes_the_rotation_each_side_selects green.

A corner belongs to two edges at once, so it separates the two implementations. On a second fixture, the same point (130, 50) written twice:

add_sheet_pin(x=130, y=50, side="right")  → response side "right",  file (at 130 50 0)
add_sheet_pin(x=130, y=50, side="top")    → response side "top",    file (at 130 50 90)

One position, two sides, two rotations. Only an implementation that reads the argument can produce both. add_sheet_pin_takes_the_rotation_from_the_side_not_the_position is that case, and it is the one test the inferring neuter kills.

Every new guard neutered and watched to fail

Each guard was removed on this head, the konnect-core suite run, and the guard restored. The right-hand column is every test that failed — not a selection. The last four rows of the first table were round 1's guards, the five after them round 2's, and the five after those round 3's.

guard removed	every test that failed
sheet_pin_rotation_for accepts any string and writes rotation 0	add_sheet_pin_refuses_a_side_that_is_not_an_edge, import_sheet_pins_refuses_a_side_that_is_not_an_edge
the across-the-edge coordinate check	add_sheet_pin_refuses_a_position_off_the_named_edge, edit_sheet_pin_refuses_a_side_the_pin_is_not_already_on, edit_sheet_pin_judges_the_side_against_the_position_it_is_given
the along-the-edge span check	add_sheet_pin_refuses_a_position_past_the_end_of_the_named_edge
corner inclusivity — the span bounds made exclusive	add_sheet_pin_accepts_a_pin_on_a_corner, add_sheet_pin_takes_the_rotation_from_the_side_not_the_position
add_sheet_pin stops writing a rotation at all (the old always-0)	add_sheet_pin_writes_the_rotation_each_side_selects, add_sheet_pin_takes_the_rotation_from_the_side_not_the_position
add_sheet_pin infers the rotation from the position instead of from side	add_sheet_pin_takes_the_rotation_from_the_side_not_the_position; also the pre-existing add_sheet_pin_writes_a_rotation_kicad_can_load and the scope test add_sheet_pin_without_a_side_still_accepts_any_position
add_sheet_pin's response echoes the requested x/y/side	add_sheet_pin_reports_the_saved_position_not_the_requested_one
edit_sheet_pin's summary is built from the in-memory pin	edit_sheet_pin_reports_the_saved_position_not_the_requested_one
edit_sheet_pin reads back under the requested name, not the saved one	edit_sheet_pin_reports_the_name_it_saved_under; also the pre-existing edit_sheet_pin_renames_and_retypes
edit_sheet_pin judges side against the position being replaced	edit_sheet_pin_judges_the_side_against_the_position_it_is_given, edit_sheet_pin_moves_a_pin_to_another_edge
side on its own is not counted as a change	edit_sheet_pin_counts_side_on_its_own_as_a_change, edit_sheet_pin_moves_a_pin_to_another_edge
edit_sheet_pin validates side but never writes the rotation	edit_sheet_pin_moves_a_pin_to_another_edge, edit_sheet_pin_counts_side_on_its_own_as_a_change, edit_sheet_pin_judges_the_side_against_the_position_it_is_given
import_sheet_pins ignores the stacking axis and always stacks down the edge	import_sheet_pins_stacks_along_a_top_or_bottom_edge
import_sheet_pins reports the requested side instead of a saved pin's	import_sheet_pins_reports_no_side_when_it_saved_no_pin
the import overflow refusal removed entirely	import_sheet_pins_refuses_a_left_edge_import_that_runs_past_the_corner, import_sheet_pins_refuses_a_top_edge_import_that_runs_past_the_corner — both on an import past the corner must refuse
the refusal kept but deferred until after the commit — the loop-writes-as-it-goes shape the review describes	the same two tests, now on a refused import must leave the file byte-identical. This is the row that proves the byte comparison is load-bearing rather than decorative: an error was returned, and the tests still failed
the import counts every pin on the sheet again instead of the pins on the selected edge	import_sheet_pins_stacks_below_the_pins_already_on_that_edge
side counted as changed whenever it is requested	edit_sheet_pin_does_not_count_a_restated_side_as_a_change
the next slot taken from a count of the edge's pins instead of the furthest one's coordinate	import_sheet_pins_starts_after_the_furthest_pin_on_a_sparse_edge, import_sheet_pins_reads_a_slot_the_division_cannot_represent_exactly, import_sheet_pins_names_the_free_slots_when_one_outlying_pin_fills_the_edge
the slot division's float tolerance removed	import_sheet_pins_reads_a_slot_the_division_cannot_represent_exactly. This is the row worth reading: before that test existed the same neuter passed 60 of 60, because every fixture sat on a slot where the error is harmless
a pin past the end of the edge measured rather than reserving a slot	import_sheet_pins_ignores_a_pin_the_sheet_shrank_away_from
the conservative reserve removed, for both the unattributable and the off-edge pin	import_sheet_pins_counts_a_pin_whose_rotation_names_no_edge, import_sheet_pins_ignores_a_pin_the_sheet_shrank_away_from
the free-slot clause dropped from the refusal	import_sheet_pins_names_the_free_slots_when_one_outlying_pin_fills_the_edge
the occupied-slot skip removed, so the stack no longer steps over anything	import_sheet_pins_counts_a_pin_whose_rotation_names_no_edge, import_sheet_pins_does_not_walk_onto_a_pin_whose_rotation_names_no_edge, import_sheet_pins_blocks_the_slot_a_stranded_pin_will_be_clamped_onto
the clamp point no longer blocks a slot	import_sheet_pins_blocks_the_slot_a_stranded_pin_will_be_clamped_onto
an unattributable pin stops blocking a slot at all	import_sheet_pins_counts_a_pin_whose_rotation_names_no_edge, import_sheet_pins_does_not_walk_onto_a_pin_whose_rotation_names_no_edge
the capacity tolerance removed	sheet_pin_edge_capacity_counts_an_exact_multiple_span
f64::clamp restored in place of the ordered bounds	import_sheet_pins_survives_a_sheet_with_a_negative_size, which fails by panicking — the failure mode it exists to prevent

Two further neuters are there to prove the scope tests are live rather than decorative — each is a deliberate widening of the change into the path it is supposed to leave alone:

widening applied	every test that failed
import_sheet_pins stacks across the edge for every side	import_sheet_pins_keeps_placing_a_left_or_right_stack_down_the_edge
the edge check made unconditional, so a call that omits side is validated too	add_sheet_pin_without_a_side_still_accepts_any_position, plus seven pre-existing tests: add_sheet_pin_rejects_duplicate_name, add_sheet_pin_writes_a_rotation_kicad_can_load, delete_sheet_pin_removes_it, edit_sheet_pin_accepts_the_position_the_pin_already_has, edit_sheet_pin_counts_side_on_its_own_as_a_change, edit_sheet_pin_renames_and_retypes, validate_sheet_pins_reports_mismatches

The mutation pass found one guard with no witness at all, and that is why it was re-run rather than inherited. import_sheet_pins reading its side off a pin it actually saved had no test: an echoed side and a derived one agree on every import that writes a pin, so the existing import_sheet_pins_stacks_along_a_top_or_bottom_edge — which does assert side == "top" — stayed green with the readback replaced by the argument. Only an import that writes nothing separates them. import_sheet_pins_reports_no_side_when_it_saved_no_pin closes it: the second import asks for bottom, saves nothing because the label already has a pin, and must answer null rather than "bottom".

Two tests in this change are scope tests, not coverage, and are declared as such rather than counted:

• add_sheet_pin_without_a_side_still_accepts_any_position asserts that the old behaviour is intact — it survives every neuter of a new guard, by design, because it exists to pin the additive contract rather than to catch a defect in the new code. It is not inert: the last row above is its control, and making the edge check unconditional is the one change that kills it.
• import_sheet_pins_keeps_placing_a_left_or_right_stack_down_the_edge is the same shape for the import's stacking axis, and its control is the row above that one — forcing the stack across the edge instead of down it is what makes it fail.

One round 3 test is a composite rather than single-guard coverage, and is declared as such. import_sheet_pins_does_not_overwrite_a_pin_on_an_exact_multiple_edge pins the end-to-end property that the shipped defect violated, and it survives every single neuter above, because the fix defends it three separate ways. It fails only when the original shape is reconstructed — untoleranced capacity, a count-based starting slot, and no occupied-slot skip, all three together — which was measured rather than assumed.

None of the four tests added this round is a scope test: each one fails under the neuter of the guard it exists for, which is the table above. The half of the left-edge test that asserts all nineteen pins do land is boundary scope inside a coverage test, and is why the capacity is checked against KiCad rather than only against our own arithmetic.

The row above is the honest shape of the last two rows of the first table as well: edit_sheet_pin_renames_and_retypes and add_sheet_pin_writes_a_rotation_kicad_can_load are pre-existing tests, not this change's coverage, and they are listed because the instruction was every test that failed, not every test I would have chosen.

Review checklist

• The diff is focused and contains no generated output, personal data, or unrelated cleanup.
• The branch has no merge conflicts, and CI passed on this exact head (8586359) — all ten required checks green.
• The branch is based on latest upstream/main (dbc01de).
• The PR shows only its unique commits and diff; it depends on nothing.
• Every review conversation is resolved — three items in the first review, one in the second, three in the third, all answered above.
• New names follow docs/NAMING_CONVENTIONS.md, including the KiCad spelling in new prose and errors; no public renames.
• New behavior and failure paths have regression coverage, each guard neutered and watched to fail.
• File mutations are atomic and preserve unrelated content; a refused import and a no-op edit write nothing at all.
• IPC mutations — none in this change.
• No tools added or removed, so no counts to update; tool-directory.md's three sheet-pin rows carry the new public side contract, and the additive argument is recorded in docs/API_MIGRATIONS.md.

[neusse]

Please close the import-overflow and result-truth gaps before this can merge.

Exact head reviewed: bdb9988d48fcc30e8965ba4a2a45b2c522c51595; all ten hosted checks are green, and the four-side mapping, explicit add/edit validation, readback, and closure reference otherwise match #514. One complete correction pass is needed:

1. import_sheet_pins generates positions indefinitely from the total existing-pin count but never validates them against the selected edge. Enough pins can be written past a corner and then clamped/overlapped by KiCad—the exact silent-relocation failure this issue fixes. Precompute the full import, place relative to pins already on the selected edge, validate every generated point before the single write, and refuse atomically on overflow. Add left/right and top/bottom overflow regressions proving no pin is written.
2. For the new side behavior, derive changed_fields from the saved delta. Restating the side already present must not be reported as an actual change merely because it was requested; return an honest no-op or omit side from changed fields.
3. Update the three tool-directory.md rows for the new public side contract and use the required KiCad spelling in newly added prose/errors. Preserve the real-KiCad round-trip evidence in the PR body.

Then rerun the ten checks and reply with the new head SHA. Closes #514 remains correct after these cases pass.

[triglav-modular]

All three are done. New head: 5d9569813be39e33b90c39b063960d0b50ec554c, on base 4a5dd41 — main had not moved when this was pushed, re-fetched immediately before, so no rebase was taken. All ten checks green on that exact head, read off gh pr checks rather than a watcher's exit code.

1. The import overflow. You were right that this is a defect and not a nitpick — import_sheet_pins was doing the thing the rest of the PR condemns. It generated positions from the total pin count on the sheet and never checked one against the edge it was placing on, so enough labels, or enough pins already on the other three edges, walked the stack past a corner into the clamp. Fixed as specified: the stack now continues below only the pins already carrying the selected edge's rotation, the whole import is planned before any of it is written, every generated point goes through ensure_pin_is_on_sheet_edge — the same guard add_sheet_pin uses, so the two cannot drift — and an overflow refuses the import entire with nothing written.

Two regressions, and both compare the file's bytes before and after rather than only asserting an error came back:

• import_sheet_pins_refuses_a_left_edge_import_that_runs_past_the_corner — the 50 mm left edge holds 19 at 2.54 mm; all 19 land, the last at y = 98.26, and the twentieth is refused against an edge already carrying 19.
• import_sheet_pins_refuses_a_top_edge_import_that_runs_past_the_corner — 32 labels onto an 80 mm top edge that holds 31, overflowing from empty, so a loop that wrote as it went would leave 31 pins behind.

The byte comparison is load-bearing, and that is measured rather than asserted. Two neuters: removing the refusal fails both tests on an import past the corner must refuse; keeping the refusal but deferring it until after the commit — the loop-writes-as-it-goes shape — fails both on a refused import must leave the file byte-identical. An error was returned in that second case and the tests still failed.

Live on a build of this head, sheet box x 50..130 / y 50..100, 20 labels onto the left edge:

→ The 'left' edge of sheet 'card' holds 19 sheet pins at 2.54 mm apart and already
  carries 0, so 'NET19' and everything after it would be placed past a corner. KiCad
  clamps such a pin back onto the box, piling the overflow on the corner instead of
  leaving it where it was written, so the import is refused entire and nothing was
  written. Enlarge the sheet with edit_sheet, or import onto more than one side.

sha256 of the parent schematic unchanged, zero sheet pins in the file afterwards.

The capacity is checked against KiCad rather than only against our arithmetic, since a limit that is wrong in the permissive direction would not show up in our own tests. That 19-pin file plus a top pin round-trips through kicad-cli sch upgrade with all 20 (at …) lines byte-identical, and kicad-cli sch erc on the unmodified file places every left pin from y = 52.54 to y = 98.26 and reports no pin at the corner (50, 100).

2. changed_fields from the saved delta. side is pushed only when the rotation actually differs; the commit is skipped when nothing differs, so a restated side is not written and then described as unwritten — it is genuinely not written. changed and requested_fields join the response, spelled as edit_sheet already spells them, so the no-op is legible rather than silent. Live:

edit_sheet_pin(pin_name=CLK, side="bottom")          ← the side it already has
  → {"changed": false, "changed_fields": [], "requested_fields": ["side"],
     "pin": {"name": "CLK", "side": "bottom", "x": 70.0, "y": 100.0}, ...}

edit_sheet_pin(pin_name=CLK, side="top", x=70, y=50) ← a real move
  → {"changed": true, "changed_fields": ["position", "side"],
     "requested_fields": ["position", "side"],
     "pin": {"name": "CLK", "side": "top", "x": 70.0, "y": 50.0}, ...}

File hash unchanged across the first. edit_sheet_pin_does_not_count_a_restated_side_as_a_change covers it and fails when side is counted unconditionally. I kept this to side — the argument this PR introduces — rather than changing how name, pin_type and position have always reported; say the word if you want those brought to the same rule and I will do it here or in a follow-up, whichever you prefer.

3. tool-directory.md and the spelling. All three sheet-pin rows now carry the public side contract, including the refusal and the changed_fields rule. docs/API_MIGRATIONS.md gains the atomic-refusal and response-derivation paragraphs, still inserted above the existing entries with none of them touched. On the spelling: the 15 lines this branch added with KiCAD are corrected to KiCad, and the four pre-existing ones in the same file are left alone, per docs/NAMING_CONVENTIONS.md line 15 — done mechanically by diffing against the base commit and rewriting only lines outside the pre-existing set, not by eye.

The real-KiCad round-trip evidence is preserved and was re-run on 5d95698 rather than inherited — same fixture, sch upgrade exit 0 with all six sided pins byte-identical, the side-omitted positive control still moving 50 mm to (at 130 50 0), and ERC on the unmodified file still placing it at (130, 50). The refusal strings quoted in the body are re-captured from this head, so they show the corrected spelling as the binary now emits it.

Gate on this head: cargo fmt --all -- --check clean, cargo test --workspace --locked --lib --tests 1794 passed / 0 failed / 28 ignored, cargo test --workspace --locked --doc 8 passed / 0 failed / 2 ignored, cargo clippy --workspace --locked --all-targets -- -D warnings clean. Also re-run under env -i with an empty HOME and no KICAD10_*: exit 0, konnect-core at 1212. The pcb_board flake noted in the body did not recur this round.

Four new guards this round, four new tests, each neutered and watched to fail with its own message; the full table is in the body. Closes #514 is unchanged.

[triglav-modular]

Following up on my own reply: re-reading that commit after posting it, I found a regression it introduced, and it is in the exact line your first item asked me to change. New head 5a9c9f3918416d8ca1cd19cc46ef781b29025aa8.

Counting only the pins carrying the selected edge's rotation — which is what you asked for and is right — made the count precise, and that quietly removed a safety margin the blunt total had by accident. sheet_pin_side_for_rotation answers None for a rotation that is absent or names no edge, so a pin like that counted toward no edge and the import stacked from slot 1 straight over it. The old total-count code pushed the stack outward for such a pin; mine pushed nothing.

The fix is conservative counting, not a refusal: a pin whose edge cannot be determined is counted as occupying the edge being imported to, on the grounds that if we cannot tell which edge it is on, it could be this one — stacking further out is always safe where stacking over it is not. It adds no failure mode and changes nothing for a well-formed file, which has no such pin. Refusing the import outright would match the malformed-file principle, but it is a harsher behaviour change than this review asked for; say the word if you would rather have the refusal. import_sheet_pins_counts_a_pin_whose_rotation_names_no_edge was written first and watched to fail on the previous head with both pins on the same coordinate, and fails again with its own message when the clause is neutered back.

One thing I cannot do yet, and I would rather say so than let it look green: the ten checks have not run on this head. #485 merged while I was working, its docs/API_MIGRATIONS.md entry now conflicts with ours — the only conflicting file — and GitHub will not build the merge ref for a conflicting PR, so no workflow run was created for 5a9c9f3. The local gate is clean on it (fmt clean, 1795 workspace tests passed / 0 failed / 28 ignored, 8 doc tests, clippy -D warnings clean, plus an env -i run with an empty HOME and no KICAD10_* at exit 0). I am holding the rebase rather than taking one unasked, since you asked for no repeated rebases. Say the word and I will rebase onto current main, resolve that one file the way #485 required — every existing entry byte-for-byte, ours inserted above — and report the ten checks on the resulting head.

[triglav-modular]

Reconstructed onto current main as you asked on #499. Base moved 4a5dd41 → 8ed319a (7 commits: #485 and #527). New head: 67a651648436238e264dee0c9aeb85147919c650, three unique commits, MERGEABLE.

Two files conflicted. sch_hierarchy.rs: I adopted #527's signature rather than working around it — commit_edited_sheet_item now returns Option<CallToolResult>, and all three sheet-pin tools return the refusal at the call site, with the label argument switched from prose to the tool name. Both of this PR's no-op skips are kept, with the new pattern nested inside them: if !changed.is_empty() in edit_sheet_pin, so restating a side still commits nothing, and if !imported.is_empty() in the import. #527's prospective-file validation and this PR's request-against-the-edge validation are complementary and both stay — one checks what the file would become, the other refuses a request before anything is built — and read_back_sheet_pin stays too, since #527's post-commit step verifies the sheet item where this reads the pin for the response.

docs/API_MIGRATIONS.md: ours inserted above every existing entry, same as #485. Proved rather than asserted — cutting our entry out leaves the file byte-identical to main, and git diff main is 44 insertions, 0 deletions.

No behavioural change surfaced, and I checked specifically because the commit path was rewritten underneath this feature. The four-side KiCad 10.0.6 round trip was re-run on 67a6516: sch upgrade exit 0, all six sided pins byte-identical, and the side-omitted positive control still moving (at 80 50 0) → (at 130 50 0). The pins Konnect writes are byte-for-byte what it wrote before the rebase. The overflow refusal and the changed_fields no-op were re-run live too, both with the file hash unchanged across the refused call.

Gate on this head: fmt clean, 1804 workspace tests passed / 0 failed / 28 ignored, 8 doc tests, clippy -D warnings clean, plus env -i with an empty HOME and no KICAD10_* at exit 0 (konnect-core 1222). All ten checks green on 67a6516, read off gh pr checks. Closes #514 unchanged.

[neusse]

One functional import-placement gap remains on exact head 67a651648436238e264dee0c9aeb85147919c650.

The previous overflow, atomicity, truthful-result, and documentation requests are addressed. However, import_sheet_pins chooses the next slot from the count of pins attributed to the selected edge, not from the furthest occupied coordinate. With existing same-edge pins in slots 1 and 3, count + 1 selects slot 3 again and overlaps the existing pin. The current regression imports onto an empty opposite edge, so it does not exercise this sparse same-edge case.

Please compute the first slot after the furthest valid pin on the selected edge, retain the atomic overflow refusal, and add a sparse same-edge regression proving no overlap. Also refresh the PR body's exact-head evidence/checklist, which still names an earlier head. Then rerun all ten checks and reply with the new exact head. Closes #514 remains correct once this is fixed.

[triglav-modular]

Fixed, and the fix found two more things on the way — one of which was a regression in my own first attempt at it. New head: 10fbcb35c070d34b7afa726b245434c775a1231f, base 8ed319a unchanged.

You were right, and the round 1 regression could not have caught it. The next slot came from the count of pins attributed to the edge. A count does not say where they are: slots 1 and 3 hold two pins, so the import took slot 3 and wrote on top of the second. I reproduced it before changing anything rather than reading it off the source — the imported pin landed at (50.0, 57.62), byte-identical coordinates to the pin already there. The slot is now derived from each pin's coordinate and the largest taken. import_sheet_pins_starts_after_the_furthest_pin_on_a_sparse_edge is the sparse same-edge regression; the atomic refusal is untouched.

1. The slot division needs a tolerance, and the suite could not see that it was missing. (62.7 - 50.0) / 2.54 is 5.000000000000001, so rounding outward without one reads slot 5 as slot 6. It goes wrong at slots 5, 10, 12, 13, 17, 24, 26 and 31 within one sheet. Neutering the tolerance left all 60 tests passing — every fixture in the file sits on slot 1, 3 or 19, the three where the error falls the harmless way. I only found it by enumerating every slot on a sheet rather than trusting the region the fixtures occupied. import_sheet_pins_reads_a_slot_the_division_cannot_represent_exactly lands on slot 5, and the neuter fails once it exists.

2. "Valid" is doing real work in your wording, and my first version ignored it and broke a working call. edit_sheet resizes a sheet without moving its pins, so shrinking one strands them past the new corner. Those pins still carry the edge's rotation, so measuring them reported an edge "filled through slot 19" whose capacity was now 7 — refusing every import onto an edge that held no valid pin at all. I caught it by building the pre-change and post-change binaries and running the same scenario through both: the old one placed the pin at y = 55.08, mine refused. A pin past the end of the edge now reserves a slot, exactly as a pin whose rotation names no edge already did, and the two binaries agree again. import_sheet_pins_ignores_a_pin_the_sheet_shrank_away_from covers it.

3. One consequence is intended, but I would rather you chose it than inherit it from my reading of your sentence. A single pin on an edge's last slot fills that edge while every slot before stays free, so the import now refuses where it used to place pins into the gap. That is what "the first slot after the furthest valid pin" says, so that is what is implemented — but it seemed wrong to then tell that caller to enlarge a sheet with 18 free slots, so the refusal says this instead:

The 'left' edge of sheet 'card' holds 19 sheet pins at 2.54 mm apart and is already
filled through slot 19, so 'NET0' and everything after it would be placed past a
corner. […] 18 slot(s) before it are empty, but an import continues after the
furthest pin rather than filling gaps, so moving or deleting the outlying pin is the
smaller fix here. Enlarge the sheet with edit_sheet, or import onto more than one side.

If you meant the import to fill gaps rather than stack past them, say the word and I will change it here — it is small, and it would make that refusal unreachable. I did not assume it, because your sentence says the opposite.

One wording fix came out of the same pass: an empty edge was being described as "already filled through slot 0". It reads "is empty" now, and the top-edge overflow regression — which overflows from an empty edge — carries the assertion.

The capacity arithmetic now lives in one sheet_pin_edge_capacity, used both by the refusal that reports the number and by the import that decides a pin is off the edge by it, so the two cannot drift.

Evidence re-measured on this head, not relabelled, since round 2 changes where an imported pin lands. The four-side KiCad 10.0.6 round trip: sch upgrade exit 0, all six sided pins byte-identical, and the side-omitted positive control still moving (at 80 50 0) → (at 130 50 0). A sparse import on this head puts the new pin in slot 4 with the slot 2 gap left alone, and KiCad returns all three byte-identical — it does not compact the gap either. Both refusals re-captured live with the file hash unchanged across each.

Five guards this round, five tests, each neutered and watched to fail with its own message; the table in the body has them. Gate on this head: cargo fmt --all -- --check clean, cargo test --workspace --locked --lib --tests 1808 passed / 0 failed / 28 ignored, cargo test --workspace --locked --doc 8 passed / 0 failed / 2 ignored, cargo clippy --workspace --locked --all-targets -- -D warnings clean. Also under env -i with an empty HOME and no KICAD10_*: exit 0, konnect-core at 1226.

main moved to 30e0abd (#522) while this was in progress. It does not conflict — git merge-tree --write-tree returns a single tree oid with no conflict section, even though #522 touches both docs/API_MIGRATIONS.md and tool-directory.md — so I have not taken a rebase. Say the word if you would rather I moved the branch forward anyway.

The PR body is refreshed: exact head, base, commit count, the validation numbers, the round trip and refusal captures, the neuter table and the checklist. Closes #514 is unchanged.

[neusse]

Please fix these three remaining placement-safety cases, then refresh the branch onto current main and rerun all ten checks:

• Preserve actual occupied positions for stranded/off-edge pins. A scalar reserved count loses the slot, so a multi-label import can advance into that occupied coordinate or a corner where KiCad clamps the pin.
• Preserve actual same information for pins whose rotation maps to no supported edge. A sparse unknown-rotation pin must not be replaced by a generated pin at the same position.
• Make sheet_pin_edge_capacity tolerant at exact 2.54 mm multiples. Binary floating-point can represent an exact 31-slot span as 30.999…; an untoleranced floor() undercounts it as 30.

Add focused regressions for: a shrunk-edge multi-label/corner case, an unknown-rotation sparse-position case, and an exact-multiple capacity case. The existing sparse-valid-pin correction remains good, but exact head 10fbcb35c070d34b7afa726b245434c775a1231f does not yet make Closes #514 safe and is also based before current main (f0f5ad0). This stays behind the independent current-main work in the merge train until those cases are covered.

[triglav-modular]

All three were real and all three were mine, each reproduced before it was fixed. New head a4cfe12, rebased onto current main (f0f5ad0) as you asked, all ten checks green on that exact head.

• Items 1 and 3 were one defect. An untoleranced floor() undercounts any span that is an exact number of slots — 132 ordinary geometries, not only 31 — and capacity decides which pins count as being on the edge, so a valid last-slot pin was judged stranded and overwritten.
• Item 2 was your round-2 lesson applied too narrowly. I fixed "a count is not a position" for the pins that could be measured and left a scalar reserve for the pins that could not, so a multi-label import walked onto them.
• The fix. Every pin that could occupy a slot contributes its actual slot — including the one KiCad would clamp a stranded pin onto — and the generator steps over what is taken. Your rule is unchanged: the stack still continues after the furthest pin on the edge, and gaps below it are not filled.

One behaviour change, declared: such a pin now costs only the slots it occupies, so a shrunk-edge import starts at 52.54 where it started at 55.08. Across eleven before/after scenarios that is the only difference besides the two overlaps disappearing.

An audit found a panic I had introduced — f64::clamp asserts min <= max, and a negative-size sheet inverts the span. Ordered bounds now, with a regression that fails by panicking if the clamp returns. edit_sheet's missing positivity check is untouched.

Five new guards, five tests, each neutered and watched to fail. Live output, the neuter table and the full evidence are in the PR body. Gate: fmt clean, 1827 workspace tests, 8 doc tests, clippy -D warnings clean, env -i exit 0 at konnect-core 1245. Closes #514 unchanged.

[triglav-modular]

Follow-up on the panic above, having verified the underlying cause against the shipped v0.11.1 release rather than a branch: it is not only edit_sheet. add_hierarchical_sheet accepts a negative or zero size too, both write it ((size 80 -20), (size -80 50), (size 0 0)), and neither schema declares a minimum. KiCad does not object either — sch upgrade preserves the size byte-for-byte and sch erc reports no violations — so nothing downstream catches it.

One consequence lands inside this PR, and I would rather name it than have you find it. On such a box the span is inverted, so the edge validation added here refuses every position and prints the range backwards: "the 'left' edge of sheet 'card', which runs from y = 50 to 30". Safe, and no longer a crash, but unreadable. I can make it name the malformed box instead if you want that here; otherwise it belongs with the upstream fix and I will leave this PR alone.

[neusse]

Keep this PR focused: the current fail-safe refusal for a malformed negative/zero-size sheet is acceptable here, so no additional wording change is required in #524. Please open a separate focused issue for validating positive sheet dimensions at the add/edit boundary and for making any remaining malformed-box diagnostic explicit.

For #524, incorporate current main (39e006131cb7d5ba3f1d3c9f07021a507a9f7370), rerun all ten required checks, and reply with the new exact head. The three requested placement cases appear addressed in a4cfe12; final review will be against the refreshed head.

[triglav-modular]

Refreshed onto 39e0061 as asked. New head 90087bc, all ten checks green on that exact head.

The rebase carried no content change: git range-diff f0f5ad0..a4cfe12 39e0061..90087bc reports all six commits =, so what you reviewed in a4cfe12 is unchanged and only the base moved. docs/API_MIGRATIONS.md re-checked against the new base — cutting our entry out leaves the file byte-identical to 39e0061.

Gate on this head: fmt clean, 1827 workspace tests, 8 doc tests, clippy -D warnings clean, and env -i with an empty HOME and no KICAD10_* at exit 0, konnect-core 1245, with the native dependency rebuilt from source inside that run rather than reused.

Noted on the malformed-box wording — #524 keeps its current fail-safe refusal and I have changed nothing for it. The separate issue is #543: positive-dimension validation at both add_hierarchical_sheet and edit_sheet, and making the malformed-box diagnostic explicit. Closes #514 unchanged.

[triglav-modular]

Reconstructed onto current main (dbc01de). New head 8586359, all ten checks green on that exact head.

The rebase carried no content change: git range-diff 39e0061..90087bc dbc01de..8586359~2 reports five of the six commits =, and the sixth differs only by a use line that now also imports opt_positive_f64.

Two test-only commits follow, because #551 changed what two of this branch's tests were worth. Both were re-verified by neutering rather than by passing.

The negative-size regression had gone vacuous. It asked edit_sheet to invert the sheet's span and discarded the result. opt_positive_f64 now refuses that call, so the sheet stayed rectangular and the test passed while exercising an ordinary box — it asserted only that nothing panicked, and nothing was there to panic on. A backwards span is still reachable from a file an older version or another tool wrote, so the fixture now writes the inverted span into the schematic and asserts both that the substitution landed and that the sheet reads back inverted. Restoring coord.clamp(span_start, span_end) makes it panic inside f64::clamp; restoring the ordered bounds makes it pass.

Nothing exercised the side enum at the dispatch. Every other test of this feature calls the handlers directly, so a schema narrowed back to right|left would leave top and bottom unreachable with the suite still green. The new test dispatches all three sheet-pin tools with each of the four edges and asserts none is refused for side; middle is its own positive control, so an inert validator fails it rather than passing it.

Writing that test also confirmed #584's closure is live: one argument set across three tools was refused for an unexpected pin_name.

The migration entry sits at the top of API_MIGRATIONS.md — happy to move it if it belongs elsewhere in the order.