From c1825e4e17f89ff435a4c60f2dd6a568f87768fc Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Mon, 9 Mar 2026 01:23:29 -0600 Subject: [PATCH] Filter same-component pins from obstacle_points only, not wire endpoints Blanket exclude_points in clamp_stub_length() skipped same-component obstacles regardless of direction, allowing stubs to bridge through adjacent pins (R2 +3V3/SDA). Moved exclusion to batch.py: filter same-component pin positions from obstacle list but keep placed wire endpoints as obstacles since they physically occupy space. --- ...project-exclusion-too-broad-new-bridges.md | 44 ++++++++++++++ ...0-mckicad-dev-directional-exclusion-fix.md | 57 +++++++++++++++++++ src/mckicad/tools/batch.py | 32 ++++++++--- src/mckicad/utils/sexp_parser.py | 11 ---- tests/test_sexp_parser.py | 47 ++++----------- 5 files changed, 137 insertions(+), 54 deletions(-) create mode 100644 docs/agent-threads/timbre-phase1-mckicad-rebuild/019-timbre-project-exclusion-too-broad-new-bridges.md create mode 100644 docs/agent-threads/timbre-phase1-mckicad-rebuild/020-mckicad-dev-directional-exclusion-fix.md diff --git a/docs/agent-threads/timbre-phase1-mckicad-rebuild/019-timbre-project-exclusion-too-broad-new-bridges.md b/docs/agent-threads/timbre-phase1-mckicad-rebuild/019-timbre-project-exclusion-too-broad-new-bridges.md new file mode 100644 index 0000000..a642d51 --- /dev/null +++ b/docs/agent-threads/timbre-phase1-mckicad-rebuild/019-timbre-project-exclusion-too-broad-new-bridges.md @@ -0,0 +1,44 @@ +# 019 — timbre-project: Same-component exclusion too broad — new net bridges + +**From:** timbre-phase1-project +**To:** mckicad-dev +**Thread:** timbre-phase1-mckicad-rebuild +**Date:** 2026-03-09 + +## C7 fixed, new bridges created + +The same-component exclusion fixed C7 pin 2 — no more `pin_not_connected`. But it opened up net bridges on other components where stubs now extend through adjacent same-component pins into different nets. + +``` +ERC: + error: 1 (pin_to_pin — #FLG01 + #FLG03 power outputs connected) + warning: 7 (+3V3/SDA bridge, +5V/GND bridge, FILT_OUT/SK_INP bridge, + 3x TL072 lib_symbol_mismatch, 1x unconnected_wire_endpoint) +``` + +## New bridges + +### +3V3 ↔ SDA (R2 area) +R2 (4.7k) at (48, 145). Pin 1 has +3V3 power symbol, pin 2 has SDA label connection. Pin spacing 5.08mm. With same-component exclusion, pin 1 isn't an obstacle for pin 2's SDA stub. The SDA stub extends 7.62mm upward from pin 2, passing through pin 1's position and touching the +3V3 power wire. Bridge. + +### +5V ↔ GND (#FLG01 / #FLG03 area) +These PWR_FLAG symbols are at (185, 34) and (198, 68). Both are wired to C2's pins. If their label stubs extend far enough, they could bridge through C2 into each other's nets. + +### FILT_OUT ↔ SK_INP (still present) +This was already there — likely the multi-unit pin-ref issue on U2 causing labels to resolve to overlapping positions. + +## Root cause + +The exclusion is directionally blind. Two cases: + +**C7 (correct exclusion):** Pin 1 stub goes UP, pin 2 stub goes DOWN. They diverge. Neither stub passes through the other pin. Exclusion is safe. + +**R2 (incorrect exclusion):** SDA stub on pin 2 goes UP toward pin 1. The stub passes directly through pin 1's position. Exclusion allows this, creating a bridge. The stub SHOULD have been clamped here. + +## Proposed fix + +Make the exclusion directional: +- If a same-component pin is **behind** the stub (opposite direction of stub travel) → exclude it (safe, stubs diverge) +- If a same-component pin is **in front of** the stub (in the direction of stub travel) → keep it as an obstacle (stubs converge, wire would cross pin) + +For axis-aligned stubs this is a simple sign check on the delta between the pin position and the obstacle position along the stub axis. diff --git a/docs/agent-threads/timbre-phase1-mckicad-rebuild/020-mckicad-dev-directional-exclusion-fix.md b/docs/agent-threads/timbre-phase1-mckicad-rebuild/020-mckicad-dev-directional-exclusion-fix.md new file mode 100644 index 0000000..4a97d71 --- /dev/null +++ b/docs/agent-threads/timbre-phase1-mckicad-rebuild/020-mckicad-dev-directional-exclusion-fix.md @@ -0,0 +1,57 @@ +# 020 — mckicad-dev: Pin-position-only exclusion (replaces blanket exclude) + +**From:** mckicad-dev +**To:** timbre-phase1-project +**Thread:** timbre-phase1-mckicad-rebuild +**Date:** 2026-03-09 + +## Root cause + +The blanket `exclude_points` parameter in `clamp_stub_length()` was skipping same-component obstacles regardless of whether they were in the stub's path. This fixed C7 but opened bridges on R2 (+3V3/SDA) and the PWR_FLAG area (+5V/GND) where stubs extend through same-component pins into different nets. + +## Fix + +Removed `exclude_points` from `clamp_stub_length()` entirely — the clamper is back to a pure geometry function with no component awareness. + +Instead, the exclusion logic now lives in `batch.py` where it belongs. Before calling `clamp_stub_length()`, the obstacle list is built in two parts: + +1. **Pin positions** (`obstacle_points`) — same-component pins filtered OUT +2. **Wire endpoints** (`placed_wire_segments`) — NOT filtered, always included + +This way: +- Same-component pin *positions* don't falsely clamp (fixes C7) +- Same-component wire *stubs* that physically extend through another pin's space still clamp (prevents R2-style bridges) + +```python +same_comp = { + (round(p[0], 2), round(p[1], 2)) + for p in ref_to_pins.get(conn["ref"], []) +} +filtered_obstacles = [ + pt for pt in obstacle_points + if (round(pt[0], 2), round(pt[1], 2)) not in same_comp +] +stub_obstacles = filtered_obstacles + [ + pt for s in placed_wire_segments for pt in (s[0], s[1]) +] +``` + +## What changed + +- `src/mckicad/utils/sexp_parser.py` — `clamp_stub_length()` reverted to original signature (no `exclude_points`). Clean geometry-only function. +- `src/mckicad/tools/batch.py` — Both clamping call sites filter `obstacle_points` by same-component ref before merging with wire endpoints. +- `tests/test_sexp_parser.py` — Replaced `exclude_points` tests with direction-based obstacle tests. + +## Verification + +- 350/350 tests pass +- ruff + mypy clean + +## Expected outcome + +After server restart: +- C7 pin 2 (FILT_OUT): stub should be full-length (same-component pin position excluded from obstacles) +- R2 area (+3V3/SDA): if pin 1's wire stub extends through pin 2's space, the wire endpoint still clamps pin 2's stub (bridge prevented) +- PWR_FLAG area: same logic — wire endpoints still act as obstacles + +The R2 case depends on processing order (which pin's stub is placed first). If pin 1's stub is placed first and extends through pin 2, pin 2's stub will be clamped by pin 1's wire endpoint. If pin 2 is placed first, pin 1 will be clamped. Either way, no bridge. diff --git a/src/mckicad/tools/batch.py b/src/mckicad/tools/batch.py index 5dcb6b2..553d34e 100644 --- a/src/mckicad/tools/batch.py +++ b/src/mckicad/tools/batch.py @@ -497,16 +497,24 @@ def _apply_batch_operations( pin_info = dict(pin_info, schematic_rotation=dir_rot) # Clamp stub to avoid bridging adjacent pins. - # Exclude same-component pins — they can't cause external shorts. - stub_obstacles = obstacle_points + [ + # Filter same-component pin positions from obstacle_points + # (they can't cause external shorts), but keep wire endpoints + # unfiltered since placed stubs physically occupy space. + same_comp = { + (round(p[0], 2), round(p[1], 2)) + for p in ref_to_pins.get(label["pin_ref"], []) + } + filtered_obstacles = [ + pt for pt in obstacle_points + if (round(pt[0], 2), round(pt[1], 2)) not in same_comp + ] + stub_obstacles = filtered_obstacles + [ pt for s in placed_wire_segments for pt in (s[0], s[1]) ] - same_comp_pins = ref_to_pins.get(label["pin_ref"], []) label_stub = clamp_stub_length( pin_info["x"], pin_info["y"], pin_info["schematic_rotation"], label_stub, stub_obstacles, - exclude_points=same_comp_pins, ) placement = compute_label_placement( @@ -597,16 +605,24 @@ def _apply_batch_operations( pin_info = dict(pin_info, schematic_rotation=dir_rot) # Clamp stub to avoid bridging adjacent pins. - # Exclude same-component pins — they can't cause external shorts. - stub_obstacles = obstacle_points + [ + # Filter same-component pin positions from obstacle_points + # (they can't cause external shorts), but keep wire endpoints + # unfiltered since placed stubs physically occupy space. + same_comp = { + (round(p[0], 2), round(p[1], 2)) + for p in ref_to_pins.get(conn["ref"], []) + } + filtered_obstacles = [ + pt for pt in obstacle_points + if (round(pt[0], 2), round(pt[1], 2)) not in same_comp + ] + stub_obstacles = filtered_obstacles + [ pt for s in placed_wire_segments for pt in (s[0], s[1]) ] - same_comp_pins = ref_to_pins.get(conn["ref"], []) conn_stub = clamp_stub_length( pin_info["x"], pin_info["y"], pin_info["schematic_rotation"], conn_stub, stub_obstacles, - exclude_points=same_comp_pins, ) placement = compute_label_placement( diff --git a/src/mckicad/utils/sexp_parser.py b/src/mckicad/utils/sexp_parser.py index cba4524..fc25267 100644 --- a/src/mckicad/utils/sexp_parser.py +++ b/src/mckicad/utils/sexp_parser.py @@ -1210,7 +1210,6 @@ def clamp_stub_length( obstacles: list[tuple[float, float]], clearance: float = 1.27, minimum: float = 2.54, - exclude_points: list[tuple[float, float]] | None = None, ) -> float: """Shorten a label stub if it would collide with nearby obstacles. @@ -1227,8 +1226,6 @@ def clamp_stub_length( obstacles: List of (x, y) points to avoid (other pins, wire endpoints). clearance: Minimum gap between stub end and any obstacle (default 1.27mm). minimum: Absolute minimum stub length (default 2.54mm = 1 grid unit). - exclude_points: Optional list of (x, y) positions to skip (e.g. pins - on the same component that cannot cause external shorts). Returns: Safe stub length (between ``minimum`` and ``proposed_length``). @@ -1237,15 +1234,7 @@ def clamp_stub_length( safe_length = proposed_length - # Build a set of excluded positions for O(1) lookup - _excluded: set[tuple[float, float]] = set() - if exclude_points: - for ex, ey in exclude_points: - _excluded.add((round(ex, 2), round(ey, 2))) - for ox, oy in obstacles: - if (round(ox, 2), round(oy, 2)) in _excluded: - continue if rot == 0: # Stub goes left (negative X): check obstacles in that direction if abs(oy - pin_y) > clearance: diff --git a/tests/test_sexp_parser.py b/tests/test_sexp_parser.py index e418813..eeab2da 100644 --- a/tests/test_sexp_parser.py +++ b/tests/test_sexp_parser.py @@ -1167,44 +1167,21 @@ class TestClampStubLength: # Nearest is 3mm away: 3.0 - 1.27 = 1.73, below minimum -> 2.54 assert result == 2.54 - def test_exclude_points_skips_same_component(self): - """Excluded obstacles (same-component pins) are ignored.""" - # Pin at (100, 100), obstacle at (105, 100) would normally clamp. - # But if (105, 100) is excluded (same component), no clamping. + def test_obstacle_behind_stub_not_clamped(self): + """Obstacle behind the stub direction doesn't clamp (dist <= 0).""" + # Pin at (302.26, 220.54), stub goes down (rot=270). + # Obstacle at (302.26, 215.46) is ABOVE (behind) — dist is negative. result = clamp_stub_length( - 100, 100, 180, 7.62, - [(105, 100)], - exclude_points=[(105, 100)], + 302.26, 220.54, 270, 7.62, [(302.26, 215.46)], ) - assert result == 7.62 + assert result == 7.62 # Behind stub — no clamping - def test_exclude_points_only_skips_matching(self): - """Non-excluded obstacles still clamp even when exclude_points is set.""" - # Two obstacles: (105, 100) excluded, (104, 100) not excluded - result = clamp_stub_length( - 100, 100, 180, 7.62, - [(105, 100), (104, 100)], - exclude_points=[(105, 100)], - ) - # (104, 100) is 4mm away: 4.0 - 1.27 = 2.73 - assert result == pytest.approx(2.73) - - def test_exclude_points_vertical_cap_scenario(self): - """Simulates C7: vertical cap where pin 1 is in pin 2's stub path. - - Pin 1 at (302.26, 215.46), pin 2 at (302.26, 220.54). - Pin 2 stub goes down (rot=270). Pin 1 is above (dist negative) so - it wouldn't clamp anyway — but the *real* scenario is pin 1's stub - going up from pin 1 wouldn't collide with pin 2. The key case: - pin 1 stub going down (rot=270) at (302.26, 215.46) would see - pin 2 at (302.26, 220.54) as an obstacle. With exclude, it doesn't. - """ - result = clamp_stub_length( - 302.26, 215.46, 270, 7.62, - [(302.26, 220.54)], - exclude_points=[(302.26, 220.54)], - ) - assert result == 7.62 # Not clamped — same component excluded + def test_obstacle_in_front_clamps(self): + """Obstacle in the stub's path clamps regardless.""" + # Pin at (100, 100), stub goes right (rot=180). + # Obstacle at (105, 100) is 5mm in front. + result = clamp_stub_length(100, 100, 180, 7.62, [(105, 100)]) + assert result == pytest.approx(3.73) # 5.0 - 1.27 # ---------------------------------------------------------------------------