Skip to content

Make main the VNS working version: current upstream plus ten fixes - #180

Closed
JYPochez wants to merge 22 commits into
mixelpixx:mainfrom
JYPochez:vns
Closed

Make main the VNS working version: current upstream plus ten fixes#180
JYPochez wants to merge 22 commits into
mixelpixx:mainfrom
JYPochez:vns

Conversation

@JYPochez

@JYPochez JYPochez commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Brings this fork's main up to current upstream and makes it the VNS working version:
mixelpixx/Konnect@5cd6454 plus ten fixes, merged and gated together.

Until now main here was a byte-identical mirror of upstream, and the fork's actual work
lived on publish — a branch built from a vendored v0.2.0 tarball with no shared ancestry
with upstream. That made it useless as a base: a diff against upstream showed 208 files and
6060 deletions, because git could only read it as reverting everything upstream had done
since. Every fix here has now been replayed onto real upstream history instead.

Correctness fixes

Multi-unit symbols were treated as one placement. A multi-unit part is one (symbol …)
instance per unit, all sharing the reference, and four tools took the first match.
batch_connect_to_net resolved every pin against instance #1 and transformed it by that
unit's placement, so two different nets landed on one coordinate and were silently
shorted — no error, and the tool returned success. Found in real use: X_CLK_IN and
X_DATA_IN both on U6 pin 1 of a 74HC14. batch_edit_schematic_components wrote fields into
unit 1 only; the delete tools left the other units as orphans; bulk_move tore the part
apart.

No user symbol library resolved. resolve_lib_symbol scanned a hardcoded list of install
directories for a file named after the library nickname and never read sym-lib-table. On any
non-standard KiCad install — a macOS bundle outside /Applications, a portable Windows copy —
nothing resolved. Footprints got table-aware lookup in v0.2.1; symbols never did.

add_schematic_text wrote files KiCad could not open. It inserted the node after the
symbol instances (KiCad 10 requires those last) and wrote literal newlines where the format
wants \n. Either alone makes the schematic fail to load with no indication of which element
is at fault, while the tool reports success. Worse, once the file will not load kicad-cli sch erc leaves the previous report in place — so a stale ERC result is what gets read next.

String escapes decoded wrongly. unescape collapsed backslashes last, so C:\\new came
back as C:\ plus a real newline. Hits Windows paths in Datasheet, 3D-model references and
Sheetfile.

Added tooling

  • sch_bus — add_bus, batch_add_bus, add_bus_entry, connect_pins_to_bus.
    konnect-sexp already had format_bus and BusEntryDirection and SchematicBuilder
    already round-tripped bus nodes, but nothing could create one. connect_pins_to_bus writes
    the stub, the entry and the member label per pin, because bus membership in KiCad is by
    name — a stub without a label joins nothing.
  • set_schematic_page — nothing could set the sheet size, and content off the frame still
    exports and still nets up, so an undersized page is a silent defect.
  • update_symbols_from_library — eeschema's Update Symbols from Library. Refuses any
    symbol whose pin anchors moved, since wires sit at the old coordinates.
  • rename_project — renames the four project files and the internal (project "name")
    references. Renaming files alone makes KiCad treat the design as unannotated, losing every
    reference designator.
  • batch_add_no_connect — batch_delete_no_connect existed with no batch add; one MCU's
    unused pins is routinely 15–20 round trips.
  • reload_server — execs into the rebuilt binary in place, same PID and stdio pipes, so
    the MCP session survives a rebuild. Verifies the new binary first.

Validation

cargo fmt --all -- --check, cargo clippy --workspace -- -D warnings, and
cargo test --workspace --lib --tests — 515 passing. cargo check --locked --manifest-path crates/schematic-viewer/Cargo.toml passes too, which the viewer CI job
requires.

Counts reconciled across the merge: 19 toolsets, 195 registered tools + 7 meta-tools.
sch_components 17 → 19, sch_wiring 19 → 20, project 6 → 7, and the tools_added
assertion in protocol_stdio.rs 36 → 39.

Three merge hazards were resolved rather than papered over: two pairs of branches appended
test modules at the same point in sch_batch.rs and sch_components.rs, and every branch
claimed the same next tool_count.

Relationship to the upstream PRs

Each fix is also offered upstream on its own branch, one per PR (up/*), each based directly
on upstream/main and carrying only its own change. This branch is the integrated result for
use here; those are the reviewable units. As they land upstream, rebuild this branch on the
new upstream/main rather than merging it back.

Not included, and still fork-only: publish-to-fork.sh, version_history.md, CLAUDE.md,
docs/CODE_REVIEW.md and the KiCad bug reports.

JYPochez and others added 22 commits August 14, 2026 14:35
@JYPochez @claude
fix(sexp): decode string escapes in one pass
04e582e 
unescape chained four replace calls with the backslash collapse last, so
an already-escaped backslash was re-read as the introducer of the next
escape. KiCad escapes backslashes in property values, so a serialized
`C:\\new` (the literal path `C:\new`) decoded to `C:\` followed by a real
newline, and `a\\tb` to `a\` plus a tab. Any Windows path whose next
character was n, t or r came back corrupted.

Replaced with a single left-to-right scan. Unknown escapes now keep both
characters rather than dropping the backslash, so values round-trip.

Blast radius was limited to reads: the parser is read-only and all writes
go through targeted text edits, so no file was ever damaged by this — the
wrong string was just handed to the caller.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JYPochez @claude
fix(sch-editor): resolve symbol libraries through sym-lib-table
91601e3 
resolve_lib_symbol never read sym-lib-table. It scanned a hardcoded list of
install directories for a file named after the library nickname, which fails
two ways, both total rather than partial.

Every user library was invisible: registered in the table but living anywhere
else on disk, so add_schematic_component and replace_component failed for any
part not shipped with KiCad. And KiCad's own libraries were invisible on any
non-standard install — the macOS candidates are /Applications/KiCad,
/usr/local/share/kicad and ~/Applications, so a bundle kept anywhere else makes
find_symbol_dirs() return empty and *no* symbol resolves at all.

The scan also assumed nickname == filename, which KiCad never requires: a table
may register .../TM16xx.kicad_sym under the nickname JY-TM16xx.

Footprints already resolved through the lib-table (mixelpixx#61). Symbols did not.

sym-lib-table is now consulted first, with the directory scan kept as a fallback
for setups with no table. The reader follows (type "Table") indirection
depth-bounded, expands ${KIPRJMOD} against the table's own directory, expands
exported environment variables, and expands user path variables from
kicad_common.json — those are set in Preferences > Configure Paths, are not
process environment variables, and are the normal way to write a portable table.
${KICAD*_SYMBOL_DIR} falls back to the install root recovered from the table's
own location.

Verified live: JY-TM16xx:TM1637, JY-KYX3561AS:3661BW, MCU_WCH_RiscV:CH32V003FxPx,
Device:R and power:GND all resolve where previously none did.

430 tests pass (8 added); clippy --workspace -D warnings clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JYPochez @claude
fix(sch): resolve pins against the owning unit of a multi-unit symbol
473c591 
A multi-unit symbol — a 74HC14, an op-amp, anything eeschema splits into gates
— is placed as one instance per unit, all sharing the reference.
batch_connect_to_net took the first instance matching the reference and
transformed every requested pin by that instance's placement, so asking for a
pin owned by unit 2 put the label on unit 1's pin.

Two different nets then occupied one coordinate and were silently shorted. No
error, no warning: the tool reported success with a plausible-looking position.
Caught in real use — X_CLK_IN and X_DATA_IN both landed on U6 pin 1 while
wiring a 74HC14 scale buffer, and only a coordinate that looked wrong gave it
away.

Now every instance sharing the reference is searched and the pin is resolved
against the one whose unit owns it, via extract_lib_pins_for_unit, so the
instance lookup and the pin transform come from the same unit.

The test places two units 15.24mm apart, each owning a pin at the same local
coordinate, and asserts the resolved positions differ and match their own
unit's placement — which fails against the old code.

439 tests pass (2 added); clippy --workspace -D warnings clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JYPochez @claude
fix(sch): apply component-level edits to every unit of a multi-unit s…
742b02c 
…ymbol

find_symbol_instance_block returns the first (symbol ...) block matching a
reference. A multi-unit part is one block per unit, all repeating that
reference, so three handlers that mean "the component" were silently acting
on unit 1 alone:

  - batch_edit_schematic_components wrote Value/Footprint/custom fields into
    unit 1 only, leaving a part claiming a footprint in one unit and none in
    the rest. eeschema keeps these copies identical; the file was left
    internally inconsistent. Found assigning SOIC-14 to a 74HC14: 1 of 7
    units took it.
  - batch_delete_schematic_components and batch_delete removed unit 1's block
    and left the others as orphans.
  - bulk_move_schematic_components shifted unit 1 and tore the part apart.

Add find_all_symbol_instance_blocks, returning every instance in file order.
find_symbol_instance_block now delegates to it and takes the first, so
single-placement callers are unchanged. field_value_range and
find_symbol_block become plural and the three handlers apply their edit to
each unit; batch_edit reports the unit count so the fan-out is visible.

Tests: five over a 3-unit part beside a single-unit part — field edit reaches
all three copies and spares the neighbour, single-unit still edits once,
missing field and unknown reference yield nothing, delete removes every unit
but keeps the lib_symbols definition, and the blocks are disjoint and ordered
as apply_edits requires. 439 -> 444.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JYPochez @claude
feat(sch): add the sch_bus toolset — buses, bus entries, pin fan-out
7f07ab9 
Konnect could not draw a bus. SchematicBuilder already round-tripped bus and
bus_entry nodes and knew their slot in KiCad's required element order, so a
bus drawn in eeschema survived a Konnect edit, but nothing could create one.
Any repeated multi-signal link had to be one wire per signal or bare labels.

sch_wiring is at 19 tools and the registry invariant caps a toolset at 20, so
this lands as a new toolset rather than crowding that limit.

  - add_bus / batch_add_bus: bus segments. Geometrically identical to wires;
    KiCad distinguishes them by node name and the attached label.
  - add_bus_entry: the 45-degree tick bridging wire and bus. Not decoration —
    a wire and bus that merely touch are not connected without one.
  - connect_pins_to_bus: per pin, the stub, the entry, AND the member label,
    because bus membership is by name and a stub with no label joins nothing.
    Pin resolution goes through the owning unit, so multi-unit parts work.

Also adds format_bus/format_bus_entry to konnect-sexp.

Verified against KiCad rather than only unit-tested: two 4-pin connectors
fanned onto one bus export a netlist with D0..D3 each carrying exactly its
two pins, ERC 0 violations. Unit tests cover the node name, the signed size
offset on an entry (wrong sign puts the tick past the bus, connecting
nothing), and registration. Registry counts and tool-directory.md updated:
18 -> 19 toolsets, 187 -> 191 tools. 447 -> 450.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JYPochez @claude
fix(sch): add_schematic_text produced schematics KiCad could not open
030b562 
Two independent defects, either fatal at the file level:

  - The node was inserted at content.rfind(')'), placing it after the symbol
    instances and sheet_instances. KiCad 10 requires instances last.
    sch_wiring::insert_before_close already existed for this, with the
    ordering rule documented above it; this handler never used it.
  - Only backslash and quote were escaped, so a multi-line annotation was
    written with literal newline bytes inside the quoted string. KiCad's
    reader wants the two-character escape.

Either one yields "Failed to load schematic" with no indication of which
element is at fault, while the tool reports success. Worse, once the file
will not load, kicad-cli erc leaves any previous report in place, so a stale
ERC result is what gets read next.

Make insert_before_close pub(crate) and use it; escape \r (dropped), \n and
\t alongside the existing handling.

Tests: text lands before the first symbol instance and after lib_symbols; a
multi-line string is written with \n escapes not raw bytes; quotes,
backslashes and tabs are escaped. The first two fail against the old code.
444 -> 447.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JYPochez @claude
feat(sch): add batch_add_no_connect
09815b1 
batch_delete_no_connect existed with no batch add — an asymmetry that only
shows up when it bites. Marking one MCU's unused pins is routinely 15-20
flags, each a separate add_no_connect round trip; this design needed 23 for
the CH32V203, the programmer header and the three USB sockets.

Mirrors the delete tool. A malformed entry is reported and skipped rather
than aborting, so placing twenty flags fails as "nineteen landed and one is
named" instead of losing the batch.

Tests: all flags land in one pass, one bad entry does not cost the good ones,
empty list is a no-op. sch_wiring 19 -> 20 tools (its cap), 192 -> 193 total.
455 -> 458.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JYPochez @claude
feat(sch): add set_schematic_page
93d2045 
Nothing could set the sheet size. create_schematic emits a blank sheet with
no (paper ...) node at all, and there was no way to change one, so a design
that outgrew A4 either kept content hanging off the frame or forced a hand
edit of the .kicad_sch — the thing the tooling exists to prevent.

An undersized page is silent: content outside the frame still exports and
still nets up, and ERC says nothing. The tool returns the chosen size in mm
so a caller can check it against the layout extents.

Handles A0-A5, A-E, US Letter/Legal/Ledger; replaces an existing paper node
or inserts one into the header ahead of any element; an unknown size is a
structured InvalidArgument that leaves the file untouched.

Tests: replace without leaving the old size, insert into a blank sheet before
the first element, portrait flag, unknown size writes nothing, and a table
invariant that every entry is stored landscape since portrait swaps them.
Registry and tool-directory updated: sch_components 18 -> 19, 191 -> 192
tools. 450 -> 455.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JYPochez @claude
feat(sch): add update_symbols_from_library
be00b25 
Editing a symbol in its library had no effect on schematics already using it.
add_schematic_component and replace_component both go through
ensure_lib_symbol_in_schematic, which short-circuits when a definition with that
lib_id is already embedded, so the schematic keeps its stale copy. Hit for real:
widening the TM16xx body in the library left every placed instance rendering at
the old width, and replace_component with the same lib_id did not refresh it.
eeschema has Tools > Update Symbols from Library for this; Konnect had nothing,
leaving a manual GUI step as the only route.

The new tool re-resolves each embedded lib_symbols entry from disk and replaces
it, with an optional lib_id filter and dry_run.

Wires and labels attach at pin coordinates, so a library edit that moved a pin
would silently orphan them. The tool therefore compares pin anchors before and
after and refuses any symbol whose pins moved, reporting why, unless
allow_pin_moves is passed. Body size and pin length changes leave anchors
untouched — the safe case, and the one that motivated this.

sch_components 17 -> 18 tools; registry tool_count updated.

437 tests pass (4 added); clippy --workspace -D warnings clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JYPochez @claude
feat(project): add rename_project
10ff1e4 
Renaming a KiCad project by hand is a four-file rename plus an edit that is
easy to miss: every symbol instance stores (project "name"), and the sheet
instance path is keyed on it. Rename only the files and KiCad loads the design
as unannotated — every reference designator is gone, and re-annotating
renumbers parts that a PCB is already laid out against.

rename_project renames the .kicad_pro/.kicad_sch/.kicad_pcb/.kicad_prl set and
rewrites the internal references together, the equivalent of eeschema's
File > Save As. dry_run reports the planned moves and rewrites without
touching anything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JYPochez @claude
feat(mcp): add a reload_server meta-tool
97c94f2 
Changing Konnect's source and rebuilding does nothing until the MCP client is
restarted, because the client spawns the server and holds it for the session.
Replacing the binary underneath a running server just kills the connection.

A stdio server cannot restart itself by exiting — the client owns the process
lifecycle and does not respawn it mid-session. So reload_server execs into the
binary on disk instead: that replaces the process image while keeping the PID
and the inherited stdin/stdout pipes, so the connection is never broken and the
client goes on talking to the new build.

exec is a one-way door, so the new binary is run once (--version) and checked
before the switch. A half-written copy, a failed link, or an unsigned binary
macOS would kill becomes a refused call naming the reason rather than a server
that is simply gone. confirm=true is required so a stray call cannot restart
mid-task, and the reply is written before the switch since exec never returns
on success. Windows gets a clear unsupported error — no exec equivalent keeps
the pipes.

Router state does not survive; the new image starts at the starter kit. That is
self-healing: a call to a previously loaded tool returns toolset_not_loaded
naming its toolset, so recovery is one hop.

Also pins the meta-tool count in a test. It is quoted in DEV.md, README.md and
tool-directory.md, so adding one now forces those to be updated in the same
commit instead of drifting. Meta-tools 6 -> 7, docs updated to 194 total.

433 tests pass (3 added); clippy --workspace -D warnings clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JYPochez @claude
build(viewer): record serde_json in the schematic-viewer lockfile
a7b07a1 
crates/schematic-viewer is its own workspace with its own Cargo.lock, and it
depends on konnect-schematic-editor by path. Adding serde_json to that crate
therefore invalidates the viewer's lockfile as well as the root one, and the
viewer CI job runs with --locked, so it fails with "cannot update the lock
file ... because --locked was passed".

One added edge, no version churn. Verified with the command CI runs:
cargo check --locked --manifest-path crates/schematic-viewer/Cargo.toml

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JYPochez
Merge branch 'up/sym-lib-table' into vns
eada9f6
@JYPochez
Merge branch 'up/multi-unit' into vns
caa1158
@JYPochez
Merge branch 'up/sch-text' into vns
bcf905f 
# Conflicts:
#	crates/konnect-core/src/tools/sch_batch.rs
@JYPochez
Merge branch 'up/sch-bus' into vns
bd2f09e
@JYPochez
Merge branch 'up/batch-no-connect' into vns
e1368c7 
# Conflicts:
#	tool-directory.md
@JYPochez
Merge branch 'up/page-size' into vns
ad01dd8 
# Conflicts:
#	tool-directory.md
@JYPochez
Merge branch 'up/update-symbols' into vns
df2deb7 
# Conflicts:
#	crates/konnect-core/src/tools/sch_components.rs
#	tool-directory.md
@JYPochez
Merge branch 'up/rename-project' into vns
8503c87 
# Conflicts:
#	tool-directory.md
@JYPochez
Merge branch 'up/reload-server' into vns
febd5c0 
# Conflicts:
#	tool-directory.md
@JYPochez @claude
chore: reconcile tool counts and docs across the merged fixes
53e6134 
sch_components gains both set_schematic_page and update_symbols_from_library
(17 -> 19) and sch_wiring gains batch_add_no_connect (19 -> 20), so the
load_toolset batch assertion for [sch_components, sch_wiring] moves 36 -> 39.
Registry counts, tool-directory rows and totals brought in line: 19 toolsets,
195 registered tools + 7 meta-tools.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pauliuszaleckas

pauliuszaleckas commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

I guess this PR was created by accident?

@mixelpixx

mixelpixx commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Thanks for putting this together, and for taking the trouble to replay the fork's work onto real upstream history rather than leaving it on a vendored branch — that part was the right call and it made this reviewable at all.

I'm going to take the ten individual PRs (#170–#179) instead of this bundle, for a concrete reason rather than a policy one.

I verified this branch is the exact union of the ten: every single-owner file diff is byte-identical to the corresponding PR, and the multi-owner files reconcile line-for-line. So nothing is missing. The problem is the history it would bring with it:

  1. Three intermediate commits carry literal conflict markers. df2deb7df, 8503c8717 and febd5c077 all contain <<<<<<< HEAD / ======= / >>>>>>> up/update-symbols in crates/konnect-core/src/tools/sch_components.rs. Those commits cannot compile. CI is green because it only builds the head, so this doesn't show up as a failure — but it would poison git bisect permanently, and bisect is how we found the add_via regression in v0.2.1.
  2. A duplicated attribute survives the cleanup — sch_components.rs:2209-2210 has #[cfg(test)] twice over mod page_tests. Legal Rust, so it compiles, but it's residue from the conflict resolution.
  3. The reconcile commit contradicts itself. 53e6134f0 takes tool-directory.md to "19 toolsets, 195 registered + 7 meta = 202", but DEV.md still says "18 toolsets, 187 tools ... 194 total" and DEV.md:55 still says "6 always-visible meta-tools", while README.md:16 is untouched at "187 tools across 18 on-demand toolsets". That's the drift CONTRIBUTING asks contributors to avoid, landing in the commit whose job was to fix it.

The ten separate PRs don't have any of these problems — they're each individually correct against main, which is exactly the "short PR series" shape CONTRIBUTING asks for. I'd rather merge them one at a time and keep a history that builds at every commit.

I've reviewed all ten and will comment on each with its own verdict. Short version: #174 and #171 are clean merges, #179 is a genuine data-corruption fix I want, #170/#173/#175 need small changes, and #172/#176/#177/#178 need rework or are superseded — details on each PR.

Closing this one in favour of the series. None of the work is lost.

@mixelpixx mixelpixx closed this Aug 14, 2026
@JYPochez

JYPochez commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

oups yes ! was meant to be local to my repo ! really sorry !

JYPochez added a commit to JYPochez/VNS-Kicad-Konnect that referenced this pull request Aug 14, 2026
@JYPochez
fix: remove duplicated cfg(test) and reconcile the tool counts in REA…
7aa5eba 
…DME/DEV

Both are residue from merging the ten fix branches, and both were caught in
upstream review of the bundle (mixelpixx#180):

  - sch_components.rs carried '#[cfg(test)]' twice over mod page_tests, left
    behind when the two branches' test modules were stitched together. Legal
    Rust, so it compiled and no test noticed.
  - the reconcile commit updated tool-directory.md to 19 toolsets / 195 tools
    + 7 meta but left DEV.md at '18 toolsets, 187 tools' and '6 meta-tools',
    and README.md at '187 tools across 18 on-demand toolsets' — the exact
    drift that commit existed to remove.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JYPochez @pauliuszaleckas @mixelpixx