fix: rotate_schematic_component mirror parameter no longer crashes

Previously passing mirror='x' or mirror='y' to rotate_schematic_component
always raised:
  'NoneType' object has no attribute 'value'

Root cause: kicad-skip has no API for setting (mirror x/y) on a placed
symbol instance. The handler tried to use a non-existent kicad-skip
attribute, returning None, then calling .value on it.

Fix: add _apply_mirror_to_symbol_sexp() which directly patches the
(mirror x/y) S-expression token in the .kicad_sch file. The rotate
handler now applies rotation via kicad-skip (which works fine) and
delegates mirror to this helper.

The helper:
- Inserts (mirror x/y) immediately after the (at x y rot) token
- Removes any existing mirror token before inserting the new one
  (prevents duplicate tokens when toggling axis)
- Returns False gracefully when the reference is not found

Tests: 6 unit tests in tests/test_rotate_schematic_mirror.py
covering: add x, add y, remove, replace, unknown ref, no-mirror smoke test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

python/kicad_interface.py

@@ -2123,6 +2123,87 @@ class KiCADInterface:
             logger.error(traceback.format_exc())
             return {"success": False, "message": str(e)}
 
+    @staticmethod
+    def _apply_mirror_to_symbol_sexp(
+        schematic_path: str,
+        reference: str,
+        mirror: Optional[str],
+    ) -> bool:
+        """
+        Apply or remove (mirror x/y) on a placed symbol instance by direct
+        S-expression manipulation of the .kicad_sch file.
+
+        KiCad stores mirroring as a sibling token to (at ...):
+            (symbol ... (at x y rot) (mirror x) ...)
+        kicad-skip has no API for this, so we patch the raw text.
+
+        Args:
+            schematic_path: Absolute path to .kicad_sch file
+            reference:      Reference designator, e.g. "Q1"
+            mirror:         "x", "y", or None (None removes any existing mirror)
+
+        Returns:
+            True on success, False if symbol not found.
+        """
+        import re
+
+        with open(schematic_path, "r", encoding="utf-8") as f:
+            content = f.read()
+
+        def find_symbol_block(text: str, ref: str):
+            """Return (start, end) indices of the symbol block for ref."""
+            i = 0
+            n = len(text)
+            while i < n:
+                idx = text.find("(symbol ", i)
+                if idx == -1:
+                    break
+                depth = 0
+                j = idx
+                while j < n:
+                    if text[j] == "(":
+                        depth += 1
+                    elif text[j] == ")":
+                        depth -= 1
+                        if depth == 0:
+                            block = text[idx : j + 1]
+                            ref_pattern = (
+                                r'\(property\s+"Reference"\s+"' + re.escape(ref) + r'"'
+                            )
+                            if re.search(ref_pattern, block):
+                                return idx, j + 1
+                            break
+                    j += 1
+                i = idx + 1
+            return None, None
+
+        start, end = find_symbol_block(content, reference)
+        if start is None:
+            logger.warning(f"Symbol '{reference}' not found in {schematic_path}")
+            return False
+
+        block = content[start:end]
+
+        # Remove any existing (mirror ...) from block
+        block = re.sub(r"\s*\(mirror\s+[xy]\)", "", block)
+
+        # Insert new mirror token right after the (at ...) token
+        if mirror in ("x", "y"):
+            block = re.sub(
+                r"(\(at\s+[\d\.\-]+\s+[\d\.\-]+\s+[\d\.\-]+\))",
+                r"\1" + f" (mirror {mirror})",
+                block,
+                count=1,
+            )
+
+        new_content = content[:start] + block + content[end:]
+
+        with open(schematic_path, "w", encoding="utf-8", newline="\n") as f:
+            f.write(new_content)
+
+        logger.info(f"Applied mirror={mirror!r} to symbol '{reference}' in {schematic_path}")
+        return True
+
     def _handle_rotate_schematic_component(self, params: Dict[str, Any]) -> Dict[str, Any]:
         """Rotate a schematic component"""
         logger.info("Rotating schematic component")
@@ -2130,7 +2211,7 @@ class KiCADInterface:
             schematic_path = params.get("schematicPath")
             reference = params.get("reference")
             angle = params.get("angle", 0)
-            mirror = params.get("mirror")
+            mirror = params.get("mirror")  # "x", "y", or None
 
             if not schematic_path or not reference:
                 return {
@@ -2152,17 +2233,26 @@ class KiCADInterface:
                     pos[2] = angle
                     symbol.at.value = pos
 
-                    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"
-                            )
-
                     SchematicManager.save_schematic(schematic, schematic_path)
-                    return {"success": True, "reference": reference, "angle": angle}
+
+                    # Apply mirror via direct S-expression manipulation
+                    # (kicad-skip has no API for setting mirror on a placed symbol)
+                    if mirror is not None:
+                        success = KiCADInterface._apply_mirror_to_symbol_sexp(
+                            schematic_path, reference, mirror
+                        )
+                        if not success:
+                            return {
+                                "success": False,
+                                "message": f"Rotation applied but mirror failed for '{reference}'",
+                            }
+
+                    return {
+                        "success": True,
+                        "reference": reference,
+                        "angle": angle,
+                        "mirror": mirror,
+                    }
 
             return {"success": False, "message": f"Component {reference} not found"}
 

tests/test_rotate_schematic_mirror.py

@@ -0,0 +1,171 @@
+"""
+Unit tests for rotate_schematic_component mirror fix.
+
+Verifies that _apply_mirror_to_symbol_sexp correctly adds, removes
+and toggles (mirror x/y) in .kicad_sch S-expression files.
+"""
+
+import os
+import sys
+import importlib.util
+import tempfile
+import textwrap
+from unittest.mock import patch, MagicMock
+
+# Stub KiCad-only and server-only modules so kicad_interface can be
+# imported in a plain Python environment.
+_pcbnew_mock = MagicMock()
+_pcbnew_mock.__file__ = "/fake/pcbnew.so"       # prevents AttributeError at module level
+_pcbnew_mock.GetBuildVersion.return_value = "9.0.0"
+sys.modules.setdefault("pcbnew", _pcbnew_mock)
+
+for _modname in ("skip", "resources", "schemas",
+                 "resources.resource_definitions", "schemas.tool_schemas"):
+    sys.modules.setdefault(_modname, MagicMock())
+
+sys.modules["resources.resource_definitions"].RESOURCE_DEFINITIONS = {}
+sys.modules["resources.resource_definitions"].handle_resource_read = MagicMock()
+sys.modules["schemas.tool_schemas"].TOOL_SCHEMAS = []
+
+# Import KiCADInterface directly, bypassing package __init__ chains
+_spec = importlib.util.spec_from_file_location(
+    "kicad_interface",
+    os.path.join(os.path.dirname(__file__), "..", "python", "kicad_interface.py"),
+)
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
+KiCADInterface = _mod.KiCADInterface
+SchematicManager = _mod.SchematicManager
+
+# ---------------------------------------------------------------------------
+# Minimal .kicad_sch snippets
+# ---------------------------------------------------------------------------
+
+SCHEMATIC_TEMPLATE = textwrap.dedent("""\
+    (kicad_sch (version 20250114) (generator "test")
+      (symbol (lib_id "Transistor_BJT:MMBT3904")
+              (at 75 105 0)
+              (unit 1)
+              (property "Reference" "Q1" (at 0 0 0))
+              (property "Value" "MMBT3904" (at 0 0 0))
+      )
+    )
+""")
+
+SCHEMATIC_TEMPLATE_MIRRORED_X = textwrap.dedent("""\
+    (kicad_sch (version 20250114) (generator "test")
+      (symbol (lib_id "Transistor_BJT:MMBT3904")
+              (at 75 105 0) (mirror x)
+              (unit 1)
+              (property "Reference" "Q1" (at 0 0 0))
+              (property "Value" "MMBT3904" (at 0 0 0))
+      )
+    )
+""")
+
+
+def _write_temp_sch(content: str) -> str:
+    fd, path = tempfile.mkstemp(suffix=".kicad_sch")
+    os.close(fd)
+    with open(path, "w", encoding="utf-8") as f:
+        f.write(content)
+    return path
+
+
+def _read(path: str) -> str:
+    with open(path, encoding="utf-8") as f:
+        return f.read()
+
+
+# ---------------------------------------------------------------------------
+# Tests for _apply_mirror_to_symbol_sexp
+# ---------------------------------------------------------------------------
+
+def test_apply_mirror_x_adds_token():
+    """mirror='x' should insert (mirror x) after the (at ...) token."""
+    path = _write_temp_sch(SCHEMATIC_TEMPLATE)
+    try:
+        result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "Q1", "x")
+        assert result is True
+        content = _read(path)
+        assert "(mirror x)" in content
+        assert "(mirror y)" not in content
+    finally:
+        os.unlink(path)
+
+
+def test_apply_mirror_y_adds_token():
+    """mirror='y' should insert (mirror y) after the (at ...) token."""
+    path = _write_temp_sch(SCHEMATIC_TEMPLATE)
+    try:
+        result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "Q1", "y")
+        assert result is True
+        content = _read(path)
+        assert "(mirror y)" in content
+        assert "(mirror x)" not in content
+    finally:
+        os.unlink(path)
+
+
+def test_apply_mirror_none_removes_existing():
+    """mirror=None should remove an existing (mirror x) token."""
+    path = _write_temp_sch(SCHEMATIC_TEMPLATE_MIRRORED_X)
+    try:
+        result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "Q1", None)
+        assert result is True
+        content = _read(path)
+        assert "(mirror x)" not in content
+        assert "(mirror y)" not in content
+    finally:
+        os.unlink(path)
+
+
+def test_apply_mirror_x_replaces_existing_y():
+    """mirror='x' should replace an existing (mirror y) without duplication."""
+    path = _write_temp_sch(SCHEMATIC_TEMPLATE_MIRRORED_X.replace("mirror x", "mirror y"))
+    try:
+        result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "Q1", "x")
+        assert result is True
+        content = _read(path)
+        assert content.count("(mirror x)") == 1
+        assert "(mirror y)" not in content
+    finally:
+        os.unlink(path)
+
+
+def test_apply_mirror_returns_false_for_unknown_reference():
+    """Should return False gracefully when reference not found."""
+    path = _write_temp_sch(SCHEMATIC_TEMPLATE)
+    try:
+        result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "U99", "x")
+        assert result is False
+        assert "(mirror x)" not in _read(path)
+    finally:
+        os.unlink(path)
+
+
+def test_rotate_handler_no_mirror_still_works(tmp_path):
+    """rotate_schematic_component without mirror param should not crash."""
+    sch_path = str(tmp_path / "test.kicad_sch")
+    with open(sch_path, "w") as f:
+        f.write(SCHEMATIC_TEMPLATE)
+
+    iface = KiCADInterface.__new__(KiCADInterface)
+
+    with patch.object(_mod.SchematicManager, "load_schematic") as mock_load, \
+         patch.object(_mod.SchematicManager, "save_schematic"):
+        mock_sym = MagicMock()
+        mock_sym.property.Reference.value = "Q1"
+        mock_sym.at.value = [75, 105, 0]
+        mock_sch = MagicMock()
+        mock_sch.symbol = [mock_sym]
+        mock_load.return_value = mock_sch
+
+        result = iface._handle_rotate_schematic_component({
+            "schematicPath": sch_path,
+            "reference": "Q1",
+            "angle": 90,
+        })
+
+    assert result["success"] is True
+    assert result["angle"] == 90