Port pin safety fixes from v0.4.1 to v0.5-dev (issue #432)

Add normalize_pin() for consistent int/str coercion at YAML boundary,
resolve_pin() for label-to-number resolution, and fix GraphViz port
index rendering for loops and shorts on non-sequential connectors.

Key changes:
- wv_utils: add normalize_pin(), reject floats/bools, refactor expand()
- wv_dataclasses: normalize pins/pinlabels/wirelabels in __post_init__,
  add Connector.resolve_pin(), resolve loop/short pins through labels,
  detect duplicate pins and minimum pin count in loops/shorts
- wv_graphviz: convert pin IDs to position indices in loop/short edges,
  fix short membership test from pin.index to pin.id
- wv_harness: delegate connect() pin resolution to resolve_pin()
- tests: 50 tests covering resolution, coercion, rendering, validation

src/wireviz/wv_dataclasses.py

@@ -25,6 +25,7 @@ from wireviz.wv_utils import (
     aspect_ratio,
     awg_equiv,
     mm2_equiv,
+    normalize_pin,
     parse_number_and_unit,
     remove_links,
 )
@@ -409,6 +410,13 @@ class Connector(TopLevelGraphicalComponent):
         elif self.strip is None:
             self.strip = Strip()
 
+        # Normalize pin-like fields so int/str types are consistent
+        # regardless of YAML quoting (e.g. "1" vs 1).
+        if self.pins:
+            self.pins = [normalize_pin(p) for p in self.pins]
+        if self.pinlabels:
+            self.pinlabels = [normalize_pin(p) for p in self.pinlabels]
+
         self.ports_left = False
         self.ports_right = False
         self.visible_pins = {}
@@ -478,41 +486,43 @@ class Connector(TopLevelGraphicalComponent):
                 loDict[key] = self.loops[loIndex]
             self.loops = loDict
 
-        # TODO: allow using pin labels in addition to pin numbers,
-        #       just like when defining regular connections
-        # TODO: include properties of wire used to create the loop
-        for loopName in self.loops:
-            for pin in self.loops[loopName]:
-                if pin not in self.pins:
+        # Resolve loop pins: normalize, resolve labels, validate
+        for loopName, loPins in self.loops.items():
+            loPins = [normalize_pin(p) for p in loPins]
+            resolved = [self.resolve_pin(p) for p in loPins]
+            if len(resolved) < 2:
                 raise Exception(
-                        f'Unknown loop pin "{pin}" for connector "{self.designator}"!'
+                    f'Loop "{loopName}" in connector "{self.designator}" '
+                    f'must connect at least 2 pins (got {len(resolved)}).'
                 )
-                # Make sure loop connected pins are not hidden.
-                self.activate_pin(pin, None)
-        for short in self.shorts:
-            for pin in self.shorts[short]:
-                if pin not in self.pins:
+            if len(resolved) != len(set(resolved)):
+                duplicates = [p for p in set(resolved) if resolved.count(p) > 1]
                 raise Exception(
-                        f'Unknown loop pin "{pin}" for connector "{self.designator}"!'
+                    f'Loop "{loopName}" in connector "{self.designator}" '
+                    f'contains duplicate pin(s): {duplicates}.'
                 )
-                # Make sure loop connected pins are not hidden.
-                self.activate_pin(pin, None)
+            for pin in resolved:
+                self.activate_pin(pin, None, is_connection=False)
+            self.loops[loopName] = resolved
 
-        # TODO: Remove the outcommented code here if it is no longer needed as reference
-        # for loop in self.loops:
-        #     # TODO: allow using pin labels in addition to pin numbers,
-        #     #       just like when defining regular connections
-        #     # TODO: include properties of wire used to create the loop
-        #     if len(loop) != 2:
-        #         raise Exception("Loops must be between exactly two pins!")
-        #     for pin in loop:
-        #         if pin not in self.pins:
-        #             raise Exception(
-        #                 f'Unknown loop pin "{pin}" for connector "{self.name}"!'
-        #             )
-        #         # Make sure loop connected pins are not hidden.
-        #         # side=None, determine side to show loops during rendering
-        #         self.activate_pin(pin, side=None, is_connection=True)
+        # Resolve short pins: normalize, resolve labels, validate
+        for shortName, shPins in self.shorts.items():
+            shPins = [normalize_pin(p) for p in shPins]
+            resolved = [self.resolve_pin(p) for p in shPins]
+            if len(resolved) < 2:
+                raise Exception(
+                    f'Short "{shortName}" in connector "{self.designator}" '
+                    f'must connect at least 2 pins (got {len(resolved)}).'
+                )
+            if len(resolved) != len(set(resolved)):
+                duplicates = [p for p in set(resolved) if resolved.count(p) > 1]
+                raise Exception(
+                    f'Short "{shortName}" in connector "{self.designator}" '
+                    f'contains duplicate pin(s): {duplicates}.'
+                )
+            for pin in resolved:
+                self.activate_pin(pin, None, is_connection=False)
+            self.shorts[shortName] = resolved
 
         for i, item in enumerate(self.additional_components):
             if isinstance(item, dict):
@@ -526,6 +536,52 @@ class Connector(TopLevelGraphicalComponent):
         elif side == Side.RIGHT:
             self.ports_right = True
 
+    def resolve_pin(self, pin: Pin) -> Pin:
+        """Resolve a pin identifier to its canonical pin number.
+
+        Given a value that may be either a pin number (from self.pins)
+        or a pin label (from self.pinlabels), returns the corresponding
+        pin number from self.pins.
+
+        Resolution order:
+            1. Value in both pins and pinlabels at the same position -> return directly.
+            2. Value in both at different positions -> raise.
+            3. Value only in pinlabels -> return corresponding pin number.
+            4. Value only in pins -> return directly.
+            5. Not found -> raise.
+        """
+        pin = normalize_pin(pin)
+        in_pins = pin in self.pins
+        in_labels = pin in self.pinlabels if self.pinlabels else False
+
+        if in_pins and in_labels:
+            if self.pinlabels.count(pin) > 1:
+                raise Exception(
+                    f'Pin label "{pin}" in connector "{self.designator}" '
+                    f"is defined more than once in pinlabels."
+                )
+            if self.pins.index(pin) != self.pinlabels.index(pin):
+                raise Exception(
+                    f'"{pin}" in connector "{self.designator}" exists in both '
+                    f"pins and pinlabels at different positions."
+                )
+            return pin
+
+        if in_labels:
+            if self.pinlabels.count(pin) > 1:
+                raise Exception(
+                    f'Pin label "{pin}" in connector "{self.designator}" '
+                    f"is defined more than once."
+                )
+            return self.pins[self.pinlabels.index(pin)]
+
+        if in_pins:
+            return pin
+
+        raise Exception(
+            f'Unknown pin "{pin}" for connector "{self.designator}"!'
+        )
+
     def compute_qty_multipliers(self):
         # do not run before all connections in harness have been made!
         num_populated_pins = len(
@@ -777,6 +833,7 @@ class Cable(TopLevelGraphicalComponent):
             self.wirecount = len(self.colors)
 
         if self.wirelabels:
+            self.wirelabels = [normalize_pin(w) for w in self.wirelabels]
             if self.shield and "s" in self.wirelabels:
                 raise Exception(
                     '"s" may not be used as a wire label for a shielded cable.'

src/wireviz/wv_graphviz.py

@@ -305,7 +305,7 @@ def gv_pin_table(component) -> Table:
 def gv_short_row_part(pin, connector) -> List:
     short_row = [] # Td("ADA"), Td("DAD")
     for short, shPins in connector.shorts.items():
-        if pin.index + 1 in shPins:
+        if pin.id in shPins:
             short_row.append(Td("", port=f"p{pin.index+1}j"))
         else:
             short_row.append(Td(""))
@@ -344,8 +344,13 @@ def gv_connector_loops(connector: Connector) -> List:
             loColor = comp.color.html
 
         for i in range(1, len(loPins)):
-            head = f"{connector.designator}:p{loPins[i - 1]}{loop_side}:{loop_dir}"
-            tail = f"{connector.designator}:p{loPins[i]}{loop_side}:{loop_dir}"
+            # Convert pin IDs to 1-based port indices.
+            # Ports are named p{index+1} in the connector table,
+            # not p{pin_id} — these differ for non-sequential pins.
+            idx_prev = connector.pin_objects[loPins[i - 1]].index + 1
+            idx_curr = connector.pin_objects[loPins[i]].index + 1
+            head = f"{connector.designator}:p{idx_prev}{loop_side}:{loop_dir}"
+            tail = f"{connector.designator}:p{idx_curr}{loop_side}:{loop_dir}"
             loop_edges.append((head, tail, loColor))
     return loop_edges
 
@@ -360,8 +365,11 @@ def gv_connector_shorts(connector: Connector) -> List:
             shColor = f"#FFFFFF:{comp.color.html}:#FFFFFF"
 
         for i in range(1, len(shPins)):
-            head = f"{connector.designator}:p{shPins[i - 1]}j:c"
-            tail = f"{connector.designator}:p{shPins[i]}j:c"
+            # Convert pin IDs to 1-based port indices (same fix as loops).
+            idx_prev = connector.pin_objects[shPins[i - 1]].index + 1
+            idx_curr = connector.pin_objects[shPins[i]].index + 1
+            head = f"{connector.designator}:p{idx_prev}j:c"
+            tail = f"{connector.designator}:p{idx_curr}j:c"
             short_edges.append((head, tail, shColor))
     return short_edges
 

src/wireviz/wv_harness.py

@@ -236,30 +236,17 @@ class Harness:
         to_name: str,
         to_pin: Union[int, str],
     ) -> None:
-        # check from and to connectors
-        for name, pin in zip([from_name, to_name], [from_pin, to_pin]):
+        # resolve pin labels to pin numbers via Connector.resolve_pin()
+        for name, pin, is_from in [
+            (from_name, from_pin, True),
+            (to_name, to_pin, False),
+        ]:
             if name is not None and name in self.connectors:
-                connector = self.connectors[name]
-                # check if provided name is ambiguous
-                if pin in connector.pins and pin in connector.pinlabels:
-                    if connector.pins.index(pin) != connector.pinlabels.index(pin):
-                        raise Exception(
-                            f"{name}:{pin} is defined both in pinlabels and pins, "
-                            "for different pins."
-                        )
-                    # TODO: Maybe issue a warning if present in both lists
-                    # but referencing the same pin?
-                if pin in connector.pinlabels:
-                    if connector.pinlabels.count(pin) > 1:
-                        raise Exception(f"{name}:{pin} is defined more than once.")
-                    index = connector.pinlabels.index(pin)
-                    pin = connector.pins[index]  # map pin name to pin number
-                    if name == from_name:
-                        from_pin = pin
-                    if name == to_name:
-                        to_pin = pin
-                if not pin in connector.pins:
-                    raise Exception(f"{name}:{pin} not found.")
+                resolved = self.connectors[name].resolve_pin(pin)
+                if is_from:
+                    from_pin = resolved
+                else:
+                    to_pin = resolved
 
         # check via cable
         if via_name in self.cables:

src/wireviz/wv_utils.py

@@ -37,6 +37,25 @@ def mm2_equiv(awg):
     return mm2_equiv_table.get(str(awg), "Unknown")
 
 
+def normalize_pin(value):
+    """Normalize a pin value: try int() first, fall back to str().
+
+    Rejects bools and non-integer floats to prevent silent data loss.
+    Matches the coercion convention used by expand().
+    """
+    if isinstance(value, bool):
+        raise ValueError(f"Boolean {value!r} is not a valid pin identifier")
+    if isinstance(value, float) and not value.is_integer():
+        raise ValueError(
+            f"Float {value!r} is not a valid pin identifier "
+            f"(would be silently truncated to {int(value)})"
+        )
+    try:
+        return int(value)
+    except (ValueError, TypeError):
+        return str(value) if value is not None else value
+
+
 def expand(yaml_data):
     # yaml_data can be:
     # - a singleton (normally str or int)
@@ -60,15 +79,11 @@ def expand(yaml_data):
                         output.append(x)  # descending range
                 else:  # a == b
                     output.append(a)  # range of length 1
-            except:
+            except (ValueError, TypeError):
                 # '-' was not a delimiter between two ints, pass e through unchanged
                 output.append(e)
         else:
-            try:
-                x = int(e)  # single int
-            except Exception:
-                x = e  # string
-            output.append(x)
+            output.append(normalize_pin(e))
     return output
 
 

tests/test_resolve_pin.py

@@ -0,0 +1,472 @@
+# -*- coding: utf-8 -*-
+
+"""Tests for Connector.resolve_pin() and loop/short pin resolution.
+
+Port of the v0.4.1 test suite (issue #432) adapted for v0.5-dev architecture:
+- designator= instead of name=
+- loops/shorts are dicts (auto-keyed from lists)
+- pin_objects dict with PinClass instances
+- _num_connections instead of visible_pins
+- activate_pin() has is_connection parameter
+"""
+
+import pytest
+
+from wireviz.wv_dataclasses import Cable, Connector
+
+
+def make_connector(pins=None, pinlabels=None, loops=None, shorts=None, **kwargs):
+    """Helper to build a Connector with minimal required fields."""
+    args = {"designator": "X1"}
+    if pins is not None:
+        args["pins"] = pins
+    if pinlabels is not None:
+        args["pinlabels"] = pinlabels
+    if loops is not None:
+        args["loops"] = loops
+    if shorts is not None:
+        args["shorts"] = shorts
+    args.update(kwargs)
+    return Connector(**args)
+
+
+# --- resolve_pin() happy paths ---
+
+
+class TestResolvePinHappyPaths:
+    def test_pin_number_passthrough(self):
+        c = make_connector(pins=[1, 2, 3])
+        assert c.resolve_pin(2) == 2
+
+    def test_label_resolves_to_pin_number(self):
+        c = make_connector(pins=[1, 2, 3], pinlabels=["VCC", "GND", "SIG"])
+        assert c.resolve_pin("GND") == 2
+
+    def test_label_with_non_sequential_pins(self):
+        c = make_connector(pins=[10, 20, 30], pinlabels=["A", "B", "C"])
+        assert c.resolve_pin("B") == 20
+
+    def test_value_in_both_lists_same_position(self):
+        c = make_connector(pins=["A", "B", "C"], pinlabels=["A", "X", "Y"])
+        assert c.resolve_pin("A") == "A"
+
+    def test_empty_pinlabels_falls_through(self):
+        c = make_connector(pins=[1, 2, 3], pinlabels=[])
+        assert c.resolve_pin(3) == 3
+
+
+# --- resolve_pin() error paths ---
+
+
+class TestResolvePinErrors:
+    def test_unknown_pin_raises(self):
+        c = make_connector(pins=[1, 2, 3], pinlabels=["A", "B", "C"])
+        with pytest.raises(Exception, match="Unknown pin"):
+            c.resolve_pin("NONEXISTENT")
+
+    def test_unknown_number_raises(self):
+        c = make_connector(pins=[1, 2, 3])
+        with pytest.raises(Exception, match="Unknown pin"):
+            c.resolve_pin(99)
+
+    def test_ambiguous_pin_different_positions(self):
+        c = make_connector(pins=["A", "B", "C"], pinlabels=["X", "A", "Y"])
+        with pytest.raises(Exception, match="exists in both"):
+            c.resolve_pin("A")
+
+    def test_duplicate_label_only_in_labels(self):
+        c = make_connector(pins=[1, 2, 3], pinlabels=["A", "B", "A"])
+        with pytest.raises(Exception, match="defined more than once"):
+            c.resolve_pin("A")
+
+    def test_duplicate_label_in_both_lists(self):
+        c = make_connector(pins=["A", "B", "C"], pinlabels=["A", "X", "A"])
+        with pytest.raises(Exception, match="defined more than once"):
+            c.resolve_pin("A")
+
+
+# --- Loop resolution (dict format in v0.5) ---
+
+
+class TestLoopResolution:
+    def test_loop_with_labels(self):
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            loops=[["VCC", "GND"]],
+        )
+        # v0.5 auto-converts list to dict with AutoLO keys
+        assert list(c.loops.values())[0] == [1, 2]
+
+    def test_loop_with_mixed_number_and_label(self):
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            loops=[[1, "GND"]],
+        )
+        assert list(c.loops.values())[0] == [1, 2]
+
+    def test_loop_with_non_sequential_pins(self):
+        c = make_connector(
+            pins=[10, 20, 30, 40],
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            loops=[["VCC", "GND"]],
+        )
+        assert list(c.loops.values())[0] == [10, 20]
+
+    def test_loop_self_reference_raises(self):
+        with pytest.raises(Exception, match="duplicate pin"):
+            make_connector(
+                pins=[1, 2, 3],
+                pinlabels=["A", "B", "C"],
+                loops=[["A", "A"]],
+            )
+
+    def test_loop_self_reference_via_label_and_number(self):
+        with pytest.raises(Exception, match="duplicate pin"):
+            make_connector(
+                pins=[1, 2, 3],
+                pinlabels=["A", "B", "C"],
+                loops=[[2, "B"]],
+            )
+
+    def test_loop_activates_pins(self):
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            loops=[["TX", "RX"]],
+        )
+        # In v0.5, activation is tracked via _num_connections on pin_objects.
+        # Loops use is_connection=False, so _num_connections stays 0,
+        # but the pin should still be accessible (not hidden).
+        # The key check: pins are found in pin_objects (always true),
+        # and the loop was processed without error.
+        assert 3 in c.pin_objects
+        assert 4 in c.pin_objects
+
+    def test_multiple_loops(self):
+        c = make_connector(
+            pins=[1, 2, 3, 4, 5, 6],
+            pinlabels=["A", "B", "C", "D", "E", "F"],
+            loops=[["A", "B"], ["E", "F"]],
+        )
+        values = list(c.loops.values())
+        assert values[0] == [1, 2]
+        assert values[1] == [5, 6]
+
+    def test_loop_dict_format(self):
+        """Loops can be specified as dicts directly (named loops)."""
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            loops={"MyLoop": ["VCC", "GND"]},
+        )
+        assert c.loops["MyLoop"] == [1, 2]
+
+    def test_multi_pin_loop(self):
+        """v0.5 allows >2 pins per loop."""
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            pinlabels=["A", "B", "C", "D"],
+            loops=[["A", "B", "C"]],
+        )
+        assert list(c.loops.values())[0] == [1, 2, 3]
+
+
+# --- Short resolution (new in v0.5) ---
+
+
+class TestShortResolution:
+    def test_short_with_labels(self):
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            shorts=[["VCC", "GND"]],
+        )
+        assert list(c.shorts.values())[0] == [1, 2]
+
+    def test_short_with_non_sequential_pins(self):
+        c = make_connector(
+            pins=[10, 20, 30, 40],
+            pinlabels=["A", "B", "C", "D"],
+            shorts=[["A", "C"]],
+        )
+        assert list(c.shorts.values())[0] == [10, 30]
+
+    def test_short_self_reference_raises(self):
+        with pytest.raises(Exception, match="duplicate pin"):
+            make_connector(
+                pins=[1, 2, 3],
+                pinlabels=["A", "B", "C"],
+                shorts=[["A", "A"]],
+            )
+
+    def test_short_dict_format(self):
+        c = make_connector(
+            pins=[1, 2, 3],
+            pinlabels=["A", "B", "C"],
+            shorts={"MyShort": ["A", "C"]},
+        )
+        assert c.shorts["MyShort"] == [1, 3]
+
+    def test_multiple_shorts(self):
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            pinlabels=["A", "B", "C", "D"],
+            shorts=[["A", "B"], ["C", "D"]],
+        )
+        values = list(c.shorts.values())
+        assert values[0] == [1, 2]
+        assert values[1] == [3, 4]
+
+
+# --- Pin type coercion ---
+
+
+class TestPinTypeCoercion:
+    def test_str_numeric_pins_normalize_to_int(self):
+        c = make_connector(pins=["1", "2", "3"])
+        assert c.pins == [1, 2, 3]
+        assert all(isinstance(p, int) for p in c.pins)
+
+    def test_mixed_type_pins_normalize(self):
+        c = make_connector(pins=[1, "2", 3])
+        assert c.pins == [1, 2, 3]
+
+    def test_leading_zeros_normalize(self):
+        c = make_connector(pins=["01", "02", "03"])
+        assert c.pins == [1, 2, 3]
+
+    def test_non_numeric_pins_stay_str(self):
+        c = make_connector(pins=["A", "B", "C"])
+        assert c.pins == ["A", "B", "C"]
+        assert all(isinstance(p, str) for p in c.pins)
+
+    def test_duplicate_after_normalization_raises(self):
+        with pytest.raises(Exception, match="Pins are not unique"):
+            make_connector(pins=[1, "1"])
+
+    def test_pinlabels_normalize(self):
+        c = make_connector(pins=[1, 2], pinlabels=["10", "20"])
+        assert c.pinlabels == [10, 20]
+
+    def test_loop_pins_normalize(self):
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            loops=[["1", "2"]],
+        )
+        assert list(c.loops.values())[0] == [1, 2]
+
+    def test_str_loop_pins_match_auto_generated_int_pins(self):
+        """String loop pins match auto-generated sequential int pins."""
+        c = make_connector(
+            pincount=4,
+            loops=[["1", "3"]],
+        )
+        assert list(c.loops.values())[0] == [1, 3]
+
+    def test_short_pins_normalize(self):
+        c = make_connector(
+            pins=[1, 2, 3, 4],
+            shorts=[["1", "2"]],
+        )
+        assert list(c.shorts.values())[0] == [1, 2]
+
+    def test_wirelabels_normalize(self):
+        cable = Cable(
+            designator="W1", wirecount=3,
+            colors=["BK", "RD", "GN"],
+            wirelabels=["1", "2", "3"],
+        )
+        assert cable.wirelabels == [1, 2, 3]
+
+
+# --- Loop/Short rendering: non-sequential pins ---
+
+
+class TestRendering:
+    """Verify GraphViz output uses port indices, not pin numbers."""
+
+    def _make_harness(self):
+        from wireviz.wv_dataclasses import Metadata, Options, Tweak
+        from wireviz.wv_harness import Harness
+        return Harness(
+            metadata=Metadata({}),
+            options=Options(),
+            tweak=Tweak(),
+        )
+
+    def test_loop_non_sequential_pins_use_indices(self):
+        import re
+        harness = self._make_harness()
+        harness.add_connector(
+            "X1",
+            pins=[10, 20, 30, 40],
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            loops=[["VCC", "GND"]],
+        )
+        harness.add_cable("W1", wirecount=2, colors=["BK", "RD"])
+        harness.connect("X1", "TX", "W1", 1, None, None)
+        harness.connect(None, None, "W1", 2, "X1", "RX")
+        graph = harness.create_graph()
+        gv = graph.source
+
+        # Loop should reference p1 and p2 (indices), not p10 and p20
+        loop_edges = re.findall(r"X1:p(\d+)\w:\w -- X1:p(\d+)\w:\w", gv)
+        assert len(loop_edges) >= 1, f"Expected loop edge, found none in:\n{gv}"
+        idx_a, idx_b = loop_edges[0]
+        assert idx_a == "1" and idx_b == "2", (
+            f"Loop ports should be p1/p2 (indices), got p{idx_a}/p{idx_b}"
+        )
+
+    def test_short_non_sequential_pins_use_indices(self):
+        import re
+        harness = self._make_harness()
+        harness.add_connector(
+            "X1",
+            pins=[10, 20, 30, 40],
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            shorts=[["VCC", "GND"]],
+        )
+        harness.add_cable("W1", wirecount=2, colors=["BK", "RD"])
+        harness.connect("X1", "TX", "W1", 1, None, None)
+        harness.connect(None, None, "W1", 2, "X1", "RX")
+        graph = harness.create_graph()
+        gv = graph.source
+
+        # Short should reference p1j and p2j (indices), not p10j and p20j
+        short_edges = re.findall(r"X1:p(\d+)j:c -- X1:p(\d+)j:c", gv)
+        assert len(short_edges) >= 1, f"Expected short edge, found none in:\n{gv}"
+        idx_a, idx_b = short_edges[0]
+        assert idx_a == "1" and idx_b == "2", (
+            f"Short ports should be p1j/p2j (indices), got p{idx_a}j/p{idx_b}j"
+        )
+
+    def test_sequential_pins_still_work(self):
+        harness = self._make_harness()
+        harness.add_connector(
+            "X1",
+            pinlabels=["VCC", "GND", "TX", "RX"],
+            loops=[["VCC", "GND"]],
+        )
+        harness.add_cable("W1", wirecount=2, colors=["BK", "RD"])
+        harness.connect("X1", "TX", "W1", 1, None, None)
+        harness.connect(None, None, "W1", 2, "X1", "RX")
+        graph = harness.create_graph()
+        gv = graph.source
+        assert ":p1" in gv
+        assert ":p2" in gv
+
+
+# --- Harness.connect() delegation ---
+
+
+class TestHarnessConnectDelegation:
+    def _make_harness(self):
+        from wireviz.wv_dataclasses import Metadata, Options, Tweak
+        from wireviz.wv_harness import Harness
+        return Harness(
+            metadata=Metadata({}),
+            options=Options(),
+            tweak=Tweak(),
+        )
+
+    def test_connect_resolves_labels(self):
+        harness = self._make_harness()
+        harness.add_connector(
+            "X1", pins=[1, 2, 3], pinlabels=["VCC", "GND", "SIG"]
+        )
+        harness.add_connector("X2", pins=[1, 2, 3])
+        harness.add_cable("W1", wirecount=1, colors=["BK"])
+
+        harness.connect("X1", "SIG", "W1", 1, "X2", 1)
+
+        # The connection should store a PinClass with the resolved pin id (3)
+        conn = harness.cables["W1"]._connections[0]
+        assert conn.from_.id == 3
+
+    def test_connect_rejects_unknown_pin(self):
+        harness = self._make_harness()
+        harness.add_connector("X1", pins=[1, 2, 3])
+        harness.add_connector("X2", pins=[1, 2, 3])
+        harness.add_cable("W1", wirecount=1, colors=["BK"])
+
+        with pytest.raises(Exception, match="Unknown pin"):
+            harness.connect("X1", 99, "W1", 1, "X2", 1)
+
+    def test_connect_normalizes_string_pin(self):
+        """Programmatic callers passing str '3' should match normalized int 3."""
+        harness = self._make_harness()
+        harness.add_connector("X1", pins=[1, 2, 3])
+        harness.add_connector("X2", pins=[1, 2, 3])
+        harness.add_cable("W1", wirecount=1, colors=["BK"])
+
+        harness.connect("X1", "3", "W1", 1, "X2", 1)
+        conn = harness.cables["W1"]._connections[0]
+        assert conn.from_.id == 3
+
+
+# --- Apollo review: validation edge cases ---
+
+
+class TestApolloValidation:
+    """Tests added per Apollo code review findings."""
+
+    def test_partial_duplicate_loop_raises(self):
+        """C1: Loop [A, B, A] -> [1, 2, 1] has duplicate pin 1."""
+        with pytest.raises(Exception, match="duplicate pin"):
+            make_connector(
+                pins=[1, 2, 3],
+                pinlabels=["A", "B", "C"],
+                loops=[["A", "B", "A"]],
+            )
+
+    def test_partial_duplicate_short_raises(self):
+        """C1: Short with partial duplicates detected."""
+        with pytest.raises(Exception, match="duplicate pin"):
+            make_connector(
+                pins=[1, 2, 3],
+                pinlabels=["A", "B", "C"],
+                shorts=[["A", "B", "A"]],
+            )
+
+    def test_empty_loop_raises(self):
+        """C2: Empty loop has no physical meaning."""
+        with pytest.raises(Exception, match="at least 2 pins"):
+            make_connector(pins=[1, 2, 3], loops=[[]])
+
+    def test_single_pin_loop_raises(self):
+        """C2: Single-pin loop has no physical meaning."""
+        with pytest.raises(Exception, match="at least 2 pins"):
+            make_connector(pins=[1, 2, 3], loops=[[1]])
+
+    def test_empty_short_raises(self):
+        """C2: Empty short has no physical meaning."""
+        with pytest.raises(Exception, match="at least 2 pins"):
+            make_connector(pins=[1, 2, 3], shorts=[[]])
+
+    def test_single_pin_short_raises(self):
+        """C2: Single-pin short has no physical meaning."""
+        with pytest.raises(Exception, match="at least 2 pins"):
+            make_connector(pins=[1, 2, 3], shorts=[[1]])
+
+    def test_float_pin_raises(self):
+        """C3: Non-integer float pin should be rejected, not truncated."""
+        with pytest.raises(ValueError, match="Float"):
+            make_connector(pins=[3.5, 2, 3])
+
+    def test_bool_pin_raises(self):
+        """C3: Boolean pin should be rejected, not coerced to int."""
+        with pytest.raises(ValueError, match="Boolean"):
+            make_connector(pins=[True, 2, 3])
+
+    def test_integer_float_pin_accepted(self):
+        """C3: Integer-valued float (e.g. 3.0) should normalize to int 3."""
+        c = make_connector(pins=[1.0, 2.0, 3.0])
+        assert c.pins == [1, 2, 3]
+        assert all(isinstance(p, int) for p in c.pins)
+
+    def test_resolve_pin_normalizes_input(self):
+        """I1: resolve_pin() normalizes at its own boundary."""
+        c = make_connector(pins=[1, 2, 3])
+        assert c.resolve_pin("2") == 2