fix(design-review): resolve audit nets through the net graph

[pauliuszaleckas]

Summary

audit_decoupling, audit_connections, audit_power_rails and
run_design_review's coverage found a pin's net by scanning the file text for
(label " or (global_label " within 0.5 mm of the point. That follows no
wires, reads no junctions and knows no power symbols, so on any sheet drawn the
normal way almost every pin was netless. run_design_review is the "is my board
ready?" tool, and it returned NOT READY — critical issues must be fixed before manufacturing on a correct schematic, with all of its errors false.

Found dogfooding on a real board (25 footprints, 99 nets, KiCad 10.0). On that
sheet:

Tool	Reported	Actually
audit_decoupling	0/2 power pins have decoupling	C5 (100 nF) and C6 (1 µF) on U1's VCC, C1/C2 on U2's VIN
audit_power_rails	power_nets: ["VBUS"]	+3V3 and GND are rails too — they are power symbols
audit_power_rails	Power rail 'VBUS' has no decoupling capacitors	C1 and C2 sit on the VBUS wire
run_design_review	coverage.named_nets: 24	26, the two missing ones being the rails

The VBUS row is the sharpest: that rail is label-named and the audit still
missed its caps, because C1's pin is 3.81 mm from the label — eight times the
tolerance. So the failure was never only "power symbols are invisible"; any pin
more than 0.5 mm from the label naming its net was netless to these audits. A
sheet drawn entirely with labels at pins would pass, which is exactly the
sheet the tools were written against.

Not linked to an issue; the tracker has nothing on it. #184 and #247 made
run_design_review fail closed when coverage is incomplete, and neither touches
what the audits themselves resolve. #323 gave the connectivity tools one shared
index and left find_net_at_point out because it never used the net graph —
this is what that exclusion cost.

Introduced by dd49a86 (the initial public release). 83ec1cd fixed the tag it
searched for (label, not net_label) without changing the approach.

Approach

Resolve through net_graph_for — the graph sch_analysis and sch_export
already share — so a pin reaches its net along the wires and junctions it is
drawn with, and a rail named by a power symbol is a net like any other.
find_net_at_point is deleted, and with it the last 0.5 mm text scan on the
schematic side.

• collect_power_nets reads extract_all_net_labels instead of scanning for
  (label ", so a power-symbol rail is a rail. PWR_FLAG still is not one: its
  pin is power_out, which the extractor skips. The list is now sorted; it was
  HashSet iteration order.
• One pin_nets(graph, pins) is the only definition of "the nets this placed
  unit's pins reach", with capacitor_nets, test_point_nets and
  pull_up_nets as filters over it. Its pins come from
  tools::placed_pins_by_reference, so the walk is unit-aware: a dual
  amplifier whose power pin lives on unit 3 no longer reports that pin at
  units 1 and 2 as well. audit_decoupling and audit_connections walk the
  same representation, so unit selection and lib_name resolution have one
  definition rather than three. That is the design-review half of sch_batch, sch_export, sch_analysis and design_review still resolve pins against unit 1 #182; the
  sch_analysis, sch_export and sch_batch half is untouched and still open.
• With the audits unit-aware, run_design_review's
  multi_unit_review_incomplete diagnostic is false — it made every review of
  a multi-unit sheet INCOMPLETE. Dropped; coverage.multi_unit_symbols stays,
  as a count.
• pull_up_nets is built once per sheet. has_pull_up_on_net was called per
  I2C or reset pin and re-walked every resistor on the sheet each time.
• Test-point nets were a stub that always returned an empty set — its body a
  comment saying a proper implementation would trace the wire — so every rail
  was reported as having no test point. It traces the wire now.
• run_design_review's coverage.named_nets counts extract_all_net_labels,
  which makes it agree with what list_schematic_nets reports for the same
  sheet.

net_graph_for rather than the ConnectivityIndex #323 landed: the audits want
net names at points, which is that function's exact contract, and it is the
primitive the other ten read-only tools already use. ConnectivityIndex parses
pin geometry these audits do not need.

Compatibility and safety

No tool names, arguments or response keys change. Read paths only — nothing is
written, no IPC. The findings these audits produce do change, which is the point
of the fix: fewer, and true.

Validation

Eight tests in a new net_resolution_tests module, over sheets shaped like the
one above: a capacitor reaching a power pin down a wire under a junction dot, a
rail named only by a power symbol, a test point at the far end of the wire, and
an I2C pull-up whose two ends are a wire and a power symbol.

Four of the five net-resolution sheets fail against the old resolver, and its
output on those sheets reproduces every row of the table above. The fifth is the fitted pull-up,
which the old code also passed — by never resolving either pin and reporting
nothing — so its pair, the missing pull-up that used to go unreported, is tested
alongside it. That direction matters as much: a pin whose net could not be
resolved was skipped silently, so the fault the audit exists to find was dropped.

Three of the eight cover the unit-aware walk: a dual amplifier whose V+ sits
on a third unit is one power pin and not three, that same sheet reviews as
complete rather than INCOMPLETE, and each placement of the checked-in
ecc83_multiunit.kicad_sch (two triodes and a heater, placed apart) walks only
its own three pins. All three fail against the unit-agnostic walk.

Not re-run against the board itself; it is not in this repository.

• cargo fmt --all -- --check
• cargo test --workspace --locked --lib --tests (what CI runs)
• cargo test --workspace --locked --doc
• cargo clippy --workspace --locked -- -D warnings
• Relevant viewer, plugin, packaging, and real-KiCad checks — not applicable

konnect-sexp's every_installed_demo_board_scans_losslessly fails on my
machine, on main with this branch's commit reverted as well: it skips 2 zones
from royalblue54L_feather in the locally installed KiCad demo corpus. Pre-existing
and unrelated to this change, which touches one file in konnect-core.

Known limitations

• NetGraph::net_at returns the first point_nets entry whose union-find root
  matches — HashMap order. A net carrying two names therefore resolves to an
  arbitrary one of them while both are listed as rails, so the other reports as
  undecoupled, and which one changes between runs. Pre-existing in
  sch_connectivity, but this change makes design_review its heaviest caller.
  Fixing it means comparing nets by identity rather than by name, which is its
  own change.
• pull_up_nets still decides "the other end is a rail" with the
  is_power_net_name heuristic, so a pull-up to a rail named by a power symbol
  but not by that regex — VBAT, VDDA_EXT — is still invisible.

Review checklist

• The diff is focused and contains no generated output, personal data, or unrelated cleanup.
• New names follow docs/NAMING_CONVENTIONS.md; public renames include compatibility handling.
• New behavior and failure paths have regression coverage.
• File mutations are atomic and preserve unrelated content — read paths only, nothing written.
• IPC mutations verify the requested board and do not leave partial batches — no IPC.
• If tools were added/removed: counts and docs updated — no tools added or removed.

[neusse]

This PR's own Known limitations section points to #182: the new pin_nets helper still applies each placed unit's transform to extract_lib_pins(sym), which contains every unit's pins. On a multi-unit part that superimposes phantom pins/nets at every placement.

Could this PR make that walker unit-aware before merge? The smallest correction is to iterate extract_lib_pins_for_unit(sym, inst.unit) in pin_nets. The earlier handle_audit_decoupling and handle_audit_connections walks should receive the same treatment, or preferably consume the shared placed_pins_by_reference representation so unit selection and lib_name resolution stay centralized.

The checked-in crates/konnect-core/tests/fixtures/ecc83_multiunit.kicad_sch fixture is a good regression instrument: U1 has three separately placed units and the heater unit owns different pins from the two triodes. A test should prove an audit never reports another unit's pins/nets at the current unit's coordinates.

I am taking the independent sch_analysis/sch_export and sch_batch coverage portion of #182 separately, so there should be no file overlap with this PR. If you prefer to leave the unit-aware review change as a follow-up, please keep the known-limitation note and I will re-audit immediately after this merges.

[pauliuszaleckas]

Done in cdfa5d8 — the walker is unit-aware before merge, and by the route you preferred: pin_nets, handle_audit_decoupling and handle_audit_connections all consume tools::placed_pins_by_reference, so unit selection and lib_name resolution stay in one place rather than three. The audits pick up the derived-symbol handling (#143) with it.

Regression coverage is three tests. Two are a synthetic dual amplifier whose V+ sits on unit 3 with units 1 and 2 placed elsewhere: audit_decoupling reports one power pin instead of three (two of the phantoms on no net, so they were false errors), and run_design_review on that sheet is complete. The third walks ecc83_multiunit.kicad_sch and asserts each placement contributes only its own pins — 6/7/8, 1/2/3, 4/5/9 — never another unit's at its coordinates. All three fail against the unit-agnostic walk.

One consequence worth flagging: with the audits unit-aware, the multi_unit_review_incomplete diagnostic was false — it made every review of a multi-unit sheet INCOMPLETE — so it is gone. coverage.multi_unit_symbols stays as a plain count.

The sch_analysis/sch_export/sch_batch half of #182 is untouched here, so no file overlap with your change, and #182 stays open for it.