Merge pull request #196 from gcolonese/feat-schematic-justify

feat(schematic): expose justify directive on field labels via MCP

package-lock.json

@@ -32,7 +32,8 @@
       "resolved": "https://registry.npmjs.org/@cfworker/json-schema/-/json-schema-4.1.1.tgz",
       "integrity": "sha512-gAmrUZSGtKc3AiBL71iNWxDsyUC5uMaKKGdvzYsBoTW/xi42JQHl7eKV2OYzCUqvc+D2RCcf7EXY2iCyFIk6og==",
       "devOptional": true,
-      "license": "MIT"
+      "license": "MIT",
+      "peer": true
     },
     "node_modules/@eslint-community/eslint-utils": {
       "version": "4.9.1",
@@ -496,6 +497,7 @@
       "integrity": "sha512-30ScMRHIAD33JJQkgfGW1t8CURZtjc2JpTrq5n2HFhOefbAhb7ucc7xJwdWcrEtqUIYJ73Nybpsggii6GtAHjA==",
       "dev": true,
       "license": "MIT",
+      "peer": true,
       "dependencies": {
         "@typescript-eslint/scope-manager": "8.57.2",
         "@typescript-eslint/types": "8.57.2",
@@ -739,6 +741,7 @@
       "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==",
       "dev": true,
       "license": "MIT",
+      "peer": true,
       "bin": {
         "acorn": "bin/acorn"
       },
@@ -1136,6 +1139,7 @@
       "integrity": "sha512-S9jlY/ELKEUwwQnqWDO+f+m6sercqOPSqXM5Go94l7DOmxHVDgmSFGWEzeE/gwgTAr0W103BWt0QLe/7mabIvA==",
       "dev": true,
       "license": "MIT",
+      "peer": true,
       "dependencies": {
         "@eslint-community/eslint-utils": "^4.8.0",
         "@eslint-community/regexpp": "^4.12.2",
@@ -1393,6 +1397,7 @@
       "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz",
       "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==",
       "license": "MIT",
+      "peer": true,
       "dependencies": {
         "accepts": "^2.0.0",
         "body-parser": "^2.2.1",
@@ -1709,6 +1714,7 @@
       "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.23.tgz",
       "integrity": "sha512-eIaZ9qDgu7XV0pxOCrg7/WhnQ6Ivm22UcxhXx/A3dcbqbbYgBEkc6e/J/s7j2tS96zoB0S9VBdLwQNCWwUo4LA==",
       "license": "MIT",
+      "peer": true,
       "engines": {
         "node": ">=16.9.0"
       }
@@ -2594,6 +2600,7 @@
       "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==",
       "dev": true,
       "license": "MIT",
+      "peer": true,
       "engines": {
         "node": ">=12"
       },
@@ -2679,6 +2686,7 @@
       "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==",
       "dev": true,
       "license": "Apache-2.0",
+      "peer": true,
       "bin": {
         "tsc": "bin/tsc",
         "tsserver": "bin/tsserver"
@@ -2802,6 +2810,7 @@
       "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz",
       "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==",
       "license": "MIT",
+      "peer": true,
       "funding": {
         "url": "https://github.com/sponsors/colinhacks"
       }

python/kicad_interface.py

@@ -1616,7 +1616,10 @@ class KiCADInterface:
         Args:
             block: The full text of the (symbol ...) block.
             name: Property name (e.g. "MPN", "Manufacturer").
-            spec: Dict that may contain keys: value, x, y, angle, hide, fontSize.
+            spec: Dict that may contain keys: value, x, y, angle, hide, fontSize,
+                justify.  ``justify`` is a space-separated string of KiCad
+                alignment keywords (e.g. "left", "right top"). "center" removes
+                the directive (KiCad default).
             default_position: (x, y) of the parent symbol — used as the default
                 location for newly-created properties so the field is anchored
                 near the component, not at (0, 0).
@@ -1631,6 +1634,7 @@ class KiCADInterface:
         new_y = spec.get("y")
         new_angle = spec.get("angle")
         new_hide = spec.get("hide")
+        new_justify = spec.get("justify")
         font_size = spec.get("fontSize", 1.27)
 
         existing_match = re.search(
@@ -1671,6 +1675,9 @@ class KiCADInterface:
             if new_hide is not None:
                 block = self._set_hide_on_property(block, name, bool(new_hide))
 
+            if new_justify is not None:
+                block = self._set_justify_on_property(block, name, str(new_justify))
+
             return block, "updated"
 
         # Property does not exist — append a new one after the last existing property
@@ -1689,9 +1696,16 @@ class KiCADInterface:
         escaped = self._escape_sexpr_string(str(new_value))
         escaped_name = self._escape_sexpr_string(str(name))
 
+        # Build optional (justify ...) token for the effects block.
+        justify_str = ""
+        if new_justify is not None:
+            tokens = str(new_justify).strip().split()
+            if not all(t == "center" for t in tokens):
+                justify_str = f" (justify {str(new_justify).strip()})"
+
         new_prop = (
             f'    (property "{escaped_name}" "{escaped}" (at {cx} {cy} {ca})\n'
-            f"      (effects (font (size {font_size} {font_size})) {hide_str})\n"
+            f"      (effects (font (size {font_size} {font_size})){justify_str} {hide_str})\n"
             f"    )"
         )
 
@@ -1752,6 +1766,53 @@ class KiCADInterface:
         new_effects = "(" + eff_inner + ")"
         return block[:eff_start] + new_effects + block[eff_end + 1 :]
 
+    def _set_justify_on_property(self, block: str, name: str, justify: str) -> str:
+        """Set or clear the (justify ...) directive on a named property's effects clause.
+
+        ``justify`` is a space-separated string of KiCad alignment keywords:
+            horizontal: "left" | "right" | "center"
+            vertical:   "top"  | "bottom" | "center"
+        Any combination of one or two tokens is accepted, e.g. "left", "right top".
+        Passing "center" (the KiCad default) removes the (justify ...) directive
+        entirely, which is how KiCad represents centered alignment.
+
+        Handles effects clauses that already contain a (justify ...) token, and
+        those that do not.
+        """
+        import re
+
+        prop_match = re.search(
+            r'\(property\s+"' + re.escape(name) + r'"',
+            block,
+        )
+        if not prop_match:
+            return block
+        prop_start = prop_match.start()
+        prop_end = self._find_matching_paren(block, prop_start)
+        if prop_end < 0:
+            return block
+
+        prop_segment = block[prop_start : prop_end + 1]
+        eff_match = re.search(r"\(effects\b", prop_segment)
+        if not eff_match:
+            return block
+        eff_start = prop_start + eff_match.start()
+        eff_end = self._find_matching_paren(block, eff_start)
+        if eff_end < 0:
+            return block
+
+        eff_inner = block[eff_start + 1 : eff_end]  # 'effects (font ...) ...'
+        # Remove any pre-existing (justify ...) token
+        eff_inner = re.sub(r"\s*\(justify\b[^)]*\)", "", eff_inner)
+        # "center" is the KiCad default — omitting the directive means centered
+        tokens = justify.strip().split()
+        is_center_only = all(t == "center" for t in tokens)
+        if not is_center_only:
+            eff_inner = eff_inner.rstrip() + f" (justify {justify.strip()})"
+
+        new_effects = "(" + eff_inner + ")"
+        return block[:eff_start] + new_effects + block[eff_end + 1 :]
+
     def _remove_property_from_block(self, block: str, name: str) -> Tuple[str, bool]:
         """Remove a property from the symbol block. Returns (new_block, removed_bool)."""
         import re
@@ -1962,6 +2023,11 @@ class KiCADInterface:
                         rf"\1(at {x} {y} {angle})",
                         block_text,
                     )
+                    justify = pos.get("justify")
+                    if justify is not None:
+                        block_text = self._set_justify_on_property(
+                            block_text, field_name, str(justify)
+                        )
 
             properties_added: Dict[str, Any] = {}
             properties_updated: Dict[str, Any] = {}
@@ -2050,7 +2116,7 @@ class KiCADInterface:
             return {"success": False, "message": "value is required"}
 
         spec: Dict[str, Any] = {"value": params["value"]}
-        for key in ("x", "y", "angle", "hide", "fontSize"):
+        for key in ("x", "y", "angle", "hide", "fontSize", "justify"):
             if params.get(key) is not None:
                 spec[key] = params[key]
 
@@ -6072,7 +6138,7 @@ print("ok")
             # Create circle on Edge.Cuts layer for the hole
             circle = BoardCircle()
             circle.center = Vector2.from_xy(from_mm(x), from_mm(y))
-            circle.radius = from_mm(diameter / 2)
+            circle.radius = from_mm(diameter / 2)  # type: ignore[assignment,method-assign]
             circle.layer = BoardLayer.BL_Edge_Cuts
             circle.attributes.stroke.width = from_mm(0.1)
 

src/tools/schematic.ts

@@ -200,11 +200,21 @@ use edit_component instead.`,
             x: z.number(),
             y: z.number(),
             angle: z.number().optional().default(0),
+            justify: z
+              .union([z.string(), z.array(z.string())])
+              .optional()
+              .describe(
+                'Text justification: "left", "right", "center", "top", "bottom", or combined ' +
+                  '"left top" / "right bottom". Array form ["left", "top"] is also accepted. ' +
+                  'Omit to leave the existing justify unchanged. Pass "center" to reset to ' +
+                  "the KiCad default (removes the justify directive).",
+              ),
           }),
         )
         .optional()
         .describe(
-          'Reposition field labels: map of field name to {x, y, angle} (e.g. {"Reference": {"x": 12.5, "y": 17.0}})',
+          "Reposition field labels: map of field name to {x, y, angle, justify?} " +
+            '(e.g. {"Reference": {"x": 12.5, "y": 17.0, "justify": "left"}})',
         ),
       properties: z
         .record(
@@ -252,7 +262,10 @@ use edit_component instead.`,
       footprint?: string;
       value?: string;
       newReference?: string;
-      fieldPositions?: Record<string, { x: number; y: number; angle?: number }>;
+      fieldPositions?: Record<
+        string,
+        { x: number; y: number; angle?: number; justify?: string | string[] }
+      >;
       properties?: Record<
         string,
         | string
@@ -267,6 +280,16 @@ use edit_component instead.`,
       >;
       removeProperties?: string[];
     }) => {
+      // Normalise array-form justify (["left", "top"]) to the space-separated
+      // string form ("left top") that the Python backend expects.
+      if (args.fieldPositions) {
+        for (const fieldName of Object.keys(args.fieldPositions)) {
+          const pos = args.fieldPositions[fieldName];
+          if (Array.isArray(pos.justify)) {
+            pos.justify = (pos.justify as string[]).join(" ");
+          }
+        }
+      }
       const result = await callKicadScript("edit_schematic_component", args);
       if (result.success) {
         const updated = result.updated ?? {};
@@ -349,6 +372,14 @@ with the \`properties\` parameter instead.`,
           "Hide the property text on the schematic canvas. Defaults to true for newly-created custom properties.",
         ),
       fontSize: z.number().optional().describe("Font size in mm for the label (default: 1.27)"),
+      justify: z
+        .string()
+        .optional()
+        .describe(
+          'Text justification for the property label. KiCad alignment keywords: "left", "right", ' +
+            '"center", "top", "bottom", or combined e.g. "left top". Omit to leave unchanged. ' +
+            'Pass "center" to reset to the KiCad default.',
+        ),
     },
     async (args: {
       schematicPath: string;
@@ -360,6 +391,7 @@ with the \`properties\` parameter instead.`,
       angle?: number;
       hide?: boolean;
       fontSize?: number;
+      justify?: string;
     }) => {
       const result = await callKicadScript("set_schematic_component_property", args);
       if (result.success) {

tests/test_schematic_field_justify.py

@@ -0,0 +1,295 @@
+"""
+Tests for the justify directive support in edit_schematic_component fieldPositions
+and set_schematic_component_property.
+
+Covers:
+  - _set_justify_on_property() helper (unit tests, no file I/O)
+  - fieldPositions[field].justify via edit_schematic_component (integration)
+  - justify forwarding via set_schematic_component_property (integration)
+  - Backward-compat: existing calls without justify must be unaffected
+"""
+
+import re
+import sys
+from pathlib import Path
+from typing import Any
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).parent.parent / "python"))
+
+TEMPLATE_SCH = Path(__file__).parent.parent / "python" / "templates" / "empty.kicad_sch"
+
+# Minimal placed-symbol block used across integration tests
+PLACED_RESISTOR_BLOCK = """\
+  (symbol (lib_id "Device:R") (at 50 50 0) (unit 1)
+    (in_bom yes) (on_board yes) (dnp no)
+    (uuid "aaaaaaaa-bbbb-cccc-dddd-ffffffffffff")
+    (property "Reference" "R2" (at 51.27 47.46 0)
+      (effects (font (size 1.27 1.27)))
+    )
+    (property "Value" "4k7" (at 51.27 52.54 0)
+      (effects (font (size 1.27 1.27)))
+    )
+    (property "Footprint" "Resistor_SMD:R_0603_1608Metric" (at 50 50 0)
+      (effects (font (size 1.27 1.27)) hide)
+    )
+    (property "Datasheet" "~" (at 50 50 0)
+      (effects (font (size 1.27 1.27)) hide)
+    )
+  )
+"""
+
+
+def _make_test_schematic(tmp_dir: Path, extra_block: str = "") -> Path:
+    dest = tmp_dir / "justify_test.kicad_sch"
+    src_content = TEMPLATE_SCH.read_text(encoding="utf-8")
+    if extra_block:
+        src_content = src_content.rstrip()
+        if src_content.endswith(")"):
+            src_content = src_content[:-1] + "\n" + extra_block + ")\n"
+    dest.write_text(src_content, encoding="utf-8")
+    return dest
+
+
+# ---------------------------------------------------------------------------
+# Unit tests — exercise _set_justify_on_property() directly, no file I/O
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.unit
+class TestSetJustifyOnPropertyHelper:
+    """Unit tests for the _set_justify_on_property helper."""
+
+    def _get_interface(self) -> Any:
+        from kicad_interface import KiCADInterface
+
+        return KiCADInterface()
+
+    def test_adds_justify_left(self) -> None:
+        iface = self._get_interface()
+        block = PLACED_RESISTOR_BLOCK
+        result = iface._set_justify_on_property(block, "Reference", "left")
+        assert "(justify left)" in result
+
+    def test_adds_justify_right(self) -> None:
+        iface = self._get_interface()
+        block = PLACED_RESISTOR_BLOCK
+        result = iface._set_justify_on_property(block, "Reference", "right")
+        assert "(justify right)" in result
+
+    def test_adds_combined_justify(self) -> None:
+        iface = self._get_interface()
+        block = PLACED_RESISTOR_BLOCK
+        result = iface._set_justify_on_property(block, "Reference", "right top")
+        assert "(justify right top)" in result
+
+    def test_center_removes_directive(self) -> None:
+        """Passing 'center' should remove any (justify ...) token (KiCad default)."""
+        iface = self._get_interface()
+        block = PLACED_RESISTOR_BLOCK
+        # First set a non-center justify so there is something to remove
+        block = iface._set_justify_on_property(block, "Reference", "left")
+        assert "(justify left)" in block
+        # Now reset to center
+        block = iface._set_justify_on_property(block, "Reference", "center")
+        assert "(justify" not in block
+
+    def test_replaces_existing_justify(self) -> None:
+        iface = self._get_interface()
+        block = PLACED_RESISTOR_BLOCK
+        block = iface._set_justify_on_property(block, "Reference", "left")
+        block = iface._set_justify_on_property(block, "Reference", "right")
+        # Should have exactly one justify token and it should be 'right'
+        matches = re.findall(r"\(justify\s+[^)]+\)", block)
+        assert len(matches) == 1
+        assert "right" in matches[0]
+
+    def test_does_not_affect_other_fields(self) -> None:
+        iface = self._get_interface()
+        block = PLACED_RESISTOR_BLOCK
+        result = iface._set_justify_on_property(block, "Reference", "left")
+        # Value field must not have acquired a justify token
+        value_section = result[result.find('(property "Value"') :]
+        ref_section = result[result.find('(property "Reference"') :]
+        # The justify token should appear before the Value property section
+        justify_pos = result.find("(justify left)")
+        value_prop_pos = result.find('(property "Value"')
+        assert justify_pos < value_prop_pos
+
+    def test_unknown_field_returns_block_unchanged(self) -> None:
+        iface = self._get_interface()
+        block = PLACED_RESISTOR_BLOCK
+        result = iface._set_justify_on_property(block, "NoSuchField", "left")
+        assert result == block
+
+
+# ---------------------------------------------------------------------------
+# Integration tests — real file I/O through edit_schematic_component
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.integration
+class TestFieldPositionsJustify:
+    """Integration tests for justify via fieldPositions in edit_schematic_component."""
+
+    @pytest.fixture
+    def sch_with_r2(self, tmp_path: Any) -> Any:
+        return _make_test_schematic(tmp_path, PLACED_RESISTOR_BLOCK)
+
+    def _get_interface(self) -> Any:
+        from kicad_interface import KiCADInterface
+
+        return KiCADInterface()
+
+    def test_set_justify_left_on_reference(self, sch_with_r2: Any) -> None:
+        iface = self._get_interface()
+        result = iface.handle_command(
+            "edit_schematic_component",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "fieldPositions": {"Reference": {"x": 51.27, "y": 47.46, "justify": "left"}},
+            },
+        )
+        assert result["success"] is True
+        content = sch_with_r2.read_text(encoding="utf-8")
+        assert "(justify left)" in content
+
+    def test_set_justify_right_on_value(self, sch_with_r2: Any) -> None:
+        iface = self._get_interface()
+        result = iface.handle_command(
+            "edit_schematic_component",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "fieldPositions": {"Value": {"x": 51.27, "y": 52.54, "justify": "right"}},
+            },
+        )
+        assert result["success"] is True
+        content = sch_with_r2.read_text(encoding="utf-8")
+        assert "(justify right)" in content
+
+    def test_set_justify_center_removes_directive(self, sch_with_r2: Any) -> None:
+        iface = self._get_interface()
+        # First add a left justify so there is something to remove
+        iface.handle_command(
+            "edit_schematic_component",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "fieldPositions": {"Reference": {"x": 51.27, "y": 47.46, "justify": "left"}},
+            },
+        )
+        # Now reset to center (default)
+        result = iface.handle_command(
+            "edit_schematic_component",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "fieldPositions": {"Reference": {"x": 51.27, "y": 47.46, "justify": "center"}},
+            },
+        )
+        assert result["success"] is True
+        content = sch_with_r2.read_text(encoding="utf-8")
+        # Check that the Reference property block itself has no (justify ...) token.
+        # (The lib_symbols section may legitimately contain justify directives.)
+        ref_prop_start = content.find('(property "Reference" "R2"')
+        assert ref_prop_start >= 0, "Reference property block not found"
+        ref_prop_end = content.find("\n    )", ref_prop_start) + len("\n    )")
+        ref_prop_block = content[ref_prop_start:ref_prop_end]
+        assert "(justify" not in ref_prop_block
+
+    def test_fieldpositions_without_justify_unchanged(self, sch_with_r2: Any) -> None:
+        """Omitting justify must not alter any existing justify on the field."""
+        iface = self._get_interface()
+        # Set a justify first
+        iface.handle_command(
+            "edit_schematic_component",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "fieldPositions": {"Reference": {"x": 51.27, "y": 47.46, "justify": "left"}},
+            },
+        )
+        # Reposition without passing justify — the left directive must survive
+        iface.handle_command(
+            "edit_schematic_component",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "fieldPositions": {"Reference": {"x": 60.0, "y": 47.46}},
+            },
+        )
+        content = sch_with_r2.read_text(encoding="utf-8")
+        assert "(justify left)" in content
+
+    def test_position_and_justify_applied_together(self, sch_with_r2: Any) -> None:
+        """Both position and justify should be applied in a single call."""
+        iface = self._get_interface()
+        result = iface.handle_command(
+            "edit_schematic_component",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "fieldPositions": {"Reference": {"x": 99.0, "y": 88.0, "justify": "right"}},
+            },
+        )
+        assert result["success"] is True
+        content = sch_with_r2.read_text(encoding="utf-8")
+        assert "(justify right)" in content
+        assert "99.0 88.0" in content
+
+
+# ---------------------------------------------------------------------------
+# Integration tests — justify via set_schematic_component_property
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.integration
+class TestSetPropertyJustify:
+    """Integration tests for justify via set_schematic_component_property."""
+
+    @pytest.fixture
+    def sch_with_r2(self, tmp_path: Any) -> Any:
+        return _make_test_schematic(tmp_path, PLACED_RESISTOR_BLOCK)
+
+    def _get_interface(self) -> Any:
+        from kicad_interface import KiCADInterface
+
+        return KiCADInterface()
+
+    def test_set_property_with_justify(self, sch_with_r2: Any) -> None:
+        iface = self._get_interface()
+        result = iface.handle_command(
+            "set_schematic_component_property",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "name": "Tolerance",
+                "value": "1%",
+                "justify": "left",
+            },
+        )
+        assert result["success"] is True
+        content = sch_with_r2.read_text(encoding="utf-8")
+        assert "(justify left)" in content
+
+    def test_set_property_without_justify_no_regression(self, sch_with_r2: Any) -> None:
+        """Omitting justify on set_property must not add any justify token."""
+        iface = self._get_interface()
+        result = iface.handle_command(
+            "set_schematic_component_property",
+            {
+                "schematicPath": str(sch_with_r2),
+                "reference": "R2",
+                "name": "Rating",
+                "value": "100mW",
+            },
+        )
+        assert result["success"] is True
+        content = sch_with_r2.read_text(encoding="utf-8")
+        # No justify should appear in the newly added Rating property
+        rating_start = content.find('(property "Rating"')
+        rating_end = content.find(")", rating_start)
+        assert "(justify" not in content[rating_start:rating_end]