From f11d453c31a7ad85bb10ce1370417c5336c0a51a Mon Sep 17 00:00:00 2001
From: Eugene Mikhantyev <mikhantiev@gmail.com>
Date: Sun, 26 Apr 2026 15:13:47 +0100
Subject: [PATCH] Tidy auto-junction-sync code

- Refactor _handle_rotate_schematic_component to use raw sexp throughout
  and write the schematic once instead of three times
- Hoist sexpdata and WireManager imports to module scope; drop the
  unnecessary underscore aliases in move/rotate handlers
- Move WireManager._SUB_UNIT_RE to the top of the class body
- Promote the per-call Symbol("symbol")/Symbol("unit") allocations in
  _parse_lib_pins / _collect_pin_positions to module-level _SYM_*
  constants
- Document the assumption behind _SUB_UNIT_RE's greedy match

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 python/commands/wire_manager.py |  20 +++---
 python/kicad_interface.py       | 104 ++++++++++++++++++++------------
 2 files changed, 76 insertions(+), 48 deletions(-)

diff --git a/python/commands/wire_manager.py b/python/commands/wire_manager.py
index 20c3043..bcbc257 100644
--- a/python/commands/wire_manager.py
+++ b/python/commands/wire_manager.py
@@ -37,6 +37,8 @@ _SYM_LIB_SYMBOLS = Symbol("lib_symbols")
 _SYM_LIB_ID = Symbol("lib_id")
 _SYM_MIRROR = Symbol("mirror")
 _SYM_PIN = Symbol("pin")
+_SYM_SYMBOL = Symbol("symbol")
+_SYM_UNIT = Symbol("unit")
 _IU_PER_MM = 10000
 
 
@@ -127,6 +129,11 @@ def _make_sheet_pin_text(
 class WireManager:
     """Manage wires in KiCad schematics using S-expression manipulation"""
 
+    # Regex to parse sub-unit names like "LM324_2_1" → (base="LM324", unit=2, style=1)
+    # The sub-unit suffix is <base>_<unit>_<style> where unit and style are integers.
+    # Assumes KiCad's <base>_<unit>_<style> convention (rightmost two underscore-separated numeric groups are unit/style); unparseable names fall back to including all pins via the else branch in _parse_lib_pins.
+    _SUB_UNIT_RE = re.compile(r"^(.+)_(\d+)_(\d+)$")
+
     @staticmethod
     def add_wire(
         schematic_path: Path,
@@ -498,10 +505,6 @@ class WireManager:
             [_SYM_UUID, str(uuid.uuid4())],
         ]
 
-    # Regex to parse sub-unit names like "LM324_2_1" → (base="LM324", unit=2, style=1)
-    # The sub-unit suffix is <base>_<unit>_<style> where unit and style are integers.
-    _SUB_UNIT_RE = re.compile(r"^(.+)_(\d+)_(\d+)$")
-
     @staticmethod
     def _parse_lib_pins(sym_def: list, unit: int = 1) -> List[Tuple[float, float]]:
         """Extract pin local (x, y) positions for *unit* from a lib_symbols symbol definition.
@@ -515,7 +518,6 @@ class WireManager:
 
         Uses a stack instead of recursion to handle nested sub-unit symbols.
         """
-        _SYM_SYMBOL = Symbol("symbol")
         pins: List[Tuple[float, float]] = []
 
         # Separate top-level direct children into sub-unit symbols vs other nodes.
@@ -583,8 +585,6 @@ class WireManager:
         Parses lib_symbols for pin local coordinates (unit-aware), then applies the KiCad
         transform chain (y-negate → mirror → rotate → translate) to each pin.
         """
-        _SYM_UNIT = Symbol("unit")
-
         # Build {lib_id: sym_def} from the embedded lib_symbols section.
         # We defer pin extraction until we know which unit each placed instance uses.
         lib_sym_defs: dict = {}
@@ -593,9 +593,7 @@ class WireManager:
                 continue
             for sym_def in item[1:]:
                 if not (
-                    isinstance(sym_def, list)
-                    and len(sym_def) > 1
-                    and sym_def[0] == Symbol("symbol")
+                    isinstance(sym_def, list) and len(sym_def) > 1 and sym_def[0] == _SYM_SYMBOL
                 ):
                     continue
                 lib_id = sym_def[1] if isinstance(sym_def[1], str) else str(sym_def[1])
@@ -605,7 +603,7 @@ class WireManager:
         # Transform each placed symbol's pins to world coordinates
         world_positions: List[Tuple[float, float]] = []
         for item in sch_data:
-            if not (isinstance(item, list) and len(item) > 0 and item[0] == Symbol("symbol")):
+            if not (isinstance(item, list) and len(item) > 0 and item[0] == _SYM_SYMBOL):
                 continue
             lib_id_part = next(
                 (
diff --git a/python/kicad_interface.py b/python/kicad_interface.py
index 34a3563..ebc30f5 100644
--- a/python/kicad_interface.py
+++ b/python/kicad_interface.py
@@ -15,7 +15,9 @@ import traceback
 from pathlib import Path
 from typing import Any, Dict, List, Optional, Tuple
 
+import sexpdata
 from annotations import AnnotationLoader
+from commands.wire_manager import WireManager
 from resources.resource_definitions import RESOURCE_DEFINITIONS, handle_resource_read
 
 # Import tool schemas, resource definitions, and IPC API annotations
@@ -2531,7 +2533,6 @@ class KiCADInterface:
         """Move a schematic component to a new position, dragging connected wires."""
         logger.info("Moving schematic component")
         try:
-            import sexpdata as _sexpdata
             from commands.wire_dragger import WireDragger
 
             schematic_path = params.get("schematicPath")
@@ -2553,7 +2554,7 @@ class KiCADInterface:
                 }
 
             with open(schematic_path, "r", encoding="utf-8") as f:
-                sch_data = _sexpdata.loads(f.read())
+                sch_data = sexpdata.loads(f.read())
 
             # Find symbol and record old position
             found = WireDragger.find_symbol(sch_data, reference)
@@ -2592,12 +2593,10 @@ class KiCADInterface:
             # Update symbol position
             WireDragger.update_symbol_position(sch_data, reference, float(new_x), float(new_y))
 
-            from commands.wire_manager import WireManager as _WireManager
-
-            _WireManager.sync_junctions(sch_data)
+            WireManager.sync_junctions(sch_data)
 
             with open(schematic_path, "w", encoding="utf-8") as f:
-                f.write(_sexpdata.dumps(sch_data))
+                f.write(sexpdata.dumps(sch_data))
 
             return {
                 "success": True,
@@ -2630,46 +2629,77 @@ class KiCADInterface:
                     "message": "schematicPath and reference are required",
                 }
 
-            schematic = SchematicManager.load_schematic(schematic_path)
-            if not schematic:
-                return {"success": False, "message": "Failed to load schematic"}
+            sym_k = sexpdata.Symbol("symbol")
+            prop_k = sexpdata.Symbol("property")
+            at_k = sexpdata.Symbol("at")
+            mirror_k = sexpdata.Symbol("mirror")
 
-            for symbol in schematic.symbol:
-                if not hasattr(symbol.property, "Reference"):
+            with open(schematic_path, "r", encoding="utf-8") as _f:
+                sch_data = sexpdata.load(_f)
+
+            target = None
+            for item in sch_data:
+                if not (isinstance(item, list) and item and item[0] == sym_k):
                     continue
-                if symbol.property.Reference.value == reference:
-                    pos = list(symbol.at.value)
-                    while len(pos) < 3:
-                        pos.append(0)
-                    pos[2] = angle
-                    symbol.at.value = pos
+                ref_val = None
+                for sub in item[1:]:
+                    if (
+                        isinstance(sub, list)
+                        and len(sub) >= 3
+                        and sub[0] == prop_k
+                        and str(sub[1]).strip('"') == "Reference"
+                    ):
+                        ref_val = str(sub[2]).strip('"')
+                        break
+                if ref_val == reference:
+                    target = item
+                    break
 
-                    if mirror:
-                        if hasattr(symbol, "mirror"):
-                            symbol.mirror.value = mirror
-                        else:
-                            logger.warning(
-                                f"Mirror '{mirror}' requested for {reference}, "
-                                f"but symbol has no mirror attribute; skipped"
-                            )
+            if target is None:
+                return {"success": False, "message": f"Component {reference} not found"}
 
-                    SchematicManager.save_schematic(schematic, schematic_path)
+            # Update (at x y rot)
+            at_node = None
+            mirror_idx = None
+            for idx, sub in enumerate(target[1:], start=1):
+                if not isinstance(sub, list) or not sub:
+                    continue
+                if sub[0] == at_k:
+                    at_node = sub
+                elif sub[0] == mirror_k:
+                    mirror_idx = idx
 
-                    # Re-open with raw sexpdata so sync_junctions can operate on
-                    # the mutated file (mirrors the pattern used in move handler).
-                    import sexpdata as _sexpdata
+            if at_node is None:
+                return {
+                    "success": False,
+                    "message": f"Component {reference} has no (at ...) node",
+                }
+            while len(at_node) < 3:
+                at_node.append(0)
+            if len(at_node) < 4:
+                at_node.append(angle)
+            else:
+                at_node[3] = angle
 
-                    with open(schematic_path, "r", encoding="utf-8") as _f:
-                        sch_data = _sexpdata.load(_f)
-                    from commands.wire_manager import WireManager as _WireManager
+            if mirror:
+                mirror_node = [mirror_k, sexpdata.Symbol(str(mirror))]
+                if mirror_idx is not None:
+                    target[mirror_idx] = mirror_node
+                else:
+                    # Insert after (at ...) for stability
+                    insert_at = len(target)
+                    for idx, sub in enumerate(target[1:], start=1):
+                        if isinstance(sub, list) and sub and sub[0] == at_k:
+                            insert_at = idx + 1
+                            break
+                    target.insert(insert_at, mirror_node)
 
-                    _WireManager.sync_junctions(sch_data)
-                    with open(schematic_path, "w", encoding="utf-8") as _f:
-                        _f.write(_sexpdata.dumps(sch_data))
+            WireManager.sync_junctions(sch_data)
 
-                    return {"success": True, "reference": reference, "angle": angle}
+            with open(schematic_path, "w", encoding="utf-8") as _f:
+                _f.write(sexpdata.dumps(sch_data))
 
-            return {"success": False, "message": f"Component {reference} not found"}
+            return {"success": True, "reference": reference, "angle": angle}
 
         except Exception as e:
             logger.error(f"Error rotating schematic component: {e}")