feat(pid): pid_symbol_list and pid_symbol_insert — blocks with tags, ports and XDATA

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

README.md

@@ -19,7 +19,7 @@ Live through COM on Windows, or headless through ezdxf anywhere — one typed co
 
 </div>
 
-> **v1.5 release snapshot:** 157 tools · 6 resources · 5 prompt templates · 1369 collected tests.
+> **v1.5 release snapshot:** 159 tools · 6 resources · 5 prompt templates · 1369 collected tests.
 > 154 is the **registered** count; a default install advertises 149 over `tools/list`,
 > because `ENABLE_3D` is unset. `system_about` is the runtime authority.
 

discovery/aliases.py

@@ -60,9 +60,9 @@ class ToolAliases:
 #       commands in AutoCAD itself; this server splits each into discrete tools.
 #   SETVAR -- one command both reads and writes a system variable; the server
 #       splits read and write.
-#   ERASE, INSERT -- the server genuinely exposes one operation through two
-#       tools (single vs. batch delete; block insert vs. block-reference
-#       creation), so both are correct destinations.
+#   ERASE, INSERT -- the server genuinely exposes one operation through more
+#       than one tool (single vs. batch delete; block insert vs. block-reference
+#       creation vs. the P&ID catalogue insert), so each is a correct destination.
 #   QSELECT -- AutoCAD's Quick Select dialog is both "filter by properties"
 #       (selection_filter) and "find things like this one"
 #       (entity_select_smart); a drafter typing it could mean either.
@@ -1344,6 +1344,29 @@ TOOL_ALIASES: dict[str, ToolAliases] = {
         acad=("ZOOM",),
         synonyms=("zoom into a region", "close up of an area", "magnify a rectangle"),
     ),
+    # ── P&ID ────────────────────────────────────────────────────────────────
+    "pid_symbol_list": ToolAliases(
+        acad=(),
+        synonyms=(
+            "p&id symbols",
+            "pid symbol catalogue",
+            "which valves can i draw",
+            "instrument bubble types",
+        ),
+    ),
+    "pid_symbol_insert": ToolAliases(
+        acad=("INSERT",),
+        synonyms=(
+            "p&id symbol",
+            "insert a valve",
+            "place a pump",
+            "draw a vessel",
+            "instrument bubble",
+            "control valve",
+            "off-page connector",
+            "process equipment symbol",
+        ),
+    ),
 }
 
 

docs/tool-inventory.json

@@ -3,10 +3,10 @@
   "schema_version": 1,
   "package_version": "1.5.1",
   "totals": {
-    "tools": 157,
+    "tools": 159,
     "resources": 6,
     "prompts": 5,
-    "groups": 19
+    "groups": 20
   },
   "groups": {
     "analysis": 12,
@@ -21,6 +21,7 @@
     "entity_query": 9,
     "layers": 14,
     "layouts": 12,
+    "pid": 2,
     "premium": 12,
     "solids": 5,
     "system": 7,
@@ -1716,6 +1717,34 @@
         "readOnlyHint": false
       }
     },
+    {
+      "name": "pid_symbol_insert",
+      "group": "pid",
+      "cost": "mutate",
+      "summary": "Place a P&ID symbol (valve, pump, vessel, instrument bubble, connector) as a tagged block with ports.",
+      "tags": [
+        "create",
+        "pid"
+      ],
+      "annotations": {
+        "title": "P&ID: Insert Symbol",
+        "readOnlyHint": false
+      }
+    },
+    {
+      "name": "pid_symbol_list",
+      "group": "pid",
+      "cost": "read",
+      "summary": "List the P&ID symbol catalogue: families, variants, ports, parameters.",
+      "tags": [
+        "pid",
+        "query"
+      ],
+      "annotations": {
+        "title": "P&ID: Symbol Catalogue",
+        "readOnlyHint": true
+      }
+    },
     {
       "name": "point_from_snap",
       "group": "premium",

engineering/layers.py

@@ -36,6 +36,7 @@ PID_LAYERS: list[tuple[str, int, str, float, str]] = [
     ("INSTRUMENT-SYMBOL", 4, "Continuous", 0.35, "instrument bubbles / FCFs"),
     ("INSTRUMENT-LINE-SIGNAL", 4, "DASHED", 0.18, "pneumatic / electrical signal"),
     ("INSTRUMENT-TAG-TEXT", 7, "Continuous", 0.25, "tag labels"),
+    ("PROCESS-LINE-TEXT", 7, "Continuous", 0.25, "line numbers"),
     ("ELECTRICAL-LINE", 6, "DASHDOT", 0.18, "electrical line"),
     ("UTILITY-LINE", 5, "PHANTOM", 0.25, "utility (steam, water, air)"),
     ("INSULATION-HATCH", 8, "Continuous", 0.13, "insulation hatch"),

engineering/pid/__init__.py

@@ -1 +1,12 @@
 """P&ID expert and reader: symbol catalogue, lines, ISA-5.1 tags, graph."""
+
+from .insert import ensure_block, ensure_layer, insert_symbol, place_symbol  # noqa: F401
+from .symbols import (  # noqa: F401
+    CATALOG_VERSION,
+    Port,
+    SymbolSpec,
+    all_specs,
+    list_symbols,
+    resolve,
+)
+from .tags import parse_tag, split_instrument_tag  # noqa: F401

engineering/pid/insert.py

@@ -0,0 +1,129 @@
+"""Placing catalogue symbols: define the block on first use, insert with
+attributes, write the ACADMCP_PID payload, return the ports in WCS."""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+from engineering.layers import PID_LAYERS
+
+from .symbols import FAMILY_LAYER, SymbolSpec, resolve, transform_port
+from .tags import parse_tag, split_instrument_tag
+from .xdata import symbol_payload, write_payload
+
+if TYPE_CHECKING:
+    from backends.base import AutoCADBackend
+
+FAIL_POSITIONS = ("", "FO", "FC", "FL")
+_PID_LAYER_DEFS = {name: (color, linetype, lw) for name, color, linetype, lw, _desc in PID_LAYERS}
+
+
+async def ensure_layer(backend: AutoCADBackend, layer_name: str) -> bool:
+    """Create ``layer_name`` if missing — from PID_LAYERS when it is one of ours."""
+    existing = {lyr.name.lower() for lyr in await backend.layer_list()}
+    if layer_name.lower() in existing:
+        return False
+    color, linetype, lineweight = _PID_LAYER_DEFS.get(layer_name, (7, "Continuous", 0.25))
+    await backend.layer_create(
+        name=layer_name, color=color, linetype=linetype, lineweight=lineweight
+    )
+    return True
+
+
+async def ensure_block(backend: AutoCADBackend, spec: SymbolSpec) -> bool:
+    """Define ``spec``'s block if the drawing does not have it yet."""
+    names = {blk.name for blk in await backend.block_list()}
+    if spec.name in names:
+        return False
+    await backend.block_define(
+        spec.name, list(spec.primitives), list(spec.attdefs), 0.0, 0.0, False
+    )
+    return True
+
+
+async def insert_symbol(
+    backend: AutoCADBackend,
+    spec: SymbolSpec,
+    x: float,
+    y: float,
+    rotation: float = 0.0,
+    scale: float = 1.0,
+    attributes: dict | None = None,
+    layer: str | None = None,
+    payload: dict | None = None,
+) -> dict:
+    layer_name = layer or FAMILY_LAYER[spec.family]
+    if layer_name == "line":
+        raise ValueError(f"{spec.name}: markers need an explicit layer (the line's)")
+    created = await ensure_layer(backend, layer_name)
+    defined = await ensure_block(backend, spec)
+    values = {att["tag"]: att["default"] for att in spec.attdefs}
+    values.update({k: str(v) for k, v in (attributes or {}).items()})
+    ref = await backend.block_insert(
+        spec.name, x, y, scale, scale, rotation, values or None, layer_name
+    )
+    await write_payload(backend, ref.handle, payload or symbol_payload(spec))
+    ports = {p.name: transform_port(p, x, y, rotation, scale) for p in spec.ports}
+    return {
+        "handle": ref.handle,
+        "block_name": spec.name,
+        "defined": defined,
+        "layers_created": [layer_name] if created else [],
+        "ports": ports,
+        "layer": layer_name,
+    }
+
+
+async def place_symbol(
+    backend: AutoCADBackend,
+    symbol: str,
+    x: float,
+    y: float,
+    rotation: float = 0.0,
+    scale: float = 1.0,
+    tag: str | None = None,
+    desc: str | None = None,
+    fail: str | None = None,
+    link: str | None = None,
+    layer: str | None = None,
+    **options,
+) -> dict:
+    """The ``pid_symbol_insert`` tool body."""
+    if scale <= 0:
+        raise ValueError("scale must be > 0")
+    spec = resolve(symbol, **options)
+    if spec.family == "marker":
+        raise ValueError(f"{symbol} is a line marker; pid_line_draw places markers and arrows")
+    tags_present = {att["tag"] for att in spec.attdefs}
+    attributes: dict[str, str] = {}
+    warnings: list[str] = []
+    tag_text: str | None = None
+    if tag:
+        tag_text = tag.strip()
+        if spec.family == "instrument":
+            parsed = parse_tag(tag_text, kind="instrument")
+            func, loop = split_instrument_tag(tag_text)
+            attributes.update(FUNC=func, LOOP=loop)
+        else:
+            parsed = parse_tag(tag_text, kind="auto")
+            attributes["TAG"] = tag_text
+        if not parsed["valid"]:
+            warnings.extend(parsed["errors"])
+    if desc is not None:
+        if "DESC" not in tags_present:
+            warnings.append(f"{symbol} has no DESC attribute; description ignored")
+        else:
+            attributes["DESC"] = desc
+    if fail is not None:
+        if "FAIL" not in tags_present:
+            raise ValueError(f"{symbol}: 'fail' needs an actuator (no FAIL attribute)")
+        if fail.upper() not in FAIL_POSITIONS:
+            raise ValueError("fail must be one of FO, FC, FL (or empty)")
+        attributes["FAIL"] = fail.upper()
+    if link is not None:
+        if "LINK" not in tags_present:
+            raise ValueError(f"{symbol}: 'link' is only valid for offpage connectors")
+        attributes["LINK"] = link
+    result = await insert_symbol(backend, spec, x, y, rotation, scale, attributes, layer)
+    result.update(tag=tag_text, tag_warnings=warnings, backend=backend.name)
+    return result

server.py

@@ -689,6 +689,7 @@ async def _registered_tool_count() -> int | None:
 # specific tags (engineering/premium/corner) take precedence over the broad
 # entity/drawing tags. Tools whose tags match none fall into "other".
 _GROUP_TAG_PRIORITY = (
+    "pid",
     "engineering",
     "premium",
     "corner",
@@ -713,6 +714,7 @@ _GROUP_TAG_PRIORITY = (
 
 # Map the winning tag to a human-readable group label for the breakdown.
 _GROUP_TAG_LABELS = {
+    "pid": "pid",
     "engineering": "engineering",
     "premium": "premium",
     "corner": "corner_ops",
@@ -6354,6 +6356,113 @@ async def solid_boolean(
     return await _backend(ctx).solid_boolean(target_handle, tool_handle, operation)
 
 
+# ---------------------------------------------------------------------------
+# ── SECTION 17: P&ID (2 tools) ──────────────────────────────────────────────
+# ---------------------------------------------------------------------------
+
+
+@cad_tool(
+    summary="List the P&ID symbol catalogue: families, variants, ports, parameters.",
+    cost="read",
+)
+@mcp.tool(
+    annotations={"title": "P&ID: Symbol Catalogue", "readOnlyHint": True},
+    tags={"pid", "query"},
+)
+async def pid_symbol_list(
+    family: Annotated[
+        str | None,
+        "valve | rotating | vessel | heat | misc | instrument | connector | marker; omit for all",
+    ] = None,
+    ctx: Context = None,
+) -> dict:
+    """The symbols `pid_symbol_insert` can place, with their ports and variant options.
+
+    Authored from ISO 10628-2 (equipment, valves) and ISA-5.1 (instrumentation);
+    each row's `source` names the figure. Symbol-local sizes are millimetres at
+    A3/A1 paper scale (bubble Ø10, valve 8×4); `scale` at insertion rescales.
+    """
+    from engineering.pid.symbols import CATALOG_VERSION, list_symbols
+
+    return {"catalog_version": CATALOG_VERSION, "symbols": list_symbols(family)}
+
+
+@cad_tool(
+    summary=(
+        "Place a P&ID symbol (valve, pump, vessel, instrument bubble, connector) "
+        "as a tagged block with ports."
+    ),
+    cost="mutate",
+)
+@mcp.tool(
+    annotations={"title": "P&ID: Insert Symbol", "readOnlyHint": False},
+    tags={"pid", "create"},
+)
+async def pid_symbol_insert(
+    symbol: Annotated[
+        str,
+        "Catalogue symbol name, e.g. gate, centrifugal_pump, vertical_vessel, instrument, offpage",
+    ],
+    x: Annotated[float, "Insertion X (WCS)"],
+    y: Annotated[float, "Insertion Y (WCS)"],
+    rotation: Annotated[float, "Rotation in degrees; 0 = flow left to right"] = 0.0,
+    scale: Annotated[float, Field(default=1.0, gt=0, description="Uniform scale")] = 1.0,
+    tag: Annotated[
+        str | None,
+        "Tag: equipment P-101, valve HV-101, instrument FIC-101 (split into FUNC/LOOP)",
+    ] = None,
+    desc: Annotated[str | None, "Description attribute"] = None,
+    actuator: Annotated[
+        str | None, "Valves: none | diaphragm | piston | motor | solenoid | hand"
+    ] = None,
+    fail: Annotated[str | None, "Actuated valves: FO | FC | FL"] = None,
+    type: Annotated[str | None, "Instruments: discrete | dcs | computer | plc"] = None,
+    location: Annotated[
+        str | None,
+        "Instruments: field | primary | auxiliary | primary_rear | auxiliary_rear",
+    ] = None,
+    params: Annotated[
+        dict | None,
+        "Parametric vessels: {width, height, nozzles: [{name, side, fraction}], roof, trays, jacketed}",
+    ] = None,
+    shape: Annotated[str | None, "Reducer: concentric | eccentric"] = None,
+    direction: Annotated[str | None, "Off-page connector: in | out"] = None,
+    link: Annotated[str | None, "Off-page connector link id shared by both ends"] = None,
+    layer: Annotated[str | None, "Override the family layer"] = None,
+    ctx: Context = None,
+) -> dict:
+    """Insert a catalogue symbol as a real block with TAG attributes and named ports.
+
+    Defines the block on first use (`defined`), creates a missing family layer
+    from the P&ID layer set (`layers_created`), writes an ACADMCP_PID payload
+    so readers without the catalogue still see the ports, and returns every
+    port in WCS — connect them with `pid_line_draw`. An invalid ISA-5.1 tag is
+    written and reported in `tag_warnings`; `drawing_critique` flags it.
+    """
+    from engineering.pid.insert import place_symbol
+
+    await ctx.info(f"P&ID symbol {symbol} at ({x}, {y})")
+    return await place_symbol(
+        _backend(ctx),
+        symbol,
+        x,
+        y,
+        rotation,
+        scale,
+        tag,
+        desc,
+        fail,
+        link,
+        layer,
+        actuator=actuator,
+        type=type,
+        location=location,
+        params=params,
+        shape=shape,
+        direction=direction,
+    )
+
+
 # ---------------------------------------------------------------------------
 # ── RESOURCES ───────────────────────────────────────────────────────────────
 # ---------------------------------------------------------------------------

tests/test_pid_insert.py

@@ -0,0 +1,104 @@
+"""pid_symbol_insert: define on first use, insert with attributes, WCS ports, XDATA."""
+
+from __future__ import annotations
+
+import pytest
+
+from engineering.pid.insert import ensure_layer, place_symbol
+from engineering.pid.symbols import resolve
+from engineering.pid.xdata import read_payload
+
+pytestmark = pytest.mark.asyncio
+
+
+async def test_first_insert_defines_the_block_and_creates_its_layer(backend):
+    result = await place_symbol(backend, "gate", 100.0, 50.0, tag="HV-101")
+    assert result["defined"] is True and result["layers_created"] == ["PROCESS-VALVES"]
+    assert result["block_name"] == "PID_VALVE_GATE" and result["layer"] == "PROCESS-VALVES"
+    assert result["ports"]["in"] == {
+        "name": "in",
+        "x": 96.0,
+        "y": 50.0,
+        "direction_deg": 180.0,
+        "kind": "process",
+        "radius": 0.0,
+        "inferred": False,
+    }
+    assert result["tag"] == "HV-101" and result["tag_warnings"] == []
+    assert await backend.block_get_attributes(result["handle"]) == {"TAG": "HV-101", "DESC": ""}
+    again = await place_symbol(backend, "gate", 200.0, 50.0, tag="HV-102")
+    assert again["defined"] is False and again["layers_created"] == []
+
+
+async def test_rotation_and_scale_move_the_ports(backend):
+    result = await place_symbol(
+        backend, "centrifugal_pump", 0.0, 0.0, rotation=90.0, scale=2.0, tag="P-101"
+    )
+    discharge = result["ports"]["discharge"]
+    assert (discharge["x"], discharge["y"], discharge["direction_deg"]) == pytest.approx(
+        (-12.0, 0.0, 180.0)
+    )
+
+
+async def test_instrument_tag_splits_into_func_and_loop(backend):
+    result = await place_symbol(
+        backend, "instrument", 10, 10, tag="10-FIC-101A", type="dcs", location="primary"
+    )
+    assert await backend.block_get_attributes(result["handle"]) == {"FUNC": "FIC", "LOOP": "101A"}
+    assert result["block_name"] == "PID_INST_DCS_PRIMARY" and result["layer"] == "INSTRUMENT-SYMBOL"
+    assert result["ports"]["signal"]["radius"] == 5.0
+    assert result["ports"]["signal"]["direction_deg"] is None
+
+
+async def test_invalid_tag_is_written_and_warned_not_refused(backend):
+    result = await place_symbol(backend, "instrument", 0, 0, tag="FCI-9")
+    assert await backend.block_get_attributes(result["handle"]) == {"FUNC": "FCI", "LOOP": "9"}
+    assert result["tag_warnings"] and "I" in result["tag_warnings"][0]
+
+
+async def test_actuator_fail_and_offpage_link_attributes(backend):
+    cv = await place_symbol(backend, "globe", 0, 0, tag="FCV-105", actuator="diaphragm", fail="FC")
+    assert (await backend.block_get_attributes(cv["handle"]))["FAIL"] == "FC"
+    assert "signal" in cv["ports"]
+    with pytest.raises(ValueError, match="FO, FC, FL"):
+        await place_symbol(backend, "globe", 0, 0, actuator="diaphragm", fail="XX")
+    off = await place_symbol(
+        backend, "offpage", 0, 0, tag="TO P&ID-002", link="L-17", direction="out"
+    )
+    assert (await backend.block_get_attributes(off["handle"]))["LINK"] == "L-17"
+    with pytest.raises(ValueError, match="link"):
+        await place_symbol(backend, "gate", 0, 0, link="L-1")
+
+
+async def test_xdata_payload_carries_ports_and_params(backend):
+    result = await place_symbol(
+        backend, "vertical_vessel", 0, 0, tag="V-201", params={"width": 20, "height": 40}
+    )
+    payload = await read_payload(backend, result["handle"])
+    assert payload["kind"] == "symbol" and payload["catalog"] == "1"
+    assert payload["params"]["width"] == 20.0
+    assert set(payload["ports"]) == {"N1", "N2", "N3", "N4"}
+
+
+async def test_refusals(backend):
+    with pytest.raises(ValueError, match="scale"):
+        await place_symbol(backend, "gate", 0, 0, scale=0)
+    with pytest.raises(ValueError, match="pid_line_draw"):
+        await place_symbol(backend, "arrow_flow", 0, 0)
+    with pytest.raises(ValueError, match="params"):
+        await place_symbol(backend, "gate", 0, 0, params={"width": 1})
+    with pytest.raises(ValueError, match="actuator"):
+        await place_symbol(backend, "check", 0, 0, actuator="motor")
+
+
+async def test_layer_override_and_ensure_layer(backend):
+    assert await ensure_layer(backend, "PROCESS-LINE-TEXT") is True
+    assert await ensure_layer(backend, "PROCESS-LINE-TEXT") is False
+    layers = {lyr.name: lyr for lyr in await backend.layer_list()}
+    assert layers["PROCESS-LINE-TEXT"].lineweight == 25.0, (
+        "LayerInfo reports DXF hundredths of a millimetre"
+    )
+    result = await place_symbol(backend, "gate", 0, 0, layer="MY-VALVES")
+    assert result["layer"] == "MY-VALVES" and result["layers_created"] == ["MY-VALVES"]
+    spec = resolve("gate")
+    assert spec.name in {b.name for b in await backend.block_list()}

tests/test_tool_registry.py

@@ -495,7 +495,7 @@ async def test_tool_groups_is_byte_identical_to_the_source_declared_grouping():
 
 
 async def test_tool_group_sizes_are_unchanged():
-    """Frozen snapshot of the surface (157 tools, 19 groups).
+    """Frozen snapshot of the surface (159 tools, 20 groups).
 
     Taken before @cad_tool landed at 131 tools; `batch` moved 2 -> 3 when
     v1.5.0's `cad_batch` joined `entity_batch_create`/`entity_batch_modify`, and
@@ -507,8 +507,12 @@ async def test_tool_group_sizes_are_unchanged():
     both engines) joined SECTION 7. `entity_query` moved 8 -> 9 and
     `entity_modification` 15 -> 16 when v1.6's `entity_get_xdata` /
     `entity_set_xdata` joined SECTION 5 (the getter is tagged `query`, the
-    setter `modify`, so the pair splits across two groups). Every other number
-    here has been unchanged since the snapshot was taken.
+    setter `modify`, so the pair splits across two groups). `pid` appeared
+    (0 -> 2) when v1.6's SECTION 17 opened with `pid_symbol_list` /
+    `pid_symbol_insert`; the `pid` tag is ranked first in
+    `_GROUP_TAG_PRIORITY` so those tools file under `pid` rather than under
+    their secondary `query` / `create` tags. Every other number here has been
+    unchanged since the snapshot was taken.
     """
     sizes = {label: len(names) for label, names in (await server._tool_groups()).items()}
     assert sizes == {
@@ -524,6 +528,7 @@ async def test_tool_group_sizes_are_unchanged():
         "entity_query": 9,
         "layers": 14,
         "layouts": 12,
+        "pid": 2,
         "premium": 12,
         "solids": 5,
         "system": 7,
@@ -532,4 +537,4 @@ async def test_tool_group_sizes_are_unchanged():
         "validation": 1,
         "view": 4,
     }
-    assert sum(sizes.values()) == 157
+    assert sum(sizes.values()) == 159