fix(task-7): instrument bubble outline per ISA-5.1 Table 5.4.1, location lines clipped to the outline

build_instrument drew a circle for every type, so `computer` shipped as
circle + hexagon and `plc` as circle + square + diamond (10 of the 20 bubble
blocks wrong, and the circle poked 0.67 mm outside the hexagon's flat top).
Spec 4.4: discrete = circle; dcs = circle inside a square; computer =
hexagon; plc = diamond inside a square. The outline is now chosen per type.

Location lines were always drawn from -r to +r, overshooting the hexagon's
slanted sides by 0.46 mm at y=+/-0.8 and the circle's chord by 0.06 mm.
They now end on the outline (`_bubble_half_width`): chord for the circle,
full width for the square, `r - |y| / tan 60` for the hexagon.

Tests now assert the exact primitive list per type and that every location
line stays on/inside its outline (both fail against the previous code).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd

engineering/pid/symbols.py

@@ -242,20 +242,35 @@ def make_spec(
 # ── instrument bubble (ISA-5.1-2009 Table 5.4.1) ─────────────────────────────
 
 
+def _bubble_half_width(type: str, y: float) -> float:
+    """Half-width of the instrument outline at height ``y`` so a location line
+    ends on the outline instead of overshooting it (circle: chord; square:
+    full width; flat-topped hexagon: ``r - |y| / tan 60``)."""
+    r = BUBBLE_RADIUS
+    if type == "discrete":
+        return math.sqrt(max(r * r - y * y, 0.0))
+    if type == "computer":
+        return r - abs(y) / math.tan(math.radians(60.0))
+    return r  # dcs / plc: the square is the outline
+
+
 def build_instrument(type: str = "discrete", location: str = "field") -> SymbolSpec:
     if type not in INSTRUMENT_TYPES:
         raise ValueError(f"instrument: type must be one of {', '.join(INSTRUMENT_TYPES)}")
     if location not in INSTRUMENT_LOCATIONS:
         raise ValueError(f"instrument: location must be one of {', '.join(INSTRUMENT_LOCATIONS)}")
     r = BUBBLE_RADIUS
-    prims: list[dict] = [circle(0, 0, r)]
     square = polyline([(-r, -r), (r, -r), (r, r), (-r, r)])
-    if type == "dcs":
-        prims.append(square)
+    # Outline per type (spec 4.4): discrete = circle; dcs = circle inside a square;
+    # computer = hexagon; plc = diamond inside a square -- no circle in the last two.
+    if type == "discrete":
+        prims: list[dict] = [circle(0, 0, r)]
+    elif type == "dcs":
+        prims = [circle(0, 0, r), square]
     elif type == "computer":
-        prims.append(polyline(regular_polygon(0, 0, r, 6, 0.0)))
-    elif type == "plc":
-        prims += [square, polyline([(0, r), (r, 0), (0, -r), (-r, 0)])]
+        prims = [polyline(regular_polygon(0, 0, r, 6, 0.0))]
+    else:  # plc
+        prims = [square, polyline([(0, r), (r, 0), (0, -r), (-r, 0)])]
     ys = {
         "field": [],
         "primary": [0.0],
@@ -264,7 +279,8 @@ def build_instrument(type: str = "discrete", location: str = "field") -> SymbolS
         "auxiliary_rear": [0.8, -0.8],
     }[location]
     for y in ys:
-        prims += dashed_line(-r, y, r, y) if location.endswith("_rear") else [line(-r, y, r, y)]
+        hw = _bubble_half_width(type, y)
+        prims += dashed_line(-hw, y, hw, y) if location.endswith("_rear") else [line(-hw, y, hw, y)]
     attdefs = (
         attdef("FUNC", 0.0, 2.0, DESC_HEIGHT, align="middle_center"),
         attdef("LOOP", 0.0, -2.0, DESC_HEIGHT, align="middle_center"),

tests/test_pid_symbols.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import math
 import re
 
 import pytest
@@ -111,6 +112,56 @@ def test_instrument_generator_covers_four_types_by_five_locations():
     assert len(hexagon["points"]) == 6
 
 
+def test_instrument_outline_per_type_follows_isa_table_5_4_1():
+    """Spec 4.4: discrete = circle; dcs = circle inside a square; computer = hexagon;
+    plc = diamond inside a square -- no circle in the last two."""
+    outline = {
+        t: [p["type"] for p in resolve("instrument", type=t, location="field").primitives]
+        for t in INSTRUMENT_TYPES
+    }
+    assert outline == {
+        "discrete": ["circle"],
+        "dcs": ["circle", "polyline"],
+        "computer": ["polyline"],
+        "plc": ["polyline", "polyline"],
+    }
+    plc = resolve("instrument", type="plc", location="field").primitives
+    assert sorted(len(p["points"]) for p in plc) == [4, 4], "square + diamond"
+
+
+def test_instrument_location_lines_end_on_the_outline():
+    """A location line must not poke outside the bubble outline: chord of the circle,
+    full width of the square, clipped to the flat-topped hexagon's slanted sides."""
+    r = BUBBLE_RADIUS
+    half_width = {
+        "discrete": lambda y: math.sqrt(r * r - y * y),
+        "dcs": lambda y: r,
+        "plc": lambda y: r,
+        "computer": lambda y: r - abs(y) / math.tan(math.radians(60.0)),
+    }
+    for t in INSTRUMENT_TYPES:
+        for loc in INSTRUMENT_LOCATIONS:
+            spec = resolve("instrument", type=t, location=loc)
+            lines = [p for p in spec.primitives if p["type"] == "line"]
+            ys = {p["y1"] for p in lines}
+            expected_ys = {"field": set(), "primary": {0.0}, "auxiliary": {0.8, -0.8}}[
+                loc.removesuffix("_rear")
+            ]
+            assert ys == expected_ys, (t, loc)
+            for p in lines:
+                assert p["y1"] == p["y2"], (t, loc)
+                hw = half_width[t](p["y1"])
+                assert -hw - 1e-9 <= min(p["x1"], p["x2"]), (t, loc, p)
+                assert max(p["x1"], p["x2"]) <= hw + 1e-9, (t, loc, p)
+            # a solid line spans the whole outline (not merely stays inside it); a dashed
+            # one starts on it and its last dash may stop short of the far side
+            if lines:
+                hw = half_width[t](next(iter(ys)))
+                assert min(p["x1"] for p in lines) == pytest.approx(-hw)
+                if not loc.endswith("_rear"):
+                    assert max(p["x2"] for p in lines) == pytest.approx(hw)
+
+
 def test_unknown_symbol_or_option_names_the_valid_values():
     with pytest.raises(ValueError, match="instrument"):
         resolve("thermometer")